From 25a82e7d22d9eb2567c9b9e738049050e8e7434d Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 16 Sep 2016 12:02:19 +1000 Subject: [PATCH] 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 | 21 ++++++++++++--------- app/models/user.rb | 11 ++++++++--- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 9831ee88c33..da047671b79 100644 --- a/app/controllers/notifications_controller.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index 1e87cadaf4b..d53b52b03bb 100644 --- a/app/models/user.rb +++ b/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 )