fix: avoid anonymous completion lookup in navigation#38024
Conversation
|
Thanks for the pull request, @meet0208! 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. DetailsWhere 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. |
|
Hi @meet0208! Thank you for this contribution! It looks like you're contributing on behalf of an organization. If so, could you please have your manager reach out to oscm@axim.org to have you added to our existing entity agreement? That way, we can get your CLA check to turn green. |
|
@meet0208 you've been added to the entity CLA and that should be reflected in GitHub this evening. |
|
@e0d or @mphilbrick211 Hey can you approve a re run for this PR. |
|
Hi @meet0208, I can't reproduce the bug:
I tested it with the current |
|
@meet0208 Thank you for your contribution. I was able to reproduce this issue in master. @ChrisChV Here are the steps:
curl -i "http://localhost:18000/api/course_home/v1/navigation/course-v1:edX+DemoX+Demo_Course"
@meet0208 Could you please add a test in |
|
@navinkarkera Hey, I added a test case can you review and let me know. |
|
@meet0208 Thanks! Can you please rebase with master. |
1979db2 to
ebbb97c
Compare
|
@navinkarkera I did can you check now. |
|
@navinkarkera Why the tests are failing to install "No matching distribution found for django-autocomplete-light==4.0.1" ? Even I rebased my branch with master. |
… outline.
Return empty completions_dict for AnonymousUser in CourseNavigationBlocksView to prevent 500 on /api/course_home/v1/navigation/{course_key} for public anonymous access. Keeps existing outline access filtering unchanged.
…ted public courses
ebbb97c to
3b135ce
Compare
|
@navinkarkera #38779 this is merged can you run pipeline now. |
|
@navinkarkera One test case failed as my test was mocked BlockCompletion.objects.filter around the full API request. In other words, the test was exercising more than the specific guard. So, can you rerun the CI now. |
|
@navinkarkera All the test cases are passed. |
navinkarkera
left a comment
There was a problem hiding this comment.
@meet0208 Thanks for you work!
Description
This PR fixes course outline loading for anonymous users in the Learning MFE by preventing a server error in the course navigation API.
Previously, anonymous requests to
GET /api/course_home/v1/navigation/{course_key}could raise a500when completion data was queried withAnonymousUser. This caused the course outline sidebar to fail to render for logged-out users, even when course visibility and waffle settings allowed public access.This change adds an anonymous-user guard in
CourseNavigationBlocksView.completions_dict:request.useris anonymous, return an empty completion map ({}).filter_inaccessible_blocks) unchanged to preserve access control.Implications
500.Supporting information
Relevant code / endpoint
lms/djangoapps/course_home_api/outline/views.pyGET /api/course_home/v1/navigation/{course_key}AnonymousUserConfiguration context (unchanged)
seo.enable_anonymous_courseware_accessenabledpublicorpublic_outlineTesting instructions
Ensure the course is configured with:
public(orpublic_outline)seo.enable_anonymous_courseware_accessenabledOpen the courseware URL in an incognito / logged-out browser session.
Confirm
GET /api/course_home/v1/navigation/{course_key}returns200(not500).Verify the outline sidebar renders in the Learning MFE.
Log in as a learner and as staff/admin and confirm outline behavior is unchanged.
Optional regression checks:
Deadline
None.
Other information
User roles impacted
UI changes
UI impact: course outline now renders for anonymous users on eligible
public/public_outlinecourses.Suggested screenshots to attach:
500200/api/course_home/v1/navigation/{course_key}(before vs after)Configuration changes
Dependencies / special concerns