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

diff --git a/app/controllers/admin/accounts_controller.rb b/app/controllers/admin/accounts_controller.rb
index 71cb8edd876303d3dfc65b9495560d3b62cbef0d..0e9e52f42a1e8a88d52b3db710a50cc2fdbf8547 100644 (file)
--- 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?
-    end
-
-    def show; end
-
-    def suspend
-      Admin::SuspensionWorker.perform_async(@account.id)
-      redirect_to admin_accounts_path
+      @accounts = filtered_accounts.page(params[:page])
     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 (file)
index 0000000..81a3008
--- /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 (file)
index 0000000..5d9048d
--- /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 (file)
index 0000000..a8d8c88
--- /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 ba1c3bae7b802c260218d7e58c747b4801ad20b1..22901aed1e4b97e105c28f101cf90bac8c955d32 100644 (file)
--- 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 78bf7870cdb5d59349e98c5408eae187aa5ea129..ca3f31055f54629729280c04561f1a043d486724 100644 (file)
--- 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 (file)
index 0000000..7c541d7
--- /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 (file)
index 0000000..9096f06
--- /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 (file)
index 0000000..1599c5a
--- /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