DEV: Immediately send post for language detection for dual-text translations (#226)

Not too long ago, we moved language detections to a scheduled job. (see internal /t/134867/21)

But the new inline translation feature (when turned on) sends all content for translation the moment it is created. So far, we are not seeing an issue with load for this.

This commit reverts the batching for language detection to the original way, in-line with the new translation feature.

This commit also does a bit of cleanup - introducing three files to encapsulate the plugin event hooks into their own functionality (automatic translation, inline translation, dual-text translation)

This changes here will help to simplify the problem of 🌐 showing up in the post briefly, but in a subsequent commit.

Author: nattsw

app/jobs/regular/detect_translatable_language.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module ::Jobs
+  class DetectTranslatableLanguage < ::Jobs::Base
+    def execute(args)
+      return unless SiteSetting.translator_enabled
+
+      return if !%w[Post Topic].include?(args[:type])
+      return if !args[:translatable_id].is_a?(Integer)
+
+      translatable = args[:type].constantize.find_by(id: args[:translatable_id])
+      return if translatable.blank?
+      begin
+        translator = "DiscourseTranslator::#{SiteSetting.translator}".constantize
+        translator.detect(translatable)
+      rescue ::DiscourseTranslator::ProblemCheckedTranslationError
+        # problem-checked translation errors gracefully
+      end
+    end
+  end
+end

app/jobs/scheduled/detect_posts_language.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+module DiscourseTranslator
+  class AutomaticTranslations
+    def inject(plugin)
+      plugin.on(:post_process_cooked) do |_, post|
+        if SiteSetting.automatic_translation_target_languages.present? && post.user_id > 0
+          Jobs.enqueue(:translate_translatable, type: "Post", translatable_id: post.id)
+        end
+      end
+
+      plugin.on(:topic_created) do |topic|
+        if SiteSetting.automatic_translation_target_languages.present? && topic.user_id > 0
+          Jobs.enqueue(:translate_translatable, type: "Topic", translatable_id: topic.id)
+        end
+      end
+
+      plugin.on(:topic_edited) do |topic|
+        if SiteSetting.automatic_translation_target_languages.present? && topic.user_id > 0
+          Jobs.enqueue(:translate_translatable, type: "Topic", translatable_id: topic.id)
+        end
+      end
+    end
+  end
+end

lib/discourse_translator/automatic_translations.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module DiscourseTranslator
+  class DualTextTranslation
+    def inject(plugin)
+      # in dual-text translations,
+      # we don't want to send the post for detection if automatic translation already happens,
+      # as automatic translations send content for language detection as a side effect of translating
+
+      plugin.on(:post_process_cooked) do |_, post|
+        if SiteSetting.automatic_translation_target_languages.blank? &&
+             Guardian.new.can_detect_language?(post) && post.user_id > 0
+          Jobs.enqueue(:detect_translatable_language, type: "Post", translatable_id: post.id)
+        end
+      end
+
+      plugin.on(:topic_created) do |topic|
+        if SiteSetting.automatic_translation_target_languages.blank? && topic.user_id > 0
+          Jobs.enqueue(:detect_translatable_language, type: "Topic", translatable_id: topic.id)
+        end
+      end
+    end
+  end
+end

lib/discourse_translator/dual_text_translation.rb

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+module DiscourseTranslator
+  class InlineTranslation
+    def inject(plugin)
+      plugin.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
+      end
+
+      plugin.register_modifier(:topic_serializer_fancy_title) do |fancy_title, serializer|
+        if !SiteSetting.experimental_topic_translation ||
+             serializer.scope&.request&.params&.[]("show") == "original" ||
+             serializer.object.locale_matches?(I18n.locale)
+          fancy_title
+        else
+          serializer.object.translation_for(I18n.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_topic_translation ||
+             serializer.scope&.request&.params&.[]("show") == "original" ||
+             serializer.object.topic.locale_matches?(I18n.locale)
+          fancy_title
+        else
+          serializer
+            .object
+            .topic
+            .translation_for(I18n.locale)
+            .presence
+            &.then { |t| Topic.fancy_title(t) }
+        end
+      end
+    end
+  end
+end

lib/discourse_translator/inline_translation.rb

@@ -32,66 +32,12 @@ module ::DiscourseTranslator
     TopicViewSerializer.prepend(DiscourseTranslator::TopicViewSerializerExtension)
   end
 
-  on(:post_process_cooked) do |_, post|
-    if Guardian.new.can_detect_language?(post) && post.user_id > 0
-      Discourse.redis.sadd?(DiscourseTranslator::LANG_DETECT_NEEDED, post.id)
-    end
-  end
-
-  on(:post_process_cooked) do |_, post|
-    if SiteSetting.automatic_translation_target_languages.present? && post.user_id > 0
-      Jobs.enqueue(:translate_translatable, type: "Post", translatable_id: post.id)
-    end
-  end
-
-  on(:topic_created) do |topic|
-    if SiteSetting.automatic_translation_target_languages.present? && topic.user_id > 0
-      Jobs.enqueue(:translate_translatable, type: "Topic", translatable_id: topic.id)
-    end
-  end
-
-  on(:topic_edited) do |topic|
-    if SiteSetting.automatic_translation_target_languages.present? && topic.user_id > 0
-      Jobs.enqueue(:translate_translatable, type: "Topic", translatable_id: topic.id)
-    end
-  end
-
   add_to_serializer :post, :can_translate do
     scope.can_translate?(object)
   end
 
-  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
-  end
-
-  register_modifier(:topic_serializer_fancy_title) do |fancy_title, serializer|
-    if !SiteSetting.experimental_topic_translation ||
-         serializer.scope&.request&.params&.[]("show") == "original" ||
-         serializer.object.locale_matches?(I18n.locale)
-      fancy_title
-    else
-      serializer.object.translation_for(I18n.locale).presence&.then { |t| Topic.fancy_title(t) }
-    end
-  end
+  DiscourseTranslator::DualTextTranslation.new.inject(self)
+  DiscourseTranslator::InlineTranslation.new.inject(self)
 
-  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.locale_matches?(I18n.locale)
-      fancy_title
-    else
-      serializer
-        .object
-        .topic
-        .translation_for(I18n.locale)
-        .presence
-        &.then { |t| Topic.fancy_title(t) }
-    end
-  end
+  DiscourseTranslator::AutomaticTranslations.new.inject(self)
 end

plugin.rb

@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+require "aws-sdk-translate"
+
+describe Jobs::DetectTranslatableLanguage do
+  fab!(:post)
+  fab!(:topic)
+  let!(:job) { Jobs::DetectTranslatableLanguage.new }
+
+  before do
+    SiteSetting.translator_enabled = true
+    SiteSetting.translator = "Amazon"
+    client = Aws::Translate::Client.new(stub_responses: true)
+    client.stub_responses(
+      :translate_text,
+      { translated_text: "大丈夫", source_language_code: "en", target_language_code: "jp" },
+    )
+    Aws::Translate::Client.stubs(:new).returns(client)
+  end
+
+  it "does nothing when type is not post or topic" do
+    expect { job.execute(type: "X", translatable_id: 1) }.not_to raise_error
+  end
+
+  it "does nothing when id is not int" do
+    expect { job.execute(type: "Post", translatable_id: "A") }.not_to raise_error
+  end
+
+  it "updates detected locale" do
+    job.execute(type: "Post", translatable_id: post.id)
+    job.execute(type: "Topic", translatable_id: topic.id)
+
+    expect(post.detected_locale).not_to be_nil
+    expect(topic.detected_locale).not_to be_nil
+  end
+
+  it "does not update detected locale the translator is disabled" do
+    SiteSetting.translator_enabled = false
+
+    job.execute(type: "Post", translatable_id: post.id)
+    job.execute(type: "Topic", translatable_id: topic.id)
+
+    expect(post.detected_locale).to be_nil
+    expect(topic.detected_locale).to be_nil
+  end
+
+  it "skips content that no longer exist" do
+    non_existent_id = -1
+
+    expect { job.execute(type: "Post", translatable_id: non_existent_id) }.not_to raise_error
+    expect { job.execute(type: "Topic", translatable_id: non_existent_id) }.not_to raise_error
+  end
+end

spec/jobs/detect_posts_language_spec.rb

@@ -73,23 +73,19 @@
     fab!(:topic)
     fab!(:user) { Fabricate(:user, groups: [group]) }
 
-    before { Jobs.run_immediately! }
-
     it "queues the post for language detection when user and posts are in the right group" do
       SiteSetting.restrict_translation_by_poster_group = "#{group.id}"
-      post =
-        PostCreator.new(
-          user,
-          {
-            title: "a topic about cats",
-            raw: "tomtom is a cat",
-            category: Fabricate(:category).id,
-          },
-        ).create
 
-      expect(
-        Discourse.redis.sismember(DiscourseTranslator::LANG_DETECT_NEEDED, post.id),
-      ).to be_truthy
+      post = Fabricate(:post, user: user)
+      CookedPostProcessor.new(post).post_process
+
+      expect_job_enqueued(
+        job: :detect_translatable_language,
+        args: {
+          type: "Post",
+          translatable_id: post.id,
+        },
+      )
     end
 
     it "does not queue bot posts for language detection" do