From 70eaa976a563302dae888bae1e2e34b5bb9979cc Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Fri, 7 Feb 2025 14:12:28 +0800 Subject: [PATCH] DEV: Refresh translation override status when updating (#31233) Translation overrides can be marked as "invalid interpolation keys" or "outdated" if the original translation is changed. We run a job every hour to check for this. We also have an admin problem check for it. The problem is we don't refresh this status when an admin updates the override. So even if the invalid keys are removed, the override will still show up under the "invalid" filter. There's a similar situation with the "outdated" status. The admin is shown a prompt which they can dismiss, which in turn updates the status, but updating the translation should also count as "addressing" it. This PR runs a refresh on the override status when updating. --- app/models/translation_override.rb | 17 ++++++++ spec/jobs/check_translation_overrides_spec.rb | 11 ++--- spec/models/translation_override_spec.rb | 41 ++++++++++++++++++- 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/app/models/translation_override.rb b/app/models/translation_override.rb index c6c45ef8c80..5604d6f0f97 100644 --- a/app/models/translation_override.rb +++ b/app/models/translation_override.rb @@ -64,6 +64,8 @@ class TranslationOverride < ActiveRecord::Base .where.not("translation_key LIKE '%_MF'") end + before_update :refresh_status + def self.upsert!(locale, key, value) params = { locale: locale, translation_key: key } @@ -209,6 +211,21 @@ class TranslationOverride < ActiveRecord::Base rescue MessageFormat::Compiler::CompileError => e errors.add(:base, e.cause.message) end + + def refresh_status + self.original_translation = current_default + + self.status = + if original_translation_deleted? + "deprecated" + elsif invalid_interpolation_keys.present? + "invalid_interpolation_keys" + elsif original_translation_updated? + "outdated" + else + "up_to_date" + end + end end # == Schema Information diff --git a/spec/jobs/check_translation_overrides_spec.rb b/spec/jobs/check_translation_overrides_spec.rb index fd776d144c6..7e3f63efcc2 100644 --- a/spec/jobs/check_translation_overrides_spec.rb +++ b/spec/jobs/check_translation_overrides_spec.rb @@ -17,11 +17,12 @@ RSpec.describe Jobs::CheckTranslationOverrides do end 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") + expect do + invalid_translation.update_attribute("value", "Invalid %{foo}") + described_class.new.execute({}) + end.to change { invalid_translation.reload.status }.from("up_to_date").to( + "invalid_interpolation_keys", + ) end it "marks translations that are outdated" do diff --git a/spec/models/translation_override_spec.rb b/spec/models/translation_override_spec.rb index 336e7937166..c7f64cd1823 100644 --- a/spec/models/translation_override_spec.rb +++ b/spec/models/translation_override_spec.rb @@ -352,7 +352,7 @@ RSpec.describe TranslationOverride do end end - describe "invalid_interpolation_keys" do + describe "#invalid_interpolation_keys" do fab!(:translation) do Fabricate( :translation_override, @@ -367,6 +367,45 @@ RSpec.describe TranslationOverride do end end + describe "#refresh_status" do + context "when fixing a translation with invalid interpolation keys" do + fab!(:translation) do + Fabricate( + :translation_override, + translation_key: "system_messages.welcome_user.subject_template", + status: "invalid_interpolation_keys", + ) + end + + before do + translation.update_attribute("value", "Hello, %{name}! Welcome to %{site_name}. %{foo}") + end + + it "refreshes to status to up to date" do + expect { + translation.update_attribute("value", "Hello, %{name}! Welcome to %{site_name}.") + }.to change { translation.status }.from("invalid_interpolation_keys").to("up_to_date") + end + end + + context "when updating a translation that has had the original updated" do + fab!(:translation) do + Fabricate( + :translation_override, + translation_key: "title", + original_translation: "outdated", + status: "outdated", + ) + end + + it "refreshes to status to up to date" do + expect { translation.update_attribute("value", "Discourse") }.to change { + translation.status + }.from("outdated").to("up_to_date") + end + end + end + describe "#message_format?" do subject(:override) { described_class.new(translation_key: key) }