From 341acacba83ad30aa62a479e913839cfd26076f5 Mon Sep 17 00:00:00 2001
From: Ted Johansson <ted@discourse.org>
Date: Wed, 19 Jul 2023 23:06:13 +0800
Subject: [PATCH] DEV: Add endpoint for dismissing outdated translations
 (#22509)

Recently we started giving admins a notice in the advice panel when their translations have become outdated due to changes in core. However, we didn't include any additional information.

This PR adds more information about the outdated translation inside the site text edit page, together with an option to dismiss the warning.
---
 .../addon/controllers/admin-site-text-edit.js | 15 +++++
 .../admin/addon/models/site-text.js           |  9 +++
 .../admin/addon/templates/site-text-edit.hbs  | 16 +++++
 .../stylesheets/common/admin/admin_base.scss  | 12 ++++
 .../admin/site_texts_controller.rb            | 20 ++++++
 app/models/translation_override.rb            | 16 ++---
 app/serializers/site_text_serializer.rb       | 34 +++++++++-
 config/locales/client.en.yml                  |  6 ++
 config/routes.rb                              |  8 +++
 .../admin/site_texts_controller_spec.rb       | 67 +++++++++++++++++++
 10 files changed, 192 insertions(+), 11 deletions(-)

diff --git a/app/assets/javascripts/admin/addon/controllers/admin-site-text-edit.js b/app/assets/javascripts/admin/addon/controllers/admin-site-text-edit.js
index 7dbc7229b21..45b0e797cba 100644
--- a/app/assets/javascripts/admin/addon/controllers/admin-site-text-edit.js
+++ b/app/assets/javascripts/admin/addon/controllers/admin-site-text-edit.js
@@ -16,6 +16,11 @@ export default Controller.extend(bufferedProperty("siteText"), {
     return this.siteText.value === value;
   },
 
+  @discourseComputed("siteText.status")
+  isOutdated(status) {
+    return status === "outdated";
+  },
+
   @action
   saveChanges() {
     const attrs = this.buffered.getProperties("value");
@@ -49,6 +54,16 @@ export default Controller.extend(bufferedProperty("siteText"), {
     });
   },
 
+  @action
+  dismissOutdated() {
+    this.siteText
+      .dismissOutdated(this.locale)
+      .then(() => {
+        this.siteText.set("status", "up_to_date");
+      })
+      .catch(popupAjaxError);
+  },
+
   get interpolationKeys() {
     return this.siteText.interpolation_keys.join(", ");
   },
diff --git a/app/assets/javascripts/admin/addon/models/site-text.js b/app/assets/javascripts/admin/addon/models/site-text.js
index 916030b8d3a..0460ab3d9dc 100644
--- a/app/assets/javascripts/admin/addon/models/site-text.js
+++ b/app/assets/javascripts/admin/addon/models/site-text.js
@@ -8,4 +8,13 @@ export default class SiteText extends RestModel {
       type: "DELETE",
     }).then((result) => getProperties(result.site_text, "value", "can_revert"));
   }
+
+  dismissOutdated(locale) {
+    return ajax(
+      `/admin/customize/site_texts/${this.id}/dismiss_outdated?locale=${locale}`,
+      {
+        type: "PUT",
+      }
+    );
+  }
 }
diff --git a/app/assets/javascripts/admin/addon/templates/site-text-edit.hbs b/app/assets/javascripts/admin/addon/templates/site-text-edit.hbs
index ef327ab4067..33eee6fc3f1 100644
--- a/app/assets/javascripts/admin/addon/templates/site-text-edit.hbs
+++ b/app/assets/javascripts/admin/addon/templates/site-text-edit.hbs
@@ -7,6 +7,22 @@
     <h4>{{i18n "admin.site_text.locale"}} {{this.localeFullName}}</h4>
   </div>
 
