From e85cffb2362f914c0f2f7ced4112430b30bc7997 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Mon, 2 Apr 2018 22:04:14 +0200 Subject: Feature: Report improvements (#6967) (#7000) * Implement Assignment of Reports (#6967) * Change translation of admin.report.comment.label to "Report Comment" for clarity As we'll soon add the ability for reports to have comments on them, this clarification makes sense. * Implement notes for Reports This enables moderators to leave comments about a report whilst they work on it * Fix display of report moderation notes * Allow reports to be reopened / marked as unresolved * Redirect to reports listing upon resolution of report * Implement "resolve with note" functionality * Add inverse relationship for report notes * Remove additional database querying when loading report notes * Fix tests for reports * Fix localisations for report notes / reports --- spec/controllers/admin/reports_controller_spec.rb | 40 +++++++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) (limited to 'spec/controllers/admin') diff --git a/spec/controllers/admin/reports_controller_spec.rb b/spec/controllers/admin/reports_controller_spec.rb index 71a185147..9be298df6 100644 --- a/spec/controllers/admin/reports_controller_spec.rb +++ b/spec/controllers/admin/reports_controller_spec.rb @@ -61,7 +61,7 @@ describe Admin::ReportsController do report = Fabricate(:report) put :update, params: { id: report, outcome: 'resolve' } - expect(response).to redirect_to(admin_report_path(report)) + 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 @@ -74,7 +74,7 @@ describe Admin::ReportsController do allow(Admin::SuspensionWorker).to receive(:perform_async) put :update, params: { id: report, outcome: 'suspend' } - expect(response).to redirect_to(admin_report_path(report)) + 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 @@ -88,12 +88,46 @@ describe Admin::ReportsController do report = Fabricate(:report) put :update, params: { id: report, outcome: 'silence' } - expect(response).to redirect_to(admin_report_path(report)) + 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(report.target_account).to be_silenced end end + + describe 'with an outsome of `reopen`' do + it 'reopens the report' do + report = Fabricate(:report) + + put :update, params: { id: report, outcome: 'reopen' } + expect(response).to redirect_to(admin_report_path(report)) + report.reload + expect(report.action_taken_by_account).to eq nil + expect(report.action_taken).to eq false + end + end + + describe 'with an outsome of `assign_to_self`' do + it 'reopens the report' do + report = Fabricate(:report) + + put :update, params: { id: report, outcome: 'assign_to_self' } + expect(response).to redirect_to(admin_report_path(report)) + report.reload + expect(report.assigned_account).to eq user.account + end + end + + describe 'with an outsome of `unassign`' do + it 'reopens the report' do + report = Fabricate(:report) + + put :update, params: { id: report, outcome: 'unassign' } + expect(response).to redirect_to(admin_report_path(report)) + report.reload + expect(report.assigned_account).to eq nil + end + end end end -- cgit From 2e59751823585a8ef8729d4287239b326ab02193 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Tue, 3 Apr 2018 13:07:32 +0200 Subject: Improve require_admin! and require_staff! filters (#7018) Previously these returns 302 redirects instead of 403s, which meant posting links to admin pages in slack caused them to unfurl, rather than stay as a link. Additionally, require_admin! doesn't appear to be actively used, on require_staff! --- app/controllers/application_controller.rb | 4 +-- spec/controllers/admin/base_controller_spec.rb | 19 +++++++---- spec/controllers/application_controller_spec.rb | 42 +++++++++++++++++++++++-- 3 files changed, 55 insertions(+), 10 deletions(-) (limited to 'spec/controllers/admin') diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6e5042617..588526447 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -39,11 +39,11 @@ class ApplicationController < ActionController::Base end def require_admin! - redirect_to root_path unless current_user&.admin? + forbidden unless current_user&.admin? end def require_staff! - redirect_to root_path unless current_user&.staff? + forbidden unless current_user&.staff? end def check_suspension diff --git a/spec/controllers/admin/base_controller_spec.rb b/spec/controllers/admin/base_controller_spec.rb index 2b60e7e92..9ac833623 100644 --- a/spec/controllers/admin/base_controller_spec.rb +++ b/spec/controllers/admin/base_controller_spec.rb @@ -9,18 +9,25 @@ describe Admin::BaseController, type: :controller do end end - it 'renders admin layout' do + it 'requires administrator or moderator' do routes.draw { get 'success' => 'admin/base#success' } - sign_in(Fabricate(:user, admin: true)) + sign_in(Fabricate(:user, admin: false, moderator: false)) get :success - expect(response).to render_template layout: 'admin' + + expect(response).to have_http_status(:forbidden) end - it 'requires administrator' do + it 'renders admin layout as a moderator' do routes.draw { get 'success' => 'admin/base#success' } - sign_in(Fabricate(:user, admin: false)) + sign_in(Fabricate(:user, moderator: true)) get :success + expect(response).to render_template layout: 'admin' + end - expect(response).to redirect_to('/') + it 'renders admin layout as an admin' do + routes.draw { get 'success' => 'admin/base#success' } + sign_in(Fabricate(:user, admin: true)) + get :success + expect(response).to render_template layout: 'admin' end end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 51c0e3c70..3e4d27e05 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -181,10 +181,48 @@ describe ApplicationController, type: :controller do routes.draw { get 'sucesss' => 'anonymous#sucesss' } end - it 'redirects to root path if current user is not admin' do + it 'returns a 403 if current user is not admin' do sign_in(Fabricate(:user, admin: false)) get 'sucesss' - expect(response).to redirect_to('/') + expect(response).to have_http_status(403) + end + + it 'returns a 403 if current user is only a moderator' do + sign_in(Fabricate(:user, moderator: true)) + get 'sucesss' + expect(response).to have_http_status(403) + end + + it 'does nothing if current user is admin' do + sign_in(Fabricate(:user, admin: true)) + get 'sucesss' + expect(response).to have_http_status(200) + end + end + + describe 'require_staff!' do + controller do + before_action :require_staff! + + def sucesss + head 200 + end + end + + before do + routes.draw { get 'sucesss' => 'anonymous#sucesss' } + end + + it 'returns a 403 if current user is not admin or moderator' do + sign_in(Fabricate(:user, admin: false, moderator: false)) + get 'sucesss' + expect(response).to have_http_status(403) + end + + it 'does nothing if current user is moderator' do + sign_in(Fabricate(:user, moderator: true)) + get 'sucesss' + expect(response).to have_http_status(200) end it 'does nothing if current user is admin' do -- cgit