From 14f6ce2885f7999f2fcbbdda6241a035271076d4 Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 14 May 2019 19:05:02 +0200 Subject: [PATCH] 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.rb | 8 +--- app/controllers/home_controller.rb | 2 +- app/models/account.rb | 40 ++++++++++------- app/models/concerns/account_finder_concern.rb | 2 +- app/models/domain_block.rb | 7 ++- app/models/status.rb | 4 +- app/models/user.rb | 2 +- .../activitypub/process_account_service.rb | 12 ++--- app/services/block_domain_service.rb | 6 +-- app/services/post_status_service.rb | 2 +- app/services/process_mentions_service.rb | 2 +- app/services/resolve_account_service.rb | 6 +-- app/services/subscribe_service.rb | 2 +- app/services/suspend_account_service.rb | 4 +- app/services/unblock_domain_service.rb | 15 ++++--- app/views/admin/domain_blocks/show.html.haml | 17 +++---- config/locales/en.yml | 4 +- ...dd_silenced_at_suspended_at_to_accounts.rb | 41 +++++++++++++++++ ...emove_suspended_silenced_account_fields.rb | 45 +++++++++++++++++++ db/schema.rb | 6 +-- lib/cli.rb | 2 +- lib/mastodon/accounts_cli.rb | 4 +- .../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 +++++++++---------- 30 files changed, 226 insertions(+), 115 deletions(-) create mode 100644 db/migrate/20190511134027_add_silenced_at_suspended_at_to_accounts.rb create mode 100644 db/post_migrate/20190511152737_remove_suspended_silenced_account_fields.rb diff --git a/app/controllers/admin/domain_blocks_controller.rb b/app/controllers/admin/domain_blocks_controller.rb index dd3f833890..71597763bf 100644 --- a/app/controllers/admin/domain_blocks_controller.rb +++ b/app/controllers/admin/domain_blocks_controller.rb @@ -41,7 +41,7 @@ module Admin def destroy authorize @domain_block, :destroy? - UnblockDomainService.new.call(@domain_block, retroactive_unblock?) + UnblockDomainService.new.call(@domain_block) log_action :destroy, @domain_block redirect_to admin_instances_path(limited: '1'), notice: I18n.t('admin.domain_blocks.destroyed_msg') end @@ -53,11 +53,7 @@ module Admin end def resource_params - params.require(:domain_block).permit(:domain, :severity, :reject_media, :reject_reports, :retroactive) - end - - def retroactive_unblock? - ActiveRecord::Type.lookup(:boolean).cast(resource_params[:retroactive]) + params.require(:domain_block).permit(:domain, :severity, :reject_media, :reject_reports) end end end diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index d1bd0601e9..85622a7b59 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -58,7 +58,7 @@ class HomeController < ApplicationController if request.path.start_with?('/web') new_user_session_path elsif single_user_mode? - short_account_path(Account.local.where(suspended: false).first) + short_account_path(Account.local.without_suspended.first) else about_path end diff --git a/app/models/account.rb b/app/models/account.rb index 51e01246e6..a894d5be54 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -28,8 +28,6 @@ # header_updated_at :datetime # avatar_remote_url :string # subscription_expires_at :datetime -# silenced :boolean default(FALSE), not null -# suspended :boolean default(FALSE), not null # locked :boolean default(FALSE), not null # header_remote_url :string default(""), not null # last_webfingered_at :datetime @@ -45,6 +43,8 @@ # actor_type :string # discoverable :boolean # also_known_as :string is an Array +# silenced_at :datetime +# suspended_at :datetime # class Account < ApplicationRecord @@ -82,10 +82,10 @@ class Account < ApplicationRecord scope :local, -> { where(domain: nil) } scope :expiring, ->(time) { remote.where.not(subscription_expires_at: nil).where('subscription_expires_at < ?', time) } scope :partitioned, -> { order(Arel.sql('row_number() over (partition by domain)')) } - scope :silenced, -> { where(silenced: true) } - scope :suspended, -> { where(suspended: true) } - scope :without_suspended, -> { where(suspended: false) } - scope :without_silenced, -> { where(silenced: false) } + scope :silenced, -> { where.not(silenced_at: nil) } + scope :suspended, -> { where.not(suspended_at: nil) } + scope :without_suspended, -> { where(suspended_at: nil) } + scope :without_silenced, -> { where(silenced_at: nil) } scope :recent, -> { reorder(id: :desc) } scope :bots, -> { where(actor_type: %w(Application Service)) } scope :alphabetic, -> { order(domain: :asc, username: :asc) } @@ -165,25 +165,35 @@ class Account < ApplicationRecord ResolveAccountService.new.call(acct) end - def silence! - update!(silenced: true) + def silenced? + silenced_at.present? + end + + def silence!(date = nil) + date ||= Time.now.utc + update!(silenced_at: date) end def unsilence! - update!(silenced: false) + update!(silenced_at: nil) end - def suspend! + def suspended? + suspended_at.present? + end + + def suspend!(date = nil) + date ||= Time.now.utc transaction do user&.disable! if local? - update!(suspended: true) + update!(suspended_at: date) end end def unsuspend! transaction do user&.enable! if local? - update!(suspended: false) + update!(suspended_at: nil) end end @@ -399,7 +409,7 @@ class Account < ApplicationRecord ts_rank_cd(#{textsearch}, #{query}, 32) AS rank FROM accounts WHERE #{query} @@ #{textsearch} - AND accounts.suspended = false + AND accounts.suspended_at IS NULL AND accounts.moved_to_account_id IS NULL ORDER BY rank DESC LIMIT ? OFFSET ? @@ -427,7 +437,7 @@ class Account < ApplicationRecord LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) OR (accounts.id = f.target_account_id AND f.account_id = ?) WHERE accounts.id IN (SELECT * FROM first_degree) AND #{query} @@ #{textsearch} - AND accounts.suspended = false + AND accounts.suspended_at IS NULL AND accounts.moved_to_account_id IS NULL GROUP BY accounts.id ORDER BY rank DESC @@ -443,7 +453,7 @@ class Account < ApplicationRecord FROM accounts LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) OR (accounts.id = f.target_account_id AND f.account_id = ?) WHERE #{query} @@ #{textsearch} - AND accounts.suspended = false + AND accounts.suspended_at IS NULL AND accounts.moved_to_account_id IS NULL GROUP BY accounts.id ORDER BY rank DESC diff --git a/app/models/concerns/account_finder_concern.rb b/app/models/concerns/account_finder_concern.rb index 0ac49cc126..ccd7bfa123 100644 --- a/app/models/concerns/account_finder_concern.rb +++ b/app/models/concerns/account_finder_concern.rb @@ -13,7 +13,7 @@ module AccountFinderConcern end def representative - find_local(Setting.site_contact_username.strip.gsub(/\A@/, '')) || Account.local.find_by(suspended: false) + find_local(Setting.site_contact_username.strip.gsub(/\A@/, '')) || Account.local.without_suspended.first end def find_local(username) diff --git a/app/models/domain_block.rb b/app/models/domain_block.rb index 0b12617c6c..84c08c1587 100644 --- a/app/models/domain_block.rb +++ b/app/models/domain_block.rb @@ -17,8 +17,6 @@ class DomainBlock < ApplicationRecord enum severity: [:silence, :suspend, :noop] - attr_accessor :retroactive - validates :domain, presence: true, uniqueness: true has_many :accounts, foreign_key: :domain, primary_key: :domain @@ -36,4 +34,9 @@ class DomainBlock < ApplicationRecord return false if other_block.silence? && noop? (reject_media || !other_block.reject_media) && (reject_reports || !other_block.reject_reports) end + + def affected_accounts_count + scope = suspend? ? accounts.where(suspended_at: created_at) : accounts.where(silenced_at: created_at) + scope.count + end end diff --git a/app/models/status.rb b/app/models/status.rb index 8d31fd3824..2f6101b908 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -84,8 +84,8 @@ class Status < ApplicationRecord scope :without_reblogs, -> { where('statuses.reblog_of_id IS NULL') } scope :with_public_visibility, -> { where(visibility: :public) } scope :tagged_with, ->(tag) { joins(:statuses_tags).where(statuses_tags: { tag_id: tag }) } - scope :excluding_silenced_accounts, -> { left_outer_joins(:account).where(accounts: { silenced: false }) } - scope :including_silenced_accounts, -> { left_outer_joins(:account).where(accounts: { silenced: true }) } + scope :excluding_silenced_accounts, -> { left_outer_joins(:account).where(accounts: { silenced_at: nil }) } + scope :including_silenced_accounts, -> { left_outer_joins(:account).where.not(accounts: { silenced_at: nil }) } scope :not_excluded_by_account, ->(account) { where.not(account_id: account.excluded_from_timeline_account_ids) } scope :not_domain_blocked_by_account, ->(account) { account.excluded_from_timeline_domains.blank? ? left_outer_joins(:account) : left_outer_joins(:account).where('accounts.domain IS NULL OR accounts.domain NOT IN (?)', account.excluded_from_timeline_domains) } scope :tagged_with_all, ->(tags) { diff --git a/app/models/user.rb b/app/models/user.rb index bce28aa5fb..6941b040de 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -88,7 +88,7 @@ class User < ApplicationRecord scope :confirmed, -> { where.not(confirmed_at: nil) } scope :enabled, -> { where(disabled: false) } scope :inactive, -> { where(arel_table[:current_sign_in_at].lt(ACTIVE_DURATION.ago)) } - scope :active, -> { confirmed.where(arel_table[:current_sign_in_at].gteq(ACTIVE_DURATION.ago)).joins(:account).where(accounts: { suspended: false }) } + scope :active, -> { confirmed.where(arel_table[:current_sign_in_at].gteq(ACTIVE_DURATION.ago)).joins(:account).where.not(accounts: { suspended_at: nil }) } scope :matches_email, ->(value) { where(arel_table[:email].matches("#{value}%")) } scope :emailable, -> { confirmed.enabled.joins(:account).merge(Account.searchable) } diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 6d0609ca04..ad22d37fe0 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -50,12 +50,12 @@ class ActivityPub::ProcessAccountService < BaseService def create_account @account = Account.new - @account.protocol = :activitypub - @account.username = @username - @account.domain = @domain - @account.suspended = true if auto_suspend? - @account.silenced = true if auto_silence? - @account.private_key = nil + @account.protocol = :activitypub + @account.username = @username + @account.domain = @domain + @account.private_key = nil + @account.suspended_at = domain_block.created_at if auto_suspend? + @account.silenced_at = domain_block.created_at if auto_silence? end def update_account diff --git a/app/services/block_domain_service.rb b/app/services/block_domain_service.rb index a1fe93665b..497f0394bf 100644 --- a/app/services/block_domain_service.rb +++ b/app/services/block_domain_service.rb @@ -29,7 +29,7 @@ class BlockDomainService < BaseService end def silence_accounts! - blocked_domain_accounts.in_batches.update_all(silenced: true) + blocked_domain_accounts.without_silenced.in_batches.update_all(silenced_at: @domain_block.created_at) end def clear_media! @@ -43,9 +43,9 @@ class BlockDomainService < BaseService end def suspend_accounts! - blocked_domain_accounts.where(suspended: false).reorder(nil).find_each do |account| + blocked_domain_accounts.without_suspended.reorder(nil).find_each do |account| UnsubscribeService.new.call(account) if account.subscribed? - SuspendAccountService.new.call(account) + SuspendAccountService.new.call(account, suspended_at: @domain_block.created_at) end end diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index e7366c7e8c..b470a40e9d 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -49,7 +49,7 @@ class PostStatusService < BaseService def preprocess_attributes! @text = @options.delete(:spoiler_text) if @text.blank? && @options[:spoiler_text].present? @visibility = @options[:visibility] || @account.user&.setting_default_privacy - @visibility = :unlisted if @visibility == :public && @account.silenced + @visibility = :unlisted if @visibility == :public && @account.silenced? @scheduled_at = @options[:scheduled_at]&.to_datetime @scheduled_at = nil if scheduled_in_the_past? rescue ArgumentError diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index 2595c5fd36..d27a7dab4c 100644 --- a/app/services/process_mentions_service.rb +++ b/app/services/process_mentions_service.rb @@ -25,7 +25,7 @@ class ProcessMentionsService < BaseService end end - next match if mention_undeliverable?(mentioned_account) || mentioned_account&.suspended + next match if mention_undeliverable?(mentioned_account) || mentioned_account&.suspended? mentions << mentioned_account.mentions.where(status: status).first_or_create(status: status) diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb index 4ff351c5f7..11e33a83ad 100644 --- a/app/services/resolve_account_service.rb +++ b/app/services/resolve_account_service.rb @@ -119,9 +119,9 @@ class ResolveAccountService < BaseService Rails.logger.debug "Creating new remote account for #{@username}@#{@domain}" @account = Account.new(username: @username, domain: @domain) - @account.suspended = true if auto_suspend? - @account.silenced = true if auto_silence? - @account.private_key = nil + @account.suspended_at = domain_block.created_at if auto_suspend? + @account.silenced_at = domain_block.created_at if auto_silence? + @account.private_key = nil end def update_account diff --git a/app/services/subscribe_service.rb b/app/services/subscribe_service.rb index 2893b54103..83fd64396a 100644 --- a/app/services/subscribe_service.rb +++ b/app/services/subscribe_service.rb @@ -43,7 +43,7 @@ class SubscribeService < BaseService end def some_local_account - @some_local_account ||= Account.local.where(suspended: false).first + @some_local_account ||= Account.local.without_suspended.first end # Any response in the 3xx or 4xx range, except for 429 (rate limit) diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index 6c2ecad30f..412873f841 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -88,8 +88,8 @@ class SuspendAccountService < BaseService return if @options[:destroy] - @account.silenced = false - @account.suspended = true + @account.silenced_at = nil + @account.suspended_at = @options[:suspended_at] || Time.now.utc @account.locked = false @account.display_name = '' @account.note = '' diff --git a/app/services/unblock_domain_service.rb b/app/services/unblock_domain_service.rb index 946b6d465c..9b8526fbe5 100644 --- a/app/services/unblock_domain_service.rb +++ b/app/services/unblock_domain_service.rb @@ -3,9 +3,9 @@ class UnblockDomainService < BaseService attr_accessor :domain_block - def call(domain_block, retroactive) + def call(domain_block) @domain_block = domain_block - process_retroactive_updates if retroactive + process_retroactive_updates domain_block.destroy end @@ -14,14 +14,19 @@ class UnblockDomainService < BaseService end def blocked_accounts - Account.where(domain: domain_block.domain) + scope = Account.where(domain: domain_block.domain) + if domain_block.silence? + scope.where(silenced_at: @domain_block.created_at) + else + scope.where(suspended_at: @domain_block.created_at) + end end def update_options - { domain_block_impact => false } + { domain_block_impact => nil } end def domain_block_impact - domain_block.silence? ? :silenced : :suspended + domain_block.silence? ? :silenced_at : :suspended_at end end diff --git a/app/views/admin/domain_blocks/show.html.haml b/app/views/admin/domain_blocks/show.html.haml index ea1929d44f..dca4dbac77 100644 --- a/app/views/admin/domain_blocks/show.html.haml +++ b/app/views/admin/domain_blocks/show.html.haml @@ -3,18 +3,11 @@ = simple_form_for @domain_block, url: admin_domain_block_path(@domain_block), method: :delete do |f| - - if (@domain_block.noop?) - = f.input :retroactive, - as: :hidden, - input_html: { :value => "0" } - - else - = f.input :retroactive, - as: :boolean, - wrapper: :with_label, - label: t(".retroactive.#{@domain_block.severity}"), - hint: t(:affected_accounts, - scope: [:admin, :domain_blocks, :show], - count: @domain_block.accounts_count) + - unless (@domain_block.noop?) + %p= t(".retroactive.#{@domain_block.severity}") + %p.hint= t(:affected_accounts, + scope: [:admin, :domain_blocks, :show], + count: @domain_block.affected_accounts_count) .actions = f.button :button, t('.undo'), type: :submit diff --git a/config/locales/en.yml b/config/locales/en.yml index 6d59411a58..4acea6dc0d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -293,8 +293,8 @@ en: one: One account in the database affected other: "%{count} accounts in the database affected" retroactive: - silence: Unsilence all existing accounts from this domain - suspend: Unsuspend all existing accounts from this domain + silence: Unsilence existing affected accounts from this domain + suspend: Unsuspend existing affected accounts from this domain title: Undo domain block for %{domain} undo: Undo undo: Undo domain block diff --git a/db/migrate/20190511134027_add_silenced_at_suspended_at_to_accounts.rb b/db/migrate/20190511134027_add_silenced_at_suspended_at_to_accounts.rb new file mode 100644 index 0000000000..1e5cd669c3 --- /dev/null +++ b/db/migrate/20190511134027_add_silenced_at_suspended_at_to_accounts.rb @@ -0,0 +1,41 @@ +class AddSilencedAtSuspendedAtToAccounts < ActiveRecord::Migration[5.2] + class Account < ApplicationRecord + # Dummy class, to make migration possible across version changes + end + + class DomainBlock < ApplicationRecord + # Dummy class, to make migration possible across version changes + enum severity: [:silence, :suspend, :noop] + + has_many :accounts, foreign_key: :domain, primary_key: :domain + end + + def up + add_column :accounts, :silenced_at, :datetime + add_column :accounts, :suspended_at, :datetime + + # Record suspend date of blocks and silences for users whose limitations match + # a domain block + DomainBlock.where(severity: [:silence, :suspend]).find_each do |block| + scope = block.accounts + if block.suspend? + block.accounts.where(suspended: true).in_batches.update_all(suspended_at: block.created_at) + else + block.accounts.where(silenced: true).in_batches.update_all(silenced_at: block.created_at) + end + end + + # Set dates for accounts which have limitations not related to a domain block + Account.where(suspended: true, suspended_at: nil).in_batches.update_all(suspended_at: Time.now.utc) + Account.where(silenced: true, silenced_at: nil).in_batches.update_all(silenced_at: Time.now.utc) + end + + def down + # Block or silence accounts that have a date set + Account.where(suspended: false).where.not(suspended_at: nil).in_batches.update_all(suspended: true) + Account.where(silenced: false).where.not(silenced_at: nil).in_batches.update_all(silenced: true) + + remove_column :accounts, :silenced_at + remove_column :accounts, :suspended_at + end +end diff --git a/db/post_migrate/20190511152737_remove_suspended_silenced_account_fields.rb b/db/post_migrate/20190511152737_remove_suspended_silenced_account_fields.rb new file mode 100644 index 0000000000..a46349cb73 --- /dev/null +++ b/db/post_migrate/20190511152737_remove_suspended_silenced_account_fields.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +class RemoveSuspendedSilencedAccountFields < ActiveRecord::Migration[5.2] + class Account < ApplicationRecord + # Dummy class, to make migration possible across version changes + end + + class DomainBlock < ApplicationRecord + # Dummy class, to make migration possible across version changes + enum severity: [:silence, :suspend, :noop] + + has_many :accounts, foreign_key: :domain, primary_key: :domain + end + + disable_ddl_transaction! + + def up + # Record suspend date of blocks and silences for users whose limitations match + # a domain block + DomainBlock.where(severity: [:silence, :suspend]).find_each do |block| + scope = block.accounts + if block.suspend? + block.accounts.where(suspended: true).in_batches.update_all(suspended_at: block.created_at) + else + block.accounts.where(silenced: true).in_batches.update_all(silenced_at: block.created_at) + end + end + + # Set dates for accounts which have limitations not related to a domain block + Account.where(suspended: true, suspended_at: nil).in_batches.update_all(suspended_at: Time.now.utc) + Account.where(silenced: true, silenced_at: nil).in_batches.update_all(silenced_at: Time.now.utc) + + safety_assured do + remove_column :accounts, :suspended, :boolean, null: false, default: false + remove_column :accounts, :silenced, :boolean, null: false, default: false + end + end + + def down + safety_assured do + add_column :accounts, :suspended, :boolean, null: false, default: false + add_column :accounts, :silenced, :boolean, null: false, default: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 4dcbea4e75..782727942e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_05_09_164208) do +ActiveRecord::Schema.define(version: 2019_05_11_152737) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -131,8 +131,6 @@ ActiveRecord::Schema.define(version: 2019_05_09_164208) do t.datetime "header_updated_at" t.string "avatar_remote_url" t.datetime "subscription_expires_at" - t.boolean "silenced", default: false, null: false - t.boolean "suspended", default: false, null: false t.boolean "locked", default: false, null: false t.string "header_remote_url", default: "", null: false t.datetime "last_webfingered_at" @@ -148,6 +146,8 @@ ActiveRecord::Schema.define(version: 2019_05_09_164208) do t.string "actor_type" t.boolean "discoverable" t.string "also_known_as", array: true + t.datetime "silenced_at" + t.datetime "suspended_at" t.index "(((setweight(to_tsvector('simple'::regconfig, (display_name)::text), 'A'::\"char\") || setweight(to_tsvector('simple'::regconfig, (username)::text), 'B'::\"char\")) || setweight(to_tsvector('simple'::regconfig, (COALESCE(domain, ''::character varying))::text), 'C'::\"char\")))", name: "search_index", using: :gin t.index "lower((username)::text), lower((domain)::text)", name: "index_accounts_on_username_and_domain_lower", unique: true t.index ["moved_to_account_id"], name: "index_accounts_on_moved_to_account_id" diff --git a/lib/cli.rb b/lib/cli.rb index 5780e3e873..be276583d2 100644 --- a/lib/cli.rb +++ b/lib/cli.rb @@ -106,7 +106,7 @@ module Mastodon [json, account.id, inbox_url] end - account.update_column(:suspended, true) + account.suspend! end processed += 1 diff --git a/lib/mastodon/accounts_cli.rb b/lib/mastodon/accounts_cli.rb index 3131647f35..7d02153131 100644 --- a/lib/mastodon/accounts_cli.rb +++ b/lib/mastodon/accounts_cli.rb @@ -87,8 +87,8 @@ module Mastodon end end - account.suspended = false - user.account = account + account.suspended_at = nil + user.account = account if user.save if options[:confirmed] diff --git a/spec/controllers/admin/domain_blocks_controller_spec.rb b/spec/controllers/admin/domain_blocks_controller_spec.rb index 2a8675c21a..fb23658c01 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 e092e6c099..f12464ef3e 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 c506cd87f1..5f8eb86a80 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 db2d87de20..a851014d9c 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 94c2d5fc2e..50286ef77b 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 7ef9e2770e..c689b57e34 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 39a681abbf..a387d9407e 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 8e8893d635..619aefb5c0 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 + 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