Fix duplicate push notification for mention and comment event
Event notifiers used the `mentionees` DB association to exclude mentioned users from comment/card notifications. Since mentions are created async via Mention::CreateJob, a race condition meant the mentionee list could be empty when the event notification job ran first, causing the user to receive both a comment and a mention push notification. Use `scan_mentionees` instead, which scans the rich text body directly for mentioned users without depending on Mention records existing yet. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
committed by
Rosa Gutierrez
parent
7b93ec35b4
commit
60b3c9c772
@@ -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
|
||||
|
||||
private
|
||||
def mentionees_from_attachments
|
||||
rich_text_associations.flat_map { send(it.name)&.body&.attachments&.collect { it.attachable } }.compact
|
||||
end
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user