Improved error handling of system_agent_limit setting it the number was supplied as string.

app/models/role.rb

@@ -186,7 +186,7 @@ returns
   end
 
   def validate_agent_limit_by_attributes
-    return true if !Setting.get('system_agent_limit')
+    return true if Setting.get('system_agent_limit').blank?
     return true if !will_save_change_to_attribute?('active')
     return true if active != true
     return true if !with_permission?('ticket.agent')
@@ -194,19 +194,19 @@ returns
     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')
+    raise Exceptions::UnprocessableEntity, 'Agent limit exceeded, please check your account settings.' if count > Setting.get('system_agent_limit').to_i
     true
   end
 
   def validate_agent_limit_by_permission(permission)
-    return true if !Setting.get('system_agent_limit')
+    return true if Setting.get('system_agent_limit').blank?
     return true if active != true
     return true if permission.active != true
     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 }).distinct().count
-    raise Exceptions::UnprocessableEntity, 'Agent limit exceeded, please check your account settings.' if count > Setting.get('system_agent_limit')
+    raise Exceptions::UnprocessableEntity, 'Agent limit exceeded, please check your account settings.' if count > Setting.get('system_agent_limit').to_i
     true
   end
 

app/models/user.rb

@@ -1066,18 +1066,18 @@ raise 'Minimum one user need to have admin permissions'
   end
 
   def validate_agent_limit_by_attributes
-    return true if !Setting.get('system_agent_limit')
+    return true if Setting.get('system_agent_limit').blank?
     return true if !will_save_change_to_attribute?('active')
     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 }).distinct().count + 1
-    raise Exceptions::UnprocessableEntity, 'Agent limit exceeded, please check your account settings.' if count > Setting.get('system_agent_limit')
+    raise Exceptions::UnprocessableEntity, 'Agent limit exceeded, please check your account settings.' if count > Setting.get('system_agent_limit').to_i
     true
   end
 
   def validate_agent_limit_by_role(role)
-    return true if !Setting.get('system_agent_limit')
+    return true if Setting.get('system_agent_limit').blank?
     return true if active != true
     return true if role.active != true
     return true if !role.with_permission?('ticket.agent')
@@ -1100,7 +1100,7 @@ raise 'Minimum one user need to have admin permissions'
         count += 1
       end
     end
-    raise Exceptions::UnprocessableEntity, 'Agent limit exceeded, please check your account settings.' if count > Setting.get('system_agent_limit')
+    raise Exceptions::UnprocessableEntity, 'Agent limit exceeded, please check your account settings.' if count > Setting.get('system_agent_limit').to_i
     true
   end
 

spec/models/user_spec.rb

@@ -665,6 +665,131 @@ RSpec.describe User do
         end
       end
     end
+
+    context '#validate_agent_limit_by_role by string' do
+
+      context 'agent creation limit not reached' do
+
+        it 'grants agent creation' do
+          Setting.set('system_agent_limit', (current_agent_count + 1).to_s)
+
+          expect do
+            create(:agent_user)
+          end.to change {
+            current_agent_count
+          }.by(1)
+        end
+
+        it 'grants role change' do
+          Setting.set('system_agent_limit', (current_agent_count + 1).to_s)
+
+          future_agent = create(:customer_user)
+
+          expect do
+            future_agent.roles = [agent_role]
+          end.to change {
+            current_agent_count
+          }.by(1)
+        end
+
+        context 'role updates' do
+
+          it 'grants update by instances' do
+            Setting.set('system_agent_limit', (current_agent_count + 1).to_s)
+
+            agent = create(:agent_user)
+
+            expect do
+              agent.roles = [
+                admin_role,
+                agent_role
+              ]
+              agent.save!
+            end.not_to raise_error
+          end
+
+          it 'grants update by id (Integer)' do
+            Setting.set('system_agent_limit', (current_agent_count + 1).to_s)
+
+            agent = create(:agent_user)
+
+            expect do
+              agent.role_ids = [
+                admin_role.id,
+                agent_role.id
+              ]
+              agent.save!
+            end.not_to raise_error
+          end
+
+          it 'grants update by id (String)' do
+            Setting.set('system_agent_limit', (current_agent_count + 1).to_s)
+
+            agent = create(:agent_user)
+
+            expect do
+              agent.role_ids = [
+                admin_role.id.to_s,
+                agent_role.id.to_s
+              ]
+              agent.save!
+            end.not_to raise_error
+          end
+        end
+      end
+
+      context 'agent creation limit surpassing prevention' do
+
+        it 'creation of new agents' do
+          Setting.set('system_agent_limit', (current_agent_count + 2).to_s)
+
+          create_list(:agent_user, 2)
+
+          initial_agent_count = current_agent_count
+
+          expect do
+            create(:agent_user)
+          end.to raise_error(Exceptions::UnprocessableEntity)
+
+          expect(current_agent_count).to eq(initial_agent_count)
+        end
+
+        it 'prevents role change' do
+          Setting.set('system_agent_limit', current_agent_count.to_s)
+
+          future_agent = create(:customer_user)
+
+          initial_agent_count = current_agent_count
+
+          expect do
+            future_agent.roles = [agent_role]
+          end.to raise_error(Exceptions::UnprocessableEntity)
+
+          expect(current_agent_count).to eq(initial_agent_count)
+        end
+      end
+    end
+
+    context '#validate_agent_limit_by_attributes' do
+
+      context 'agent creation limit surpassing prevention' do
+
+        it 'prevents re-activation of agents' do
+          Setting.set('system_agent_limit', current_agent_count.to_s)
+
+          inactive_agent = create(:agent_user, active: false)
+
+          initial_agent_count = current_agent_count
+
+          expect do
+            inactive_agent.update!(active: true)
+          end.to raise_error(Exceptions::UnprocessableEntity)
+
+          expect(current_agent_count).to eq(initial_agent_count)
+        end
+      end
+    end
+
   end
 
   context 'when phone attribute' do

test/unit/role_validate_agent_limit_test.rb

@@ -71,6 +71,75 @@ class RoleValidateAgentLimit < ActiveSupport::TestCase
     Setting.set('system_agent_limit', nil)
   end
 
+  test 'role validate agent limit as string' do
+
+    agent_max = User.with_permissions('ticket.agent').count
+    UserInfo.current_user_id = 1
+    Setting.set('system_agent_limit', (agent_max + 2).to_s)
+
+    permission_ticket_agent = Permission.where(name: 'ticket.agent')
+
+    role_agent_limit_success = Role.create!(
+      name: 'agent-limit-test-success',
+      note: 'agent-limit-test-success Role.',
+      permissions: [],
+      active: true,
+    )
+    role_agent_limit_fail = Role.create!(
+      name: 'agent-limit-test-fail',
+      note: 'agent-limit-test-fail Role.',
+      permissions: [],
+      active: true,
+    )
+
+    user1 = User.create!(
+      firstname: 'Firstname',
+      lastname:  'Lastname',
+      email:     'some-agentlimit-role@example.com',
+      login:     'some-agentlimit-role@example.com',
+      roles:     [role_agent_limit_success],
+      active:    true,
+    )
+    user2 = User.create!(
+      firstname: 'Firstname1',
+      lastname:  'Lastname1',
+      email:     'some-agentlimit-role-1@example.com',
+      login:     'some-agentlimit-role-1@example.com',
+      roles:     [role_agent_limit_success],
+      active:    true,
+    )
+    user3 = User.create!(
+      firstname: 'Firstname2',
+      lastname:  'Lastname2',
+      email:     'some-agentlimit-role-2@example.com',
+      login:     'some-agentlimit-role-2@example.com',
+      roles:     [role_agent_limit_fail],
+      active:    true,
+    )
+
+    role_agent_limit_success.permissions = permission_ticket_agent
+    assert_raises(Exceptions::UnprocessableEntity) do
+      role_agent_limit_fail.permissions = permission_ticket_agent
+    end
+
+    role_agent_limit_fail.active = false
+    role_agent_limit_fail.save!
+
+    role_agent_limit_fail.permissions = permission_ticket_agent
+
+    assert_raises(Exceptions::UnprocessableEntity) do
+      role_agent_limit_fail.active = true
+      role_agent_limit_fail.save!
+    end
+
+    user1.destroy!
+    user2.destroy!
+    user3.destroy!
+    role_agent_limit_success.destroy!
+    role_agent_limit_fail.destroy!
+    Setting.set('system_agent_limit', nil)
+  end
+
   test 'role validate agent limit - 1 user 2 ticket.agent roles' do
 
     current_agent_max = User.with_permissions('ticket.agent').count + 1