Skip to content

Commit 7fc45d5

Browse files
authored
DEV: Move translation custom fields into their own tables (#201)
Currently, `Topic` and `Post` have detected_language and translations in custom fields, e.g. ``` post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] post.custom_fields[DiscourseTranslator::TRANSLATED_CUSTOM_FIELD] topic.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] topic.custom_fields[DiscourseTranslator::TRANSLATED_CUSTOM_FIELD] ``` We are moving this into 4 tables/models ``` post has_one :content_locale, class_name: "DiscourseTranslator::PostLocale" post has_many :translations, class_name: "DiscourseTranslator::PostTranslation" topic has_one :content_locale, class_name: "DiscourseTranslator::TopicLocale" topic has_many :translations, class_name: "DiscourseTranslator::TopicTranslation" ``` Since there are a lot of duplicates, this is implemented on the `Post` and `Topic` using a `Concern`, and any future translatable content can inherit this concern. This PR also gets rid of the previous N+1 which happens when determining if the 🌐 translate button should appear for each post.
1 parent 5b1c4d4 commit 7fc45d5

31 files changed

+644
-191
lines changed
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# frozen_string_literal: true
2+
3+
module DiscourseTranslator
4+
module Translatable
5+
extend ActiveSupport::Concern
6+
7+
prepended do
8+
has_many :translations,
9+
class_name: "DiscourseTranslator::#{name}Translation",
10+
dependent: :destroy
11+
has_one :content_locale, class_name: "DiscourseTranslator::#{name}Locale", dependent: :destroy
12+
end
13+
14+
def set_detected_locale(locale)
15+
# locales should be "en-US" instead of "en_US" per https://www.rfc-editor.org/rfc/rfc5646#section-2.1
16+
locale = locale.to_s.gsub("_", "-")
17+
(content_locale || build_content_locale).update!(detected_locale: locale)
18+
end
19+
20+
def set_translation(locale, text)
21+
locale = locale.to_s.gsub("_", "-")
22+
translations.find_or_initialize_by(locale: locale).update!(translation: text)
23+
end
24+
25+
def translation_for(locale)
26+
translations.find_by(locale: locale)&.translation
27+
end
28+
29+
def detected_locale
30+
content_locale&.detected_locale
31+
end
32+
33+
private
34+
35+
def clear_translations
36+
return if !SiteSetting.translator_enabled
37+
38+
translations.delete_all
39+
content_locale&.destroy
40+
end
41+
end
42+
end
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# frozen_string_literal: true
2+
3+
module DiscourseTranslator
4+
class PostLocale < ActiveRecord::Base
5+
self.table_name = "discourse_translator_post_locales"
6+
7+
belongs_to :post
8+
9+
validates :post_id, presence: true, uniqueness: true
10+
validates :detected_locale, presence: true
11+
end
12+
end
13+
14+
# == Schema Information
15+
#
16+
# Table name: discourse_translator_post_locales
17+
#
18+
# id :bigint not null, primary key
19+
# post_id :integer not null
20+
# detected_locale :string(20) not null
21+
# created_at :datetime not null
22+
# updated_at :datetime not null
23+
#
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# frozen_string_literal: true
2+
3+
module DiscourseTranslator
4+
class PostTranslation < ActiveRecord::Base
5+
self.table_name = "discourse_translator_post_translations"
6+
7+
belongs_to :post
8+
9+
validates :post_id, presence: true
10+
validates :locale, presence: true
11+
validates :translation, presence: true
12+
validates :locale, uniqueness: { scope: :post_id }
13+
14+
def self.translation_for(post_id, locale)
15+
find_by(post_id: post_id, locale: locale)&.translation
16+
end
17+
end
18+
end
19+
20+
# == Schema Information
21+
#
22+
# Table name: discourse_translator_post_translations
23+
#
24+
# id :bigint not null, primary key
25+
# post_id :integer not null
26+
# locale :string not null
27+
# translation :text not null
28+
# created_at :datetime not null
29+
# updated_at :datetime not null
30+
#
31+
# Indexes
32+
#
33+
# idx_on_post_id_locale_0cc3d81e5b (post_id,locale) UNIQUE
34+
#
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# frozen_string_literal: true
2+
3+
module DiscourseTranslator
4+
class TopicLocale < ActiveRecord::Base
5+
self.table_name = "discourse_translator_topic_locales"
6+
7+
belongs_to :topic
8+
9+
validates :topic_id, presence: true, uniqueness: true
10+
validates :detected_locale, presence: true
11+
end
12+
end
13+
14+
# == Schema Information
15+
#
16+
# Table name: discourse_translator_topic_locales
17+
#
18+
# id :bigint not null, primary key
19+
# topic_id :integer not null
20+
# detected_locale :string(20) not null
21+
# created_at :datetime not null
22+
# updated_at :datetime not null
23+
#
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# frozen_string_literal: true
2+
3+
module DiscourseTranslator
4+
class TopicTranslation < ActiveRecord::Base
5+
self.table_name = "discourse_translator_topic_translations"
6+
7+
belongs_to :topic
8+
9+
validates :topic_id, presence: true
10+
validates :locale, presence: true
11+
validates :translation, presence: true
12+
validates :locale, uniqueness: { scope: :topic_id }
13+
14+
def self.translation_for(topic_id, locale)
15+
find_by(topic_id: topic_id, locale: locale)&.translation
16+
end
17+
end
18+
end
19+
20+
# == Schema Information
21+
#
22+
# Table name: discourse_translator_topic_translations
23+
#
24+
# id :bigint not null, primary key
25+
# topic_id :integer not null
26+
# locale :string not null
27+
# translation :text not null
28+
# created_at :datetime not null
29+
# updated_at :datetime not null
30+
#
31+
# Indexes
32+
#
33+
# idx_on_topic_id_locale_70b2f83213 (topic_id,locale) UNIQUE
34+
#

app/services/discourse_translator/base.rb

Lines changed: 34 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,14 @@ def self.cache_key
2727
# Returns the stored translation of a post or topic.
2828
# If the translation does not exist yet, it will be translated first via the API then stored.
2929
# If the detected language is the same as the target language, the original text will be returned.
30-
# @param topic_or_post [Post|Topic]
31-
def self.translate(topic_or_post)
32-
return if text_for_translation(topic_or_post).blank?
33-
detected_lang = detect(topic_or_post)
30+
# @param translatable [Post|Topic]
31+
def self.translate(translatable)
32+
return if text_for_translation(translatable).blank?
33+
detected_lang = detect(translatable)
3434

35-
return detected_lang, get_text(topic_or_post) if (detected_lang&.to_s == I18n.locale.to_s)
35+
return detected_lang, get_text(translatable) if (detected_lang&.to_s == I18n.locale.to_s)
3636

37-
existing_translation = get_translation(topic_or_post)
37+
existing_translation = get_translation(translatable)
3838
return detected_lang, existing_translation if existing_translation.present?
3939

4040
unless translate_supported?(detected_lang, I18n.locale)
@@ -46,27 +46,27 @@ def self.translate(topic_or_post)
4646
),
4747
)
4848
end
49-
[detected_lang, translate!(topic_or_post)]
49+
[detected_lang, translate!(translatable)]
5050
end
5151

