-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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: return navigation disabled as sequence metadata #34049
feat: return navigation disabled as sequence metadata #34049
Conversation
Thanks for the pull request, @mariajgrimaldi! 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:
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. |
f4e7726
to
bb72b35
Compare
I tested the PRs and they work as expected. |
c671fbc
to
0bb1c72
Compare
0bb1c72
to
39ef6ce
Compare
Return navigation disabled as sequence metadata when Hide From TOC is enabled, so the student cannot navigate to another sequences in the course outline. https://openedx.atlassian.net/wiki/spaces/OEPM/pages/3853975595/Feature+Enhancement+Proposal+Hide+Sections+from+course+outline
39ef6ce
to
2f29486
Compare
Adolfo helped with testing this alongside the MFE: openedx/frontend-app-learning#1273 (comment) |
Hi folks @MaferMazu @felipemontoya, could you help me with a review here? You can see this working in a sandbox here: openedx/frontend-app-learning#1273 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this on local and it works as expected, for subsections that have never configured the hide from toc the API returns the default (false). The code is clean and tests are passing.
This looks good to me and good to be merged.
@mariajgrimaldi 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
2U Release Notice: This PR has been deployed to the edX production environment. |
Description
This PR returns a
navigation_desabled
field as sequence metadata to disable navigation in the Learning MFE within sequences when Hide From TOC is enabled for the current sequence. These changes are part of a series of PR(s) that implement the Feature Enhancement Proposal: Hide Sections from course outline: https://github.com/openedx/edx-platform/pulls?q=is%3Apr+is%3Aopen+hide+from+tocSupporting information
https://openedx.atlassian.net/wiki/spaces/OEPM/pages/3853975595/Feature+Enhancement+Proposal+Hide+Sections+from+course+outline
Testing instructions
For the Learning MFE changes, please follow the instructions listed here. Then, go to the LMS using the link to your hidden subsection, it should look something like this:
Without prev, next and sequence breadcrumbs. You can also test the metadata API call individually by loading a course subsection in the LMS, and search for this call in the dev console:
GET http://local.edly.io:8000/api/courseware/sequence/
You'll find:
When hide from TOC enabled for the subsection, and otherwise:
Deadline
After PR #33952
Other information
As I mentioned, this PR is part of a series of PRs implementing the feature enhancement of Hide From TOC. This initiative is an open-source contribution to the Open edX platform funded by a Unidigital project from the Spanish Government - 2023.