Right now the experimental user menu sorts notifications the same way that the old menu does: unread high-priority notifications are shown first in reverse-chronological order followed by everything else also in reverse-chronological order. However, since the experimental user menu has dedicated tabs for some notification types and each tab displays a badge with the count of unread notifications in the tab, we feel like it makes sense to change how notifications are sorted in the experimental user menu to this:
1. unread high-priority notifications
2. unread regular notifications
3. all read notifications (both high-priority and regular)
4. within each group, notifications are sorted in reverse-chronological order (i.e. newest is shown first).
This new sorting logic applies to all tabs in the experimental user menu, however it doesn't change anything in the old menu. With this change, if a tab in the experimental user menu shows an unread notification badge for a really old notification, it will be surfaced to the top and prevents confusing scenarios where a user sees an unread notification badge on a tab, but the tab doesn't show the unread notification because it's too old to make it to the list.
Internal topic: t72199.
Some of the changes in this PR are extracted from https://github.com/discourse/discourse/pull/17379.
Similar to the bookmarks tab in the new user menu, the messages tab also displays a mix of notifications and messages. When there are unread message notifications, the tab displays all of these notifications at the top and fills the remaining space in the menu with a list of the user's messages. The bubble/badge count on the messages tab indicates how many unread message notifications there are.
* REFACTOR: Improve support for consolidating notifications.
Before this commit, we didn't have a single way of consolidating notifications. For notifications like group summaries, we manually removed old ones before creating a new one. On the other hand, we used an after_create callback for likes and group membership requests, which caused unnecessary work, as we need to delete the record we created to replace it with a consolidated one.
We now have all the consolidation rules centralized in a single place: the consolidation planner class. Other parts of the app looking to create a consolidable notification can do so by calling Notification#consolidate_or_save!, instead of the default Notification#create! method.
Finally, we added two more rules: one for re-using existing group summaries and another for deleting duplicated dashboard problems PMs notifications when the user is tracking the moderator's inbox. Setting the threshold to one forces the planner to apply this rule every time.
I plan to add plugin support for adding custom rules in another PR to keep this one relatively small.
* DEV: Introduces a plugin API for consolidating notifications.
This commit removes the `Notification#filter_by_consolidation_data` scope since plugins could have to define their criteria. The Plan class now receives two blocks, one to query for an already consolidated notification, which we'll try to update, and another to query for existing ones to consolidate.
It also receives a consolidation window, which accepts an ActiveSupport::Duration object, and filter notifications created since that value.
Introduce the concept of "high priority notifications" which include PM and bookmark reminder notifications. Now bookmark reminder notifications act in the same way as PM notifications (float to top of recent list, show in the green bubble) and most instances of unread_private_messages in the UI have been replaced with unread_high_priority_notifications.
The user email digest is changed to just have a section about unread high priority notifications, the unread PM section has been removed.
A high_priority boolean column has been added to the Notification table and relevant indices added to account for it.
unread_private_messages has been kept on the User model purely for backwards compat, but now just returns unread_high_priority_notifications count so this may cause some inconsistencies in the UI.
* This PR implements the scheduling and notification system for bookmark reminders. Every 5 minutes a schedule runs to check any reminders that need to be sent before now, limited to **300** reminders at a time. Any leftover reminders will be sent in the next run. This is to avoid having to deal with fickle sidekiq and reminders in the far-flung future, which would necessitate having a background job anyway to clean up any missing `enqueue_at` reminders.
* If a reminder is sent its `reminder_at` time is cleared and the `reminder_last_sent_at` time is filled in. Notifications are only user-level notifications for now.
* All JavaScript and frontend code related to displaying the bookmark reminder notification is contained here. The reminder functionality is now re-enabled in the bookmark modal as well.
* This PR also implements the "Remind me next time I am at my desktop" bookmark reminder functionality. When the user is on a mobile device they are able to select this option. When they choose this option we set a key in Redis saying they have a pending at desktop reminder. The next time they change devices we check if the new device is desktop, and if it is we send reminders using a DistributedMutex. There is also a job to ensure consistency of these reminders in Redis (in case Redis drops the ball) and the at desktop reminders expire after 20 days.
* Also in this PR is a fix to delete all Bookmarks for a user via `UserDestroyer`
Introduces a new site setting `max_notifications_per_user`.
Out-of-the-box this is set to 10,000. If a user exceeds this number of
notifications, we will delete the oldest notifications keeping only 10,000.
To disable this safeguard set the setting to 0.
Enforcement happens weekly.
This is in place to protect the system from pathological states where a
single user has enormous amounts of notifications causing various queries
to time out. In practice nobody looks back more than a few hundred notifications.
This simplifies the code for refreshing notification counts. It now
unconditionally looks up the user object which protects against stale objects
in memory.
This also removes a pile of conditional logic we no longer need.
Doing .pluck(:column).first is a very common pattern in Discourse and in
most cases, a limit cause isn't being added. Instead of adding a limit
clause to all these callsites, this commit adds two new methods to
ActiveRecord::Relation:
pluck_first, equivalent to limit(1).pluck(*columns).first
and pluck_first! which, like other finder methods, raises an exception
when no record is found
Zeitwerk simplifies working with dependencies in dev and makes it easier reloading class chains.
We no longer need to use Rails "require_dependency" anywhere and instead can just use standard
Ruby patterns to require files.
This is a far reaching change and we expect some followups here.
If enabled, this will fire a webhook whenever a user's notification has
been created. This could potentially be a lot of data depending on your
forum, and should be used carefully since it includes everything all users
will see in their feeds.
User.publish_notifications_state is called every time a notification is
created, this can become a very critical code path.
On some heavy notification related sites this can be a major CPU user on PG
This index makes it much cheaper to publish notification state, cause a
simple index lookup does the trick.
This reduces chances of errors where consumers of strings mutate inputs
and reduces memory usage of the app.
Test suite passes now, but there may be some stuff left, so we will run
a few sites on a branch prior to merging
Introduce new patterns for direct sql that are safe and fast.
MiniSql is not prone to memory bloat that can happen with direct PG usage.
It also has an extremely fast materializer and very a convenient API
- DB.exec(sql, *params) => runs sql returns row count
- DB.query(sql, *params) => runs sql returns usable objects (not a hash)
- DB.query_hash(sql, *params) => runs sql returns an array of hashes
- DB.query_single(sql, *params) => runs sql and returns a flat one dimensional array
- DB.build(sql) => returns a sql builder
See more at: https://github.com/discourse/mini_sql
Osama Sayegh
Mark VanLandingham
Alan Guo Xiang Tan
Rafael dos Santos Silva
Martin Brennan
Mark VanLandingham
Roman Rizzi
Joffrey JAFFEUX
Krzysztof Kotlarek
Martin Brennan
Sam Saffron
Vinoth Kannan
Vinoth Kannan
Gerhard Schlager
Dan Ungureanu
Daniel Waterworth
Robin Ward
Bianca Nenciu
Penar Musaraj
Guo Xiang Tan