FIX: 'Show original' button only shows in topics where there are translated content (#240)

Currently, the button above shows up even when there are no posts translated. With this commit, we will not show the button if there are zero posts in the topic that is translated.

This commit also
- only returns translations of supported languages (in SiteSetting.automatic_translation_target_languages)
- optimises loading translations when the feature is enabled under certain circumstances

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

app/models/discourse_translator/post_translation.rb

@@ -10,10 +10,6 @@ class PostTranslation < ActiveRecord::Base
     validates :locale, presence: true
     validates :translation, presence: true
     validates :locale, uniqueness: { scope: :post_id }
-
-    def self.translation_for(post_id, locale)
-      find_by(post_id: post_id, locale: locale)&.translation
-    end
   end
 end
 

app/models/discourse_translator/topic_translation.rb

@@ -10,10 +10,6 @@ class TopicTranslation < ActiveRecord::Base
     validates :locale, presence: true
     validates :translation, presence: true
     validates :locale, uniqueness: { scope: :topic_id }
-
-    def self.translation_for(topic_id, locale)
-      find_by(topic_id: topic_id, locale: locale)&.translation
-    end
   end
 end
 

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

@@ -6,6 +6,13 @@ 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 ||
+      args.topic.postStream.posts.some(({ is_translated }) => is_translated)
+    );
+  }
+
   @service router;
   @tracked isTranslated = true;
 

lib/discourse_translator/guardian_extension.rb

@@ -24,15 +24,17 @@ def can_detect_language?(post)
   def can_translate?(post)
     return false if post.user&.bot?
     return false if !user_group_allow_translate?
-    return false if post.locale_matches?(I18n.locale)
 
     # we want to return false if the post is created within a short buffer ago,
     # this prevents the 🌐from appearing and then disappearing if the lang is same as user's lang
     return false if post.created_at > POST_DETECTION_BUFFER.ago && post.detected_locale.blank?
 
     if SiteSetting.experimental_inline_translation
-      post.translation_for(I18n.locale).nil?
+      locale = DiscourseTranslator::InlineTranslation.effective_locale
+      return false if post.locale_matches?(locale)
+      post.translation_for(locale).nil?
     else
+      return false if post.locale_matches?(I18n.locale)
       poster_group_allow_translate?(post)
     end
   end

lib/discourse_translator/inline_translation.rb

@@ -2,41 +2,69 @@
 
 module DiscourseTranslator
   class InlineTranslation
+    def self.effective_locale
+      if LocaleMatcher.user_locale_is_default? || LocaleMatcher.user_locale_in_target_languages?
+        I18n.locale
+      else
+        SiteSetting.default_locale
+      end
+    end
+
     def inject(plugin)
+      # since locales are eager loaded but translations may not,
+      # always return early if topic and posts are in the user's effective_locale.
+      # this prevents the need to load translations.
+
       plugin.register_modifier(:basic_post_serializer_cooked) do |cooked, serializer|
         if !SiteSetting.experimental_inline_translation ||
-             serializer.scope&.request&.params&.[]("show") == "original" ||
-             serializer.object.detected_locale == I18n.locale.to_s.gsub("_", "-")
+             serializer.object.locale_matches?(InlineTranslation.effective_locale) ||
+             serializer.scope&.request&.params&.[]("show") == "original"
           cooked
         else
-          serializer.object.translation_for(I18n.locale).presence
+          serializer.object.translation_for(InlineTranslation.effective_locale).presence
         end
       end
 
       plugin.register_modifier(:topic_serializer_fancy_title) do |fancy_title, serializer|
         if !SiteSetting.experimental_inline_translation ||
-             serializer.scope&.request&.params&.[]("show") == "original" ||
-             serializer.object.locale_matches?(I18n.locale)
+             serializer.object.locale_matches?(InlineTranslation.effective_locale) ||
+             serializer.scope&.request&.params&.[]("show") == "original"
           fancy_title
         else
-          serializer.object.translation_for(I18n.locale).presence&.then { |t| Topic.fancy_title(t) }
+          serializer
+            .object
+            .translation_for(InlineTranslation.effective_locale)
+            .presence
+            &.then { |t| Topic.fancy_title(t) }
         end
       end
 
       plugin.register_modifier(:topic_view_serializer_fancy_title) do |fancy_title, serializer|
         if !SiteSetting.experimental_inline_translation ||
