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

feat: add course authoring mfe base url to env vars #182

Conversation

rpenido
Copy link
Contributor

@rpenido rpenido commented Jan 2, 2024

Description

This PR adds the "COURSE_AUTHORING_MFE_BASE_URL" env var pointing to the Course Authoring MFE address.

Additional Info

We are rendering a course authoring URL in an iFrame inside the library authoring as part of the Tagging Project, so we need to know the Course Authoring MFE address.

@rpenido rpenido marked this pull request as ready for review January 5, 2024 01:26
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.

Thanks for your contribution! Can you please add a changelog entry? https://docs.tutor.overhang.io/tutor.html#contributing

@rpenido rpenido force-pushed the rpenido/add-course-authoring-mfe-base-url branch from 4407799 to 85abfde Compare January 8, 2024 18:11
@rpenido
Copy link
Contributor Author

rpenido commented Jan 8, 2024

Done!

@@ -38,6 +38,7 @@ MFE_CONFIG["ACCOUNT_SETTINGS_URL"] = ACCOUNT_MICROFRONTEND_URL
{% if get_mfe("course-authoring") %}
MFE_CONFIG["ENABLE_NEW_EDITOR_PAGES"] = True
MFE_CONFIG["ENABLE_PROGRESS_GRAPH_SETTINGS"] = True
MFE_CONFIG["COURSE_AUTHORING_MFE_BASE_URL"] = "http://{{ MFE_HOST }}:{{ get_mfe("course-authoring")["port"] }}"

Choose a reason for hiding this comment

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

@rpenido Shouldn't this be "http://{{ MFE_HOST }}:{{ get_mfe("course-authoring")["port"] }}/course-authoring" ? That seems more consistent with all of the other MFE _URL variables in this file.

Also, there seems to be some inconsistency about whether these have a trailing slash or not, but most of them (except account and learner-dashboard) do not so that's probably the way to go.

Copy link
Contributor Author

@rpenido rpenido Jan 8, 2024

Choose a reason for hiding this comment

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

To give some context, we are using this in the iFrame.
We are using the COURSE_AUTHORING_MFE_BASE_URL here: https://github.com/openedx/frontend-app-library-authoring/pull/400/files

We have the url /course-authoring in the COURSE_AUTHORING_MICROFRONTEND_URL variable already set here:

COURSE_AUTHORING_MICROFRONTEND_URL = "http://{{ MFE_HOST }}:{{ get_mfe('course-authoring')["port"] }}/course-authoring"

But this URL renders an error to me:
http://apps.local.edly.io:2001/course-authoring/home
image

The course authoring URL in my tutor instance is the following:
image

Is there some misconfiguration on my side?

PS: Feel free to close this is this is not the right way to fix it

Choose a reason for hiding this comment

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

It's actually a bug in the course authoring MFE: openedx/frontend-app-authoring#784

"Studio Home" should be showing at http://apps.local.overhang.io:2001/course-authoring/home (or edly.io)

So I think COURSE_AUTHORING_MICROFRONTEND_URL is all you need, and you can close this PR.

@rpenido rpenido marked this pull request as draft January 9, 2024 20:21
@rpenido
Copy link
Contributor Author

rpenido commented Jan 9, 2024

Thank you @bradenmacdonald!
Closing!

@rpenido rpenido closed this Jan 9, 2024
@rpenido rpenido deleted the rpenido/add-course-authoring-mfe-base-url branch March 1, 2024 12:43
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.

3 participants