commit: 8857cabca42fc21d3063862c9954e079e7efd275
parent: affd75936e069a28247274683e6015d2b66910c1
Author: Matt Jankowski <mjankowski@thoughtbot.com>
Date: Wed, 26 Apr 2017 14:09:01 -0400
Domain block service cleanup (#2490)
* Add coverage for domain block service with silence
* Get rid of warning about find_each and order
* Move domain_block to attr_reader
* Move optional clear_media into silence_accounts method
* Use blocked_domain method to reduce passed vars
* Extract blocked_domain_accounts method to find accounts on the domain
* Extract media_from_blocked_domain method to find relevant attachments
* Separate destruction of account images and account attachments
Diffstat:
2 files changed, 76 insertions(+), 25 deletions(-)
diff --git a/app/services/block_domain_service.rb b/app/services/block_domain_service.rb
@@ -1,36 +1,62 @@
# frozen_string_literal: true
class BlockDomainService < BaseService
+ attr_reader :domain_block
+
def call(domain_block)
+ @domain_block = domain_block
+ process_domain_block
+ end
+
+ private
+
+ def process_domain_block
if domain_block.silence?
- silence_accounts!(domain_block.domain)
- clear_media!(domain_block.domain) if domain_block.reject_media?
+ silence_accounts!
else
- suspend_accounts!(domain_block.domain)
+ suspend_accounts!
end
end
- private
+ def silence_accounts!
+ blocked_domain_accounts.update_all(silenced: true)
+ clear_media! if domain_block.reject_media?
+ end
- def silence_accounts!(domain)
- Account.where(domain: domain).update_all(silenced: true)
+ def clear_media!
+ clear_account_images
+ clear_account_attachments
+ end
+
+ def suspend_accounts!
+ blocked_domain_accounts.where(suspended: false).find_each do |account|
+ account.subscription(api_subscription_url(account.id)).unsubscribe if account.subscribed?
+ SuspendAccountService.new.call(account)
+ end
end
- def clear_media!(domain)
- Account.where(domain: domain).find_each do |account|
+ def clear_account_images
+ blocked_domain_accounts.find_each do |account|
account.avatar.destroy
account.header.destroy
end
+ end
- MediaAttachment.where(account: Account.where(domain: domain)).find_each do |attachment|
+ def clear_account_attachments
+ media_from_blocked_domain.find_each do |attachment|
attachment.file.destroy
end
end
- def suspend_accounts!(domain)
- Account.where(domain: domain).where(suspended: false).find_each do |account|
- account.subscription(api_subscription_url(account.id)).unsubscribe if account.subscribed?
- SuspendAccountService.new.call(account)
- end
+ def blocked_domain
+ domain_block.domain
+ end
+
+ def blocked_domain_accounts
+ Account.where(domain: blocked_domain)
+ end
+
+ def media_from_blocked_domain
+ MediaAttachment.where(account: blocked_domain_accounts).reorder(nil)
end
end
diff --git a/spec/services/block_domain_service_spec.rb b/spec/services/block_domain_service_spec.rb
@@ -13,21 +13,46 @@ RSpec.describe BlockDomainService do
bad_status1
bad_status2
bad_attachment
-
- subject.call(DomainBlock.create!(domain: 'evil.org', severity: :suspend))
end
- it 'creates a domain block' do
- expect(DomainBlock.blocked?('evil.org')).to be true
- end
+ describe 'for a suspension' do
+ before do
+ subject.call(DomainBlock.create!(domain: 'evil.org', severity: :suspend))
+ end
+
+ it 'creates a domain block' do
+ expect(DomainBlock.blocked?('evil.org')).to be true
+ end
+
+ it 'removes remote accounts from that domain' do
+ expect(Account.find_remote('badguy666', 'evil.org').suspended?).to be true
+ end
- it 'removes remote accounts from that domain' do
- expect(Account.find_remote('badguy666', 'evil.org').suspended?).to be true
+ it 'removes the remote accounts\'s statuses and media attachments' do
+ expect { bad_status1.reload }.to raise_exception ActiveRecord::RecordNotFound
+ expect { bad_status2.reload }.to raise_exception ActiveRecord::RecordNotFound
+ expect { bad_attachment.reload }.to raise_exception ActiveRecord::RecordNotFound
+ end
end
- it 'removes the remote accounts\'s statuses and media attachments' do
- expect { bad_status1.reload }.to raise_exception ActiveRecord::RecordNotFound
- expect { bad_status2.reload }.to raise_exception ActiveRecord::RecordNotFound
- expect { bad_attachment.reload }.to raise_exception ActiveRecord::RecordNotFound
+ describe 'for a silence with reject media' do
+ before do
+ subject.call(DomainBlock.create!(domain: 'evil.org', severity: :silence, reject_media: true))
+ end
+
+ it 'does not create a domain block' do
+ expect(DomainBlock.blocked?('evil.org')).to be false
+ end
+
+ it 'silences remote accounts from that domain' do
+ expect(Account.find_remote('badguy666', 'evil.org').silenced?).to be true
+ end
+
+ it 'leaves the domains status and attachements, but clears media' do
+ expect { bad_status1.reload }.not_to raise_error
+ expect { bad_status2.reload }.not_to raise_error
+ expect { bad_attachment.reload }.not_to raise_error
+ expect(bad_attachment.file.exists?).to be false
+ end
end
end