5252
# Subclasses must implement this method to translate the text of a post or topic
5353
# then use the save_translation method to store the translated text.
54-
# @param topic_or_post [Post|Topic]
55-
def self.translate!(topic_or_post)
54+
# @param translatable [Post|Topic]
55+
def self.translate!(translatable)
5656
raise "Not Implemented"
5757
end
5858

5959
# Returns the stored detected locale of a post or topic.
6060
# If the locale does not exist yet, it will be detected first via the API then stored.
61-
# @param topic_or_post [Post|Topic]
62-
def self.detect(topic_or_post)
63-
return if text_for_detection(topic_or_post).blank?
64-
get_detected_locale(topic_or_post) || detect!(topic_or_post)
61+
# @param translatable [Post|Topic]
62+
def self.detect(translatable)
63+
return if text_for_detection(translatable).blank?
64+
get_detected_locale(translatable) || detect!(translatable)
6565
end
6666

67-
# Subclasses must implement this method to translate the text of a post or topic
68-
# then use the save_translation method to store the translated text.
69-
# @param topic_or_post [Post|Topic]
67+
# Subclasses must implement this method to detect the text of a post or topic
68+
# then use the save_detected_locale method to store the detected locale.
69+
# @param translatable [Post|Topic]
7070
def self.detect!(post)
7171
raise "Not Implemented"
7272
end
@@ -75,52 +75,33 @@ def self.access_token
7575
raise "Not Implemented"
7676
end
7777

