From 8556a649d58a7291db6942a2594533f9b8c06165 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 15 Dec 2022 17:45:02 +0100 Subject: [PATCH] Fix changing domain block severity not undoing individual account effects (#22135) * Fix changing domain block severity not undoing individual account effects Fixes #22133 * Add tests --- .../admin/domain_blocks_controller.rb | 8 +--- .../api/v1/admin/domain_blocks_controller.rb | 6 +-- .../admin/domain_blocks_controller_spec.rb | 47 +++++++++++++++++++ .../v1/admin/domain_blocks_controller_spec.rb | 47 +++++++++++++++++++ 4 files changed, 98 insertions(+), 10 deletions(-) diff --git a/app/controllers/admin/domain_blocks_controller.rb b/app/controllers/admin/domain_blocks_controller.rb index e79f7a43e1e..74764640b8f 100644 --- a/app/controllers/admin/domain_blocks_controller.rb +++ b/app/controllers/admin/domain_blocks_controller.rb @@ -55,12 +55,8 @@ module Admin def update authorize :domain_block, :update? - @domain_block.update(update_params) - - severity_changed = @domain_block.severity_changed? - - if @domain_block.save - DomainBlockWorker.perform_async(@domain_block.id, severity_changed) + if @domain_block.update(update_params) + DomainBlockWorker.perform_async(@domain_block.id, @domain_block.severity_previously_changed?) log_action :update, @domain_block redirect_to admin_instances_path(limited: '1'), notice: I18n.t('admin.domain_blocks.created_msg') else diff --git a/app/controllers/api/v1/admin/domain_blocks_controller.rb b/app/controllers/api/v1/admin/domain_blocks_controller.rb index df5b1b3fcb8..8b77e9717da 100644 --- a/app/controllers/api/v1/admin/domain_blocks_controller.rb +++ b/app/controllers/api/v1/admin/domain_blocks_controller.rb @@ -40,10 +40,8 @@ class Api::V1::Admin::DomainBlocksController < Api::BaseController def update authorize @domain_block, :update? - @domain_block.update(domain_block_params) - severity_changed = @domain_block.severity_changed? - @domain_block.save! - DomainBlockWorker.perform_async(@domain_block.id, severity_changed) + @domain_block.update!(domain_block_params) + DomainBlockWorker.perform_async(@domain_block.id, @domain_block.severity_previously_changed?) log_action :update, @domain_block render json: @domain_block, serializer: REST::Admin::DomainBlockSerializer end diff --git a/spec/controllers/admin/domain_blocks_controller_spec.rb b/spec/controllers/admin/domain_blocks_controller_spec.rb index 98cda50047c..f432060d986 100644 --- a/spec/controllers/admin/domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/domain_blocks_controller_spec.rb @@ -70,6 +70,53 @@ RSpec.describe Admin::DomainBlocksController, type: :controller do end end + describe 'PUT #update' do + let!(:remote_account) { Fabricate(:account, domain: 'example.com') } + let(:domain_block) { Fabricate(:domain_block, domain: 'example.com', severity: original_severity) } + + before do + BlockDomainService.new.call(domain_block) + end + + let(:subject) do + post :update, params: { id: domain_block.id, domain_block: { domain: 'example.com', severity: new_severity } } + end + + context 'downgrading a domain suspension to silence' do + let(:original_severity) { 'suspend' } + let(:new_severity) { 'silence' } + + it 'changes the block severity' do + expect { subject }.to change { domain_block.reload.severity }.from('suspend').to('silence') + end + + it 'undoes individual suspensions' do + expect { subject }.to change { remote_account.reload.suspended? }.from(true).to(false) + end + + it 'performs individual silences' do + expect { subject }.to change { remote_account.reload.silenced? }.from(false).to(true) + end + end + + context 'upgrading a domain silence to suspend' do + let(:original_severity) { 'silence' } + let(:new_severity) { 'suspend' } + + it 'changes the block severity' do + expect { subject }.to change { domain_block.reload.severity }.from('silence').to('suspend') + end + + it 'undoes individual silences' do + expect { subject }.to change { remote_account.reload.silenced? }.from(true).to(false) + end + + it 'performs individual suspends' do + expect { subject }.to change { remote_account.reload.suspended? }.from(false).to(true) + end + end + end + describe 'DELETE #destroy' do it 'unblocks the domain' do service = double(call: true) diff --git a/spec/controllers/api/v1/admin/domain_blocks_controller_spec.rb b/spec/controllers/api/v1/admin/domain_blocks_controller_spec.rb index f12285b2a62..606def602fc 100644 --- a/spec/controllers/api/v1/admin/domain_blocks_controller_spec.rb +++ b/spec/controllers/api/v1/admin/domain_blocks_controller_spec.rb @@ -71,6 +71,53 @@ RSpec.describe Api::V1::Admin::DomainBlocksController, type: :controller do end end + describe 'PUT #update' do + let!(:remote_account) { Fabricate(:account, domain: 'example.com') } + let(:domain_block) { Fabricate(:domain_block, domain: 'example.com', severity: original_severity) } + + before do + BlockDomainService.new.call(domain_block) + end + + let(:subject) do + post :update, params: { id: domain_block.id, domain: 'example.com', severity: new_severity } + end + + context 'downgrading a domain suspension to silence' do + let(:original_severity) { 'suspend' } + let(:new_severity) { 'silence' } + + it 'changes the block severity' do + expect { subject }.to change { domain_block.reload.severity }.from('suspend').to('silence') + end + + it 'undoes individual suspensions' do + expect { subject }.to change { remote_account.reload.suspended? }.from(true).to(false) + end + + it 'performs individual silences' do + expect { subject }.to change { remote_account.reload.silenced? }.from(false).to(true) + end + end + + context 'upgrading a domain silence to suspend' do + let(:original_severity) { 'silence' } + let(:new_severity) { 'suspend' } + + it 'changes the block severity' do + expect { subject }.to change { domain_block.reload.severity }.from('silence').to('suspend') + end + + it 'undoes individual silences' do + expect { subject }.to change { remote_account.reload.silenced? }.from(true).to(false) + end + + it 'performs individual suspends' do + expect { subject }.to change { remote_account.reload.suspended? }.from(false).to(true) + end + end + end + describe 'DELETE #destroy' do let!(:block) { Fabricate(:domain_block) }