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.
This commit is contained in:
Ted Johansson 2023-07-14 16:52:39 +08:00 committed by GitHub
parent bfad5607bd
commit 7a53fb65da
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 45 additions and 2 deletions

View File

@ -5,17 +5,21 @@ module Jobs
every 1.day every 1.day
def execute(args) def execute(args)
deprecated_ids = []
invalid_ids = [] invalid_ids = []
outdated_ids = [] outdated_ids = []
TranslationOverride.find_each do |override| 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 invalid_ids << override.id
elsif override.original_translation_updated? elsif override.original_translation_updated?
outdated_ids << override.id outdated_ids << override.id
end end
end end
TranslationOverride.where(id: deprecated_ids).update_all(status: "deprecated")
TranslationOverride.where(id: outdated_ids).update_all(status: "outdated") TranslationOverride.where(id: outdated_ids).update_all(status: "outdated")
TranslationOverride.where(id: invalid_ids).update_all(status: "invalid_interpolation_keys") TranslationOverride.where(id: invalid_ids).update_all(status: "invalid_interpolation_keys")
end end

View File

@ -45,7 +45,7 @@ class TranslationOverride < ActiveRecord::Base
validate :check_interpolation_keys 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) def self.upsert!(locale, key, value)
params = { locale: locale, translation_key: key } params = { locale: locale, translation_key: key }
@ -129,6 +129,12 @@ class TranslationOverride < ActiveRecord::Base
private_class_method :i18n_changed private_class_method :i18n_changed
private_class_method :expire_cache 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? def original_translation_updated?
return false if original_translation.blank? return false if original_translation.blank?
@ -158,6 +164,10 @@ class TranslationOverride < ActiveRecord::Base
private private
def transformed_key
@transformed_key ||= self.class.transform_pluralized_key(translation_key)
end
def check_interpolation_keys def check_interpolation_keys
invalid_keys = invalid_interpolation_keys invalid_keys = invalid_interpolation_keys

View File

@ -2,11 +2,18 @@
RSpec.describe Jobs::CheckTranslationOverrides do RSpec.describe Jobs::CheckTranslationOverrides do
fab!(:up_to_date_translation) { Fabricate(:translation_override, translation_key: "title") } 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 fab!(:outdated_translation) do
Fabricate(:translation_override, translation_key: "posts", original_translation: "outdated") Fabricate(:translation_override, translation_key: "posts", original_translation: "outdated")
end end
fab!(:invalid_translation) { Fabricate(:translation_override, translation_key: "topics") } 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 it "marks translations with invalid interpolation keys" do
invalid_translation.update_attribute("value", "Invalid %{foo}") invalid_translation.update_attribute("value", "Invalid %{foo}")

View File

@ -284,6 +284,28 @@ RSpec.describe TranslationOverride do
end end
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 describe "#original_translation_updated?" do
context "when the translation is up to date" do context "when the translation is up to date" do
fab!(:translation) { Fabricate(:translation_override, translation_key: "title") } fab!(:translation) { Fabricate(:translation_override, translation_key: "title") }