Merge pull request #4662 from tgxworld/fix_localized_group_name_change

Fix localized group name change

app/jobs/regular/update_group_mentions.rb

@@ -0,0 +1,14 @@
+module Jobs
+
+  class UpdateGroupMentions < Jobs::Base
+
+    def execute(args)
+      group = Group.find_by(id: args[:group_id])
+      return unless group
+
+      previous_group_name = args[:previous_name]
+
+      GroupMentionsUpdater.update(group.name, previous_group_name)
+    end
+  end
+end

app/models/group.rb

@@ -22,6 +22,9 @@ class Group < ActiveRecord::Base
   after_save :update_primary_group
   after_save :update_title
 
+  after_save :enqueue_update_mentions_job,
+    if: Proc.new { |g| g.name_was && g.name_changed? }
+
   after_save :expire_cache
   after_destroy :expire_cache
 
@@ -161,7 +164,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
@@ -487,6 +496,15 @@ SQL
         builder.exec
       end
     end
+
+  private
+
+    def enqueue_update_mentions_job
+      Jobs.enqueue(:update_group_mentions,
+        previous_name: self.name_was,
+        group_id: self.id
+      )
+    end
 end
 
 # == Schema Information

app/services/group_mentions_updater.rb

@@ -0,0 +1,14 @@
+class GroupMentionsUpdater
+  def self.update(current_name, previous_name)
+    Post.where(
+      "cooked LIKE '%class=\"mention-group\"%' AND raw LIKE :previous_name",
+      previous_name: "%@#{previous_name}%"
+    ).find_in_batches do |posts|
+
+      posts.each do |post|
+        post.raw.gsub!(/(^|\s)(@#{previous_name})(\s|$)/, "\\1@#{current_name}\\3")
+        post.save!
+      end
+    end
+  end
+end

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

spec/services/group_mentions_updater_spec.rb

@@ -0,0 +1,39 @@
+require 'rails_helper'
+
+RSpec.describe GroupMentionsUpdater do
+  let(:post) { Fabricate(:post) }
+
+  describe '.update' do
+    it 'should update valid group mentions' do
+      new_group_name = 'awesome_team'
+      old_group_name = 'team'
+
+      [
+        ["@#{old_group_name} is awesome!", "@#{new_group_name} is awesome!"],
+        ["This @#{old_group_name} is awesome!", "This @#{new_group_name} is awesome!"],
+        ["Mention us @ @#{old_group_name}", "Mention us @ @#{new_group_name}"],
+      ].each do |raw, expected_raw|
+        group = Fabricate(:group, name: old_group_name)
+        post.update!(raw: raw)
+        group.update!(name: new_group_name)
+        post.reload
+
+        expect(post.raw_mentions).to eq([new_group_name])
+        expect(post.raw).to eq(expected_raw)
+
+        group.destroy!
+      end
+    end
+
+    it 'should not update invalid group mentions' do
+      group = Fabricate(:group, name: 'team')
+      post.update!(raw: 'This is not valid@team.com')
+
+      expect(post.reload.raw_mentions).to eq([])
+
+      group.update!(name: 'new_team_name')
+
+      expect(post.reload.raw_mentions).to eq([])
+    end
+  end
+end