From 14f6ce2885f7999f2fcbbdda6241a035271076d4 Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 14 May 2019 19:05:02 +0200 Subject: Record account suspend/silence time and keep track of domain blocks (#10660) * Record account suspend/silence time and keep track of domain blocks * Also unblock users who were suspended/silenced before dates were recorded * Add tests * Keep track of suspending date for users suspended through the CLI * Show accurate number of accounts that would be affected by unsuspending an instance * Change migration to set silenced_at and suspended_at * Revert "Also unblock users who were suspended/silenced before dates were recorded" This reverts commit a015c65d2d1e28c7b7cfab8b3f8cd5fb48b8b71c. * Switch from using suspended and silenced to suspended_at and silenced_at * Add post-deployment migration script to remove `suspended` and `silenced` columns * Use Account#silence! and Account#suspend! instead of updating the underlying property * Add silenced_at and suspended_at migration to post-migration * Change account fabricator to translate suspended and silenced attributes * Minor fixes * Make unblocking domains always retroactive --- .../admin/domain_blocks_controller_spec.rb | 4 +- spec/fabricators/account_fabricator.rb | 3 ++ spec/lib/feed_manager_spec.rb | 4 +- spec/lib/status_filter_spec.rb | 4 +- .../concerns/status_threading_concern_spec.rb | 4 +- spec/services/block_domain_service_spec.rb | 40 +++++++++++++------ spec/services/notify_service_spec.rb | 4 +- spec/services/unblock_domain_service_spec.rb | 45 ++++++++++------------ 8 files changed, 63 insertions(+), 45 deletions(-) (limited to 'spec') diff --git a/spec/controllers/admin/domain_blocks_controller_spec.rb b/spec/controllers/admin/domain_blocks_controller_spec.rb index 2a8675c21..fb23658c0 100644 --- a/spec/controllers/admin/domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/domain_blocks_controller_spec.rb @@ -63,9 +63,9 @@ RSpec.describe Admin::DomainBlocksController, type: :controller do service = double(call: true) allow(UnblockDomainService).to receive(:new).and_return(service) domain_block = Fabricate(:domain_block) - delete :destroy, params: { id: domain_block.id, domain_block: { retroactive: '1' } } + delete :destroy, params: { id: domain_block.id } - expect(service).to have_received(:call).with(domain_block, true) + expect(service).to have_received(:call).with(domain_block) expect(flash[:notice]).to eq I18n.t('admin.domain_blocks.destroyed_msg') expect(response).to redirect_to(admin_instances_path(limited: '1')) end diff --git a/spec/fabricators/account_fabricator.rb b/spec/fabricators/account_fabricator.rb index e092e6c09..f12464ef3 100644 --- a/spec/fabricators/account_fabricator.rb +++ b/spec/fabricators/account_fabricator.rb @@ -3,8 +3,11 @@ public_key = keypair.public_key.to_pem private_key = keypair.to_pem Fabricator(:account) do + transient :suspended, :silenced username { sequence(:username) { |i| "#{Faker::Internet.user_name(nil, %w(_))}#{i}" } } last_webfingered_at { Time.now.utc } public_key { public_key } private_key { private_key } + suspended_at { |attrs| attrs[:suspended] ? Time.now.utc : nil } + silenced_at { |attrs| attrs[:silenced] ? Time.now.utc : nil } end diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb index c506cd87f..5f8eb86a8 100644 --- a/spec/lib/feed_manager_spec.rb +++ b/spec/lib/feed_manager_spec.rb @@ -168,13 +168,13 @@ RSpec.describe FeedManager do it 'returns true for status by silenced account who recipient is not following' do status = Fabricate(:status, text: 'Hello world', account: alice) - alice.update(silenced: true) + alice.silence! expect(FeedManager.instance.filter?(:mentions, status, bob.id)).to be true end it 'returns false for status by followed silenced account' do status = Fabricate(:status, text: 'Hello world', account: alice) - alice.update(silenced: true) + alice.silence! bob.follow!(alice) expect(FeedManager.instance.filter?(:mentions, status, bob.id)).to be false end diff --git a/spec/lib/status_filter_spec.rb b/spec/lib/status_filter_spec.rb index db2d87de2..a851014d9 100644 --- a/spec/lib/status_filter_spec.rb +++ b/spec/lib/status_filter_spec.rb @@ -15,7 +15,7 @@ describe StatusFilter do context 'when status account is silenced' do before do - status.account.update(silenced: true) + status.account.silence! end it { is_expected.to be_filtered } @@ -65,7 +65,7 @@ describe StatusFilter do context 'when status account is silenced' do before do - status.account.update(silenced: true) + status.account.silence! end it { is_expected.to be_filtered } diff --git a/spec/models/concerns/status_threading_concern_spec.rb b/spec/models/concerns/status_threading_concern_spec.rb index 94c2d5fc2..50286ef77 100644 --- a/spec/models/concerns/status_threading_concern_spec.rb +++ b/spec/models/concerns/status_threading_concern_spec.rb @@ -35,7 +35,7 @@ describe StatusThreadingConcern do end it 'does not return conversation history from silenced and not followed users' do - jeff.update(silenced: true) + jeff.silence! expect(reply3.ancestors(4, viewer)).to_not include(reply1) end @@ -110,7 +110,7 @@ describe StatusThreadingConcern do end it 'does not return replies from silenced and not followed users' do - jeff.update(silenced: true) + jeff.silence! expect(status.descendants(4, viewer)).to_not include(reply3) end diff --git a/spec/services/block_domain_service_spec.rb b/spec/services/block_domain_service_spec.rb index 7ef9e2770..c689b57e3 100644 --- a/spec/services/block_domain_service_spec.rb +++ b/spec/services/block_domain_service_spec.rb @@ -1,20 +1,14 @@ require 'rails_helper' RSpec.describe BlockDomainService, type: :service do - let(:bad_account) { Fabricate(:account, username: 'badguy666', domain: 'evil.org') } - let(:bad_status1) { Fabricate(:status, account: bad_account, text: 'You suck') } - let(:bad_status2) { Fabricate(:status, account: bad_account, text: 'Hahaha') } - let(:bad_attachment) { Fabricate(:media_attachment, account: bad_account, status: bad_status2, file: attachment_fixture('attachment.jpg')) } + let!(:bad_account) { Fabricate(:account, username: 'badguy666', domain: 'evil.org') } + let!(:bad_status1) { Fabricate(:status, account: bad_account, text: 'You suck') } + let!(:bad_status2) { Fabricate(:status, account: bad_account, text: 'Hahaha') } + let!(:bad_attachment) { Fabricate(:media_attachment, account: bad_account, status: bad_status2, file: attachment_fixture('attachment.jpg')) } + let!(:already_banned_account) { Fabricate(:account, username: 'badguy', domain: 'evil.org', suspended: true, silenced: true) } subject { BlockDomainService.new } - before do - bad_account - bad_status1 - bad_status2 - bad_attachment - end - describe 'for a suspension' do before do subject.call(DomainBlock.create!(domain: 'evil.org', severity: :suspend)) @@ -28,6 +22,18 @@ RSpec.describe BlockDomainService, type: :service do expect(Account.find_remote('badguy666', 'evil.org').suspended?).to be true end + it 'records suspension date appropriately' do + expect(Account.find_remote('badguy666', 'evil.org').suspended_at).to eq DomainBlock.find_by(domain: 'evil.org').created_at + end + + it 'keeps already-banned accounts banned' do + expect(Account.find_remote('badguy', 'evil.org').suspended?).to be true + end + + it 'does not overwrite suspension date of already-banned accounts' do + expect(Account.find_remote('badguy', 'evil.org').suspended_at).to_not eq DomainBlock.find_by(domain: 'evil.org').created_at + 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 @@ -48,6 +54,18 @@ RSpec.describe BlockDomainService, type: :service do expect(Account.find_remote('badguy666', 'evil.org').silenced?).to be true end + it 'records suspension date appropriately' do + expect(Account.find_remote('badguy666', 'evil.org').silenced_at).to eq DomainBlock.find_by(domain: 'evil.org').created_at + end + + it 'keeps already-banned accounts banned' do + expect(Account.find_remote('badguy', 'evil.org').silenced?).to be true + end + + it 'does not overwrite suspension date of already-banned accounts' do + expect(Account.find_remote('badguy', 'evil.org').silenced_at).to_not eq DomainBlock.find_by(domain: 'evil.org').created_at + 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 diff --git a/spec/services/notify_service_spec.rb b/spec/services/notify_service_spec.rb index 39a681abb..a387d9407 100644 --- a/spec/services/notify_service_spec.rb +++ b/spec/services/notify_service_spec.rb @@ -39,12 +39,12 @@ RSpec.describe NotifyService, type: :service do end it 'does not notify when sender is silenced and not followed' do - sender.update(silenced: true) + sender.silence! is_expected.to_not change(Notification, :count) end it 'does not notify when recipient is suspended' do - recipient.update(suspended: true) + recipient.suspend! is_expected.to_not change(Notification, :count) end diff --git a/spec/services/unblock_domain_service_spec.rb b/spec/services/unblock_domain_service_spec.rb index 8e8893d63..619aefb5c 100644 --- a/spec/services/unblock_domain_service_spec.rb +++ b/spec/services/unblock_domain_service_spec.rb @@ -7,36 +7,33 @@ describe UnblockDomainService, type: :service do describe 'call' do before do - @silenced = Fabricate(:account, domain: 'example.com', silenced: true) - @suspended = Fabricate(:account, domain: 'example.com', suspended: true) + @independently_suspended = Fabricate(:account, domain: 'example.com', suspended_at: 1.hour.ago) + @independently_silenced = Fabricate(:account, domain: 'example.com', silenced_at: 1.hour.ago) @domain_block = Fabricate(:domain_block, domain: 'example.com') + @silenced = Fabricate(:account, domain: 'example.com', silenced_at: @domain_block.created_at) + @suspended = Fabricate(:account, domain: 'example.com', suspended_at: @domain_block.created_at) end - context 'without retroactive' do - it 'removes the domain block' do - subject.call(@domain_block, false) - expect_deleted_domain_block - end - end - - context 'with retroactive' do - it 'unsilences accounts and removes block' do - @domain_block.update(severity: :silence) + it 'unsilences accounts and removes block' do + @domain_block.update(severity: :silence) - subject.call(@domain_block, true) - expect_deleted_domain_block - expect(@silenced.reload.silenced).to be false - expect(@suspended.reload.suspended).to be true - end + subject.call(@domain_block) + expect_deleted_domain_block + expect(@silenced.reload.silenced?).to be false + expect(@suspended.reload.suspended?).to be true + expect(@independently_suspended.reload.suspended?).to be true + expect(@independently_silenced.reload.silenced?).to be true + end - it 'unsuspends accounts and removes block' do - @domain_block.update(severity: :suspend) + it 'unsuspends accounts and removes block' do + @domain_block.update(severity: :suspend) - subject.call(@domain_block, true) - expect_deleted_domain_block - expect(@suspended.reload.suspended).to be false - expect(@silenced.reload.silenced).to be true - end + subject.call(@domain_block) + expect_deleted_domain_block + expect(@suspended.reload.suspended?).to be false + expect(@silenced.reload.silenced?).to be true + expect(@independently_suspended.reload.suspended?).to be true + expect(@independently_silenced.reload.silenced?).to be true end end -- cgit