feat!: remove Transifex calls for OEP-58 | FC-0012 #383

hopewise · 2023-10-14T03:39:15Z

Breaking change!

This change breaks the Jenkins transifex integration which has been deprecated in favor of the new GitHub Transifex App integration as part of OEP-58.

Changes

Removes all direct use of tx pull and tx push commands from the micro-frontend in favor
of the atlas pull command.
Remove source and language translations from the repositories, hence no .json files will be committed into the repos.
src/i18n/index.js should export and empty array so the make pull_translations override it with the dynamic list of languages.
Remove the experimental OPENEDX_ATLAS_PULL flag to make atlas pull the default.
Remove all Transifex related Makefile targets and other files.

Test results

Verify that make pull_translations works as expected.

make pull_translations test results

# I've run the following commands:
$ make requirements
$ make pull_translations
$ git diff

# Output of the commmands:

make pull_translations
rm -rf src/i18n/messages
cd src/i18n/ \
	  && atlas pull translations/studio-frontend/src/i18n/messages:messages
Pulling translation files
 - directory: translations/studio-frontend/src/i18n/messages:messages
 - repository: openedx/openedx-translations
 - branch: main
 - filter: Not Specified
 - expand-glob: Not Specified
Creating a temporary Git repository to pull translations into "./translations_TEMP"...
Done.
Setting git sparse-checkout rules...
Done.
Pulling translation files from the repository...
remote: Enumerating objects: 13, done.
remote: Counting objects: 100% (13/13), done.
remote: Compressing objects: 100% (11/11), done.
remote: Total 13 (delta 2), reused 12 (delta 2), pack-reused 0
Receiving objects: 100% (13/13), 64.55 KiB | 434.00 KiB/s, done.
Resolving deltas: 100% (2/2), done.
Your branch is up to date with 'origin/main'.
Done.
Copying translations from "./translations_TEMP/translations/studio-frontend/src/i18n/messages" to "./messages"...
Done.
Removing temporary directory...
Done.

Translations pulled successfully!
src/i18n/scripts/generateSupportedLangs.js
generateSupportedLangs.js: Finished generating the 'currentlySupportedLangs.jsx' file.

Merge timeline

This should only be merged after Translation Infrastructure update OEP-58 is fully implemented.

The timing announcement will be shared by @brian-smith-tcril on #translations-project-fc-0012 Open edX Slack channel.

Keep this pull request as a draft to prevent accidental merge.

Pre-merge checklist

Wait for approval on Deprecate pre-OEP-58 Transifex scripts from all repositories wg-translations#20

References

This contribution is part of the FC-0012 project which is sparked by the Translation Infrastructure update OEP-58.

Up-to-date project overview and details are available in the Approach Memo and Technical Discovery: Translations Infrastructure Implementation document.

Join the conversation on Open edX Slack #translations-project-fc-0012.

Check the links above for full information about the overall project.

openedx-webhooks · 2023-10-14T03:39:19Z

Thanks for the pull request, @hopewise! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

OmarIthawi

@hopewise Thanks for the work.

Please check the comments regarding the testing error and other required changes.

Makefile

OmarIthawi · 2023-10-18T06:39:20Z

Makefile

-# Pulls translations from Transifex.
-pull_translations:
-	tx pull -t -f --mode reviewed --languages=$(transifex_langs)
-else
 # Experimental: OEP-58 Pulls translations using atlas


Will be no longer experimental.

Suggested change

# Experimental: OEP-58 Pulls translations using atlas

OmarIthawi · 2023-10-18T06:39:56Z

Makefile

@@ -93,18 +90,12 @@ push_translations:
 	./node_modules/@edx/reactifex/bash_scripts/put_comments_v3.sh


Please remove the push_translations program.

Makefile

OmarIthawi · 2023-10-18T06:48:29Z

Makefile

 # Experimental: OEP-58 Pulls translations using atlas
 pull_translations:
 	rm -rf src/i18n/messages
 	cd src/i18n/ \
-	  && atlas pull --filter=$(transifex_langs) translations/studio-frontend/src/i18n/messages:messages
+	  && atlas pull translations/studio-frontend/src/i18n/messages:messages
 	$(generate_supported_langs) $(transifex_langs)


