From 59dfb51a35ce37c91d34e9c1817c6e536735f1bb Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 18 Jan 2017 12:20:23 +0800 Subject: [PATCH] FIX: Don't change automatic group name if localized name has been taken. --- app/models/group.rb | 8 +++++++- spec/models/group_spec.rb | 39 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index b97c34356c5..fa6cb204d1f 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -161,7 +161,13 @@ class Group < ActiveRecord::Base # don't allow shoddy localization to break this localized_name = I18n.t("groups.default_names.#{name}") validator = UsernameValidator.new(localized_name) - group.name = validator.valid_format? ? localized_name : name + + group.name = + if !Group.where(name: localized_name).exists? && validator.valid_format? + localized_name + else + name + end # the everyone group is special, it can include non-users so there is no # way to have the membership in a table diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 8a6477ed731..86f79597c33 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -154,9 +154,42 @@ describe Group do end - it "makes sure the everyone group is not visible" do - g = Group.refresh_automatic_group!(:everyone) - expect(g.visible).to eq(false) + describe '.refresh_automatic_group!' do + it "makes sure the everyone group is not visible" do + g = Group.refresh_automatic_group!(:everyone) + expect(g.visible).to eq(false) + 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') + ) + + group = Group.refresh_automatic_group!(:staff) + + expect(group.name).to eq('staff') + ensure + I18n.locale = SiteSetting.default_locale = default_locale + end + end end it "Correctly handles removal of primary group" do