Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update weblab model to do I18n.t translations #51846

Merged
merged 5 commits into from
May 16, 2023
Merged

Conversation

elf-code
Copy link
Contributor

@elf-code elf-code commented May 11, 2023

omg y'all I've been spelunking.

It all started with P20-164 a totally innocent report that 14 lessons don't have translations showing.

And it was a great way to learn about the i18n pipeline and how the whole system works! So I took a look

Full saga below the fold

  • I checked in CrowdIn to see if we were asking for these lessons to be translated, we were. The hierarchical model of the strings in crowdin made it easy to confirm that these lessons have translations in es-ES.
  • I checked on the i18n-dev server to confirm that the translations were being copied down from crowdin and merged into the expected spots according to the diagram of truth.
  • Ok so the translations exist in the spot we would expect.. is there a pattern for what courses aren't being translated? Yes, yes there is! They all seem to be WebLab labs, not Blocks or App Lab or AI Lab.
  • So from the frontend, do WebLab labs not know how to set locale? Maybe. Is that affecting the instructions? No. WebLab.js doesn't use the setLocale Redux store that I just updated but it also doesn't read from that store, so it doesn't matter. P5Lab does setLocale here where it's reading from appOptions
  • Alright.. so where does appOptions come from server-side? For levels that are translated, like this one the longInstructions in that appOptions are already in Spanish so the server is sending in localized input.
  • In levels_helper.rb you can see a method called app_options that puts together all of this configuration data. Within the app_options, we only really care about the level_options - data specific to the level, not the user or other configuration. This is set down here since this is a non_blockly_puzzle_level.
  • Blockly puzzles go through a lot of massaging to get their i18n set up. But this is a weblab level. So under the hood in the weblab model, we implement that method.
  • But how do other levels implement that method?? Well... blockly levels go through a separate translation process with the options being set in this method. And _free_responses does a cool translation process directly

I copied over the logic from the free_response.rb model over into the weblab.rb model.

Testing story

In production: you can tell from the title that the locale is Spanish but that the instructions are still in English
Screenshot 2023-05-11 at 9 11 06 AM

In development: the instructions are now in Spanish too!
Screenshot 2023-05-11 at 9 10 27 AM

In development: in English, the instructions are still in English
Screenshot 2023-05-12 at 8 07 47 AM

Follow-up work

This only fixes weblab levels, in looking through s/allthethings/ in Spanish there are a couple other level types that don't have translations visible but I'm not sure how important updating them are (blocks are the bread and butter).

  • Clock
  • Odometer
  • Netsim
  • External Markdown
  • Blocks to Math (it's hard to tell cause I'm testing in Spanish and I don't actually speak Spanish).

It also seems like FND=985 was created to do this as a platform wide solution but 馃し it seems to have been de-prioritized.

#32650

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@elf-code elf-code requested review from a team May 12, 2023 15:14
@elf-code elf-code marked this pull request as ready for review May 12, 2023 15:15
@elf-code
Copy link
Contributor Author

Looked into which properties were being translated gist link with the changes in this commit and it seems like Teacher Markdown gets set (and then reset) so just directly updated the long instructions rather than all of the properties.

Also added in the TODO for the shared library FND-985.

Copy link
Contributor

@wilkie wilkie left a comment

Choose a reason for hiding this comment

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

That's a great saga! I remember this rabbit hole well from doing a couple of these "instructions" translations work myself. Looks good! 馃憤

@elf-code elf-code merged commit 1f984ca into staging May 16, 2023
2 checks passed
@elf-code elf-code deleted the elf-i18n-weblab branch May 16, 2023 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants