From 72f39d0d9db5c7bd3fce079c543ee1e0844f2ad1 Mon Sep 17 00:00:00 2001
From: Rolf Schmidt <rolf.schmidt@zammad.com>
Date: Wed, 22 Sep 2021 14:39:10 +0200
Subject: [PATCH] Maintenance: Remove while loop user check login.

---
 app/models/user.rb       | 17 ++++++++---------
 spec/models/user_spec.rb | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index a93432e4e..2f01b9c57 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -926,17 +926,16 @@ try to find correct name
     end
 
     # check if login already exists
-    self.login = login.downcase.strip
-    check      = true
-    while check
+    base_login = login.downcase.strip
+
+    alternatives = [nil] + Array(1..20) + [ SecureRandom.uuid ]
+    alternatives.each do |suffix|
+      self.login = "#{base_login}#{suffix}"
       exists = User.find_by(login: login)
-      if exists && exists.id != id
-        self.login = "#{login}#{rand(999)}" # rubocop:disable Zammad/ForbidRand
-      else
-        check = false
-      end
+      return true if !exists || exists.id == id
     end
-    true
+
+    raise Exceptions::UnprocessableEntity, "Invalid user login generation for login #{login}!"
   end
 
   def check_mail_delivery_failed
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 06fbe43c9..6bbc6eca7 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -573,6 +573,38 @@ RSpec.describe User, type: :model do
         end
       end
     end
+
+    describe '#check_login' do
+      let(:agent) { create(:agent) }
+
+      it 'does use the origin login' do
+        new_agent = create(:agent)
+        expect(new_agent.login).not_to end_with('1')
+      end
+
+      it 'does number up agent logins (1)' do
+        new_agent = create(:agent, login: agent.login)
+        expect(new_agent.login).to eq("#{agent.login}1")
+      end
+
+      it 'does number up agent logins (5)' do
+        new_agent = create(:agent, login: agent.login)
+        4.times do
+          new_agent = create(:agent, login: agent.login)
+        end
+
+        expect(new_agent.login).to eq("#{agent.login}5")
+      end
+
+      it 'does backup with uuid in cases of many duplicates' do
+        new_agent = create(:agent, login: agent.login)
+        20.times do
+          new_agent = create(:agent, login: agent.login)
+        end
+
+        expect(new_agent.login.sub!(agent.login, '')).to be_a_uuid
+      end
+    end
   end
 
   describe 'Attributes:' do