FIX: Revert translating raw for translator providers as they strip newlines (#250)

In 6c197c5, we changed to translate raw instead of cooked. This was largely due to DiscourseAI.

However some tests have shown that the original decision of translating cooked is likely due to translation providers like Google or Azure (and the rest) stripping newlines when translating. However these translator providers keep html as-is, so translating cooked is reasonable.

This commit reverts the other providers to use cooked, but keeps DiscourseAI as the only provider using raw.

The following is a trivial example, but in some more complex cases, the absence of certain characters (besides newline) removed by the provider when dealing with raw is a bit more jarring.

Author: nattsw

app/services/discourse_translator/base.rb

@@ -33,7 +33,7 @@ def self.translate(translatable, target_locale_sym = I18n.locale)
       detected_lang = detect(translatable)
 
       if translatable.locale_matches?(target_locale_sym)
-        return detected_lang, get_untranslated_cooked(translatable)
+        return detected_lang, get_untranslated(translatable)
       end
 
       translation = translatable.translation_for(target_locale_sym)
@@ -50,9 +50,7 @@ def self.translate(translatable, target_locale_sym = I18n.locale)
       end
 
       translated = translate!(translatable, target_locale_sym)
-      save_translation(translatable, target_locale_sym) do
-        TranslatedContentNormalizer.normalize(translatable, translated)
-      end
+      save_translation(translatable, target_locale_sym) { translated }
       [detected_lang, translated]
     end
 
@@ -125,27 +123,18 @@ def self.translate_supported?(detected_lang, target_lang)
     private
 
     def self.text_for_detection(translatable)
-      get_untranslated_raw(translatable).truncate(DETECTION_CHAR_LIMIT, omission: nil)
+      get_untranslated(translatable, raw: true).truncate(DETECTION_CHAR_LIMIT, omission: nil)
     end
 
-    def self.text_for_translation(translatable)
+    def self.text_for_translation(translatable, raw: false)
       max_char = SiteSetting.max_characters_per_translation
-      get_untranslated_raw(translatable).truncate(max_char, omission: nil)
-    end
-
-    def self.get_untranslated_raw(translatable)
-      case translatable.class.name
-      when "Post"
-        translatable.raw
-      when "Topic"
-        translatable.title
-      end
+      get_untranslated(translatable, raw:).truncate(max_char, omission: nil)
     end
 
-    def self.get_untranslated_cooked(translatable)
+    def self.get_untranslated(translatable, raw: false)
       case translatable.class.name
       when "Post"
-        translatable.cooked
+        raw ? translatable.raw : translatable.cooked
       when "Topic"
         translatable.title
       end

app/services/discourse_translator/discourse_ai.rb

@@ -33,7 +33,12 @@ def self.translate!(translatable, target_locale_sym = I18n.locale)
                 ),
               )
       end
-      ::DiscourseAi::Translator.new(text_for_translation(translatable), target_locale_sym).translate
+      translated =
+        ::DiscourseAi::Translator.new(
+          text_for_translation(translatable, raw: true),
+          target_locale_sym,
+        ).translate
+      DiscourseTranslator::TranslatedContentNormalizer.normalize(translatable, translated)
     end
 
     private

spec/jobs/automatic_translation_backfill_spec.rb

@@ -102,7 +102,7 @@ def expect_google_translate(text)
         described_class.new.execute
 
         expect(topic.translations.pluck(:locale, :translation)).to eq([%w[es hola]])
-        expect(post.translations.pluck(:locale, :translation)).to eq([%w[de <p>hallo</p>]])
+        expect(post.translations.pluck(:locale, :translation)).to eq([%w[de hallo]])
       end
     end
 
@@ -126,7 +126,7 @@ def expect_google_translate(text)
 
         expect(topic.translations.pluck(:locale, :translation)).to eq([%w[de hallo]])
         expect(posts.map { |p| p.translations.pluck(:locale, :translation).flatten }).to eq(
-          [%w[de <p>hallo</p>]] * 4,
+          [%w[de hallo]] * 4,
         )
       end
     end

spec/services/amazon_spec.rb

@@ -19,7 +19,7 @@
   describe ".detect" do
     let(:post) { Fabricate(:post) }
     let!(:client) { Aws::Translate::Client.new(stub_responses: true) }
-    let(:text) { described_class.truncate(post.raw) }
+    let(:text) { described_class.truncate(post.cooked) }
     let(:detected_lang) { "en" }
 
     before do
@@ -40,7 +40,7 @@
       expect(post.detected_locale).to eq(detected_lang)
     end
 
-    it "should fail graciously when the raw translated text is blank" do
+    it "should fail graciously when the cooked translated text is blank" do
       post.raw = ""
       expect(described_class.detect(post)).to be_nil
     end

spec/services/base_spec.rb

@@ -52,11 +52,18 @@ class EmptyTranslator < DiscourseTranslator::Base
     fab!(:post)
 
     it "truncates to max_characters_per_translation" do
-      post.raw = "a" * (SiteSetting.max_characters_per_translation + 1)
+      post.cooked = "a" * (SiteSetting.max_characters_per_translation + 1)
       expect(DiscourseTranslator::Base.text_for_translation(post).length).to eq(
         SiteSetting.max_characters_per_translation,
       )
     end
+
+    it "uses raw if required" do
+      post.raw = "a" * (SiteSetting.max_characters_per_translation + 1)
+      expect(DiscourseTranslator::Base.text_for_translation(post, raw: true).length).to eq(
+        SiteSetting.max_characters_per_translation,
+      )
+    end
   end
 
   describe ".detect" do
@@ -93,15 +100,15 @@ class EmptyTranslator < DiscourseTranslator::Base
     fab!(:post)
 
     it "returns nil when text is blank" do
-      post.raw = ""
+      post.cooked = ""
       expect(TestTranslator.translate(post)).to be_nil
     end
 
     it "returns original text when detected language matches current locale" do
       TestTranslator.save_detected_locale(post) { I18n.locale.to_s }
-      post.update(raw: "hello")
+      post.cooked = "hello"
 
-      expect(TestTranslator.translate(post)).to eq(%w[en <p>hello</p>])
+      expect(TestTranslator.translate(post)).to eq(%w[en hello])
     end
 
     it "returns cached translation if available" do

spec/services/discourse_ai_spec.rb

@@ -42,7 +42,7 @@
       DiscourseAi::Completions::Llm.with_prepared_responses(["some translated text"]) do
         locale, translated_text = DiscourseTranslator::DiscourseAi.translate(post)
         expect(locale).to eq "de"
-        expect(translated_text).to eq "some translated text"
+        expect(translated_text).to eq "<p>some translated text</p>"
       end
     end
   end

spec/services/google_spec.rb

@@ -149,10 +149,10 @@
 
     it "truncates text for translation to max_characters_per_translation setting" do
       SiteSetting.max_characters_per_translation = 50
-      post.raw = "a" * 100
+      post.cooked = "a" * 100
       post.set_detected_locale("de")
       body = {
-        q: post.raw.truncate(SiteSetting.max_characters_per_translation, omission: nil),
+        q: post.cooked.truncate(SiteSetting.max_characters_per_translation, omission: nil),
         source: "de",
         target: "en",
         key: api_key,

spec/services/libre_translate_spec.rb

@@ -44,7 +44,7 @@
     it "truncates text for translation to max_characters_per_translation setting" do
       SiteSetting.max_characters_per_translation = 50
       post.set_detected_locale("de")
-      body = { q: post.raw, source: "de", target: "en", format: "html", api_key: api_key }
+      body = { q: post.cooked, source: "de", target: "en", format: "html", api_key: api_key }
 
       translated_text = "hur dur hur dur"
       # https://publicapi.dev/libre-translate-api

spec/services/microsoft_spec.rb

@@ -176,7 +176,7 @@ def translate_endpoint
       it "raises an error if the post is too long to be translated" do
         I18n.locale = "ja"
         SiteSetting.max_characters_per_translation = 100_000
-        post.update_columns(raw: "*" * (DiscourseTranslator::Microsoft::LENGTH_LIMIT + 1))
+        post.update_columns(cooked: "*" * (DiscourseTranslator::Microsoft::LENGTH_LIMIT + 1))
 
         expect { described_class.translate(post) }.to raise_error(
           DiscourseTranslator::TranslatorError,
@@ -190,7 +190,7 @@ def translate_endpoint
           :post,
           "https://api.cognitive.microsofttranslator.com/translate?api-version=3.0&from=en&textType=html&to=ja",
         ).with(
-          body: "[{\"Text\":\"Hello world\"}]",
+          body: "[{\"Text\":\"\\u003cp\\u003eHello world\\u003c/p\\u003e\"}]",
           headers: {
             "Ocp-Apim-Subscription-Key" => SiteSetting.translator_azure_subscription_key,
             "Content-Type" => "application/json",