feat: allow runtieme configuration #586

dcoa · 2022-07-13T19:27:20Z

Description

This PR is part of the work to make it possible to configure the frontend applications at runtime (you can referer to this openedx/wg-frontend#103).

Changes

Update frontend-platform to version 2.6.2
Update the footer and header to avoid peer dependency errors.
Create a component Head, that uses Helmet library and integrates internationalization to change the MFE name in the title tag according to the language, and change the favicon in runtime.
Update test according to i18n v5.

How to test

To allow runtime configuration set a new environment variables MFE_CONFIG_API_URL and APP_ID in the env file and add the api url ( To test this you can use the API from this feat: add mfe config api edx-platform#30473 or use an external tool to mock the API response).
The API should respond with a JSON with the config values, something like:

{
  "SITE_NAME": "Test Site",
  "LOGO_URL": "https://testimage.com/logo.svg",
  "LOGO_TRADEMARK_URL": "https://testimage.com/logo.svg",
  "LOGO_WHITE_URL": "https://testimage.com/logo.svg",
  "FAVICON_URL": "https://testimage.com/favicon.ico",
}

The initialize process should work normally.
Note: You can combine buildtime and runtime configuration

openedx-webhooks · 2022-07-13T19:27:23Z

Thanks for the pull request, @dcoa! 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.

codecov · 2022-07-13T19:33:11Z

Codecov Report

Base: 64.96% // Head: 65.14% // Increases project coverage by +0.17% 🎉

Coverage data is based on head (fe2a6c7) compared to base (963ff21).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #586      +/-   ##
==========================================
+ Coverage   64.96%   65.14%   +0.17%     
==========================================
  Files          45       47       +2     
  Lines         805      809       +4     
  Branches      156      156              
==========================================
+ Hits          523      527       +4     
  Misses        272      272              
  Partials       10       10

Impacted Files	Coverage Δ
src/index.jsx	`0.00% <ø> (ø)`
src/head/Head.jsx	`100.00% <100.00%> (ø)`
src/head/messages.js	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

natabene · 2022-07-20T20:02:43Z

@dcoa Thank you for your contribution, is this ready for our review?

dcoa · 2022-07-20T23:04:56Z

Hi @natabene , yes is ready.

kdmccormick · 2022-10-12T17:22:55Z

I plan to approve and merge this on Monday 10/17 unless specific concerns are raised.

MaferMazu · 2022-10-13T14:44:25Z

I tested it, and it works. 🌟

kdmccormick

Just a few small suggestions, but this looks good to me otherwise.

kdmccormick · 2022-10-13T14:46:21Z

src/head/Head.test.jsx

+
+describe('Head', () => {
+  const props = {};
+  it('should match render title tag and fivicon with the site configuration values', () => {


Suggested change

it('should match render title tag and fivicon with the site configuration values', () => {

it('should match render title tag and favicon with the site configuration values', () => {

kdmccormick · 2022-10-13T14:47:35Z

.env

+APP_ID=
+MFE_CONFIG_API_URL=


I suggest using single quotes here. The effect is the same, but it would make these new lines consistent with the rest of the file, which could be less confusing for someone who doesn't know whether or not the quotes are important.

Suggested change

APP_ID=

MFE_CONFIG_API_URL=

APP_ID=''

MFE_CONFIG_API_URL=''

kdmccormick · 2022-10-13T14:47:47Z

.env.development

+APP_ID=
+MFE_CONFIG_API_URL=


Same suggestion

Suggested change

APP_ID=

MFE_CONFIG_API_URL=

APP_ID=''

MFE_CONFIG_API_URL=''

kdmccormick · 2022-10-13T14:47:59Z

.env.test

+APP_ID=
+MFE_CONFIG_API_URL=


Same suggestion

Suggested change

APP_ID=

MFE_CONFIG_API_URL=

APP_ID=''

MFE_CONFIG_API_URL=''

openedx-webhooks · 2022-10-17T16:14:03Z

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

arbrandes · 2022-10-21T14:32:27Z

@hurtstotouchfire reports that this broke 2U's stage build. At this point it's still unclear what exact change caused this (and why it wasn't caught in CI), but here's the trace for the record:

npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! While resolving: @edx/frontend-app-profile@1.0.0-semantically-released
npm ERR! Found: @edx/frontend-platform@2.6.2
npm ERR! node_modules/@edx/frontend-platform
npm ERR!   @edx/frontend-platform@"2.6.2" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer @edx/frontend-platform@"^1.15.2" from @edx/frontend-component-footer@3.5.0
npm ERR! node_modules/@edx/frontend-component-footer
npm ERR!   @edx/frontend-component-footer@"npm:@edx/frontend-component-footer-edx@^3.4.0" from the root project
npm ERR! 
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR! 
npm ERR! See /home/go/.npm/eresolve-report.txt for a full report.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/go/.npm/_logs/2022-10-18T06_14_35_314Z-debug-0.log
Traceback (most recent call last):
  File "/usr/local/bin/frontend_build.py", line 8, in <module>
    sys.exit(frontend_build())
  File "/usr/local/lib/python3.8/dist-packages/click/core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/lib/python3.8/dist-packages/click/core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "/usr/local/lib/python3.8/dist-packages/click/core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/lib/python3.8/dist-packages/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/usr/local/lib/python3.8/dist-packages/tubular/scripts/frontend_build.py", line 64, in frontend_build
    builder.install_requirements()
  File "/usr/local/lib/python3.8/dist-packages/tubular/scripts/frontend_utils.py", line 64, in install_requirements
    self.install_requirements_npm_aliases()
  File "/usr/local/lib/python3.8/dist-packages/tubular/scripts/frontend_utils.py", line 85, in install_requirements_npm_aliases
    self.FAIL('Could not run `npm install` aliases {} for app {}.'.format(
TypeError: _fail() missing 1 required positional argument: 'message'
[go] Task status: failed, took: 48.802s, exited: 1
[go] Current job status: failed

kdmccormick · 2022-10-21T14:35:30Z

Oof, looks like there's an error in the error handling code. The pipeline is trying to print out more information (self.FAIL(...)), but the FAIL function itself is failing, so we don't get the extra info:

    self.FAIL('Could not run `npm install` aliases {} for app {}.'.format(
TypeError: _fail() missing 1 required positional argument: 'message'

dcoa · 2022-10-21T15:02:48Z

Hi, @arbrandes
My understanding of the error is footer component is overridden with frontend-component-footer-edx (version 3.x only support frontend-platform 1.x) this could be solved using frontend-component-footer-edx in a version >= 4.1.0 edx/frontend-component-footer-edx@v4.0.0...v4.1.0 (that support frontend-platform v2.x)

Allows frontend-app-profile to be configured at runtime using the LMS's new MFE Configuration API. Part of openedx/wg-frontend#103

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jul 13, 2022

dcoa mentioned this pull request Jul 13, 2022

MFE runtime configuration openedx/wg-frontend#103

Closed

6 tasks

kdmccormick mentioned this pull request Aug 4, 2022

feat: build an image that uses runtime configuration kdmccormick/tutor-mfe#1

Closed

dcoa force-pushed the dcoa/runtime-favicon-title branch from c156919 to 300f72e Compare October 11, 2022 19:05

feat: allow runtime configuration

9e2ee48

dcoa force-pushed the dcoa/runtime-favicon-title branch from 300f72e to 9e2ee48 Compare October 11, 2022 19:28

kdmccormick reviewed Oct 13, 2022

View reviewed changes

fix: add single quotes to env variables and fix typo

fe2a6c7

kdmccormick approved these changes Oct 13, 2022

View reviewed changes

dcoa force-pushed the dcoa/runtime-favicon-title branch from 5cd5bdd to fe2a6c7 Compare October 13, 2022 19:34

ghassanmas mentioned this pull request Oct 16, 2022

MFE dynamic config overhangio/tutor-mfe#69

Merged

kdmccormick merged commit 4f00bc4 into openedx:master Oct 17, 2022

arbrandes linked an issue Oct 21, 2022 that may be closed by this pull request

MFE runtime configuration openedx/wg-frontend#103

Closed

6 tasks

ghassanmas mentioned this pull request Oct 24, 2022

PR(s) Needs to be backported or created to Olive openedx/wg-build-test-release#205

Closed

arbrandes removed a link to an issue Nov 3, 2022

MFE runtime configuration openedx/wg-frontend#103

Closed

6 tasks

arbrandes pushed a commit that referenced this pull request Nov 22, 2022

feat: allow runtieme configuration (#586)

d22a165

Allows frontend-app-profile to be configured at runtime using the LMS's new MFE Configuration API. Part of openedx/wg-frontend#103

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow runtieme configuration #586

feat: allow runtieme configuration #586

dcoa commented Jul 13, 2022 •

edited

Loading

openedx-webhooks commented Jul 13, 2022

codecov bot commented Jul 13, 2022 •

edited

Loading

natabene commented Jul 20, 2022

dcoa commented Jul 20, 2022

kdmccormick commented Oct 12, 2022

MaferMazu commented Oct 13, 2022 •

edited

Loading

kdmccormick left a comment

kdmccormick Oct 13, 2022

kdmccormick Oct 13, 2022

kdmccormick Oct 13, 2022

kdmccormick Oct 13, 2022

openedx-webhooks commented Oct 17, 2022

arbrandes commented Oct 21, 2022

kdmccormick commented Oct 21, 2022

dcoa commented Oct 21, 2022 •

edited

Loading

	it('should match render title tag and fivicon with the site configuration values', () => {
	it('should match render title tag and favicon with the site configuration values', () => {

-APP_ID=
-MFE_CONFIG_API_URL=
+APP_ID=''
+MFE_CONFIG_API_URL=''

feat: allow runtieme configuration #586

feat: allow runtieme configuration #586

Conversation

dcoa commented Jul 13, 2022 • edited Loading

openedx-webhooks commented Jul 13, 2022

codecov bot commented Jul 13, 2022 • edited Loading

Codecov Report

natabene commented Jul 20, 2022

dcoa commented Jul 20, 2022

kdmccormick commented Oct 12, 2022

MaferMazu commented Oct 13, 2022 • edited Loading

kdmccormick left a comment

Choose a reason for hiding this comment

kdmccormick Oct 13, 2022

Choose a reason for hiding this comment

kdmccormick Oct 13, 2022

Choose a reason for hiding this comment

kdmccormick Oct 13, 2022

Choose a reason for hiding this comment

kdmccormick Oct 13, 2022

Choose a reason for hiding this comment

openedx-webhooks commented Oct 17, 2022

arbrandes commented Oct 21, 2022

kdmccormick commented Oct 21, 2022

dcoa commented Oct 21, 2022 • edited Loading

dcoa commented Jul 13, 2022 •

edited

Loading

codecov bot commented Jul 13, 2022 •

edited

Loading

MaferMazu commented Oct 13, 2022 •

edited

Loading

dcoa commented Oct 21, 2022 •

edited

Loading