FEATURE: Translate full topics #199

nattsw · 2025-01-23T11:35:56Z

Needs #198 (first few commits are off that branch)

This feature is behind experimental_topic_translation and translates topics based on the language switcher selection.

topt.mp4

As we start to translate more pages, we'll need a way for other sites to link back to our translated topics. This commit gives us the ability to use the "lang" URL param to define what language a site should be in. Related: discourse/discourse-translator#199

tgxworld · 2025-01-24T04:32:48Z

plugin.rb

+  add_to_serializer :post, :cooked, respect_plugin_enabled: false do
+    return super() if cooked_hidden
+    DiscourseTranslator::TranslatorHelper.translated_value(super(), object, scope)
+  end


Overriding a core's serializer attribute is going to be fragile to maintain long term. Perhaps we need an API extension in core?

@tgxworld We discussed this internally, can take a look at t/146193/5

I've read through the post but it doesn't address my concern about us overriding a core's serializer attribute from a plugin which is a form of monkey patching. Just to clarify, my concern here is not about us changing the value of the cooked attribute but the approach in which we are doing so.

I can revert to 2️⃣ in t/146193/1 if that helps. But the new 'problem' is that we'll be transmitting a lot more over the wire (translated_cooked+cooked) for something a user might not use (cooked)

I don't have strong opinions on whether 1 or 2 is the better approach. What I am saying is that if we are going with 1, we probably want to introduce a plugin modifier/API in Discourse core that allows us to modify the value of the cooked attribute in the serializer safely.

I think it makes sense to have built-in protection for the existing add_to_serializer method so I'll see if I can keep it there.

Since this is experimental, I think we can add a note above the two add_to_serializer calls here, noting that we may/should refactor them before this can be production-ready.

tgxworld · 2025-01-24T04:32:57Z

plugin.rb

+  add_to_serializer :basic_topic, :fancy_title do
+    DiscourseTranslator::TranslatorHelper.translated_value(object.fancy_title, object, scope)
+  end


Same concern as https://github.com/discourse/discourse-translator/pull/199/files#r1928075461

This one doesn't have an easy decorator unlike post.cooked.. 🤔

lis2 · 2025-01-24T04:33:12Z

assets/javascripts/discourse/components/show-original-content.gjs

+  @action
+  async showOriginal() {
+    this.active = !this.active;
+    window.location.search = this.active ? "" : `show=original`;


Is it possible that there will be params already? Maybe it would be safer to append params?

https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/append

lis2 · 2025-01-24T04:36:36Z

assets/javascripts/discourse/initializers/extend-for-translate-button.js

-  if (currentUser) {
+  if (siteSettings.experimental_topic_translation) {
+    api.renderInOutlet("topic-navigation", ShowOriginalContent);
+  } else {


I think that we should still check if(currentUser || SiteSetting.experimental_anon_language_switcher)

What do you mean here? This feature will be available for anons also.

Do you mean we should disable it "if anon + no language switcher"? Ok, that makes sense.

tgxworld · 2025-01-24T04:42:35Z

lib/discourse_translator/translator_helper.rb

+      translated = model.custom_fields[TRANSLATED_CUSTOM_FIELD]
+      return original_value if (translated.blank? || translated[I18n.locale].blank?)
+
+      translated[I18n.locale]


I'm not sure if we are ready to solve this yet but if the translation is missing, do we display any indication in the interface for the user? It would be odd to the user if they request for a translation and we return the original one.

This will come in a subsequent commit. The idea is that in the future, all topics will be translated by default.

There needs to be a new outlet in core for the "indicator" on both the topic posts itself and the topic list view titles.

As we start to translate more pages, we'll need a way for other sites to link back to our translated topics. This commit gives us the ability to use the "lang" URL param to define what language a site should be in. Related: discourse/discourse-translator#199

pmusaraj · 2025-01-24T16:08:55Z

plugin.rb

+  add_to_serializer :post, :cooked, respect_plugin_enabled: false do
+    return super() if cooked_hidden
+    DiscourseTranslator::TranslatorHelper.translated_value(super(), object, scope)
+  end


Since this is experimental, I think we can add a note above the two add_to_serializer calls here, noting that we may/should refactor them before this can be production-ready.

pmusaraj · 2025-01-24T16:10:16Z

assets/stylesheets/common/common.scss

+}
+
+.discourse-translator_toggle-original {
+  button.active svg {


Suggested change

button.active svg {

button.active .d-icon {

We usually target the icon class. Shouldn't active buttons be using the correct color in core?

pmusaraj · 2025-01-24T17:08:55Z

@nattsw I pulled this branch locally (too excited to try it out!), but noticed that on /latest, with Experimental topic translation enabled, I get a Rails error:

Attempted to access the non preloaded custom field 'translated_text' on the 'Topic' class. This is disallowed to prevent N+1 queries.

nattsw · 2025-01-27T10:05:32Z

@pmusaraj oh, super weird that I never saw it! I'll add add_preloaded_topic_list_custom_field

nattsw mentioned this pull request Jan 23, 2025

FEATURE: Allow setting locale from 'lang' param discourse/discourse#30952

Merged

nattsw force-pushed the translate-full-topics branch from 98662e8 to 4ea54e1 Compare January 24, 2025 03:51

nattsw force-pushed the translate-full-topics branch from 4ea54e1 to 48cf984 Compare January 24, 2025 03:59

nattsw changed the base branch from main to always-use-body-to-detect-lang January 24, 2025 04:02

nattsw changed the base branch from always-use-body-to-detect-lang to main January 24, 2025 04:02

nattsw added 4 commits January 24, 2025 12:09

Remove change from other branch

5f78743

Remove test from other branch

36d3867

FEATURE: Show full topic translations

3301215

arrange imports

0302b81

nattsw force-pushed the translate-full-topics branch from 1bfe5ab to 0302b81 Compare January 24, 2025 04:09

nattsw marked this pull request as ready for review January 24, 2025 04:17

Fix a rebase

23a6131

tgxworld reviewed Jan 24, 2025

View reviewed changes

lis2 reviewed Jan 24, 2025

View reviewed changes

tgxworld reviewed Jan 24, 2025

View reviewed changes

pmusaraj reviewed Jan 24, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FEATURE: Translate full topics #199

FEATURE: Translate full topics #199

nattsw commented Jan 23, 2025 •

edited

Loading

tgxworld Jan 24, 2025

nattsw Jan 24, 2025 •

edited

Loading

tgxworld Jan 24, 2025

nattsw Jan 24, 2025 •

edited

Loading

tgxworld Jan 24, 2025

nattsw Jan 24, 2025

pmusaraj Jan 24, 2025

tgxworld Jan 24, 2025

nattsw Jan 24, 2025

lis2 Jan 24, 2025

lis2 Jan 24, 2025

nattsw Jan 24, 2025 •

edited

Loading

tgxworld Jan 24, 2025

nattsw Jan 24, 2025 •

edited

Loading

pmusaraj Jan 24, 2025

pmusaraj Jan 24, 2025

pmusaraj commented Jan 24, 2025

nattsw commented Jan 27, 2025

FEATURE: Translate full topics #199

Are you sure you want to change the base?

FEATURE: Translate full topics #199

Conversation

nattsw commented Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

nattsw Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nattsw Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nattsw Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nattsw Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmusaraj commented Jan 24, 2025

nattsw commented Jan 27, 2025

nattsw commented Jan 23, 2025 •

edited

Loading

nattsw Jan 24, 2025 •

edited

Loading

nattsw Jan 24, 2025 •

edited

Loading

nattsw Jan 24, 2025 •

edited

Loading

nattsw Jan 24, 2025 •

edited

Loading