Browse Source

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
Eugen Rochko 6 years ago
parent
commit
10f51c9886

+ 2 - 1
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
 

+ 1 - 1
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
 

+ 1 - 1
app/javascript/mastodon/features/account_timeline/containers/header_container.js

@@ -96,7 +96,7 @@ const mapDispatchToProps = (dispatch, { intl }) => ({
 
   onBlockDomain (domain) {
     dispatch(openModal('CONFIRM', {
-      message: <FormattedMessage id='confirmations.domain_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.' values={{ domain: <strong>{domain}</strong> }} />,
+      message: <FormattedMessage id='confirmations.domain_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. You will not see content from that domain in any public timelines or your notifications. Your followers from that domain will be removed.' values={{ domain: <strong>{domain}</strong> }} />,
       confirm: intl.formatMessage(messages.blockDomainConfirm),
       onConfirm: () => dispatch(blockDomain(domain)),
     }));

+ 1 - 1
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"
       }
     ],

+ 1 - 1
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",

+ 42 - 0
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

+ 0 - 8
app/services/block_domain_from_account_service.rb

@@ -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

+ 11 - 0
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

+ 0 - 14
app/workers/soft_block_domain_followers_worker.rb

@@ -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

+ 0 - 17
app/workers/soft_block_worker.rb

@@ -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

+ 25 - 0
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

+ 0 - 19
spec/services/block_domain_from_account_service_spec.rb

@@ -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