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

This commit is contained in:
Claire 2021-11-25 23:46:30 +01:00 committed by GitHub
parent 6e50134a42
commit 013bee6afb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 57 additions and 3 deletions

View file

@ -67,8 +67,49 @@ class NotifyService < BaseService
message? && @notification.target_status.direct_visibility? message? && @notification.target_status.direct_visibility?
end end
# Returns true if the sender has been mentionned by the recipient up the thread
def response_to_recipient? 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 end
def from_staff? def from_staff?

View file

@ -64,8 +64,9 @@ RSpec.describe NotifyService, type: :service do
is_expected.to_not change(Notification, :count) is_expected.to_not change(Notification, :count)
end 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(: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)) } let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: reply_to)) }
it 'does not notify' do it 'does not notify' do
@ -73,8 +74,20 @@ RSpec.describe NotifyService, type: :service do
end end
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(: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)) } let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: reply_to)) }
it 'does notify' do it 'does notify' do