Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Extend notifications to be marked as read or unread #2549

Merged
merged 6 commits into from Mar 23, 2018
Merged

Extend notifications to be marked as read or unread #2549

merged 6 commits into from Mar 23, 2018

Conversation

voodoorai2000
Copy link
Member

@voodoorai2000 voodoorai2000 commented Mar 23, 2018 •
edited

References

Issue: #2478
Backport: AyuntamientoMadrid#1304

Objectives

  • Add links to mark notifications as read or unread
  • Displays notifications in two tabs (Unread, All Notifications)
  • Notifications are no longer deleted when read, instead, they are just marked as read
  • Refactor notification item in header into a partial
  • Refactor parameters to use objects instead of ids, a little simpler to read and same performance
  • Increase test coverage for existing features
  • Add notifications for all users in dev seeds

Visual Changes

New notifications

notifications_new

All notifications

notifications_all

Mobile size

notifications_mobile

Notes

Existing notifications will be kept as unread.

Enjoy! 😌

voodoorai2000 and others added 6 commits March 23, 2018 11:47
@voodoorai2000
Extend notifications to be marked as read and unread
09c44ee
@voodoorai2000
Extend dev seeds to have notifications for all users
f7bfe5e 
Even though an action that triggers a notification is made, the
notification is created in a separate step, reflecting how it is done
in the corresponding controller

https://github.com/AyuntamientoMadrid/consul/blob/master/app/controllers
/comments_controller.rb#L16
@decabeza @voodoorai2000
Adds styles for notifications views
6824086
@bertocq @voodoorai2000
Add Notifications partial to admin menu
92c9ad4
@voodoorai2000
Remove unrelated budget recommendation's link
b35c750 
During the backport for “Read Notifications”[1] this link was added,
which belongs to a different backport “Budget Recommendations” which is
not quite ready to bring to upstream, yet 😌

[1] AyuntamientoMadrid#1304
@voodoorai2000
Fix dev seed specs
c9cad05
@voodoorai2000 voodoorai2000 changed the title [WIP] Read notifications Read notifications Mar 23, 2018
@voodoorai2000 voodoorai2000 changed the title Read notifications Extend notifications to be marked as read or unread Mar 23, 2018
@voodoorai2000 voodoorai2000 merged commit 0300d97 into master Mar 23, 2018
@voodoorai2000 voodoorai2000 deleted the read_notifications branch March 23, 2018 21:30
@voodoorai2000 voodoorai2000 mentioned this pull request Mar 23, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bertocq bertocq bertocq approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@voodoorai2000 @bertocq @decabeza