From 60b3c9c772595dbd3c8b2178cdd9e2113ee892ef Mon Sep 17 00:00:00 2001
From: Rosa Gutierrez <rosa@37signals.com>
Date: Tue, 24 Feb 2026 11:58:22 +0100
Subject: [PATCH] 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>
---
 app/models/concerns/mentions.rb               |  8 +++----
 app/models/notifier/card_event_notifier.rb    |  4 ++--
 app/models/notifier/comment_event_notifier.rb |  2 +-
 test/models/notifier/event_notifier_test.rb   | 21 +++++++++++++++++++
 4 files changed, 28 insertions(+), 7 deletions(-)

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