FIX: ensure 'notifications_state' is up to date after creating a notification

app/assets/javascripts/discourse/components/site-header.js.es6

@@ -16,7 +16,7 @@ const SiteHeaderComponent = MountWidget.extend(Docking, {
   _topic: null,
 
   @observes('currentUser.unread_notifications', 'currentUser.unread_private_messages')
-  _notificationsChanged() {
+  notificationsChanged() {
     this.queueRerender();
   },
 

app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6

@@ -35,7 +35,6 @@ export default {
 
       bus.subscribe(`/notification/${user.get('id')}`, data => {
         const store = container.lookup('service:store');
-
         const oldUnread = user.get('unread_notifications');
         const oldPM = user.get('unread_private_messages');
 
@@ -96,21 +95,16 @@ export default {
       });
 
       bus.subscribe("/client_settings", data => Ember.set(siteSettings, data.name, data.value));
-
+      bus.subscribe("/refresh_client", data => Discourse.set("assetVersion", data));
-      bus.subscribe("/refresh_client", data => {
-        Discourse.set("assetVersion", data);
-      });
 
       if (!Ember.testing) {
-
         bus.subscribe(alertChannel(user), data => onNotification(data, user));
         initDesktopNotifications(bus, appEvents);
 
         if (isPushNotificationsEnabled(user, site.mobileView)) {
           disableDesktopNotifications();
           registerPushNotifications(Discourse.User.current(), site.mobileView, router, appEvents);
-        }
+        } else {
-        else {
           unsubscribePushNotifications(user);
         }
       }

app/models/notification.rb

@@ -59,22 +59,21 @@ class Notification < ActiveRecord::Base
 
   def self.mark_posts_read(user, topic_id, post_numbers)
     count = Notification
-      .where(user_id: user.id,
+      .where(user_id: user.id)
-             topic_id: topic_id,
+      .where(topic_id: topic_id)
-             post_number: post_numbers,
+      .where(post_number: post_numbers)
-             read: false)
+      .where(read: false)
-      .update_all("read = 't'")
+      .update_all(read: true)
 
-    if count > 0
+    user.publish_notifications_state if count > 0
-      user.publish_notifications_state
-    end
 
     count
   end
 
   def self.read(user, notification_ids)
-    count = Notification.where(user_id: user.id)
+    count = Notification
       .where(id: notification_ids)
+      .where(user_id: user.id)
       .where(read: false)
       .update_all(read: true)
 
@@ -202,7 +201,7 @@ class Notification < ActiveRecord::Base
   protected
 
   def refresh_notification_count
-    user.publish_notifications_state
+    user.reload.publish_notifications_state
   end
 
   def send_email

app/models/user.rb

@@ -377,14 +377,15 @@ class User < ActiveRecord::Base
 
   def unread_notifications_of_type(notification_type)
     # perf critical, much more efficient than AR
-    sql = "
+    sql = <<~SQL
-       SELECT COUNT(*) FROM notifications n
+        SELECT COUNT(*)
-       LEFT JOIN topics t ON n.topic_id = t.id
+          FROM notifications n
-       WHERE
+     LEFT JOIN topics t ON t.id = n.topic_id
-        t.deleted_at IS NULL AND
+         WHERE t.deleted_at IS NULL
-        n.notification_type = :type AND
+           AND n.notification_type = :type
-        n.user_id = :user_id AND
+           AND n.user_id = :user_id
-        NOT read"
+           AND NOT read
+    SQL
 
     User.exec_sql(sql, user_id: id, type: notification_type).getvalue(0, 0).to_i
   end
@@ -394,23 +395,24 @@ class User < ActiveRecord::Base
   end
 
   def unread_notifications
-    @unread_notifications ||=
+    @unread_notifications ||= begin
-      begin
       # perf critical, much more efficient than AR
-        sql = "
+      sql = <<~SQL
-           SELECT COUNT(*) FROM notifications n
+          SELECT COUNT(*)
-           LEFT JOIN topics t ON n.topic_id = t.id
+            FROM notifications n
-           WHERE
+       LEFT JOIN topics t ON t.id = n.topic_id
-            t.deleted_at IS NULL AND
+           WHERE t.deleted_at IS NULL
-            n.notification_type <> :pm AND
+             AND n.notification_type <> :pm
-            n.user_id = :user_id AND
+             AND n.user_id = :user_id
-            NOT read AND
+             AND n.id > :seen_notification_id
-            n.id > :seen_notification_id"
+             AND NOT read
+      SQL
 
-        User.exec_sql(sql, user_id: id,
+      User.exec_sql(sql,
+        user_id: id,
         seen_notification_id: seen_notification_id,
-                           pm:  Notification.types[:private_message])
+        pm:  Notification.types[:private_message]
-          .getvalue(0, 0).to_i
+    ).getvalue(0, 0).to_i
     end
   end
 
@@ -440,8 +442,7 @@ class User < ActiveRecord::Base
   end
 
   def publish_notifications_state
-    # publish last notification json with the message so we
+    # publish last notification json with the message so we can apply an update
-    # can apply an update
     notification = notifications.visible.order('notifications.id desc').first
     json = NotificationSerializer.new(notification).as_json if notification
 
@@ -477,17 +478,17 @@ class User < ActiveRecord::Base
       [id.to_i, read]
     end
 
-    MessageBus.publish("/notification/#{id}",
+    payload = {
-                       { unread_notifications: unread_notifications,
+      unread_notifications: unread_notifications,
       unread_private_messages: unread_private_messages,
       total_unread_notifications: total_unread_notifications,
       read_first_notification: read_first_notification?,
       last_notification: json,
       recent: recent,
-                         seen_notification_id: seen_notification_id
+      seen_notification_id: seen_notification_id,
-                       },
+    }
-                       user_ids: [id] # only publish the notification to this user
+
-    )
+    MessageBus.publish("/notification/#{id}", payload, user_ids: [id])
   end
 
   # A selection of people to autocomplete on @mention

app/services/post_alerter.rb

@@ -425,8 +425,10 @@ class PostAlerter
 
     if SiteSetting.allow_user_api_key_scopes.split("|").include?("push") && SiteSetting.allowed_user_api_push_urls.present?
       clients = user.user_api_keys
-        .where("('push' = ANY(scopes) OR 'notifications' = ANY(scopes)) AND push_url IS NOT NULL AND position(push_url in ?) > 0 AND revoked_at IS NULL",
+        .where("('push' = ANY(scopes) OR 'notifications' = ANY(scopes))")
-                  SiteSetting.allowed_user_api_push_urls)
+        .where("push_url IS NOT NULL")
+        .where("position(push_url IN ?) > 0", SiteSetting.allowed_user_api_push_urls)
+        .where("revoked_at IS NULL")
         .pluck(:client_id, :push_url)
 
       if clients.length > 0

lib/post_jobs_enqueuer.rb

@@ -45,10 +45,9 @@ class PostJobsEnqueuer
     TopicTrackingState.publish_unread(@post) if @post.post_number > 1
     TopicTrackingState.publish_latest(@topic, @post.whisper?)
 
-    Jobs.enqueue_in(
+    Jobs.enqueue_in(SiteSetting.email_time_window_mins.minutes,
-        SiteSetting.email_time_window_mins.minutes,
       :notify_mailing_list_subscribers,
-        post_id: @post.id
+      post_id: @post.id,
     )
   end