From d38e2fa19d15cc48c9eaa82923b16abe8e0e4f3e Mon Sep 17 00:00:00 2001 From: Nat Date: Thu, 12 Jun 2025 16:53:27 +0800 Subject: [PATCH] FIX: When allowing private content translation, only translate group PMs and not personal PMs --- app/jobs/regular/detect_translate_post.rb | 10 ++- app/jobs/regular/detect_translate_topic.rb | 7 +- app/jobs/regular/localize_posts.rb | 14 +++- app/jobs/regular/localize_topics.rb | 14 +++- .../regular/detect_translate_post_spec.rb | 78 ++++++++++++++++-- .../regular/detect_translate_topic_spec.rb | 79 +++++++++++++++++-- spec/jobs/regular/localize_posts_spec.rb | 61 +++++++++----- spec/jobs/regular/localize_topics_spec.rb | 61 +++++++++----- 8 files changed, 264 insertions(+), 60 deletions(-) diff --git a/app/jobs/regular/detect_translate_post.rb b/app/jobs/regular/detect_translate_post.rb index 7b50a464f..f6358026f 100644 --- a/app/jobs/regular/detect_translate_post.rb +++ b/app/jobs/regular/detect_translate_post.rb @@ -12,10 +12,14 @@ def execute(args) 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 - if topic.blank? || topic.category&.read_restricted? || - topic.archetype == Archetype.private_message + 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 diff --git a/app/jobs/regular/detect_translate_topic.rb b/app/jobs/regular/detect_translate_topic.rb index 47ee26d01..cd507e9ce 100644 --- a/app/jobs/regular/detect_translate_topic.rb +++ b/app/jobs/regular/detect_translate_topic.rb @@ -15,7 +15,12 @@ def execute(args) 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? diff --git a/app/jobs/regular/localize_posts.rb b/app/jobs/regular/localize_posts.rb index 2e487b2ae..72dcf7eb8 100644 --- a/app/jobs/regular/localize_posts.rb +++ b/app/jobs/regular/localize_posts.rb @@ -29,12 +29,22 @@ def execute(args) .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 diff --git a/app/jobs/regular/localize_topics.rb b/app/jobs/regular/localize_topics.rb index 1febff81d..90f58350f 100644 --- a/app/jobs/regular/localize_topics.rb +++ b/app/jobs/regular/localize_topics.rb @@ -29,7 +29,19 @@ def execute(args) .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 diff --git a/spec/jobs/regular/detect_translate_post_spec.rb b/spec/jobs/regular/detect_translate_post_spec.rb index 21b05a70a..007eb8151 100644 --- a/spec/jobs/regular/detect_translate_post_spec.rb +++ b/spec/jobs/regular/detect_translate_post_spec.rb @@ -88,16 +88,78 @@ 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 - SiteSetting.ai_translation_backfill_limit_to_public_content = true - post.topic.category.update!(read_restricted: true) + describe "with public content and PM limitations" 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) } - DiscourseAi::Translation::PostLocaleDetector.expects(:detect_locale).with(post).never - DiscourseAi::Translation::PostLocalizer.expects(:localize).never + fab!(:personal_pm_topic) { Fabricate(:private_message_topic) } + 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) } + + context "when ai_translation_backfill_limit_to_public_content is true" do + 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 - pm_post = Fabricate(:post, topic: Fabricate(:private_message_topic)) - job.execute({ post_id: pm_post.id }) + 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 diff --git a/spec/jobs/regular/detect_translate_topic_spec.rb b/spec/jobs/regular/detect_translate_topic_spec.rb index fca842f1a..101761a62 100644 --- a/spec/jobs/regular/detect_translate_topic_spec.rb +++ b/spec/jobs/regular/detect_translate_topic_spec.rb @@ -87,13 +87,80 @@ 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 - SiteSetting.ai_translation_backfill_limit_to_public_content = true - topic.category.update!(read_restricted: true) + describe "with public content and PM limitations" do + fab!(:private_category) { Fabricate(:private_category, group: Group[:staff]) } + fab!(:private_topic) { Fabricate(:topic, category: private_category) } - DiscourseAi::Translation::TopicLocaleDetector.expects(:detect_locale).never - DiscourseAi::Translation::TopicLocalizer.expects(:localize).never + fab!(:personal_pm_topic) { Fabricate(:private_message_topic) } - 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 diff --git a/spec/jobs/regular/localize_posts_spec.rb b/spec/jobs/regular/localize_posts_spec.rb index 6b0ad3ac9..f0b6fd60c 100644 --- a/spec/jobs/regular/localize_posts_spec.rb +++ b/spec/jobs/regular/localize_posts_spec.rb @@ -127,36 +127,57 @@ 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 - SiteSetting.ai_translation_backfill_limit_to_public_content = true - SiteSetting.experimental_content_localization_supported_locales = "ja" - end + fab!(:personal_pm_topic) { Fabricate(:private_message_topic) } + fab!(:personal_pm_post) { Fabricate(:post, topic: personal_pm_topic, locale: "es") } - it "only processes posts from public categories" do - DiscourseAi::Translation::PostLocalizer.expects(:localize).with(public_post, "ja").once + 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") } - DiscourseAi::Translation::PostLocalizer - .expects(:localize) - .with(private_post, any_parameters) - .never + before { SiteSetting.experimental_content_localization_supported_locales = "ja" } - DiscourseAi::Translation::PostLocalizer.expects(:localize).with(pm_post, any_parameters).never + context "when ai_translation_backfill_limit_to_public_content is true" do + before { SiteSetting.ai_translation_backfill_limit_to_public_content = true } - job.execute({}) + 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 "processes all posts when setting is disabled" do - SiteSetting.ai_translation_backfill_limit_to_public_content = false + context "when ai_translation_backfill_limit_to_public_content is false" do + before { SiteSetting.ai_translation_backfill_limit_to_public_content = false } - 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 + it "processes public posts and group PMs but not personal PMs" do + DiscourseAi::Translation::PostLocalizer.expects(:localize).with(public_post, "ja").once + DiscourseAi::Translation::PostLocalizer.expects(:localize).with(private_post, "ja").once - job.execute({}) + DiscourseAi::Translation::PostLocalizer.expects(:localize).with(group_pm_post, "ja").once + + DiscourseAi::Translation::PostLocalizer + .expects(:localize) + .with(personal_pm_post, any_parameters) + .never + + job.execute({}) + end end end diff --git a/spec/jobs/regular/localize_topics_spec.rb b/spec/jobs/regular/localize_topics_spec.rb index 427a357ab..07f86732c 100644 --- a/spec/jobs/regular/localize_topics_spec.rb +++ b/spec/jobs/regular/localize_topics_spec.rb @@ -133,33 +133,56 @@ 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 - DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "en").once - DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "ja").once - DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "de").once + fab!(:group_pm_topic) do + Fabricate(:group_private_message_topic, recipient_group: Fabricate(:group), locale: "es") + end - DiscourseAi::Translation::TopicLocalizer - .expects(:localize) - .with(private_topic, any_parameters) - .never + before { SiteSetting.experimental_content_localization_supported_locales = "ja" } - job.execute({}) + 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 topics from public categories" do + DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "ja").once + + 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 - it "processes all topics when setting is disabled" do - SiteSetting.ai_translation_backfill_limit_to_public_content = false + context "when ai_translation_backfill_limit_to_public_content is false" do + before { SiteSetting.ai_translation_backfill_limit_to_public_content = false } - DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "en").once - DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "ja").once - DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(public_topic, "de").once + 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, "en").once - DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(private_topic, "ja").once - DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(private_topic, "de").once + DiscourseAi::Translation::TopicLocalizer.expects(:localize).with(private_topic, "ja").once - job.execute({}) + 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