FIX: When allowing private content translation, only translate group PMs and not personal PMs (#1432)

We want to avoid translating PMs that are not group PMs. This condition is applied when `SiteSetting.ai_translation_backfill_limit_to_public_content = false`

app/jobs/regular/detect_translate_post.rb

@@ -12,10 +12,14 @@ module Jobs
       post = Post.find_by(id: args[:post_id])
       return if post.blank? || post.raw.blank? || post.deleted_at.present? || post.user_id <= 0
 
+      topic = post.topic
+      return if topic.blank?
+
       if SiteSetting.ai_translation_backfill_limit_to_public_content
-        topic = post.topic
+        return if topic.category&.read_restricted? || topic.archetype == Archetype.private_message
-        if topic.blank? || topic.category&.read_restricted? ||
+      else
-             topic.archetype == Archetype.private_message
+        if topic.archetype == Archetype.private_message &&
+             !TopicAllowedGroup.exists?(topic_id: topic.id)
           return
         end
       end

app/jobs/regular/detect_translate_topic.rb

@@ -15,7 +15,12 @@ module Jobs
       end
 
       if SiteSetting.ai_translation_backfill_limit_to_public_content
-        return if topic.category&.read_restricted?
+        return if topic.category&.read_restricted? || topic.archetype == Archetype.private_message
+      else
+        if topic.archetype == Archetype.private_message &&
+             !TopicAllowedGroup.exists?(topic_id: topic.id)
+          return
+        end
       end
 
       if (detected_locale = topic.locale).blank?

app/jobs/regular/localize_posts.rb

@@ -29,12 +29,22 @@ module Jobs
             .where.not(locale: locale)
             .where("pl.id IS NULL")
 
+        posts = posts.joins(:topic)
+
         if SiteSetting.ai_translation_backfill_limit_to_public_content
+          # exclude all PMs
+          # and only include posts from public categories
           posts =
             posts
-              .joins(:topic)
-              .where(topics: { category_id: Category.where(read_restricted: false).select(:id) })
               .where.not(topics: { archetype: Archetype.private_message })
+              .where(topics: { category_id: Category.where(read_restricted: false).select(:id) })
+        else
+          # all regular topics, and group PMs
+          posts =
+            posts.where(
+              "topics.archetype != ? OR topics.id IN (SELECT topic_id FROM topic_allowed_groups)",
+              Archetype.private_message,
+            )
         end
 
         if SiteSetting.ai_translation_backfill_max_age_days > 0

app/jobs/regular/localize_topics.rb

@@ -29,7 +29,19 @@ module Jobs
             .where("tl.id IS NULL")
 
         if SiteSetting.ai_translation_backfill_limit_to_public_content
-          topics = topics.where(category_id: Category.where(read_restricted: false).select(:id))
+          # exclude all PMs
+          # and only include posts from public categories
+          topics =
+            topics
+              .where.not(archetype: Archetype.private_message)
+              .where(category_id: Category.where(read_restricted: false).select(:id))
+        else
+          # all regular topics, and group PMs
+          topics =
+            topics.where(
+              "topics.archetype != ? OR topics.id IN (SELECT topic_id FROM topic_allowed_groups)",
+              Archetype.private_message,
+            )
         end
 
         if SiteSetting.ai_translation_backfill_max_age_days > 0

spec/jobs/regular/detect_translate_post_spec.rb

@@ -88,16 +88,78 @@ describe Jobs::DetectTranslatePost do
     expect { job.execute({ post_id: post.id }) }.not_to raise_error
   end
 
-  it "skips public content when `ai_translation_backfill_limit_to_public_content ` site setting is enabled" do
+  describe "with public content and PM limitations" do
-    SiteSetting.ai_translation_backfill_limit_to_public_content = true
+    fab!(:private_category) { Fabricate(:private_category, group: Group[:staff]) }
-    post.topic.category.update!(read_restricted: true)
+    fab!(:private_topic) { Fabricate(:topic, category: private_category) }
+    fab!(:private_post) { Fabricate(:post, topic: private_topic) }
 
-    DiscourseAi::Translation::PostLocaleDetector.expects(:detect_locale).with(post).never
+    fab!(:personal_pm_topic) { Fabricate(:private_message_topic) }
-    DiscourseAi::Translation::PostLocalizer.expects(:localize).never
+    fab!(:personal_pm_post) { Fabricate(:post, topic: personal_pm_topic) }
 
-    job.execute({ post_id: post.id })
+    fab!(:group_pm_topic) do
+      Fabricate(:group_private_message_topic, recipient_group: Fabricate(:group))
+    end
+    fab!(:group_pm_post) { Fabricate(:post, topic: group_pm_topic) }
 
-    pm_post = Fabricate(:post, topic: Fabricate(:private_message_topic))
+    context "when ai_translation_backfill_limit_to_public_content is true" do
-    job.execute({ post_id: pm_post.id })
+      before { SiteSetting.ai_translation_backfill_limit_to_public_content = true }
+
+      it "skips posts from restricted categories and PMs" do
+        DiscourseAi::Translation::PostLocaleDetector
+          .expects(:detect_locale)
+          .with(private_post)
+          .never
+        DiscourseAi::Translation::PostLocalizer
+          .expects(:localize)
+          .with(private_post, any_parameters)
+          .never
+        job.execute({ post_id: private_post.id })
+
+        DiscourseAi::Translation::PostLocaleDetector
+          .expects(:detect_locale)
+          .with(personal_pm_post)
+          .never
+        DiscourseAi::Translation::PostLocalizer
+          .expects(:localize)
+          .with(personal_pm_post, any_parameters)
+          .never
+        job.execute({ post_id: personal_pm_post.id })
+
+        DiscourseAi::Translation::PostLocaleDetector
+          .expects(:detect_locale)
+          .with(group_pm_post)
+          .never
+        DiscourseAi::Translation::PostLocalizer
+          .expects(:localize)
+          .with(group_pm_post, any_parameters)
+          .never
+        job.execute({ post_id: group_pm_post.id })
+      end
+    end
+
+    context "when ai_translation_backfill_limit_to_public_content is false" do
+      before { SiteSetting.ai_translation_backfill_limit_to_public_content = false }
+
+      it "processes posts from private categories and group PMs but skips personal PMs" do
+        DiscourseAi::Translation::PostLocaleDetector.expects(:detect_locale).with(private_post).once
+        job.execute({ post_id: private_post.id })
+
+        DiscourseAi::Translation::PostLocaleDetector
+          .expects(:detect_locale)
+          .with(group_pm_post)
+          .once
+        job.execute({ post_id: group_pm_post.id })
+
+        DiscourseAi::Translation::PostLocaleDetector
+          .expects(:detect_locale)
+          .with(personal_pm_post)
+          .never
+        DiscourseAi::Translation::PostLocalizer
+          .expects(:localize)
+          .with(personal_pm_post, any_parameters)
+          .never
+        job.execute({ post_id: personal_pm_post.id })
+      end
+    end
   end
 end

spec/jobs/regular/detect_translate_topic_spec.rb

@@ -87,13 +87,80 @@ describe Jobs::DetectTranslateTopic do
     expect { job.execute({ topic_id: topic.id }) }.not_to raise_error
   end
 
-  it "skips public content when `ai_translation_backfill_limit_to_public_content ` site setting is enabled" do
+  describe "with public content and PM limitations" do
-    SiteSetting.ai_translation_backfill_limit_to_public_content = true
+    fab!(:private_category) { Fabricate(:private_category, group: Group[:staff]) }
-    topic.category.update!(read_restricted: true)
+    fab!(:private_topic) { Fabricate(:topic, category: private_category) }
 
-    DiscourseAi::Translation::TopicLocaleDetector.expects(:detect_locale).never
+    fab!(:personal_pm_topic) { Fabricate(:private_message_topic) }
-    DiscourseAi::Translation::TopicLocalizer.expects(:localize).never
 
-    job.execute({ topic_id: topic.id })
+    fab!(:group_pm_topic) do
+      Fabricate(:group_private_message_topic, recipient_group: Fabricate(:group))
+    end
+
+    context "when ai_translation_backfill_limit_to_public_content is true" do
+      before { SiteSetting.ai_translation_backfill_limit_to_public_content = true }
+
+      it "skips topics from restricted categories and PMs" do
+        DiscourseAi::Translation::TopicLocaleDetector
+          .expects(:detect_locale)
+          .with(private_topic)
+          .never
+        DiscourseAi::Translation::TopicLocalizer
+          .expects(:localize)
+          .with(private_topic, any_parameters)
+          .never
+        job.execute({ topic_id: private_topic.id })
+
+        # Skip personal PMs
+        DiscourseAi::Translation::TopicLocaleDetector
+          .expects(:detect_locale)
+          .with(personal_pm_topic)
+          .never
+        DiscourseAi::Translation::TopicLocalizer
+          .expects(:localize)
+          .with(personal_pm_topic, any_parameters)
+          .never
+        job.execute({ topic_id: personal_pm_topic.id })
+
+        DiscourseAi::Translation::TopicLocaleDetector
+          .expects(:detect_locale)
+          .with(group_pm_topic)
+          .never
+        DiscourseAi::Translation::TopicLocalizer
+          .expects(:localize)
+          .with(group_pm_topic, any_parameters)
+          .never
+
+        job.execute({ topic_id: group_pm_topic.id })
+      end
+    end
+
+    context "when ai_translation_backfill_limit_to_public_content is false" do
+      before { SiteSetting.ai_translation_backfill_limit_to_public_content = false }
+
+      it "processes topics from private categories and group PMs but skips personal PMs" do
+        DiscourseAi::Translation::TopicLocaleDetector
+          .expects(:detect_locale)
+          .with(private_topic)
+          .once
+        job.execute({ topic_id: private_topic.id })
+
+        DiscourseAi::Translation::TopicLocaleDetector
+          .expects(:detect_locale)
+          .with(group_pm_topic)
+          .once
+        job.execute({ topic_id: group_pm_topic.id })
+
+        DiscourseAi::Translation::TopicLocaleDetector
+          .expects(:detect_locale)
+          .with(personal_pm_topic)
+          .never
+        DiscourseAi::Translation::TopicLocalizer
+          .expects(:localize)
+          .with(personal_pm_topic, any_parameters)
+          .never
+        job.execute({ topic_id: personal_pm_topic.id })
+      end
+    end
   end
 end

spec/jobs/regular/localize_posts_spec.rb

@@ -127,36 +127,57 @@ describe Jobs::LocalizePosts do
     fab!(:private_topic) { Fabricate(:topic, category: private_category) }
     fab!(:private_post) { Fabricate(:post, topic: private_topic, locale: "es") }
 
-    fab!(:pm_post) { Fabricate(:post, topic: Fabricate(:private_message_topic), locale: "es") }
-
     fab!(:public_post) { Fabricate(:post, locale: "es") }
 
-    before do
+    fab!(:personal_pm_topic) { Fabricate(:private_message_topic) }
-      SiteSetting.ai_translation_backfill_limit_to_public_content = true
+    fab!(:personal_pm_post) { Fabricate(:post, topic: personal_pm_topic, locale: "es") }
-      SiteSetting.experimental_content_localization_supported_locales = "ja"
+
+    fab!(:group)
+    fab!(:group_pm_topic) { Fabricate(:group_private_message_topic, recipient_group: group) }
+    fab!(:group_pm_post) { Fabricate(:post, topic: group_pm_topic, locale: "es") }
+
+    before { SiteSetting.experimental_content_localization_supported_locales = "ja" }
+
+    context "when ai_translation_backfill_limit_to_public_content is true" do
+      before { SiteSetting.ai_translation_backfill_limit_to_public_content = true }
+
+      it "only processes posts from public categories" do
+        DiscourseAi::Translation::PostLocalizer.expects(:localize).with(public_post, "ja").once
+
+        DiscourseAi::Translation::PostLocalizer
+          .expects(:localize)
+          .with(private_post, any_parameters)
+          .never
+
+        DiscourseAi::Translation::PostLocalizer
+          .expects(:localize)
+          .with(personal_pm_post, any_parameters)
+          .never
+        DiscourseAi::Translation::PostLocalizer
+          .expects(:localize)
+          .with(group_pm_post, any_parameters)
+          .never
+
+        job.execute({})
+      end
     end
 
-    it "only processes posts from public categories" do
+    context "when ai_translation_backfill_limit_to_public_content is false" do
-      DiscourseAi::Translation::PostLocalizer.expects(:localize).with(public_post, "ja").once
+      before { SiteSetting.ai_translation_backfill_limit_to_public_content = false }
 
-      DiscourseAi::Translation::PostLocalizer
+      it "processes public posts and group PMs but not personal PMs" do
-        .expects(:localize)
+        DiscourseAi::Translation::PostLocalizer.expects(:localize).with(public_post, "ja").once
-        .with(private_post, any_parameters)
+        DiscourseAi::Translation::PostLocalizer.expects(:localize).with(private_post, "ja").once
-        .never
 
-      DiscourseAi::Translation::PostLocalizer.expects(:localize).with(pm_post, any_parameters).never
+        DiscourseAi::Translation::PostLocalizer.expects(:localize).with(group_pm_post, "ja").once
 
-      job.execute({})
+        DiscourseAi::Translation::PostLocalizer
-    end
+          .expects(:localize)
+          .with(personal_pm_post, any_parameters)
+          .never
 
-    it "processes all posts when setting is disabled" do
+        job.execute({})
-      SiteSetting.ai_translation_backfill_limit_to_public_content = false
+      end
-
-      DiscourseAi::Translation::PostLocalizer.expects(:localize).with(public_post, "ja").once
-      DiscourseAi::Translation::PostLocalizer.expects(:localize).with(pm_post, "ja").once
-      DiscourseAi::Translation::PostLocalizer.expects(:localize).with(private_post, "ja").once
-
-      job.execute({})
     end
   end
 

spec/jobs/regular/localize_topics_spec.rb

@@ -133,33 +133,56 @@ describe Jobs::LocalizeTopics do
     fab!(:private_topic) { Fabricate(:topic, category: private_category, locale: "es") }
     fab!(:public_topic) { Fabricate(:topic, locale: "es") }
 
-    before { SiteSetting.ai_translation_backfill_limit_to_public_content = true }
+    fab!(:personal_pm_topic) { Fabricate(:private_message_topic, locale: "es") }
 
-    it "only processes topics from public categories" do
+    fab!(:group_pm_topic) do
-      DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "en").once
+      Fabricate(:group_private_message_topic, recipient_group: Fabricate(:group), locale: "es")
-      DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "ja").once
-      DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "de").once
-
-      DiscourseAi::Translation::TopicLocalizer
-        .expects(:localize)
-        .with(private_topic, any_parameters)
-        .never
-
-      job.execute({})
     end
 
