Skip to content

Commit d371bb3

Browse files
nattswnattsw
authored
DEV: Move saving translations outside of the API providers (#244)
Historically, each translation provider has been saving translation as part of doing the translation. We are moving the saving outside of the providers, so `detect!` and `translate!` only does the detection and translation. Saving is now done in the base class so any future changes do not have to involve each translator. This commit is to prepare / reduce complexity for the subsequent PR where we move to using raw to translate instead of cooked.
1 parent 00a202f commit d371bb3

File tree

8 files changed

+126
-131
lines changed

8 files changed

+126
-131
lines changed

‎app/services/discourse_translator/amazon.rb‎

Lines changed: 24 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -108,40 +108,36 @@ def self.access_token_key
108108
end
109109

110110
def self.detect!(topic_or_post)
111-
save_detected_locale(topic_or_post) do
112-
begin
113-
client.translate_text(
114-
{
115-
text: truncate(text_for_detection(topic_or_post)),
116-
source_language_code: "auto",
117-
target_language_code: SUPPORTED_LANG_MAPPING[I18n.locale],
118-
},
119-
)&.source_language_code
120-
rescue Aws::Errors::MissingCredentialsError
121-
raise I18n.t("translator.amazon.invalid_credentials")
122-
end
111+
begin
112+
client.translate_text(
113+
{
114+
text: truncate(text_for_detection(topic_or_post)),
115+
source_language_code: "auto",
116+
target_language_code: SUPPORTED_LANG_MAPPING[I18n.locale],
117+
},
118+
)&.source_language_code
119+
rescue Aws::Errors::MissingCredentialsError
120+
raise I18n.t("translator.amazon.invalid_credentials")
123121
end
124122
end
125123

126124
def self.translate!(translatable, target_locale_sym = I18n.locale)
127125
detected_lang = detect(translatable)
128126

129-
save_translation(translatable, target_locale_sym) do
130-
begin
131-
client.translate_text(
132-
{
133-
text: truncate(text_for_translation(translatable)),
134-
source_language_code: "auto",
135-
target_language_code: SUPPORTED_LANG_MAPPING[target_locale_sym],
136-
},
137-
)
138-
rescue Aws::Translate::Errors::UnsupportedLanguagePairException
139-
raise I18n.t(
140-
"translator.failed",
141-
source_locale: detected_lang,
142-
target_locale: target_locale_sym,
143-
)
144-
end
127+
begin
128+
client.translate_text(
129+
{
130+
text: truncate(text_for_translation(translatable)),
131+
source_language_code: "auto",
132+
target_language_code: SUPPORTED_LANG_MAPPING[target_locale_sym],
133+
},
134+
)
135+
rescue Aws::Translate::Errors::UnsupportedLanguagePairException
136+
raise I18n.t(
137+
"translator.failed",
138+
source_locale: detected_lang,
139+
target_locale: target_locale_sym,
140+
)
145141
end
146142
end
147143

‎app/services/discourse_translator/base.rb‎

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ def self.translate(translatable, target_locale_sym = I18n.locale)
3333
detected_lang = detect(translatable)
3434

3535
if translatable.locale_matches?(target_locale_sym)
36-
return detected_lang, get_text(translatable)
36+
return detected_lang, get_untranslated(translatable)
3737
end
3838

3939
translation = translatable.translation_for(target_locale_sym)
@@ -48,12 +48,18 @@ def self.translate(translatable, target_locale_sym = I18n.locale)
4848
),
4949
)
5050
end
51-
[detected_lang, translate!(translatable, target_locale_sym)]
51+
52+
translated = translate!(translatable, target_locale_sym)
53+
save_translation(translatable, target_locale_sym) { translated }
54+
[detected_lang, translated]
5255
end
5356

