Skip to content
This repository was archived by the owner on Jul 22, 2025. It is now read-only.

Commit fc83bed

Browse files
nattswnattsw
authored
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`
1 parent 9b7f1e6 commit fc83bed

File tree

8 files changed

+264
-60
lines changed

8 files changed

+264
-60
lines changed

‎app/jobs/regular/detect_translate_post.rb‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,14 @@ def execute(args)
1212
post = Post.find_by(id: args[:post_id])
1313
return if post.blank? || post.raw.blank? || post.deleted_at.present? || post.user_id <= 0
1414

15+
topic = post.topic
16+
return if topic.blank?
17+
1518
if SiteSetting.ai_translation_backfill_limit_to_public_content
16-
topic = post.topic
17-
if topic.blank? || topic.category&.read_restricted? ||
18-
topic.archetype == Archetype.private_message
19+
return if topic.category&.read_restricted? || topic.archetype == Archetype.private_message
20+
else
21+
if topic.archetype == Archetype.private_message &&
22+
!TopicAllowedGroup.exists?(topic_id: topic.id)
1923
return
2024
end
2125
end

‎app/jobs/regular/detect_translate_topic.rb‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,12 @@ def execute(args)
1515
end
1616

1717
if SiteSetting.ai_translation_backfill_limit_to_public_content
18-
return if topic.category&.read_restricted?
18+
return if topic.category&.read_restricted? || topic.archetype == Archetype.private_message
19+
else
20+
if topic.archetype == Archetype.private_message &&
21+
!TopicAllowedGroup.exists?(topic_id: topic.id)
22+
return
23+
end
1924
end
2025

2126
if (detected_locale = topic.locale).blank?

‎app/jobs/regular/localize_posts.rb‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,22 @@ def execute(args)
2929
.where.not(locale: locale)
3030
.where("pl.id IS NULL")
3131

32+
posts = posts.joins(:topic)
33+
3234
if SiteSetting.ai_translation_backfill_limit_to_public_content
35+
# exclude all PMs
36+
# and only include posts from public categories
3337
posts =
3438
posts
35-
.joins(:topic)
36-
.where(topics: { category_id: Category.where(read_restricted: false).select(:id) })
3739
.where.not(topics: { archetype: Archetype.private_message })
40+
.where(topics: { category_id: Category.where(read_restricted: false).select(:id) })
41+
else
42+
# all regular topics, and group PMs
43+
posts =
44+
posts.where(
45+
"topics.archetype != ? OR topics.id IN (SELECT topic_id FROM topic_allowed_groups)",
46+
Archetype.private_message,
47+
)
3848
end
3949

4050
if SiteSetting.ai_translation_backfill_max_age_days > 0

‎app/jobs/regular/localize_topics.rb‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,19 @@ def execute(args)
2929
.where("tl.id IS NULL")
3030

3131
if SiteSetting.ai_translation_backfill_limit_to_public_content
32-
topics = topics.where(category_id: Category.where(read_restricted: false).select(:id))
32+
# exclude all PMs
33+
# and only include posts from public categories
34+
topics =
35+
topics
36+
.where.not(archetype: Archetype.private_message)
37+
.where(category_id: Category.where(read_restricted: false).select(:id))
38+
else
39+
# all regular topics, and group PMs
40+
topics =
41+
topics.where(
42+
"topics.archetype != ? OR topics.id IN (SELECT topic_id FROM topic_allowed_groups)",
43+
Archetype.private_message,
44+
)
3345
end
3446

3547
if SiteSetting.ai_translation_backfill_max_age_days > 0

‎spec/jobs/regular/detect_translate_post_spec.rb‎

Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -88,16 +88,78 @@
8888
expect { job.execute({ post_id: post.id }) }.not_to raise_error
8989
end
9090

91-
it "skips public content when `ai_translation_backfill_limit_to_public_content ` site setting is enabled" do
92-
SiteSetting.ai_translation_backfill_limit_to_public_content = true
93-
post.topic.category.update!(read_restricted: true)
91+
describe "with public content and PM limitations" do
92+
fab!(:private_category) { Fabricate(:private_category, group: Group[:staff]) }
93+
fab!(:private_topic) { Fabricate(:topic, category: private_category) }
94+
fab!(:private_post) { Fabricate(:post, topic: private_topic) }
9495

95-
DiscourseAi::Translation::PostLocaleDetector.expects(:detect_locale).with(post).never
96-
DiscourseAi::Translation::PostLocalizer.expects(:localize).never
96+
fab!(:personal_pm_topic) { Fabricate(:private_message_topic) }
97+
fab!(:personal_pm_post) { Fabricate(:post, topic: personal_pm_topic) }
9798

98-
job.execute({ post_id: post.id })
99+
fab!(:group_pm_topic) do
100+
Fabricate(:group_private_message_topic, recipient_group: Fabricate(:group))
101+
end
102+
fab!(:group_pm_post) { Fabricate(:post, topic: group_pm_topic) }
103+
104+
context "when ai_translation_backfill_limit_to_public_content is true" do
105+
before { SiteSetting.ai_translation_backfill_limit_to_public_content = true }
106+
107+
it "skips posts from restricted categories and PMs" do
108+
DiscourseAi::Translation::PostLocaleDetector
109+
.expects(:detect_locale)
110+
.with(private_post)
111+
.never
112+
DiscourseAi::Translation::PostLocalizer
113+
.expects(:localize)
114+
.with(private_post, any_parameters)
115+
.never
116+
job.execute({ post_id: private_post.id })
117+
118+
DiscourseAi::Translation::PostLocaleDetector
119+
.expects(:detect_locale)
120+
.with(personal_pm_post)
121+
.never
122+
DiscourseAi::Translation::PostLocalizer
123+
.expects(:localize)
124+
.with(personal_pm_post, any_parameters)
125+
.never
126+
job.execute({ post_id: personal_pm_post.id })
127+
128+
DiscourseAi::Translation::PostLocaleDetector
129+
.expects(:detect_locale)
130+
.with(group_pm_post)
131+
.never
132+
DiscourseAi::Translation::PostLocalizer
133+
.expects(:localize)
134+
.with(group_pm_post, any_parameters)
135+
.never
136+
job.execute({ post_id: group_pm_post.id })
137+
end
138+
end
99139

100-
pm_post = Fabricate(:post, topic: Fabricate(:private_message_topic))
101-
job.execute({ post_id: pm_post.id })
140+
context "when ai_translation_backfill_limit_to_public_content is false" do
141+
before { SiteSetting.ai_translation_backfill_limit_to_public_content = false }
142+
143+
it "processes posts from private categories and group PMs but skips personal PMs" do
144+
DiscourseAi::Translation::PostLocaleDetector.expects(:detect_locale).with(private_post).once
145+
job.execute({ post_id: private_post.id })
146+
147+
DiscourseAi::Translation::PostLocaleDetector
148+
.expects(:detect_locale)
149+
.with(group_pm_post)
150+
.once
151+
job.execute({ post_id: group_pm_post.id })
152+
153+
DiscourseAi::Translation::PostLocaleDetector
154+
.expects(:detect_locale)
155+
.with(personal_pm_post)
156+
.never
157+
DiscourseAi::Translation::PostLocalizer
158+
.expects(:localize)
159+
.with(personal_pm_post, any_parameters)
160+
.never
161+
job.execute({ post_id: personal_pm_post.id })
162+
end
163+
end
102164
end
103165
end

‎spec/jobs/regular/detect_translate_topic_spec.rb‎

Lines changed: 73 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,80 @@
8787
expect { job.execute({ topic_id: topic.id }) }.not_to raise_error
8888
end
8989

90-
it "skips public content when `ai_translation_backfill_limit_to_public_content ` site setting is enabled" do
91-
SiteSetting.ai_translation_backfill_limit_to_public_content = true
92-
topic.category.update!(read_restricted: true)
90+
describe "with public content and PM limitations" do
91+
fab!(:private_category) { Fabricate(:private_category, group: Group[:staff]) }
92+
fab!(:private_topic) { Fabricate(:topic, category: private_category) }
9393

94-
DiscourseAi::Translation::TopicLocaleDetector.expects(:detect_locale).never
95-
DiscourseAi::Translation::TopicLocalizer.expects(:localize).never
94+
fab!(:personal_pm_topic) { Fabricate(:private_message_topic) }
9695

97-
job.execute({ topic_id: topic.id })
96+
fab!(:group_pm_topic) do
97+
Fabricate(:group_private_message_topic, recipient_group: Fabricate(:group))
98+
end
99+
100+
context "when ai_translation_backfill_limit_to_public_content is true" do
101+
before { SiteSetting.ai_translation_backfill_limit_to_public_content = true }
102+
103+
it "skips topics from restricted categories and PMs" do
104+
DiscourseAi::Translation::TopicLocaleDetector
105+
.expects(:detect_locale)
106+
.with(private_topic)
107+
.never
108+
DiscourseAi::Translation::TopicLocalizer
109+
.expects(:localize)
110+
.with(private_topic, any_parameters)
111+
.never
112+
job.execute({ topic_id: private_topic.id })
113+
114+
# Skip personal PMs
115+
DiscourseAi::Translation::TopicLocaleDetector
116+
.expects(:detect_locale)
117+
.with(personal_pm_topic)
118+
.never
119+
DiscourseAi::Translation::TopicLocalizer
120+
.expects(:localize)
121+
.with(personal_pm_topic, any_parameters)
122+
.never
123+
job.execute({ topic_id: personal_pm_topic.id })
124+
125+
DiscourseAi::Translation::TopicLocaleDetector
126+
.expects(:detect_locale)
127+
.with(group_pm_topic)
128+
.never
129+
DiscourseAi::Translation::TopicLocalizer
130+
.expects(:localize)
131+
.with(group_pm_topic, any_parameters)
132+
.never
133+
134+
job.execute({ topic_id: group_pm_topic.id })
135+
end
136+
end
137+
138+
context "when ai_translation_backfill_limit_to_public_content is false" do
139+
before { SiteSetting.ai_translation_backfill_limit_to_public_content = false }
140+
141+
it "processes topics from private categories and group PMs but skips personal PMs" do
142+
DiscourseAi::Translation::TopicLocaleDetector
143+
.expects(:detect_locale)
144+
.with(private_topic)
145+
.once
146+
job.execute({ topic_id: private_topic.id })
147+
148+
DiscourseAi::Translation::TopicLocaleDetector
149+
.expects(:detect_locale)
150+
.with(group_pm_topic)
151+
.once
152+
job.execute({ topic_id: group_pm_topic.id })
153+
154+
DiscourseAi::Translation::TopicLocaleDetector
155+
.expects(:detect_locale)
156+
.with(personal_pm_topic)
157+
.never
158+
DiscourseAi::Translation::TopicLocalizer
159+
.expects(:localize)
160+
.with(personal_pm_topic, any_parameters)
161+
.never
162+
job.execute({ topic_id: personal_pm_topic.id })
163+
end
164+
end
98165
end
99166
end

‎spec/jobs/regular/localize_posts_spec.rb‎

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -127,36 +127,57 @@
127127
fab!(:private_topic) { Fabricate(:topic, category: private_category) }
128128
fab!(:private_post) { Fabricate(:post, topic: private_topic, locale: "es") }
129129

130-
fab!(:pm_post) { Fabricate(:post, topic: Fabricate(:private_message_topic), locale: "es") }
131-
132130
fab!(:public_post) { Fabricate(:post, locale: "es") }
133131

134-
before do
135-
SiteSetting.ai_translation_backfill_limit_to_public_content = true
136-
SiteSetting.experimental_content_localization_supported_locales = "ja"
137-
end
132+
fab!(:personal_pm_topic) { Fabricate(:private_message_topic) }
133+
fab!(:personal_pm_post) { Fabricate(:post, topic: personal_pm_topic, locale: "es") }
138134

139-
it "only processes posts from public categories" do
140-
DiscourseAi::Translation::PostLocalizer.expects(:localize).with(public_post, "ja").once
135+
fab!(:group)
136+
fab!(:group_pm_topic) { Fabricate(:group_private_message_topic, recipient_group: group) }
137+
fab!(:group_pm_post) { Fabricate(:post, topic: group_pm_topic, locale: "es") }
141138

142-
DiscourseAi::Translation::PostLocalizer
143-
.expects(:localize)
144-
.with(private_post, any_parameters)
145-
.never
139+
before { SiteSetting.experimental_content_localization_supported_locales = "ja" }
146140

147-
DiscourseAi::Translation::PostLocalizer.expects(:localize).with(pm_post, any_parameters).never
141+
context "when ai_translation_backfill_limit_to_public_content is true" do
142+
before { SiteSetting.ai_translation_backfill_limit_to_public_content = true }
148143

149-
job.execute({})
144+
it "only processes posts from public categories" do
145+
DiscourseAi::Translation::PostLocalizer.expects(:localize).with(public_post, "ja").once
146+
147+
DiscourseAi::Translation::PostLocalizer
148+
.expects(:localize)
149+
.with(private_post, any_parameters)
150+
.never
151+
152+
DiscourseAi::Translation::PostLocalizer
153+
.expects(:localize)
154+
.with(personal_pm_post, any_parameters)
155+
.never
156+
DiscourseAi::Translation::PostLocalizer
157+
.expects(:localize)
158+
.with(group_pm_post, any_parameters)
159+
.never
160+
161+
job.execute({})
162+
end
150163
end
151164

152-
it "processes all posts when setting is disabled" do
153-
SiteSetting.ai_translation_backfill_limit_to_public_content = false
165+
context "when ai_translation_backfill_limit_to_public_content is false" do
166+
before { SiteSetting.ai_translation_backfill_limit_to_public_content = false }
154167

155-
DiscourseAi::Translation::PostLocalizer.expects(:localize).with(public_post, "ja").once
156-
DiscourseAi::Translation::PostLocalizer.expects(:localize).with(pm_post, "ja").once
157-
DiscourseAi::Translation::PostLocalizer.expects(:localize).with(private_post, "ja").once
168+
it "processes public posts and group PMs but not personal PMs" do
169+
DiscourseAi::Translation::PostLocalizer.expects(:localize).with(public_post, "ja").once
170+
DiscourseAi::Translation::PostLocalizer.expects(:localize).with(private_post, "ja").once
158171

159-
job.execute({})
172+
DiscourseAi::Translation::PostLocalizer.expects(:localize).with(group_pm_post, "ja").once
173+
174+
DiscourseAi::Translation::PostLocalizer
175+
.expects(:localize)
176+
.with(personal_pm_post, any_parameters)
177+
.never
178+
179+
job.execute({})
180+
end
160181
end
161182
end
162183

0 commit comments

Comments
 (0)