Fix filtering DMs from non-followed users (#17042)

diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb
index a1b5ca1e3762842ce0c0bda28018727c064535e6..09e28b76b02f5b1f1be0a8e2b4a5e46eec925688 100644 (file)
--- a/app/services/notify_service.rb
+++ b/app/services/notify_service.rb
@@ -67,8 +67,49 @@ class NotifyService < BaseService
     message? && @notification.target_status.direct_visibility?
   end
 
+  # Returns true if the sender has been mentionned by the recipient up the thread
   def response_to_recipient?
-    @notification.target_status.in_reply_to_account_id == @recipient.id && @notification.target_status.thread&.direct_visibility?
+    return false if @notification.target_status.in_reply_to_id.nil?
+
+    # Using an SQL CTE to avoid unneeded back-and-forth with SQL server in case of long threads
+    !Status.count_by_sql([<<-SQL.squish, id: @notification.target_status.in_reply_to_id, recipient_id: @recipient.id, sender_id: @notification.from_account.id]).zero?
+      WITH RECURSIVE ancestors(id, in_reply_to_id, replying_to_sender) AS (
+          SELECT
+            s.id, s.in_reply_to_id, (CASE
+              WHEN s.account_id = :recipient_id THEN
+                EXISTS (
+                  SELECT *
+                  FROM mentions m
+                  WHERE m.silent = FALSE AND m.account_id = :sender_id AND m.status_id = s.id
+                )
+              ELSE
+                FALSE
+             END)
+          FROM statuses s
+          WHERE s.id = :id
+        UNION ALL
+          SELECT
+            s.id,
+            s.in_reply_to_id,
+            (CASE
+              WHEN s.account_id = :recipient_id THEN
+                EXISTS (
+                  SELECT *
+                  FROM mentions m
+                  WHERE m.silent = FALSE AND m.account_id = :sender_id AND m.status_id = s.id
+                )
+              ELSE
+                FALSE
+             END)
+          FROM ancestors st
+          JOIN statuses s ON s.id = st.in_reply_to_id
+          WHERE st.replying_to_sender IS FALSE
+      )
+      SELECT COUNT(*)
+      FROM ancestors st
+      JOIN statuses s ON s.id = st.id
+      WHERE st.replying_to_sender IS TRUE AND s.visibility = 3
+    SQL
   end
 
   def from_staff?

diff --git a/spec/services/notify_service_spec.rb b/spec/services/notify_service_spec.rb
index f2cb22c5ef7508d1fd0a535dd379ecf0aec8a550..7433866b76db141dfd9b1bc9d3e1b6132dd023a3 100644 (file)
--- a/spec/services/notify_service_spec.rb
+++ b/spec/services/notify_service_spec.rb
@@ -64,8 +64,9 @@ RSpec.describe NotifyService, type: :service do
         is_expected.to_not change(Notification, :count)
       end
 
-      context 'if the message chain initiated by recipient, but is not direct message' do
+      context 'if the message chain is initiated by recipient, but is not direct message' do
         let(:reply_to) { Fabricate(:status, account: recipient) }
+        let!(:mention) { Fabricate(:mention, account: sender, status: reply_to) }
         let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: reply_to)) }
 
         it 'does not notify' do
@@ -73,8 +74,20 @@ RSpec.describe NotifyService, type: :service do
         end
       end
 
-      context 'if the message chain initiated by recipient and is direct message' do
+      context 'if the message chain is initiated by recipient, but without a mention to the sender, even if the sender sends multiple messages in a row' do
+        let(:reply_to) { Fabricate(:status, account: recipient) }
+        let!(:mention) { Fabricate(:mention, account: sender, status: reply_to) }
+        let(:dummy_reply) { Fabricate(:status, account: sender, visibility: :direct, thread: reply_to) }
+        let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: dummy_reply)) }
+
+        it 'does not notify' do
+          is_expected.to_not change(Notification, :count)
+        end
+      end
+
+      context 'if the message chain is initiated by the recipient with a mention to the sender' do
         let(:reply_to) { Fabricate(:status, account: recipient, visibility: :direct) }
+        let!(:mention) { Fabricate(:mention, account: sender, status: reply_to) }
         let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: reply_to)) }
 
         it 'does notify' do