From 836bc0f935ed6903bbbd0e7a5059e012311512ee Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 7 Oct 2014 15:57:48 +1100 Subject: [PATCH] FIX: incorrect edit notification in user stream FIX: missing edit notifications when post edited by multiple users --- app/models/post_alert_observer.rb | 8 +++++++- app/models/user_action_observer.rb | 4 ++-- app/services/post_alerter.rb | 11 +++++++++-- spec/models/user_action_spec.rb | 11 +++++++++++ 4 files changed, 29 insertions(+), 5 deletions(-) diff --git a/app/models/post_alert_observer.rb b/app/models/post_alert_observer.rb index 1a55a596f93..221591e6c34 100644 --- a/app/models/post_alert_observer.rb +++ b/app/models/post_alert_observer.rb @@ -46,7 +46,13 @@ class PostAlertObserver < ActiveRecord::Observer return if post.topic.private_message? return if SiteSetting.disable_edit_notifications && post_revision.user_id == Discourse::SYSTEM_USER_ID - alerter.create_notification(post.user, Notification.types[:edited], post, display_username: post_revision.user.username) + alerter.create_notification( + post.user, + Notification.types[:edited], + post, + display_username: post_revision.user.username, + post_revision: post_revision + ) end diff --git a/app/models/user_action_observer.rb b/app/models/user_action_observer.rb index 956aaefa392..d170853e9c2 100644 --- a/app/models/user_action_observer.rb +++ b/app/models/user_action_observer.rb @@ -33,7 +33,7 @@ class UserActionObserver < ActiveRecord::Observer end end - def self.log_notification(post, user, notification_type) + def self.log_notification(post, user, notification_type, acting_user_id = nil) action = case notification_type when Notification.types[:quoted] @@ -52,7 +52,7 @@ class UserActionObserver < ActiveRecord::Observer row = { action_type: action, user_id: user.id, - acting_user_id: (action == UserAction::EDIT) ? post.last_editor_id : post.user_id, + acting_user_id: acting_user_id || post.user_id, target_topic_id: post.topic_id, target_post_id: post.id } diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 1dda39f0860..b0f178a748b 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -90,7 +90,14 @@ class PostAlerter return if TopicUser.get(post.topic, user).try(:notification_level) == TopicUser.notification_levels[:muted] # Don't notify the same user about the same notification on the same post - return if user.notifications.exists?(notification_type: type, topic_id: post.topic_id, post_number: post.post_number) + existing_notification = user.notifications + .order("notifications.id desc") + .find_by(notification_type: type, topic_id: post.topic_id, post_number: post.post_number) + + if(existing_notification) + return unless existing_notification.notification_type == Notification.types[:edited] && + existing_notification.data_hash["display_username"] = opts[:display_username] + end collapsed = false @@ -116,7 +123,7 @@ class PostAlerter opts[:display_username] = I18n.t('embed.replies', count: count) if count > 1 end - UserActionObserver.log_notification(original_post, user, type) + UserActionObserver.log_notification(original_post, user, type, opts[:post_revision].try(:user_id)) # Create the notification user.notifications.create(notification_type: type, diff --git a/spec/models/user_action_spec.rb b/spec/models/user_action_spec.rb index f66314d2596..8ca87bfc261 100644 --- a/spec/models/user_action_spec.rb +++ b/spec/models/user_action_spec.rb @@ -90,6 +90,17 @@ describe UserAction do # duplicate should not exception out log_test_action + + # recategorize belongs to the right user + category2 = Fabricate(:category) + admin = Fabricate(:admin) + public_topic.acting_user = admin + public_topic.change_category_to_id(category2.id) + + action = UserAction.stream(user_id: public_topic.user_id, guardian: Guardian.new)[0] + action.acting_user_id.should == admin.id + action.action_type.should == UserAction::EDIT + end end