DEV: Use backend plugin api instead of frontend apis to override translation values (#210)

In #205, we returned translated values using existing JS apis. While this was convenient, it does not allow us to modify crawler views such as ...

https://github.com/discourse/discourse/blob/main/app/views/topics/show.html.erb 

... when we want to serve translated pages.

This commit makes use of discourse/discourse#31261 to override the values.

Author: nattsw

.discourse-compatibility

@@ -1,4 +1,4 @@
-< 3.5.0.beta1-dev: f7d6ca6b32c0cff27422dd9f7583f41b203c0c57
+< 3.5.0.beta1-dev: 65d7ea2dbc9e7bf276e5ac3f9e23c2111e64e278
 < 3.4.0.beta3-dev: b4cf3a065884816fa3f770248c2bf908ba65d8ac
 < 3.4.0.beta1-dev: 5346b4bafba2c2fb817f030a473b7bbca97b909c
 < 3.3.0.beta1-dev: 6750e10a6d9dfd3fc2c9a0cac5a83aca1a8ee401

assets/javascripts/discourse/initializers/extend-for-translate-button.js

@@ -29,25 +29,6 @@ function initializeTranslation(api) {
     (currentUser || siteSettings.experimental_anon_language_switcher)
   ) {
     api.renderInOutlet("topic-navigation", ShowOriginalContent);
-    api.decorateCookedElement((cookedElement, helper) => {
-      if (helper) {
-        const translatedCooked = helper.getModel().get("translated_cooked");
-        if (translatedCooked) {
-          cookedElement.innerHTML = translatedCooked;
-        } else {
-          // this experimental feature does not yet support
-          // translating individual untranslated posts
-        }
-      }
-    });
-
-    api.registerModelTransformer("topic", (topics) => {
-      topics.forEach((topic) => {
-        if (topic.translated_title) {
-          topic.set("fancy_title", topic.translated_title);
-        }
-      });
-    });
   }
 
   if (!siteSettings.experimental_topic_translation) {

plugin.rb

@@ -60,17 +60,33 @@ module ::DiscourseTranslator
     scope.can_translate?(object)
   end
 
-  add_to_serializer :post, :translated_cooked do
-    if !SiteSetting.experimental_topic_translation || scope.request.params["show"] == "original"
-      return nil
+  register_modifier(:basic_post_serializer_cooked) do |cooked, serializer|
+    if !SiteSetting.experimental_topic_translation ||
+         serializer.scope.request.params["show"] == "original" ||
+         serializer.object.detected_locale == I18n.locale.to_s.gsub("_", "-")
+      cooked
+    else
+      serializer.object.translation_for(I18n.locale).presence
     end
-    object.translation_for(I18n.locale) || nil
   end
 
-  add_to_serializer :topic_view, :translated_title do
-    if !SiteSetting.experimental_topic_translation || scope.request.params["show"] == "original"
-      return nil
+  register_modifier(:topic_serializer_fancy_title) do |fancy_title, serializer|
+    if !SiteSetting.experimental_topic_translation ||
+         serializer.scope.request.params["show"] == "original" ||
+         serializer.object.detected_locale == I18n.locale.to_s.gsub("_", "-")
+      fancy_title
+    else
+      serializer.object.translation_for(I18n.locale).presence
+    end
+  end
+
+  register_modifier(:topic_view_serializer_fancy_title) do |fancy_title, serializer|
+    if !SiteSetting.experimental_topic_translation ||
+         serializer.scope.request.params["show"] == "original" ||
+         serializer.object.topic.detected_locale == I18n.locale.to_s.gsub("_", "-")
+      fancy_title
+    else
+      serializer.object.topic.translation_for(I18n.locale).presence
     end
-    object.topic.translation_for(I18n.locale) || nil
   end
 end

spec/serializers/basic_topic_serializer_spec.rb

@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+describe BasicTopicSerializer do
+  fab!(:user) { Fabricate(:user, locale: "ja") }
+  fab!(:topic)
+
+  before do
+    SiteSetting.translator_enabled = true
+    SiteSetting.experimental_topic_translation = true
+  end
+
+  describe "#fancy_title" do
+    let!(:guardian) { Guardian.new(user) }
+    let!(:original_title) { "FUS ROH DAAHHH" }
+    let!(:jap_title) { "フス・ロ・ダ・ア" }
+
+    before do
+      topic.title = original_title
+      SiteSetting.experimental_topic_translation = true
+      I18n.locale = "ja"
+    end
+
+    def serialize_topic(guardian_user: user, params: {})
+      env = { "action_dispatch.request.parameters" => params, "REQUEST_METHOD" => "GET" }
+      request = ActionDispatch::Request.new(env)
+      guardian = Guardian.new(guardian_user, request)
+      BasicTopicSerializer.new(topic, scope: guardian)
+    end
+
+    it "does not replace fancy_title with translation when experimental_topic_translation is disabled" do
+      SiteSetting.experimental_topic_translation = false
+      topic.set_translation("ja", jap_title)
+
+      expect(serialize_topic.fancy_title).to eq(topic.fancy_title)
+    end
+
+    it "does not replace fancy_title with translation when show_original param is present" do
+      topic.set_translation("ja", jap_title)
+      expect(serialize_topic(params: { "show" => "original" }).fancy_title).to eq(topic.fancy_title)
+    end
+
+    it "does not replace fancy_title with translation when no translation exists" do
+      expect(serialize_topic.fancy_title).to eq(topic.fancy_title)
+    end
+
+    it "does not replace fancy_title when topic is already in correct locale" do
+      I18n.locale = "ja"
+      topic.set_detected_locale("ja")
+      topic.set_translation("ja", jap_title)
+
+      expect(serialize_topic.fancy_title).to eq(topic.fancy_title)
+    end
+
+    it "returns translated title in fancy_title when translation exists for current locale" do
+      topic.set_translation("ja", jap_title)
+      expect(serialize_topic.fancy_title).to eq(jap_title)
+    end
+  end
+end

spec/serializers/post_serializer_spec.rb

@@ -88,31 +88,42 @@ def serialize_post(guardian_user: user, params: {})
       PostSerializer.new(post, scope: guardian)
     end
 
-    before { SiteSetting.experimental_topic_translation = true }
+    before do
+      SiteSetting.translator_enabled = true
+      SiteSetting.experimental_topic_translation = true
+    end
 
     it "does not return translated_cooked when experimental_topic_translation is disabled" do
       SiteSetting.experimental_topic_translation = false
-      expect(serialize_post.translated_cooked).to eq(nil)
+      expect(serialize_post.cooked).to eq(post.cooked)
     end
 
     it "does not return translated_cooked when show=original param is present" do
       I18n.locale = "ja"
       post.set_translation("ja", "こんにちは")
 
-      expect(serialize_post(params: { "show" => "original" }).translated_cooked).to eq(nil)
-      expect(serialize_post(params: { "show" => "derp" }).translated_cooked).to eq("こんにちは")
+      expect(serialize_post(params: { "show" => "original" }).cooked).to eq(post.cooked)
+      expect(serialize_post(params: { "show" => "derp" }).cooked).to eq("こんにちは")
+    end
+
+    it "does not return translated_cooked when post is already in correct locale" do
+      I18n.locale = "ja"
+      post.set_detected_locale("ja")
+      post.set_translation("ja", "こんにちは")
+
+      expect(serialize_post.cooked).to eq(post.cooked)
     end
 
     it "returns translated content based on locale" do
       I18n.locale = "ja"
       post.set_translation("ja", "こんにちは")
       post.set_translation("es", "Hola")
-      expect(serialize_post.translated_cooked).to eq("こんにちは")
+      expect(serialize_post.cooked).to eq("こんにちは")
     end
 
     it "does not return translated_cooked when plugin is disabled" do
       SiteSetting.translator_enabled = false
-      expect(serialize_post.translated_cooked).to eq(nil)
+      expect(serialize_post.cooked).to eq(post.cooked)
     end
   end
 end

spec/serializers/topic_view_serializer_spec.rb

@@ -31,7 +31,7 @@
     expect(topic_view.posts.first.association(:content_locale)).to be_loaded
   end
 
-  describe "#translated_title" do
+  describe "#fancy_title" do
     fab!(:user) { Fabricate(:user, locale: "ja") }
     fab!(:topic)
 
@@ -52,25 +52,33 @@ def serialize_topic(guardian_user: user, params: {})
       TopicViewSerializer.new(TopicView.new(topic), scope: guardian)
     end
 
-    it "does not return translated_title when experimental_topic_translation is disabled" do
+    it "does not replace fancy_title with translation when experimental_topic_translation is disabled" do
       SiteSetting.experimental_topic_translation = false
       topic.set_translation("ja", jap_title)
 
-      expect(serialize_topic.translated_title).to eq(nil)
+      expect(serialize_topic.fancy_title).to eq(topic.fancy_title)
     end
 
-    it "does not return translated_title when show_original param is present" do
+    it "does not replace fancy_title with translation when show_original param is present" do
       topic.set_translation("ja", jap_title)
-      expect(serialize_topic(params: { "show" => "original" }).translated_title).to eq(nil)
+      expect(serialize_topic(params: { "show" => "original" }).fancy_title).to eq(topic.fancy_title)
     end
 
-    it "does not return translated_title when no translation exists" do
-      expect(serialize_topic.translated_title).to eq(nil)
+    it "does not replace fancy_title with translation when no translation exists" do
+      expect(serialize_topic.fancy_title).to eq(topic.fancy_title)
     end
 
-    it "returns translated title when translation exists for current locale" do
+    it "does not replace fancy_title when topic is already in correct locale" do
+      I18n.locale = "ja"
+      topic.set_detected_locale("ja")
+      topic.set_translation("ja", jap_title)
+
+      expect(serialize_topic.fancy_title).to eq(topic.fancy_title)
+    end
+
+    it "returns translated title in fancy_title when translation exists for current locale" do
       topic.set_translation("ja", jap_title)
-      expect(serialize_topic.translated_title).to eq(jap_title)
+      expect(serialize_topic.fancy_title).to eq(jap_title)
     end
   end
 end