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 962944145c0..2f747246d07 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 @@ -8,10 +8,11 @@ export default class AdminSiteTextIndexController extends Controller { searching = false; siteTexts = null; preferred = false; - queryParams = ["q", "overridden", "locale"]; + queryParams = ["q", "overridden", "outdated", "locale"]; locale = null; q = null; overridden = false; + outdated = false; init() { super.init(...arguments); @@ -21,7 +22,10 @@ export default class AdminSiteTextIndexController extends Controller { _performSearch() { this.store - .find("site-text", this.getProperties("q", "overridden", "locale")) + .find( + "site-text", + this.getProperties("q", "overridden", "outdated", "locale") + ) .then((results) => { this.set("siteTexts", results); }) @@ -58,6 +62,13 @@ export default class AdminSiteTextIndexController extends Controller { discourseDebounce(this, this._performSearch, 400); } + @action + toggleOutdated() { + this.toggleProperty("outdated"); + this.set("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 ecf7baafd55..daf37261093 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 @@ -6,13 +6,14 @@ export default class AdminSiteTextIndexRoute extends Route { queryParams = { q: { replace: true }, overridden: { replace: true }, + outdated: { replace: true }, locale: { replace: true }, }; model(params) { return this.store.find( "site-text", - getProperties(params, "q", "overridden", "locale") + getProperties(params, "q", "overridden", "outdated", "locale") ); } 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 e3f7caa855c..2a6c64a6125 100644 --- a/app/assets/javascripts/admin/addon/templates/site-text-index.hbs +++ b/app/assets/javascripts/admin/addon/templates/site-text-index.hbs @@ -34,12 +34,23 @@ + +

diff --git a/app/assets/javascripts/discourse/tests/acceptance/admin-site-text-test.js b/app/assets/javascripts/discourse/tests/acceptance/admin-site-text-test.js index 38cfee1af71..36bb84091f6 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/admin-site-text-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/admin-site-text-test.js @@ -24,7 +24,7 @@ acceptance("Admin - Site Texts", function (needs) { assert.ok(exists(".site-text.overridden")); // Only show overridden - await click(".search-area .filter-options input"); + await click(".search-area .filter-options #toggle-overridden"); assert.strictEqual( currentURL(), "/admin/customize/site_texts?overridden=true&q=Test" @@ -32,6 +32,14 @@ acceptance("Admin - Site Texts", function (needs) { assert.ok(!exists(".site-text:not(.overridden)")); assert.ok(exists(".site-text.overridden")); + await click(".search-area .filter-options #toggle-overridden"); + + // Only show outdated + await click(".search-area .filter-options #toggle-outdated"); + assert.strictEqual( + currentURL(), + "/admin/customize/site_texts?outdated=true&q=Test" + ); }); test("edit and revert a site text by key", async function (assert) { diff --git a/app/controllers/admin/site_texts_controller.rb b/app/controllers/admin/site_texts_controller.rb index 8f2e851ccb7..bc434c41a00 100644 --- a/app/controllers/admin/site_texts_controller.rb +++ b/app/controllers/admin/site_texts_controller.rb @@ -20,17 +20,18 @@ class Admin::SiteTextsController < Admin::AdminController def index overridden = params[:overridden] == "true" + outdated = params[:outdated] == "true" extras = {} query = params[:q] || "" locale = fetch_locale(params[:locale]) - if query.blank? && !overridden + if query.blank? && !overridden && !outdated extras[:recommended] = true results = self.class.preferred_keys.map { |k| record_for(key: k, locale: locale) } else - results = find_translations(query, overridden, locale) + results = find_translations(query, overridden, outdated, locale) if results.any? extras[:regex] = I18n::Backend::DiscourseI18n.create_search_regexp(query, as_string: true) @@ -188,10 +189,18 @@ class Admin::SiteTextsController < Admin::AdminController raise Discourse::NotFound end - def find_translations(query, overridden, locale) + def find_translations(query, overridden, outdated, locale) translations = Hash.new { |hash, key| hash[key] = {} } search_results = I18n.with_locale(locale) { I18n.search(query, only_overridden: overridden) } + if outdated + outdated_keys = + TranslationOverride.where(status: %i[outdated invalid_interpolation_keys]).pluck( + :translation_key, + ) + search_results.select! { |k, _| outdated_keys.include?(k) } + end + search_results.each do |key, value| if PLURALIZED_REGEX.match(key) translations[$1][$2] = value diff --git a/app/jobs/scheduled/check_translation_overrides.rb b/app/jobs/scheduled/check_translation_overrides.rb new file mode 100644 index 00000000000..11d7d4cd97a --- /dev/null +++ b/app/jobs/scheduled/check_translation_overrides.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Jobs + class CheckTranslationOverrides < ::Jobs::Scheduled + every 1.day + + def execute(args) + invalid_ids = [] + outdated_ids = [] + + TranslationOverride.find_each do |override| + if override.invalid_interpolation_keys.present? + invalid_ids << override.id + elsif override.original_translation_updated? + outdated_ids << override.id + end + end + + TranslationOverride.where(id: outdated_ids).update_all(status: "outdated") + TranslationOverride.where(id: invalid_ids).update_all(status: "invalid_interpolation_keys") + end + end +end diff --git a/app/models/admin_dashboard_data.rb b/app/models/admin_dashboard_data.rb index 2b85dc6b5c0..a57b4d7c1b4 100644 --- a/app/models/admin_dashboard_data.rb +++ b/app/models/admin_dashboard_data.rb @@ -206,7 +206,8 @@ class AdminDashboardData :out_of_date_themes, :unreachable_themes, :watched_words_check, - :google_analytics_version_check + :google_analytics_version_check, + :translation_overrides_check register_default_scheduled_problem_checks @@ -360,6 +361,12 @@ class AdminDashboardData end end + def translation_overrides_check + if TranslationOverride.exists?(status: %i[outdated invalid_interpolation_keys]) + I18n.t("dashboard.outdated_translations_warning", base_path: Discourse.base_path) + end + end + def image_magick_check if SiteSetting.create_thumbnails && !system("command -v convert >/dev/null;") I18n.t("dashboard.image_magick_warning") diff --git a/app/models/translation_override.rb b/app/models/translation_override.rb index 92b8ed59df0..0cad3951a8b 100644 --- a/app/models/translation_override.rb +++ b/app/models/translation_override.rb @@ -45,14 +45,18 @@ class TranslationOverride < ActiveRecord::Base validate :check_interpolation_keys + enum :status, %i[up_to_date outdated invalid_interpolation_keys] + def self.upsert!(locale, key, value) params = { locale: locale, translation_key: key } translation_override = find_or_initialize_by(params) sanitized_value = translation_override.sanitize_field(value, additional_attributes: ["data-auto-route"]) + original_translation = + I18n.overrides_disabled { I18n.t(transform_pluralized_key(key), locale: :en) } - data = { value: sanitized_value } + data = { value: sanitized_value, original_translation: original_translation } if key.end_with?("_MF") _, filename = JsLocaleHelper.find_message_format_locale([locale], fallback_to_english: false) data[:compiled_js] = JsLocaleHelper.compile_message_format(filename, locale, sanitized_value) @@ -125,39 +129,48 @@ class TranslationOverride < ActiveRecord::Base private_class_method :i18n_changed private_class_method :expire_cache - private + def original_translation_updated? + return false if original_translation.blank? - def check_interpolation_keys + transformed_key = self.class.transform_pluralized_key(translation_key) + + original_translation != I18n.overrides_disabled { I18n.t(transformed_key, locale: :en) } + end + + def invalid_interpolation_keys transformed_key = self.class.transform_pluralized_key(translation_key) original_text = I18n.overrides_disabled { I18n.t(transformed_key, locale: :en) } - if original_text - original_interpolation_keys = I18nInterpolationKeysFinder.find(original_text) - new_interpolation_keys = I18nInterpolationKeysFinder.find(value) - custom_interpolation_keys = [] + return [] if original_text.blank? - ALLOWED_CUSTOM_INTERPOLATION_KEYS.select do |keys, value| - custom_interpolation_keys = value if keys.any? { |key| transformed_key.start_with?(key) } - end + original_interpolation_keys = I18nInterpolationKeysFinder.find(original_text) + new_interpolation_keys = I18nInterpolationKeysFinder.find(value) + custom_interpolation_keys = [] - 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.invalid_interpolation_keys", - keys: invalid_keys.join(I18n.t("word_connector.comma")), - count: invalid_keys.size, - ), - ) - - false - end + ALLOWED_CUSTOM_INTERPOLATION_KEYS.select do |keys, value| + custom_interpolation_keys = value if keys.any? { |key| transformed_key.start_with?(key) } end + + (original_interpolation_keys | new_interpolation_keys) - original_interpolation_keys - + custom_interpolation_keys + end + + private + + def check_interpolation_keys + invalid_keys = invalid_interpolation_keys + + return if invalid_keys.blank? + + self.errors.add( + :base, + I18n.t( + "activerecord.errors.models.translation_overrides.attributes.value.invalid_interpolation_keys", + keys: invalid_keys.join(I18n.t("word_connector.comma")), + count: invalid_keys.size, + ), + ) end end @@ -165,13 +178,15 @@ end # # Table name: translation_overrides # -# id :integer not null, primary key -# locale :string not null -# translation_key :string not null -# value :string not null -# created_at :datetime not null -# updated_at :datetime not null -# compiled_js :text +# id :integer not null, primary key +# locale :string not null +# translation_key :string not null +# value :string not null +# created_at :datetime not null +# updated_at :datetime not null +# compiled_js :text +# original_translation :text +# status :integer default("up_to_date"), not null # # Indexes # diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6ae5abc6d18..31b336b653d 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -6068,6 +6068,7 @@ en: go_back: "Back to Search" recommended: "We recommend customizing the following text to suit your needs:" show_overriden: "Only show overridden" + show_outdated: "Only show outdated/invalid" locale: "Language:" more_than_50_results: "There are more than 50 results. Please refine your search." interpolation_keys: "Available interpolation keys:" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 297e50eb14e..61f878df49c 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1478,6 +1478,7 @@ en: image_magick_warning: 'The server is configured to create thumbnails of large images, but ImageMagick is not installed. Install ImageMagick using your favorite package manager or download the latest release.' failing_emails_warning: 'There are %{num_failed_jobs} email jobs that failed. Check your app.yml and ensure that the mail server settings are correct. See the failed jobs in Sidekiq.' subfolder_ends_in_slash: "Your subfolder setup is incorrect; the DISCOURSE_RELATIVE_URL_ROOT ends in a slash." + outdated_translations_warning: "Some of your translation overrides are out of date. Please check your text customizations." email_polling_errored_recently: one: "Email polling has generated an error in the past 24 hours. Look at the logs for more details." other: "Email polling has generated %{count} errors in the past 24 hours. Look at the logs for more details." diff --git a/db/migrate/20230703035052_add_status_to_translation_overrides.rb b/db/migrate/20230703035052_add_status_to_translation_overrides.rb new file mode 100644 index 00000000000..d210514d968 --- /dev/null +++ b/db/migrate/20230703035052_add_status_to_translation_overrides.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddStatusToTranslationOverrides < ActiveRecord::Migration[7.0] + def change + add_column :translation_overrides, :original_translation, :text + add_column :translation_overrides, :status, :integer, null: false, default: 0 + end +end diff --git a/spec/fabricators/translation_override_fabricator.rb b/spec/fabricators/translation_override_fabricator.rb new file mode 100644 index 00000000000..4e329fbc72c --- /dev/null +++ b/spec/fabricators/translation_override_fabricator.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +Fabricator(:translation_override) do + translation_key "title" + value "Discourse" + locale "en" +end diff --git a/spec/jobs/check_translation_overrides_spec.rb b/spec/jobs/check_translation_overrides_spec.rb new file mode 100644 index 00000000000..2982af7acb1 --- /dev/null +++ b/spec/jobs/check_translation_overrides_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +RSpec.describe Jobs::CheckTranslationOverrides do + fab!(:up_to_date_translation) { Fabricate(:translation_override, translation_key: "title") } + fab!(:outdated_translation) do + Fabricate(:translation_override, translation_key: "posts", original_translation: "outdated") + end + fab!(:invalid_translation) { Fabricate(:translation_override, translation_key: "topics") } + + it "marks translations with invalid interpolation keys" do + invalid_translation.update_attribute("value", "Invalid %{foo}") + + expect { described_class.new.execute({}) }.to change { invalid_translation.reload.status }.from( + "up_to_date", + ).to("invalid_interpolation_keys") + end + + it "marks translations that are outdated" do + expect { described_class.new.execute({}) }.to change { + outdated_translation.reload.status + }.from("up_to_date").to("outdated") + end + + it "does not mark up to date translations" do + expect { described_class.new.execute({}) }.not_to change { + up_to_date_translation.reload.status + } + end +end diff --git a/spec/models/admin_dashboard_data_spec.rb b/spec/models/admin_dashboard_data_spec.rb index bc7b947e5b5..4f93c786fd3 100644 --- a/spec/models/admin_dashboard_data_spec.rb +++ b/spec/models/admin_dashboard_data_spec.rb @@ -345,4 +345,26 @@ RSpec.describe AdminDashboardData do expect(dashboard_data.out_of_date_themes).to eq(nil) end end + + describe "#translation_overrides_check" do + subject(:dashboard_data) { described_class.new } + + context "when there are outdated translations" do + before { Fabricate(:translation_override, translation_key: "foo.bar", status: "outdated") } + + it "outputs the correct message" do + expect(dashboard_data.translation_overrides_check).to eq( + I18n.t("dashboard.outdated_translations_warning", base_path: Discourse.base_path), + ) + end + end + + context "when there are no outdated translations" do + before { Fabricate(:translation_override, status: "up_to_date") } + + it "outputs nothing" do + expect(dashboard_data.translation_overrides_check).to eq(nil) + end + end + end end diff --git a/spec/models/translation_override_spec.rb b/spec/models/translation_override_spec.rb index d8c8a5c3971..e690eb04fc6 100644 --- a/spec/models/translation_override_spec.rb +++ b/spec/models/translation_override_spec.rb @@ -283,4 +283,43 @@ RSpec.describe TranslationOverride do end end end + + describe "#original_translation_updated?" do + context "when the translation is up to date" do + fab!(:translation) { Fabricate(:translation_override, translation_key: "title") } + + it { expect(translation.original_translation_updated?).to eq(false) } + end + + context "when the translation is outdated" do + fab!(:translation) do + Fabricate(:translation_override, translation_key: "title", original_translation: "outdated") + end + + it { expect(translation.original_translation_updated?).to eq(true) } + end + + context "when we can't tell because the translation is too old" do + fab!(:translation) do + Fabricate(:translation_override, translation_key: "title", original_translation: nil) + end + + it { expect(translation.original_translation_updated?).to eq(false) } + end + end + + describe "invalid_interpolation_keys" do + fab!(:translation) do + Fabricate( + :translation_override, + translation_key: "system_messages.welcome_user.subject_template", + ) + end + + it "picks out invalid keys and ignores known and custom keys" do + translation.update_attribute("value", "Hello, %{name}! Welcome to %{site_name}. %{foo}") + + expect(translation.invalid_interpolation_keys).to contain_exactly("foo") + end + end end