PERF: only publish notification state if we changed it

also publish seen_notification_id so we can tell what is new and what is old
cleanup controller so it correctly checks user
fix bug around clearing notification when people click mark read

app/controllers/notifications_controller.rb

@@ -6,27 +6,29 @@ class NotificationsController < ApplicationController
 
   def index
     user = current_user
-    if params[:recent].present?
+    user = User.find_by_username(params[:username].to_s) if params[:username]
 
+    guardian.ensure_can_see_notifications!(user)
+
+    if params[:recent].present?
       limit = (params[:limit] || 15).to_i
       limit = 50 if limit > 50
 
       notifications = Notification.recent_report(current_user, limit)
+      changed = false
 
       if notifications.present?
         # ordering can be off due to PMs
         max_id = notifications.map(&:id).max
-        current_user.saw_notification_id(max_id) unless params.has_key?(:silent)
+        changed = current_user.saw_notification_id(max_id) unless params.has_key?(:silent)
       end
-      current_user.reload
-      current_user.publish_notifications_state
+      user.reload
+      user.publish_notifications_state if changed
 
-      render_serialized(notifications, NotificationSerializer, root: :notifications)
+      render_json_dump(notifications: serialize_data(notifications, NotificationSerializer),
+                       seen_notification_id: current_user.seen_notification_id)
     else
       offset = params[:offset].to_i
-      user = User.find_by_username(params[:username].to_s) if params[:username]
-
-      guardian.ensure_can_see_notifications!(user)
 
       notifications = Notification.where(user_id: user.id)
                                   .visible
@@ -37,6 +39,7 @@ class NotificationsController < ApplicationController
       notifications = notifications.offset(offset).limit(60)
       render_json_dump(notifications: serialize_data(notifications, NotificationSerializer),
                        total_rows_notifications: total_rows,
+                       seen_notification_id: user.seen_notification_id,
                        load_more_notifications: notifications_path(username: user.username, offset: offset + 60))
     end
 
@@ -45,7 +48,7 @@ class NotificationsController < ApplicationController
   def mark_read
     Notification.where(user_id: current_user.id).includes(:topic).where(read: false).update_all(read: true)
 
-    current_user.saw_notification_id(Notification.recent_report(current_user, 1).max)
+    current_user.saw_notification_id(Notification.recent_report(current_user, 1).max.try(:id))
     current_user.reload
     current_user.publish_notifications_state
 

app/models/user.rb

@@ -329,8 +329,12 @@ class User < ActiveRecord::Base
   end
 
   def saw_notification_id(notification_id)
-    User.where("id = ? and seen_notification_id < ?", id, notification_id)
-        .update_all ["seen_notification_id = ?", notification_id]
+    if seen_notification_id.to_i < notification_id.to_i
+      update_columns(seen_notification_id: notification_id.to_i)
+      true
+    else
+      false
+    end
   end
 
   def publish_notifications_state
@@ -375,7 +379,8 @@ class User < ActiveRecord::Base
                         unread_private_messages: unread_private_messages,
                         total_unread_notifications: total_unread_notifications,
                         last_notification: json,
-                        recent: recent
+                        recent: recent,
+                        seen_notification_id: seen_notification_id
                        },
                        user_ids: [id] # only publish the notification to this user
     )