]> cat aescling's git repositories - mastodon.git/commitdiff
Fix domain hiding logic (#7765)
authorEugen Rochko <eugen@zeonfederated.com>
Sat, 9 Jun 2018 20:46:54 +0000 (22:46 +0200)
committerGitHub <noreply@github.com>
Sat, 9 Jun 2018 20:46:54 +0000 (22:46 +0200)
* 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

12 files changed:
app/controllers/api/v1/domain_blocks_controller.rb
app/controllers/settings/follower_domains_controller.rb
app/javascript/mastodon/features/account_timeline/containers/header_container.js
app/javascript/mastodon/locales/defaultMessages.json
app/javascript/mastodon/locales/en.json
app/services/after_block_domain_from_account_service.rb [new file with mode: 0644]
app/services/block_domain_from_account_service.rb [deleted file]
app/workers/after_account_domain_block_worker.rb [new file with mode: 0644]
app/workers/soft_block_domain_followers_worker.rb [deleted file]
app/workers/soft_block_worker.rb [deleted file]
spec/services/after_block_domain_from_account_service_spec.rb [new file with mode: 0644]
spec/services/block_domain_from_account_service_spec.rb [deleted file]

index ae6ad7936a66088c8a38f9eaf14d129950f8b700..e55d622c3a7a76418aeb10f641e240c334d20a93 100644 (file)
@@ -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
 
index 91b521e7ff0c9e894f101a059f669287d88301a3..a128bd1361f7f89785660805b500657d14a8b9f0 100644 (file)
@@ -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
 
index 4d53082191496aa61ee0b059715af88df52e7d67..7681430b789175665dd92078ddd12a9dceefff1c 100644 (file)
@@ -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)),
     }));
index ab2f3475bf4ef3b1e956d11d23cdb6c9845366f6..2fdf99a4b2bdc4f107ea8a13e33003bcbcefd64d 100644 (file)
         "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"
       }
     ],
index 855a29622ea80fc42289737e1c079cf9d7500182..3a03fe27afa8661f890439008de83db59d63537d 100644 (file)
@@ -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 (file)
index 0000000..0f1a850
--- /dev/null
@@ -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 (file)
index cae7abc..0000000
+++ /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 (file)
index 0000000..043c333
--- /dev/null
@@ -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 (file)
index 85445c7..0000000
+++ /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 (file)
index 312d880..0000000
+++ /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 (file)
index 0000000..006e3f4
--- /dev/null
@@ -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 (file)
index 365c0a4..0000000
+++ /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