Revert "FIX: Username uniqueness check should not happen to current user_id"

This reverts commit f71a18facd.

app/models/group.rb

@@ -42,7 +42,7 @@ class Group < ActiveRecord::Base
     ApplicationSerializer.expire_cache_fragment!("group_names")
   end
 
-  validate :name_format_validator, if: :will_save_change_to_name?
+  validate :name_format_validator
   validates :name, presence: true
   validate :automatic_membership_email_domains_format_validator
   validate :incoming_email_validator
@@ -599,7 +599,17 @@ class Group < ActiveRecord::Base
       self.name.strip!
       self.name.downcase!
 
-      UsernameValidator.perform_validation(self, 'name')
+      UsernameValidator.perform_validation(self, 'name') || begin
+        if will_save_change_to_name?
+          existing = Group.exec_sql(
+            User::USERNAME_EXISTS_SQL, username: self.name
+          ).values.present?
+
+          if existing
+            errors.add(:name, I18n.t("activerecord.errors.messages.taken"))
+          end
+        end
+      end
     end
 
     def automatic_membership_email_domains_format_validator

app/models/user.rb

@@ -770,8 +770,8 @@ class User < ActiveRecord::Base
     Guardian.new(self)
   end
 
-  def username_validator
-    UsernameValidator.perform_validation(self, 'username', self.id)
+  def username_format_validator
+    UsernameValidator.perform_validation(self, 'username')
   end
 
   def email_confirmed?
@@ -1121,6 +1121,32 @@ class User < ActiveRecord::Base
     self.username_lower = username.downcase
   end
 
+  USERNAME_EXISTS_SQL = <<~SQL
+  (SELECT 1 FROM users
+  WHERE users.username_lower = :username)
+
+  UNION ALL
+
+  (SELECT 1 FROM groups
+  WHERE lower(groups.name) = :username)
+  SQL
+
+  def username_validator
+    username_format_validator || begin
+      if will_save_change_to_username?
+        lower = username.downcase
+
+        existing = User.exec_sql(
+          USERNAME_EXISTS_SQL, username: lower
+        ).values.present?
+
+        if existing
+          errors.add(:username, I18n.t(:'user.username.unique'))
+        end
+      end
+    end
+  end
+
   def send_approval_email
     if SiteSetting.must_approve_users
       Jobs.enqueue(:critical_user_email,

app/models/username_validator.rb

@@ -6,23 +6,21 @@ class UsernameValidator
   #
   # object - Object in which we're performing the validation
   # field_name - name of the field that we're validating
-  # user_id - skip id while validating username uniqueness
   #
   # Example: UsernameValidator.perform_validation(user, 'name')
-  def self.perform_validation(object, field_name, user_id = nil)
-    validator = UsernameValidator.new(object.send(field_name), user_id)
+  def self.perform_validation(object, field_name)
+    validator = UsernameValidator.new(object.send(field_name))
     unless validator.valid_format?
       validator.errors.each { |e| object.errors.add(field_name.to_sym, e) }
     end
   end
 
-  def initialize(username, user_id = nil)
+  def initialize(username)
     @username = username
-    @user_id = user_id
     @errors = []
   end
   attr_accessor :errors
-  attr_reader :username, :user_id
+  attr_reader :username
 
   def user
     @user ||= User.new(user)
@@ -37,7 +35,6 @@ class UsernameValidator
     username_last_char_valid?
     username_no_double_special?
     username_does_not_end_with_confusing_suffix?
-    username_unique?
     errors.empty?
   end
 
@@ -100,21 +97,4 @@ class UsernameValidator
       self.errors << I18n.t(:'user.username.must_not_end_with_confusing_suffix')
     end
   end
-
-  def username_unique?
-    lower = username.downcase
-    sql = <<~SQL
-    (SELECT 1 FROM users
-    WHERE users.username_lower = :username#{" AND users.id != #{user_id}" if user_id.present?})
-
-    UNION ALL
-
-    (SELECT 1 FROM groups
-    WHERE lower(groups.name) = :username)
-    SQL
-
-    if User.exec_sql(sql, username: lower).values.present?
-      self.errors << user_id.present? ? I18n.t(:'user.username.unique') : I18n.t("activerecord.errors.messages.taken")
-    end
-  end
 end