Skip to content
This repository has been archived by the owner on May 4, 2022. It is now read-only.

MUMUP-2875 Fixes bug with dataObject in notification feed caused it to not display #387

Merged
merged 2 commits into from
Mar 30, 2017

Conversation

vertein
Copy link
Contributor

@vertein vertein commented Mar 30, 2017

Fixes a bug where the data object to find to show notification would return but we were checking if length > 0. Well, length>0 is only valid when it's an array, so if it was just an object, it would fail and not show the notification. Added test case as well.


Contributor License Agreement adherence:

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 39.032% when pulling a742eb4 on vertein:fixBugPersonalNotification into bf6e3d8 on UW-Madison-DoIT:master.

Copy link
Contributor

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

return notification;
}
} else if (objectToFind) {
return notification
Copy link
Contributor

@ChristianMurphy ChristianMurphy Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎨 should the return statement be closed with a semicolon?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how I could have missed that!

Copy link
Contributor

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎨 👍

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 39.032% when pulling bd853ed on vertein:fixBugPersonalNotification into bf6e3d8 on UW-Madison-DoIT:master.

@vertein vertein merged commit c4397da into uPortal-Attic:master Mar 30, 2017
@vertein vertein deleted the fixBugPersonalNotification branch March 30, 2017 22:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants