From a16faa27cd0088361c1a486a68c98e731ab57878 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Mon, 26 Aug 2024 11:25:36 +1000
Subject: [PATCH] FEATURE: Allow showing site text search in selected locale
 (#28453)

When searching for site texts for admin using the english
version of the text, previously we would show the english
version in the results _even if_ there was another locale
translated version available when a locale was selected
from the dropdown.

This commit adds a "Only show results in selected locale"
checkbox option which will instead make it so the results
shown are in the target locale, making it easier for translators
to tell when there is actually translations vs. missing tranlsations.
---
 .../addon/components/site-text-summary.gjs    |  63 ++++++++++
 .../addon/components/site-text-summary.hbs    |   9 --
 .../addon/components/site-text-summary.js     |  41 ------
 .../controllers/admin-site-text-index.js      |  26 +++-
 .../addon/routes/admin-site-text-index.js     |   1 +
 .../admin/addon/templates/site-text-index.hbs |  12 +-
 .../stylesheets/common/admin/admin_base.scss  |   2 +-
 .../admin/site_texts_controller.rb            |  13 +-
 config/locales/client.en.yml                  |   1 +
 .../admin/site_texts_controller_spec.rb       |  17 +++
 spec/system/admin_site_texts_spec.rb          | 119 ++++++++++++++++++
 .../page_objects/pages/admin_site_texts.rb    |  53 ++++++++
 12 files changed, 300 insertions(+), 57 deletions(-)
 create mode 100644 app/assets/javascripts/admin/addon/components/site-text-summary.gjs
 delete mode 100644 app/assets/javascripts/admin/addon/components/site-text-summary.hbs
 delete mode 100644 app/assets/javascripts/admin/addon/components/site-text-summary.js
 create mode 100644 spec/system/admin_site_texts_spec.rb
 create mode 100644 spec/system/page_objects/pages/admin_site_texts.rb

diff --git a/app/assets/javascripts/admin/addon/components/site-text-summary.gjs b/app/assets/javascripts/admin/addon/components/site-text-summary.gjs
new file mode 100644
index 00000000000..5eb204fc135
--- /dev/null
+++ b/app/assets/javascripts/admin/addon/components/site-text-summary.gjs
@@ -0,0 +1,63 @@
+import Component from "@glimmer/component";
+import { fn } from "@ember/helper";
+import { action } from "@ember/object";
+import didInsert from "@ember/render-modifiers/modifiers/did-insert";
+import DButton from "discourse/components/d-button";
+import concatClass from "discourse/helpers/concat-class";
+import highlightHTML from "discourse/lib/highlight-html";
+import { bind } from "discourse-common/utils/decorators";
+
+export default class SiteTextSummary extends Component {
+  @action
+  highlightSearchTerm(element) {
+    const term = this.#searchTerm();
+
+    if (term) {
+      highlightHTML(
+        element.querySelector(".site-text-id, .site-text-value"),
+        term,
+        {
+          className: "text-highlight",
+        }
+      );
+    }
+  }
+
+  @action
+  onClick() {
+    this.args.editAction(this.siteText);
+  }
+
+  @bind
+  #searchTerm() {
+    const regex = this.args.searchRegex;
+    const siteText = this.args.siteText;
+
+    if (regex && siteText) {
+      const matches = siteText.value.match(new RegExp(regex, "i"));
+      if (matches) {
+        return matches[0];
+      }
+    }
+
+    return this.args.term;
+  }
+
+  <template>
+    <div
+      class={{concatClass "site-text" (if @siteText.overridden "overridden")}}
+      {{didInsert this.highlightSearchTerm}}
+      data-site-text-id={{@siteText.id}}
+    >
+      <DButton
+        @label="admin.site_text.edit"
+        @action={{fn @editAction @siteText}}
+        class="btn-default site-text-edit"
+      />
+      <h3 class="site-text-id">{{@siteText.id}}</h3>
+      <div class="site-text-value">{{@siteText.value}}</div>
+
+      <div class="clearfix"></div>
+    </div>
+  </template>
+}
diff --git a/app/assets/javascripts/admin/addon/components/site-text-summary.hbs b/app/assets/javascripts/admin/addon/components/site-text-summary.hbs
deleted file mode 100644
index d73f2be2ea9..00000000000
--- a/app/assets/javascripts/admin/addon/components/site-text-summary.hbs
+++ /dev/null
@@ -1,9 +0,0 @@
-<DButton
-  @label="admin.site_text.edit"
-  @action={{fn this.editAction this.siteText}}
-  class="btn-default edit"
-/>
-<h3 class="site-text-id">{{this.siteText.id}}</h3>
-<div class="site-text-value">{{this.siteText.value}}</div>
-
-<div class="clearfix"></div>
\ No newline at end of file
diff --git a/app/assets/javascripts/admin/addon/components/site-text-summary.js b/app/assets/javascripts/admin/addon/components/site-text-summary.js
deleted file mode 100644
index 1add22e7b9e..00000000000
--- a/app/assets/javascripts/admin/addon/components/site-text-summary.js
+++ /dev/null
@@ -1,41 +0,0 @@
-import Component from "@ember/component";
-import { classNameBindings, classNames } from "@ember-decorators/component";
-import highlightHTML from "discourse/lib/highlight-html";
-
-@classNames("site-text")
-@classNameBindings("siteText.overridden")
-export default class SiteTextSummary extends Component {
-  didInsertElement() {
-    super.didInsertElement(...arguments);
-
-    const term = this._searchTerm();
-
-    if (term) {
-      highlightHTML(
-        this.element.querySelector(".site-text-id, .site-text-value"),
-        term,
-        {
-          className: "text-highlight",
-        }
-      );
-    }
-  }
-
-  click() {
-    this.editAction(this.siteText);
-  }
-
-  _searchTerm() {
-    const regex = this.searchRegex;
-    const siteText = this.siteText;
-
-    if (regex && siteText) {
-      const matches = siteText.value.match(new RegExp(regex, "i"));
-      if (matches) {
-        return matches[0];
-      }
-    }
-
-    return this.term;
-  }
-}
diff --git a/app/assets/javascripts/admin/addon/controllers/admin-site-text-index.js b/app/assets/javascripts/admin/addon/controllers/admin-site-text-index.js
index aba95fbc457..d4ce5a2dad9 100644
--- a/app/assets/javascripts/admin/addon/controllers/admin-site-text-index.js
+++ b/app/assets/javascripts/admin/addon/controllers/admin-site-text-index.js
@@ -20,13 +20,21 @@ export default class AdminSiteTextIndexController extends Controller {
   @tracked overridden;
   @tracked outdated;
   @tracked untranslated;
+  @tracked onlySelectedLocale;
 
   @tracked model;
 
   @tracked searching = false;
   @tracked preferred = false;
 
-  queryParams = ["q", "overridden", "outdated", "locale", "untranslated"];
+  queryParams = [
+    "q",
+    "overridden",
+    "outdated",
+    "locale",
+    "untranslated",
+    "onlySelectedLocale",
+  ];
 
   get resolvedOverridden() {
     return [true, "true"].includes(this.overridden) ?? false;
@@ -40,6 +48,10 @@ export default class AdminSiteTextIndexController extends Controller {
     return [true, "true"].includes(this.untranslated) ?? false;
   }
 
+  get resolvedOnlySelectedLocale() {
+    return [true, "true"].includes(this.onlySelectedLocale) ?? false;
+  }
+
   get resolvedLocale() {
     return this.locale ?? this.siteSettings.default_locale;
   }
@@ -59,6 +71,7 @@ export default class AdminSiteTextIndexController extends Controller {
         outdated: this.resolvedOutdated,
         locale: this.resolvedLocale,
         untranslated: this.resolvedUntranslated,
+        only_selected_locale: this.resolvedOnlySelectedLocale,
       });
     } finally {
       this.searching = false;
@@ -119,6 +132,17 @@ export default class AdminSiteTextIndexController extends Controller {
     discourseDebounce(this, this._performSearch, 400);
   }
 
+  @action
+  toggleOnlySelectedLocale() {
+    if (this.resolvedOnlySelectedLocale) {
+      this.onlySelectedLocale = null;
+    } else {
+      this.onlySelectedLocale = true;
+    }
+    this.searching = true;
+    discourseDebounce(this, this._performSearch, 400);
+  }
+
   @action
   search() {
     const q = this.q;
diff --git a/app/assets/javascripts/admin/addon/routes/admin-site-text-index.js b/app/assets/javascripts/admin/addon/routes/admin-site-text-index.js
index 07a231cf6f9..77c6b223399 100644
--- a/app/assets/javascripts/admin/addon/routes/admin-site-text-index.js
+++ b/app/assets/javascripts/admin/addon/routes/admin-site-text-index.js
@@ -22,6 +22,7 @@ export default class AdminSiteTextIndexRoute extends Route {
       outdated: params.outdated ?? false,
       untranslated: params.untranslated ?? false,
       locale: params.locale ?? this.siteSettings.default_locale,
+      only_selected_locale: params.onlySelectedLocale ?? false,
     });
   }
 }