-             serializer.scope&.request&.params&.[]("show") == "original" ||
-             serializer.object.topic.locale_matches?(I18n.locale)
+             serializer.object.topic.locale_matches?(InlineTranslation.effective_locale) ||
+             serializer.scope&.request&.params&.[]("show") == "original"
           fancy_title
         else
           serializer
             .object
             .topic
-            .translation_for(I18n.locale)
+            .translation_for(InlineTranslation.effective_locale)
             .presence
             &.then { |t| Topic.fancy_title(t) }
         end
       end
+
+      plugin.add_to_serializer(:basic_post, :is_translated) do
+        SiteSetting.experimental_inline_translation &&
+          !object.locale_matches?(InlineTranslation.effective_locale) &&
+          object.translation_for(InlineTranslation.effective_locale).present?
+      end
+
+      plugin.add_to_serializer(:topic_view, :is_translated) do
+        SiteSetting.experimental_inline_translation &&
+          !object.topic.locale_matches?(InlineTranslation.effective_locale) &&
+          object.topic.translation_for(InlineTranslation.effective_locale).present?
+      end
     end
   end
 end

lib/discourse_translator/locale_matcher.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module DiscourseTranslator
+  class LocaleMatcher
+    def self.user_locale_in_target_languages?
+      # e.g. "en|es|pt_BR" vs :en_UK
+      SiteSetting.automatic_translation_target_languages.split("|").include?(I18n.locale.to_s)
+    end
+
+    def self.user_locale_is_default?
+      # e.g. :en_UK vs "en_UK"
+      I18n.locale.to_s == SiteSetting.default_locale
+    end
+  end
+end

lib/discourse_translator/topic_view_serializer_extension.rb

@@ -4,7 +4,20 @@ 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)
+        # this is kind of a micro-optimization.
+        # we do not want to eager load translations if the user is using the site's language.
+        # we will only load them if the user is using a different language that is supported by the site.
+        if SiteSetting.experimental_inline_translation && !LocaleMatcher.user_locale_is_default? &&
+             LocaleMatcher.user_locale_in_target_languages?
+          locale = InlineTranslation.effective_locale.to_s.gsub("_", "-")
+          posts_query =
+            posts_query
+              .includes(:translations)
+              .references(:translations)
+              .where(translations: { locale: [nil, locale] })
+        end
+        object.instance_variable_set(:@posts, posts_query)
       end
       super
     end

spec/lib/guardian_extension_spec.rb

@@ -177,7 +177,12 @@
       end
 
       describe "when experimental_inline_translation enabled" do
-        before { SiteSetting.experimental_inline_translation = true }
+        before do
+          SiteSetting.experimental_inline_translation = true
+
+          SiteSetting.automatic_translation_backfill_maximum_translations_per_hour = 1
+          SiteSetting.automatic_translation_target_languages = "pt"
+        end
 
         describe "logged in user" do
           it "cannot translate when user is not in restrict_translation_by_group" do
@@ -190,7 +195,7 @@
             before { SiteSetting.restrict_translation_by_group = "#{group.id}" }
 
             it "cannot translate when post has translation for user locale" do
-              post.set_detected_locale("jp")
+              post.set_detected_locale("ja")
               post.set_translation("pt", "Olá, mundo!")
 
               expect(guardian.can_translate?(post)).to eq(false)

spec/serializers/basic_topic_serializer_spec.rb

@@ -20,6 +20,9 @@
       topic.title = original_title
       SiteSetting.experimental_inline_translation = true
       I18n.locale = "ja"
+
+      SiteSetting.automatic_translation_backfill_maximum_translations_per_hour = 1
+      SiteSetting.automatic_translation_target_languages = "ja"
     end
 
     def serialize_topic(guardian_user: user, params: {})
@@ -53,6 +56,14 @@ def serialize_topic(guardian_user: user, params: {})
       expect(serialize_topic.fancy_title).to eq(topic.fancy_title)
     end
 
+    it "does not replace fancy_title when user's locale is not in target languages" do
+      I18n.locale = "es"
+      topic.set_detected_locale("en")
+      topic.set_translation("es", 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("<h1>フス・ロ・ダ・ア</h1>")