diff --git a/app/models/group.rb b/app/models/group.rb index 9ed02989bb2..0f2959048a3 100644 --- a/app/models/group.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index 0de21d1e60c..6c9e3be075d 100644 --- a/app/models/user.rb +++ b/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 diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 37ef6736430..8419e89976c 100644 --- a/spec/models/group_spec.rb +++ b/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