diff --git a/app/assets/javascripts/admin/addon/templates/site-text-index.hbs b/app/assets/javascripts/admin/addon/templates/site-text-index.hbs
index 330c3c41cb0..52e9aa33530 100644
--- a/app/assets/javascripts/admin/addon/templates/site-text-index.hbs
+++ b/app/assets/javascripts/admin/addon/templates/site-text-index.hbs
@@ -25,7 +25,7 @@
       <ComboBox
         @valueProperty="value"
         @content={{this.availableLocales}}
-        @value={{this.locale}}
+        @value={{this.resolvedLocale}}
         @onChange={{this.updateLocale}}
         @options={{hash filterable=true}}
         class="locale-search"
@@ -52,6 +52,16 @@
       {{i18n "admin.site_text.show_outdated"}}
     </label>
 
+    <label class="checkbox-label">
+      <input
+        id="toggle-only-locale"
+        type="checkbox"
+        checked={{this.resolvedOnlySelectedLocale}}
+        {{on "click" this.toggleOnlySelectedLocale}}
+      />
+      {{i18n "admin.site_text.only_show_selected_locale"}}
+    </label>
+
     {{#if this.showUntranslated}}
       <label class="checkbox-label">
         <input
diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss
index 604425c11ac..4f39bda247f 100644
--- a/app/assets/stylesheets/common/admin/admin_base.scss
+++ b/app/assets/stylesheets/common/admin/admin_base.scss
@@ -249,7 +249,7 @@ $mobile-breakpoint: 700px;
         word-wrap: break-word;
       }
     }
-    button.edit {
+    .site-text-edit {
       float: right;
     }
 
diff --git a/app/controllers/admin/site_texts_controller.rb b/app/controllers/admin/site_texts_controller.rb
index d4aa36d7587..b79352dd1c7 100644
--- a/app/controllers/admin/site_texts_controller.rb
+++ b/app/controllers/admin/site_texts_controller.rb
@@ -22,6 +22,7 @@ class Admin::SiteTextsController < Admin::AdminController
     overridden = params[:overridden] == "true"
     outdated = params[:outdated] == "true"
     untranslated = params[:untranslated] == "true"
+    only_selected_locale = params[:only_selected_locale] == "true"
     extras = {}
 
     query = params[:q] || ""
@@ -32,7 +33,8 @@ class Admin::SiteTextsController < Admin::AdminController
       extras[:recommended] = true
       results = self.class.preferred_keys.map { |k| record_for(key: k, locale: locale) }
     else
-      results = find_translations(query, overridden, outdated, locale, untranslated)
+      results =
+        find_translations(query, overridden, outdated, locale, untranslated, only_selected_locale)
 
       if results.any?
         extras[:regex] = I18n::Backend::DiscourseI18n.create_search_regexp(query, as_string: true)
@@ -173,6 +175,7 @@ class Admin::SiteTextsController < Admin::AdminController
   def record_for(key:, value: nil, locale:)
     en_key = TranslationOverride.transform_pluralized_key(key)
     value ||= I18n.with_locale(locale) { I18n.t(key) }
+
     interpolation_keys =
       I18nInterpolationKeysFinder.find(I18n.overrides_disabled { I18n.t(en_key, locale: :en) })
     custom_keys = TranslationOverride.custom_interpolation_keys(en_key)
@@ -203,7 +206,7 @@ class Admin::SiteTextsController < Admin::AdminController
     raise Discourse::NotFound
   end
 
-  def find_translations(query, overridden, outdated, locale, untranslated)
+  def find_translations(query, overridden, outdated, locale, untranslated, only_selected_locale)
     translations = Hash.new { |hash, key| hash[key] = {} }
     search_results =
       I18n.with_locale(locale) do
@@ -232,7 +235,7 @@ class Admin::SiteTextsController < Admin::AdminController
       next unless I18n.exists?(key, :en)
 
       if value.is_a?(Hash)
-        fix_plural_keys(key, value, locale).each do |plural|
+        fix_plural_keys(key, value, locale, only_selected_locale).each do |plural|
           plural_key = plural[0]
           plural_value = plural[1]
 
@@ -243,6 +246,7 @@ class Admin::SiteTextsController < Admin::AdminController
           )
         end
       else
+        value = I18n.with_locale(locale) { I18n.t(key) } if only_selected_locale
         results << record_for(key: key, value: value, locale: locale)
       end
     end
@@ -250,7 +254,7 @@ class Admin::SiteTextsController < Admin::AdminController
     results
   end
 
