From 3a9eb81a8006af0306e8abc54bd8aca8381eee25 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 13 Apr 2017 07:04:23 -0400 Subject: [PATCH] Admin accounts controller cleanup (#1664) * Remove unused account_params method in admin/accounts controller * Introduce AccountFilter to find accounts * Use AccountFilter in admin/accounts controller * Use more restful routes admin silence and suspension area * Add admin/silences and admin/suspensions controllers --- app/controllers/admin/accounts_controller.rb | 48 ++++++------------- app/controllers/admin/silences_controller.rb | 23 +++++++++ .../admin/suspensions_controller.rb | 23 +++++++++ app/models/account_filter.rb | 36 ++++++++++++++ app/views/admin/accounts/show.html.haml | 8 ++-- config/routes.rb | 9 ++-- .../admin/silences_controller_spec.rb | 24 ++++++++++ .../admin/suspensions_controller_spec.rb | 24 ++++++++++ spec/models/account_filter_spec.rb | 31 ++++++++++++ 9 files changed, 182 insertions(+), 44 deletions(-) create mode 100644 app/controllers/admin/silences_controller.rb create mode 100644 app/controllers/admin/suspensions_controller.rb create mode 100644 app/models/account_filter.rb create mode 100644 spec/controllers/admin/silences_controller_spec.rb create mode 100644 spec/controllers/admin/suspensions_controller_spec.rb create mode 100644 spec/models/account_filter_spec.rb diff --git a/app/controllers/admin/accounts_controller.rb b/app/controllers/admin/accounts_controller.rb index 71cb8edd87..0e9e52f42a 100644 --- a/app/controllers/admin/accounts_controller.rb +++ b/app/controllers/admin/accounts_controller.rb @@ -2,49 +2,29 @@ module Admin class AccountsController < BaseController - before_action :set_account, except: :index - def index - @accounts = Account.alphabetic.page(params[:page]) - - @accounts = @accounts.local if params[:local].present? - @accounts = @accounts.remote if params[:remote].present? - @accounts = @accounts.where(domain: params[:by_domain]) if params[:by_domain].present? - @accounts = @accounts.silenced if params[:silenced].present? - @accounts = @accounts.recent if params[:recent].present? - @accounts = @accounts.suspended if params[:suspended].present? + @accounts = filtered_accounts.page(params[:page]) end - def show; end - - def suspend - Admin::SuspensionWorker.perform_async(@account.id) - redirect_to admin_accounts_path - end - - def unsuspend - @account.update(suspended: false) - redirect_to admin_accounts_path - end - - def silence - @account.update(silenced: true) - redirect_to admin_accounts_path - end - - def unsilence - @account.update(silenced: false) - redirect_to admin_accounts_path + def show + @account = Account.find(params[:id]) end private - def set_account - @account = Account.find(params[:id]) + def filtered_accounts + AccountFilter.new(filter_params).results end - def account_params - params.require(:account).permit(:silenced, :suspended) + def filter_params + params.permit( + :local, + :remote, + :by_domain, + :silenced, + :recent, + :suspended + ) end end end diff --git a/app/controllers/admin/silences_controller.rb b/app/controllers/admin/silences_controller.rb new file mode 100644 index 0000000000..81a3008b96 --- /dev/null +++ b/app/controllers/admin/silences_controller.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Admin + class SilencesController < BaseController + before_action :set_account + + def create + @account.update(silenced: true) + redirect_to admin_accounts_path + end + + def destroy + @account.update(silenced: false) + redirect_to admin_accounts_path + end + + private + + def set_account + @account = Account.find(params[:account_id]) + end + end +end diff --git a/app/controllers/admin/suspensions_controller.rb b/app/controllers/admin/suspensions_controller.rb new file mode 100644 index 0000000000..5d9048d94d --- /dev/null +++ b/app/controllers/admin/suspensions_controller.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Admin + class SuspensionsController < BaseController + before_action :set_account + + def create + Admin::SuspensionWorker.perform_async(@account.id) + redirect_to admin_accounts_path + end + + def destroy + @account.update(suspended: false) + redirect_to admin_accounts_path + end + + private + + def set_account + @account = Account.find(params[:account_id]) + end + end +end diff --git a/app/models/account_filter.rb b/app/models/account_filter.rb new file mode 100644 index 0000000000..a8d8c88376 --- /dev/null +++ b/app/models/account_filter.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class AccountFilter + attr_reader :params + + def initialize(params) + @params = params + end + + def results + scope = Account.alphabetic + params.each do |key, value| + scope = scope.merge scope_for(key, value) + end + scope + end + + def scope_for(key, value) + case key + when /local/ + Account.local + when /remote/ + Account.remote + when /by_domain/ + Account.where(domain: value) + when /silenced/ + Account.silenced + when /recent/ + Account.recent + when /suspended/ + Account.suspended + else + raise "Unknown filter: #{key}" + end + end +end diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml index ba1c3bae7b..22901aed1e 100644 --- a/app/views/admin/accounts/show.html.haml +++ b/app/views/admin/accounts/show.html.haml @@ -62,11 +62,11 @@ = number_to_human_size @account.media_attachments.sum('file_file_size') - if @account.silenced? - = link_to 'Undo silence', unsilence_admin_account_path(@account.id), method: :post, class: 'button' + = link_to 'Undo silence', admin_account_silence_path(@account.id), method: :delete, class: 'button' - else - = link_to 'Silence', silence_admin_account_path(@account.id), method: :post, class: 'button' + = link_to 'Silence', admin_account_silence_path(@account.id), method: :post, class: 'button' - if @account.suspended? - = link_to 'Undo suspension', unsuspend_admin_account_path(@account.id), method: :post, class: 'button' + = link_to 'Undo suspension', admin_account_suspension_path(@account.id), method: :delete, class: 'button' - else - = link_to 'Perform full suspension', suspend_admin_account_path(@account.id), method: :post, data: { confirm: 'Are you sure?' }, class: 'button' + = link_to 'Perform full suspension', admin_account_suspension_path(@account.id), method: :post, data: { confirm: 'Are you sure?' }, class: 'button' diff --git a/config/routes.rb b/config/routes.rb index 78bf7870cd..ca3f31055f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,3 +1,4 @@ + # frozen_string_literal: true require 'sidekiq/web' @@ -89,12 +90,8 @@ Rails.application.routes.draw do end resources :accounts, only: [:index, :show] do - member do - post :silence - post :unsilence - post :suspend - post :unsuspend - end + resource :silence, only: [:create, :destroy] + resource :suspension, only: [:create, :destroy] end end diff --git a/spec/controllers/admin/silences_controller_spec.rb b/spec/controllers/admin/silences_controller_spec.rb new file mode 100644 index 0000000000..7c541d7970 --- /dev/null +++ b/spec/controllers/admin/silences_controller_spec.rb @@ -0,0 +1,24 @@ +require 'rails_helper' + +describe Admin::SilencesController do + let(:account) { Fabricate(:account) } + before do + sign_in Fabricate(:user, admin: true), scope: :user + end + + describe 'POST #create' do + it 'redirects to admin accounts page' do + post :create, params: { account_id: account.id } + + expect(response).to redirect_to(admin_accounts_path) + end + end + + describe 'DELETE #destroy' do + it 'redirects to admin accounts page' do + delete :destroy, params: { account_id: account.id } + + expect(response).to redirect_to(admin_accounts_path) + end + end +end diff --git a/spec/controllers/admin/suspensions_controller_spec.rb b/spec/controllers/admin/suspensions_controller_spec.rb new file mode 100644 index 0000000000..9096f067ef --- /dev/null +++ b/spec/controllers/admin/suspensions_controller_spec.rb @@ -0,0 +1,24 @@ +require 'rails_helper' + +describe Admin::SuspensionsController do + let(:account) { Fabricate(:account) } + before do + sign_in Fabricate(:user, admin: true), scope: :user + end + + describe 'POST #create' do + it 'redirects to admin accounts page' do + post :create, params: { account_id: account.id } + + expect(response).to redirect_to(admin_accounts_path) + end + end + + describe 'DELETE #destroy' do + it 'redirects to admin accounts page' do + delete :destroy, params: { account_id: account.id } + + expect(response).to redirect_to(admin_accounts_path) + end + end +end diff --git a/spec/models/account_filter_spec.rb b/spec/models/account_filter_spec.rb new file mode 100644 index 0000000000..1599c5ae8b --- /dev/null +++ b/spec/models/account_filter_spec.rb @@ -0,0 +1,31 @@ +require 'rails_helper' + +describe AccountFilter do + describe 'with empty params' do + it 'defaults to alphabetic account list' do + filter = AccountFilter.new({}) + + expect(filter.results).to eq Account.alphabetic + end + end + + describe 'with invalid params' do + it 'raises with key error' do + filter = AccountFilter.new(wrong: true) + + expect { filter.results }.to raise_error(/wrong/) + end + end + + describe 'with valid params' do + it 'combines filters on Account' do + filter = AccountFilter.new(by_domain: 'test.com', silenced: true) + + allow(Account).to receive(:where).and_return(Account.none) + allow(Account).to receive(:silenced).and_return(Account.none) + filter.results + expect(Account).to have_received(:where).with(domain: 'test.com') + expect(Account).to have_received(:silenced) + end + end +end