From 7a53fb65da6dbc0aa3c8ba4374af9a76f6ee3fe5 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Fri, 14 Jul 2023 16:52:39 +0800 Subject: [PATCH] FIX: Don't show admin warnings about deleted translation overrides (#22614) We recently introduced this advice to admins when some translation overrides are outdated or using unknown interpolation keys: However we missed the case where the original translation key has been renamed or altogether removed. When this happens they are no longer visible in the admin interface, leading to the confusing situation where we say there are outdated translations, but none are shown. Because we don't explicitly handle this case, some deleted translations were incorrectly marked as having unknown interpolation keys. (This is because I18n.t will return a string like "Translation missing: foo", which obviously has no interpolation keys inside.) This change adds an additional status, deprecated for TranslationOverride, and the job that checks them will check for this status first, taking precedence over invalid_interpolation_keys. Since the advice only checks for the outdated and invalid_interpolation_keys statuses, this fixes the problem. --- .../scheduled/check_translation_overrides.rb | 6 ++++- app/models/translation_override.rb | 12 +++++++++- spec/jobs/check_translation_overrides_spec.rb | 7 ++++++ spec/models/translation_override_spec.rb | 22 +++++++++++++++++++ 4 files changed, 45 insertions(+), 2 deletions(-) diff --git a/app/jobs/scheduled/check_translation_overrides.rb b/app/jobs/scheduled/check_translation_overrides.rb index 11d7d4cd97a..5efed5592dc 100644 --- a/app/jobs/scheduled/check_translation_overrides.rb +++ b/app/jobs/scheduled/check_translation_overrides.rb @@ -5,17 +5,21 @@ module Jobs every 1.day def execute(args) + deprecated_ids = [] invalid_ids = [] outdated_ids = [] TranslationOverride.find_each do |override| - if override.invalid_interpolation_keys.present? + if override.original_translation_deleted? + deprecated_ids << override.id + elsif override.invalid_interpolation_keys.present? invalid_ids << override.id elsif override.original_translation_updated? outdated_ids << override.id end end + TranslationOverride.where(id: deprecated_ids).update_all(status: "deprecated") TranslationOverride.where(id: outdated_ids).update_all(status: "outdated") TranslationOverride.where(id: invalid_ids).update_all(status: "invalid_interpolation_keys") end diff --git a/app/models/translation_override.rb b/app/models/translation_override.rb index 0cad3951a8b..0ac4cbeb090 100644 --- a/app/models/translation_override.rb +++ b/app/models/translation_override.rb @@ -45,7 +45,7 @@ class TranslationOverride < ActiveRecord::Base validate :check_interpolation_keys - enum :status, %i[up_to_date outdated invalid_interpolation_keys] + enum :status, %i[up_to_date outdated invalid_interpolation_keys deprecated] def self.upsert!(locale, key, value) params = { locale: locale, translation_key: key } @@ -129,6 +129,12 @@ class TranslationOverride < ActiveRecord::Base private_class_method :i18n_changed private_class_method :expire_cache + def original_translation_deleted? + !I18n.overrides_disabled { I18n.t!(transformed_key, locale: :en) }.is_a?(String) + rescue I18n::MissingTranslationData + true + end + def original_translation_updated? return false if original_translation.blank? @@ -158,6 +164,10 @@ class TranslationOverride < ActiveRecord::Base private + def transformed_key + @transformed_key ||= self.class.transform_pluralized_key(translation_key) + end + def check_interpolation_keys invalid_keys = invalid_interpolation_keys diff --git a/spec/jobs/check_translation_overrides_spec.rb b/spec/jobs/check_translation_overrides_spec.rb index 2982af7acb1..2bd0e63dd44 100644 --- a/spec/jobs/check_translation_overrides_spec.rb +++ b/spec/jobs/check_translation_overrides_spec.rb @@ -2,11 +2,18 @@ RSpec.describe Jobs::CheckTranslationOverrides do fab!(:up_to_date_translation) { Fabricate(:translation_override, translation_key: "title") } + fab!(:deprecated_translation) { Fabricate(:translation_override, translation_key: "foo.bar") } 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 keys which no longer exist in the locale file" do + expect { described_class.new.execute({}) }.to change { + deprecated_translation.reload.status + }.from("up_to_date").to("deprecated") + end + it "marks translations with invalid interpolation keys" do invalid_translation.update_attribute("value", "Invalid %{foo}") diff --git a/spec/models/translation_override_spec.rb b/spec/models/translation_override_spec.rb index e690eb04fc6..54e0caceafc 100644 --- a/spec/models/translation_override_spec.rb +++ b/spec/models/translation_override_spec.rb @@ -284,6 +284,28 @@ RSpec.describe TranslationOverride do end end + describe "#original_translation_deleted?" do + context "when the original translation still exists" do + fab!(:translation) { Fabricate(:translation_override, translation_key: "title") } + + it { expect(translation.original_translation_deleted?).to eq(false) } + end + + context "when the original translation has been turned into a nested key" do + fab!(:translation) { Fabricate(:translation_override, translation_key: "title") } + + before { translation.update_attribute("translation_key", "dates") } + + it { expect(translation.original_translation_deleted?).to eq(true) } + end + + context "when the original translation no longer exists" do + fab!(:translation) { Fabricate(:translation_override, translation_key: "foo.bar") } + + it { expect(translation.original_translation_deleted?).to eq(true) } + end + end + describe "#original_translation_updated?" do context "when the translation is up to date" do fab!(:translation) { Fabricate(:translation_override, translation_key: "title") }