The problem with this line is that it depends on the transifex_langs hard-coded variable, which is no longer used.

One solution is to update the src/i18n/scripts/generateSupportedLangs.js script to generate the list based on the following algorithm:

list all files in src/i18n/messages:

studio-frontend $ ls src/i18n/messages | cat ar.json de_DE.json de.json es_419.json ...

Use the list of languages from the files, instead of the process.argv:

studio-frontend/src/i18n/scripts/generateSupportedLangs.js

Line 100 in 2116cdb

languages: languagesString.split(','),

@OmarIthawi fixes got applied, you can check please.

Thanks @hopewise, I've checked the fixes and it looks good. It appears that we don't have tests for the new changes which I think is important to have.

Would you mind adding tests and fix the failed nodejs_CI tests build as well?

This pull request needs to be rebased over feat: generate currentlySupportedLangs.jsx dynamically | FC-0012 #389 once the latter is merged.

OmarIthawi · 2024-03-14T09:01:51Z

@itsjeyd @timmc-edx @brian-smith-tcril this pull request is ready for merge.

OmarIthawi

@brian-smith-tcril @timmc-edx this is now ready for merge.

Please note that this is a rather arcane frontend that is not a usual MFE, so if something isn't working in production, please revert this PR and ping me for a fix.

OmarIthawi · 2024-03-18T14:49:10Z

@brian-smith-tcril @spencertiberi the CI failure is a post-test issue.

Please check the link below and let me know how it can be fixed:

https://github.com/openedx/frontend-app-payment/actions/runs/8316512467/job/22756054443?pr=820#step:11:1

brian-smith-tcril · 2024-03-18T19:55:47Z

@OmarIthawi I'm not sure what you mean by a post-test issue. From what I can see the test is failing because it is expecting the generated currentlySupportedLangs.jsx file to include

    import './ar.json';
    import './fr.json';
    import './fr_CA.json';
    import './zh_CN.json';

but the generated file doesn't include that.

Summary of all failing tests
FAIL src/utils/i18n/scripts/generateSupportedLangs.test.js
  ● generate with three languages › should generate currentlySupportedLangs.jsx

    expect(received).toEqual(expected)
    
    Expected value to equal:
      "// This file is generated by the \"generateSupportedLangs.js\" script.
    import './ar.json';
    import './fr.json';
    import './fr_CA.json';
    import './zh_CN.json';
    import arData from 'react-intl/locale-data/ar';
    import frData from 'react-intl/locale-data/fr';
    import zhData from 'react-intl/locale-data/zh';
    
    export default {
      'ar': arData,
      'fr': frData,
      'fr-ca': frData,
      'zh-cn': zhData,
    };
    "
    Received:
      "// This file is generated by the \"generateSupportedLangs.js\" script.
    import arData from 'react-intl/locale-data/ar';
    import frData from 'react-intl/locale-data/fr';
    import zhData from 'react-intl/locale-data/zh';
    
    export default {
      'ar': arData,
      'fr': frData,
      'fr-ca': frData,
      'zh-cn': zhData,
    };
    "

OmarIthawi · 2024-03-19T05:33:47Z

@brian-smith-tcril that doesn't look right. I'll check why this test has failed.

codecov · 2024-03-19T06:05:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.18%. Comparing base (500ae54) to head (46769f8).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #383      +/-   ##
==========================================
- Coverage   99.18%   99.18%   -0.01%     
==========================================
  Files          75       75              
  Lines        1713     1711       -2     
  Branches      416      416              
==========================================
- Hits         1699     1697       -2     
  Misses         14       14

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

OmarIthawi · 2024-03-19T06:06:40Z

@brian-smith-tcril the comment was posted here by mistake.

I've fixed the issue anyway.

cc: @spencertiberi

openedx-webhooks · 2024-03-20T13:33:37Z

@hopewise 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

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

hopewise force-pushed the samir/studio-frontend branch from 9ed6a4e to 480b6d2 Compare October 14, 2023 03:55

hopewise marked this pull request as draft October 14, 2023 04:02

mphilbrick211 added needs test run Author's first PR to this repository, awaiting test authorization from Axim and removed needs test run Author's first PR to this repository, awaiting test authorization from Axim labels Oct 16, 2023

OmarIthawi suggested changes Oct 18, 2023

