fix(i18n): extract translations on `plugins` folder #1589

igobranco · 2025-01-14T13:02:15Z

Description

This pull request fixes the extract translations process for the plugins folder.
This increases the messages that are going to be extracted, making it possible to translate more of this application.

This PR is useful for "Operators" that use Studio and Course authoring MFE on a different language than English.

Testing instructions

Run the extract_translations on master and on this PR:

make extract_translations
wc -l temp/babel-plugin-formatjs/Default.messages.json

openedx-webhooks · 2025-01-14T13:02:20Z

Thanks for the pull request, @igobranco!

This repository is currently maintained by @openedx/2u-tnl.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

e0d · 2025-01-23T20:52:21Z

@igobranco I added you to the triage team so tests will run automatically for you going forward. This PR seems to have stuck jobs, can you push an empty commit or similar to see if it kicks off the checks again?

codecov · 2025-01-23T21:01:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.50%. Comparing base (8385c4e) to head (35d164c).
Report is 79 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1589      +/-   ##
==========================================
+ Coverage   93.07%   93.50%   +0.43%     
==========================================
  Files        1092     1128      +36     
  Lines       21617    22925    +1308     
  Branches     4577     4858     +281     
==========================================
+ Hits        20120    21436    +1316     
+ Misses       1431     1421      -10     
- Partials       66       68       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

igobranco · 2025-01-24T11:51:47Z

@igobranco I added you to the triage team so tests will run automatically for you going forward. This PR seems to have stuck jobs, can you push an empty commit or similar to see if it kicks off the checks again?

Thanks @e0d

mphilbrick211 · 2025-01-29T21:34:30Z

@openedx/2u-tnl this is ready for review. Thanks!

OmarIthawi · 2025-04-06T06:51:18Z

package.json

