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.
This commit is contained in:
Martin Brennan 2024-08-26 11:25:36 +10:00 committed by GitHub
parent ace8db23d2
commit a16faa27cd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 300 additions and 57 deletions

View File

@ -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>
}

View File

@ -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>

View File

@ -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;
}
}

View File

@ -20,13 +20,21 @@ export default class AdminSiteTextIndexController extends Controller {
@tracked overridden; @tracked overridden;
@tracked outdated; @tracked outdated;
@tracked untranslated; @tracked untranslated;
@tracked onlySelectedLocale;
@tracked model; @tracked model;
@tracked searching = false; @tracked searching = false;
@tracked preferred = false; @tracked preferred = false;
queryParams = ["q", "overridden", "outdated", "locale", "untranslated"]; queryParams = [
"q",
"overridden",
"outdated",
"locale",
"untranslated",
"onlySelectedLocale",
];
get resolvedOverridden() { get resolvedOverridden() {
return [true, "true"].includes(this.overridden) ?? false; return [true, "true"].includes(this.overridden) ?? false;
@ -40,6 +48,10 @@ export default class AdminSiteTextIndexController extends Controller {
return [true, "true"].includes(this.untranslated) ?? false; return [true, "true"].includes(this.untranslated) ?? false;
} }
get resolvedOnlySelectedLocale() {
return [true, "true"].includes(this.onlySelectedLocale) ?? false;
}
get resolvedLocale() { get resolvedLocale() {
return this.locale ?? this.siteSettings.default_locale; return this.locale ?? this.siteSettings.default_locale;
} }
@ -59,6 +71,7 @@ export default class AdminSiteTextIndexController extends Controller {
outdated: this.resolvedOutdated, outdated: this.resolvedOutdated,
locale: this.resolvedLocale, locale: this.resolvedLocale,
untranslated: this.resolvedUntranslated, untranslated: this.resolvedUntranslated,
only_selected_locale: this.resolvedOnlySelectedLocale,
}); });
} finally { } finally {
this.searching = false; this.searching = false;
@ -119,6 +132,17 @@ export default class AdminSiteTextIndexController extends Controller {
discourseDebounce(this, this._performSearch, 400); 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 @action
search() { search() {
const q = this.q; const q = this.q;

View File

@ -22,6 +22,7 @@ export default class AdminSiteTextIndexRoute extends Route {
outdated: params.outdated ?? false, outdated: params.outdated ?? false,
untranslated: params.untranslated ?? false, untranslated: params.untranslated ?? false,
locale: params.locale ?? this.siteSettings.default_locale, locale: params.locale ?? this.siteSettings.default_locale,
only_selected_locale: params.onlySelectedLocale ?? false,
}); });
} }
} }

View File

@ -25,7 +25,7 @@
<ComboBox <ComboBox
@valueProperty="value" @valueProperty="value"
@content={{this.availableLocales}} @content={{this.availableLocales}}
@value={{this.locale}} @value={{this.resolvedLocale}}
@onChange={{this.updateLocale}} @onChange={{this.updateLocale}}
@options={{hash filterable=true}} @options={{hash filterable=true}}
class="locale-search" class="locale-search"
@ -52,6 +52,16 @@
{{i18n "admin.site_text.show_outdated"}} {{i18n "admin.site_text.show_outdated"}}
</label> </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}} {{#if this.showUntranslated}}
<label class="checkbox-label"> <label class="checkbox-label">
<input <input

View File

@ -249,7 +249,7 @@ $mobile-breakpoint: 700px;
word-wrap: break-word; word-wrap: break-word;
} }
} }
button.edit { .site-text-edit {
float: right; float: right;
} }

View File

