From f166b30f2fb785ef03101ab5386cd7b34cf41ade Mon Sep 17 00:00:00 2001
From: Jorge Manrubia <jorge@37signals.com>
Date: Thu, 24 Apr 2025 12:17:04 +0200
Subject: [PATCH] Fix: dismiss mention notifications in cards and comments when
 reading a card

---
 app/controllers/cards/readings_controller.rb |  3 +--
 app/models/card.rb                           |  3 ++-
 app/models/card/readable.rb                  | 26 ++++++++++++++++++++
 test/fixtures/mentions.yml                   |  7 +++++-
 test/fixtures/notifications.yml              | 12 +++++++++
 test/models/card/readable_test.rb            | 23 +++++++++++++++++
 test/models/user/mentionable_test.rb         |  2 ++
 7 files changed, 72 insertions(+), 4 deletions(-)
 create mode 100644 app/models/card/readable.rb
 create mode 100644 test/models/card/readable_test.rb

diff --git a/app/controllers/cards/readings_controller.rb b/app/controllers/cards/readings_controller.rb
index efd06e601..2334cb8f5 100644
--- a/app/controllers/cards/readings_controller.rb
+++ b/app/controllers/cards/readings_controller.rb
@@ -2,7 +2,6 @@ class Cards::ReadingsController < ApplicationController
   include CardScoped
 
   def create
-    @notifications = Current.user.notifications.where(source: @card.events)
-    @notifications.each(&:read)
+    @notifications = @card.read_by(Current.user)
   end
 end
diff --git a/app/models/card.rb b/app/models/card.rb
index 808ff9467..a1d8e5115 100644
--- a/app/models/card.rb
+++ b/app/models/card.rb
@@ -1,6 +1,7 @@
 class Card < ApplicationRecord
   include Assignable, Colored, Engageable, Eventable, Golden, Mentions,
-    Messages, Pinnable, Closeable, Searchable, Staged, Statuses, Taggable, Watchable
+    Messages, Pinnable, Closeable, Readable, Searchable, Staged,
+    Statuses, Taggable, Watchable
 
   belongs_to :collection, touch: true
   belongs_to :creator, class_name: "User", default: -> { Current.user }
diff --git a/app/models/card/readable.rb b/app/models/card/readable.rb
new file mode 100644
index 000000000..2511a6b45
--- /dev/null
+++ b/app/models/card/readable.rb
@@ -0,0 +1,26 @@
+module Card::Readable
+  extend ActiveSupport::Concern
+
+  def read_by(user)
+    notifications_for(user).tap do |notifications|
+      notifications.each(&:read)
+    end
+  end
+
+  private
+    def notifications_for(user)
+      user.notifications.unread.where(source: notification_sources)
+    end
+
+    def notification_sources
+      events + mentions + comment_mentions
+    end
+
+    def comment_mentions
+      Mention.where(source: comments)
+    end
+
+    def comments
+      Comment.where(id: messages.comments.pluck(:messageable_id))
+    end
+end
diff --git a/test/fixtures/mentions.yml b/test/fixtures/mentions.yml
index 70373600f..77963954c 100644
--- a/test/fixtures/mentions.yml
+++ b/test/fixtures/mentions.yml
@@ -1,4 +1,9 @@
 logo_card_david_mention_by_jz:
-  source: logo (card)
+  source: logo (Card)
+  mentioner: jz
+  mentionee: david
+
+logo_comment_david_mention_by_jz:
+  source: logo_agreement_jz (Comment)
   mentioner: jz
   mentionee: david
diff --git a/test/fixtures/notifications.yml b/test/fixtures/notifications.yml
index bea0e4d33..8c88d23d0 100644
--- a/test/fixtures/notifications.yml
+++ b/test/fixtures/notifications.yml
@@ -15,3 +15,15 @@ layout_commented_kevin:
   source: layout_commented (Event)
   created_at: <%= 1.week.ago %>
   creator: david
+
+logo_card_david_mention_by_jz:
+  user: david
+  source: logo_card_david_mention_by_jz (Mention)
+  created_at: <%= 1.week.ago %>
+  creator: david
+
+logo_comment_david_mention_by_jz:
+  user: david
+  source: logo_comment_david_mention_by_jz (Mention)
+  created_at: <%= 1.week.ago %>
+  creator: david
diff --git a/test/models/card/readable_test.rb b/test/models/card/readable_test.rb
new file mode 100644
index 000000000..b60d98a09
--- /dev/null
+++ b/test/models/card/readable_test.rb
@@ -0,0 +1,23 @@
+require "test_helper"
+
+class Card::ReadableTest < ActiveSupport::TestCase
+  test "read clears events notifications" do
+    assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: false, to: true do
+      assert_changes -> { notifications(:logo_assignment_kevin).reload.read? }, from: false, to: true do
+        cards(:logo).read_by(users(:kevin))
+      end
+    end
+  end
+
+  test "read clear mentions in the description" do
+    assert_changes -> { notifications(:logo_card_david_mention_by_jz).reload.read? }, from: false, to: true do
+      cards(:logo).read_by(users(:david))
+    end
+  end
+
+  test "read clear mentions in comments" do
+    assert_changes -> { notifications(:logo_comment_david_mention_by_jz).reload.read? }, from: false, to: true do
+      cards(:logo).read_by(users(:david))
+    end
+  end
+end
diff --git a/test/models/user/mentionable_test.rb b/test/models/user/mentionable_test.rb
index 7231553c0..9bc441fe5 100644
--- a/test/models/user/mentionable_test.rb
+++ b/test/models/user/mentionable_test.rb
@@ -6,6 +6,8 @@ class User::MentionableTest < ActiveSupport::TestCase
   end
 
   test "mentioned by" do
+    users(:david).mentions.destroy_all
+
     assert_difference -> { users(:david).mentions.count }, +1 do
       users(:david).mentioned_by users(:jz), at: cards(:logo)
     end