@@ -11,7 +11,7 @@
  ],
  "scripts": {
    "build": "fedx-scripts webpack",
-    "i18n_extract": "fedx-scripts formatjs extract",
+    "i18n_extract": "formatjs extract --format node_modules/@openedx/frontend-build/lib/formatter.js --ignore {src,plugins}/**/*.json --out-file ./temp/babel-plugin-formatjs/Default.messages.json -- {src,plugins}/**/*.js*",


Thanks @igobranco for the pull request and really sorry for the delay. Somehow I didn't include this pull request on my todo list.

I'll test locally, but one question. I noticed that you're setting the file to become ./temp/babel-plugin-formatjs/Default.messages.json, what's reason for this specific file choice and why did you mention it explicitly?

Are you aware of the specifics of fedx-scripts formatjs extract? What features do we expect to lose when removing the fedx-scripts meta-script?

OmarIthawi

Thanks @igobranco for your contribution, please check the comments and let me know what do you think.

OmarIthawi · 2025-04-08T11:06:17Z

package.json

@@ -11,7 +11,7 @@
  ],
  "scripts": {
    "build": "fedx-scripts webpack",
-    "i18n_extract": "fedx-scripts formatjs extract",
+    "i18n_extract": "formatjs extract --format node_modules/@openedx/frontend-build/lib/formatter.js --ignore {src,plugins}/**/*.json --out-file ./temp/babel-plugin-formatjs/Default.messages.json -- {src,plugins}/**/*.js*",


I've checked the fedx-scripts and I think there's a better solution because the fedx-scripts accepts additional arguments and I'd like to ask you to try it first:

Suggested change

"i18n_extract": "formatjs extract --format node_modules/@openedx/frontend-build/lib/formatter.js --ignore {src,plugins}/**/*.json --out-file ./temp/babel-plugin-formatjs/Default.messages.json -- {src,plugins}/**/*.js*",

"i18n_extract": ""i18n_extract": "fedx-scripts formatjs extract plugins/**/*.js*",

We might need to add quotes, I'm unsure whether bash is running here or not:

Suggested change

"i18n_extract": "formatjs extract --format node_modules/@openedx/frontend-build/lib/formatter.js --ignore {src,plugins}/**/*.json --out-file ./temp/babel-plugin-formatjs/Default.messages.json -- {src,plugins}/**/*.js*",

"i18n_extract": ""i18n_extract": "fedx-scripts formatjs extract 'plugins/**/*.js*'",

Please check:

https://github.com/openedx/frontend-build/blob/c70ef4e0a5e8702f7144f36b1b23195b94f5e09a/bin/fedx-scripts.js#L71-L81

@OmarIthawi
Yes, I checked when I open this PR.

That is why I included ./temp/babel-plugin-formatjs/Default.messages.json because behind the scenes the fedx-scripts is using that.
In general the problem is that the fedx-scripts formatjs don't allow to add additional source folders. But probably this isn't the idea of fedx-scripts of having too much custom options, just a basic framework for different Open edX frontend packages.

So I think this resume the execution, where each step executes the next one:

npm run i18n_extract

fedx-scripts formatjs extract that is installed on node_modules/@openedx/frontend-build/bin/fedx-scripts.js that adds those commonArgs to the next process

node_modules/@formatjs/cli/bin/formatjs extract --format node_modules/@openedx/frontend-build/lib/formatter.js --ignore src/**/*.json --out-file ./temp/babel-plugin-formatjs/Default.messages.json -- src/**/*.js* but this command don't include the plugins folder where this repository has additional code.

So my proposal is to replace the fedx-scripts formatjs extract executing a lower level command directly that also includes the plugins folder.

If you execute this PR before and after you will an increase on the strings to be translated, 145 strings on the branch that I opened the PR.

$ npm run i18n_extract > @edx/[email protected] i18n_extract > formatjs extract --format node_modules/@openedx/frontend-build/lib/formatter.js --ignore {src,plugins}/**/*.json --out-file ./temp/babel-plugin-formatjs/Default.messages.json -- {src,plugins}/**/*.js* [@formatjs/cli] [WARN] [FormatJS CLI] Duplicate message id: "authoring.proctoring.alert.error", but the `description` and/or `defaultMessage` are different. $ grep '"id"' temp/babel-plugin-formatjs/Default.messages.json | wc -l 1860

I can't make it run with your proposal:

package.json changed to:

"i18n_extract": "fedx-scripts formatjs extract plugins/**/*.js*",

Execute and output:

$ npm run i18n_extract > @edx/[email protected] i18n_extract > fedx-scripts formatjs extract plugins/**/*.js* Running with resolved config: /home/ibranco/projects/openedx/frontend-app-authoring/node_modules/@openedx/frontend-build/config/babel.config.js [@formatjs/cli] [WARN] Error: Error processing file plugins/course-apps/calculator/package.json Debug Failure. Output generation failed

Yes, it passes the arguments, but because the fedx-scripts will add more arguments, then the real formatjs executed will be kind of different!

fixes openedx#1588

igobranco · 2025-04-09T11:36:59Z

@OmarIthawi I reviewed the command.

Changed to:

"i18n_extract": "formatjs extract --format node_modules/@openedx/frontend-build/lib/formatter.js --ignore {src,plugins}/**/*.json --out-file ./temp/babel-plugin-formatjs/Default.messages.json -- {src,plugins}/**/*.js* {src,plugins}/**/messages.ts",

To include the messages.ts files.

I tested and it has increased the extraction to 2376 strings.

$ npm run i18n_extract &&  grep '"id"' temp/babel-plugin-formatjs/Default.messages.json | wc -l

> @edx/[email protected] i18n_extract
> formatjs extract --format node_modules/@openedx/frontend-build/lib/formatter.js --ignore {src,plugins}/**/*.json --out-file ./temp/babel-plugin-formatjs/Default.messages.json -- {src,plugins}/**/*.js* {src,plugins}/**/messages.ts

[@formatjs/cli] [WARN] [FormatJS CLI] Duplicate message id: "authoring.proctoring.alert.error", but the `description` and/or `defaultMessage` are different.[@formatjs/cli] [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.library-authoring.component.advanced.olx-save", but the `description` and/or `defaultMessage` are different.[@formatjs/cli] [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.library-authoring.create-library", but the `description` and/or `defaultMessage` are different.[@formatjs/cli] [WARN] [FormatJS CLI] Duplicate message id: "course-authoring.library-authoring.publish.error", but the `description` and/or `defaultMessage` are different.



2376

I tested and now I could translate the "Select problem type", the others should also work...

Nevertheless, the problem types aren't going to be translated, someone has been creative:

frontend-app-authoring/src/editors/data/constants/problem.ts

Line 22 in 35d164c

title: 'Single select',

OmarIthawi · 2025-04-10T06:39:18Z

Thanks @igobranco for the debugging in and the additional information.

So my proposal is to replace the fedx-scripts formatjs extract executing a lower level command directly that also includes the plugins folder.

I think that makes sense, a more appropriate and acceptable change is to add an argument to the extract command: fedx-scripts formatjs extract --include=plugins --include=plugins2

Which would be more sensible and avoid hidden assumptions.

The issue with removing fedx-scripts use is that future upgrades will break and will cause a hard time for engineers to debug before discovering that frontend-app-authoring is a special case which I don't recommend.

igobranco · 2025-04-10T08:16:29Z

@OmarIthawi to resolve part of the "problem", related to my last comment, I just opened a new PR #1801 that renames the message.ts files to message.js.
And I agree with you a better solution is to improve the https://github.com/openedx/frontend-build/blob/c70ef4e0a5e8702f7144f36b1b23195b94f5e09a/bin/fedx-scripts.js#L71-L81 to support additional source folders.

igobranco · 2025-04-10T09:08:44Z

@OmarIthawi I opened a new PR openedx/frontend-build#650 for frontend-build with your suggestion to add that --include= additional argument.

igobranco requested a review from a team as a code owner January 14, 2025 13:02

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jan 14, 2025

mphilbrick211 added the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Jan 15, 2025

e0d changed the title ~~fix(i18n): extract translations on plugins folder~~ fix(i18n): extract translations on plugins folder Jan 23, 2025

mphilbrick211 removed the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Jan 29, 2025

brian-smith-tcril requested a review from OmarIthawi February 19, 2025 13:56

OmarIthawi reviewed Apr 6, 2025

View reviewed changes

OmarIthawi self-requested a review April 6, 2025 06:51

OmarIthawi suggested changes Apr 8, 2025

View reviewed changes

fix(i18n): extract translations on plugins folder

35d164c

fixes openedx#1588

igobranco force-pushed the igobranco/issues/1588/translations-extract-plugins-folder branch from a434a17 to 35d164c Compare April 9, 2025 11:35

igobranco mentioned this pull request Apr 9, 2025

fix: translation on select problem type #1798

Open

igobranco closed this Apr 10, 2025

github-project-automation bot moved this from Ready for Review to Done in Contributions Apr 10, 2025

igobranco mentioned this pull request Apr 10, 2025

feat(formatjs): support additional src folders openedx/frontend-build#650

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(i18n): extract translations on `plugins` folder #1589

fix(i18n): extract translations on `plugins` folder #1589

Uh oh!

igobranco commented Jan 14, 2025

Uh oh!

openedx-webhooks commented Jan 14, 2025 •

edited

Loading

Uh oh!

e0d commented Jan 23, 2025

Uh oh!

codecov bot commented Jan 23, 2025 •

edited

Loading

Uh oh!

igobranco commented Jan 24, 2025

Uh oh!

mphilbrick211 commented Jan 29, 2025

Uh oh!

OmarIthawi Apr 6, 2025

Uh oh!

OmarIthawi left a comment

Uh oh!

OmarIthawi Apr 8, 2025

Uh oh!

igobranco Apr 9, 2025 •

edited

Loading

Uh oh!

igobranco commented Apr 9, 2025 •

edited

Loading

Uh oh!

OmarIthawi commented Apr 10, 2025

Uh oh!

igobranco commented Apr 10, 2025

Uh oh!

igobranco commented Apr 10, 2025

Uh oh!

Uh oh!

	"i18n_extract": "formatjs extract --format node_modules/@openedx/frontend-build/lib/formatter.js --ignore {src,plugins}/*/.json --out-file ./temp/babel-plugin-formatjs/Default.messages.json -- {src,plugins}/*/.js*",
	"i18n_extract": ""i18n_extract": "fedx-scripts formatjs extract plugins/*/.js*",

fix(i18n): extract translations on plugins folder #1589

fix(i18n): extract translations on plugins folder #1589

Uh oh!

Conversation

igobranco commented Jan 14, 2025

Description

Testing instructions

Uh oh!

openedx-webhooks commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

e0d commented Jan 23, 2025

Uh oh!

codecov bot commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

igobranco commented Jan 24, 2025

Uh oh!

mphilbrick211 commented Jan 29, 2025

Uh oh!

OmarIthawi Apr 6, 2025

Choose a reason for hiding this comment

Uh oh!

OmarIthawi left a comment

Choose a reason for hiding this comment

Uh oh!

OmarIthawi Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

igobranco Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

igobranco commented Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OmarIthawi commented Apr 10, 2025

Uh oh!

igobranco commented Apr 10, 2025

Uh oh!

igobranco commented Apr 10, 2025

Uh oh!

Uh oh!

fix(i18n): extract translations on `plugins` folder #1589

fix(i18n): extract translations on `plugins` folder #1589

openedx-webhooks commented Jan 14, 2025 •

edited

Loading

codecov bot commented Jan 23, 2025 •

edited

Loading

igobranco Apr 9, 2025 •

edited

Loading

igobranco commented Apr 9, 2025 •

edited

Loading