@ -22,6 +22,7 @@ class Admin::SiteTextsController < Admin::AdminController
overridden = params[:overridden] == "true" overridden = params[:overridden] == "true"
outdated = params[:outdated] == "true" outdated = params[:outdated] == "true"
untranslated = params[:untranslated] == "true" untranslated = params[:untranslated] == "true"
only_selected_locale = params[:only_selected_locale] == "true"
extras = {} extras = {}
query = params[:q] || "" query = params[:q] || ""
@ -32,7 +33,8 @@ class Admin::SiteTextsController < Admin::AdminController
extras[:recommended] = true extras[:recommended] = true
results = self.class.preferred_keys.map { |k| record_for(key: k, locale: locale) } results = self.class.preferred_keys.map { |k| record_for(key: k, locale: locale) }
else else
results = find_translations(query, overridden, outdated, locale, untranslated) results =
find_translations(query, overridden, outdated, locale, untranslated, only_selected_locale)
if results.any? if results.any?
extras[:regex] = I18n::Backend::DiscourseI18n.create_search_regexp(query, as_string: true) 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:) def record_for(key:, value: nil, locale:)
en_key = TranslationOverride.transform_pluralized_key(key) en_key = TranslationOverride.transform_pluralized_key(key)
value ||= I18n.with_locale(locale) { I18n.t(key) } value ||= I18n.with_locale(locale) { I18n.t(key) }
interpolation_keys = interpolation_keys =
I18nInterpolationKeysFinder.find(I18n.overrides_disabled { I18n.t(en_key, locale: :en) }) I18nInterpolationKeysFinder.find(I18n.overrides_disabled { I18n.t(en_key, locale: :en) })
custom_keys = TranslationOverride.custom_interpolation_keys(en_key) custom_keys = TranslationOverride.custom_interpolation_keys(en_key)
@ -203,7 +206,7 @@ class Admin::SiteTextsController < Admin::AdminController
raise Discourse::NotFound raise Discourse::NotFound
end 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] = {} } translations = Hash.new { |hash, key| hash[key] = {} }
search_results = search_results =
I18n.with_locale(locale) do I18n.with_locale(locale) do
@ -232,7 +235,7 @@ class Admin::SiteTextsController < Admin::AdminController
next unless I18n.exists?(key, :en) next unless I18n.exists?(key, :en)
if value.is_a?(Hash) 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_key = plural[0]
plural_value = plural[1] plural_value = plural[1]
@ -243,6 +246,7 @@ class Admin::SiteTextsController < Admin::AdminController
) )
end end
else else
value = I18n.with_locale(locale) { I18n.t(key) } if only_selected_locale
results << record_for(key: key, value: value, locale: locale) results << record_for(key: key, value: value, locale: locale)
end end
end end
@ -250,7 +254,7 @@ class Admin::SiteTextsController < Admin::AdminController
results results
end end
def fix_plural_keys(key, value, locale) def fix_plural_keys(key, value, locale, only_selected_locale = false)
value = value.with_indifferent_access value = value.with_indifferent_access
plural_keys = I18n.with_locale(locale) { I18n.t("i18n.plural.keys") } 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) } 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: {}) fallback_value = I18n.t(key, locale: :en, default: {})
plural_keys.map do |k| plural_keys.map do |k|
if value[k] if value[k]
value[k] = I18n.with_locale(locale) { I18n.t("#{key}.#{k}") } if only_selected_locale
[k, value[k], locale] [k, value[k], locale]
else else
[k, fallback_value[k] || fallback_value[:other], :en] [k, fallback_value[k] || fallback_value[:other], :en]

View File

@ -6890,6 +6890,7 @@ en:
show_overriden: "Only show overridden" show_overriden: "Only show overridden"
show_outdated: "Only show outdated/invalid" show_outdated: "Only show outdated/invalid"
show_untranslated: "Only show untranslated" show_untranslated: "Only show untranslated"
only_show_selected_locale: "Only show results in selected locale"
locale: "Language:" locale: "Language:"
more_than_50_results: "There are more than 50 results. Please refine your search." more_than_50_results: "There are more than 50 results. Please refine your search."
no_results: "No matching site texts found" no_results: "No matching site texts found"

View File

@ -52,6 +52,23 @@ RSpec.describe Admin::SiteTextsController do
expect(value).to eq(I18n.t("js.yes_value", locale: :de)) expect(value).to eq(I18n.t("js.yes_value", locale: :de))
end 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 it "returns an error on invalid locale" do
get "/admin/customize/site_texts.json", params: { locale: "?" } get "/admin/customize/site_texts.json", params: { locale: "?" }
expect(response.status).to eq(400) expect(response.status).to eq(400)

View File

@ -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

View File

@ -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