View reviewed changes

OmarIthawi mentioned this pull request Oct 18, 2023

Deprecate pre-OEP-58 Transifex scripts from all repositories openedx/wg-translations#20

Closed

1 task

hopewise force-pushed the samir/studio-frontend branch 5 times, most recently from 808af1e to 1e8859b Compare October 29, 2023 05:11

itsjeyd added the blocked by other work PR cannot be finished until other work is complete label Nov 2, 2023

OmarIthawi mentioned this pull request Feb 2, 2024

feat: generate currentlySupportedLangs.jsx dynamically | FC-0012 #389

Merged

itsjeyd removed the blocked by other work PR cannot be finished until other work is complete label Feb 23, 2024

hopewise marked this pull request as ready for review March 14, 2024 07:11

OmarIthawi force-pushed the samir/studio-frontend branch from 1e8859b to 38a0f3b Compare March 14, 2024 09:00

OmarIthawi force-pushed the samir/studio-frontend branch 3 times, most recently from 2039041 to 9d4ffd1 Compare March 17, 2024 11:14

OmarIthawi approved these changes Mar 17, 2024

View reviewed changes

OmarIthawi force-pushed the samir/studio-frontend branch from 9d4ffd1 to 5b04f0c Compare March 19, 2024 05:36

feat!: remove Transifex calls for OEP-58

46769f8

OmarIthawi force-pushed the samir/studio-frontend branch from 5b04f0c to 46769f8 Compare March 19, 2024 06:02

brian-smith-tcril approved these changes Mar 19, 2024

View reviewed changes

brian-smith-tcril merged commit 39a7255 into openedx:master Mar 20, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: remove Transifex calls for OEP-58 | FC-0012 #383

feat!: remove Transifex calls for OEP-58 | FC-0012 #383

hopewise commented Oct 14, 2023 •

edited

Loading

openedx-webhooks commented Oct 14, 2023 •

edited

Loading

OmarIthawi left a comment

OmarIthawi Oct 18, 2023

OmarIthawi Oct 18, 2023

OmarIthawi Oct 18, 2023

hopewise Oct 29, 2023

OmarIthawi Nov 5, 2023

OmarIthawi Feb 2, 2024

OmarIthawi commented Mar 14, 2024

OmarIthawi left a comment

OmarIthawi commented Mar 18, 2024

brian-smith-tcril commented Mar 18, 2024

OmarIthawi commented Mar 19, 2024 •

edited

Loading

codecov bot commented Mar 19, 2024

OmarIthawi commented Mar 19, 2024

openedx-webhooks commented Mar 20, 2024

		@@ -93,18 +90,12 @@ push_translations:
		./node_modules/@edx/reactifex/bash_scripts/put_comments_v3.sh

feat!: remove Transifex calls for OEP-58 | FC-0012 #383

feat!: remove Transifex calls for OEP-58 | FC-0012 #383

Conversation

hopewise commented Oct 14, 2023 • edited Loading

Breaking change!

Changes

Test results

Merge timeline

Pre-merge checklist

References

openedx-webhooks commented Oct 14, 2023 • edited Loading

OmarIthawi left a comment

Choose a reason for hiding this comment

OmarIthawi Oct 18, 2023

Choose a reason for hiding this comment

OmarIthawi Oct 18, 2023

Choose a reason for hiding this comment

OmarIthawi Oct 18, 2023

Choose a reason for hiding this comment

hopewise Oct 29, 2023

Choose a reason for hiding this comment

OmarIthawi Nov 5, 2023

Choose a reason for hiding this comment

OmarIthawi Feb 2, 2024

Choose a reason for hiding this comment

OmarIthawi commented Mar 14, 2024

OmarIthawi left a comment

Choose a reason for hiding this comment

OmarIthawi commented Mar 18, 2024

brian-smith-tcril commented Mar 18, 2024

OmarIthawi commented Mar 19, 2024 • edited Loading

codecov bot commented Mar 19, 2024

Codecov Report

OmarIthawi commented Mar 19, 2024

openedx-webhooks commented Mar 20, 2024

hopewise commented Oct 14, 2023 •

edited

Loading

openedx-webhooks commented Oct 14, 2023 •

edited

Loading

OmarIthawi commented Mar 19, 2024 •

edited

Loading