FIX: Reverting multiple translation overrides didn't clear cache

app/models/translation_override.rb

@@ -28,38 +28,48 @@ class TranslationOverride < ActiveRecord::Base
 
     translation_override = find_or_initialize_by(params)
     params.merge!(data) if translation_override.new_record?
-    i18n_changed([key]) if translation_override.update(data)
+    i18n_changed(locale, [key]) if translation_override.update(data)
     translation_override
   end
 
-  def self.revert!(locale, *keys)
+  def self.revert!(locale, keys)
+    keys = Array.wrap(keys)
     TranslationOverride.where(locale: locale, translation_key: keys).delete_all
-    i18n_changed(keys)
+    i18n_changed(locale, keys)
   end
 
-  def self.i18n_changed(keys)
+  def self.reload_locale!
     I18n.reload!
     ExtraLocalesController.clear_cache!
     MessageBus.publish('/i18n-flush', refresh: true)
-
-    keys.flatten.each do |key|
-      return if expire_cache(key)
-    end
   end
 
-  def self.expire_cache(key)
+  def self.clear_cached_keys!(locale, keys)
+    should_clear_anon_cache = false
+    keys.each do |key|
+      should_clear_anon_cache |= expire_cache(locale, key)
+    end
+    Site.clear_anon_cache! if should_clear_anon_cache
+  end
+
+  def self.i18n_changed(locale, keys)
+    reload_locale!
+    clear_cached_keys!(locale, keys)
+  end
+
+  def self.expire_cache(locale, key)
     if key.starts_with?('post_action_types.')
-      ApplicationSerializer.expire_cache_fragment!("post_action_types_#{I18n.locale}")
+      ApplicationSerializer.expire_cache_fragment!("post_action_types_#{locale}")
     elsif key.starts_with?('topic_flag_types.')
-      ApplicationSerializer.expire_cache_fragment!("post_action_flag_types_#{I18n.locale}")
+      ApplicationSerializer.expire_cache_fragment!("post_action_flag_types_#{locale}")
     else
       return false
     end
-
-    Site.clear_anon_cache!
     true
   end
 
+  private_class_method :reload_locale!
+  private_class_method :clear_cached_keys!
   private_class_method :i18n_changed
   private_class_method :expire_cache
 

spec/models/translation_override_spec.rb

@@ -93,40 +93,59 @@ describe TranslationOverride do
 
   context "site cache" do
     def cached_value(guardian, types_name, name_key, attribute)
-      json = Site.json_for(guardian)
+      I18n.with_locale(:en) do
+        json = Site.json_for(guardian)
 
-      JSON.parse(json)[types_name]
-        .find { |x| x['name_key'] == name_key }[attribute]
+        JSON.parse(json)[types_name]
+          .find { |x| x['name_key'] == name_key }[attribute]
+      end
     end
 
     shared_examples "resets site text" do
       it "resets the site cache when translations of post_action_types are changed" do
         anon_guardian = Guardian.new
         user_guardian = Guardian.new(Fabricate(:user))
-        original_value = I18n.t(translation_key)
-        types_name, name_key, attribute = translation_key.split('.')
 
-        expect(cached_value(user_guardian, types_name, name_key, attribute)).to eq(original_value)
-        expect(cached_value(anon_guardian, types_name, name_key, attribute)).to eq(original_value)
+        I18n.locale = :de
 
-        TranslationOverride.upsert!('en', translation_key, 'bar')
-        expect(cached_value(user_guardian, types_name, name_key, attribute)).to eq('bar')
-        expect(cached_value(anon_guardian, types_name, name_key, attribute)).to eq('bar')
+        translation_keys.each do |translation_key|
+          original_value = I18n.t(translation_key, locale: 'en')
+          types_name, name_key, attribute = translation_key.split('.')
 
-        TranslationOverride.revert!('en', translation_key)
-        expect(cached_value(user_guardian, types_name, name_key, attribute)).to eq(original_value)
-        expect(cached_value(anon_guardian, types_name, name_key, attribute)).to eq(original_value)
+          expect(cached_value(user_guardian, types_name, name_key, attribute)).to eq(original_value)
+          expect(cached_value(anon_guardian, types_name, name_key, attribute)).to eq(original_value)
+
+          TranslationOverride.upsert!('en', translation_key, 'bar')
+          expect(cached_value(user_guardian, types_name, name_key, attribute)).to eq('bar')
+          expect(cached_value(anon_guardian, types_name, name_key, attribute)).to eq('bar')
+        end
+
+        TranslationOverride.revert!('en', translation_keys)
+
+        translation_keys.each do |translation_key|
+          original_value = I18n.t(translation_key, locale: 'en')
+          types_name, name_key, attribute = translation_key.split('.')
+
+          expect(cached_value(user_guardian, types_name, name_key, attribute)).to eq(original_value)
+          expect(cached_value(anon_guardian, types_name, name_key, attribute)).to eq(original_value)
+        end
       end
     end
 
     context "post_action_types" do
-      let(:translation_key) { 'post_action_types.off_topic.description' }
+      let(:translation_keys) { ['post_action_types.off_topic.description'] }
 
       include_examples "resets site text"
     end
 
     context "topic_flag_types" do
-      let(:translation_key) { 'topic_flag_types.spam.description' }
+      let(:translation_keys) { ['topic_flag_types.spam.description'] }
+
+      include_examples "resets site text"
+    end
+
+    context "multiple keys" do
+      let(:translation_keys) { ['post_action_types.off_topic.description', 'topic_flag_types.spam.description'] }
 
       include_examples "resets site text"
     end