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

Update configuration to use new messages model #674

Merged
merged 6 commits into from
Aug 15, 2017
Merged

Update configuration to use new messages model #674

merged 6 commits into from
Aug 15, 2017

Conversation

thevoiceofzeke
Copy link
Contributor

@thevoiceofzeke thevoiceofzeke commented Aug 9, 2017

Related to: uw-frame#484
Related to: angularjs-portal overlay updates

In this PR:

  • Update override paths to use new app-config
  • Update static messages source

Contributor License Agreement adherence:

'groupURL': '/portal/api/groups',
'kvURL': '/storage',
'loginSilentURL': '/portal/Login?silent=true&profile=bucky',
},
'MESSAGES': {
'announcementsEnabled': true,
'notificationsEnabled': true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need these flags, or could the enablement of announcements and notifications be implied by whether there are announcement-flavored and notification-flavored items in the messages feed? If there's no announcements in the feed, then they're not enabled. If there's no notifications in the feed, then they're not enabled.

'MESSAGES': {
'announcementsEnabled': true,
'notificationsEnabled': true,
'groupFiltering': true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this flag, or could group filtering be implied by whether there are groups in the items in the messages feed? If groups are specified, filter to them. If they're not, don't worry about it.

Copy link
Contributor Author

@thevoiceofzeke thevoiceofzeke Aug 10, 2017

Choose a reason for hiding this comment

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

¯\_(ツ)_/¯

Valid questions, but I don't feel confident that the decision to remove the flags should be up to me. As it is, I've just repurposed what was already there.

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, thanks @thevoiceofzeke! 👍

@ChristianMurphy
Copy link
Contributor

ChristianMurphy commented Aug 14, 2017

@thevoiceofzeke CI failure is coming for RemarkLint.
Cherry-picking or applying change from 372b343 should resolve.

edit: opened #675 to resolve issue.

@thevoiceofzeke thevoiceofzeke merged commit 622aa14 into uPortal-Attic:master Aug 15, 2017
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.

4 participants