Skip to content

Commit 92b7d48

Browse files
committed
tests
1 parent 3326e73 commit 92b7d48

File tree

6 files changed

+95
-32
lines changed

6 files changed

+95
-32
lines changed

app/models/discourse_translator/post_translation.rb

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,6 @@ class PostTranslation < ActiveRecord::Base
1010
validates :locale, presence: true
1111
validates :translation, presence: true
1212
validates :locale, uniqueness: { scope: :post_id }
13-
14-
def self.translation_for(post_id, locale)
15-
find_by(post_id: post_id, locale: locale)&.translation
16-
end
1713
end
1814
end
1915

app/models/discourse_translator/topic_translation.rb

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,6 @@ class TopicTranslation < ActiveRecord::Base
1010
validates :locale, presence: true
1111
validates :translation, presence: true
1212
validates :locale, uniqueness: { scope: :topic_id }
13-
14-
def self.translation_for(topic_id, locale)
15-
find_by(topic_id: topic_id, locale: locale)&.translation
16-
end
1713
end
1814
end
1915

lib/discourse_translator/inline_translation.rb

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,23 @@ def inject(plugin)
4242
:basic_post,
4343
:is_translated,
4444
include_condition: -> { SiteSetting.experimental_inline_translation },
45-
) { object.translation_for(I18n.locale).present? }
45+
) { !object.locale_matches?(I18n.locale) && object.translation_for(I18n.locale).present? }
4646

4747
plugin.add_to_serializer(
4848
:topic_view,
4949
:is_translated,
5050
include_condition: -> { SiteSetting.experimental_inline_translation },
51-
) { object.topic.translations.present? || object.posts.any? { |p| p.translations.present? } }
51+
) do
52+
# since locales are eager loaded, but translations may not
53+
# return early if topic and posts are all in the user's locale
54+
if object.topic.locale_matches?(I18n.locale) &&
55+
object.posts.all? { |p| p.locale_matches?(I18n.locale) }
56+
return false
57+
end
58+
59+
object.topic.translation_for(I18n.locale).present? ||
60+
object.posts.any? { |p| p.translation_for(I18n.locale).present? }
61+
end
5262
end
5363
end
5464
end

lib/discourse_translator/topic_view_serializer_extension.rb

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,18 @@ module TopicViewSerializerExtension
55
def posts
66
if SiteSetting.translator_enabled?
77
posts_query = object.posts.includes(:content_locale)
8+
# this is kind of a micro-optimization.
9+
# we do not want to eager load translations if the user is using the site's language.
10+
# we will only load them if the user is using a different language that is supported by the site.
811
posts_query =
9-
posts_query.includes(:translations) if SiteSetting.experimental_inline_translation
12+
posts_query.includes(:translations) if SiteSetting.experimental_inline_translation &&
13+
(
14+
I18n.locale.to_s != SiteSetting.default_locale &&
15+
SiteSetting
16+
.automatic_translation_target_languages
17+
.split("|")
18+
.include?(I18n.locale.to_s)
19+
)
1020
object.instance_variable_set(:@posts, posts_query)
1121
end
1222
super

spec/serializers/post_serializer_spec.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
SiteSetting.translator_enabled = true
7979
SiteSetting.experimental_inline_translation = true
8080
I18n.locale = "ja"
81+
post.set_detected_locale("en")
8182
post.set_translation("ja", "こんにちは")
8283
serializer = PostSerializer.new(post, scope: Guardian.new)
8384

spec/serializers/topic_view_serializer_spec.rb

Lines changed: 71 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -45,27 +45,77 @@
4545
expect(topic_view.posts.first.association(:content_locale)).to be_loaded
4646
end
4747

48-
it "preloads translations when experimental_inline_translation is enabled" do
49-
SiteSetting.experimental_inline_translation = true
50-
51-
en_post.set_translation("es", "Hola")
52-
es_post.set_translation("en", "Hello")
53-
54-
topic_view = TopicView.new(topic)
55-
serializer = TopicViewSerializer.new(topic_view, scope: Guardian.new(user), root: false)
56-
57-
topic_view.posts.reload
58-
59-
queries =
60-
track_sql_queries do
61-
json = serializer.as_json
62-
json[:post_stream][:posts].each { |p| p[:translations] }
63-
end
64-
65-
translation_queries =
66-
queries.count { |q| q.include?("discourse_translator_post_translations") }
67-
expect(translation_queries).to eq(1)
68-
expect(topic_view.posts.first.association(:translations)).to be_loaded
48+
describe "preloading translations with SiteSetting.experimental_inline_translations" do
49+
before do
50+
SiteSetting.experimental_inline_translation = true
51+
52+
en_post.set_translation("es", "Hola")
53+
es_post.set_translation("en", "Hello")
54+
end
55+
56+
it "does not preload translations when user locale matches site default locale" do
57+
SiteSetting.default_locale = "en"
58+
I18n.locale = "en"
59+
60+
topic_view = TopicView.new(topic)
61+
serializer = TopicViewSerializer.new(topic_view, scope: Guardian.new(user), root: false)
62+
63+
topic_view.posts.reload
64+
65+
queries =
66+
track_sql_queries do
67+
json = serializer.as_json
68+
json[:post_stream][:posts].each { |p| p[:translations] }
69+
end
70+
71+
expect(queries.count { |q| q.include?("discourse_translator_post_translations") }).to eq(3)
72+
end
73+
74+
it "does not preload translations when locales are different and not in automatic_translation_target_languages" do
75+
SiteSetting.default_locale = "en"
76+
I18n.locale = "ja"
77+
SiteSetting.automatic_translation_backfill_maximum_translations_per_hour = 1
78+
SiteSetting.automatic_translation_target_languages = "es"
79+
80+
topic_view = TopicView.new(topic)
81+
serializer = TopicViewSerializer.new(topic_view, scope: Guardian.new(user), root: false)
82+
83+
topic_view.posts.reload
84+
85+
queries =
86+
track_sql_queries do
87+
json = serializer.as_json
88+
json[:post_stream][:posts].each { |p| p[:translations] }
89+
end
90+
91+
translation_queries =
92+
queries.count { |q| q.include?("discourse_translator_post_translations") }
93+
expect(translation_queries).to eq(3)
94+
expect(topic_view.posts.first.association(:translations)).to be_loaded
95+
end
96+
97+
it "preloads translations when locales are different and in automatic_translation_target_languages" do
98+
SiteSetting.default_locale = "en"
99+
I18n.locale = "es"
100+
SiteSetting.automatic_translation_backfill_maximum_translations_per_hour = 1
101+
SiteSetting.automatic_translation_target_languages = "es"
102+
103+
topic_view = TopicView.new(topic)
104+
serializer = TopicViewSerializer.new(topic_view, scope: Guardian.new(user), root: false)
105+
106+
topic_view.posts.reload
107+
108+
queries =
109+
track_sql_queries do
110+
json = serializer.as_json
111+
json[:post_stream][:posts].each { |p| p[:translations] }
112+
end
113+
114+
translation_queries =
115+
queries.count { |q| q.include?("discourse_translator_post_translations") }
116+
expect(translation_queries).to eq(1)
117+
expect(topic_view.posts.first.association(:translations)).to be_loaded
118+
end
69119
end
70120
end
71121

0 commit comments

Comments
 (0)