From 2374a00c1062a70e9092d88579e1351e4c8128f9 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 22 Aug 2018 11:53:41 +0200 Subject: [PATCH] Add confirmation step to account suspensions (#8353) * Add confirmation page for suspensions * Suspension confirmation closes reports, linked from report UI * Fix tests --- app/controllers/admin/reports_controller.rb | 8 +--- .../admin/suspensions_controller.rb | 39 +++++++++++++++++-- app/javascript/styles/mastodon/forms.scss | 6 +++ app/models/account.rb | 7 ++++ .../form/admin_suspension_confirmation.rb | 7 ++++ app/views/admin/accounts/show.html.haml | 2 +- app/views/admin/reports/show.html.haml | 2 +- app/views/admin/suspensions/new.html.haml | 25 ++++++++++++ config/locales/en.yml | 6 +++ config/routes.rb | 2 +- .../admin/reports_controller_spec.rb | 15 ------- .../admin/suspensions_controller_spec.rb | 2 +- 12 files changed, 92 insertions(+), 29 deletions(-) create mode 100644 app/models/form/admin_suspension_confirmation.rb create mode 100644 app/views/admin/suspensions/new.html.haml diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb index d00b3d222..5d7f43e00 100644 --- a/app/controllers/admin/reports_controller.rb +++ b/app/controllers/admin/reports_controller.rb @@ -44,14 +44,8 @@ module Admin when 'resolve' @report.resolve!(current_account) log_action :resolve, @report - when 'suspend' - Admin::SuspensionWorker.perform_async(@report.target_account.id) - - log_action :resolve, @report - log_action :suspend, @report.target_account - - resolve_all_target_account_reports when 'silence' + @report.resolve!(current_account) @report.target_account.update!(silenced: true) log_action :resolve, @report diff --git a/app/controllers/admin/suspensions_controller.rb b/app/controllers/admin/suspensions_controller.rb index 5f222e125..0c7bdad9e 100644 --- a/app/controllers/admin/suspensions_controller.rb +++ b/app/controllers/admin/suspensions_controller.rb @@ -4,11 +4,24 @@ module Admin class SuspensionsController < BaseController before_action :set_account + def new + @suspension = Form::AdminSuspensionConfirmation.new(report_id: params[:report_id]) + end + def create authorize @account, :suspend? - Admin::SuspensionWorker.perform_async(@account.id) - log_action :suspend, @account - redirect_to admin_accounts_path + + @suspension = Form::AdminSuspensionConfirmation.new(suspension_params) + + if suspension_params[:acct] == @account.acct + resolve_report! if suspension_params[:report_id] + perform_suspend! + mark_reports_resolved! + redirect_to admin_accounts_path + else + flash.now[:alert] = I18n.t('admin.suspensions.bad_acct_msg') + render :new + end end def destroy @@ -23,5 +36,25 @@ module Admin def set_account @account = Account.find(params[:account_id]) end + + def suspension_params + params.require(:form_admin_suspension_confirmation).permit(:acct, :report_id) + end + + def resolve_report! + report = Report.find(suspension_params[:report_id]) + report.resolve!(current_account) + log_action :resolve, report + end + + def perform_suspend! + @account.suspend! + Admin::SuspensionWorker.perform_async(@account.id) + log_action :suspend, @account + end + + def mark_reports_resolved! + Report.where(target_account: @account).unresolved.update_all(action_taken: true, action_taken_by_account_id: current_account.id) + end end end diff --git a/app/javascript/styles/mastodon/forms.scss b/app/javascript/styles/mastodon/forms.scss index 22dbfa8cf..020be5ad2 100644 --- a/app/javascript/styles/mastodon/forms.scss +++ b/app/javascript/styles/mastodon/forms.scss @@ -50,6 +50,12 @@ code { color: $highlight-text-color; } } + + code { + border-radius: 3px; + padding: 0.2em 0.4em; + background: darken($ui-base-color, 12%); + } } .card { diff --git a/app/models/account.rb b/app/models/account.rb index c33ec4bd5..440a731e3 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -193,6 +193,13 @@ class Account < ApplicationRecord ResolveAccountService.new.call(acct) end + def suspend! + transaction do + user&.disable! if local? + update!(suspended: true) + end + end + def unsuspend! transaction do user&.enable! if local? diff --git a/app/models/form/admin_suspension_confirmation.rb b/app/models/form/admin_suspension_confirmation.rb new file mode 100644 index 000000000..c34b5b30e --- /dev/null +++ b/app/models/form/admin_suspension_confirmation.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class Form::AdminSuspensionConfirmation + include ActiveModel::Model + + attr_accessor :acct, :report_id +end diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml index ed8190af5..f2c53e3fe 100644 --- a/app/views/admin/accounts/show.html.haml +++ b/app/views/admin/accounts/show.html.haml @@ -123,7 +123,7 @@ - if @account.suspended? = link_to t('admin.accounts.undo_suspension'), admin_account_suspension_path(@account.id), method: :delete, class: 'button' if can?(:unsuspend, @account) - else - = link_to t('admin.accounts.perform_full_suspension'), admin_account_suspension_path(@account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button' if can?(:suspend, @account) + = link_to t('admin.accounts.perform_full_suspension'), new_admin_account_suspension_path(@account.id), class: 'button' if can?(:suspend, @account) - if !@account.local? && @account.hub_url.present? %hr.spacer/ diff --git a/app/views/admin/reports/show.html.haml b/app/views/admin/reports/show.html.haml index b13bb5303..ef0e4aa41 100644 --- a/app/views/admin/reports/show.html.haml +++ b/app/views/admin/reports/show.html.haml @@ -8,7 +8,7 @@ - if @report.unresolved? %div{ style: 'float: right' } = link_to t('admin.reports.silence_account'), admin_report_path(@report, outcome: 'silence'), method: :put, class: 'button' - = link_to t('admin.reports.suspend_account'), admin_report_path(@report, outcome: 'suspend'), method: :put, class: 'button' + = link_to t('admin.reports.suspend_account'), new_admin_account_suspension_path(@report.target_account_id, report_id: @report.id), class: 'button' %div{ style: 'float: left' } = link_to t('admin.reports.mark_as_resolved'), admin_report_path(@report, outcome: 'resolve'), method: :put, class: 'button' - else diff --git a/app/views/admin/suspensions/new.html.haml b/app/views/admin/suspensions/new.html.haml new file mode 100644 index 000000000..5ffbbbe46 --- /dev/null +++ b/app/views/admin/suspensions/new.html.haml @@ -0,0 +1,25 @@ +- content_for :page_title do + = t('admin.suspensions.title', acct: @account.acct) + += simple_form_for @suspension, url: admin_account_suspension_path(@account.id), method: :post do |f| + %p.hint= t('admin.suspensions.warning_html') + + .fields-group + %ul + %li.negative-hint + = number_to_human @account.statuses_count, strip_insignificant_zeros: true + = t('accounts.posts') + %li.negative-hint + = number_to_human @account.following_count, strip_insignificant_zeros: true + = t('accounts.following') + %li.negative-hint + = number_to_human @account.followers_count, strip_insignificant_zeros: true + = t('accounts.followers') + + %p.hint= t('admin.suspensions.hint_html', value: content_tag(:code, @account.acct)) + + = f.input :acct + = f.input_field :report_id, as: :hidden + + .actions + = f.button :button, t('admin.suspensions.proceed'), type: :submit, class: 'negative' diff --git a/config/locales/en.yml b/config/locales/en.yml index 7809b8e68..45b321312 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -414,6 +414,12 @@ en: last_delivery: Last delivery title: WebSub topic: Topic + suspensions: + bad_acct_msg: The confirmation value didn't match up. Are you suspending the right account? + hint_html: 'To confirm the suspension of the account, please enter %{value} into the field below:' + proceed: Proceed + title: Suspend %{acct} + warning_html: 'Suspending this account will irreversibly delete data from this account, which includes:' title: Administration admin_mailer: new_report: diff --git a/config/routes.rb b/config/routes.rb index da7cb8061..80a8b7b4c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -174,7 +174,7 @@ Rails.application.routes.draw do resource :change_email, only: [:show, :update] resource :reset, only: [:create] resource :silence, only: [:create, :destroy] - resource :suspension, only: [:create, :destroy] + resource :suspension, only: [:new, :create, :destroy] resources :statuses, only: [:index, :create, :update, :destroy] resource :confirmation, only: [:create] do diff --git a/spec/controllers/admin/reports_controller_spec.rb b/spec/controllers/admin/reports_controller_spec.rb index e50c02a72..bcc789c57 100644 --- a/spec/controllers/admin/reports_controller_spec.rb +++ b/spec/controllers/admin/reports_controller_spec.rb @@ -68,21 +68,6 @@ describe Admin::ReportsController do end end - describe 'with an outcome of `suspend`' do - it 'suspends the reported account' do - report = Fabricate(:report) - allow(Admin::SuspensionWorker).to receive(:perform_async) - - put :update, params: { id: report, outcome: 'suspend' } - expect(response).to redirect_to(admin_reports_path) - report.reload - expect(report.action_taken_by_account).to eq user.account - expect(report.action_taken).to eq true - expect(Admin::SuspensionWorker). - to have_received(:perform_async).with(report.target_account_id) - end - end - describe 'with an outsome of `silence`' do it 'silences the reported account' do report = Fabricate(:report) diff --git a/spec/controllers/admin/suspensions_controller_spec.rb b/spec/controllers/admin/suspensions_controller_spec.rb index ddfc938d1..babb1ed96 100644 --- a/spec/controllers/admin/suspensions_controller_spec.rb +++ b/spec/controllers/admin/suspensions_controller_spec.rb @@ -12,7 +12,7 @@ describe Admin::SuspensionsController do account = Fabricate(:account, suspended: false) expect(Admin::SuspensionWorker).to receive(:perform_async).with(account.id) - post :create, params: { account_id: account.id } + post :create, params: { account_id: account.id, form_admin_suspension_confirmation: { acct: account.acct } } expect(response).to redirect_to(admin_accounts_path) end