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
This commit is contained in:
ThibG
2019-05-14 19:05:02 +02:00
committed by Eugen Rochko
parent 564106c5d6
commit 14f6ce2885
30 changed files with 226 additions and 115 deletions

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@ -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
it 'unsilences accounts and removes block' do
@domain_block.update(severity: :silence)
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
context 'with retroactive' do
it 'unsilences accounts and removes block' do
@domain_block.update(severity: :silence)
it 'unsuspends accounts and removes block' do
@domain_block.update(severity: :suspend)
subject.call(@domain_block, true)
expect_deleted_domain_block
expect(@silenced.reload.silenced).to be false
expect(@suspended.reload.suspended).to be true
end
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