FIX: 'Show original' button only shows in topics where there are translated content

Author: nattsw

app/models/concerns/discourse_translator/translatable.rb

@@ -30,7 +30,10 @@ def set_translation(locale, text)
 
     def translation_for(locale)
       locale = locale.to_s.gsub("_", "-")
-      translations.find_by(locale: locale)&.translation
+      # this is a tricky perf balancing act when loading translations for a topic with many posts.
+      # the topic_view_serializer includes(:translations) for posts in the topic,
+      # the alternative is to do a find_by(locale: locale) which would result in a query per post.
+      translations.to_a.find { |t| t.locale == locale }&.translation
     end
 
     def detected_locale

assets/javascripts/discourse/components/show-original-content.gjs

@@ -6,6 +6,10 @@ import DButton from "discourse/components/d-button";
 import concatClass from "discourse/helpers/concat-class";
 
 export default class ShowOriginalContent extends Component {
+  static shouldRender(args) {
+    return args.topic.is_translated;
+  }
+
   @service router;
   @tracked isTranslated = true;
 

lib/discourse_translator/inline_translation.rb

@@ -37,6 +37,18 @@ def inject(plugin)
             &.then { |t| Topic.fancy_title(t) }
         end
       end
+
+      plugin.add_to_serializer(
+        :basic_post,
+        :is_translated,
+        include_condition: -> { SiteSetting.experimental_inline_translation },
+      ) { object.translation_for(I18n.locale).present? }
+
+      plugin.add_to_serializer(
+        :topic_view,
+        :is_translated,
+        include_condition: -> { SiteSetting.experimental_inline_translation },
+      ) { object.topic.translations.present? || object.posts.any? { |p| p.translations.present? } }
     end
   end
 end

lib/discourse_translator/topic_view_serializer_extension.rb

@@ -4,7 +4,10 @@ module DiscourseTranslator
   module TopicViewSerializerExtension
     def posts
       if SiteSetting.translator_enabled?
-        object.instance_variable_set(:@posts, object.posts.includes(:content_locale))
+        posts_query = object.posts.includes(:content_locale)
+        posts_query =
+          posts_query.includes(:translations) if SiteSetting.experimental_inline_translation
+        object.instance_variable_set(:@posts, posts_query)
       end
       super
     end

spec/serializers/post_serializer_spec.rb

@@ -56,6 +56,45 @@
     end
   end
 
+  describe "#is_translated" do
+    fab!(:post)
+
+    it "returns false when translator disabled" do
+      SiteSetting.translator_enabled = false
+      serializer = PostSerializer.new(post, scope: Guardian.new)
+
+      expect(serializer.is_translated).to eq(false)
+    end
+
+    it "returns false when experimental inline translation disabled" do
+      SiteSetting.translator_enabled = true
+      SiteSetting.experimental_inline_translation = false
+      serializer = PostSerializer.new(post, scope: Guardian.new)
+
+      expect(serializer.is_translated).to eq(false)
+    end
+
+    it "returns true when there is a translation for the current locale" do
+      SiteSetting.translator_enabled = true
+      SiteSetting.experimental_inline_translation = true
+      I18n.locale = "ja"
+      post.set_translation("ja", "こんにちは")
+      serializer = PostSerializer.new(post, scope: Guardian.new)
+
+      expect(serializer.is_translated).to eq(true)
+    end
+
+    it "returns false when there is no translation for the current locale" do
+      SiteSetting.translator_enabled = true
+      SiteSetting.experimental_inline_translation = true
+      I18n.locale = "ja"
+      post.set_translation("es", "Hola")
+      serializer = PostSerializer.new(post, scope: Guardian.new)
+
+      expect(serializer.is_translated).to eq(false)
+    end
+  end
+
   describe "#cooked" do
     def serialize_post(guardian_user: user, params: {})
       env = { "action_dispatch.request.parameters" => params, "REQUEST_METHOD" => "GET" }

spec/serializers/topic_view_serializer_spec.rb

@@ -3,39 +3,75 @@
 require "rails_helper"
 
 describe TopicViewSerializer do
-  fab!(:user)
   fab!(:topic)
-  fab!(:post1) { Fabricate(:post, topic: topic).set_detected_locale("en") }
-  fab!(:post2) { Fabricate(:post, topic: topic).set_detected_locale("es") }
-  fab!(:post3) { Fabricate(:post, topic: topic).set_detected_locale("ja") }
 
   before do
     SiteSetting.translator_enabled = true
     SiteSetting.restrict_translation_by_group = "#{Group::AUTO_GROUPS[:everyone]}"
     SiteSetting.restrict_translation_by_poster_group = "#{Group::AUTO_GROUPS[:everyone]}"
   end
 
-  it "preloads translations without N+1 queries" do
-    topic_view = TopicView.new(topic)
-    serializer = TopicViewSerializer.new(topic_view, scope: Guardian.new(user), root: false)
+  describe "preloading" do
+    fab!(:user)
+    fab!(:en_post) do
+      post = Fabricate(:post, topic: topic)
+      post.set_detected_locale("en")
+      post
+    end
+    fab!(:es_post) do
+      post = Fabricate(:post, topic: topic)
+      post.set_detected_locale("es")
+      post
+    end
+    fab!(:ja_post) do
+      post = Fabricate(:post, topic: topic)
+      post.set_detected_locale("ja")
+      post
+    end
+
+    it "preloads locale without N+1 queries" do
+      topic_view = TopicView.new(topic)
+      serializer = TopicViewSerializer.new(topic_view, scope: Guardian.new(user), root: false)
+
+      # ensure translation data is included in the JSON
+      json = {}
+      queries = track_sql_queries { json = serializer.as_json }
+      posts_json = json[:post_stream][:posts]
+      expect(posts_json.map { |p| p[:can_translate] }).to eq([false, true, true])
+
+      translation_queries = queries.count { |q| q.include?("discourse_translator_post_locales") }
+      expect(translation_queries).to eq(1) # would be 3 (posts) if not preloaded
+
+      expect(topic_view.posts.first.association(:content_locale)).to be_loaded
+    end
 
-    # ensure translation data is included in the JSON
-    json = {}
-    queries = track_sql_queries { json = serializer.as_json }
-    posts_json = json[:post_stream][:posts]
-    expect(posts_json.map { |p| p[:can_translate] }).to eq([false, true, true])
+    it "preloads translations when experimental_inline_translation is enabled" do
+      SiteSetting.experimental_inline_translation = true
+
+      en_post.set_translation("es", "Hola")
+      es_post.set_translation("en", "Hello")
+
+      topic_view = TopicView.new(topic)
+      serializer = TopicViewSerializer.new(topic_view, scope: Guardian.new(user), root: false)
+
+      topic_view.posts.reload
 
-    translation_queries = queries.count { |q| q.include?("discourse_translator_post_locales") }
-    expect(translation_queries).to eq(1) # would be 3 (posts) if not preloaded
+      queries =
+        track_sql_queries do
+          json = serializer.as_json
+          json[:post_stream][:posts].each { |p| p[:translations] }
+        end
 
-    expect(topic_view.posts.first.association(:content_locale)).to be_loaded
+      translation_queries =
+        queries.count { |q| q.include?("discourse_translator_post_translations") }
+      expect(translation_queries).to eq(1)
+      expect(topic_view.posts.first.association(:translations)).to be_loaded
+    end
   end
 
   describe "#fancy_title" do
     fab!(:user) { Fabricate(:user, locale: "ja") }
-    fab!(:topic)
 
-    let!(:guardian) { Guardian.new(user) }
     let!(:original_title) { "<h1>FUS ROH DAAHHH</h1>" }
     let!(:jap_title) { "<h1>フス・ロ・ダ・ア</h1>" }
 
@@ -81,4 +117,39 @@ def serialize_topic(guardian_user: user, params: {})
       expect(serialize_topic.fancy_title).to eq("&lt;h1&gt;フス・ロ・ダ・ア&lt;/h1&gt;")
     end
   end
+
+  describe "#is_translated" do
+    fab!(:user)
+
+    def serialize_topic(guardian_user: user)
+      TopicViewSerializer.new(TopicView.new(topic), scope: Guardian.new(guardian_user))
+    end
+
+    it "returns false when translator is disabled or experimental inline translation is disabled" do
+      SiteSetting.translator_enabled = true
+      SiteSetting.experimental_inline_translation = true
+      I18n.locale = "ja"
+      Fabricate(:post, topic: topic)
+
+      expect(serialize_topic.is_translated).to eq(false)
+    end
+
+    it "returns true when there is translation for the topic" do
+      SiteSetting.translator_enabled = true
+      SiteSetting.experimental_inline_translation = true
+      I18n.locale = "ja"
+      topic.set_translation("ja", "こんにちは")
+
+      expect(serialize_topic.is_translated).to eq(true)
+    end
+
+    it "returns true when there is translation for a post in the topic" do
+      SiteSetting.translator_enabled = true
+      SiteSetting.experimental_inline_translation = true
+      I18n.locale = "ja"
+      Fabricate(:post, topic: topic).set_translation("ja", "こんにちは")
+
+      expect(serialize_topic.is_translated).to eq(true)
+    end
+  end
 end

spec/system/full_page_translation_spec.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-RSpec.describe "Full page translation", type: :system do
+RSpec.describe "Inline translation", type: :system do
   fab!(:japanese_user) { Fabricate(:user, locale: "ja") }
   fab!(:site_local_user) { Fabricate(:user, locale: "en") }
   fab!(:author) { Fabricate(:user) }
@@ -51,6 +51,14 @@
       visit("/")
       visit("/t/#{topic.id}")
       expect(topic_page.has_topic_title?("孫子兵法からの人生戦略")).to eq(true)
+
+      page.find(".discourse-translator_toggle-original button").click
+      expect(page).to have_current_path(/.*show=original.*/)
+
+      expect(topic_page.has_topic_title?("Life strategies from The Art of War")).to eq(true)
+      expect(find(topic_page.post_by_number_selector(1))).to have_content(
+        "The masterpiece isn’t just about military strategy",
+      )
     end
   end
 end