diff --git a/app/models/concerns/discourse_translator/translatable.rb b/app/models/concerns/discourse_translator/translatable.rb new file mode 100644 index 00000000..02948ecb --- /dev/null +++ b/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 diff --git a/app/models/discourse_translator/post_locale.rb b/app/models/discourse_translator/post_locale.rb new file mode 100644 index 00000000..305c7b62 --- /dev/null +++ b/app/models/discourse_translator/post_locale.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module DiscourseTranslator + class PostLocale < ActiveRecord::Base + self.table_name = "discourse_translator_post_locales" + + belongs_to :post + + validates :post_id, presence: true, uniqueness: true + validates :detected_locale, presence: true + end +end + +# == Schema Information +# +# Table name: discourse_translator_post_locales +# +# id :bigint not null, primary key +# post_id :integer not null +# detected_locale :string(20) not null +# created_at :datetime not null +# updated_at :datetime not null +# diff --git a/app/models/discourse_translator/post_translation.rb b/app/models/discourse_translator/post_translation.rb new file mode 100644 index 00000000..a13f8acb --- /dev/null +++ b/app/models/discourse_translator/post_translation.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module DiscourseTranslator + class PostTranslation < ActiveRecord::Base + self.table_name = "discourse_translator_post_translations" + + belongs_to :post + + validates :post_id, presence: true + validates :locale, presence: true + validates :translation, presence: true + validates :locale, uniqueness: { scope: :post_id } + + def self.translation_for(post_id, locale) + find_by(post_id: post_id, locale: locale)&.translation + end + end +end + +# == Schema Information +# +# Table name: discourse_translator_post_translations +# +# id :bigint not null, primary key +# post_id :integer not null +# locale :string not null +# translation :text not null +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# idx_on_post_id_locale_0cc3d81e5b (post_id,locale) UNIQUE +# diff --git a/app/models/discourse_translator/topic_locale.rb b/app/models/discourse_translator/topic_locale.rb new file mode 100644 index 00000000..6646e763 --- /dev/null +++ b/app/models/discourse_translator/topic_locale.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module DiscourseTranslator + class TopicLocale < ActiveRecord::Base + self.table_name = "discourse_translator_topic_locales" + + belongs_to :topic + + validates :topic_id, presence: true, uniqueness: true + validates :detected_locale, presence: true + end +end + +# == Schema Information +# +# Table name: discourse_translator_topic_locales +# +# id :bigint not null, primary key +# topic_id :integer not null +# detected_locale :string(20) not null +# created_at :datetime not null +# updated_at :datetime not null +# diff --git a/app/models/discourse_translator/topic_translation.rb b/app/models/discourse_translator/topic_translation.rb new file mode 100644 index 00000000..07384e05 --- /dev/null +++ b/app/models/discourse_translator/topic_translation.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module DiscourseTranslator + class TopicTranslation < ActiveRecord::Base + self.table_name = "discourse_translator_topic_translations" + + belongs_to :topic + + validates :topic_id, presence: true + validates :locale, presence: true + validates :translation, presence: true + validates :locale, uniqueness: { scope: :topic_id } + + def self.translation_for(topic_id, locale) + find_by(topic_id: topic_id, locale: locale)&.translation + end + end +end + +# == Schema Information +# +# Table name: discourse_translator_topic_translations +# +# id :bigint not null, primary key +# topic_id :integer not null +# locale :string not null +# translation :text not null +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# idx_on_topic_id_locale_70b2f83213 (topic_id,locale) UNIQUE +# diff --git a/app/services/discourse_translator/base.rb b/app/services/discourse_translator/base.rb index 6d927e22..d714825f 100644 --- a/app/services/discourse_translator/base.rb +++ b/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 diff --git a/db/migrate/20250205082400_create_translation_tables.rb b/db/migrate/20250205082400_create_translation_tables.rb new file mode 100644 index 00000000..c9a7037f --- /dev/null +++ b/db/migrate/20250205082400_create_translation_tables.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +class CreateTranslationTables < ActiveRecord::Migration[7.2] + def change + create_table :discourse_translator_topic_locales do |t| + t.integer :topic_id, null: false + t.string :detected_locale, limit: 20, null: false + t.timestamps + end + + create_table :discourse_translator_topic_translations do |t| + t.integer :topic_id, null: false + t.string :locale, null: false + t.text :translation, null: false + t.timestamps + end + + create_table :discourse_translator_post_locales do |t| + t.integer :post_id, null: false + t.string :detected_locale, limit: 20, null: false + t.timestamps + end + + create_table :discourse_translator_post_translations do |t| + t.integer :post_id, null: false + t.string :locale, null: false + t.text :translation, null: false + t.timestamps + end + end +end diff --git a/db/migrate/20250205082401_move_translations_custom_fields_to_table.rb b/db/migrate/20250205082401_move_translations_custom_fields_to_table.rb new file mode 100644 index 00000000..6f43d94e --- /dev/null +++ b/db/migrate/20250205082401_move_translations_custom_fields_to_table.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +class MoveTranslationsCustomFieldsToTable < ActiveRecord::Migration[7.2] + BATCH_SIZE = 1000 + + def up + migrate_custom_fields("topic") + migrate_custom_fields("post") + end + + def down + execute "TRUNCATE discourse_translator_topic_locales" + execute "TRUNCATE discourse_translator_topic_translations" + execute "TRUNCATE discourse_translator_post_locales" + execute "TRUNCATE discourse_translator_post_translations" + end + + private + + def migrate_custom_fields(model) + 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 length(value) <= 20 + AND id >= :start_id + AND id < :end_id + ORDER BY id + ), + 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_detect + ), + to_translate AS ( + SELECT #{model}_id, value::jsonb, created_at, updated_at + FROM #{model}_custom_fields + WHERE name = 'translated_text' + AND value LIKE '{%}' + AND id >= :start_id + AND id < :end_id + ORDER BY id + ), + 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 + ) + SELECT 1 + SQL + start_id += BATCH_SIZE + end + end +end diff --git a/db/migrate/20250205082402_create_translation_indexes.rb b/db/migrate/20250205082402_create_translation_indexes.rb new file mode 100644 index 00000000..6855128e --- /dev/null +++ b/db/migrate/20250205082402_create_translation_indexes.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class CreateTranslationIndexes < ActiveRecord::Migration[7.2] + disable_ddl_transaction! + + def change + add_index :discourse_translator_topic_translations, + %i[topic_id locale], + unique: true, + algorithm: :concurrently + + add_index :discourse_translator_post_translations, + %i[post_id locale], + unique: true, + algorithm: :concurrently + end +end diff --git a/lib/discourse_translator/guardian_extension.rb b/lib/discourse_translator/guardian_extension.rb index 1380651c..5ac094e8 100644 --- a/lib/discourse_translator/guardian_extension.rb +++ b/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, diff --git a/lib/discourse_translator/post_extension.rb b/lib/discourse_translator/post_extension.rb index 08b87d0e..80b0b2c0 100644 --- a/lib/discourse_translator/post_extension.rb +++ b/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 diff --git a/lib/discourse_translator/topic_extension.rb b/lib/discourse_translator/topic_extension.rb index d0e3f4ec..03271c85 100644 --- a/lib/discourse_translator/topic_extension.rb +++ b/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 diff --git a/lib/discourse_translator/topic_view_serializer_extension.rb b/lib/discourse_translator/topic_view_serializer_extension.rb new file mode 100644 index 00000000..546772b8 --- /dev/null +++ b/lib/discourse_translator/topic_view_serializer_extension.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module DiscourseTranslator + module TopicViewSerializerExtension + def posts + if SiteSetting.translator_enabled? + object.instance_variable_set(:@posts, object.posts.includes(:content_locale)) + end + super + end + end +end diff --git a/plugin.rb b/plugin.rb index f74dc226..7e927c71 100644 --- a/plugin.rb +++ b/plugin.rb @@ -16,8 +16,6 @@ module ::DiscourseTranslator PLUGIN_NAME = "discourse-translator".freeze - DETECTED_LANG_CUSTOM_FIELD = "post_detected_lang".freeze - TRANSLATED_CUSTOM_FIELD = "translated_text".freeze LANG_DETECT_NEEDED = "lang_detect_needed".freeze end @@ -27,15 +25,11 @@ 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) Topic.prepend(DiscourseTranslator::TopicExtension) + TopicViewSerializer.prepend(DiscourseTranslator::TopicViewSerializerExtension) end on(:post_process_cooked) do |_, post| diff --git a/spec/fabricators/post_locale.rb b/spec/fabricators/post_locale.rb new file mode 100644 index 00000000..583c505e --- /dev/null +++ b/spec/fabricators/post_locale.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true +Fabricator(:post_locale, from: DiscourseTranslator::PostLocale) do + post + detected_locale { %w[en de es en-GB ja pt pt-BR].sample } +end diff --git a/spec/fabricators/post_translation.rb b/spec/fabricators/post_translation.rb new file mode 100644 index 00000000..4eb8cb24 --- /dev/null +++ b/spec/fabricators/post_translation.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +Fabricator(:post_translation, from: DiscourseTranslator::PostTranslation) do + post + 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 diff --git a/spec/fabricators/topic_locale.rb b/spec/fabricators/topic_locale.rb new file mode 100644 index 00000000..a868646b --- /dev/null +++ b/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 diff --git a/spec/fabricators/topic_translation.rb b/spec/fabricators/topic_translation.rb new file mode 100644 index 00000000..cb75c4da --- /dev/null +++ b/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 diff --git a/spec/jobs/detect_posts_language_spec.rb b/spec/jobs/detect_posts_language_spec.rb index f0605c7b..6dfd1f3c 100644 --- a/spec/jobs/detect_posts_language_spec.rb +++ b/spec/jobs/detect_posts_language_spec.rb @@ -23,7 +23,7 @@ posts.each do |post| post.reload - expect(post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD]).not_to be_nil + expect(post.detected_locale).not_to be_nil end expect(Discourse.redis.smembers(redis_key)).to be_empty @@ -35,7 +35,7 @@ posts.each do |post| post.reload - expect(post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD]).to be_nil + expect(post.detected_locale).to be_nil end expect(Discourse.redis.smembers(redis_key)).to match_array(posts.map(&:id).map(&:to_s)) diff --git a/spec/lib/guardian_extension_spec.rb b/spec/lib/guardian_extension_spec.rb index c9ac4b48..8dafd241 100644 --- a/spec/lib/guardian_extension_spec.rb +++ b/spec/lib/guardian_extension_spec.rb @@ -151,20 +151,19 @@ describe "locale is :xx" do before { I18n.stubs(:locale).returns(:pt) } - it "cannot translate when post does not have DETECTED_LANG_CUSTOM_FIELD" do + it "cannot translate when post does not have detected locale" do + expect(post.detected_locale).to eq(nil) expect(guardian.can_translate?(post)).to eq(false) end - it "cannot translate when post has DETECTED_LANG_CUSTOM_FIELD matches locale" do - post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] = "pt" - post.save + it "cannot translate when post detected locale matches i18n locale" do + post.set_detected_locale("pt") expect(guardian.can_translate?(post)).to eq(false) end - it "can translate when post has DETECTED_LANG_CUSTOM_FIELD does not match locale" do - post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] = "jp" - post.save + it "can translate when post detected locale does not match i18n locale" do + post.set_detected_locale("jp") expect(guardian.can_translate?(post)).to eq(true) end diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 3ee7b4cd..7cf507ec 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -5,26 +5,63 @@ RSpec.describe Post do before { SiteSetting.translator_enabled = true } - describe "translator custom fields" do - let(:post) do - Fabricate( - :post, - raw: "this is a sample post", - custom_fields: { - ::DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD => "en", - ::DiscourseTranslator::TRANSLATED_CUSTOM_FIELD => { - "en" => "lol", - }, - }, - ) + describe "translatable" do + fab!(:post) + + it "should reset translation data when post title has been updated" do + Fabricate(:post_translation, post:) + Fabricate(:post_locale, post:) + post.update!(raw: "this is an updated title") + + expect(DiscourseTranslator::PostLocale.where(post:)).to be_empty + expect(DiscourseTranslator::PostLocale.find_by(post:)).to be_nil end - it "should reset custom fields when post has been updated" do - post.update!(raw: "this is an updated post") + describe "#set_translation" do + it "creates new translation" do + post.set_translation("en", "Hello") + + translation = post.translations.find_by(locale: "en") + expect(translation.translation).to eq("Hello") + end + + it "updates existing translation" do + post.set_translation("en", "Hello") + post.set_translation("en", "Updated hello") + + expect(post.translations.where(locale: "en").count).to eq(1) + expect(post.translation_for("en")).to eq("Updated hello") + end - expect(post.custom_fields[::DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD]).to be_nil + it "converts underscore to hyphen in locale" do + post.set_translation("en_US", "Hello") - expect(post.custom_fields[::DiscourseTranslator::TRANSLATED_CUSTOM_FIELD]).to be_nil + expect(post.translations.find_by(locale: "en-US")).to be_present + expect(post.translations.find_by(locale: "en_US")).to be_nil + end + end + + describe "#translation_for" do + it "returns nil when translation doesn't exist" do + expect(post.translation_for("fr")).to be_nil + end + + it "returns translation when it exists" do + post.set_translation("es", "Hola") + expect(post.translation_for("es")).to eq("Hola") + end + end + + describe "#set_locale" do + it "creates new locale" do + post.set_detected_locale("en-US") + expect(post.content_locale.detected_locale).to eq("en-US") + end + + it "converts underscore to hyphen" do + post.set_detected_locale("en_US") + expect(post.content_locale.detected_locale).to eq("en-US") + end end end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 9746e7d9..d88832ba 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -2,31 +2,66 @@ require "rails_helper" -RSpec.describe Topic do - describe "translator custom fields" do - fab!(:topic) do - Fabricate( - :topic, - title: "this is a sample title", - custom_fields: { - ::DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD => "en", - ::DiscourseTranslator::TRANSLATED_CUSTOM_FIELD => { - "en" => "lol", - }, - }, - ) - end +describe Topic do + describe "translatable" do + fab!(:topic) before { SiteSetting.translator_enabled = true } - after { SiteSetting.translator_enabled = false } - - it "should reset custom fields when topic title has been updated" do + it "should reset translation data when topic title has been updated" do + Fabricate(:topic_translation, topic:) + Fabricate(:topic_locale, topic:) topic.update!(title: "this is an updated title") - expect(topic.custom_fields[::DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD]).to be_nil + expect(DiscourseTranslator::TopicLocale.where(topic:)).to be_empty + expect(DiscourseTranslator::TopicLocale.find_by(topic:)).to be_nil + end + + describe "#set_translation" do + it "creates new translation" do + topic.set_translation("en", "Hello") + + translation = topic.translations.find_by(locale: "en") + expect(translation.translation).to eq("Hello") + end + + it "updates existing translation" do + topic.set_translation("en", "Hello") + topic.set_translation("en", "Updated hello") + + expect(topic.translations.where(locale: "en").count).to eq(1) + expect(topic.translation_for("en")).to eq("Updated hello") + end + + it "converts underscore to hyphen in locale" do + topic.set_translation("en_US", "Hello") + + expect(topic.translations.find_by(locale: "en-US")).to be_present + expect(topic.translations.find_by(locale: "en_US")).to be_nil + end + end + + describe "#translation_for" do + it "returns nil when translation doesn't exist" do + expect(topic.translation_for("fr")).to be_nil + end + + it "returns translation when it exists" do + topic.set_translation("es", "Hola") + expect(topic.translation_for("es")).to eq("Hola") + end + end + + describe "#set_locale" do + it "creates new locale" do + topic.set_detected_locale("en-US") + expect(topic.content_locale.detected_locale).to eq("en-US") + end - expect(topic.custom_fields[::DiscourseTranslator::TRANSLATED_CUSTOM_FIELD]).to be_nil + it "converts underscore to hyphen" do + topic.set_detected_locale("en_US") + expect(topic.content_locale.detected_locale).to eq("en-US") + end end end end diff --git a/spec/models/translation_tables_spec.rb b/spec/models/translation_tables_spec.rb new file mode 100644 index 00000000..091e4253 --- /dev/null +++ b/spec/models/translation_tables_spec.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +require_relative "../../db/migrate/20250205082401_move_translations_custom_fields_to_table" + +module DiscourseTranslator + describe MoveTranslationsCustomFieldsToTable do + DETECTED_LANG_CUSTOM_FIELD = "post_detected_lang".freeze + TRANSLATED_CUSTOM_FIELD = "translated_text".freeze + + let!(:batch_size) { 3 } + + before { described_class.const_set(:BATCH_SIZE, batch_size) } + + def create_translation_custom_fields(count) + count.times do + t_post = Fabricate(:post) + t_topic = Fabricate(:topic) + + t_post.custom_fields[DETECTED_LANG_CUSTOM_FIELD] = "pt" + t_post.save_custom_fields + + t_topic.custom_fields[DETECTED_LANG_CUSTOM_FIELD] = "es" + t_topic.save_custom_fields + + t_post.custom_fields[TRANSLATED_CUSTOM_FIELD] = { + en_GB: "The Romance of the Three Kingdoms", + de: "Die Romanze der Drei Königreiche", + } + t_post.save_custom_fields + + t_topic.custom_fields[TRANSLATED_CUSTOM_FIELD] = { + en_GB: "The Romance of the Three Kingdoms", + de: "Die Romanze der Drei Königreiche", + } + t_topic.save_custom_fields + end + end + + it "correctly migrates custom fields in batches" do + # batch size is 3 + create_translation_custom_fields(4) + # create some random custom fields in between + # to test the migrate loop doesn't end prematurely + 4.times do + post = Fabricate(:post) + post.custom_fields["x"] = "x" + post.save_custom_fields + + topic = Fabricate(:topic) + topic.custom_fields["x"] = "x" + topic.save_custom_fields + end + # another 4 + create_translation_custom_fields(4) + + migration = described_class.new + migration.up + + expect(PostLocale.count).to eq(8) + expect(PostTranslation.count).to eq(16) + + expect(TopicLocale.count).to eq(8) + expect(TopicTranslation.count).to eq(16) + + expect(PostLocale.last.detected_locale).to eq("pt") + + expect(PostTranslation.where(post_id: Post.last.id).pluck(:locale, :translation)).to include( + ["en_GB", "The Romance of the Three Kingdoms"], + ["de", "Die Romanze der Drei Königreiche"], + ) + + migration.down + expect(PostLocale.count).to eq(0) + expect(PostTranslation.count).to eq(0) + expect(TopicLocale.count).to eq(0) + expect(TopicTranslation.count).to eq(0) + end + + it "ignores invalid JSON in translated_text" do + post = Fabricate(:post) + post.custom_fields[TRANSLATED_CUSTOM_FIELD] = "invalid json" + post.save_custom_fields(true) + + migration = described_class.new + expect { migration.up }.not_to raise_error + expect(PostTranslation.count).to eq(0) + end + + it "ignores translations with locale longer than 20 chars" do + post = Fabricate(:post) + post.custom_fields[TRANSLATED_CUSTOM_FIELD] = { very_very_long_locale_name: "test" } + post.custom_fields[DETECTED_LANG_CUSTOM_FIELD] = "very_very_long_locale_name" + post.save_custom_fields(true) + + migration = described_class.new + expect { migration.up }.not_to raise_error + + expect(PostLocale.count).to eq(0) + expect(PostTranslation.count).to eq(0) + end + end +end diff --git a/spec/serializers/post_serializer_spec.rb b/spec/serializers/post_serializer_spec.rb index 04d46b48..c3b230cb 100644 --- a/spec/serializers/post_serializer_spec.rb +++ b/spec/serializers/post_serializer_spec.rb @@ -58,20 +58,19 @@ I18n.stubs(:locale).returns(:pt) end - it "cannot translate when post does not have DETECTED_LANG_CUSTOM_FIELD" do + it "cannot translate when post does not have detected locale" do + expect(post.detected_locale).to eq(nil) expect(serializer.can_translate).to eq(false) end - it "cannot translate when post has DETECTED_LANG_CUSTOM_FIELD matches locale" do - post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] = "pt" - post.save + it "cannot translate when post detected locale matches i18n locale" do + post.set_detected_locale("pt") expect(serializer.can_translate).to eq(false) end - it "can translate when post has DETECTED_LANG_CUSTOM_FIELD does not match locale" do - post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] = "jp" - post.save + it "can translate when post detected locale does not match i18n locale" do + post.set_detected_locale("jp") expect(serializer.can_translate).to eq(true) end diff --git a/spec/serializers/topic_view_serializer_spec.rb b/spec/serializers/topic_view_serializer_spec.rb new file mode 100644 index 00000000..58c1bc79 --- /dev/null +++ b/spec/serializers/topic_view_serializer_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe TopicViewSerializer do + fab!(:user) + fab!(:topic) + fab!(:post1) { Fabricate(:post, topic: topic).set_detected_locale("en") } + fab!(:post2) { Fabricate(:post, topic: topic).set_detected_locale("es") } + fab!(:post3) { Fabricate(:post, topic: topic).set_detected_locale("ja") } + + before do + SiteSetting.translator_enabled = true + SiteSetting.restrict_translation_by_group = "#{Group::AUTO_GROUPS[:everyone]}" + SiteSetting.restrict_translation_by_poster_group = "#{Group::AUTO_GROUPS[:everyone]}" + end + + it "preloads translations without N+1 queries" do + topic_view = TopicView.new(topic) + serializer = TopicViewSerializer.new(topic_view, scope: Guardian.new(user), root: false) + + # ensure translation data is included in the JSON + json = {} + queries = track_sql_queries { json = serializer.as_json } + posts_json = json[:post_stream][:posts] + expect(posts_json.map { |p| p[:can_translate] }).to eq([false, true, true]) + + translation_queries = queries.count { |q| q.include?("discourse_translator_post_locales") } + expect(translation_queries).to eq(1) # would be 3 (posts) if not preloaded + + expect(topic_view.posts.first.association(:content_locale)).to be_loaded + end +end diff --git a/spec/services/amazon_spec.rb b/spec/services/amazon_spec.rb index 87c26b18..d8b4e0fd 100644 --- a/spec/services/amazon_spec.rb +++ b/spec/services/amazon_spec.rb @@ -37,11 +37,7 @@ it "should store the detected language in a custom field" do expect(described_class.detect(post)).to eq(detected_lang) - 2.times do - expect(post.custom_fields[::DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD]).to eq( - detected_lang, - ) - end + expect(post.detected_locale).to eq(detected_lang) end it "should fail graciously when the cooked translated text is blank" do @@ -65,8 +61,7 @@ }, ) described_class.stubs(:client).returns(client) - post.custom_fields[::DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] = "en" - post.save_custom_fields + post.set_detected_locale("en") I18n.stubs(:locale).returns(:es) end diff --git a/spec/services/base_spec.rb b/spec/services/base_spec.rb index 3d9aa5f0..0f224ddd 100644 --- a/spec/services/base_spec.rb +++ b/spec/services/base_spec.rb @@ -116,7 +116,6 @@ class EmptyTranslator < DiscourseTranslator::Base end it "performs detection if no cached result" do - TestTranslator.save_detected_locale(post) { nil } TestTranslator.expects(:detect!).with(post).returns("es") expect(TestTranslator.detect(post)).to eq("es") @@ -147,7 +146,6 @@ class EmptyTranslator < DiscourseTranslator::Base it "raises error when translation not supported" do TestTranslator.save_detected_locale(post) { "xx" } - TestTranslator.save_translation(post) { nil } TestTranslator.expects(:translate_supported?).with("xx", :en).returns(false) expect { TestTranslator.translate(post) }.to raise_error(DiscourseTranslator::TranslatorError) @@ -155,7 +153,6 @@ class EmptyTranslator < DiscourseTranslator::Base it "performs translation when needed" do TestTranslator.save_detected_locale(post) { "es" } - TestTranslator.save_translation(post) { nil } TestTranslator.expects(:translate!).returns("hello") expect(TestTranslator.translate(post)).to eq(%w[es hello]) diff --git a/spec/services/discourse_ai_spec.rb b/spec/services/discourse_ai_spec.rb index c7cf1188..e46b4b4b 100644 --- a/spec/services/discourse_ai_spec.rb +++ b/spec/services/discourse_ai_spec.rb @@ -26,23 +26,19 @@ end end - describe ".detect" do - it "stores the detected language in a custom field" do + describe ".detect!" do + it "stores the detected language" do locale = "de" DiscourseAi::Completions::Llm.with_prepared_responses(["de"]) do - DiscourseTranslator::DiscourseAi.detect(post) - post.save_custom_fields + DiscourseTranslator::DiscourseAi.detect!(post) end - expect(post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD]).to eq locale + expect(post.detected_locale).to eq locale end end describe ".translate" do - before do - post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] = "de" - post.save_custom_fields - end + before { post.set_detected_locale("de") } it "translates the post and returns [locale, translated_text]" do DiscourseAi::Completions::Llm.with_prepared_responses( diff --git a/spec/services/google_spec.rb b/spec/services/google_spec.rb index 746ff66a..01d92553 100644 --- a/spec/services/google_spec.rb +++ b/spec/services/google_spec.rb @@ -35,12 +35,7 @@ .once expect(described_class.detect(post)).to eq(detected_lang) - - 2.times do - expect(post.custom_fields[::DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD]).to eq( - detected_lang, - ) - end + expect(post.detected_locale).to eq(detected_lang) end it "should truncate string to 1000 characters" do @@ -168,8 +163,7 @@ end it "returns error with source and target locale when translation is not supported" do - post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] = "cat" - post.save_custom_fields + post.set_detected_locale("cat") I18n.stubs(:locale).returns(:dog) Excon.expects(:post).returns( @@ -184,8 +178,7 @@ it "truncates text for translation to max_characters_per_translation setting" do SiteSetting.max_characters_per_translation = 50 post.cooked = "a" * 100 - post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] = "de" - post.save_custom_fields + post.set_detected_locale("de") body = { q: post.cooked.truncate(SiteSetting.max_characters_per_translation, omission: nil), source: "de", diff --git a/spec/services/microsoft_spec.rb b/spec/services/microsoft_spec.rb index 1ca1f22a..7c2cfb80 100644 --- a/spec/services/microsoft_spec.rb +++ b/spec/services/microsoft_spec.rb @@ -20,12 +20,10 @@ def detect_endpoint before { SiteSetting.translator_azure_subscription_key = "e1bba646088021aaf1ef972a48" } shared_examples "language detected" do - it "stores detected language in a custom field" do + it "stores detected language" do described_class.detect(post) - expect(post.custom_fields[::DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD]).to eq( - detected_lang, - ) + expect(post.detected_locale).to eq(detected_lang) end end @@ -131,8 +129,7 @@ def translate_endpoint end before do - post.custom_fields = { DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD => "en" } - post.save_custom_fields + post.set_detected_locale("en") SiteSetting.translator_azure_subscription_key = "e1bba646088021aaf1ef972a48" end @@ -161,21 +158,14 @@ def translate_endpoint it "returns stored translation if post has already been translated" do I18n.locale = "en" - post.custom_fields = { - DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD => "tr", - DiscourseTranslator::TRANSLATED_CUSTOM_FIELD => { - "en" => "some english text", - }, - } - - post.save_custom_fields + post.set_detected_locale("tr") + post.set_translation("en", "some english text") expect(described_class.translate(post)).to eq(["tr", "some english text"]) end it "raises an error if detected language of the post is not supported" do - post.custom_fields = { DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD => "donkey" } - post.save_custom_fields + post.set_detected_locale("donkey") expect { described_class.translate(post) }.to raise_error( DiscourseTranslator::TranslatorError, diff --git a/spec/services/yandex_spec.rb b/spec/services/yandex_spec.rb index 24ce0e41..3a59ecf1 100644 --- a/spec/services/yandex_spec.rb +++ b/spec/services/yandex_spec.rb @@ -28,11 +28,7 @@ .once expect(described_class.detect(post)).to eq(detected_lang) - 2.times do - expect(post.custom_fields[::DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD]).to eq( - detected_lang, - ) - end + expect(post.detected_locale).to eq(detected_lang) end end