From c02d7f2159049f7748e4c3b5a6ab0a8dc9674b67 Mon Sep 17 00:00:00 2001 From: Ryan Lue Date: Thu, 31 Jan 2019 12:41:54 +0800 Subject: [PATCH] Refactoring: Clean up user_spec.rb --- spec/factories/user.rb | 25 +- spec/models/user_spec.rb | 1029 +++++++++++++++----------------------- 2 files changed, 423 insertions(+), 631 deletions(-) diff --git a/spec/factories/user.rb b/spec/factories/user.rb index 8d2a53953..8375a57fc 100644 --- a/spec/factories/user.rb +++ b/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' - firstname 'Nicole' - lastname 'Braun' - email { generate(:email) } - password nil - active true - login_failed 0 - updated_by_id 1 - created_by_id 1 + login { 'nicole.braun' } + firstname { 'Nicole' } + lastname { 'Braun' } + sequence(:email) { |n| "nicole.braun#{n}@zammad.org" } + password { nil } + active { true } + login_failed { 0 } + updated_by_id { 1 } + created_by_id { 1 } factory :customer_user, aliases: %i[customer] do role_ids { Role.signup_role_ids.sort } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 39a8b18c1..09be4b516 100644 --- a/spec/models/user_spec.rb +++ b/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,643 +475,158 @@ RSpec.describe User do end end - 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 '.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') } - - 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) } - - it 'grants agent creation' do - expect { create(:agent_user) } - .to change { current_agent_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) - end - - describe 'role updates' do - let(:agent) { create(:agent_user) } - - it 'grants update by instances' do - expect { agent.roles = [admin_role, agent_role] } - .not_to raise_error + describe 'Callbacks & Observers -' do + describe 'System-wide agent limit checks:' do + 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_agents.count + 1) } + + it 'grants agent creation' do + expect { create(:agent_user) } + .to change { current_agents.count }.by(1) end - it 'grants update by id (Integer)' do - expect { agent.role_ids = [admin_role.id, agent_role.id] } - .not_to raise_error + it 'grants role change' do + future_agent = create(:customer_user) + + expect { future_agent.roles = [agent_role] } + .to change { current_agents.count }.by(1) end - it 'grants update by id (String)' do - expect { agent.role_ids = [admin_role.id.to_s, agent_role.id.to_s] } - .not_to raise_error + describe 'role updates' do + let(:agent) { create(:agent_user) } + + it 'grants update by instances' do + expect { agent.roles = [admin_role, agent_role] } + .not_to raise_error + end + + it 'grants update by id (Integer)' do + expect { agent.role_ids = [admin_role.id, agent_role.id] } + .not_to raise_error + end + + it 'grants update by id (String)' do + expect { agent.role_ids = [admin_role.id.to_s, agent_role.id.to_s] } + .not_to raise_error + end + end + end + + context 'when exceeding the agent limit' do + it 'creation of new agents' do + 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_agents.count }.by(0) + end + + it 'prevents role change' do + 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_agents.count }.by(0) end end end - context 'when exceeding the agent limit' do - it 'creation of new agents' do - Setting.set('system_agent_limit', current_agent_count + 2) + context 'for String value of system_agent_limit' do + context 'before exceeding the agent limit' do + before { Setting.set('system_agent_limit', (current_agents.count + 1).to_s) } - create_list(:agent_user, 2) + it 'grants agent creation' do + expect { create(:agent_user) } + .to change { current_agents.count }.by(1) + end - expect { create(:agent_user) } - .to raise_error(Exceptions::UnprocessableEntity) - .and change { current_agent_count }.by(0) + it 'grants role change' do + future_agent = create(:customer_user) + + expect { future_agent.roles = [agent_role] } + .to change { current_agents.count }.by(1) + end + + describe 'role updates' do + let(:agent) { create(:agent_user) } + + it 'grants update by instances' do + expect { agent.roles = [admin_role, agent_role] } + .not_to raise_error + end + + it 'grants update by id (Integer)' do + expect { agent.role_ids = [admin_role.id, agent_role.id] } + .not_to raise_error + end + + it 'grants update by id (String)' do + expect { agent.role_ids = [admin_role.id.to_s, agent_role.id.to_s] } + .not_to raise_error + end + end end - it 'prevents role change' do - Setting.set('system_agent_limit', current_agent_count) + context 'when exceeding the agent limit' do + it 'creation of new agents' do + Setting.set('system_agent_limit', (current_agents.count + 2).to_s) - future_agent = create(:customer_user) + create_list(:agent_user, 2) - expect { future_agent.roles = [agent_role] } - .to raise_error(Exceptions::UnprocessableEntity) - .and change { current_agent_count }.by(0) + expect { create(:agent_user) } + .to raise_error(Exceptions::UnprocessableEntity) + .and change { current_agents.count }.by(0) + end + + it 'prevents role change' do + 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_agents.count }.by(0) + end 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) } + describe '#validate_agent_limit_by_attributes' do + context 'for Integer value of system_agent_limit' do + before { Setting.set('system_agent_limit', current_agents.count) } - it 'grants agent creation' do - expect { create(:agent_user) } - .to change { current_agent_count }.by(1) - end + context 'when exceeding the agent limit' do + it 'prevents re-activation of agents' do + inactive_agent = create(:agent_user, active: false) - it 'grants role change' do - future_agent = create(:customer_user) - - expect { future_agent.roles = [agent_role] } - .to change { current_agent_count }.by(1) - end - - describe 'role updates' do - let(:agent) { create(:agent_user) } - - it 'grants update by instances' do - expect { agent.roles = [admin_role, agent_role] } - .not_to raise_error - end - - it 'grants update by id (Integer)' do - expect { agent.role_ids = [admin_role.id, agent_role.id] } - .not_to raise_error - end - - it 'grants update by id (String)' do - expect { agent.role_ids = [admin_role.id.to_s, agent_role.id.to_s] } - .not_to raise_error + expect { inactive_agent.update!(active: true) } + .to raise_error(Exceptions::UnprocessableEntity) + .and change { current_agents.count }.by(0) end end end - context 'when exceeding the agent limit' do - it 'creation of new agents' do - Setting.set('system_agent_limit', (current_agent_count + 2).to_s) + context 'for String value of system_agent_limit' do + before { Setting.set('system_agent_limit', current_agents.count.to_s) } - create_list(:agent_user, 2) + context 'when exceeding the agent limit' do + it 'prevents re-activation of agents' do + inactive_agent = create(:agent_user, active: false) - expect { create(:agent_user) } - .to raise_error(Exceptions::UnprocessableEntity) - .and change { current_agent_count }.by(0) - end - - it 'prevents role change' do - Setting.set('system_agent_limit', current_agent_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) - end - end - end - end - - 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) } - - context 'when exceeding the agent limit' do - it 'prevents re-activation of agents' do - inactive_agent = create(:agent_user, active: false) - - expect { inactive_agent.update!(active: true) } - .to raise_error(Exceptions::UnprocessableEntity) - .and change { current_agent_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) } - - context 'when exceeding the agent limit' do - it 'prevents re-activation of agents' do - inactive_agent = create(:agent_user, active: false) - - expect { inactive_agent.update!(active: true) } - .to raise_error(Exceptions::UnprocessableEntity) - .and change { current_agent_count }.by(0) + expect { inactive_agent.update!(active: true) } + .to raise_error(Exceptions::UnprocessableEntity) + .and change { current_agents.count }.by(0) + end end end end