feat: add atlas pull for the communications app FC-0012

OmarIthawi commented 1 year ago

Adds atlas pull to the Communications MFE. Support for other MFEs will come.

Previously open issues

Here's a couple issues that needs to be tackled:

Issue: How to enable the atlas_pull conditionally?
- Old solution: CORE_MFE_APPS feature.
- Updated solution: Hard-coded if app_name == communications
Issue: How to install openedx-atlas and platform-frontend which are required for atlas pull?
- Solution: In the package.json of the communications MFE.
Issue: Should we use make pull translations? or hand-craft an atlas pull command?
- For discovery Regis recommended to avoid using make and writing an atlas command instead.
- For MFEs, it's better to use the Makefile step to allow each frontend to control its own plugins.
Issue: After changing directory to cd src/i18n/messages the node_modules/.bin is no longer available.
- Suggested solution: Use ENV PATH /openedx/app/node_modules/.bin:${PATH} in base instead of the relative PATH.
Issue: atlas pull is only available for master because https://github.com/openedx/frontend-app-communications/pull/124 is a post-Palm pull request.
- Patch has been done done on palm.master: https://github.com/openedx/frontend-app-communications/pull/156

TODO before merge

[x] Test the translations and post a screenshot
Screenshot: Je parle français!
[x] Backport the atlas pull to the Palm: https://github.com/openedx/frontend-app-communications/pull/156
- [x] Wait for Quince support and merge into Quince: https://github.com/overhangio/tutor-mfe/pull/156
[x] Verify that atlas pull and intl-import.js are both working as expected:
Debug atlas pull and the generated src/i18n/index.js

RUN ls -R /openedx/app/src/i18n/; cat /openedx/app/src/i18n/index.js; exit 1         0.4s
------                                                                                                                        
 > [communications-i18n 4/8] RUN ls -R /openedx/app/src/i18n/; cat /openedx/app/src/i18n/index.js; exit 1:                    
#0 0.385 /openedx/app/src/i18n/:                                                                                              
#0 0.385 index.js                                                                                                             
#0 0.385 messages                                                                                                             
#0 0.385                                                                                                                      
#0 0.385 /openedx/app/src/i18n/messages:
#0 0.385 frontend-app-communications
#0 0.385 frontend-component-footer
#0 0.385 frontend-component-header
#0 0.385 paragon
#0 0.385 
#0 0.385 /openedx/app/src/i18n/messages/frontend-app-communications:
#0 0.385 fr_CA.json
#0 0.385 index.js
#0 0.385 
#0 0.385 /openedx/app/src/i18n/messages/frontend-component-footer:
#0 0.385 ar.json
#0 0.385 de.json
#0 0.385 fr_CA.json
#0 0.385 index.js
#0 0.385 
#0 0.385 /openedx/app/src/i18n/messages/frontend-component-header:
#0 0.385 ar.json
#0 0.385 de.json
#0 0.385 fr_CA.json
#0 0.385 index.js
#0 0.385 
#0 0.385 /openedx/app/src/i18n/messages/paragon:
#0 0.385 ar.json
#0 0.385 de.json
#0 0.385 fr_CA.json
#0 0.385 index.js
#0 0.385 // This file is generated by the openedx/frontend-platform's "intl-import.js" script.
#0 0.385 //
#0 0.385 // Refer to the i18n documents in https://docs.openedx.org/en/latest/developers/references/i18n.html to update
#0 0.385 // the file and use the Micro-frontend i18n pattern in new repositories.
#0 0.385 //
#0 0.385 
#0 0.385 import messagesFromFrontendComponentHeader from './messages/frontend-component-header';
#0 0.385 import messagesFromFrontendComponentFooter from './messages/frontend-component-footer';
#0 0.385 import messagesFromParagon from './messages/paragon';
#0 0.385 import messagesFromFrontendAppCommunications from './messages/frontend-app-communications';
#0 0.385 
#0 0.385 export default [
#0 0.385   messagesFromFrontendComponentHeader,
#0 0.385   messagesFromFrontendComponentFooter,
#0 0.385   messagesFromParagon,
#0 0.385   messagesFromFrontendAppCommunications,
#0 0.385 ];
------

Nightly vs Master

All atlas integration is going to target nightly branches so it's included in Quince but not Palm.

The exception is for the Communications MFE because it has not translations and needs atlas in Palm, therefore it's being used on master.

References

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

OmarIthawi commented 1 year ago

@arbrandes I've spent few hours trying to get this working on Palm but couldn't because MFEs got their support for Atlas after Palm.

I've also tried to work on nightly branches. It was successful, but then the Communications MFE shows a blank screen even without any changes to Tutor:

Would you mind checking and letting me know how to proceed?

http://apps.local.overhang.io:1984/communications/courses/course-v1:s+s+s/bulk_email

OmarIthawi commented 1 year ago

@regisb @arbrandes I've tested this pull request which requires the following plugin:

from tutormfe.hooks import MFE_APPS

@MFE_APPS.add()
def _use_comm_mfe_zeitlabs_fork(mfes):
    mfes["communications"] = {
        "repository": "https://github.com/Zeit-Labs/frontend-app-communications",
        "refs": "https://api.github.com/repos/Zeit-Labs/frontend-app-communications/git/refs/heads",
        "version": "atlas-palm",
        "port": 1984,
    }
    return mfes

So it pulls the right repository until the palm.master pull request is merged.

Please review it and let me know what do you think.

OmarIthawi commented 1 year ago

@arbrandes would you mind giving this pull request one more look? Thanks!!

cc: @regisb

OmarIthawi commented 1 year ago

Status: This is pending the Quince upgrade https://github.com/overhangio/tutor-mfe/pull/156 so I'm presuming that this PR is blocked until the following PR is merged: