Adding permission validations from authz for Course Updates#2938
Adding permission validations from authz for Course Updates#2938jacobo-dominguez-wgu wants to merge 2 commits into
Conversation
|
Thanks for the pull request, @jacobo-dominguez-wgu! 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2938 +/- ##
==========================================
+ Coverage 95.55% 95.57% +0.02%
==========================================
Files 1393 1393
Lines 32992 33023 +31
Branches 7644 7659 +15
==========================================
+ Hits 31524 31561 +37
+ Misses 1413 1408 -5
+ Partials 55 54 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
88405df to
1df4ca1
Compare
|
@jacobo-dominguez-wgu Is this ready for review? |
Yes, I will address the conflicts and mark it as ready. |
dec3abf to
879f895
Compare
MaferMazu
left a comment
There was a problem hiding this comment.
@jacobo-dominguez-wgu thanks for this PR.
It works ✨
But I had a comment: when the user hasn't managed_course_updates, it doesn't show the options, but before that, it loads all the views.
I think it is easier if you see it:
Screencast.from.2026-05-14.12-52-25.webm
Also, I asked Gemini about major issues, and it told me the following:
- Hook Logic Error
File: src/authz/hooks.tsLines: ~113-120 (within useUserPermissionsWithAuthzCourse)
Feedback:
"If isAuthzEnabled is false, isLoadingUserPermissions might remain true (or undefined), blocking the else if block. This will trigger a false PermissionDenied for all users when the waffle flag is disabled. Please move the true fallback logic outside of the loading check if authorization is disabled."
- Missing Loading State (UX)
File: src/course-updates/CourseUpdates.tsxLine: ~66 (before the first if)
Feedback:
"The isLoading state returned by the hook is not being validated. Currently, while the permissions API is fetching, the variables will be undefined, causing the component to briefly render the PermissionDeniedAlert before the actual content appears (flicker). Please add a loading guard here."
Being honest, I don't know much about this, or if any of these issues may be causing the weird loading in the interface. So please, feel free to judge this feedback.
e251a87 to
16ed82c
Compare
There was a problem hiding this comment.
When the permissions validation is loading, the UI flashes the buttons, I can click them, and just when the permissions request finishes, the buttons are hidden:
Screencast.from.15-05-26.16.49.04.webm
I'd say it would be better if, while the permissions validation is loading, those buttons are hidden... or what do you opine?
16ed82c to
65a5642
Compare
bra-i-am
left a comment
There was a problem hiding this comment.
Perfect! Thanks for addressing my comments ✨
65a5642 to
8e44be1
Compare
Grabacion.de.pantalla.2026-05-15.a.la.s.4.27.58.p.m.movI have added a loading spinner to show while the permissions are loading so we do not display an incorrect UI. |
…rPermissions Remove the deprecated useUserPermissionsWithAuthzCourse hook and migrate all consumers to use useCourseUserPermissions, which provides a flatter API by spreading permission booleans directly into the return object.
8e44be1 to
40add9e
Compare
Description
This pr add the permission validations for the Course Updates section on the Content menu from the header.
First this validates the
enableAuthzCourseAuthoringwaffle flag and if it is enabled it checks the permissions from authz over the course updates section.These are the checks included:
courses.view_course_updatespermission to display or hide the Course Updates option on the header menu.courses.manage_course_updatesto display or hide the "+ New update" button and the edit and remove icon buttons for the available course updates and course handouts.Resolves #2932
Supporting information
Depends on backend ticket openedx/openedx-authz#191
Testing instructions
Prerequisites
enableAuthzCourseAuthoringwaffle flagTest Scenarios
1. Authz Disabled (Legacy Behavior)
Setup: Ensure
enableAuthzCourseAuthoringwaffle flag is disabled2. Authz Enabled — User Has Full Permissions
Setup:
enableAuthzCourseAuthoringwaffle flagcourses.view_course_updatesandcourses.manage_course_updatespermissions3. Authz Enabled — User Has View-Only Permissions
Setup:
enableAuthzCourseAuthoringwaffle flagcourses.view_course_updatespermission (nomanagepermission)4. Authz Enabled — User Has No Permissions
Setup:
enableAuthzCourseAuthoringwaffle flagcourses.view_course_updatesandcourses.manage_course_updatespermissionsBest Practices Checklist
We're trying to move away from some deprecated patterns in this codebase. Please
check if your PR meets these recommendations before asking for a review:
.ts,.tsx).propTypesanddefaultPropsin any new or modified code.src/testUtils.tsx(specificallyinitializeMocks)apiHooks.tsin this repo for examples.messages.tsfiles have adescriptionfor translators to use.../in import paths. To import from parent folders, use@src, e.g.import { initializeMocks } from '@src/testUtils';instead offrom '../../../../testUtils'