From 091e7ef3caf67c0552e85d752ff10eae5886aad9 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Tue, 8 Mar 2016 15:26:06 -0500 Subject: [PATCH] FIX: when a post is edited by someone other than the original author and a mention is added, the mention notification is from the person who edited --- app/models/post.rb | 4 ++++ app/services/post_alerter.rb | 27 ++++++++++++++++++--------- spec/services/post_alerter_spec.rb | 9 +++++++++ 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index dfed7e86f1b..1b1766115c6 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -222,6 +222,10 @@ class Post < ActiveRecord::Base @acting_user = pu end + def last_editor + self.last_editor_id ? (User.find_by_id(self.last_editor_id) || user) : user + end + def whitelisted_spam_hosts hosts = SiteSetting diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index a8c6f78a070..150e33c35b8 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -38,14 +38,23 @@ class PostAlerter # mentions (users/groups) mentioned_groups, mentioned_users = extract_mentions(post) - expand_group_mentions(mentioned_groups, post) do |group, users| - notify_users(users - notified, :group_mentioned, post, group: group) - notified += users - end + if mentioned_groups || mentioned_users + mentioned_opts = {} + if post.last_editor_id != post.user_id + # Mention comes from an edit by someone else, so notification should say who added the mention. + editor = post.last_editor + mentioned_opts = {user_id: editor.id, original_username: editor.username, display_username: editor.username} + end - if mentioned_users - notify_users(mentioned_users - notified, :mentioned, post) - notified += mentioned_users + expand_group_mentions(mentioned_groups, post) do |group, users| + notify_users(users - notified, :group_mentioned, post, mentioned_opts.merge({group: group})) + notified += users + end + + if mentioned_users + notify_users(mentioned_users - notified, :mentioned, post, mentioned_opts) + notified += mentioned_users + end end # replies @@ -232,7 +241,7 @@ class PostAlerter # Make sure the user can see the post return unless Guardian.new(user).can_see?(post) - notifier_id = opts[:user_id] || post.user_id + notifier_id = opts[:user_id] || post.user_id # xxxxx look at revision history # apply muting here return if notifier_id && MutedUser.where(user_id: user.id, muted_user_id: notifier_id) @@ -285,7 +294,7 @@ class PostAlerter end original_post = post - original_username = opts[:display_username] || post.username + original_username = opts[:display_username] || post.username # xxxxx need something here too if collapsed post = first_unread_post(user, post.topic) || post diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index f029469692b..6c8b34e47e1 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -251,6 +251,15 @@ describe PostAlerter do }.not_to change(user.notifications, :count) end + it "notification comes from editor is mention is added later" do + admin = Fabricate(:admin) + post = create_post_with_alerts(user: user, raw: 'No mention here.') + expect { + post.revise(admin, { raw: "Mention @eviltrout in this edit." }) + }.to change(evil_trout.notifications, :count) + n = evil_trout.notifications.last + n.data_hash["original_username"].should == admin.username + end end describe ".create_notification" do