diff --git a/app/controllers/api/v1/admin/domain_blocks_controller.rb b/app/controllers/api/v1/admin/domain_blocks_controller.rb index 2538c7c7c..8786431d9 100644 --- a/app/controllers/api/v1/admin/domain_blocks_controller.rb +++ b/app/controllers/api/v1/admin/domain_blocks_controller.rb @@ -29,10 +29,11 @@ class Api::V1::Admin::DomainBlocksController < Api::BaseController def create authorize :domain_block, :create? + @domain_block = DomainBlock.new(resource_params) existing_domain_block = resource_params[:domain].present? ? DomainBlock.rule_for(resource_params[:domain]) : nil - return render json: existing_domain_block, serializer: REST::Admin::ExistingDomainBlockErrorSerializer, status: 422 if existing_domain_block.present? + return render json: existing_domain_block, serializer: REST::Admin::ExistingDomainBlockErrorSerializer, status: 422 if conflicts_with_existing_block?(@domain_block, existing_domain_block) - @domain_block = DomainBlock.create!(resource_params) + @domain_block.save! DomainBlockWorker.perform_async(@domain_block.id) log_action :create, @domain_block render json: @domain_block, serializer: REST::Admin::DomainBlockSerializer @@ -55,6 +56,10 @@ class Api::V1::Admin::DomainBlocksController < Api::BaseController private + def conflicts_with_existing_block?(domain_block, existing_domain_block) + existing_domain_block.present? && (existing_domain_block.domain == TagManager.instance.normalize_domain(domain_block.domain) || !domain_block.stricter_than?(existing_domain_block)) + end + def set_domain_blocks @domain_blocks = filtered_domain_blocks.order(id: :desc).to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) end diff --git a/spec/requests/api/v1/admin/domain_blocks_spec.rb b/spec/requests/api/v1/admin/domain_blocks_spec.rb index a6cfdb48d..58641c20d 100644 --- a/spec/requests/api/v1/admin/domain_blocks_spec.rb +++ b/spec/requests/api/v1/admin/domain_blocks_spec.rb @@ -135,14 +135,10 @@ RSpec.describe 'Domain Blocks' do it_behaves_like 'forbidden for wrong role', '' it_behaves_like 'forbidden for wrong role', 'Moderator' - it 'returns http success' do + it 'creates a domain block with the expected domain name and severity', :aggregate_failures do subject expect(response).to have_http_status(200) - end - - it 'returns expected domain name and severity' do - subject body = body_as_json @@ -160,7 +156,44 @@ RSpec.describe 'Domain Blocks' do expect(DomainBlock.find_by(domain: 'foo.bar.com')).to be_present end - context 'when a stricter domain block already exists' do + context 'when a looser domain block already exists on a higher level domain' do + let(:params) { { domain: 'foo.bar.com', severity: :suspend } } + + before do + Fabricate(:domain_block, domain: 'bar.com', severity: :silence) + end + + it 'creates a domain block with the expected domain name and severity', :aggregate_failures do + subject + + body = body_as_json + + expect(response).to have_http_status(200) + expect(body).to match a_hash_including( + { + domain: 'foo.bar.com', + severity: 'suspend', + } + ) + + expect(DomainBlock.find_by(domain: 'foo.bar.com')).to be_present + end + end + + context 'when a domain block already exists on the same domain' do + before do + Fabricate(:domain_block, domain: 'foo.bar.com', severity: :silence) + end + + it 'returns existing domain block in error', :aggregate_failures do + subject + + expect(response).to have_http_status(422) + expect(body_as_json[:existing_domain_block][:domain]).to eq('foo.bar.com') + end + end + + context 'when a stricter domain block already exists on a higher level domain' do before do Fabricate(:domain_block, domain: 'bar.com', severity: :suspend) end