FIX: Re-enable invalid interpolation keys check and allow default keys to be left out of translation overrides.

https://meta.discourse.org/t/bulk-invite-from-file-resets-the-invite-forum-mailer-customized-text/67606/16

app/models/translation_override.rb

@@ -2,6 +2,15 @@ require 'js_locale_helper'
 require "i18n/i18n_interpolation_keys_finder"
 
 class TranslationOverride < ActiveRecord::Base
+  # Whitelist i18n interpolation keys that can be included when customizing translations
+  CUSTOM_INTERPOLATION_KEYS_WHITELIST = {
+    "user_notifications.user_" => %w{
+      topic_title_url_encoded
+      site_title_url_encoded
+      context
+    }
+  }
+
   validates_uniqueness_of :translation_key, scope: :locale
   validates_presence_of :locale, :translation_key, :value
 
@@ -41,12 +50,23 @@ class TranslationOverride < ActiveRecord::Base
       if original_text
         original_interpolation_keys = I18nInterpolationKeysFinder.find(original_text)
         new_interpolation_keys = I18nInterpolationKeysFinder.find(value)
-        missing_keys = (original_interpolation_keys - new_interpolation_keys)
 
-        if missing_keys.present?
+        custom_interpolation_keys = []
+
+        CUSTOM_INTERPOLATION_KEYS_WHITELIST.select do |key, value|
+          if self.translation_key.start_with?(key)
+            custom_interpolation_keys = value
+          end
+        end
+
+        invalid_keys = (original_interpolation_keys | new_interpolation_keys) -
+          original_interpolation_keys -
+          custom_interpolation_keys
+
+        if invalid_keys.present?
           self.errors.add(:base, I18n.t(
-            'activerecord.errors.models.translation_overrides.attributes.value.missing_interpolation_keys',
-            keys: missing_keys.join(', ')
+            'activerecord.errors.models.translation_overrides.attributes.value.invalid_interpolation_keys',
+            keys: invalid_keys.join(', ')
           ))
 
           return false

config/locales/server.en.yml

@@ -423,7 +423,7 @@ en:
         translation_overrides:
           attributes:
             value:
-              missing_interpolation_keys: 'The following interpolation key(s) are missing: "%{keys}"'
+              invalid_interpolation_keys: 'The following interpolation key(s) are invalid: "%{keys}"'
         watched_word:
           attributes:
             word:

spec/controllers/admin/site_texts_controller_spec.rb

@@ -53,7 +53,7 @@ describe Admin::SiteTextsController do
 
         it 'returns the right error message' do
           put :update, params: {
-            id: 'some_key', site_text: { value: 'hello %{key}' }
+            id: 'some_key', site_text: { value: 'hello %{key} %{omg}' }
           }, format: :json
 
           expect(response.status).to eq(422)
@@ -61,8 +61,8 @@ describe Admin::SiteTextsController do
           body = JSON.parse(response.body)
 
           expect(body['message']).to eq(I18n.t(
-            'activerecord.errors.models.translation_overrides.attributes.value.missing_interpolation_keys',
-            keys: 'first, second'
+            'activerecord.errors.models.translation_overrides.attributes.value.invalid_interpolation_keys',
+            keys: 'key, omg'
           ))
         end
       end

spec/mailers/user_notifications_spec.rb

@@ -547,6 +547,8 @@ describe UserNotifications do
           You are now officially notified.
           %{header_instructions}
           %{message} %{respond_instructions}
+          %{topic_title_url_encoded}
+          %{site_title_url_encoded}
           BODY
 
           body << "%{context}" if notification_type != :invited_to_topic

spec/models/translation_override_spec.rb

@@ -10,14 +10,29 @@ describe TranslationOverride do
       describe 'when interpolation keys are missing' do
         it 'should not be valid' do
           translation_override = TranslationOverride.upsert!(
-            I18n.locale, 'some_key', '%{first}'
+            I18n.locale, 'some_key', '%{key} %{omg}'
           )
 
           expect(translation_override.errors.full_messages).to include(I18n.t(
-            'activerecord.errors.models.translation_overrides.attributes.value.missing_interpolation_keys',
-            keys: 'second'
+            'activerecord.errors.models.translation_overrides.attributes.value.invalid_interpolation_keys',
+            keys: 'key, omg'
           ))
         end
+
+        context 'when custom interpolation keys are included' do
+          it 'should be valid' do
+            translation_override = TranslationOverride.upsert!(
+              I18n.locale,
+              'some_key',
+              "#{described_class::CUSTOM_INTERPOLATION_KEYS_WHITELIST['user_notifications.user_'].join(", ")} %{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
       end
     end
   end

spec/requests/admin/email_templates_controller_spec.rb

@@ -113,27 +113,52 @@ RSpec.describe Admin::EmailTemplatesController do
       end
 
       context "when subject is invalid" do
-        let(:email_subject) { 'Subject with missing interpolation key' }
-        let(:email_body) { 'The body contains [%{site_name}](%{base_url}) and %{email_token}.' }
-        let(:expected_errors) { ['<b>Subject</b>: The following interpolation key(s) are missing: "email_prefix"'] }
+        let(:email_subject) { '%{email_wrongfix} Foo' }
+        let(:email_body) { 'Body with missing interpolation keys' }
+
+        let(:expected_errors) do
+          [
+            "<b>Subject</b>: #{I18n.t(
+              'activerecord.errors.models.translation_overrides.attributes.value.invalid_interpolation_keys',
+              keys: 'email_wrongfix'
+            )}"
+          ]
+        end
 
         include_examples "invalid email template"
       end
 
       context "when body is invalid" do
-        let(:email_subject) { '%{email_prefix} Foo' }
-        let(:email_body) { 'Body with some missing interpolation keys: %{email_token}' }
-        let(:expected_errors) { ['<b>Body</b>: The following interpolation key(s) are missing: "site_name, base_url"'] }
+        let(:email_subject) { 'Subject with missing interpolation key' }
+        let(:email_body) { 'Body with %{invalid} interpolation key' }
+
+        let(:expected_errors) do
+          [
+            "<b>Body</b>: #{I18n.t(
+              'activerecord.errors.models.translation_overrides.attributes.value.invalid_interpolation_keys',
+              keys: 'invalid'
+            )}"
+          ]
+        end
 
         include_examples "invalid email template"
       end
 
       context "when subject and body are invalid invalid" do
-        let(:email_subject) { 'Subject with missing interpolation key' }
-        let(:email_body) { 'Body with some missing interpolation keys: %{email_token}' }
+        let(:email_subject) { 'Subject with %{invalid} interpolation key' }
+        let(:email_body) { 'Body with some invalid interpolation keys: %{invalid}' }
+
         let(:expected_errors) do
-          ['<b>Subject</b>: The following interpolation key(s) are missing: "email_prefix"',
-           '<b>Body</b>: The following interpolation key(s) are missing: "site_name, base_url"']
+          [
+            "<b>Subject</b>: #{I18n.t(
+              'activerecord.errors.models.translation_overrides.attributes.value.invalid_interpolation_keys',
+              keys: 'invalid'
+            )}",
+            "<b>Body</b>: #{I18n.t(
+              'activerecord.errors.models.translation_overrides.attributes.value.invalid_interpolation_keys',
+              keys: 'invalid'
+            )}",
+          ]
         end
 
         include_examples "invalid email template"