From 740be2662550c428b70a432e4ae7eaeb94d680b8 Mon Sep 17 00:00:00 2001 From: Natalie Tay Date: Mon, 23 Jun 2025 19:07:40 +0800 Subject: [PATCH] DEV: Also make sure locale detection skips PMs that are not group PMs when public content only (#1457) In the earlier PR https://github.com/discourse/discourse-ai/pull/1432, when `SiteSetting.ai_translation_backfill_limit_to_public_content = false`, we **translate** PMs but **skip translating** PMs that do not involve groups. This commit covers the missing case on **locale detection**. --- .../posts_locale_detection_backfill.rb | 11 ++++++-- .../topics_locale_detection_backfill.rb | 12 +++++++- .../posts_locale_detection_backfill_spec.rb | 27 ++++++++++++++---- .../topics_locale_detection_backfill_spec.rb | 28 ++++++++++++++++--- 4 files changed, 65 insertions(+), 13 deletions(-) diff --git a/app/jobs/scheduled/posts_locale_detection_backfill.rb b/app/jobs/scheduled/posts_locale_detection_backfill.rb index 71091efd..61a77b62 100644 --- a/app/jobs/scheduled/posts_locale_detection_backfill.rb +++ b/app/jobs/scheduled/posts_locale_detection_backfill.rb @@ -20,12 +20,17 @@ module Jobs .where.not(raw: [nil, ""]) if SiteSetting.ai_translation_backfill_limit_to_public_content - public_categories = Category.where(read_restricted: false).pluck(:id) posts = posts .joins(:topic) - .where(topics: { category_id: public_categories }) - .where(topics: { archetype: "regular" }) + .where(topics: { category_id: Category.where(read_restricted: false).select(:id) }) + .where("archetype != ?", Archetype.private_message) + else + posts = + posts.joins(:topic).where( + "topics.archetype != ? OR EXISTS (SELECT 1 FROM topic_allowed_groups WHERE topic_id = topics.id)", + Archetype.private_message, + ) end if SiteSetting.ai_translation_backfill_max_age_days > 0 diff --git a/app/jobs/scheduled/topics_locale_detection_backfill.rb b/app/jobs/scheduled/topics_locale_detection_backfill.rb index bed5f4e4..fe5858b7 100644 --- a/app/jobs/scheduled/topics_locale_detection_backfill.rb +++ b/app/jobs/scheduled/topics_locale_detection_backfill.rb @@ -15,7 +15,17 @@ module Jobs topics = Topic.where(locale: nil, deleted_at: nil).where("topics.user_id > 0") if SiteSetting.ai_translation_backfill_limit_to_public_content - topics = topics.where(category_id: Category.where(read_restricted: false).select(:id)) + topics = + topics.where(category_id: Category.where(read_restricted: false).select(:id)).where( + "archetype != ?", + Archetype.private_message, + ) + else + topics = + topics.where( + "archetype != ? OR EXISTS (SELECT 1 FROM topic_allowed_groups WHERE topic_id = topics.id)", + Archetype.private_message, + ) end if SiteSetting.ai_translation_backfill_max_age_days > 0 diff --git a/spec/jobs/scheduled/posts_locale_detection_backfill_spec.rb b/spec/jobs/scheduled/posts_locale_detection_backfill_spec.rb index 00693f9a..f78af32e 100644 --- a/spec/jobs/scheduled/posts_locale_detection_backfill_spec.rb +++ b/spec/jobs/scheduled/posts_locale_detection_backfill_spec.rb @@ -82,23 +82,40 @@ describe Jobs::PostsLocaleDetectionBackfill do describe "with public content limitation" do fab!(:private_category) { Fabricate(:private_category, group: Group[:staff]) } - fab!(:private_topic) { Fabricate(:topic, category: private_category) } - fab!(:private_post) { Fabricate(:post, topic: private_topic, locale: nil) } + fab!(:private_cat_topic) { Fabricate(:topic, category: private_category) } + fab!(:private_cat_post) { Fabricate(:post, topic: private_cat_topic, locale: nil) } + + fab!(:group) + fab!(:group_pm_topic) { Fabricate(:private_message_topic, allowed_groups: [group]) } + fab!(:group_pm_post) { Fabricate(:post, topic: group_pm_topic, locale: nil) } + + fab!(:pm_topic) { Fabricate(:private_message_topic) } + fab!(:pm_post) { Fabricate(:post, topic: pm_topic, locale: nil) } before { SiteSetting.ai_translation_backfill_limit_to_public_content = true } it "only processes posts from public categories" do DiscourseAi::Translation::PostLocaleDetector.expects(:detect_locale).with(post).once - DiscourseAi::Translation::PostLocaleDetector.expects(:detect_locale).with(private_post).never + DiscourseAi::Translation::PostLocaleDetector + .expects(:detect_locale) + .with(private_cat_post) + .never + DiscourseAi::Translation::PostLocaleDetector.expects(:detect_locale).with(group_pm_post).never + DiscourseAi::Translation::PostLocaleDetector.expects(:detect_locale).with(pm_post).never job.execute({}) end - it "processes all posts when setting is disabled" do + it "processes all public content and group PMs and private categories when setting is disabled" do SiteSetting.ai_translation_backfill_limit_to_public_content = false DiscourseAi::Translation::PostLocaleDetector.expects(:detect_locale).with(post).once - DiscourseAi::Translation::PostLocaleDetector.expects(:detect_locale).with(private_post).once + DiscourseAi::Translation::PostLocaleDetector + .expects(:detect_locale) + .with(private_cat_post) + .once + DiscourseAi::Translation::PostLocaleDetector.expects(:detect_locale).with(group_pm_post).once + DiscourseAi::Translation::PostLocaleDetector.expects(:detect_locale).with(pm_post).never job.execute({}) end diff --git a/spec/jobs/scheduled/topics_locale_detection_backfill_spec.rb b/spec/jobs/scheduled/topics_locale_detection_backfill_spec.rb index 3fbcb84f..285096cd 100644 --- a/spec/jobs/scheduled/topics_locale_detection_backfill_spec.rb +++ b/spec/jobs/scheduled/topics_locale_detection_backfill_spec.rb @@ -82,8 +82,14 @@ describe Jobs::TopicsLocaleDetectionBackfill do describe "with public content limitation" do fab!(:private_category) { Fabricate(:private_category, group: Group[:staff]) } + fab!(:private_cat_topic) { Fabricate(:topic, category: private_category, locale: nil) } + + fab!(:group) + fab!(:group_pm_topic) { Fabricate(:private_message_topic, allowed_groups: [group]) } + + fab!(:pm_topic) { Fabricate(:private_message_topic) } + fab!(:public_topic) { Fabricate(:topic, locale: nil) } - fab!(:private_topic) { Fabricate(:topic, category: private_category, locale: nil) } before do DiscourseAi::Translation::TopicLocaleDetector.expects(:detect_locale).at_least_once @@ -93,19 +99,33 @@ describe Jobs::TopicsLocaleDetectionBackfill do it "only processes topics from public categories" do DiscourseAi::Translation::TopicLocaleDetector.expects(:detect_locale).with(public_topic).once + DiscourseAi::Translation::TopicLocaleDetector .expects(:detect_locale) - .with(private_topic) + .with(private_cat_topic) .never + DiscourseAi::Translation::TopicLocaleDetector + .expects(:detect_locale) + .with(group_pm_topic) + .never + DiscourseAi::Translation::TopicLocaleDetector.expects(:detect_locale).with(pm_topic).never job.execute({ limit: 10 }) end - it "processes all topics when setting is disabled" do + it "processes public category topics, group PMs, and private category topics when setting is disabled" do SiteSetting.ai_translation_backfill_limit_to_public_content = false DiscourseAi::Translation::TopicLocaleDetector.expects(:detect_locale).with(public_topic).once - DiscourseAi::Translation::TopicLocaleDetector.expects(:detect_locale).with(private_topic).once + DiscourseAi::Translation::TopicLocaleDetector + .expects(:detect_locale) + .with(group_pm_topic) + .once + DiscourseAi::Translation::TopicLocaleDetector + .expects(:detect_locale) + .with(private_cat_topic) + .once + DiscourseAi::Translation::TopicLocaleDetector.expects(:detect_locale).with(pm_topic).never job.execute({ limit: 10 }) end