fix: surface tab-data errors instead of silently swallowing them on access denial - #2054
fix: surface tab-data errors instead of silently swallowing them on access denial#2054asadali145 wants to merge 1 commit into
Conversation
…ccess denial
Previously api.js silently swallowed HTTP 403 on the dates/progress/outline
tab-data endpoints, returning {} on the assumption that course access is
always handled via the metadata request's redirect. That assumption breaks
when the tab-data endpoint itself returns 403 for a different reason (e.g. an
embargo block), leaving learners on a blank tab with no explanation.
403 now re-throws like any other error. TabPage's deriveView() derives
isDenied/isError as independent flags so a denied learner with a real
redirect target still redirects even if tab-data also failed, while a denied
learner with no redirect target (outline) now surfaces an error instead of
silently rendering with no data.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Thanks for the pull request, @asadali145! 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. |
openedx-webhooks
added
the
open-source-contribution
|
Closing in favor of #2055, which takes a different approach - scoping the country-embargo check down to the |
2 participants
What are the relevant tickets?
N/A
Description (What does it do?)
Course tab pages (dates, progress, outline) silently swallowed HTTP 403 responses from their tab-data endpoints, returning an empty object instead of surfacing an error. That assumed 403 always meant "course access denied, handled by the metadata request's redirect" — but a 403 can also come from something like an embargo block on the tab-data endpoint itself, unrelated to the metadata call. Learners hit that case saw a blank/broken tab with no explanation.
api.js: 403 now re-throws like any other error ongetDatesTabData,getProgressTabData,getOutlineTabData.TabPage.tsx:deriveView()now derivesisDeniedandisErroras independent flags instead of mutually-exclusive branches, so:Implementation details
getDatesTabData/getProgressTabDatastill swallow HTTP 401 the same way 403 used to be swallowed - left alone in this PR since removing it changes user-visible behavior beyond this fix's scope. Flagging it here as a known follow-up, not fixed now.How can this be tested?
npx jest src/tab-page/TabPage.test.jsx src/course-home/data/api.test.js src/course-home/data/apiHooks.test.tsx- 55/55 passing, including new cases covering: 403 re-thrown at the API layer, the hook surfacingisErrorfor dates/progress/outline, andTabPagerendering the right outcome across the redirect-target x tab-data-outcome matrix.Additional Context
No UI/visual change - same error page, just reachable from a case it previously silently swallowed.