From 97a5cd3cf5a81dd9651952613e1bb477dd98e1f6 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Wed, 13 Dec 2017 12:17:57 +0100 Subject: [PATCH] Improved max. agent limit check. --- app/models/role.rb | 8 ++++---- app/models/user.rb | 6 +++--- test/unit/role_validate_agent_limit_test.rb | 20 ++++++++++++++++++-- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/app/models/role.rb b/app/models/role.rb index 5130be108..130c691d4 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -185,7 +185,7 @@ returns def last_admin_check_admin_count admin_role_ids = Role.joins(:permissions).where(permissions: { name: ['admin', 'admin.user'], active: true }, roles: { active: true }).where.not(id: id).pluck(:id) - User.joins(:roles).where(roles: { id: admin_role_ids }, users: { active: true }).count + User.joins(:roles).where(roles: { id: admin_role_ids }, users: { active: true }).distinct().count end def validate_agent_limit_by_attributes @@ -194,8 +194,8 @@ returns return true if active != true return true if !with_permission?('ticket.agent') ticket_agent_role_ids = Role.joins(:permissions).where(permissions: { name: 'ticket.agent', active: true }, roles: { active: true }).pluck(:id) - currents = User.joins(:roles).where(roles: { id: ticket_agent_role_ids }, users: { active: true }).pluck(:id) - news = User.joins(:roles).where(roles: { id: id }, users: { active: true }).pluck(:id) + currents = User.joins(:roles).where(roles: { id: ticket_agent_role_ids }, users: { active: true }).distinct().pluck(:id) + news = User.joins(:roles).where(roles: { id: id }, users: { active: true }).distinct().pluck(:id) count = currents.concat(news).uniq.count raise Exceptions::UnprocessableEntity, 'Agent limit exceeded, please check your account settings.' if count > Setting.get('system_agent_limit') true @@ -208,7 +208,7 @@ returns return true if permission.name != 'ticket.agent' ticket_agent_role_ids = Role.joins(:permissions).where(permissions: { name: 'ticket.agent' }, roles: { active: true }).pluck(:id) ticket_agent_role_ids.push(id) - count = User.joins(:roles).where(roles: { id: ticket_agent_role_ids }, users: { active: true }).count + count = User.joins(:roles).where(roles: { id: ticket_agent_role_ids }, users: { active: true }).distinct().count raise Exceptions::UnprocessableEntity, 'Agent limit exceeded, please check your account settings.' if count > Setting.get('system_agent_limit') true end diff --git a/app/models/user.rb b/app/models/user.rb index ac6dac3cf..42c6bbc31 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1018,7 +1018,7 @@ raise 'Minimum one user need to have admin permissions' def last_admin_check_admin_count admin_role_ids = Role.joins(:permissions).where(permissions: { name: ['admin', 'admin.user'], active: true }, roles: { active: true }).pluck(:id) - User.joins(:roles).where(roles: { id: admin_role_ids }, users: { active: true }).count - 1 + User.joins(:roles).where(roles: { id: admin_role_ids }, users: { active: true }).distinct().count - 1 end def validate_agent_limit_by_attributes @@ -1027,7 +1027,7 @@ raise 'Minimum one user need to have admin permissions' return true if active != true return true if !permissions?('ticket.agent') ticket_agent_role_ids = Role.joins(:permissions).where(permissions: { name: 'ticket.agent', active: true }, roles: { active: true }).pluck(:id) - count = User.joins(:roles).where(roles: { id: ticket_agent_role_ids }, users: { active: true }).count + 1 + count = User.joins(:roles).where(roles: { id: ticket_agent_role_ids }, users: { active: true }).distinct().count + 1 raise Exceptions::UnprocessableEntity, 'Agent limit exceeded, please check your account settings.' if count > Setting.get('system_agent_limit') true end @@ -1038,7 +1038,7 @@ raise 'Minimum one user need to have admin permissions' return true if role.active != true return true if !role.with_permission?('ticket.agent') ticket_agent_role_ids = Role.joins(:permissions).where(permissions: { name: 'ticket.agent', active: true }, roles: { active: true }).pluck(:id) - count = User.joins(:roles).where(roles: { id: ticket_agent_role_ids }, users: { active: true }).count + count = User.joins(:roles).where(roles: { id: ticket_agent_role_ids }, users: { active: true }).distinct().count # if new added role is a ticket.agent role if ticket_agent_role_ids.include?(role.id) diff --git a/test/unit/role_validate_agent_limit_test.rb b/test/unit/role_validate_agent_limit_test.rb index ded98ea6a..4292e596b 100644 --- a/test/unit/role_validate_agent_limit_test.rb +++ b/test/unit/role_validate_agent_limit_test.rb @@ -73,9 +73,9 @@ class RoleValidateAgentLimit < ActiveSupport::TestCase test 'role validate agent limit - 1 user 2 ticket.agent roles' do - agent_max = User.with_permissions('ticket.agent').count + current_agent_max = User.with_permissions('ticket.agent').count + 1 UserInfo.current_user_id = 1 - Setting.set('system_agent_limit', agent_max + 1) + Setting.set('system_agent_limit', current_agent_max) permission_ticket_agent = Permission.find_by(name: 'ticket.agent') @@ -107,6 +107,8 @@ class RoleValidateAgentLimit < ActiveSupport::TestCase user1.role_ids = [Role.find_by(name: 'Agent').id, role_agent_limit1.id] + user1.role_ids = [Role.find_by(name: 'Agent').id, role_agent_limit1.id, role_agent_limit2.id] + assert_raises(Exceptions::UnprocessableEntity) do user2 = User.create!( firstname: 'Firstname2', @@ -118,6 +120,20 @@ class RoleValidateAgentLimit < ActiveSupport::TestCase ) end + assert_equal(current_agent_max, User.with_permissions('ticket.agent').count) + + current_agent_max = User.with_permissions('ticket.agent').count + 1 + Setting.set('system_agent_limit', current_agent_max) + + user3 = User.create!( + firstname: 'Firstname', + lastname: 'Lastname', + email: 'some-agentlimit-role-3@example.com', + login: 'some-agentlimit-role-3@example.com', + role_ids: [Role.find_by(name: 'Agent').id], + active: true, + ) + role_agent_limit1.destroy! role_agent_limit2.destroy! Setting.set('system_agent_limit', nil)