diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index df8563ba3..5f7dc99ce 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -17,11 +17,11 @@ module Mentions rich_text_associations.collect { send(it.name)&.to_plain_text }.compact.join(" ") end - private - def scan_mentionees - mentionees_from_attachments & mentionable_users - end + def scan_mentionees + mentionees_from_attachments & mentionable_users + end + private def mentionees_from_attachments rich_text_associations.flat_map { send(it.name)&.body&.attachments&.collect { it.attachable } }.compact end diff --git a/app/models/notifier/card_event_notifier.rb b/app/models/notifier/card_event_notifier.rb index 6179a4404..11e6ee8a0 100644 --- a/app/models/notifier/card_event_notifier.rb +++ b/app/models/notifier/card_event_notifier.rb @@ -8,9 +8,9 @@ class Notifier::CardEventNotifier < Notifier when "card_assigned" source.assignees.excluding(creator) when "card_published" - board.watchers.without(creator, *card.mentionees).including(*card.assignees).uniq + board.watchers.without(creator, *card.scan_mentionees).including(*card.assignees).uniq when "comment_created" - card.watchers.without(creator, *source.eventable.mentionees) + card.watchers.without(creator, *source.eventable.scan_mentionees) else board.watchers.without(creator) end diff --git a/app/models/notifier/comment_event_notifier.rb b/app/models/notifier/comment_event_notifier.rb index ee342748b..7a108a701 100644 --- a/app/models/notifier/comment_event_notifier.rb +++ b/app/models/notifier/comment_event_notifier.rb @@ -3,7 +3,7 @@ class Notifier::CommentEventNotifier < Notifier private def recipients - card.watchers.without(creator, *source.eventable.mentionees) + card.watchers.without(creator, *source.eventable.scan_mentionees) end def card diff --git a/test/models/notifier/event_notifier_test.rb b/test/models/notifier/event_notifier_test.rb index 6393ddb99..2ded4539e 100644 --- a/test/models/notifier/event_notifier_test.rb +++ b/test/models/notifier/event_notifier_test.rb @@ -93,6 +93,22 @@ class Notifier::EventNotifierTest < ActiveSupport::TestCase end end + test "don't create notifications on comment for mentionees even before mention records exist" do + comment = cards(:layout).comments.create!( + body: "Hey #{mention_html_for(users(:kevin))}, what do you think?", + creator: users(:david) + ) + event = boards(:writebook).events.create!( + action: "comment_created", creator: users(:david), eventable: comment + ) + + assert_empty comment.mentionees, "Mention records should not exist yet" + + notifications = Notifier.for(event).notify + + assert_not_includes notifications.map(&:user), users(:kevin) + end + test "assignment events notify assignees regardless of involvement level" do boards(:writebook).access_for(users(:jz)).access_only! @@ -100,4 +116,9 @@ class Notifier::EventNotifierTest < ActiveSupport::TestCase assert_equal [ users(:jz) ], notifications.map(&:user) end + + private + def mention_html_for(user) + ActionText::Attachment.from_attachable(user).to_html + end end