Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Runtime config in dev mode #187

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

arbrandes
Copy link
Collaborator

@arbrandes arbrandes commented Feb 6, 2024

Description

MFE runtime configuration in dev mode was not working properly prior to this. It would always end up querying the example.com site configuration. With this change, we force the request origin to always be the LMS host and port, as opposed to something like apps.local.edly.io:2001, which would trip up the Django site detection.

Reproducing and testing

To reproduce (and test) the problem:

  1. Check out an MFE locally. For instance, the learner dashboard: git clone https://github.com/openedx/frontend-app-learner-dashboard
  2. Configure a local mount for the MFE. tutor mounts add ./frontend-app-learner-dashboard
  3. Launch Tutor in dev mode: tutor dev launch. (This should rebuild the appropriate images.)
  4. Go to the site configuration admin page (http://local.edly.io:8000/admin/site_configuration/siteconfiguration/), and under the local.edly.io:8000 entry, add an override for the dashboard such as:
{
    "MFE_CONFIG_OVERRIDES": {
       "learner-dashboard": {
            "BOGUS": true
        }
    }
}
  1. Navigate to the MFE in a browser (http://apps.local.edly.io:1996/learner-dashboard/). Open the Network tab in the debug console, and find the "mfe_config" request. (The URL should be "http://apps.local.edly.io:1996/api/mfe_config/v1?mfe=learner-dashboard".) Click on it, and then on the "Response" tab. If the "BOGUS" entry is not in the configuration object, you have successfully reproduced the problem.

To test the fix, check out this branch, save configuration, (tutor config save), rebuild the learner dashboard dev image with tutor images build learner-dashboard-dev, and restart the dev environment. You should then be able to see the "BOGUS" entry.

Further information

For reference on changeOrigin, see https://webpack.js.org/configuration/dev-server/#devserverproxy and https://github.com/chimurai/http-proxy-middleware?tab=readme-ov-file#http-proxy-options.

MFE runtime configuration was not working properly prior to this.  It
would always end up querying the `example.com` site configuration.  With
this change, we force the request origin to always be the LMS host and
port, as opposed to something like `apps.local.edly.io:2001`, which
would trip up the Django site detection.
@arbrandes arbrandes requested a review from regisb February 6, 2024 13:28
@arbrandes arbrandes force-pushed the fix-dev-runtime-config branch from 7cadc2e to 0cc2d73 Compare February 6, 2024 13:58
@arbrandes
Copy link
Collaborator Author

@regisb, objections?

Copy link
Contributor

@regisb regisb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooooook I got it! LGTM :)

@DawoudSheraz DawoudSheraz self-requested a review February 20, 2024 09:19
@arbrandes arbrandes merged commit 5b08860 into overhangio:master Feb 27, 2024
2 of 3 checks passed
@arbrandes arbrandes deleted the fix-dev-runtime-config branch February 27, 2024 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants