wip

Author: nattsw

app/models/concerns/discourse_translator/translatable.rb

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+module DiscourseTranslator
+  module Translatable
+    extend ActiveSupport::Concern
+
+    prepended do
+      has_many :translations,
+               class_name: "DiscourseTranslator::#{name}Translation",
+               dependent: :destroy
+      has_one :content_locale, class_name: "DiscourseTranslator::#{name}Locale", dependent: :destroy
+    end
+
+    def set_detected_locale(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)
+    end
+
+    def set_translation(locale, text)
+      locale = locale.to_s.gsub("_", "-")
+      translations.find_or_initialize_by(locale: locale).update!(translation: text)
+    end
+
+    def translation_for(locale)
+      translations.find_by(locale: locale)&.translation
+    end
+
+    def detected_locale
+      content_locale&.detected_locale
+    end
+
+    private
+
+    def clear_translations
+      return if !SiteSetting.translator_enabled
+
+      translations.delete_all
+      content_locale&.destroy
+    end
+  end
+end

app/services/discourse_translator/base.rb

@@ -27,14 +27,14 @@ def self.cache_key
     # 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.
     # If the detected language is the same as the target language, the original text will be returned.
-    # @param topic_or_post [Post|Topic]
-    def self.translate(topic_or_post)
-      return if text_for_translation(topic_or_post).blank?
-      detected_lang = detect(topic_or_post)
+    # @param translatable [Post|Topic]
+    def self.translate(translatable)
+      return if text_for_translation(translatable).blank?
+      detected_lang = detect(translatable)
 
-      return detected_lang, get_text(topic_or_post) if (detected_lang&.to_s == I18n.locale.to_s)
+      return detected_lang, get_text(translatable) if (detected_lang&.to_s == I18n.locale.to_s)
 
-      existing_translation = get_translation(topic_or_post)
+      existing_translation = get_translation(translatable)
       return detected_lang, existing_translation if existing_translation.present?
 
       unless translate_supported?(detected_lang, I18n.locale)
@@ -46,27 +46,27 @@ def self.translate(topic_or_post)
                 ),
               )
       end
-      [detected_lang, translate!(topic_or_post)]
+      [detected_lang, translate!(translatable)]
     end
 
     # Subclasses must implement this method to translate the text of a post or topic
     # then use the save_translation method to store the translated text.
-    # @param topic_or_post [Post|Topic]
-    def self.translate!(topic_or_post)
+    # @param translatable [Post|Topic]
+    def self.translate!(translatable)
       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 topic_or_post [Post|Topic]
-    def self.detect(topic_or_post)
-      return if text_for_detection(topic_or_post).blank?
-      get_detected_locale(topic_or_post) || detect!(topic_or_post)
+    # @param translatable [Post|Topic]
+    def self.detect(translatable)
+      return if text_for_detection(translatable).blank?
+      get_detected_locale(translatable) || detect!(translatable)
     end
 
-    # Subclasses must implement this method to translate the text of a post or topic
-    # then use the save_translation method to store the translated text.
-    # @param topic_or_post [Post|Topic]
+    # Subclasses must implement this method to detect the text of a post or topic
+    # then use the save_detected_locale method to store the detected locale.
+    # @param translatable [Post|Topic]
     def self.detect!(post)
       raise "Not Implemented"
     end
@@ -75,52 +75,33 @@ def self.access_token
       raise "Not Implemented"
     end
 
-    def self.get_translation(topic_or_post)
-      translated_custom_field =
-        topic_or_post.custom_fields[DiscourseTranslator::TRANSLATED_CUSTOM_FIELD] || {}
-      translated_custom_field[I18n.locale]
+    def self.get_translation(translatable)
+      translatable.translation_for(I18n.locale)
     end
 
-    def self.save_translation(topic_or_post)
-      translated_custom_field =
-        topic_or_post.custom_fields[DiscourseTranslator::TRANSLATED_CUSTOM_FIELD] || {}
-      translated_text = translated_custom_field[I18n.locale]
-
-      if translated_text.nil?
-        translated_text = yield
-
-        topic_or_post.custom_fields[
-          DiscourseTranslator::TRANSLATED_CUSTOM_FIELD
-        ] = translated_custom_field.merge(I18n.locale => translated_text)
-
-        topic_or_post.save!
-      end
-
-      translated_text
+    def self.save_translation(translatable)
+      translation = yield
+      translatable.set_translation(I18n.locale, translation)
+      translation
     end
 
-    def self.get_detected_locale(topic_or_post)
-      topic_or_post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD]
+    def self.get_detected_locale(translatable)
+      translatable.detected_locale
     end
 
-    def self.save_detected_locale(topic_or_post)
+    def self.save_detected_locale(translatable)
       detected_locale = yield
-      topic_or_post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] = detected_locale
-
-      if !topic_or_post.custom_fields_clean?
-        topic_or_post.save_custom_fields
-        topic_or_post.publish_change_to_clients!(:revised) if topic_or_post.class.name == "Post"
-      end
+      translatable.set_detected_locale(detected_locale)
 
       detected_locale
     end
 
-    def self.get_text(topic_or_post)
-      case topic_or_post.class.name
+    def self.get_text(translatable)
+      case translatable.class.name
       when "Post"
-        topic_or_post.cooked
+        translatable.cooked
       when "Topic"
-        topic_or_post.title
+        translatable.title
       end
     end
 
@@ -143,15 +124,12 @@ def self.strip_tags_for_detection(detection_text)
       html_doc.to_html
     end
 
-    def self.text_for_detection(topic_or_post)
-      strip_tags_for_detection(get_text(topic_or_post)).truncate(
-        DETECTION_CHAR_LIMIT,
-        omission: nil,
-      )
+    def self.text_for_detection(translatable)
+      strip_tags_for_detection(get_text(translatable)).truncate(DETECTION_CHAR_LIMIT, omission: nil)
     end
 
-    def self.text_for_translation(topic_or_post)
-      get_text(topic_or_post).truncate(SiteSetting.max_characters_per_translation, omission: nil)
+    def self.text_for_translation(translatable)
+      get_text(translatable).truncate(SiteSetting.max_characters_per_translation, omission: nil)
     end
   end
 end

db/migrate/20250205082401_move_translations_custom_fields_to_table.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class MoveTranslationsCustomFieldsToTable < ActiveRecord::Migration[7.2] # frozen_string_literal: true
+class MoveTranslationsCustomFieldsToTable < ActiveRecord::Migration[7.2]
   BATCH_SIZE = 1000
 
   def up
@@ -18,42 +18,50 @@ def down
   private
 
   def migrate_custom_fields(model)
-    start_id = 0
-    loop do
-      last_id = DB.query_single(<<~SQL, model:, start_id:, limit: BATCH_SIZE)
-        WITH to_insert AS (
-          SELECT id, #{model}_id, value
+    bounds = DB.query_single(<<~SQL, model:)
+      SELECT
+        COALESCE(MIN(id), 0) as min_id,
+        COALESCE(MAX(id), 0) as max_id
+      FROM #{model}_custom_fields
+      WHERE name IN ('post_detected_lang', 'translated_text')
+    SQL
+
+    start_id = bounds[0]
+    max_id = bounds[1]
+
+    while start_id < max_id
+      DB.exec(<<~SQL, model:, start_id:, end_id: start_id + BATCH_SIZE)
+        WITH to_detect AS (
+          SELECT #{model}_id, value
           FROM #{model}_custom_fields
           WHERE name = 'post_detected_lang'
-          AND id > :start_id
+          AND id >= :start_id
+          AND id < :end_id
           ORDER BY id
-          LIMIT :limit
         ),
-        do_insert AS (
+        do_detect AS (
           INSERT INTO discourse_translator_#{model}_locales (#{model}_id, detected_locale, created_at, updated_at)
           SELECT #{model}_id, value, NOW(), NOW()
-          FROM to_insert
+          FROM to_detect
         ),
         to_translate AS (
-          SELECT id, #{model}_id, value::jsonb, created_at, updated_at
+          SELECT #{model}_id, value::jsonb, created_at, updated_at
           FROM #{model}_custom_fields
-          WHERE id > :start_id
-          AND name = 'translated_text'
+          WHERE name = 'translated_text'
           AND value LIKE '{%}'
+          AND id >= :start_id
+          AND id < :end_id
           ORDER BY id
-          LIMIT :limit
         ),
         do_translate AS (
           INSERT INTO discourse_translator_#{model}_translations (#{model}_id, locale, translation, created_at, updated_at)
           SELECT b.#{model}_id, jb.key as locale, jb.value as translation, b.created_at, b.updated_at
           FROM to_translate b, jsonb_each_text(b.value) jb
           WHERE LENGTH(jb.key) <= 20
-        ),
-        max_value AS (SELECT COALESCE(GREATEST((SELECT MAX(id) FROM to_insert), (SELECT MAX(id) FROM to_translate) ), -1) as max_id)
-        SELECT max_id FROM max_value
+        )
+        SELECT 1
       SQL
-      start_id = last_id.last
-      break if start_id == -1
+      start_id += BATCH_SIZE
     end
   end
 end

lib/discourse_translator/guardian_extension.rb

@@ -22,14 +22,11 @@ def can_detect_language?(post)
   def can_translate?(post)
     return false if !user_group_allow_translate?
 
-    # we will deal with regionalized_strings (not syms) when comparing locales
-    # e.g. "en_GB"
-    #      not "en-GB"
-    #      nor :en_GB (I18n.locale)
-    detected_lang =
-      post.custom_fields[::DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD].to_s.sub("-", "_")
-    return false if detected_lang.blank?
+    locale = post.detected_locale
+    return false if locale.nil?
 
+    # I18n.locale is a symbol e.g. :en_GB
+    detected_lang = locale.to_s.sub("-", "_")
     detected_lang != I18n.locale.to_s &&
       "DiscourseTranslator::#{SiteSetting.translator}".constantize.language_supported?(
         detected_lang,

lib/discourse_translator/post_extension.rb

@@ -3,16 +3,7 @@
 module DiscourseTranslator
   module PostExtension
     extend ActiveSupport::Concern
-
     prepended { before_update :clear_translations, if: :raw_changed? }
-
-    private
-
-    def clear_translations
-      return if !SiteSetting.translator_enabled
-
-      self.custom_fields.delete(DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD)
-      self.custom_fields.delete(DiscourseTranslator::TRANSLATED_CUSTOM_FIELD)
-    end
+    include Translatable
   end
 end

lib/discourse_translator/topic_extension.rb

@@ -3,16 +3,7 @@
 module DiscourseTranslator
   module TopicExtension
     extend ActiveSupport::Concern
-
     prepended { before_update :clear_translations, if: :title_changed? }
-
-    private
-
-    def clear_translations
-      return if !SiteSetting.translator_enabled
-
-      self.custom_fields.delete(DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD)
-      self.custom_fields.delete(DiscourseTranslator::TRANSLATED_CUSTOM_FIELD)
-    end
+    include Translatable
   end
 end

plugin.rb

@@ -27,11 +27,6 @@ module ::DiscourseTranslator
   register_problem_check ProblemCheck::MissingTranslatorApiKey
   register_problem_check ProblemCheck::TranslatorError
 
-  Post.register_custom_field_type(::DiscourseTranslator::TRANSLATED_CUSTOM_FIELD, :json)
-  Topic.register_custom_field_type(::DiscourseTranslator::TRANSLATED_CUSTOM_FIELD, :json)
-
-  topic_view_post_custom_fields_allowlister { [::DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] }
-
   reloadable_patch do
     Guardian.prepend(DiscourseTranslator::GuardianExtension)
     Post.prepend(DiscourseTranslator::PostExtension)

spec/fabricators/topic_locale.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+Fabricator(:topic_locale, from: DiscourseTranslator::TopicLocale) do
+  topic
+  detected_locale { %w[en de es en-GB ja pt pt-BR].sample }
+end

spec/fabricators/topic_translation.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+Fabricator(:topic_translation, from: DiscourseTranslator::TopicTranslation) do
+  topic
+  locale { %w[en de es en-GB ja pt pt-BR].sample }
+  translation do |attrs|
+    {
+      "en" => "Hello",
+      "de" => "Hallo",
+      "es" => "Hola",
+      "en-GB" => "Hello",
+      "ja" => "こんにちは",
+      "pt" => "Olá",
+      "pt-BR" => "Olá",
+    }[
+      attrs[:locale]
+    ]
+  end
+end