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: support unit preview in learning MFE #35747

Merged
merged 17 commits into from
Nov 1, 2024

Conversation

KristinAoki
Copy link
Member

Description

This PR re-implements #35687 and fixes the bug where units were erroring when the user. Additionally, this PR add support to fetch the draft branch for the sequence metadata API.

Supporting information

See PR #35687 for more information

Testing instructions

  1. Open a course in Studio
  2. Publish an existing unit
  3. Make a change to the unit so that it is showing a draft version in Studio
  4. Click "View live"
  5. Confirm that the published version of unit is visible
  6. Confirm that preview is not in the URL
  7. Go back to the Studio page
  8. Click "Preview"
  9. Confirm that the draft version of the unit is visible
  10. Confirm that "preview" is at the beginning of the URL

Repeat steps above when masquerading as a verified learner and audit learner. Confirm that the preview is displayed as expected for each track.

Deadline

None

Copy link
Contributor

@nsprenkle nsprenkle left a comment

Choose a reason for hiding this comment

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

Visual review looks good, haven't gotten to test locally though.

cms/djangoapps/contentstore/utils.py Show resolved Hide resolved
xmodule/video_block/video_block.py Outdated Show resolved Hide resolved
@KristinAoki KristinAoki merged commit e13d66d into master Nov 1, 2024
49 checks passed
@KristinAoki KristinAoki deleted the KristinAoki/fix-unit-iframe-error-in-preview branch November 1, 2024 15:03
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Comment on lines +196 to +200
url_parts = list(urlparse(settings.LEARNING_MICROFRONTEND_URL))
url_parts[2] = mfe_link
url_parts[4] = query_string

return urlunparse(url_parts)
Copy link
Member

Choose a reason for hiding this comment

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

@KristinAoki This code assumes that the Learning MFE is hosted at the root of a domain, which is true for edx.org but not true for anyone who deploys Tutor. It breaks all jump_to links in Tutor, by sending them to:

  • /course/{course_key}/... and
  • /preview/course/{course_key}/...

instead of:

  • /learning/course/{course_key}/... and
  • /learning/preview/course/{course_key}/...

Could you modify this so that it works regardless of which path the Learning MFE is based on?

Copy link
Member Author

@KristinAoki KristinAoki Nov 7, 2024

Choose a reason for hiding this comment

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

I am assuming that the issue is the url builder , so I will revert back to the string builder for the returned path.

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.

Preview site not in MFE
4 participants