FIX: error overriding user notification string with valid keys (#12510)

FIX: error overriding user notification string with valid keys (#12510)

When overriding the translation for i18n keys used in user notifications like user_notifications.reply_by_email, errors were returned for valid interpolation keys. Keys like topic_title_url_encoded are supported, so no error should be raised.

https://meta.discourse.org/t/-/50305/7

diff --git a/app/models/translation_override.rb b/app/models/translation_override.rb
index 1071d5d..04815b8 100644
--- a/app/models/translation_override.rb
+++ b/app/models/translation_override.rb
@@ -5,7 +5,12 @@ require "i18n/i18n_interpolation_keys_finder"
 class TranslationOverride < ActiveRecord::Base
   # Allowlist i18n interpolation keys that can be included when customizing translations
   ALLOWED_CUSTOM_INTERPOLATION_KEYS = {
-    "user_notifications.user_" => %w{
+    [
+      "user_notifications.user_",
+      "user_notifications.only_reply_by_email",
+      "user_notifications.reply_by_email",
+      "user_notifications.visit_link_to_respond",
+    ] => %w{
       topic_title_url_encoded
       site_title_url_encoded
       context
@@ -18,6 +23,7 @@ class TranslationOverride < ActiveRecord::Base
       topic_title
     }
   }
+
   include ActiveSupport::Deprecation::DeprecatedConstantAccessor
   deprecate_constant 'CUSTOM_INTERPOLATION_KEYS_WHITELIST', 'TranslationOverride::ALLOWED_CUSTOM_INTERPOLATION_KEYS'
 
@@ -107,8 +113,8 @@ class TranslationOverride < ActiveRecord::Base
 
       custom_interpolation_keys = []
 
-      ALLOWED_CUSTOM_INTERPOLATION_KEYS.select do |key, value|
-        if transformed_key.start_with?(key)
+      ALLOWED_CUSTOM_INTERPOLATION_KEYS.select do |keys, value|
+        if keys.any? { |key| transformed_key.start_with?(key) }
           custom_interpolation_keys = value
         end
       end
diff --git a/spec/models/translation_override_spec.rb b/spec/models/translation_override_spec.rb
index 6f54eaf..dddb037 100644
--- a/spec/models/translation_override_spec.rb
+++ b/spec/models/translation_override_spec.rb
@@ -6,7 +6,11 @@ describe TranslationOverride do
   context 'validations' do
     describe '#value' do
       before do
-        I18n.backend.store_translations(I18n.locale, some_key: '%{first} %{second}')
+        I18n.backend.store_translations(
+          I18n.locale,
+          "user_notifications.user_did_something" => '%{first} %{second}'
+        )
+
         I18n.backend.store_translations(:en, something: { one: '%{key1} %{key2}', other: '%{key3} %{key4}' })
       end
 
@@ -22,17 +26,46 @@ describe TranslationOverride do
           ))
         end
 
-        context 'when custom interpolation keys are included' do
-          it 'should be valid' do
+        context "when custom interpolation keys are included" do
+          [
+            "user_notifications.user_did_something",
+            "user_notifications.only_reply_by_email",
+            "user_notifications.only_reply_by_email_pm",
+            "user_notifications.reply_by_email",
+            "user_notifications.reply_by_email_pm",
+            "user_notifications.visit_link_to_respond",
+            "user_notifications.visit_link_to_respond_pm",
+          ].each do |i18n_key|
+            it "should validate keys for #{i18n_key}" do
+              interpolation_key_names = described_class::ALLOWED_CUSTOM_INTERPOLATION_KEYS.find do |keys, _|
+                keys.include?("user_notifications.user_")
+              end
+
+              string_with_interpolation_keys = interpolation_key_names.map { |x| "%{#{x}}" }.join(" ")
+
+              translation_override = TranslationOverride.upsert!(
+                I18n.locale,
+                i18n_key,
+                "#{string_with_interpolation_keys} %{something}",
+              )
+
+              expect(translation_override.errors.full_messages).to include(I18n.t(
+                "activerecord.errors.models.translation_overrides.attributes.value.invalid_interpolation_keys",
+                keys: "something"
+              ))
+            end
+          end
+
+          it "should validate keys that shouldn't be used outside of user_notifications" do
+            I18n.backend.store_translations(:en, "not_a_notification" => "Test %{key1}")
             translation_override = TranslationOverride.upsert!(
               I18n.locale,
-              'some_key',
-              "#{described_class::ALLOWED_CUSTOM_INTERPOLATION_KEYS['user_notifications.user_'].join(", ")} %{something}"
+              "not_a_notification",
+              "Overriden %{key1} %{topic_title_url_encoded}",
             )
-
             expect(translation_override.errors.full_messages).to include(I18n.t(
-              'activerecord.errors.models.translation_overrides.attributes.value.invalid_interpolation_keys',
-              keys: 'something'
+              "activerecord.errors.models.translation_overrides.attributes.value.invalid_interpolation_keys",
+              keys: "topic_title_url_encoded"
             ))
           end
         end

GitHub sha: ca4bc9b8

This commit appears in #12510 which was approved by pmusaraj. It was merged by nlalonde.