Skip to content

Conversation

@awaisdar001
Copy link
Contributor

@awaisdar001 awaisdar001 commented Nov 2, 2021

TNL-8896
TNL-9227

Related PRs:
course authoring mfe: openedx/frontend-app-authoring#210
edge: https://github.com/edx/edge-internal/pull/395/files
edX: https://github.com/edx/edx-internal/pull/5672

Description:
This PR adds config.LEARNING_BASE_URL for all frontends to use.

Merge checklist:

  • Consider running your code modifications in the included example app within frontend-platform. This can be done by running npm start and opening http://localhost:8080.
  • Consider testing your code modifications in another local micro-frontend using local aliases configured via the module.config.js file in frontend-build.
  • Verify your commit title/body conforms to the conventional commits format (e.g., fix, feat) and is appropriate for your code change. Consider whether your code is a breaking change, and modify your commit accordingly.

Post merge:

  • After the build finishes for the merged commit, verify the new release has been pushed to NPM.

@davidjoy
Copy link
Contributor

davidjoy commented Nov 2, 2021

Note that you'll want to add this new variable to setupTest.js and the .env.development and .env.test files as well.

Also, would you be able to add a PR to update the environment variable documentation here?

https://github.com/edx/edx-developer-docs/blob/master/docs/micro_frontends/index.rst

I would have mentioned it earlier but forgot!

@awaisdar001
Copy link
Contributor Author

@davidjoy added the suggested changes, thanks for helping me out here. I really appreciate that.

src/setupTest.js Outdated
process.env.ECOMMERCE_BASE_URL = 'http://localhost:18130';
process.env.LANGUAGE_PREFERENCE_COOKIE_NAME = 'openedx-language-preference';
process.env.LMS_BASE_URL = 'http://localhost:18000';
process.env.LOGIN_URL = 'http://localhost:2000';
Copy link
Contributor

Choose a reason for hiding this comment

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

Watch out! This says LOGIN_URL still. 😀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops!!! 🤭

Copy link
Contributor

@davidjoy davidjoy left a comment

Choose a reason for hiding this comment

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

setupTest needs a tweak from LOGIN_URL to LEARNING_BASE_URL

@davidjoy davidjoy changed the title feat: adds learning mfe url feat: adds LEARNING_BASE_URL to config Nov 2, 2021
This PR adds `config.LEARNING_BASE_UR` for all frontends to use.
@awaisdar001
Copy link
Contributor Author

Done - I will update the docs after this PR is merged.

@awaisdar001 awaisdar001 requested a review from davidjoy November 3, 2021 14:26
@awaisdar001 awaisdar001 merged commit d1823fa into master Nov 3, 2021
@awaisdar001 awaisdar001 deleted the awaisdar001-patch-1 branch November 3, 2021 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants