Refactoring: Clean up user_spec.rb

spec/factories/user.rb

@@ -1,21 +1,14 @@
 FactoryBot.define do
-  sequence :email do |n|
-    "nicole.braun#{n}@zammad.org"
-  end
-end
-
-FactoryBot.define do
-
   factory :user do
-    login         'nicole.braun'
+    login            { 'nicole.braun' }
-    firstname     'Nicole'
+    firstname        { 'Nicole' }
-    lastname      'Braun'
+    lastname         { 'Braun' }
-    email         { generate(:email) }
+    sequence(:email) { |n| "nicole.braun#{n}@zammad.org" }
-    password      nil
+    password         { nil }
-    active        true
+    active           { true }
-    login_failed  0
+    login_failed     { 0 }
-    updated_by_id 1
+    updated_by_id    { 1 }
-    created_by_id 1
+    created_by_id    { 1 }
 
     factory :customer_user, aliases: %i[customer] do
       role_ids { Role.signup_role_ids.sort }

spec/models/user_spec.rb

@@ -7,7 +7,7 @@ require 'models/concerns/has_xss_sanitized_note_examples'
 require 'models/concerns/can_be_imported_examples'
 require 'models/concerns/can_lookup_examples'
 
-RSpec.describe User do
+RSpec.describe User, type: :model do
   it_behaves_like 'ApplicationModel'
   it_behaves_like 'HasGroups', group_access_factory: :agent_user
   it_behaves_like 'HasRoles', group_access_factory: :agent_user
@@ -17,12 +17,296 @@ RSpec.describe User do
   it_behaves_like 'CanLookup'
 
   subject(:user) { create(:user) }
+  let(:admin)    { create(:admin_user) }
+  let(:agent)    { create(:agent_user) }
+  let(:customer) { create(:customer_user) }
 
-  describe 'attributes' do
+  describe 'Class methods:' do
+    describe '.authenticate' do
+      subject(:user) { create(:user, password: password) }
+      let(:password) { Faker::Internet.password }
+
+      context 'with valid credentials' do
+        it 'returns the matching user' do
+          expect(described_class.authenticate(user.login, password))
+            .to eq(user)
+        end
+
+        context 'but exceeding failed login limit' do
+          before { user.update(login_failed: 999) }
+
+          it 'returns nil' do
+            expect(described_class.authenticate(user.login, password))
+              .to be(nil)
+          end
+        end
+      end
+
+      context 'with valid user and invalid password' do
+        it 'increments failed login count' do
+          expect { described_class.authenticate(user.login, password.next) }
+            .to change { user.reload.login_failed }.by(1)
+        end
+
+        it 'returns nil' do
+          expect(described_class.authenticate(user.login, password.next)).to be(nil)
+        end
+      end
+
+      context 'with inactive user’s login' do
+        before { user.update(active: false) }
+
+        it 'returns nil' do
+          expect(described_class.authenticate(user.login, password)).to be(nil)
+        end
+      end
+
+      context 'with non-existent user login' do
+        it 'returns nil' do
+          expect(described_class.authenticate('john.doe', password)).to be(nil)
+        end
+      end
+
+      context 'with empty login string' do
+        it 'returns nil' do
+          expect(described_class.authenticate('', password)).to be(nil)
+        end
+      end
+
+      context 'with empty password string' do
+        it 'returns nil' do
+          expect(described_class.authenticate(user.login, '')).to be(nil)
+        end
+      end
+    end
+
+    describe '.identify' do
+      it 'returns users by given login' do
+        expect(User.identify(user.login)).to eq(user)
+      end
+
+      it 'returns users by given email' do
+        expect(User.identify(user.email)).to eq(user)
+      end
+    end
+  end
+
+  describe 'Instance methods:' do
+    describe '#max_login_failed?' do
+      it { is_expected.to respond_to(:max_login_failed?) }
+
+      context 'with "password_max_login_failed" setting' do
+        before { Setting.set('password_max_login_failed', 5) }
+        before { user.update(login_failed: 5) }
+
+        it 'returns true once user’s #login_failed count exceeds the setting' do
+          expect { user.update(login_failed: 6) }
+            .to change { user.max_login_failed? }.to(true)
+        end
+      end
+
+      context 'without password_max_login_failed setting' do
+        before { Setting.set('password_max_login_failed', nil) }
+        before { user.update(login_failed: 0) }
+
+        it 'defaults to 0' do
+          expect { user.update(login_failed: 1) }
+            .to change { user.max_login_failed? }.to(true)
+        end
+      end
+    end
+
+    describe '#out_of_office_agent' do
+      it { is_expected.to respond_to(:out_of_office_agent) }
+
+      context 'when user has no designated substitute' do
+        it 'returns nil' do
+          expect(user.out_of_office_agent).to be(nil)
+        end
+      end
+
+      context 'when user has designated substitute, and is out of office' do
+        let(:substitute) { create(:user) }
+        subject(:user) do
+          create(:user,
+                 out_of_office:                true,
+                 out_of_office_start_at:       Time.zone.yesterday,
+                 out_of_office_end_at:         Time.zone.tomorrow,
+                 out_of_office_replacement_id: substitute.id,)
+        end
+
+        it 'returns the designated substitute' do
+          expect(user.out_of_office_agent).to eq(substitute)
+        end
+      end
+    end
+
+    describe '#by_reset_token' do
+      let(:token) { create(:token_password_reset) }
+      subject(:user) { token.user }
+
+      context 'with a valid token' do
+        it 'returns the matching user' do
+          expect(described_class.by_reset_token(token.name)).to eq(user)
+        end
+      end
+
+      context 'with an invalid token' do
+        it 'returns nil' do
+          expect(described_class.by_reset_token('not-existing')).to be(nil)
+        end
+      end
+    end
+
+    describe '#password_reset_via_token' do
+      let!(:token) { create(:token_password_reset) }
+      subject(:user) { token.user }
+
+      it 'changes the password of the token user and destroys the token' do
+        expect { described_class.password_reset_via_token(token.name, Faker::Internet.password) }
+          .to change { user.reload.password }
+          .and change { Token.count }.by(-1)
+      end
+    end
+
+    describe '#access?' do
+      context 'when an admin' do
+        subject(:user) { create(:user, roles: [partial_admin_role]) }
+
+        context 'with "admin.user" privileges' do
+          let(:partial_admin_role) do
+            create(:role).tap { |role| role.permission_grant('admin.user') }
+          end
+
+          context 'wants to read, change, or delete any user' do
+            it 'returns true' do
+              expect(admin.access?(user, 'read')).to be(true)
+              expect(admin.access?(user, 'change')).to be(true)
+              expect(admin.access?(user, 'delete')).to be(true)
+              expect(agent.access?(user, 'read')).to be(true)
+              expect(agent.access?(user, 'change')).to be(true)
+              expect(agent.access?(user, 'delete')).to be(true)
+              expect(customer.access?(user, 'read')).to be(true)
+              expect(customer.access?(user, 'change')).to be(true)
+              expect(customer.access?(user, 'delete')).to be(true)
+              expect(user.access?(user, 'read')).to be(true)
+              expect(user.access?(user, 'change')).to be(true)
+              expect(user.access?(user, 'delete')).to be(true)
+            end
+          end
+        end
+
+        context 'without "admin.user" privileges' do
+          let(:partial_admin_role) do
+            create(:role).tap { |role| role.permission_grant('admin.tag') }
+          end
+
+          context 'wants to read any user' do
+            it 'returns true' do
+              expect(admin.access?(user, 'read')).to be(true)
+              expect(agent.access?(user, 'read')).to be(true)
+              expect(customer.access?(user, 'read')).to be(true)
+              expect(user.access?(user, 'read')).to be(true)
+            end
+          end
+
+          context 'wants to change or delete any user' do
+            it 'returns false' do
+              expect(admin.access?(user, 'change')).to be(false)
+              expect(admin.access?(user, 'delete')).to be(false)
+              expect(agent.access?(user, 'change')).to be(false)
+              expect(agent.access?(user, 'delete')).to be(false)
+              expect(customer.access?(user, 'change')).to be(false)
+              expect(customer.access?(user, 'delete')).to be(false)
+              expect(user.access?(user, 'change')).to be(false)
+              expect(user.access?(user, 'delete')).to be(false)
+            end
+          end
+        end
+      end
+
+      context 'when an agent' do
+        subject(:user) { create(:agent_user) }
+
+        context 'wants to read any user' do
+          it 'returns true' do
+            expect(admin.access?(user, 'read')).to be(true)
+            expect(agent.access?(user, 'read')).to be(true)
+            expect(customer.access?(user, 'read')).to be(true)
+            expect(user.access?(user, 'read')).to be(true)
+          end
+        end
+
+        context 'wants to change' do
+          context 'any admin or agent' do
+            it 'returns false' do
+              expect(admin.access?(user, 'change')).to be(false)
+              expect(agent.access?(user, 'change')).to be(false)
+              expect(user.access?(user, 'change')).to be(false)
+            end
+          end
+
+          context 'any customer' do
+            it 'returns true' do
+              expect(customer.access?(user, 'change')).to be(true)
+            end
+          end
+        end
+
+        context 'wants to delete any user' do
+          it 'returns false' do
+            expect(admin.access?(user, 'delete')).to be(false)
+            expect(agent.access?(user, 'delete')).to be(false)
+            expect(customer.access?(user, 'delete')).to be(false)
+            expect(user.access?(user, 'delete')).to be(false)
+          end
+        end
+      end
+
+      context 'when a customer' do
+        subject(:user) { create(:customer_user, :with_org) }
+        let(:colleague) { create(:customer_user, organization: user.organization) }
+
+        context 'wants to read' do
+          context 'any admin, agent, or customer from a different organization' do
+            it 'returns false' do
+              expect(admin.access?(user, 'read')).to be(false)
+              expect(agent.access?(user, 'read')).to be(false)
+              expect(customer.access?(user, 'read')).to be(false)
+            end
+          end
+
+          context 'any customer from the same organization' do
+            it 'returns true' do
+              expect(user.access?(user, 'read')).to be(true)
+              expect(colleague.access?(user, 'read')).to be(true)
+            end
+          end
+        end
+
+        context 'wants to change or delete any user' do
+          it 'returns false' do
+            expect(admin.access?(user, 'change')).to be(false)
+            expect(admin.access?(user, 'delete')).to be(false)
+            expect(agent.access?(user, 'change')).to be(false)
+            expect(agent.access?(user, 'delete')).to be(false)
+            expect(customer.access?(user, 'change')).to be(false)
+            expect(customer.access?(user, 'delete')).to be(false)
+            expect(colleague.access?(user, 'change')).to be(false)
+            expect(colleague.access?(user, 'delete')).to be(false)
+            expect(user.access?(user, 'change')).to be(false)
+            expect(user.access?(user, 'delete')).to be(false)
+          end
+        end
+      end
+    end
+  end
+
+  describe 'Attributes:' do
     describe '#login_failed' do
       before { user.update(login_failed: 1) }
 
-      it 'resets failed login count when password is changed' do
+      it 'is reset to 0 when password is updated' do
         expect { user.update(password: Faker::Internet.password) }
           .to change { user.login_failed }.to(0)
       end
@@ -125,7 +409,7 @@ RSpec.describe User do
     end
   end
 
-  describe 'associations' do
+  describe 'Associations:' do
     describe '#organization' do
       describe 'email domain-based assignment' do
         subject(:user) { build(:user) }
@@ -191,513 +475,27 @@ RSpec.describe User do
     end
   end
 
-  describe '#max_login_failed?' do
+  describe 'Callbacks & Observers -' do
-    it { is_expected.to respond_to(:max_login_failed?) }
+    describe 'System-wide agent limit checks:' do
-
-    context 'with password_max_login_failed setting' do
-      before { Setting.set('password_max_login_failed', 5) }
-      before { user.update(login_failed: 5) }
-
-      it 'returns true once user’s #login_failed count exceeds the setting' do
-        expect { user.update(login_failed: 6) }
-          .to change { user.max_login_failed? }.to(true)
-      end
-    end
-
-    context 'without password_max_login_failed setting' do
-      before { Setting.set('password_max_login_failed', nil) }
-      before { user.update(login_failed: 0) }
-
-      it 'defaults to 0' do
-        expect { user.update(login_failed: 1) }
-          .to change { user.max_login_failed? }.to(true)
-      end
-    end
-  end
-
-  describe '#out_of_office_agent' do
-    it { is_expected.to respond_to(:out_of_office_agent) }
-
-    context 'when user has no designated substitute' do
-      it 'returns nil' do
-        expect(user.out_of_office_agent).to be(nil)
-      end
-    end
-
-    context 'when user has designated substitute, and is out of office' do
-      let(:substitute) { create(:user) }
-      subject(:user) do
-        create(:user,
-               out_of_office:                true,
-               out_of_office_start_at:       Time.zone.yesterday,
-               out_of_office_end_at:         Time.zone.tomorrow,
-               out_of_office_replacement_id: substitute.id,)
-      end
-
-      it 'returns the designated substitute' do
-        expect(user.out_of_office_agent).to eq(substitute)
-      end
-    end
-  end
-
-  describe '.authenticate' do
-    subject(:user) { create(:user, password: password) }
-    let(:password) { Faker::Internet.password }
-
-    context 'with valid credentials' do
-      it 'returns the matching user' do
-        expect(described_class.authenticate(user.login, password))
-          .to eq(user)
-      end
-    end
-
-    context 'with valid credentials, but exceeding failed login limit' do
-      before { user.update(login_failed: 999) }
-
-      it 'returns nil' do
-        expect(described_class.authenticate(user.login, password))
-          .to be(nil)
-      end
-    end
-
-    context 'with valid user and invalid password' do
-      it 'increments failed login count' do
-        expect { described_class.authenticate(user.login, password.next) }
-          .to change { user.reload.login_failed }.by(1)
-      end
-
-      it 'returns nil' do
-        expect(described_class.authenticate(user.login, password.next)).to be(nil)
-      end
-    end
-
-    context 'with inactive user’s login' do
-      before { user.update(active: false) }
-
-      it 'returns nil' do
-        expect(described_class.authenticate(user.login, password)).to be(nil)
-      end
-    end
-
-    context 'with non-existent user login' do
-      it 'returns nil' do
-        expect(described_class.authenticate('john.doe', password)).to be(nil)
-      end
-    end
-
-    context 'with empty login string' do
-      it 'returns nil' do
-        expect(described_class.authenticate('', password)).to be(nil)
-      end
-    end
-
-    context 'with empty password string' do
-      it 'returns nil' do
-        expect(described_class.authenticate(user.login, '')).to be(nil)
-      end
-    end
-  end
-
-  describe '#by_reset_token' do
-    let(:token) { create(:token_password_reset) }
-    subject(:user) { token.user }
-
-    context 'with a valid token' do
-      it 'returns the matching user' do
-        expect(described_class.by_reset_token(token.name)).to eq(user)
-      end
-    end
-
-    context 'with an invalid token' do
-      it 'returns nil' do
-        expect(described_class.by_reset_token('not-existing')).to be(nil)
-      end
-    end
-  end
-
-  describe '#password_reset_via_token' do
-    let!(:token) { create(:token_password_reset) }
-    subject(:user) { token.user }
-
-    it 'changes the password of the token user and destroys the token' do
-      expect { described_class.password_reset_via_token(token.name, Faker::Internet.password) }
-        .to change { user.reload.password }
-        .and change { Token.count }.by(-1)
-    end
-  end
-
-  describe '.identify' do
-    it 'returns users by given login' do
-      expect(User.identify(user.login)).to eq(user)
-    end
-
-    it 'returns users by given email' do
-      expect(User.identify(user.email)).to eq(user)
-    end
-  end
-
-  describe '#access?' do
-
-    let(:role_with_admin_user_permissions) do
-      create(:role).tap do |role|
-        role.permission_grant('admin.user')
-      end
-    end
-    let(:admin_with_admin_user_permissions) { create(:user, roles: [role_with_admin_user_permissions]) }
-
-    let(:role_without_admin_user_permissions) do
-      create(:role).tap do |role|
-        role.permission_grant('admin.tag')
-      end
-    end
-    let(:admin_without_admin_user_permissions) { create(:user, roles: [role_without_admin_user_permissions]) }
-
-    context 'read' do
-
-      context 'admin' do
-
-        let(:requested) { create(:admin_user) }
-
-        it 'is possible for admin.user' do
-          requester = admin_with_admin_user_permissions
-          access    = requested.access?(requester, 'read')
-          expect(access).to be(true)
-        end
-
-        it 'is possible for sub admin without admin.user' do
-          requester = admin_without_admin_user_permissions
-          access    = requested.access?(requester, 'read')
-          expect(access).to be(true)
-        end
-
-        it 'is possible for agent' do
-          requester = create(:agent_user)
-          access    = requested.access?(requester, 'read')
-          expect(access).to be(true)
-        end
-
-        it 'is not possible for customer' do
-          requester = create(:customer_user)
-          access    = requested.access?(requester, 'read')
-          expect(access).to be(false)
-        end
-      end
-
-      context 'agent' do
-
-        let(:requested) { create(:agent_user) }
-
-        it 'is possible for admin.user' do
-          requester = admin_with_admin_user_permissions
-          access    = requested.access?(requester, 'read')
-          expect(access).to be(true)
-        end
-
-        it 'is possible for sub admin without admin.user' do
-          requester = admin_without_admin_user_permissions
-          access    = requested.access?(requester, 'read')
-          expect(access).to be(true)
-        end
-
-        it 'is possible for agent' do
-          requester = create(:agent_user)
-          access    = requested.access?(requester, 'read')
-          expect(access).to be(true)
-        end
-
-        it 'is not possible for customer' do
-          requester = create(:customer_user)
-          access    = requested.access?(requester, 'read')
-          expect(access).to be(false)
-        end
-      end
-
-      context 'customer' do
-
-        let(:requested) { create(:customer_user) }
-
-        it 'is possible for admin.user' do
-          requester = admin_with_admin_user_permissions
-          access    = requested.access?(requester, 'read')
-          expect(access).to be(true)
-        end
-
-        it 'is possible for sub admin without admin.user' do
-          requester = admin_without_admin_user_permissions
-          access    = requested.access?(requester, 'read')
-          expect(access).to be(true)
-        end
-
-        it 'is possible for agent' do
-          requester = create(:agent_user)
-          access    = requested.access?(requester, 'read')
-          expect(access).to be(true)
-
-        end
-
-        it 'is possible for same customer' do
-          access = requested.access?(requested, 'read')
-          expect(access).to be(true)
-        end
-
-        it 'is possible for same organization' do
-          organization = create(:organization)
-          requester    = create(:customer_user, organization: organization)
-          requested.update!(organization: organization)
-          access = requested.access?(requester, 'read')
-          expect(access).to be(true)
-        end
-
-        it 'is not possible for different customer' do
-          requester = create(:customer_user)
-          access    = requested.access?(requester, 'read')
-          expect(access).to be(false)
-        end
-      end
-    end
-
-    context 'change' do
-
-      context 'admin' do
-
-        let(:requested) { create(:admin_user) }
-
-        it 'is possible for admin.user' do
-          requester = admin_with_admin_user_permissions
-          access    = requested.access?(requester, 'change')
-          expect(access).to be(true)
-        end
-
-        it 'is not possible for sub admin without admin.user' do
-          requester = admin_without_admin_user_permissions
-          access    = requested.access?(requester, 'change')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for same for sub admin without admin.user' do
-          access = admin_without_admin_user_permissions.access?(admin_without_admin_user_permissions, 'change')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for agent' do
-          requester = create(:agent_user)
-          access    = requested.access?(requester, 'change')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for customer' do
-          requester = create(:customer_user)
-          access    = requested.access?(requester, 'change')
-          expect(access).to be(false)
-        end
-      end
-
-      context 'agent' do
-
-        let(:requested) { create(:agent_user) }
-
-        it 'is possible for admin.user' do
-          requester = admin_with_admin_user_permissions
-          access    = requested.access?(requester, 'change')
-          expect(access).to be(true)
-        end
-
-        it 'is not possible for sub admin without admin.user' do
-          requester = admin_without_admin_user_permissions
-          access    = requested.access?(requester, 'change')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for same agent' do
-          access = requested.access?(requested, 'change')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for other agent' do
-          requester = create(:agent_user)
-          access    = requested.access?(requester, 'change')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for customer' do
-          requester = create(:customer_user)
-          access    = requested.access?(requester, 'change')
-          expect(access).to be(false)
-        end
-      end
-
-      context 'customer' do
-
-        let(:requested) { create(:customer_user) }
-
-        it 'is possible for admin.user' do
-          requester = admin_with_admin_user_permissions
-          access    = requested.access?(requester, 'change')
-          expect(access).to be(true)
-        end
-
-        it 'is not possible for sub admin without admin.user' do
-          requester = admin_without_admin_user_permissions
-          access    = requested.access?(requester, 'change')
-          expect(access).to be(false)
-        end
-
-        it 'is possible for agent' do
-          requester = create(:agent_user)
-          access    = requested.access?(requester, 'change')
-          expect(access).to be(true)
-
-        end
-
-        it 'is not possible for same customer' do
-          access = requested.access?(requested, 'change')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for same organization' do
-          organization = create(:organization)
-          requester    = create(:customer_user, organization: organization)
-          requested.update!(organization: organization)
-          access = requested.access?(requester, 'change')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for different customer' do
-          requester = create(:customer_user)
-          access    = requested.access?(requester, 'change')
-          expect(access).to be(false)
-        end
-      end
-    end
-
-    context 'delete' do
-
-      context 'admin' do
-
-        let(:requested) { create(:admin_user) }
-
-        it 'is possible for admin.user' do
-          requester = admin_with_admin_user_permissions
-          access    = requested.access?(requester, 'delete')
-          expect(access).to be(true)
-        end
-
-        it 'is not possible for sub admin without admin.user' do
-          requester = admin_without_admin_user_permissions
-          access    = requested.access?(requester, 'delete')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for agent' do
-          requester = create(:agent_user)
-          access    = requested.access?(requester, 'delete')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for customer' do
-          requester = create(:customer_user)
-          access    = requested.access?(requester, 'delete')
-          expect(access).to be(false)
-        end
-      end
-
-      context 'agent' do
-
-        let(:requested) { create(:agent_user) }
-
-        it 'is possible for admin.user' do
-          requester = admin_with_admin_user_permissions
-          access    = requested.access?(requester, 'delete')
-          expect(access).to be(true)
-        end
-
-        it 'is not possible for sub admin without admin.user' do
-          requester = admin_without_admin_user_permissions
-          access    = requested.access?(requester, 'delete')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for agent' do
-          requester = create(:agent_user)
-          access    = requested.access?(requester, 'delete')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for customer' do
-          requester = create(:customer_user)
-          access    = requested.access?(requester, 'delete')
-          expect(access).to be(false)
-        end
-      end
-
-      context 'customer' do
-
-        let(:requested) { create(:customer_user) }
-
-        it 'is possible for admin.user' do
-          requester = admin_with_admin_user_permissions
-          access    = requested.access?(requester, 'delete')
-          expect(access).to be(true)
-        end
-
-        it 'is not possible for sub admin without admin.user' do
-          requester = admin_without_admin_user_permissions
-          access    = requested.access?(requester, 'delete')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for agent' do
-          requester = create(:agent_user)
-          access    = requested.access?(requester, 'delete')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for same customer' do
-          access = requested.access?(requested, 'delete')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for same organization' do
-          organization = create(:organization)
-          requester    = create(:customer_user, organization: organization)
-          requested.update!(organization: organization)
-          access = requested.access?(requester, 'delete')
-          expect(access).to be(false)
-        end
-
-        it 'is not possible for different customer' do
-          requester = create(:customer_user)
-          access    = requested.access?(requester, 'delete')
-          expect(access).to be(false)
-        end
-      end
-    end
-  end
-
-  describe 'system-wide agent limit' do
-
-    def current_agent_count
-      User.with_permissions('ticket.agent').count
-    end
-
       let(:agent_role) { Role.lookup(name: 'Agent') }
       let(:admin_role) { Role.lookup(name: 'Admin') }
+      let(:current_agents) { User.with_permissions('ticket.agent') }
 
       describe '#validate_agent_limit_by_role' do
         context 'for Integer value of system_agent_limit' do
           context 'before exceeding the agent limit' do
-          before { Setting.set('system_agent_limit', current_agent_count + 1) }
+            before { Setting.set('system_agent_limit', current_agents.count + 1) }
 
             it 'grants agent creation' do
               expect { create(:agent_user) }
-              .to change { current_agent_count }.by(1)
+                .to change { current_agents.count }.by(1)
             end
 
             it 'grants role change' do
               future_agent = create(:customer_user)
 
               expect { future_agent.roles = [agent_role] }
-              .to change { current_agent_count }.by(1)
+                .to change { current_agents.count }.by(1)
             end
 
             describe 'role updates' do
@@ -722,41 +520,41 @@ RSpec.describe User do
 
           context 'when exceeding the agent limit' do
             it 'creation of new agents' do
-            Setting.set('system_agent_limit', current_agent_count + 2)
+              Setting.set('system_agent_limit', current_agents.count + 2)
 
               create_list(:agent_user, 2)
 
               expect { create(:agent_user) }
                 .to raise_error(Exceptions::UnprocessableEntity)
-              .and change { current_agent_count }.by(0)
+                .and change { current_agents.count }.by(0)
             end
 
             it 'prevents role change' do
-            Setting.set('system_agent_limit', current_agent_count)
+              Setting.set('system_agent_limit', current_agents.count)
 
               future_agent = create(:customer_user)
 
               expect { future_agent.roles = [agent_role] }
                 .to raise_error(Exceptions::UnprocessableEntity)
-              .and change { current_agent_count }.by(0)
+                .and change { current_agents.count }.by(0)
             end
           end
         end
 
         context 'for String value of system_agent_limit' do
           context 'before exceeding the agent limit' do
-          before { Setting.set('system_agent_limit', (current_agent_count + 1).to_s) }
+            before { Setting.set('system_agent_limit', (current_agents.count + 1).to_s) }
 
             it 'grants agent creation' do
               expect { create(:agent_user) }
-              .to change { current_agent_count }.by(1)
+                .to change { current_agents.count }.by(1)
             end
 
             it 'grants role change' do
               future_agent = create(:customer_user)
 
               expect { future_agent.roles = [agent_role] }
-              .to change { current_agent_count }.by(1)
+                .to change { current_agents.count }.by(1)
             end
 
             describe 'role updates' do
@@ -781,23 +579,23 @@ RSpec.describe User do
 
           context 'when exceeding the agent limit' do
             it 'creation of new agents' do
-            Setting.set('system_agent_limit', (current_agent_count + 2).to_s)
+              Setting.set('system_agent_limit', (current_agents.count + 2).to_s)
 
               create_list(:agent_user, 2)
 
               expect { create(:agent_user) }
                 .to raise_error(Exceptions::UnprocessableEntity)
-              .and change { current_agent_count }.by(0)
+                .and change { current_agents.count }.by(0)
             end
 
             it 'prevents role change' do
-            Setting.set('system_agent_limit', current_agent_count.to_s)
+              Setting.set('system_agent_limit', current_agents.count.to_s)
 
               future_agent = create(:customer_user)
 
               expect { future_agent.roles = [agent_role] }
                 .to raise_error(Exceptions::UnprocessableEntity)
-              .and change { current_agent_count }.by(0)
+                .and change { current_agents.count }.by(0)
             end
           end
         end
@@ -805,7 +603,7 @@ RSpec.describe User do
 
       describe '#validate_agent_limit_by_attributes' do
         context 'for Integer value of system_agent_limit' do
-        before { Setting.set('system_agent_limit', current_agent_count) }
+          before { Setting.set('system_agent_limit', current_agents.count) }
 
           context 'when exceeding the agent limit' do
             it 'prevents re-activation of agents' do
@@ -813,13 +611,13 @@ RSpec.describe User do
 
               expect { inactive_agent.update!(active: true) }
                 .to raise_error(Exceptions::UnprocessableEntity)
-              .and change { current_agent_count }.by(0)
+                .and change { current_agents.count }.by(0)
             end
           end
         end
 
         context 'for String value of system_agent_limit' do
-        before { Setting.set('system_agent_limit', current_agent_count.to_s) }
+          before { Setting.set('system_agent_limit', current_agents.count.to_s) }
 
           context 'when exceeding the agent limit' do
             it 'prevents re-activation of agents' do
@@ -827,7 +625,8 @@ RSpec.describe User do
 
               expect { inactive_agent.update!(active: true) }
                 .to raise_error(Exceptions::UnprocessableEntity)
-              .and change { current_agent_count }.by(0)
+                .and change { current_agents.count }.by(0)
+            end
           end
         end
       end