Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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?
posts_query =
posts_query.includes(:translations).where(
translations: {
locale: InlineTranslation.effective_locale.to_s.gsub("_", "-"),
},
)
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