-  def fix_plural_keys(key, value, locale)
+  def fix_plural_keys(key, value, locale, only_selected_locale = false)
     value = value.with_indifferent_access
     plural_keys = I18n.with_locale(locale) { I18n.t("i18n.plural.keys") }
     return value if value.keys.size == plural_keys.size && plural_keys.all? { |k| value.key?(k) }
@@ -258,6 +262,7 @@ class Admin::SiteTextsController < Admin::AdminController
     fallback_value = I18n.t(key, locale: :en, default: {})
     plural_keys.map do |k|
       if value[k]
+        value[k] = I18n.with_locale(locale) { I18n.t("#{key}.#{k}") } if only_selected_locale
         [k, value[k], locale]
       else
         [k, fallback_value[k] || fallback_value[:other], :en]
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index cf32a94fca3..b407d941a49 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -6890,6 +6890,7 @@ en:
         show_overriden: "Only show overridden"
         show_outdated: "Only show outdated/invalid"
         show_untranslated: "Only show untranslated"
+        only_show_selected_locale: "Only show results in selected locale"
         locale: "Language:"
         more_than_50_results: "There are more than 50 results. Please refine your search."
         no_results: "No matching site texts found"
diff --git a/spec/requests/admin/site_texts_controller_spec.rb b/spec/requests/admin/site_texts_controller_spec.rb
index 43480931a27..3267d80cd42 100644
--- a/spec/requests/admin/site_texts_controller_spec.rb
+++ b/spec/requests/admin/site_texts_controller_spec.rb
@@ -52,6 +52,23 @@ RSpec.describe Admin::SiteTextsController do
         expect(value).to eq(I18n.t("js.yes_value", locale: :de))
       end
 
+      it "can return the value of the translation in the selected locale rather than English if specified" do
+        SiteSetting.default_locale = "it"
+        get "/admin/customize/site_texts.json",
+            params: {
+              q: "all tags",
+              locale: "it",
+              only_selected_locale: true,
+            }
+        value =
+          response.parsed_body["site_texts"].find do |text|
+            text["id"] == "js.topic.browse_all_tags_or_latest"
+          end[
+            "value"
+          ]
+        expect(value).to eq(I18n.t("js.topic.browse_all_tags_or_latest", locale: "it"))
+      end
+
       it "returns an error on invalid locale" do
         get "/admin/customize/site_texts.json", params: { locale: "?" }
         expect(response.status).to eq(400)
