From 8a2d764d342d2aa47378b1fb0380629e763d64b5 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <matt@jankowski.online>
Date: Tue, 28 Jan 2025 03:08:16 -0500
Subject: [PATCH] Reduce factory creation across `controllers/admin` specs
 (#33752)

---
 .../admin/domain_blocks_controller_spec.rb    | 36 ++++---------------
 .../email_domain_blocks_controller_spec.rb    |  4 +--
 .../admin/roles_controller_spec.rb            | 32 +++++------------
 .../admin/users/roles_controller_spec.rb      | 12 ++-----
 4 files changed, 18 insertions(+), 66 deletions(-)

diff --git a/spec/controllers/admin/domain_blocks_controller_spec.rb b/spec/controllers/admin/domain_blocks_controller_spec.rb
index 4d48a0d9d3..f3f08c7940 100644
--- a/spec/controllers/admin/domain_blocks_controller_spec.rb
+++ b/spec/controllers/admin/domain_blocks_controller_spec.rb
@@ -48,15 +48,11 @@ RSpec.describe Admin::DomainBlocksController do
         post :create, params: { domain_block: { domain: 'example.com', severity: 'silence' } }
       end
 
-      it 'records a block' do
+      it 'records a block, calls a worker, redirects' do
         expect(DomainBlock.exists?(domain: 'example.com', severity: 'silence')).to be true
-      end
 
-      it 'calls DomainBlockWorker' do
         expect(DomainBlockWorker).to have_received(:perform_async)
-      end
 
-      it 'redirects with a success message' do
         expect(flash[:notice]).to eq I18n.t('admin.domain_blocks.created_msg')
         expect(response).to redirect_to(admin_instances_path(limited: '1'))
       end
@@ -68,15 +64,11 @@ RSpec.describe Admin::DomainBlocksController do
         post :create, params: { domain_block: { domain: 'example.com', severity: 'silence' } }
       end
 
-      it 'does not record a block' do
+      it 'does not record a block or call worker, renders new' do
         expect(DomainBlock.exists?(domain: 'example.com', severity: 'silence')).to be false
-      end
 
-      it 'does not call DomainBlockWorker' do
         expect(DomainBlockWorker).to_not have_received(:perform_async)
-      end
 
-      it 'renders new' do
         expect(response).to render_template :new
       end
     end
@@ -87,15 +79,11 @@ RSpec.describe Admin::DomainBlocksController do
           post :create, params: { domain_block: { domain: 'example.com', severity: 'suspend', reject_media: true, reject_reports: true } }
         end
 
-        it 'does not record a block' do
+        it 'does not record a block or call worker, renders confirm suspension' do
           expect(DomainBlock.exists?(domain: 'example.com', severity: 'suspend')).to be false
-        end
 
-        it 'does not call DomainBlockWorker' do
           expect(DomainBlockWorker).to_not have_received(:perform_async)
-        end
 
-        it 'renders confirm_suspension' do
           expect(response).to render_template :confirm_suspension
         end
       end
@@ -105,15 +93,11 @@ RSpec.describe Admin::DomainBlocksController do
           post :create, params: { :domain_block => { domain: 'example.com', severity: 'suspend', reject_media: true, reject_reports: true }, 'confirm' => '' }
         end
 
-        it 'records a block' do
+        it 'records a block and calls worker and redirects' do
           expect(DomainBlock.exists?(domain: 'example.com', severity: 'suspend')).to be true
-        end
 
-        it 'calls DomainBlockWorker' do
           expect(DomainBlockWorker).to have_received(:perform_async)
-        end
 
-        it 'redirects with a success message' do
           expect(flash[:notice]).to eq I18n.t('admin.domain_blocks.created_msg')
           expect(response).to redirect_to(admin_instances_path(limited: '1'))
         end
@@ -130,15 +114,11 @@ RSpec.describe Admin::DomainBlocksController do
           post :create, params: { domain_block: { domain: 'example.com', severity: 'suspend', reject_media: true, reject_reports: true } }
         end
 
-        it 'does not record a block' do
+        it 'does not record a block or call worker, renders confirm suspension' do
           expect(DomainBlock.exists?(domain: 'example.com', severity: 'suspend')).to be false
-        end
 
-        it 'does not call DomainBlockWorker' do
           expect(DomainBlockWorker).to_not have_received(:perform_async)
-        end
 
-        it 'renders confirm_suspension' do
           expect(response).to render_template :confirm_suspension
         end
       end
@@ -148,15 +128,11 @@ RSpec.describe Admin::DomainBlocksController do
           post :create, params: { :domain_block => { domain: 'example.com', severity: 'suspend', reject_media: true, reject_reports: true }, 'confirm' => '' }
         end
 
-        it 'updates the record' do
+        it 'updates the record and calls worker, redirects' do
           expect(DomainBlock.exists?(domain: 'example.com', severity: 'suspend')).to be true
-        end
 
-        it 'calls DomainBlockWorker' do
           expect(DomainBlockWorker).to have_received(:perform_async)
-        end
 
-        it 'redirects with a success message' do
           expect(flash[:notice]).to eq I18n.t('admin.domain_blocks.created_msg')
           expect(response).to redirect_to(admin_instances_path(limited: '1'))
         end
diff --git a/spec/controllers/admin/email_domain_blocks_controller_spec.rb b/spec/controllers/admin/email_domain_blocks_controller_spec.rb
index c7460c2110..f274c01281 100644
--- a/spec/controllers/admin/email_domain_blocks_controller_spec.rb
+++ b/spec/controllers/admin/email_domain_blocks_controller_spec.rb
@@ -58,11 +58,9 @@ RSpec.describe Admin::EmailDomainBlocksController do
         post :create, params: { email_domain_block: { domain: 'example.com' }, save: '' }
       end
 
-      it 'blocks the domain' do
+      it 'blocks the domain and redirects to email domain blocks' do
         expect(EmailDomainBlock.find_by(domain: 'example.com')).to_not be_nil
-      end
 
-      it 'redirects to e-mail domain blocks' do
         expect(response).to redirect_to(admin_email_domain_blocks_path)
       end
     end
diff --git a/spec/controllers/admin/roles_controller_spec.rb b/spec/controllers/admin/roles_controller_spec.rb
index 2c43a0ca87..173a89e5d5 100644
--- a/spec/controllers/admin/roles_controller_spec.rb
+++ b/spec/controllers/admin/roles_controller_spec.rb
@@ -68,11 +68,9 @@ RSpec.describe Admin::RolesController do
         let(:selected_position) { 1 }
         let(:selected_permissions_as_keys) { %w(manage_roles) }
 
-        it 'redirects to roles page' do
+        it 'redirects to roles page and creates role' do
           expect(response).to redirect_to(admin_roles_path)
-        end
 
-        it 'creates new role' do
           expect(UserRole.find_by(name: 'Bar')).to_not be_nil
         end
       end
@@ -81,11 +79,9 @@ RSpec.describe Admin::RolesController do
         let(:selected_position) { 100 }
         let(:selected_permissions_as_keys) { %w(manage_roles) }
 
-        it 'renders new template' do
+        it 'renders new template and does not create role' do
           expect(response).to render_template(:new)
-        end
 
-        it 'does not create new role' do
           expect(UserRole.find_by(name: 'Bar')).to be_nil
         end
       end
@@ -94,11 +90,9 @@ RSpec.describe Admin::RolesController do
         let(:selected_position) { 1 }
         let(:selected_permissions_as_keys) { %w(manage_roles manage_users manage_reports) }
 
-        it 'renders new template' do
+        it 'renders new template and does not create role' do
           expect(response).to render_template(:new)
-        end
 
-        it 'does not create new role' do
           expect(UserRole.find_by(name: 'Bar')).to be_nil
         end
       end
@@ -109,11 +103,9 @@ RSpec.describe Admin::RolesController do
         let(:selected_position) { 1 }
         let(:selected_permissions_as_keys) { %w(manage_roles manage_users manage_reports) }
 
-        it 'redirects to roles page' do
+        it 'redirects to roles page and creates new role' do
           expect(response).to redirect_to(admin_roles_path)
-        end
 
-        it 'creates new role' do
           expect(UserRole.find_by(name: 'Bar')).to_not be_nil
         end
       end
@@ -166,11 +158,9 @@ RSpec.describe Admin::RolesController do
     end
 
     context 'when user does not have permission to manage roles' do
-      it 'returns http forbidden' do
+      it 'returns http forbidden and does not update role' do
         expect(response).to have_http_status(403)
-      end
 
-      it 'does not update the role' do
         expect(role.reload.name).to eq 'Bar'
       end
     end
@@ -179,11 +169,9 @@ RSpec.describe Admin::RolesController do
       let(:permissions) { UserRole::FLAGS[:manage_roles] }
 
       context 'when role has permissions the user doesn\'t' do
-        it 'renders edit template' do
+        it 'renders edit template and does not update role' do
           expect(response).to render_template(:edit)
-        end
 
-        it 'does not update the role' do
           expect(role.reload.name).to eq 'Bar'
         end
       end
@@ -192,11 +180,9 @@ RSpec.describe Admin::RolesController do
         let(:permissions) { UserRole::FLAGS[:manage_roles] | UserRole::FLAGS[:manage_users] }
 
         context 'when user outranks the role' do
-          it 'redirects to roles page' do
+          it 'redirects to roles page and updates role' do
             expect(response).to redirect_to(admin_roles_path)
-          end
 
-          it 'updates the role' do
             expect(role.reload.name).to eq 'Baz'
           end
         end
@@ -204,11 +190,9 @@ RSpec.describe Admin::RolesController do
         context 'when role outranks user' do
           let(:role_position) { current_role.position + 1 }
 
-          it 'returns http forbidden' do
+          it 'returns http forbidden and does not update role' do
             expect(response).to have_http_status(403)
-          end
 
-          it 'does not update the role' do
             expect(role.reload.name).to eq 'Bar'
           end
         end
diff --git a/spec/controllers/admin/users/roles_controller_spec.rb b/spec/controllers/admin/users/roles_controller_spec.rb
index bfc2bb151f..a7d59181d6 100644
--- a/spec/controllers/admin/users/roles_controller_spec.rb
+++ b/spec/controllers/admin/users/roles_controller_spec.rb
@@ -44,11 +44,9 @@ RSpec.describe Admin::Users::RolesController do
       let(:permissions) { UserRole::FLAGS[:manage_roles] }
       let(:position) { 1 }
 
-      it 'updates user role' do
+      it 'updates user role and redirects' do
         expect(user.reload.role_id).to eq selected_role&.id
-      end
 
-      it 'redirects back to account page' do
         expect(response).to redirect_to(admin_account_path(user.account_id))
       end
     end
@@ -57,11 +55,9 @@ RSpec.describe Admin::Users::RolesController do
       let(:permissions) { UserRole::FLAGS[:administrator] }
       let(:position) { 100 }
 
-      it 'does not update user role' do
+      it 'does not update user role and renders edit' do
         expect(user.reload.role_id).to eq previous_role&.id
-      end
 
-      it 'renders edit form' do
         expect(response).to render_template(:show)
       end
     end
@@ -71,11 +67,9 @@ RSpec.describe Admin::Users::RolesController do
       let(:permissions) { UserRole::FLAGS[:manage_roles] }
       let(:position) { 1 }
 
-      it 'does not update user role' do
+      it 'does not update user role and returns http forbidden' do
         expect(user.reload.role_id).to eq previous_role&.id
-      end
 
-      it 'returns http forbidden' do
         expect(response).to have_http_status(403)
       end
     end