FIX: Don't double cook

Author: nattsw

app/models/concerns/discourse_translator/translatable.rb

@@ -11,18 +11,19 @@ module Translatable
       has_one :content_locale, class_name: "DiscourseTranslator::#{name}Locale", dependent: :destroy
     end
 
-    def set_detected_locale(locale)
+    def set_detected_locale(locale = I18n.locale)
       # locales should be "en-US" instead of "en_US" per https://www.rfc-editor.org/rfc/rfc5646#section-2.1
       locale = locale.to_s.gsub("_", "-")
       (content_locale || build_content_locale).update!(detected_locale: locale)
+      locale
     end
 
     # This method is used to create a translation for a translatable (Post or Topic) and a specific locale.
     # If a translation already exists for the locale, it will be updated.
     # Texts are put through a Sanitizer to clean them up before saving.
     # @param locale [String] the locale of the translation
     # @param text [String] the translated text
-    def set_translation(locale, text)
+    def set_translation(locale = I18n.locale, text)
       locale = locale.to_s.gsub("_", "-")
       translations.find_or_initialize_by(locale: locale).update!(translation: text)
     end

app/services/discourse_translator/provider/amazon.rb

@@ -120,38 +120,25 @@ def self.detect!(topic_or_post)
         end
       end
 
-      def self.translate_translatable!(translatable, target_locale_sym = I18n.locale)
-        detected_lang = detect(translatable)
+      def self.translate_post!(post, target_locale_sym = I18n.locale, opts = {})
+        raw = opts.key?(:raw) ? opts[:raw] : !opts[:cooked]
+        text = text_for_translation(post, raw:)
+        translate_text!(text, target_locale_sym)
+      end
 
-        begin
-          client.translate_text(
-            {
-              text: truncate(text_for_translation(translatable)),
-              source_language_code: "auto",
-              target_language_code: SUPPORTED_LANG_MAPPING[target_locale_sym],
-            },
-          )
-        rescue Aws::Translate::Errors::UnsupportedLanguagePairException
-          raise I18n.t(
-                  "translator.failed.#{translatable.class.name.downcase}",
-                  source_locale: detected_lang,
-                  target_locale: target_locale_sym,
-                )
-        end
+      def self.translate_topic!(topic, target_locale_sym = I18n.locale)
+        text = text_for_translation(topic)
+        translate_text!(text, target_locale_sym)
       end
 
       def self.translate_text!(text, target_locale_sym = I18n.locale)
-        begin
-          client.translate_text(
-            {
-              text: truncate(text),
-              source_language_code: "auto",
-              target_language_code: SUPPORTED_LANG_MAPPING[target_locale_sym],
-            },
-          )
-        rescue Aws::Translate::Errors::UnsupportedLanguagePairException
-          raise I18n.t("translator.not_supported")
-        end
+        client.translate_text(
+          {
+            text: truncate(text),
+            source_language_code: "auto",
+            target_language_code: SUPPORTED_LANG_MAPPING[target_locale_sym],
+          },
+        )&.translated_text
       end
 
       def self.client

app/services/discourse_translator/provider/base_provider.rb

@@ -25,12 +25,11 @@ def self.cache_key
         "#{key_prefix}#{access_token_key}"
       end
 
-      # Returns the stored translation of a post or topic.
-      # If the translation does not exist yet, it will be translated first via the API then stored.
+      # Translates and saves it into a PostTranslation/TopicTranslation
       # If the detected language is the same as the target language, the original text will be returned.
       # @param translatable [Post|Topic]
+      # @return [Array] the detected language and the translated text
       def self.translate(translatable, target_locale_sym = I18n.locale)
-        return if text_for_translation(translatable).blank?
         detected_lang = detect(translatable)
 
         if translatable.locale_matches?(target_locale_sym)
@@ -50,35 +49,46 @@ def self.translate(translatable, target_locale_sym = I18n.locale)
                 )
         end
 
-        translated = translate_translatable!(translatable, target_locale_sym)
-        save_translation(translatable, target_locale_sym) { translated }
-        [detected_lang, translated]
-      end
+        begin
+          begin
+            translated =
+              case translatable.class.name
+              when "Post"
+                translate_post!(translatable, target_locale_sym, { cooked: true })
+              when "Topic"
+                translate_topic!(translatable, target_locale_sym)
+              end
+          end
+        rescue => e
+          raise I18n.t(
+                  "translator.failed.#{translatable.class.name.downcase}",
+                  source_locale: detected_lang,
+                  target_locale: target_locale_sym,
+                )
+        end
 
-      # TODO: Deprecate this in favour of translate_<model>
-      def self.translate_translatable!(translatable, target_locale_sym = I18n.locale)
-        raise "Not Implemented"
+        translatable.set_translation(target_locale_sym, translated)
+        [detected_lang, translated]
       end
 
       def self.translate_text!(text, target_locale_sym = I18n.locale)
         raise "Not Implemented"
       end
 
-      def self.translate_post!(post, target_locale_sym = I18n.locale)
-        translate_translatable!(post, target_locale_sym)
+      def self.translate_post!(post, target_locale_sym = I18n.locale, opts = {})
+        raise "Not Implemented"
       end
 
       def self.translate_topic!(topic, target_locale_sym = I18n.locale)
-        translate_translatable!(topic, target_locale_sym)
+        raise "Not Implemented"
       end
 
       # Returns the stored detected locale of a post or topic.
       # If the locale does not exist yet, it will be detected first via the API then stored.
       # @param translatable [Post|Topic]
       def self.detect(translatable)
         return if text_for_detection(translatable).blank?
-        get_detected_locale(translatable) ||
-          save_detected_locale(translatable) { detect!(translatable) }
+        translatable.detected_locale || translatable.set_detected_locale(detect!(translatable))
       end
 
       # Subclasses must implement this method to detect the text of a post or topic
@@ -94,29 +104,6 @@ def self.access_token
         raise "Not Implemented"
       end
 
-      def self.save_translation(translatable, target_locale_sym = I18n.locale)
-        begin
-          translation = yield
-        rescue Timeout::Error
-          raise TranslatorError.new(I18n.t("translator.api_timeout"))
-        end
-        translatable.set_translation(target_locale_sym, translation)
-        translation
-      end
-
-      def self.get_detected_locale(translatable)
-        translatable.detected_locale
-      end
-
-      def self.save_detected_locale(translatable)
-        # sometimes we may have a user post that is just an emoji
-        # in that case, we will just indicate the post is in the default locale
-        detected_locale = yield.presence || SiteSetting.default_locale
-        translatable.set_detected_locale(detected_locale)
-
-        detected_locale
-      end
-
       def self.language_supported?(detected_lang)
         raise NotImplementedError unless self.const_defined?(:SUPPORTED_LANG_MAPPING)
         supported_lang = const_get(:SUPPORTED_LANG_MAPPING)

app/services/discourse_translator/provider/discourse_ai.rb

@@ -15,33 +15,22 @@ def self.detect!(topic_or_post)
         ::DiscourseAi::LanguageDetector.new(text_for_detection(topic_or_post)).detect
       end
 
-      def self.translate_translatable!(translatable, target_locale_sym = I18n.locale)
-        if (translatable.class.name == "Post")
-          translate_post!(translatable, target_locale_sym)
-        elsif (translatable.class.name == "Topic")
-          translate_topic!(translatable, target_locale_sym)
-        end
-      end
-
-      def self.translate_post!(post, target_locale_sym = I18n.locale)
+      def self.translate_post!(post, target_locale_sym = I18n.locale, opts = {})
         validate_required_settings!
 
-        text = text_for_translation(post, raw: true)
+        raw = opts.key?(:raw) ? opts[:raw] : !opts[:cooked]
+        text = text_for_translation(post, raw:)
         chunks = DiscourseTranslator::ContentSplitter.split(text)
-        translated =
-          chunks
-            .map { |chunk| ::DiscourseAi::PostTranslator.new(chunk, target_locale_sym).translate }
-            .join("")
-        DiscourseTranslator::TranslatedContentNormalizer.normalize(post, translated)
+        chunks
+          .map { |chunk| ::DiscourseAi::PostTranslator.new(chunk, target_locale_sym).translate }
+          .join("")
       end
 
       def self.translate_topic!(topic, target_locale_sym = I18n.locale)
         validate_required_settings!
 
         language = get_language_name(target_locale_sym)
-        translated =
-          ::DiscourseAi::TopicTranslator.new(text_for_translation(topic), language).translate
-        DiscourseTranslator::TranslatedContentNormalizer.normalize(topic, translated)
+        ::DiscourseAi::TopicTranslator.new(text_for_translation(topic), language).translate
       end
 
       def self.translate_text!(text, target_locale_sym = I18n.locale)

app/services/discourse_translator/provider/google.rb

@@ -96,14 +96,15 @@ def self.translate_supported?(source, target)
         end
       end
 
-      def self.translate_translatable!(translatable, target_locale_sym = I18n.locale)
-        res =
-          result(
-            TRANSLATE_URI,
-            q: text_for_translation(translatable),
-            target: SUPPORTED_LANG_MAPPING[target_locale_sym],
-          )
-        res["translations"][0]["translatedText"]
+      def self.translate_post!(post, target_locale_sym = I18n.locale, opts = {})
+        raw = opts.key?(:raw) ? opts[:raw] : !opts[:cooked]
+        text = text_for_translation(post, raw:)
+        translate_text!(text, target_locale_sym)
+      end
+
+      def self.translate_topic!(topic, target_locale_sym = I18n.locale)
+        text = text_for_translation(topic)
+        translate_text!(text, target_locale_sym)
       end
 
       def self.translate_text!(text, target_locale_sym = I18n.locale)

app/services/discourse_translator/provider/libre_translate.rb

@@ -91,18 +91,19 @@ def self.translate_supported?(source, target)
         res.any? { |obj| obj["code"] == source } && res.any? { |obj| obj["code"] == lang }
       end
 
-      def self.translate_translatable!(translatable, target_locale_sym = I18n.locale)
-        detected_lang = detect(translatable)
+      def self.translate_post!(post, target_locale_sym = I18n.locale, opts = {})
+        raw = opts.key?(:raw) ? opts[:raw] : !opts[:cooked]
+        text = text_for_translation(post, raw:)
 
-        res =
-          result(
-            translate_uri,
-            q: text_for_translation(translatable),
-            source: detected_lang,
-            target: SUPPORTED_LANG_MAPPING[target_locale_sym],
-            format: "html",
-          )
-        res["translatedText"]
+        detected_lang = detect(post)
+
+        send_for_translation(text, detected_lang, target_locale_sym)
+      end
+
+      def self.translate_topic!(topic, target_locale_sym = I18n.locale)
+        detected_lang = detect(topic)
+        text = text_for_translation(topic)
+        send_for_translation(text, detected_lang, target_locale_sym)
       end
 
       def self.translate_text!(text, target_locale_sym = I18n.locale)
@@ -154,6 +155,20 @@ def self.result(url, body)
           body
         end
       end
+
+      private
+
+      def self.send_for_translation(text, source_locale, target_locale)
+        res =
+          result(
+            translate_uri,
+            q: text,
+            source: source_locale,
+            target: SUPPORTED_LANG_MAPPING[target_locale],
+            format: "html",
+          )
+        res["translatedText"]
+      end
     end
   end
 end

app/services/discourse_translator/provider/microsoft.rb

@@ -154,27 +154,24 @@ def self.detect!(topic_or_post)
         result(uri.to_s, body, default_headers).first["language"]
       end
 
-      def self.translate_translatable!(translatable, target_locale_sym = I18n.locale)
-        detected_lang = detect(translatable)
+      def self.translate_post!(post, target_locale_sym = I18n.locale, opts = {})
+        raw = opts.key?(:raw) ? opts[:raw] : !opts[:cooked]
+        text = text_for_translation(post, raw:)
 
-        if text_for_translation(translatable).length > LENGTH_LIMIT
-          raise TranslatorError.new(I18n.t("translator.too_long"))
-        end
-        locale =
-          SUPPORTED_LANG_MAPPING[target_locale_sym] || (raise I18n.t("translator.not_supported"))
+        raise TranslatorError.new(I18n.t("translator.too_long")) if text.length > LENGTH_LIMIT
 
-        query = default_query.merge("from" => detected_lang, "to" => locale, "textType" => "html")
-        body = [{ "Text" => text_for_translation(translatable) }].to_json
-        uri = URI(translate_endpoint)
-        uri.query = URI.encode_www_form(query)
-        response_body = result(uri.to_s, body, default_headers)
-        response_body.first["translations"].first["text"]
+        translate_text!(text, target_locale_sym)
       end
 
-      def self.translate_text!(text, target_locale_sym = I18n.locale)
-        locale =
-          SUPPORTED_LANG_MAPPING[target_locale_sym] || (raise I18n.t("translator.not_supported"))
+      def self.translate_topic!(topic, target_locale_sym = I18n.locale)
+        text = text_for_translation(topic)
+        raise TranslatorError.new(I18n.t("translator.too_long")) if text.length > LENGTH_LIMIT
+
+        translate_text!(text, target_locale_sym)
+      end
 
+      def self.translate_text!(text, target_locale_sym = I18n.locale)
+        locale = SUPPORTED_LANG_MAPPING[target_locale_sym]
         query = default_query.merge("to" => locale, "textType" => "html")
         body = [{ "Text" => text }].to_json
         uri = URI(translate_endpoint)