diff --git a/spec/system/admin_site_texts_spec.rb b/spec/system/admin_site_texts_spec.rb
new file mode 100644
index 00000000000..d320b090d35
--- /dev/null
+++ b/spec/system/admin_site_texts_spec.rb
@@ -0,0 +1,119 @@
+# frozen_string_literal: true
+
+describe "Admin Site Texts Page", type: :system do
+  fab!(:admin)
+
+  let(:site_texts_page) { PageObjects::Pages::AdminSiteTexts.new }
+
+  before { sign_in(admin) }
+
+  it "can search for client text using the default locale" do
+    site_texts_page.visit
+    site_texts_page.search("skip to main content")
+    expect(site_texts_page).to have_translation_key("js.skip_to_main_content")
+    expect(site_texts_page).to have_translation_value(I18n.t("js.skip_to_main_content"))
+
+    site_texts_page.visit
+    site_texts_page.search("js.skip_to_main_content")
+    expect(site_texts_page).to have_translation_key("js.skip_to_main_content")
+    expect(site_texts_page).to have_translation_value(I18n.t("js.skip_to_main_content"))
+  end
+
+  it "can search for server text using the default locale" do
+    site_texts_page.visit
+    site_texts_page.search("Something went wrong updating theme")
+    expect(site_texts_page).to have_translation_key("themes.other_error")
+    expect(site_texts_page).to have_translation_value(I18n.t("themes.other_error"))
+
+    site_texts_page.visit
+    site_texts_page.search("themes.other_error")
+    expect(site_texts_page).to have_translation_key("themes.other_error")
+    expect(site_texts_page).to have_translation_value(I18n.t("themes.other_error"))
+  end
+
+  it "can search for text using the selected locale" do
+    site_texts_page.visit
+    site_texts_page.select_locale("it")
+    site_texts_page.search("Passa al contenuto principale")
+    expect(site_texts_page).to have_translation_key("js.skip_to_main_content")
+    expect(site_texts_page).to have_translation_value(
+      I18n.t("js.skip_to_main_content", locale: "it"),
+    )
+
+    site_texts_page.visit
+    site_texts_page.select_locale("it")
+    site_texts_page.search("js.skip_to_main_content")
+    expect(site_texts_page).to have_translation_key("js.skip_to_main_content")
+    expect(site_texts_page).to have_translation_value(
+      I18n.t("js.skip_to_main_content", locale: "it"),
+    )
+  end
+
+  it "can show only overridden translations" do
+    site_texts_page.visit
+    site_texts_page.search("skip")
+    site_texts_page.toggle_only_show_overridden
+    expect(page).to have_css(".site-text", count: 0)
+
+    TranslationOverride.create!(
+      locale: "en",
+      translation_key: "js.skip_to_main_content",
+      value: "Overridden skip text",
+      original_translation: I18n.t("js.skip_to_main_content"),
+    )
+    I18n.reload!
+
+    site_texts_page.visit
+    site_texts_page.search("skip")
+    site_texts_page.toggle_only_show_overridden
+    expect(page).to have_css(".site-text", count: 1)
+    expect(site_texts_page).to have_translation_key("js.skip_to_main_content")
+  end
+
+  it "can show only outddated translations" do
+    site_texts_page.visit
+    site_texts_page.search("skip")
+    site_texts_page.toggle_only_show_outdated
+    expect(page).to have_css(".site-text", count: 0)
+
+    TranslationOverride.create!(
+      locale: "en",
+      translation_key: "js.skip_to_main_content",
+      value: "Overridden skip text",
+      original_translation: I18n.t("js.skip_to_main_content"),
+      status: "outdated",
+    )
+    I18n.reload!
+
+    site_texts_page.visit
+    site_texts_page.search("skip")
+    site_texts_page.toggle_only_show_outdated
+    expect(page).to have_css(".site-text", count: 1)
+    expect(site_texts_page).to have_translation_key("js.skip_to_main_content")
+  end
+
+  it "can show results in the selected locale" do
+    site_texts_page.visit
+    site_texts_page.search("skip to main content")
+    expect(site_texts_page).to have_translation_key("js.skip_to_main_content")
+    expect(site_texts_page).to have_translation_value(I18n.t("js.skip_to_main_content"))
+
+    site_texts_page.toggle_only_show_results_in_selected_locale
+    site_texts_page.select_locale("it")
+    expect(site_texts_page).to have_translation_key("js.skip_to_main_content")
+    expect(site_texts_page).to have_translation_value(
+      I18n.t("js.skip_to_main_content", locale: "it"),
+    )
+  end
+
+  it "can edit a translation string" do
+    site_texts_page.visit
+    site_texts_page.search("skip to main content")
+    site_texts_page.edit_translation("js.skip_to_main_content")
+    site_texts_page.override_translation("Some overridden value")
+    site_texts_page.visit
+    site_texts_page.search("js.skip_to_main_content")
+    expect(site_texts_page).to have_translation_value("Some overridden value")
+    expect(TranslationOverride.exists?(translation_key: "js.skip_to_main_content")).to eq(true)
+  end
+end
diff --git a/spec/system/page_objects/pages/admin_site_texts.rb b/spec/system/page_objects/pages/admin_site_texts.rb
new file mode 100644
index 00000000000..41bce154c9f
--- /dev/null
+++ b/spec/system/page_objects/pages/admin_site_texts.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+module PageObjects
+  module Pages
+    class AdminSiteTexts < PageObjects::Pages::Base
+      def visit
+        page.visit("/admin/customize/site_texts")
+        self
+      end
+
+      def search(text)
+        find(".site-text-search").fill_in(with: text)
+        page.send_keys(:escape)
+      end
+
+      def has_translation_key?(key)
+        has_css?(".site-text-id", text: key)
+      end
+
+      def has_translation_value?(value)
+        has_css?(".site-text-value", text: value)
+      end
+
+      def select_locale(locale_short_name)
+        locale_selector = PageObjects::Components::SelectKit.new(".locale-search")
+        locale_selector.expand
+        locale_selector.select_row_by_value(locale_short_name)
+        locale_selector.collapse
+      end
+
+      def toggle_only_show_overridden
+        find("#toggle-overridden").click
+      end
+
+      def toggle_only_show_outdated
+        find("#toggle-outdated").click
+      end
+
+      def toggle_only_show_results_in_selected_locale
+        find("#toggle-only-locale").click
+      end
+
+      def edit_translation(key)
+        find(".site-text[data-site-text-id='#{key}']").find(".site-text-edit").click
+      end
+
+      def override_translation(value)
+        find(".site-text-value").fill_in(with: value)
+        find(".save-changes").click
+      end
+    end
+  end
+end