From fbef909c2a1ff8d24811f76237e62fbef6cc63cc Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 14 Nov 2017 21:12:57 +0100 Subject: [PATCH] Add option to block direct messages from people you don't follow (#5669) * Add option to block direct messages from people you don't follow Fix #5326 * If the DM responds to a toot by recipient, allow it through * i18n: Update Polish translation (for #5669) (#5673) --- .../settings/notifications_controller.rb | 2 +- app/services/notify_service.rb | 59 ++++++++++++++++--- .../settings/notifications/show.html.haml | 3 +- config/locales/simple_form.en.yml | 1 + config/locales/simple_form.pl.yml | 1 + config/settings.yml | 1 + spec/services/notify_service_spec.rb | 33 +++++++++++ 7 files changed, 89 insertions(+), 11 deletions(-) diff --git a/app/controllers/settings/notifications_controller.rb b/app/controllers/settings/notifications_controller.rb index 09839f16e..ce2530c54 100644 --- a/app/controllers/settings/notifications_controller.rb +++ b/app/controllers/settings/notifications_controller.rb @@ -26,7 +26,7 @@ class Settings::NotificationsController < ApplicationController def user_settings_params params.require(:user).permit( notification_emails: %i(follow follow_request reblog favourite mention digest), - interactions: %i(must_be_follower must_be_following) + interactions: %i(must_be_follower must_be_following must_be_following_dm) ) end end diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index ca53c61c5..6a24a8247 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -36,17 +36,58 @@ class NotifyService < BaseService false end + def following_sender? + return @following_sender if defined?(@following_sender) + @following_sender = @recipient.following?(@notification.from_account) || @recipient.requested?(@notification.from_account) + end + + def optional_non_follower? + @recipient.user.settings.interactions['must_be_follower'] && !@notification.from_account.following?(@recipient) + end + + def optional_non_following? + @recipient.user.settings.interactions['must_be_following'] && !following_sender? + end + + def direct_message? + @notification.type == :mention && @notification.target_status.direct_visibility? + end + + def response_to_recipient? + @notification.target_status.in_reply_to_account_id == @recipient.id + end + + def optional_non_following_and_direct? + direct_message? && + @recipient.user.settings.interactions['must_be_following_dm'] && + !following_sender? && + !response_to_recipient? + end + + def hellbanned? + @notification.from_account.silenced? && !following_sender? + end + + def from_self? + @recipient.id == @notification.from_account.id + end + + def domain_blocking? + @recipient.domain_blocking?(@notification.from_account.domain) && !following_sender? + end + def blocked? - blocked = @recipient.suspended? # Skip if the recipient account is suspended anyway - blocked ||= @recipient.id == @notification.from_account.id # Skip for interactions with self - blocked ||= @recipient.domain_blocking?(@notification.from_account.domain) && !@recipient.following?(@notification.from_account) # Skip for domain blocked accounts - blocked ||= @recipient.blocking?(@notification.from_account) # Skip for blocked accounts - blocked ||= @recipient.muting?(@notification.from_account) # Skip for muted accounts - blocked ||= (@notification.from_account.silenced? && !@recipient.following?(@notification.from_account)) # Hellban - blocked ||= (@recipient.user.settings.interactions['must_be_follower'] && !@notification.from_account.following?(@recipient)) # Options - blocked ||= (@recipient.user.settings.interactions['must_be_following'] && !@recipient.following?(@notification.from_account)) # Options + blocked = @recipient.suspended? # Skip if the recipient account is suspended anyway + blocked ||= from_self? # Skip for interactions with self + blocked ||= domain_blocking? # Skip for domain blocked accounts + blocked ||= @recipient.blocking?(@notification.from_account) # Skip for blocked accounts + blocked ||= @recipient.muting?(@notification.from_account) # Skip for muted accounts + blocked ||= hellbanned? # Hellban + blocked ||= optional_non_follower? # Options + blocked ||= optional_non_following? # Options + blocked ||= optional_non_following_and_direct? # Options blocked ||= conversation_muted? - blocked ||= send("blocked_#{@notification.type}?") # Type-dependent filters + blocked ||= send("blocked_#{@notification.type}?") # Type-dependent filters blocked end diff --git a/app/views/settings/notifications/show.html.haml b/app/views/settings/notifications/show.html.haml index 80cd615c7..b718b62df 100644 --- a/app/views/settings/notifications/show.html.haml +++ b/app/views/settings/notifications/show.html.haml @@ -11,7 +11,7 @@ = ff.input :reblog, as: :boolean, wrapper: :with_label = ff.input :favourite, as: :boolean, wrapper: :with_label = ff.input :mention, as: :boolean, wrapper: :with_label - + .fields-group = f.simple_fields_for :notification_emails, hash_to_object(current_user.settings.notification_emails) do |ff| = ff.input :digest, as: :boolean, wrapper: :with_label @@ -20,6 +20,7 @@ = f.simple_fields_for :interactions, hash_to_object(current_user.settings.interactions) do |ff| = ff.input :must_be_follower, as: :boolean, wrapper: :with_label = ff.input :must_be_following, as: :boolean, wrapper: :with_label + = ff.input :must_be_following_dm, as: :boolean, wrapper: :with_label .actions = f.button :button, t('generic.save_changes'), type: :submit diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index aafae48ce..faf41f316 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -54,6 +54,7 @@ en: interactions: must_be_follower: Block notifications from non-followers must_be_following: Block notifications from people you don't follow + must_be_following_dm: Block direct messages from people you don't follow notification_emails: digest: Send digest e-mails favourite: Send e-mail when someone favourites your status diff --git a/config/locales/simple_form.pl.yml b/config/locales/simple_form.pl.yml index 68f84d109..8b539662c 100644 --- a/config/locales/simple_form.pl.yml +++ b/config/locales/simple_form.pl.yml @@ -58,6 +58,7 @@ pl: interactions: must_be_follower: Nie wyświetlaj powiadomień od osób, które Cię nie śledzą must_be_following: Nie wyświetlaj powiadomień od osób, których nie śledzisz + must_be_following_dm: Nie wyświetlaj wiadomości bezpośrednich od osób, których nie śledzisz notification_emails: digest: Wysyłaj podsumowania e-mailem favourite: Powiadamiaj mnie e-mailem, gdy ktoś polubi mój wpis diff --git a/config/settings.yml b/config/settings.yml index 11681d7ec..a4df4094d 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -36,6 +36,7 @@ defaults: &defaults interactions: must_be_follower: false must_be_following: false + must_be_following_dm: false reserved_usernames: - admin - support diff --git a/spec/services/notify_service_spec.rb b/spec/services/notify_service_spec.rb index 7a66bd0fe..58ee66ded 100644 --- a/spec/services/notify_service_spec.rb +++ b/spec/services/notify_service_spec.rb @@ -38,6 +38,39 @@ RSpec.describe NotifyService do is_expected.to_not change(Notification, :count) end + context 'for direct messages' do + let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct)) } + + before do + user.settings.interactions = user.settings.interactions.merge('must_be_following_dm' => enabled) + end + + context 'if recipient is supposed to be following sender' do + let(:enabled) { true } + + it 'does not notify' do + is_expected.to_not change(Notification, :count) + end + + context 'if the message chain initiated by recipient' do + let(:reply_to) { Fabricate(:status, account: recipient) } + let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: reply_to)) } + + it 'does notify' do + is_expected.to change(Notification, :count) + end + end + end + + context 'if recipient is NOT supposed to be following sender' do + let(:enabled) { false } + + it 'does notify' do + is_expected.to change(Notification, :count) + end + end + end + context do let(:asshole) { Fabricate(:account, username: 'asshole') } let(:reply_to) { Fabricate(:status, account: asshole) }