From 17ce2dcbde6b256c91f668259ecb98a879ee16a8 Mon Sep 17 00:00:00 2001
From: Martin Edenhofer <me@edenhofer.de>
Date: Fri, 17 Nov 2017 15:39:56 +0100
Subject: [PATCH] Improved agent limit detection (do not complain if role with
 ticket.agent permission is assigned and agent already owns a role with
 ticket.agent permission).

---
 app/models/user.rb                          | 16 +++++-
 test/unit/role_validate_agent_limit_test.rb | 55 ++++++++++++++++++++-
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 6f09f6f10..95d280ec0 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1036,8 +1036,22 @@ raise 'Minimum one user need to have admin permissions'
     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
+
+    # if new added role is a ticket.agent role
     if ticket_agent_role_ids.include?(role.id)
-      count += 1
+
+      # if user already has a ticket.agent role
+      hint = false
+      role_ids.each do |locale_role_id|
+        next if !ticket_agent_role_ids.include?(locale_role_id)
+        hint = true
+        break
+      end
+
+      # user has not already a ticket.agent role
+      if hint == false
+        count += 1
+      end
     end
     raise Exceptions::UnprocessableEntity, 'Agent limit exceeded, please check your account settings.' if count > Setting.get('system_agent_limit')
     true
diff --git a/test/unit/role_validate_agent_limit_test.rb b/test/unit/role_validate_agent_limit_test.rb
index 5395d0c5e..b10d8b523 100644
--- a/test/unit/role_validate_agent_limit_test.rb
+++ b/test/unit/role_validate_agent_limit_test.rb
@@ -2,7 +2,7 @@
 require 'test_helper'
 
 class RoleValidateAgentLimit < ActiveSupport::TestCase
-  test 'role_validate_agent_limit' do
+  test 'role validate agent limit' do
 
     agent_max = User.with_permissions('ticket.agent').count
     UserInfo.current_user_id = 1
@@ -70,4 +70,57 @@ class RoleValidateAgentLimit < ActiveSupport::TestCase
     role_agent_limit_fail.destroy!
     Setting.set('system_agent_limit', nil)
   end
+
+  test 'role validate agent limit - 1 user 2 ticket.agent roles' do
+
+    agent_max = User.with_permissions('ticket.agent').count
+    UserInfo.current_user_id = 1
+    Setting.set('system_agent_limit', agent_max + 1)
+
+    permission_ticket_agent = Permission.find_by(name: 'ticket.agent')
+
+    role_agent_limit1 = Role.create!(
+      name: 'agent-limit-test1',
+      note: 'agent-limit-test1 Role.',
+      permissions: [permission_ticket_agent],
+      active: true,
+    )
+    role_agent_limit2 = Role.create!(
+      name: 'agent-limit-test2',
+      note: 'agent-limit-test2 Role.',
+      permissions: [permission_ticket_agent],
+      active: true,
+    )
+
+    user1 = User.create!(
+      firstname: 'Firstname',
+      lastname:  'Lastname',
+      email:     'some-agentlimit-role@example.com',
+      login:     'some-agentlimit-role@example.com',
+      roles:     [role_agent_limit1, role_agent_limit2],
+      active:    true,
+    )
+
+    user1.roles = Role.where(name: %w(Admin Agent))
+
+    user1.role_ids = [Role.find_by(name: 'Agent').id]
+
+    user1.role_ids = [Role.find_by(name: 'Agent').id, role_agent_limit1.id]
+
+    assert_raises(Exceptions::UnprocessableEntity) do
+      user2 = User.create!(
+        firstname: 'Firstname2',
+        lastname:  'Lastname2',
+        email:     'some-agentlimit-role-2@example.com',
+        login:     'some-agentlimit-role-2@example.com',
+        roles:     [role_agent_limit1],
+        active:    true,
+      )
+    end
+
+    role_agent_limit1.destroy!
+    role_agent_limit2.destroy!
+    Setting.set('system_agent_limit', nil)
+  end
+
 end