feat: allow runtime configuration #955

dcoa · 2022-07-13T20:52:17Z

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.5.0
Update the footer and header to avoid peer dependency errors.
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).
Due to peer dependency error with @edx/frontend-lib-special-exams clone the repo https://github.com/eduNEXT/frontend-lib-special-exams use the branch dcoa/update-fronted-platform and use webpack local module configuration (reference) to test it.

module.exports = {
    localModules: [
        { moduleName: '@edx/frontend-lib-special-exams', dir: '../frontend-lib-special-exams', dist: 'dist' },
    ],
};

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-13T20:52:19Z

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.

dcoa · 2022-07-15T15:24:39Z

Tests are failing because of a peer dependency error, to solve this is necessary an update of @edx/frontend-lib-special-exams (I create a PR there to upgrade frontend-platform openedx/frontend-lib-special-exams#70)

natabene · 2022-07-20T20:04:09Z

@dcoa Thank you for your contribution. Is this ready for our review?

dcoa · 2022-07-22T13:14:07Z

Hi @natabene, yes it's able to start the review process.

ghassanmas · 2022-10-12T06:42:40Z

@dcoa
I have tried is branch on nutmeg but I got an error and I also tried to rebuild again while using your change of special exam lib ref openedx/frontend-lib-special-exams/pull/70, same result:
Here is the error: (Note that the learning load norammly then it crashes and console show this error).

Error: Minified React error #321; visit https://reactjs.org/docs/error-decoder.html?invariant=321 for the full message or use the non-minified dev environment for full errors and additional helpful warnings.
    at Y (react.production.min.js:20:384)
    at useMemo (react.production.min.js:23:164)
    at d (connectAdvanced.js:111:22)
    at Va (react-dom.production.min.js:153:146)
    at Ri (react-dom.production.min.js:175:309)
    at Ti (react-dom.production.min.js:175:139)
    at ki (react-dom.production.min.js:174:178)
    at bu (react-dom.production.min.js:268:238)
    at ds (react-dom.production.min.js:246:265)
    at fs (react-dom.production.min.js:246:194)

itsjeyd · 2022-10-12T15:04:18Z

Hi @dcoa! Just checking in to see where things are at with this PR :) Are you still interested in pursuing it?

dcoa · 2022-10-12T15:50:25Z

Hi, @itsjeyd yes I'm still interested to make this PR merged.

kdmccormick · 2022-10-12T17:25:08Z

@dcoa I'd love to merge this soon! Would you mind rebasing and seeing if you can get tests to pass?

dcoa · 2022-10-12T19:00:07Z

Hi @ghassanmas
I'm not able to reproduce your error.

I test it in devstack using webpack local module to "install" @edx/frontend-lib-special-exams and works

ghassanmas · 2022-10-12T19:21:48Z

- I test this on production mode, i.e. I build it `nom run build` then run it. I think here you are testing in development mode - What are the node/npm version you are using

…

On 12 Oct 2022, at 20:25, Kyle McCormick ***@***.***> wrote: @dcoa <https://github.com/dcoa> I'd love to merge this soon! Would you mind rebasing and seeing if you can get tests to pass? — Reply to this email directly, view it on GitHub <#955 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AD42PH65QLJPTFHTWR4WM23WC3YABANCNFSM53QA7SJQ>. You are receiving this because you commented.

dcoa · 2022-10-12T19:29:19Z

@dcoa I'd love to merge this soon! Would you mind rebasing and seeing if you can get tests to pass?

hi @kdmccormick I can't make test past until @edx/frontend-lib-special-exams is updated (this PR solves that openedx/frontend-lib-special-exams#70)

dcoa · 2022-10-12T19:38:46Z

What are the node/npm version you are using

I'm using node v16.15.1 (npm v8.11.0)

MaferMazu · 2022-10-12T23:40:20Z

I tested it. I needed the fix in frontend-lib-special-exams, but It's working.

ghassanmas · 2022-10-13T12:23:51Z

I tried this many many times in production mode with tutor but it I can't make it to work when using the speficed version of special-exam.

the way I installed your version is by changing the pacakge.json i.e. "@edx/frontend-lib-special-exams": "file:../frontend-lib-special-exams",.

