Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a link to existing domain block when trying to block an already-blocked domain #10663

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 17 additions & 5 deletions app/controllers/admin/domain_blocks_controller.rb
Expand Up @@ -13,13 +13,25 @@ def create
authorize :domain_block, :create?

@domain_block = DomainBlock.new(resource_params)
existing_domain_block = resource_params[:domain].present? ? DomainBlock.find_by(domain: resource_params[:domain]) : nil

if @domain_block.save
DomainBlockWorker.perform_async(@domain_block.id)
log_action :create, @domain_block
redirect_to admin_instances_path(limited: '1'), notice: I18n.t('admin.domain_blocks.created_msg')
else
if existing_domain_block.present? && !@domain_block.stricter_than?(existing_domain_block)
@domain_block.save
flash[:alert] = I18n.t('admin.domain_blocks.existing_domain_block_html', name: existing_domain_block.domain, unblock_url: admin_domain_block_path(existing_domain_block)).html_safe # rubocop:disable Rails/OutputSafety
@domain_block.errors[:domain].clear
render :new
else
if existing_domain_block.present?
@domain_block = existing_domain_block
@domain_block.update(resource_params)
end
if @domain_block.save
DomainBlockWorker.perform_async(@domain_block.id)
log_action :create, @domain_block
redirect_to admin_instances_path(limited: '1'), notice: I18n.t('admin.domain_blocks.created_msg')
else
render :new
end
end
end

Expand Down
11 changes: 11 additions & 0 deletions app/javascript/styles/mastodon/forms.scss
Expand Up @@ -533,6 +533,17 @@ code {
color: $error-value-color;
}

a {
display: inline-block;
color: $darker-text-color;
text-decoration: none;

&:hover {
color: $primary-text-color;
text-decoration: underline;
}
}

p {
margin-bottom: 15px;
}
Expand Down
7 changes: 7 additions & 0 deletions app/models/domain_block.rb
Expand Up @@ -29,4 +29,11 @@ class DomainBlock < ApplicationRecord
def self.blocked?(domain)
where(domain: domain, severity: :suspend).exists?
end

def stricter_than?(other_block)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could overload the > method here and that would be fun, but probably best not to

return true if suspend?
return false if other_block.suspend? && (silence? || noop?)
return false if other_block.silence? && noop?
(reject_media || !other_block.reject_media) && (reject_reports || !other_block.reject_reports)
end
end
1 change: 1 addition & 0 deletions config/locales/en.yml
Expand Up @@ -269,6 +269,7 @@ en:
created_msg: Domain block is now being processed
destroyed_msg: Domain block has been undone
domain: Domain
existing_domain_block_html: You have already imposed stricter limits on %{name}, you need to <a href="%{unblock_url}">unblock it</a> first.
new:
create: Create block
hint: The domain block will not prevent creation of account entries in the database, but will retroactively and automatically apply specific moderation methods on those accounts.
Expand Down
13 changes: 12 additions & 1 deletion spec/controllers/admin/domain_blocks_controller_spec.rb
Expand Up @@ -37,14 +37,25 @@
end

it 'renders new when failed to save' do
Fabricate(:domain_block, domain: 'example.com')
Fabricate(:domain_block, domain: 'example.com', severity: 'suspend')
allow(DomainBlockWorker).to receive(:perform_async).and_return(true)

post :create, params: { domain_block: { domain: 'example.com', severity: 'silence' } }

expect(DomainBlockWorker).not_to have_received(:perform_async)
expect(response).to render_template :new
end

it 'allows upgrading a block' do
Fabricate(:domain_block, domain: 'example.com', severity: 'silence')
allow(DomainBlockWorker).to receive(:perform_async).and_return(true)

post :create, params: { domain_block: { domain: 'example.com', severity: 'silence', reject_media: true, reject_reports: true } }

expect(DomainBlockWorker).to have_received(:perform_async)
expect(flash[:notice]).to eq I18n.t('admin.domain_blocks.created_msg')
expect(response).to redirect_to(admin_instances_path(limited: '1'))
end
end

describe 'DELETE #destroy' do
Expand Down
31 changes: 31 additions & 0 deletions spec/models/domain_block_spec.rb
Expand Up @@ -36,4 +36,35 @@
expect(DomainBlock.blocked?('domain')).to eq false
end
end

describe 'stricter_than?' do
it 'returns true if the new block has suspend severity while the old has lower severity' do
suspend = DomainBlock.new(domain: 'domain', severity: :suspend)
silence = DomainBlock.new(domain: 'domain', severity: :silence)
noop = DomainBlock.new(domain: 'domain', severity: :noop)
expect(suspend.stricter_than?(silence)).to be true
expect(suspend.stricter_than?(noop)).to be true
end

it 'returns false if the new block has lower severity than the old one' do
suspend = DomainBlock.new(domain: 'domain', severity: :suspend)
silence = DomainBlock.new(domain: 'domain', severity: :silence)
noop = DomainBlock.new(domain: 'domain', severity: :noop)
expect(silence.stricter_than?(suspend)).to be false
expect(noop.stricter_than?(suspend)).to be false
expect(noop.stricter_than?(silence)).to be false
end

it 'returns false if the new block does is less strict regarding reports' do
older = DomainBlock.new(domain: 'domain', severity: :silence, reject_reports: true)
newer = DomainBlock.new(domain: 'domain', severity: :silence, reject_reports: false)
expect(newer.stricter_than?(older)).to be false
end

it 'returns false if the new block does is less strict regarding media' do
older = DomainBlock.new(domain: 'domain', severity: :silence, reject_media: true)
newer = DomainBlock.new(domain: 'domain', severity: :silence, reject_media: false)
expect(newer.stricter_than?(older)).to be false
end
end
end