+  {{#if this.isOutdated}}
+    <div class="outdated">
+      <h4>{{i18n "admin.site_text.outdated.title"}}</h4>
+      <p>{{i18n "admin.site_text.outdated.description"}}</p>
+      <h5>{{i18n "admin.site_text.outdated.old_default"}}</h5>
+      <p>{{this.siteText.old_default}}</p>
+      <h5>{{i18n "admin.site_text.outdated.new_default"}}</h5>
+      <p>{{this.siteText.new_default}}</p>
+      <DButton
+        @class="btn-default"
+        @action={{action "dismissOutdated"}}
+        @label="admin.site_text.outdated.dismiss"
+      />
+    </div>
+  {{/if}}
+
   <ExpandingTextArea
     @value={{this.buffered.value}}
     @rows="1"
diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss
index 49ec9173e85..5aa31982d8f 100644
--- a/app/assets/stylesheets/common/admin/admin_base.scss
+++ b/app/assets/stylesheets/common/admin/admin_base.scss
@@ -295,6 +295,18 @@ $mobile-breakpoint: 700px;
       line-height: var(--line-height-large);
       color: var(--primary-medium);
     }
+    .outdated {
+      border: 1px solid var(--primary-low);
+      box-sizing: border-box;
+      color: var(--primary);
+      margin-bottom: 1em;
+      max-width: 800px;
+      padding: 1em;
+
+      p {
+        color: var(--primary-high);
+      }
+    }
   }
   p.warning {
     color: var(--danger);
diff --git a/app/controllers/admin/site_texts_controller.rb b/app/controllers/admin/site_texts_controller.rb
index bc434c41a00..c7700ab61c8 100644
--- a/app/controllers/admin/site_texts_controller.rb
+++ b/app/controllers/admin/site_texts_controller.rb
@@ -128,6 +128,26 @@ class Admin::SiteTextsController < Admin::AdminController
     render_serialized(site_text, SiteTextSerializer, root: "site_text", rest_serializer: true)
   end
 
+  def dismiss_outdated
+    locale = fetch_locale(params[:locale])
+    override = TranslationOverride.find_by(locale: locale, translation_key: params[:id])
+
+    raise Discourse::NotFound if override.blank?
+
+    if override.outdated?
+      override.update!(
+        status: "up_to_date",
+        original_translation:
+          I18n.overrides_disabled do
+            I18n.t(TranslationOverride.transform_pluralized_key(params[:id]), locale: :en)
+          end,
+      )
+      render json: success_json
+    else
+      render json: failed_json.merge(message: "Can only dismiss outdated translations"), status: 422
+    end
+  end
+
   def get_reseed_options
     render_json_dump(
       categories: SeedData::Categories.with_default_locale.reseed_options,
diff --git a/app/models/translation_override.rb b/app/models/translation_override.rb
index 0ac4cbeb090..23a81a1fbc1 100644
--- a/app/models/translation_override.rb
+++ b/app/models/translation_override.rb
@@ -138,19 +138,13 @@ class TranslationOverride < ActiveRecord::Base
   def original_translation_updated?
     return false if original_translation.blank?
 
-    transformed_key = self.class.transform_pluralized_key(translation_key)
-
-    original_translation != I18n.overrides_disabled { I18n.t(transformed_key, locale: :en) }
+    original_translation != current_default
   end
 
   def invalid_interpolation_keys
-    transformed_key = self.class.transform_pluralized_key(translation_key)
+    return [] if current_default.blank?
 
-    original_text = I18n.overrides_disabled { I18n.t(transformed_key, locale: :en) }
-
-    return [] if original_text.blank?
-
-    original_interpolation_keys = I18nInterpolationKeysFinder.find(original_text)
+    original_interpolation_keys = I18nInterpolationKeysFinder.find(current_default)
     new_interpolation_keys = I18nInterpolationKeysFinder.find(value)
     custom_interpolation_keys = []
 
@@ -162,6 +156,10 @@ class TranslationOverride < ActiveRecord::Base
       custom_interpolation_keys
   end
 
+  def current_default
+    I18n.overrides_disabled { I18n.t(transformed_key, locale: :en) }
+  end
+
   private
 
   def transformed_key
diff --git a/app/serializers/site_text_serializer.rb b/app/serializers/site_text_serializer.rb
index dc452f47397..aeea09cb574 100644
--- a/app/serializers/site_text_serializer.rb
+++ b/app/serializers/site_text_serializer.rb
@@ -1,7 +1,15 @@
 # frozen_string_literal: true
 
 class SiteTextSerializer < ApplicationSerializer
-  attributes :id, :value, :interpolation_keys, :has_interpolation_keys?, :overridden?, :can_revert?
+  attributes :id,
+             :value,
+             :status,
+             :old_default,
+             :new_default,
+             :interpolation_keys,
+             :has_interpolation_keys?,
+             :overridden?,
+             :can_revert?
 
   def id
     object[:id]
@@ -11,6 +19,22 @@ class SiteTextSerializer < ApplicationSerializer
     object[:value]
   end
 
+  def status
+    if override.present?
+      override.status
+    else
+      "up_to_date"
+    end
+  end
+
+  def old_default
+    override.original_translation if override.present?
+  end
+
+  def new_default
+    override.current_default if override.present?
+  end
+
   def interpolation_keys
     object[:interpolation_keys]
   end
@@ -23,9 +47,15 @@ class SiteTextSerializer < ApplicationSerializer
     if options[:overridden_keys]
       options[:overridden_keys].include?(object[:id])
     else
-      TranslationOverride.exists?(locale: object[:locale], translation_key: object[:id])
+      override.present?
     end
   end
 
   alias_method :can_revert?, :overridden?
+
+  private
+
+  def override
+    TranslationOverride.find_by(locale: object[:locale], translation_key: object[:id])
+  end
 end
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 6e20c8912ee..5a5d1aab2cb 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -6098,6 +6098,12 @@ en:
         locale: "Language:"
         more_than_50_results: "There are more than 50 results. Please refine your search."
         interpolation_keys: "Available interpolation keys:"
+        outdated:
+          title: "This translation is outdated"
+          description: "The default translation for this key has changed since this override was created. Please check below that your translation matches any changes that have been made to the original intent."
+          old_default: "Old default"
+          new_default: "New default"
+          dismiss: "Dismiss"
 
       settings: # used by theme and site settings
         show_overriden: "Only show overridden"
diff --git a/config/routes.rb b/config/routes.rb
index 0f25e262745..68b641f10b6 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -257,6 +257,14 @@ Discourse::Application.routes.draw do
                  id: /[\w.\-\+\%\&]+/i,
                }
         delete "site_texts/:id" => "site_texts#revert", :constraints => { id: /[\w.\-\+\%\&]+/i }
+        put "site_texts/:id/dismiss_outdated" => "site_texts#dismiss_outdated",
+            :constraints => {
+              id: /[\w.\-\+\%\&]+/i,
+            }
+        put "site_texts/:id/dismiss_outdated.json" => "site_texts#dismiss_outdated",
+            :constraints => {
+              id: /[\w.\-\+\%\&]+/i,
+            }
 
         get "reseed" => "site_texts#get_reseed_options"
         post "reseed" => "site_texts#reseed"
diff --git a/spec/requests/admin/site_texts_controller_spec.rb b/spec/requests/admin/site_texts_controller_spec.rb
index 03a9e71e23e..c9336bf827d 100644
--- a/spec/requests/admin/site_texts_controller_spec.rb
+++ b/spec/requests/admin/site_texts_controller_spec.rb
@@ -224,6 +224,9 @@ RSpec.describe Admin::SiteTextsController do
                 {
                   id: "colour.#{key}",
                   value: value,
+                  status: "up_to_date",
+                  old_default: nil,
+                  new_default: nil,
                   can_revert: overridden,
                   overridden: overridden,
                   interpolation_keys: interpolation_keys,
@@ -815,6 +818,70 @@ RSpec.describe Admin::SiteTextsController do
     end
   end
 
+  describe "#dismiss_outdated" do
+    before { sign_in(admin) }
+
+    context "when using a key which isn't overridden" do
+      it "returns a not found error" do
+        put "/admin/customize/site_texts/title/dismiss_outdated.json",
+            params: {
+              locale: default_locale,
+            }
+
+        expect(response.status).to eq(404)
+
+        json = response.parsed_body
+        expect(json["error_type"]).to eq("not_found")
+      end
+    end
+
+    context "when the override isn't outdated" do
+      before do
+        Fabricate(
+          :translation_override,
+          locale: default_locale,
+          translation_key: "title",
+          value: "My Forum",
+        )
+      end
+
+      it "returns an unprocessable entity error" do
+        put "/admin/customize/site_texts/title/dismiss_outdated.json",
+            params: {
+              locale: default_locale,
+            }
+
+        expect(response.status).to eq(422)
+
+        json = response.parsed_body
+        expect(json["failed"]).to eq("FAILED")
+        expect(json["message"]).to eq("Can only dismiss outdated translations")
+      end
+    end
+
+    context "when the override is outdated" do
+      before do
+        Fabricate(
+          :translation_override,
+          locale: default_locale,
+          translation_key: "title",
+          value: "My Forum",
+          status: "outdated",
+        )
+      end
+
+      it "returns success" do
+        put "/admin/customize/site_texts/title/dismiss_outdated.json",
+            params: {
+              locale: default_locale,
+            }
+
+        expect(response.status).to eq(200)
+        expect(response.parsed_body["success"]).to eq("OK")
+      end
+    end
+  end
+
   context "when reseeding" do
     before do
       staff_category = Fabricate(:category, name: "Staff EN", user: Discourse.system_user)