From 10f51c9886123982bc9f2a95fba39dd1f24b9a05 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 9 Jun 2018 22:46:54 +0200 Subject: [PATCH] Fix domain hiding logic (#7765) * Send rejections to followers when user hides domain they're on * Use account domain blocks for "authorized followers" action Replace soft-blocking (block & unblock) behaviour with follow rejection * Split sync and async work of account domain blocking Do not create domain block when removing followers by domain, that is probably unexpected from the user's perspective. * Adjust confirmation message for domain block * yarn manage:translations --- .../api/v1/domain_blocks_controller.rb | 3 +- .../settings/follower_domains_controller.rb | 2 +- .../containers/header_container.js | 2 +- .../mastodon/locales/defaultMessages.json | 2 +- app/javascript/mastodon/locales/en.json | 2 +- ...after_block_domain_from_account_service.rb | 42 +++++++++++++++++++ .../block_domain_from_account_service.rb | 8 ---- .../after_account_domain_block_worker.rb | 11 +++++ .../soft_block_domain_followers_worker.rb | 14 ------- app/workers/soft_block_worker.rb | 17 -------- ..._block_domain_from_account_service_spec.rb | 25 +++++++++++ .../block_domain_from_account_service_spec.rb | 19 --------- 12 files changed, 84 insertions(+), 63 deletions(-) create mode 100644 app/services/after_block_domain_from_account_service.rb delete mode 100644 app/services/block_domain_from_account_service.rb create mode 100644 app/workers/after_account_domain_block_worker.rb delete mode 100644 app/workers/soft_block_domain_followers_worker.rb delete mode 100644 app/workers/soft_block_worker.rb create mode 100644 spec/services/after_block_domain_from_account_service_spec.rb delete mode 100644 spec/services/block_domain_from_account_service_spec.rb diff --git a/app/controllers/api/v1/domain_blocks_controller.rb b/app/controllers/api/v1/domain_blocks_controller.rb index ae6ad7936..e55d622c3 100644 --- a/app/controllers/api/v1/domain_blocks_controller.rb +++ b/app/controllers/api/v1/domain_blocks_controller.rb @@ -15,7 +15,8 @@ class Api::V1::DomainBlocksController < Api::BaseController end def create - BlockDomainFromAccountService.new.call(current_account, domain_block_params[:domain]) + current_account.block_domain!(domain_block_params[:domain]) + AfterAccountDomainBlockWorker.perform_async(current_account.id, domain_block_params[:domain]) render_empty end diff --git a/app/controllers/settings/follower_domains_controller.rb b/app/controllers/settings/follower_domains_controller.rb index 91b521e7f..a128bd136 100644 --- a/app/controllers/settings/follower_domains_controller.rb +++ b/app/controllers/settings/follower_domains_controller.rb @@ -13,7 +13,7 @@ class Settings::FollowerDomainsController < ApplicationController def update domains = bulk_params[:select] || [] - SoftBlockDomainFollowersWorker.push_bulk(domains) do |domain| + AfterAccountDomainBlockWorker.push_bulk(domains) do |domain| [current_account.id, domain] end diff --git a/app/javascript/mastodon/features/account_timeline/containers/header_container.js b/app/javascript/mastodon/features/account_timeline/containers/header_container.js index 4d5308219..7681430b7 100644 --- a/app/javascript/mastodon/features/account_timeline/containers/header_container.js +++ b/app/javascript/mastodon/features/account_timeline/containers/header_container.js @@ -96,7 +96,7 @@ const mapDispatchToProps = (dispatch, { intl }) => ({ onBlockDomain (domain) { dispatch(openModal('CONFIRM', { - message: {domain} }} />, + message: {domain} }} />, confirm: intl.formatMessage(messages.blockDomainConfirm), onConfirm: () => dispatch(blockDomain(domain)), })); diff --git a/app/javascript/mastodon/locales/defaultMessages.json b/app/javascript/mastodon/locales/defaultMessages.json index ab2f3475b..2fdf99a4b 100644 --- a/app/javascript/mastodon/locales/defaultMessages.json +++ b/app/javascript/mastodon/locales/defaultMessages.json @@ -449,7 +449,7 @@ "id": "confirmations.block.message" }, { - "defaultMessage": "Are you really, really sure you want to block the entire {domain}? In most cases a few targeted blocks or mutes are sufficient and preferable.", + "defaultMessage": "Are you really, really sure you want to block the entire {domain}? In most cases a few targeted blocks or mutes are sufficient and preferable. You will not see content from that domain in any public timelines or your notifications. Your followers from that domain will be removed.", "id": "confirmations.domain_block.message" } ], diff --git a/app/javascript/mastodon/locales/en.json b/app/javascript/mastodon/locales/en.json index 855a29622..3a03fe27a 100644 --- a/app/javascript/mastodon/locales/en.json +++ b/app/javascript/mastodon/locales/en.json @@ -80,7 +80,7 @@ "confirmations.delete_list.confirm": "Delete", "confirmations.delete_list.message": "Are you sure you want to permanently delete this list?", "confirmations.domain_block.confirm": "Hide entire domain", - "confirmations.domain_block.message": "Are you really, really sure you want to block the entire {domain}? In most cases a few targeted blocks or mutes are sufficient and preferable.", + "confirmations.domain_block.message": "Are you really, really sure you want to block the entire {domain}? In most cases a few targeted blocks or mutes are sufficient and preferable. You will not see content from that domain in any public timelines or your notifications. Your followers from that domain will be removed.", "confirmations.mute.confirm": "Mute", "confirmations.mute.message": "Are you sure you want to mute {name}?", "confirmations.redraft.confirm": "Delete & redraft", diff --git a/app/services/after_block_domain_from_account_service.rb b/app/services/after_block_domain_from_account_service.rb new file mode 100644 index 000000000..0f1a8505d --- /dev/null +++ b/app/services/after_block_domain_from_account_service.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +class AfterBlockDomainFromAccountService < BaseService + # This service does not create an AccountDomainBlock record, + # it's meant to be called after such a record has been created + # synchronously, to "clean up" + def call(account, domain) + @account = account + @domain = domain + + reject_existing_followers! + reject_pending_follow_requests! + end + + private + + def reject_existing_followers! + @account.passive_relationships.where(account: Account.where(domain: @domain)).includes(:account).find_each do |follow| + reject_follow!(follow) + end + end + + def reject_pending_follow_requests! + FollowRequest.where(target_account: @account).where(account: Account.where(domain: @domain)).includes(:account).find_each do |follow_request| + reject_follow!(follow_request) + end + end + + def reject_follow!(follow) + follow.destroy + + return unless follow.account.activitypub? + + json = Oj.dump(ActivityPub::LinkedDataSignature.new(ActiveModelSerializers::SerializableResource.new( + follow, + serializer: ActivityPub::RejectFollowSerializer, + adapter: ActivityPub::Adapter + ).as_json).sign!(@account)) + + ActivityPub::DeliveryWorker.perform_async(json, @account.id, follow.account.inbox_url) + end +end diff --git a/app/services/block_domain_from_account_service.rb b/app/services/block_domain_from_account_service.rb deleted file mode 100644 index cae7abcbd..000000000 --- a/app/services/block_domain_from_account_service.rb +++ /dev/null @@ -1,8 +0,0 @@ -# frozen_string_literal: true - -class BlockDomainFromAccountService < BaseService - def call(account, domain) - account.block_domain!(domain) - account.passive_relationships.where(account: Account.where(domain: domain)).delete_all - end -end diff --git a/app/workers/after_account_domain_block_worker.rb b/app/workers/after_account_domain_block_worker.rb new file mode 100644 index 000000000..043c33311 --- /dev/null +++ b/app/workers/after_account_domain_block_worker.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AfterAccountDomainBlockWorker + include Sidekiq::Worker + + def perform(account_id, domain) + AfterBlockDomainFromAccountService.new.call(Account.find(account_id), domain) + rescue ActiveRecord::RecordNotFound + true + end +end diff --git a/app/workers/soft_block_domain_followers_worker.rb b/app/workers/soft_block_domain_followers_worker.rb deleted file mode 100644 index 85445c7fb..000000000 --- a/app/workers/soft_block_domain_followers_worker.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -class SoftBlockDomainFollowersWorker - include Sidekiq::Worker - - sidekiq_options queue: 'pull' - - def perform(account_id, domain) - followers_id = Account.find(account_id).followers.where(domain: domain).pluck(:id) - SoftBlockWorker.push_bulk(followers_id) do |follower_id| - [account_id, follower_id] - end - end -end diff --git a/app/workers/soft_block_worker.rb b/app/workers/soft_block_worker.rb deleted file mode 100644 index 312d880b9..000000000 --- a/app/workers/soft_block_worker.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -class SoftBlockWorker - include Sidekiq::Worker - - sidekiq_options queue: 'pull' - - def perform(account_id, target_account_id) - account = Account.find(account_id) - target_account = Account.find(target_account_id) - - BlockService.new.call(account, target_account) - UnblockService.new.call(account, target_account) - rescue ActiveRecord::RecordNotFound - true - end -end diff --git a/spec/services/after_block_domain_from_account_service_spec.rb b/spec/services/after_block_domain_from_account_service_spec.rb new file mode 100644 index 000000000..006e3f4d2 --- /dev/null +++ b/spec/services/after_block_domain_from_account_service_spec.rb @@ -0,0 +1,25 @@ +require 'rails_helper' + +RSpec.describe AfterBlockDomainFromAccountService, type: :service do + let!(:wolf) { Fabricate(:account, username: 'wolf', domain: 'evil.org', inbox_url: 'https://evil.org/inbox', protocol: :activitypub) } + let!(:alice) { Fabricate(:account, username: 'alice') } + + subject { AfterBlockDomainFromAccountService.new } + + before do + stub_jsonld_contexts! + allow(ActivityPub::DeliveryWorker).to receive(:perform_async) + end + + it 'purge followers from blocked domain' do + wolf.follow!(alice) + subject.call(alice, 'evil.org') + expect(wolf.following?(alice)).to be false + end + + it 'sends Reject->Follow to followers from blocked domain' do + wolf.follow!(alice) + subject.call(alice, 'evil.org') + expect(ActivityPub::DeliveryWorker).to have_received(:perform_async).once + end +end diff --git a/spec/services/block_domain_from_account_service_spec.rb b/spec/services/block_domain_from_account_service_spec.rb deleted file mode 100644 index 365c0a4ad..000000000 --- a/spec/services/block_domain_from_account_service_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -require 'rails_helper' - -RSpec.describe BlockDomainFromAccountService, type: :service do - let!(:wolf) { Fabricate(:account, username: 'wolf', domain: 'evil.org') } - let!(:alice) { Fabricate(:account, username: 'alice') } - - subject { BlockDomainFromAccountService.new } - - it 'creates domain block' do - subject.call(alice, 'evil.org') - expect(alice.domain_blocking?('evil.org')).to be true - end - - it 'purge followers from blocked domain' do - wolf.follow!(alice) - subject.call(alice, 'evil.org') - expect(wolf.following?(alice)).to be false - end -end