78-
def self.get_translation(topic_or_post)
79-
translated_custom_field =
80-
topic_or_post.custom_fields[DiscourseTranslator::TRANSLATED_CUSTOM_FIELD] || {}
81-
translated_custom_field[I18n.locale]
78+
def self.get_translation(translatable)
79+
translatable.translation_for(I18n.locale)
8280
end
8381

84-
def self.save_translation(topic_or_post)
85-
translated_custom_field =
86-
topic_or_post.custom_fields[DiscourseTranslator::TRANSLATED_CUSTOM_FIELD] || {}
87-
translated_text = translated_custom_field[I18n.locale]
88-
89-
if translated_text.nil?
90-
translated_text = yield
91-
92-
topic_or_post.custom_fields[
93-
DiscourseTranslator::TRANSLATED_CUSTOM_FIELD
94-
] = translated_custom_field.merge(I18n.locale => translated_text)
95-
96-
topic_or_post.save!
97-
end
98-
99-
translated_text
82+
def self.save_translation(translatable)
83+
translation = yield
84+
translatable.set_translation(I18n.locale, translation)
85+
translation
10086
end
10187

102-
def self.get_detected_locale(topic_or_post)
103-
topic_or_post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD]
88+
def self.get_detected_locale(translatable)
89+
translatable.detected_locale
10490
end
10591

106-
def self.save_detected_locale(topic_or_post)
92+
def self.save_detected_locale(translatable)
10793
detected_locale = yield
108-
topic_or_post.custom_fields[DiscourseTranslator::DETECTED_LANG_CUSTOM_FIELD] = detected_locale
109-
110-
if !topic_or_post.custom_fields_clean?
111-
topic_or_post.save_custom_fields
112-
topic_or_post.publish_change_to_clients!(:revised) if topic_or_post.class.name == "Post"
113-
end
94+
translatable.set_detected_locale(detected_locale)
11495

11596
detected_locale
11697
end
11798

118-
def self.get_text(topic_or_post)
119-
case topic_or_post.class.name
99+
def self.get_text(translatable)
100+
case translatable.class.name
120101
when "Post"
121-
topic_or_post.cooked
102+
translatable.cooked
122103
when "Topic"
123-
topic_or_post.title
104+
translatable.title
124105
end
125106
end
126107

@@ -143,15 +124,12 @@ def self.strip_tags_for_detection(detection_text)
143124
html_doc.to_html
144125
end
145126

146-
def self.text_for_detection(topic_or_post)
147-
strip_tags_for_detection(get_text(topic_or_post)).truncate(
148-
DETECTION_CHAR_LIMIT,
149-
omission: nil,
150-
)
127+
def self.text_for_detection(translatable)
128+
strip_tags_for_detection(get_text(translatable)).truncate(DETECTION_CHAR_LIMIT, omission: nil)
151129
end
152130

153-
def self.text_for_translation(topic_or_post)
154-
get_text(topic_or_post).truncate(SiteSetting.max_characters_per_translation, omission: nil)
131+
def self.text_for_translation(translatable)
132+
get_text(translatable).truncate(SiteSetting.max_characters_per_translation, omission: nil)
155133
end
156134
end
157135
end
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# frozen_string_literal: true
2+
3+
class CreateTranslationTables < ActiveRecord::Migration[7.2]
4+
def change
5+
create_table :discourse_translator_topic_locales do |t|
6+
t.integer :topic_id, null: false
7+
t.string :detected_locale, limit: 20, null: false
8+
t.timestamps
9+
end
10+
11+
create_table :discourse_translator_topic_translations do |t|
12+
t.integer :topic_id, null: false
13+
t.string :locale, null: false
14+
t.text :translation, null: false
15+
t.timestamps
16+
end
17+
18+
create_table :discourse_translator_post_locales do |t|
19+
t.integer :post_id, null: false
20+
t.string :detected_locale, limit: 20, null: false
21+
t.timestamps
22+
end
23+
24+
create_table :discourse_translator_post_translations do |t|
25+
t.integer :post_id, null: false
26+
t.string :locale, null: false
27+
t.text :translation, null: false
28+
t.timestamps
29+
end
30+
end
31+
end

0 commit comments

Comments
 (0)