54-
# Subclasses must implement this method to translate the text of a post or topic
55-
# then use the save_translation method to store the translated text.
57+
# Subclasses must implement this method to translate the text of a
58+
# post or topic and return only the translated text.
59+
# Subclasses should use text_for_translation
5660
# @param translatable [Post|Topic]
61+
# @param target_locale_sym [Symbol]
62+
# @return [String]
5763
def self.translate!(translatable, target_locale_sym = I18n.locale)
5864
raise "Not Implemented"
5965
end
@@ -63,13 +69,16 @@ def self.translate!(translatable, target_locale_sym = I18n.locale)
6369
# @param translatable [Post|Topic]
6470
def self.detect(translatable)
6571
return if text_for_detection(translatable).blank?
66-
get_detected_locale(translatable) || detect!(translatable)
72+
get_detected_locale(translatable) ||
73+
save_detected_locale(translatable) { detect!(translatable) }
6774
end
6875

6976
# Subclasses must implement this method to detect the text of a post or topic
70-
# then use the save_detected_locale method to store the detected locale.
77+
# and return only the detected locale.
78+
# Subclasses should use text_for_detection
7179
# @param translatable [Post|Topic]
72-
def self.detect!(post)
80+
# @return [String]
81+
def self.detect!(translatable)
7382
raise "Not Implemented"
7483
end
7584

@@ -100,15 +109,6 @@ def self.save_detected_locale(translatable)
100109
detected_locale
101110
end
102111

103-
def self.get_text(translatable)
104-
case translatable.class.name
105-
when "Post"
106-
translatable.cooked
107-
when "Topic"
108-
translatable.title
109-
end
110-
end
111-
112112
def self.language_supported?(detected_lang)
113113
raise NotImplementedError unless self.const_defined?(:SUPPORTED_LANG_MAPPING)
114114
supported_lang = const_get(:SUPPORTED_LANG_MAPPING)
@@ -129,11 +129,24 @@ def self.strip_tags_for_detection(detection_text)
129129
end
130130

131131
def self.text_for_detection(translatable)
132-
strip_tags_for_detection(get_text(translatable)).truncate(DETECTION_CHAR_LIMIT, omission: nil)
132+
strip_tags_for_detection(get_untranslated(translatable)).truncate(
133+
DETECTION_CHAR_LIMIT,
134+
omission: nil,
135+
)
133136
end
134137

135138
def self.text_for_translation(translatable)
136-
get_text(translatable).truncate(SiteSetting.max_characters_per_translation, omission: nil)
139+
max_char = SiteSetting.max_characters_per_translation
140+
get_untranslated(translatable).truncate(max_char, omission: nil)
141+
end
142+
143+
def self.get_untranslated(translatable)
144+
case translatable.class.name
145+
when "Post"
146+
translatable.cooked
147+
when "Topic"
148+
translatable.title
149+
end
137150
end
138151
end
139152
end

‎app/services/discourse_translator/discourse_ai.rb‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,28 @@ def self.language_supported?(detected_lang)
1212
end
1313

1414
def self.detect!(topic_or_post)
15-
return unless required_settings_enabled
16-
17-
save_detected_locale(topic_or_post) do
18-
::DiscourseAi::LanguageDetector.new(text_for_detection(topic_or_post)).detect
15+
unless required_settings_enabled
16+
raise TranslatorError.new(
17+
I18n.t(
18+
"translator.discourse_ai.ai_helper_required",
19+
{ base_url: Discourse.base_url },
20+
),
21+
)
1922
end
23+
24+
::DiscourseAi::LanguageDetector.new(text_for_detection(topic_or_post)).detect
2025
end
2126

2227
def self.translate!(translatable, target_locale_sym = I18n.locale)
23-
return unless required_settings_enabled
24-
save_translation(translatable, target_locale_sym) do
25-
::DiscourseAi::Translator.new(
26-
text_for_translation(translatable),
27-
target_locale_sym,
28-
).translate
28+
unless required_settings_enabled
29+
raise TranslatorError.new(
30+
I18n.t(
31+
"translator.discourse_ai.ai_helper_required",
32+
{ base_url: Discourse.base_url },
33+
),
34+
)
2935
end
36+
::DiscourseAi::Translator.new(text_for_translation(translatable), target_locale_sym).translate
3037
end
3138

3239
private

‎app/services/discourse_translator/google.rb‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,11 @@ def self.access_token
7575
end
7676

7777
def self.detect!(topic_or_post)
78-
save_detected_locale(topic_or_post) do
79-
result(DETECT_URI, q: text_for_detection(topic_or_post))["detections"][0].max do |a, b|
80-
a.confidence <=> b.confidence
81-
end[
82-
"language"
83-
]
84-
end
78+
result(DETECT_URI, q: text_for_detection(topic_or_post))["detections"][0].max do |a, b|
79+
a.confidence <=> b.confidence
80+
end[
81+
"language"
82+
]
8583
end
8684

8785
def self.translate_supported?(source, target)
@@ -91,16 +89,14 @@ def self.translate_supported?(source, target)
9189

9290
def self.translate!(translatable, target_locale_sym = I18n.locale)
9391
detected_locale = detect(translatable)
94-
save_translation(translatable, target_locale_sym) do
95-
res =
96-
result(
97-
TRANSLATE_URI,
98-
q: text_for_translation(translatable),
99-
source: detected_locale,
100-
target: SUPPORTED_LANG_MAPPING[target_locale_sym],
101-
)
102-
res["translations"][0]["translatedText"]
103-
end
92+
res =
93+
result(
94+
TRANSLATE_URI,
95+
q: text_for_translation(translatable),
96+
source: detected_locale,
97+
target: SUPPORTED_LANG_MAPPING[target_locale_sym],
98+
)
99+
res["translations"][0]["translatedText"]
104100
end
105101

106102
def self.result(url, body)

‎app/services/discourse_translator/libre_translate.rb‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,12 @@ def self.access_token
7979
end
8080

8181
def self.detect!(topic_or_post)
82-
save_detected_locale(topic_or_post) do
83-
res =
84-
result(
85-
detect_uri,
86-
q: ActionController::Base.helpers.strip_tags(text_for_detection(topic_or_post)),
87-
)
88-
!res.empty? ? res[0]["language"] : "en"
89-
end
82+
res =
83+
result(
84+
detect_uri,
85+
q: ActionController::Base.helpers.strip_tags(text_for_detection(topic_or_post)),
86+
)
87+
!res.empty? ? res[0]["language"] : "en"
9088
end
9189

9290
def self.translate_supported?(source, target)
@@ -98,17 +96,15 @@ def self.translate_supported?(source, target)
9896
def self.translate!(translatable, target_locale_sym = I18n.locale)
9997
detected_lang = detect(translatable)
10098

101-
save_translation(translatable, target_locale_sym) do
102-
res =
103-
result(
104-
translate_uri,
105-
q: text_for_translation(translatable),
106-
source: detected_lang,
107-
target: SUPPORTED_LANG_MAPPING[target_locale_sym],
108-
format: "html",
109-
)
110-
res["translatedText"]
111-
end
99+
res =
100+
result(
101+
translate_uri,
102+
q: text_for_translation(translatable),
103+
source: detected_lang,
104+
target: SUPPORTED_LANG_MAPPING[target_locale_sym],
105+
format: "html",
106+
)
107+
res["translatedText"]
112108
end
113109

114110
def self.get(url)

‎app/services/discourse_translator/microsoft.rb‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,10 @@ def self.access_token_key
149149
end
150150

151151
def self.detect!(topic_or_post)
152-
save_detected_locale(topic_or_post) do
153-
body = [{ "Text" => text_for_detection(topic_or_post) }].to_json
154-
uri = URI(detect_endpoint)
155-
uri.query = URI.encode_www_form(self.default_query)
156-
result(uri.to_s, body, default_headers).first["language"]
157-
end
152+
body = [{ "Text" => text_for_detection(topic_or_post) }].to_json
153+
uri = URI(detect_endpoint)
154+
uri.query = URI.encode_www_form(self.default_query)
155+
result(uri.to_s, body, default_headers).first["language"]
158156
end
159157

160158
def self.translate!(translatable, target_locale_sym = I18n.locale)
@@ -166,17 +164,12 @@ def self.translate!(translatable, target_locale_sym = I18n.locale)
166164
locale =
167165
SUPPORTED_LANG_MAPPING[target_locale_sym] || (raise I18n.t("translator.not_supported"))
168166

169-
save_translation(translatable, target_locale_sym) do
170-
query = default_query.merge("from" => detected_lang, "to" => locale, "textType" => "html")
171-
172-
body = [{ "Text" => text_for_translation(translatable) }].to_json
173-
174-
uri = URI(translate_endpoint)
175-
uri.query = URI.encode_www_form(query)
176-
177-
response_body = result(uri.to_s, body, default_headers)
178-
response_body.first["translations"].first["text"]
179-
end
167+
query = default_query.merge("from" => detected_lang, "to" => locale, "textType" => "html")
168+
body = [{ "Text" => text_for_translation(translatable) }].to_json
169+
uri = URI(translate_endpoint)
170+
uri.query = URI.encode_www_form(query)
171+
response_body = result(uri.to_s, body, default_headers)
172+
response_body.first["translations"].first["text"]
180173
end
181174

182175
def self.translate_supported?(detected_lang, target_lang)

‎app/services/discourse_translator/yandex.rb‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -124,33 +124,29 @@ def self.access_token
124124
end
125125

126126
def self.detect!(topic_or_post)
127-
save_detected_locale(topic_or_post) do
128-
query = default_query.merge("text" => text_for_detection(topic_or_post))
129-
uri = URI(DETECT_URI)
130-
uri.query = URI.encode_www_form(query)
131-
result(uri.to_s, "", default_headers)["lang"]
132-
end
127+
query = default_query.merge("text" => text_for_detection(topic_or_post))
128+
uri = URI(DETECT_URI)
129+
uri.query = URI.encode_www_form(query)
130+
result(uri.to_s, "", default_headers)["lang"]
133131
end
134132

135133
def self.translate!(translatable, target_locale_sym = I18n.locale)
136134
detected_lang = detect(translatable)
137135
locale =
138136
SUPPORTED_LANG_MAPPING[target_locale_sym] || (raise I18n.t("translator.not_supported"))
139137

140-
save_translation(translatable, target_locale_sym) do
141-
query =
142-
default_query.merge(
143-
"lang" => "#{detected_lang}-#{locale}",
144-
"text" => text_for_translation(translatable),
145-
"format" => "html",
146-
)
138+
query =
139+
default_query.merge(
140+
"lang" => "#{detected_lang}-#{locale}",
141+
"text" => text_for_translation(translatable),
142+
"format" => "html",
143+
)
147144

148-
uri = URI(TRANSLATE_URI)
149-
uri.query = URI.encode_www_form(query)
145+
uri = URI(TRANSLATE_URI)
146+
uri.query = URI.encode_www_form(query)
150147

151-
response_body = result(uri.to_s, "", default_headers)
152-
response_body["text"][0]
153-
end
148+
response_body = result(uri.to_s, "", default_headers)
149+
response_body["text"][0]
154150
end
155151

156152
def self.translate_supported?(detected_lang, target_lang)

‎spec/services/discourse_ai_spec.rb‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,11 @@
2727
end
2828

2929
describe ".detect!" do
30-
it "stores the detected language" do
30+
it "returns the detected language" do
3131
locale = "de"
3232
DiscourseAi::Completions::Llm.with_prepared_responses(["<language>de</language>"]) do
33-
DiscourseTranslator::DiscourseAi.detect!(post)
33+
expect(DiscourseTranslator::DiscourseAi.detect!(post)).to eq locale
3434
end
35-
36-
expect(post.detected_locale).to eq locale
3735
end
3836
end
3937

0 commit comments

Comments
 (0)