-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat: add completion tracking switch to toggles sidebar api #36795
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 completion tracking switch to toggles sidebar api #36795
Conversation
|
Thanks for the pull request, @holaontiveros! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
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.
Pull Request Overview
This PR adds a completion tracking switch in the toggles API to enable the frontend to correctly display completion tracking based on a feature flag. Key changes include:
- Importing and adding ENABLE_COMPLETION_TRACKING_SWITCH in the sidebar toggles API in views.py.
- Updating tests in test_views.py to verify API responses for both enabled and disabled states of the new switch.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lms/djangoapps/courseware/views/views.py | Updated the toggles API to include the completion tracking status flag. |
| lms/djangoapps/courseware/tests/test_views.py | Added tests to validate the new completion tracking flag under different toggle configurations. |
Comments suppressed due to low confidence (1)
lms/djangoapps/courseware/tests/test_views.py:3800
- [nitpick] Consider renaming the test function to use 'completion_tracking' instead of 'completion_track' for consistency with the API field name 'enable_completion_tracking'.
def test_courseware_mfe_navigation_sidebar_enabled_aux_disabled_completion_track_disabled(self):
| return JsonResponse({ | ||
| "enable_navigation_sidebar": COURSEWARE_MICROFRONTEND_ENABLE_NAVIGATION_SIDEBAR.is_enabled(course_key), | ||
| "always_open_auxiliary_sidebar": COURSEWARE_MICROFRONTEND_ALWAYS_OPEN_AUXILIARY_SIDEBAR.is_enabled(course_key), | ||
| # Add completion tracking status for the sidebar use while a global place for switches is put in place |
Copilot
AI
May 27, 2025
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.
Consider clarifying the comment to better explain that the new completion tracking flag is a temporary solution pending a global switch management implementation.
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.
shush copilot
|
Your branch is behind the base. I've pulled in changes from master as a merge commit which will update your branch and cause the tests to be re-run. |
Thank you, and I just did again, it gets out of sync really quick |
kdmccormick
left a 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.
Thank you!
|
Thanks for the commit and helpful screenshots @holaontiveros ! One small thing -- the links you've provided (in Supporting Info and Other Info) are incorrect. Is it possible you're trying to link to issues/PRs in another repository? |
Yes I was! because I copy an pasted from my other PR, sorry for that |
|
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. |
|
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. |
|
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. |
Currently when the ENABLE_COMPLETION_TRACKING_SWITCH waffle switch is disabled the frontend doesn't take it into account and still displays completion for anything that was already completed. This PR adds the ENABLE_COMPLETION_TRACKING_SWITCH to sidebar toggles api so the frontend can leverage it.
Description
Currently when the ENABLE_COMPLETION_TRACKING_SWITCH waffle switch is disabled the frontend doesn't take it into account and still displays completion for anything that was already completed.
This PR adds the ENABLE_COMPLETION_TRACKING_SWITCH to sidebar toggles api so the frontend can leverage it.
Screenshots
before:

after:

(right side of the screen to see network response for the API that changed)
flag disabled

flag enabled

Supporting information
fixes #1692
Testing instructions
Either use the API:
GET http://local.openedx.io:8000/courses/course-v1:demo+D01+2025_T1/courseware-navigation-sidebar/toggles/
or access a course on the MFE and look for the toggles API request
Deadline
None
Other information
Include anything else that will help reviewers and consumers understand the change.
relates to The forcing of IDs to non-draft must happen in base.py instead of draft.... #1714
No