FIX: Don't try to rename group when username is taken

FIX: Always rename groups with the default locale instead of using the user's locale

app/models/group.rb

@@ -279,10 +279,10 @@ class Group < ActiveRecord::Base
     end
 
     # don't allow shoddy localization to break this
-    localized_name = I18n.t("groups.default_names.#{name}").downcase
+    localized_name = I18n.t("groups.default_names.#{name}", locale: SiteSetting.default_locale).downcase
     validator = UsernameValidator.new(localized_name)
 
-    if !Group.where("LOWER(name) = ?", localized_name).exists? && validator.valid_format?
+    if validator.valid_format? && !User.username_exists?(localized_name)
       group.name = localized_name
     end
 
@@ -626,15 +626,8 @@ class Group < ActiveRecord::Base
     UsernameValidator.perform_validation(self, 'name') || begin
       name_lower = self.name.downcase
 
-      if self.will_save_change_to_name? && self.name_was&.downcase != name_lower
-
-        existing = DB.exec(
-          User::USERNAME_EXISTS_SQL, username: name_lower
-        ) > 0
-
-        if existing
-          errors.add(:name, I18n.t("activerecord.errors.messages.taken"))
-        end
+      if self.will_save_change_to_name? && self.name_was&.downcase != name_lower && User.username_exists?(name_lower)
+        errors.add(:name, I18n.t("activerecord.errors.messages.taken"))
       end
     end
   end

app/models/user.rb

@@ -220,7 +220,7 @@ class User < ActiveRecord::Base
   def self.username_available?(username, email = nil, allow_reserved_username: false)
     lower = username.downcase
     return false if !allow_reserved_username && reserved_username?(lower)
-    return true  if DB.exec(User::USERNAME_EXISTS_SQL, username: lower) == 0
+    return true  if !username_exists?(lower)
 
     # staged users can use the same username since they will take over the account
     email.present? && User.joins(:user_emails).exists?(staged: true, username_lower: lower, user_emails: { primary: true, email: email })
@@ -1271,6 +1271,10 @@ class User < ActiveRecord::Base
   WHERE lower(groups.name) = :username)
   SQL
 
+  def self.username_exists?(username_lower)
+    DB.exec(User::USERNAME_EXISTS_SQL, username: username_lower) > 0
+  end
+
   def username_validator
     username_format_validator || begin
       lower = username.downcase

spec/models/group_spec.rb

@@ -236,7 +236,6 @@ describe Group do
 
     it "does not reset the localized name" do
       begin
-        default_locale = SiteSetting.default_locale
         I18n.locale = SiteSetting.default_locale = 'fi'
 
         group = Group.find(Group::AUTO_GROUPS[:everyone])
@@ -251,41 +250,43 @@ describe Group do
         Group.refresh_automatic_group!(:everyone)
 
         expect(group.reload.name).to eq(I18n.t("groups.default_names.everyone"))
-      ensure
-        I18n.locale = SiteSetting.default_locale = default_locale
       end
     end
 
     it "uses the localized name if name has not been taken" do
       begin
-        default_locale = SiteSetting.default_locale
         I18n.locale = SiteSetting.default_locale = 'de'
 
         group = Group.refresh_automatic_group!(:staff)
 
         expect(group.name).to_not eq('staff')
         expect(group.name).to eq(I18n.t('groups.default_names.staff'))
-      ensure
-        I18n.locale = SiteSetting.default_locale = default_locale
       end
     end
 
     it "does not use the localized name if name has already been taken" do
       begin
-        default_locale = SiteSetting.default_locale
         I18n.locale = SiteSetting.default_locale = 'de'
 
-        _another_group = Fabricate(:group,
-          name: I18n.t('groups.default_names.staff').upcase
-        )
-
+        Fabricate(:group, name: I18n.t('groups.default_names.staff').upcase)
         group = Group.refresh_automatic_group!(:staff)
-
         expect(group.name).to eq('staff')
-      ensure
-        I18n.locale = SiteSetting.default_locale = default_locale
+
+        Fabricate(:user_single_email, username: I18n.t('groups.default_names.moderators').upcase)
+        group = Group.refresh_automatic_group!(:moderators)
+        expect(group.name).to eq('moderators')
       end
     end
+
+    it "always uses the default locale" do
+      SiteSetting.default_locale = "de"
+      I18n.locale = "en"
+
+      group = Group.refresh_automatic_group!(:staff)
+
+      expect(group.name).to_not eq('staff')
+      expect(group.name).to eq(I18n.t('groups.default_names.staff', locale: "de"))
+    end
   end
 
   it "Correctly handles removal of primary group" do