The only case when this work is when I forced npm to ignore peer depedency error i.e. npm install --legacy-peer-deps (In this case I didn't use your version of special-exam).

So I guess the fail probably is not due to changes in this PR rather for frontend-lib-special-exams.

kdmccormick · 2022-10-19T18:38:15Z

@dcoa frontend-lib-special-exams should be all set now!

kdmccormick · 2022-10-21T19:52:09Z

@dcoa now that frontend-lib-special-exams is all set, could you get this PR's test passing?

dcoa · 2022-10-21T21:47:21Z

@kdmccormick I was working on that, and now all passed.

codecov · 2022-10-21T21:48:35Z

Codecov Report

Base: 84.13% // Head: 84.13% // No change to project coverage 👍

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

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #955   +/-   ##
=======================================
  Coverage   84.13%   84.13%           
=======================================
  Files         264      264           
  Lines        4519     4519           
  Branches     1158     1158           
=======================================
  Hits         3802     3802           
  Misses        697      697           
  Partials       20       20

Impacted Files	Coverage Δ
...ourseware/course/sequence/content-lock/messages.js	`100.00% <ø> (ø)`
src/index.jsx	`0.00% <ø> (ø)`
src/alerts/course-start-alert/CourseStartAlert.jsx	`100.00% <100.00%> (ø)`
...ine-tab/alerts/course-end-alert/CourseEndAlert.jsx	`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.

…t cases

kdmccormick · 2022-10-24T15:12:12Z

@dcoa @MaferMazu @ghassanmas Now that the special-exams PR is merged, could any you folks please confirm that this PR works as expected (using Tutor) and that the issues we saw before are resolved? Once that is confirmed, I am happy to merge this!

ghassanmas · 2022-10-25T12:07:34Z

I have test this using tutor/olive in production: you can access it at: https://apps.olive.zaat.dev/learning/course/course-v1:zaatdev+101+1/home Note: this is not the official open testing instance

kdmccormick

Thanks for testing Ghassan!

Sorry @dcoa , one tiny thing I just noticed 😛 Otherwise this looks great.

kdmccormick · 2022-10-25T15:42:33Z

src/courseware/course/sequence/SequenceContent.test.jsx

@@ -32,7 +32,7 @@ describe('Sequence Content', () => {
    expect(screen.queryByText('Loading locked content messaging...')).not.toBeInTheDocument();
    expect(container.querySelector('svg')).toHaveClass('fa-lock');
    expect(screen.getByText(
-      `You must complete the prerequisite: '${gatedContent.prereqSectionName}' to access this content.`,
+      `You must complete the prerequisite: ' ${gatedContent.prereqSectionName} ' to access this content.`,


Is this string change intentional?

Yes, I did it intentionally because react-intl not rendering the single quotes but I found out the real fix here is adding another single quote like this formatjs/formatjs#3870

Thanks, @kdmccormick I'll fix it

dcoa · 2022-10-25T17:14:12Z

Hi @kdmccormick I fix it, but codecov is failing and I am not sure why, seems like some messing configuration, may be re-running workflows this could be solved.

[2022-10-25T16:54:08.711Z] ['error'] There was an error running the uploader: Error uploading to [https://codecov.io:](https://codecov.io/) Error: There was an error fetching the storage URL during POST: 404 - {'detail': ErrorDetail(string='Unable to locate build via Github Actions API. Please upload with the Codecov repository upload token to resolve issue.', code='not_found')} Error: Codecov: Failed to properly upload: The process '/home/runner/work/_actions/codecov/codecov-action/v3/dist/codecov' failed with exit code 255

zacharis278 · 2022-10-27T13:55:20Z

Is this good to merge? I've got a special-exams lib release that can't be pulled in without this now that we've bumped the platform version in that lib to match this PR.

kdmccormick

Thanks for the nudge @zacharis278 , I didn't realize that codecov had finally passed.

Thanks again @dcoa !

openedx-webhooks · 2022-10-27T14:01:47Z

@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.

zacharis278 · 2022-10-28T14:03:58Z

@kdmccormick looks like this PR is possibly causing failures in the edx.org deployment pipeline. Not sure where the issue lies yet, it's yelling about an old version of the footer component that isn't even in this package-lock.

npm ERR! Could not resolve dependency:
npm ERR! peer @edx/frontend-platform@"^1.15.2" from @edx/[email protected]
npm ERR! node_modules/@edx/frontend-component-footer
npm ERR!   @edx/frontend-component-footer@"npm:@edx/frontend-component-footer-edx@^3.1.3" from the root project

dcoa · 2022-10-28T14:23:52Z

Hi @zacharis278 this occurred because footer component is overridden (using a npm package alias) 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/[email protected] (that support frontend-platform v2.x)

zacharis278 · 2022-10-28T14:52:01Z

Thank you! I didn't even realize there was a separate version of this component

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

* feat: allow runtime configuration (openedx#955) Allows frontend-app-learning to be configured at runtime using the LMS's new MFE Configuration API. Part of openedx/wg-frontend#103 * refactor: install alpha dependencies and update lint rules * refactor: update code according with paragon alpha 22 * fix: test wasn't passing correctly * chore: fix header alias --------- Co-authored-by: bra-i-am <[email protected]>

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

xitij2000 mentioned this pull request Jul 18, 2022

docs: ADR that proposes a common stylesheet for all MFEs openedx/frontend-platform#351

Closed

4 tasks

kdmccormick mentioned this pull request Aug 4, 2022

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

Closed

natabene changed the title ~~feat: allow runtieme configuration~~ feat: allow runtime configuration Aug 18, 2022

ghassanmas mentioned this pull request Oct 16, 2022

MFE dynamic config overhangio/tutor-mfe#69

Merged

kdmccormick mentioned this pull request Oct 18, 2022

fix: update frontend-platform to avoid peer dependency error openedx/frontend-lib-special-exams#70

Closed

zacharis278 mentioned this pull request Oct 19, 2022

fix: quick fix to failing release task openedx/frontend-lib-special-exams#74

Merged

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

MFE runtime configuration openedx/wg-frontend#103

Closed

6 tasks

hurtstotouchfire requested review from Tj-Tracy and removed request for Tj-Tracy October 21, 2022 17:17

dcoa added 3 commits October 21, 2022 12:44

feat: allow favicon set by runtime

52d41ee

fix: 18n date format relative import and test

d559042

fix: add space to i18n single quotes

2974b2d

chore: update frotend-patform and other dependencies

d519362

dcoa force-pushed the dcoa/runtime-favicon-title branch from 5e9dcae to 29964ac Compare October 21, 2022 21:40

test: fix CoursewareRedirectLandingPage and ActiveEnterpriseAlert tes…

351a096

…t cases

dcoa force-pushed the dcoa/runtime-favicon-title branch from 29964ac to 351a096 Compare October 21, 2022 21:55

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

kdmccormick reviewed Oct 25, 2022

View reviewed changes

fix: single quotes rendering

acaa7ce

kdmccormick approved these changes Oct 27, 2022

View reviewed changes

kdmccormick merged commit be72e36 into openedx:master Oct 27, 2022

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 14, 2022

feat: allow runtime configuration (#955)

d00961d

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

dcoa added a commit to eduNEXT/frontend-app-learning that referenced this pull request Jan 3, 2024

feat: allow runtime configuration (openedx#955)

cd5ae57

Allows frontend-app-learning 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 runtime configuration #955

feat: allow runtime configuration #955

dcoa commented Jul 13, 2022 •

edited

Loading

openedx-webhooks commented Jul 13, 2022

dcoa commented Jul 15, 2022 •

edited

Loading

natabene commented Jul 20, 2022

dcoa commented Jul 22, 2022

ghassanmas commented Oct 12, 2022

itsjeyd commented Oct 12, 2022

dcoa commented Oct 12, 2022

kdmccormick commented Oct 12, 2022

dcoa commented Oct 12, 2022

ghassanmas commented Oct 12, 2022 via email

dcoa commented Oct 12, 2022

dcoa commented Oct 12, 2022

MaferMazu commented Oct 12, 2022 •

edited

Loading

ghassanmas commented Oct 13, 2022

kdmccormick commented Oct 19, 2022

kdmccormick commented Oct 21, 2022

dcoa commented Oct 21, 2022

codecov bot commented Oct 21, 2022 •

edited

Loading

kdmccormick commented Oct 24, 2022

ghassanmas commented Oct 25, 2022

kdmccormick left a comment

kdmccormick Oct 25, 2022

dcoa Oct 25, 2022

dcoa commented Oct 25, 2022

zacharis278 commented Oct 27, 2022

kdmccormick left a comment

openedx-webhooks commented Oct 27, 2022

zacharis278 commented Oct 28, 2022

dcoa commented Oct 28, 2022

zacharis278 commented Oct 28, 2022

feat: allow runtime configuration #955

feat: allow runtime configuration #955

Conversation

dcoa commented Jul 13, 2022 • edited Loading

openedx-webhooks commented Jul 13, 2022

dcoa commented Jul 15, 2022 • edited Loading

natabene commented Jul 20, 2022

dcoa commented Jul 22, 2022

ghassanmas commented Oct 12, 2022

itsjeyd commented Oct 12, 2022

dcoa commented Oct 12, 2022

kdmccormick commented Oct 12, 2022

dcoa commented Oct 12, 2022

ghassanmas commented Oct 12, 2022 via email

dcoa commented Oct 12, 2022

dcoa commented Oct 12, 2022

MaferMazu commented Oct 12, 2022 • edited Loading

ghassanmas commented Oct 13, 2022

kdmccormick commented Oct 19, 2022

kdmccormick commented Oct 21, 2022

dcoa commented Oct 21, 2022

codecov bot commented Oct 21, 2022 • edited Loading

Codecov Report

kdmccormick commented Oct 24, 2022

ghassanmas commented Oct 25, 2022

kdmccormick left a comment

Choose a reason for hiding this comment

kdmccormick Oct 25, 2022

Choose a reason for hiding this comment

dcoa Oct 25, 2022

Choose a reason for hiding this comment

dcoa commented Oct 25, 2022

zacharis278 commented Oct 27, 2022

kdmccormick left a comment

Choose a reason for hiding this comment

openedx-webhooks commented Oct 27, 2022

zacharis278 commented Oct 28, 2022

dcoa commented Oct 28, 2022

zacharis278 commented Oct 28, 2022

dcoa commented Jul 13, 2022 •

edited

Loading

dcoa commented Jul 15, 2022 •

edited

Loading

MaferMazu commented Oct 12, 2022 •

edited

Loading

codecov bot commented Oct 21, 2022 •

edited

Loading