FEATURE: High priority bookmark reminder notifications (#9290)

FEATURE: High priority bookmark reminder notifications (#9290)

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.

diff --git a/app/assets/javascripts/discourse/components/site-header.js b/app/assets/javascripts/discourse/components/site-header.js
index b8b6ed9..03bc7b3 100644
--- a/app/assets/javascripts/discourse/components/site-header.js
+++ b/app/assets/javascripts/discourse/components/site-header.js
@@ -24,7 +24,7 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, {
 
   @observes(
     "currentUser.unread_notifications",
-    "currentUser.unread_private_messages",
+    "currentUser.unread_high_priority_notifications",
     "currentUser.reviewable_count"
   )
   notificationsChanged() {
diff --git a/app/assets/javascripts/discourse/initializers/badging.js b/app/assets/javascripts/discourse/initializers/badging.js
index f406604..8bcefa7 100644
--- a/app/assets/javascripts/discourse/initializers/badging.js
+++ b/app/assets/javascripts/discourse/initializers/badging.js
@@ -10,7 +10,7 @@ export default {
     if (!user) return; // must be logged in
 
     this.notifications =
-      user.unread_notifications + user.unread_private_messages;
+      user.unread_notifications + user.unread_high_priority_notifications;
 
     container
       .lookup("service:app-events")
diff --git a/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js b/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js
index 14c2bc5..1482cf9 100644
--- a/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js
+++ b/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js
@@ -32,24 +32,27 @@ export default {
         data => {
           const store = container.lookup("service:store");
           const oldUnread = user.get("unread_notifications");
-          const oldPM = user.get("unread_private_messages");
+          const oldHighPriority = user.get(
+            "unread_high_priority_notifications"
+          );
 
           user.setProperties({
             unread_notifications: data.unread_notifications,
-            unread_private_messages: data.unread_private_messages,
+            unread_high_priority_notifications:
+              data.unread_high_priority_notifications,
             read_first_notification: data.read_first_notification
           });
 
           if (
             oldUnread !== data.unread_notifications ||
-            oldPM !== data.unread_private_messages
+            oldHighPriority !== data.unread_high_priority_notifications
           ) {
             appEvents.trigger("notifications:changed");
 
             if (
               site.mobileView &&
               (data.unread_notifications - oldUnread > 0 ||
-                data.unread_private_messages - oldPM > 0)
+                data.unread_high_priority_notifications - oldHighPriority > 0)
             ) {
               appEvents.trigger("header:update-topic", null, 5000);
             }
diff --git a/app/assets/javascripts/discourse/initializers/title-notifications.js b/app/assets/javascripts/discourse/initializers/title-notifications.js
index 9801e1f..cfc0c3e 100644
--- a/app/assets/javascripts/discourse/initializers/title-notifications.js
+++ b/app/assets/javascripts/discourse/initializers/title-notifications.js
@@ -18,7 +18,7 @@ export default {
     if (!user) return; // must be logged in
 
     const notifications =
-      user.unread_notifications + user.unread_private_messages;
+      user.unread_notifications + user.unread_high_priority_notifications;
 
     Discourse.updateNotificationCount(notifications);
   }
diff --git a/app/assets/javascripts/discourse/widgets/header.js b/app/assets/javascripts/discourse/widgets/header.js
index e08bab8..4910913 100644
--- a/app/assets/javascripts/discourse/widgets/header.js
+++ b/app/assets/javascripts/discourse/widgets/header.js
@@ -67,8 +67,9 @@ createWidget("header-notifications", {
       );
     }
 
-    const unreadPMs = user.get("unread_private_messages");
-    if (!!unreadPMs) {
+    const unreadHighPriority = user.get("unread_high_priority_notifications");
+    if (!!unreadHighPriority) {
+      // highlight the avatar if the first ever PM is not read
       if (
         !user.get("read_first_notification") &&
         !user.get("enforcedSecondFactor")
@@ -90,14 +91,15 @@ createWidget("header-notifications", {
         }
       }
 
+      // add the counter for the unread high priority
       contents.push(
         this.attach("link", {
           action: attrs.action,
-          className: "badge-notification unread-private-messages",
-          rawLabel: unreadPMs,
+          className: "badge-notification unread-high-priority-notifications",
+          rawLabel: unreadHighPriority,
           omitSpan: true,
-          title: "notifications.tooltip.message",
-          titleOptions: { count: unreadPMs }
+          title: "notifications.tooltip.high_priority",
+          titleOptions: { count: unreadHighPriority }
         })
       );
     }
diff --git a/app/assets/stylesheets/common/base/discourse.scss b/app/assets/stylesheets/common/base/discourse.scss
index 023154c..831aba6 100644
--- a/app/assets/stylesheets/common/base/discourse.scss
+++ b/app/assets/stylesheets/common/base/discourse.scss
@@ -422,7 +422,7 @@ table {
   align-items: center;
 }
 
-.unread-private-messages {
+.unread-high-priority-notifications {
   color: $secondary;
   background: $success;
 
diff --git a/app/assets/stylesheets/common/base/header.scss b/app/assets/stylesheets/common/base/header.scss
index ba3ccb1..42e57ae 100644
--- a/app/assets/stylesheets/common/base/header.scss
+++ b/app/assets/stylesheets/common/base/header.scss
@@ -202,7 +202,7 @@
     right: -3px;
     background-color: dark-light-choose($tertiary-medium, $tertiary);
   }
-  .unread-private-messages,
+  .unread-high-priority-notifications,
   .ring {
     left: auto;
     right: 25px;
diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb
index d91ee57..7b35e9b 100644
--- a/app/mailers/user_notifications.rb
+++ b/app/mailers/user_notifications.rb
@@ -216,8 +216,8 @@ class UserNotifications < ActionMailer::Base
       value = user.unread_notifications
       @counts << { label_key: 'user_notifications.digest.unread_notifications', value: value, href: "#{Discourse.base_url}/my/notifications" } if value > 0
 
-      value = user.unread_private_messages
-      @counts << { label_key: 'user_notifications.digest.unread_messages', value: value, href: "#{Discourse.base_url}/my/messages" } if value > 0
+      value = user.unread_high_priority_notifications
+      @counts << { label_key: 'user_notifications.digest.unread_high_priority', value: value, href: "#{Discourse.base_url}/my/notifications" } if value > 0
 
       if @counts.size < 3
         value = user.unread_notifications_of_type(Notification.types[:liked])
diff --git a/app/models/notification.rb b/app/models/notification.rb
index a44086f..0407f17 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -44,6 +44,10 @@ class Notification < ActiveRecord::Base
     send_email unless NotificationConsolidator.new(self).consolidate!
   end
 
+  before_create do
+    self.high_priority = Notification.high_priority_types.include?(self.notification_type)
+  end
+
   def self.purge_old!
     return if SiteSetting.max_notifications_per_user == 0
 
@@ -64,10 +68,10 @@ class Notification < ActiveRecord::Base
   end
 
   def self.ensure_consistency!
-    DB.exec(<<~SQL, Notification.types[:private_message])
+    DB.exec(<<~SQL)
       DELETE
         FROM notifications n
-       WHERE notification_type = ?
+       WHERE high_priority
          AND NOT EXISTS (
             SELECT 1
               FROM posts p
@@ -108,6 +112,17 @@ class Notification < ActiveRecord::Base
                        )
   end
 
+  def self.high_priority_types
+    @high_priority_types ||= [
+      types[:private_message],
+      types[:bookmark_reminder]
+    ]
+  end
+
+  def self.normal_priority_types

[... diff too long, it was truncated ...]

GitHub sha: b79ea986

This commit appears in #9290 which was merged by martin.