commit: 8b74aa42176dabf77c3b4d02c80bcc47d9d70e8e
parent: a6807201d2003fc0d544813ba67cfe315d829e06
Author: Matt Jankowski <mjankowski@thoughtbot.com>
Date: Fri, 14 Apr 2017 05:10:28 -0400
Admin reports controller improvements (#1714)
* Simplify admin/reports controller filtering for index
* Rename parameter to resolved
* Fix issue where reports view could not access filter_link_to
* Add coverage for admin/reports controller
* DRY up resolution of related reports for target account
* Clean up admin/reports routes
* Add Report#statuses method
* DRY up current account action taken params
* Rubocop styles
Diffstat:
11 files changed, 182 insertions(+), 40 deletions(-)
diff --git a/app/controllers/admin/reported_statuses_controller.rb b/app/controllers/admin/reported_statuses_controller.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module Admin
+ class ReportedStatusesController < BaseController
+ def destroy
+ status = Status.find params[:id]
+
+ RemovalWorker.perform_async(status.id)
+ redirect_to admin_report_path(report)
+ end
+
+ private
+
+ def report
+ Report.find(params[:report_id])
+ end
+ end
+end
diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb
@@ -5,37 +5,59 @@ module Admin
before_action :set_report, except: [:index]
def index
- @reports = Report.includes(:account, :target_account).order('id desc').page(params[:page])
- @reports = params[:action_taken].present? ? @reports.resolved : @reports.unresolved
+ @reports = filtered_reports.page(params[:page])
end
- def show
- @statuses = Status.where(id: @report.status_ids)
- end
+ def show; end
- def resolve
- @report.update(action_taken: true, action_taken_by_account_id: current_account.id)
+ def update
+ process_report
redirect_to admin_report_path(@report)
end
- def suspend
- Admin::SuspensionWorker.perform_async(@report.target_account.id)
- Report.unresolved.where(target_account: @report.target_account).update_all(action_taken: true, action_taken_by_account_id: current_account.id)
- redirect_to admin_report_path(@report)
+ private
+
+ def process_report
+ case params[:outcome].to_s
+ when 'resolve'
+ @report.update(action_taken_by_current_attributes)
+ when 'suspend'
+ Admin::SuspensionWorker.perform_async(@report.target_account.id)
+ resolve_all_target_account_reports
+ when 'silence'
+ @report.target_account.update(silenced: true)
+ resolve_all_target_account_reports
+ else
+ raise ActiveRecord::RecordNotFound
+ end
end
- def silence
- @report.target_account.update(silenced: true)
- Report.unresolved.where(target_account: @report.target_account).update_all(action_taken: true, action_taken_by_account_id: current_account.id)
- redirect_to admin_report_path(@report)
+ def action_taken_by_current_attributes
+ { action_taken: true, action_taken_by_account_id: current_account.id }
end
- def remove
- RemovalWorker.perform_async(params[:status_id])
- redirect_to admin_report_path(@report)
+ def resolve_all_target_account_reports
+ unresolved_reports_for_target_account.update_all(
+ action_taken_by_current_attributes
+ )
end
- private
+ def unresolved_reports_for_target_account
+ Report.where(
+ target_account: @report.target_account
+ ).unresolved
+ end
+
+ def filtered_reports
+ filtering_scope.order('id desc').includes(
+ :account,
+ :target_account
+ )
+ end
+
+ def filtering_scope
+ params[:resolved].present? ? Report.resolved : Report.unresolved
+ end
def set_report
@report = Report.find(params[:id])
diff --git a/app/helpers/admin/accounts_helper.rb b/app/helpers/admin/accounts_helper.rb
@@ -2,7 +2,7 @@
module Admin::AccountsHelper
def filter_params(more_params)
- params.permit(:local, :remote, :by_domain, :silenced, :suspended, :recent).merge(more_params)
+ params.permit(:local, :remote, :by_domain, :silenced, :suspended, :recent, :resolved).merge(more_params)
end
def filter_link_to(text, more_params)
diff --git a/app/models/report.rb b/app/models/report.rb
@@ -7,4 +7,8 @@ class Report < ApplicationRecord
scope :unresolved, -> { where(action_taken: false) }
scope :resolved, -> { where(action_taken: true) }
+
+ def statuses
+ Status.where(id: status_ids)
+ end
end
diff --git a/app/views/admin/reports/index.html.haml b/app/views/admin/reports/index.html.haml
@@ -5,8 +5,8 @@
.filter-subset
%strong= t('admin.reports.status')
%ul
- %li= filter_link_to t('admin.reports.unresolved'), action_taken: nil
- %li= filter_link_to t('admin.reports.resolved'), action_taken: '1'
+ %li= filter_link_to t('admin.reports.unresolved'), resolved: nil
+ %li= filter_link_to t('admin.reports.resolved'), resolved: '1'
= form_tag do
diff --git a/app/views/admin/reports/show.html.haml b/app/views/admin/reports/show.html.haml
@@ -14,15 +14,15 @@
\:
= @report.comment.presence || t('reports.comment.none')
-- unless @statuses.empty?
+- unless @report.statuses.empty?
%hr/
- - @statuses.each do |status|
+ - @report.statuses.each do |status|
.report-status
.activity-stream.activity-stream-headless
.entry= render partial: 'stream_entries/simple_status', locals: { status: status }
.report-status__actions
- = link_to remove_admin_report_path(@report, status_id: status.id), method: :post, class: 'icon-button', style: 'font-size: 24px; width: 24px; height: 24px', title: t('admin.reports.delete') do
+ = link_to admin_report_reported_status_path(@report, status), method: :delete, class: 'icon-button', style: 'font-size: 24px; width: 24px; height: 24px', title: t('admin.reports.delete') do
= fa_icon 'trash'
- if !@report.action_taken?
@@ -30,10 +30,10 @@
%div{ style: 'overflow: hidden' }
%div{ style: 'float: right' }
- = link_to t('admin.reports.silence_account'), silence_admin_report_path(@report), method: :post, class: 'button'
- = link_to t('admin.reports.suspend_account'), suspend_admin_report_path(@report), method: :post, class: 'button'
+ = 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'
%div{ style: 'float: left' }
- = link_to t('admin.reports.mark_as_resolved'), resolve_admin_report_path(@report), method: :post, class: 'button'
+ = link_to t('admin.reports.mark_as_resolved'), admin_report_path(@report, outcome: 'resolve'), method: :put, class: 'button'
- elsif !@report.action_taken_by_account.nil?
%hr/
diff --git a/config/routes.rb b/config/routes.rb
@@ -80,13 +80,8 @@ Rails.application.routes.draw do
resources :domain_blocks, only: [:index, :new, :create]
resources :settings, only: [:index, :update]
- resources :reports, only: [:index, :show] do
- member do
- post :resolve
- post :silence
- post :suspend
- post :remove
- end
+ resources :reports, only: [:index, :show, :update] do
+ resources :reported_statuses, only: :destroy
end
resources :accounts, only: [:index, :show] do
diff --git a/spec/controllers/admin/reported_statuses_controller_spec.rb b/spec/controllers/admin/reported_statuses_controller_spec.rb
@@ -0,0 +1,21 @@
+require 'rails_helper'
+
+describe Admin::ReportedStatusesController do
+ let(:user) { Fabricate(:user, admin: true) }
+ before do
+ sign_in user, scope: :user
+ end
+
+ describe 'DELETE #destroy' do
+ it 'removes a status' do
+ report = Fabricate(:report)
+ status = Fabricate(:status)
+ allow(RemovalWorker).to receive(:perform_async)
+
+ delete :destroy, params: { report_id: report, id: status }
+ expect(response).to redirect_to(admin_report_path(report))
+ expect(RemovalWorker).
+ to have_received(:perform_async).with(status.id)
+ end
+ end
+end
diff --git a/spec/controllers/admin/reports_controller_spec.rb b/spec/controllers/admin/reports_controller_spec.rb
@@ -1,14 +1,86 @@
require 'rails_helper'
-RSpec.describe Admin::ReportsController, type: :controller do
+describe Admin::ReportsController do
+ let(:user) { Fabricate(:user, admin: true) }
+ before do
+ sign_in user, scope: :user
+ end
+
describe 'GET #index' do
- before do
- sign_in Fabricate(:user, admin: true), scope: :user
+ it 'returns http success with no filters' do
+ allow(Report).to receive(:unresolved).and_return(Report.all)
+ get :index
+
+ expect(response).to have_http_status(:success)
+ expect(Report).to have_received(:unresolved)
end
+ it 'returns http success with resolved filter' do
+ allow(Report).to receive(:resolved).and_return(Report.all)
+ get :index, params: { resolved: 1 }
+
+ expect(response).to have_http_status(:success)
+ expect(Report).to have_received(:resolved)
+ end
+ end
+
+ describe 'GET #show' do
it 'returns http success' do
- get :index
+ report = Fabricate(:report)
+
+ get :show, params: { id: report }
expect(response).to have_http_status(:success)
end
end
+
+ describe 'PUT #update' do
+ describe 'with an unknown outcome' do
+ it 'rejects the change' do
+ report = Fabricate(:report)
+ put :update, params: { id: report, outcome: 'unknown' }
+
+ expect(response).to have_http_status(:missing)
+ end
+ end
+
+ describe 'with an outcome of `resolve`' do
+ it 'resolves the report' do
+ report = Fabricate(:report)
+
+ put :update, params: { id: report, outcome: 'resolve' }
+ expect(response).to redirect_to(admin_report_path(report))
+ report.reload
+ expect(report.action_taken_by_account).to eq user.account
+ expect(report.action_taken).to eq true
+ 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_report_path(report))
+ 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)
+
+ put :update, params: { id: report, outcome: 'silence' }
+ expect(response).to redirect_to(admin_report_path(report))
+ report.reload
+ expect(report.action_taken_by_account).to eq user.account
+ expect(report.action_taken).to eq true
+ expect(report.target_account).to be_silenced
+ end
+ end
+ end
end
diff --git a/spec/fabricators/report_fabricator.rb b/spec/fabricators/report_fabricator.rb
@@ -1,4 +1,6 @@
Fabricator(:report) do
+ account
+ target_account { Fabricate(:account) }
comment "You nasty"
action_taken false
end
diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb
@@ -1,5 +1,13 @@
require 'rails_helper'
-RSpec.describe Report, type: :model do
+describe Report do
+ describe 'statuses' do
+ it 'returns the statuses for the report' do
+ status = Fabricate(:status)
+ _other = Fabricate(:status)
+ report = Fabricate(:report, status_ids: [status.id])
+ expect(report.statuses).to eq [status]
+ end
+ end
end
