From 6fc62586a2c0bd105f4da2c6d5c6f395e8da4360 Mon Sep 17 00:00:00 2001
From: Ted Johansson <ted@discourse.org>
Date: Wed, 28 Jun 2023 19:03:04 +0800
Subject: [PATCH] FEATURE: Show available interpolation keys when overriding
 translations (#22220)

This is the first of a number of PRs aimed at helping admins manage their translation overrides. It simply adds a list of available interpolation keys below the input field when editing an override.

It also includes custom interpolation key.
---
 .../addon/controllers/admin-site-text-edit.js |  4 ++
 .../admin/addon/templates/site-text-edit.hbs  |  6 +++
 .../stylesheets/common/admin/admin_base.scss  | 11 +++++
 .../admin/site_texts_controller.rb            |  6 ++-
 app/models/translation_override.rb            | 22 +++++++---
 app/serializers/site_text_serializer.rb       | 10 ++++-
 config/locales/client.en.yml                  |  1 +
 .../admin/site_texts_controller_spec.rb       | 41 +++++++++++++++++++
 8 files changed, 93 insertions(+), 8 deletions(-)

diff --git a/app/assets/javascripts/admin/addon/controllers/admin-site-text-edit.js b/app/assets/javascripts/admin/addon/controllers/admin-site-text-edit.js
index 8bcfc9b9da5..7dbc7229b21 100644
--- a/app/assets/javascripts/admin/addon/controllers/admin-site-text-edit.js
+++ b/app/assets/javascripts/admin/addon/controllers/admin-site-text-edit.js
@@ -48,4 +48,8 @@ export default Controller.extend(bufferedProperty("siteText"), {
       },
     });
   },
+
+  get interpolationKeys() {
+    return this.siteText.interpolation_keys.join(", ");
+  },
 });
diff --git a/app/assets/javascripts/admin/addon/templates/site-text-edit.hbs b/app/assets/javascripts/admin/addon/templates/site-text-edit.hbs
index 3643a123f8b..ef327ab4067 100644
--- a/app/assets/javascripts/admin/addon/templates/site-text-edit.hbs
+++ b/app/assets/javascripts/admin/addon/templates/site-text-edit.hbs
@@ -13,6 +13,12 @@
     @class="site-text-value"
   />
 
+  {{#if this.siteText.has_interpolation_keys}}
+    <div class="desc">{{i18n "admin.site_text.interpolation_keys"}}
+      {{this.interpolationKeys}}
+    </div>
+  {{/if}}
+
   <SaveControls
     @model={{this.siteText}}
     @action={{action "saveChanges"}}
diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss
index eb36f466d21..49ec9173e85 100644
--- a/app/assets/stylesheets/common/admin/admin_base.scss
+++ b/app/assets/stylesheets/common/admin/admin_base.scss
@@ -269,8 +269,13 @@ $mobile-breakpoint: 700px;
   }
   .edit-site-text {
     textarea {
+      display: block;
       width: 100%;
       max-width: 800px;
+      margin: 0;
+    }
+    .save-button {
+      margin-top: 1em;
     }
     .save-button,
     .title {
@@ -284,6 +289,12 @@ $mobile-breakpoint: 700px;
     .go-back {
       margin-top: 1em;
     }
+    .desc {
+      padding-top: 3px;
+      font-size: var(--font-down-1);
+      line-height: var(--line-height-large);
+      color: var(--primary-medium);
+    }
   }
   p.warning {
     color: var(--danger);
diff --git a/app/controllers/admin/site_texts_controller.rb b/app/controllers/admin/site_texts_controller.rb
index a92065399f9..8f2e851ccb7 100644
--- a/app/controllers/admin/site_texts_controller.rb
+++ b/app/controllers/admin/site_texts_controller.rb
@@ -156,8 +156,12 @@ class Admin::SiteTextsController < Admin::AdminController
   end
 
   def record_for(key:, value: nil, locale:)
+    en_key = TranslationOverride.transform_pluralized_key(key)
     value ||= I18n.with_locale(locale) { I18n.t(key) }
-    { id: key, value: value, locale: locale }
+    interpolation_keys =
+      I18nInterpolationKeysFinder.find(I18n.overrides_disabled { I18n.t(en_key, locale: :en) })
+    custom_keys = TranslationOverride.custom_interpolation_keys(en_key)
+    { id: key, value: value, locale: locale, interpolation_keys: interpolation_keys + custom_keys }
   end
 
   PLURALIZED_REGEX = /(.*)\.(zero|one|two|few|many|other)\z/
diff --git a/app/models/translation_override.rb b/app/models/translation_override.rb
index 00786ba7f20..92b8ed59df0 100644
--- a/app/models/translation_override.rb
+++ b/app/models/translation_override.rb
@@ -105,6 +105,21 @@ class TranslationOverride < ActiveRecord::Base
     true
   end
 
+  # We use English as the source of truth when extracting interpolation keys,
+  # but some languages, like Arabic, have plural forms (zero, two, few, many)
+  # which don't exist in English (one, other), so we map that here in order to
+  # find the correct, English translation key in which to look.
+  def self.transform_pluralized_key(key)
+    match = key.match(/(.*)\.(zero|two|few|many)\z/)
+    match ? match.to_a.second + ".other" : key
+  end
+
+  def self.custom_interpolation_keys(translation_key)
+    ALLOWED_CUSTOM_INTERPOLATION_KEYS.find do |keys, value|
+      break value if keys.any? { |k| translation_key.start_with?(k) }
+    end || []
+  end
+
   private_class_method :reload_locale!
   private_class_method :clear_cached_keys!
   private_class_method :i18n_changed
@@ -113,7 +128,7 @@ class TranslationOverride < ActiveRecord::Base
   private
 
   def check_interpolation_keys
-    transformed_key = transform_pluralized_key(translation_key)
+    transformed_key = self.class.transform_pluralized_key(translation_key)
 
     original_text = I18n.overrides_disabled { I18n.t(transformed_key, locale: :en) }
 
@@ -144,11 +159,6 @@ class TranslationOverride < ActiveRecord::Base
       end
     end
   end
-
-  def transform_pluralized_key(key)
-    match = key.match(/(.*)\.(zero|two|few|many)\z/)
-    match ? match.to_a.second + ".other" : key
-  end
 end
 
 # == Schema Information
diff --git a/app/serializers/site_text_serializer.rb b/app/serializers/site_text_serializer.rb
index 0946eb2fb73..dc452f47397 100644
--- a/app/serializers/site_text_serializer.rb
+++ b/app/serializers/site_text_serializer.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class SiteTextSerializer < ApplicationSerializer
-  attributes :id, :value, :overridden?, :can_revert?
+  attributes :id, :value, :interpolation_keys, :has_interpolation_keys?, :overridden?, :can_revert?
 
   def id
     object[:id]
@@ -11,6 +11,14 @@ class SiteTextSerializer < ApplicationSerializer
     object[:value]
   end
 
+  def interpolation_keys
+    object[:interpolation_keys]
+  end
+
+  def has_interpolation_keys?
+    object[:interpolation_keys].present?
+  end
+
   def overridden?
     if options[:overridden_keys]
       options[:overridden_keys].include?(object[:id])
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 20d92db0a39..6de7e9f2228 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -6069,6 +6069,7 @@ en:
         show_overriden: "Only show overridden"
         locale: "Language:"
         more_than_50_results: "There are more than 50 results. Please refine your search."
+        interpolation_keys: "Available interpolation keys:"
 
       settings: # used by theme and site settings
         show_overriden: "Only show overridden"
diff --git a/spec/requests/admin/site_texts_controller_spec.rb b/spec/requests/admin/site_texts_controller_spec.rb
index 14b3f2455c3..03a9e71e23e 100644
--- a/spec/requests/admin/site_texts_controller_spec.rb
+++ b/spec/requests/admin/site_texts_controller_spec.rb
@@ -213,11 +213,21 @@ RSpec.describe Admin::SiteTextsController do
               expected_translations.map do |key, value|
                 overridden =
                   defined?(expected_overridden) ? expected_overridden[key] || false : false
+                interpolation_keys =
+                  (
+                    if defined?(expected_interpolation_keys)
+                      expected_interpolation_keys[key] || []
+                    else
+                      []
+                    end
+                  )
                 {
                   id: "colour.#{key}",
                   value: value,
                   can_revert: overridden,
                   overridden: overridden,
+                  interpolation_keys: interpolation_keys,
+                  has_interpolation_keys: interpolation_keys.present?,
                 }
               end
 
@@ -228,6 +238,7 @@ RSpec.describe Admin::SiteTextsController do
         context "with English" do
           let(:locale) { :en }
           let(:expected_translations) { { one: "%{count} colour", other: "%{count} colours" } }
+          let(:expected_interpolation_keys) { { one: ["count"], other: ["count"] } }
 
           include_examples "finds correct plural keys"
         end
@@ -242,6 +253,9 @@ RSpec.describe Admin::SiteTextsController do
               other: "%{count} colours",
             }
           end
+          let(:expected_interpolation_keys) do
+            { one: ["count"], few: ["count"], many: ["count"], other: ["count"] }
+          end
 
           include_examples "finds correct plural keys"
         end
@@ -266,6 +280,9 @@ RSpec.describe Admin::SiteTextsController do
               other: "%{count} colours",
             }
           end
+          let(:expected_interpolation_keys) do
+            { one: ["count"], few: ["count"], many: ["count"], other: ["count"] }
+          end
 
           include_examples "finds correct plural keys"
         end
@@ -287,6 +304,9 @@ RSpec.describe Admin::SiteTextsController do
           let(:expected_translations) do
             { one: "ONE", few: "FEW", many: "%{count} цветов", other: "%{count} colours" }
           end
+          let(:expected_interpolation_keys) do
+            { one: ["count"], few: ["count"], many: ["count"], other: ["count"] }
+          end
           let(:expected_overridden) { { one: true, few: true } }
 
           include_examples "finds correct plural keys"
@@ -425,6 +445,27 @@ RSpec.describe Admin::SiteTextsController do
         expect(site_text["value"]).to eq("education.new-topic override")
       end
 
+      it "includes custom interpolation keys" do
+        TranslationOverride.upsert!(
+          :en,
+          "system_messages.welcome_user.title",
+          "system_messages.welcome_user.title override",
+        )
+
+        get "/admin/customize/site_texts/system_messages.welcome_user.title.json",
+            params: {
+              locale: "en_GB",
+            }
+        expect(response.status).to eq(200)
+        json = response.parsed_body
+
+        expect(json["site_text"]["interpolation_keys"]).to include(
+          "username",
+          "name",
+          "name_or_username",
+        )
+      end
+
       context "with plural keys" do
         before do
           I18n.backend.store_translations(