-    it "processes all topics when setting is disabled" do
+    before { SiteSetting.experimental_content_localization_supported_locales = "ja" }
-      SiteSetting.ai_translation_backfill_limit_to_public_content = false
 
-      DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "en").once
+    context "when ai_translation_backfill_limit_to_public_content is true" do
-      DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "ja").once
+      before { SiteSetting.ai_translation_backfill_limit_to_public_content = true }
-      DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "de").once
 
-      DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(private_topic, "en").once
+      it "only processes topics from public categories" do
-      DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(private_topic, "ja").once
+        DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "ja").once
-      DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(private_topic, "de").once
 
-      job.execute({})
+        DiscourseAi::Translation::TopicLocalizer
+          .expects(:localize)
+          .with(private_topic, any_parameters)
+          .never
+
+        DiscourseAi::Translation::TopicLocalizer
+          .expects(:localize)
+          .with(personal_pm_topic, any_parameters)
+          .never
+
+        DiscourseAi::Translation::TopicLocalizer
+          .expects(:localize)
+          .with(group_pm_topic, any_parameters)
+          .never
+
+        job.execute({})
+      end
+    end
+
+    context "when ai_translation_backfill_limit_to_public_content is false" do
+      before { SiteSetting.ai_translation_backfill_limit_to_public_content = false }
+
+      it "processes public topics, private topics and group PMs but not personal PMs" do
+        DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "ja").once
+
+        DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(private_topic, "ja").once
+
+        DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(group_pm_topic, "ja").once
+
+        DiscourseAi::Translation::TopicLocalizer
+          .expects(:localize)
+          .with(personal_pm_topic, any_parameters)
+          .never
+
+        job.execute({})
+      end
     end
   end