Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion app/models/concerns/discourse_translator/translatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ def set_translation(locale, text)

def translation_for(locale)
locale = locale.to_s.gsub("_", "-")
translations.find_by(locale: locale)&.translation
# this is a tricky perf balancing act when loading translations for a topic with many posts.
# the topic_view_serializer includes(:translations) for posts in the topic,
# the alternative is to do a find_by(locale: locale) which would result in a query per post.
translations.to_a.find { |t| t.locale == locale }&.translation
end

def detected_locale
Expand Down
4 changes: 0 additions & 4 deletions app/models/discourse_translator/post_translation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ class PostTranslation < ActiveRecord::Base
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

Expand Down
4 changes: 0 additions & 4 deletions app/models/discourse_translator/topic_translation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ class TopicTranslation < ActiveRecord::Base
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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ import DButton from "discourse/components/d-button";
import concatClass from "discourse/helpers/concat-class";

export default class ShowOriginalContent extends Component {
static shouldRender(args) {
return (
args.topic.is_translated ||
args.topic.postStream.posts.some(({ is_translated }) => is_translated)
);
}

@service router;
@tracked isTranslated = true;

Expand Down
6 changes: 4 additions & 2 deletions lib/discourse_translator/guardian_extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,17 @@ def can_detect_language?(post)
def can_translate?(post)
return false if post.user&.bot?
return false if !user_group_allow_translate?
return false if post.locale_matches?(I18n.locale)

# we want to return false if the post is created within a short buffer ago,
# this prevents the 🌐from appearing and then disappearing if the lang is same as user's lang
return false if post.created_at > POST_DETECTION_BUFFER.ago && post.detected_locale.blank?

if SiteSetting.experimental_inline_translation
post.translation_for(I18n.locale).nil?
locale = DiscourseTranslator::InlineTranslation.effective_locale
return false if post.locale_matches?(locale)
post.translation_for(locale).nil?
Comment on lines +33 to +35
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When in experimental_inline_translation, we only want to allow a user to translate a post if the user's locale is within the target languages that we support for the site (effective_locale).

else
return false if post.locale_matches?(I18n.locale)
poster_group_allow_translate?(post)
end
end
Expand Down
46 changes: 37 additions & 9 deletions lib/discourse_translator/inline_translation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,41 +2,69 @@

module DiscourseTranslator
class InlineTranslation
def self.effective_locale
if LocaleMatcher.user_locale_is_default? || LocaleMatcher.user_locale_in_target_languages?
I18n.locale
else
SiteSetting.default_locale
end
end

def inject(plugin)
# since locales are eager loaded but translations may not,
# always return early if topic and posts are in the user's effective_locale.
# this prevents the need to load translations.
Comment on lines +14 to +16
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All plugin api usages will check for locale first.


plugin.register_modifier(:basic_post_serializer_cooked) do |cooked, serializer|
if !SiteSetting.experimental_inline_translation ||
serializer.scope&.request&.params&.[]("show") == "original" ||
serializer.object.detected_locale == I18n.locale.to_s.gsub("_", "-")
serializer.object.locale_matches?(InlineTranslation.effective_locale) ||
serializer.scope&.request&.params&.[]("show") == "original"
cooked
else
serializer.object.translation_for(I18n.locale).presence
serializer.object.translation_for(InlineTranslation.effective_locale).presence
end
end

plugin.register_modifier(:topic_serializer_fancy_title) do |fancy_title, serializer|
if !SiteSetting.experimental_inline_translation ||
serializer.scope&.request&.params&.[]("show") == "original" ||
serializer.object.locale_matches?(I18n.locale)
serializer.object.locale_matches?(InlineTranslation.effective_locale) ||
serializer.scope&.request&.params&.[]("show") == "original"
fancy_title
else
serializer.object.translation_for(I18n.locale).presence&.then { |t| Topic.fancy_title(t) }
serializer
.object
.translation_for(InlineTranslation.effective_locale)
.presence
&.then { |t| Topic.fancy_title(t) }
end
end

plugin.register_modifier(:topic_view_serializer_fancy_title) do |fancy_title, serializer|
if !SiteSetting.experimental_inline_translation ||
serializer.scope&.request&.params&.[]("show") == "original" ||
serializer.object.topic.locale_matches?(I18n.locale)
serializer.object.topic.locale_matches?(InlineTranslation.effective_locale) ||
serializer.scope&.request&.params&.[]("show") == "original"
fancy_title
else
serializer
.object
.topic
.translation_for(I18n.locale)
.translation_for(InlineTranslation.effective_locale)
.presence
&.then { |t| Topic.fancy_title(t) }
end
end

plugin.add_to_serializer(:basic_post, :is_translated) do
SiteSetting.experimental_inline_translation &&
!object.locale_matches?(InlineTranslation.effective_locale) &&
object.translation_for(InlineTranslation.effective_locale).present?
end

plugin.add_to_serializer(:topic_view, :is_translated) do
SiteSetting.experimental_inline_translation &&
!object.topic.locale_matches?(InlineTranslation.effective_locale) &&
object.topic.translation_for(InlineTranslation.effective_locale).present?
end
Comment on lines +57 to +67
Copy link
Contributor Author

@nattsw nattsw Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two flags are used in the frontend to check if a topic is showing translated content

end
end
end
15 changes: 15 additions & 0 deletions lib/discourse_translator/locale_matcher.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# frozen_string_literal: true

module DiscourseTranslator
class LocaleMatcher
def self.user_locale_in_target_languages?
# e.g. "en|es|pt_BR" vs :en_UK
SiteSetting.automatic_translation_target_languages.split("|").include?(I18n.locale.to_s)
end

def self.user_locale_is_default?
# e.g. :en_UK vs "en_UK"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

I18n.locale.to_s == SiteSetting.default_locale
end
end
end
15 changes: 14 additions & 1 deletion lib/discourse_translator/topic_view_serializer_extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,20 @@ module DiscourseTranslator
module TopicViewSerializerExtension
def posts
if SiteSetting.translator_enabled?
object.instance_variable_set(:@posts, object.posts.includes(:content_locale))
posts_query = object.posts.includes(:content_locale)
# this is kind of a micro-optimization.
# we do not want to eager load translations if the user is using the site's language.
# we will only load them if the user is using a different language that is supported by the site.
if SiteSetting.experimental_inline_translation && !LocaleMatcher.user_locale_is_default? &&
LocaleMatcher.user_locale_in_target_languages?
locale = InlineTranslation.effective_locale.to_s.gsub("_", "-")
posts_query =
posts_query
.includes(:translations)
.references(:translations)
.where(translations: { locale: [nil, locale] })
end
object.instance_variable_set(:@posts, posts_query)
end
super
end
Expand Down
9 changes: 7 additions & 2 deletions spec/lib/guardian_extension_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,12 @@
end

describe "when experimental_inline_translation enabled" do
before { SiteSetting.experimental_inline_translation = true }
before do
SiteSetting.experimental_inline_translation = true

SiteSetting.automatic_translation_backfill_maximum_translations_per_hour = 1
SiteSetting.automatic_translation_target_languages = "pt"
end

describe "logged in user" do
it "cannot translate when user is not in restrict_translation_by_group" do
Expand All @@ -190,7 +195,7 @@
before { SiteSetting.restrict_translation_by_group = "#{group.id}" }

it "cannot translate when post has translation for user locale" do
post.set_detected_locale("jp")
post.set_detected_locale("ja")
post.set_translation("pt", "Olá, mundo!")

expect(guardian.can_translate?(post)).to eq(false)
Expand Down
11 changes: 11 additions & 0 deletions spec/serializers/basic_topic_serializer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
topic.title = original_title
SiteSetting.experimental_inline_translation = true
I18n.locale = "ja"

SiteSetting.automatic_translation_backfill_maximum_translations_per_hour = 1
SiteSetting.automatic_translation_target_languages = "ja"
end

def serialize_topic(guardian_user: user, params: {})
Expand Down Expand Up @@ -53,6 +56,14 @@ def serialize_topic(guardian_user: user, params: {})
expect(serialize_topic.fancy_title).to eq(topic.fancy_title)
end

it "does not replace fancy_title when user's locale is not in target languages" do
I18n.locale = "es"
topic.set_detected_locale("en")
topic.set_translation("es", jap_title)

expect(serialize_topic.fancy_title).to eq(topic.fancy_title)
end

it "returns translated title in fancy_title when translation exists for current locale" do
topic.set_translation("ja", jap_title)
expect(serialize_topic.fancy_title).to eq("&lt;h1&gt;フス・ロ・ダ・ア&lt;/h1&gt;")
Expand Down
69 changes: 67 additions & 2 deletions spec/serializers/post_serializer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,63 @@
end
end

describe "#is_translated" do
fab!(:post)

it "returns false when translator disabled" do
SiteSetting.translator_enabled = false
serializer = PostSerializer.new(post, scope: Guardian.new)

expect(serializer.is_translated).to eq(false)
end

it "returns false when experimental inline translation disabled" do
SiteSetting.translator_enabled = true
SiteSetting.experimental_inline_translation = false
serializer = PostSerializer.new(post, scope: Guardian.new)

expect(serializer.is_translated).to eq(false)
end

it "returns true when there is a translation for the user's locale in target languages" do
SiteSetting.translator_enabled = true
SiteSetting.experimental_inline_translation = true
SiteSetting.automatic_translation_backfill_maximum_translations_per_hour = 1
SiteSetting.automatic_translation_target_languages = "ja"
I18n.locale = "ja"
post.set_detected_locale("en")
post.set_translation("ja", "こんにちは")
serializer = PostSerializer.new(post, scope: Guardian.new)

expect(serializer.is_translated).to eq(true)
end

it "returns false when there is a translation for the user's locale not in target languages" do
SiteSetting.translator_enabled = true
SiteSetting.experimental_inline_translation = true
SiteSetting.automatic_translation_backfill_maximum_translations_per_hour = 1
SiteSetting.automatic_translation_target_languages = "es"
I18n.locale = "ja"
post.set_detected_locale("en")
post.set_translation("ja", "こんにちは")
serializer = PostSerializer.new(post, scope: Guardian.new)

expect(serializer.is_translated).to eq(false)
end

it "returns false when there is no translation for the current locale in target languages" do
SiteSetting.translator_enabled = true
SiteSetting.experimental_inline_translation = true
SiteSetting.automatic_translation_backfill_maximum_translations_per_hour = 1
SiteSetting.automatic_translation_target_languages = "ja"
I18n.locale = "ja"
post.set_translation("es", "Hola")
serializer = PostSerializer.new(post, scope: Guardian.new)

expect(serializer.is_translated).to eq(false)
end
end

describe "#cooked" do
def serialize_post(guardian_user: user, params: {})
env = { "action_dispatch.request.parameters" => params, "REQUEST_METHOD" => "GET" }
Expand All @@ -76,6 +133,8 @@ def serialize_post(guardian_user: user, params: {})

it "does not return translated_cooked when show=original param is present" do
I18n.locale = "ja"
SiteSetting.automatic_translation_backfill_maximum_translations_per_hour = 1
SiteSetting.automatic_translation_target_languages = "ja"
post.set_translation("ja", "こんにちは")

expect(serialize_post(params: { "show" => "original" }).cooked).to eq(post.cooked)
Expand All @@ -90,11 +149,17 @@ def serialize_post(guardian_user: user, params: {})
expect(serialize_post.cooked).to eq(post.cooked)
end

it "returns translated content based on locale" do
I18n.locale = "ja"
it "returns translated content based on locale presence in target languages" do
SiteSetting.automatic_translation_backfill_maximum_translations_per_hour = 1
post.set_translation("ja", "こんにちは")
post.set_translation("es", "Hola")
I18n.locale = "ja"

SiteSetting.automatic_translation_target_languages = "ja"
expect(serialize_post.cooked).to eq("こんにちは")

SiteSetting.automatic_translation_target_languages = "es"
expect(serialize_post.cooked).to eq(post.cooked)
end

it "does not return translated_cooked when plugin is disabled" do
Expand Down
Loading
Loading