feat: add view permissions for advanced settings, certificates, group configurations, and library updates - #385
Conversation
|
Thanks for the pull request, @wgu-taylor-payne! 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. |
019cf47 to
5e77342
Compare
|
Hi @wgu-taylor-payne, thanks for this! Could we also include this new permission in the PR? #386 |
… configurations, and library updates Add four new read-only view permissions: - courses.view_advanced_settings - courses.view_certificates - courses.view_group_configurations - courses.view_library_updates All four are granted to all four course roles (Admin, Staff, Editor, Auditor). Previously, users without the corresponding manage_* permissions were completely blocked from viewing these pages. Closes openedx#328, Closes openedx#329, Closes openedx#386, Closes openedx#387
5e77342 to
2f899c8
Compare
@BryanttV I've added that permission. Thanks. |
BryanttV
left a comment
There was a problem hiding this comment.
I tested this on my local, and it works as expected! All 4 course roles have the 4 new permissions. Thanks!
Description
Add four new read-only view permissions for the Advanced Settings, Certificates, Group Configurations, and Library Updates pages:
courses.view_advanced_settings— allows viewing advanced settings without edit accesscourses.view_certificates— allows viewing certificates without edit accesscourses.view_group_configurations— allows viewing group configurations without edit accesscourses.view_library_updates— allows viewing library updates without edit accessAll four permissions are granted to all four course roles (Admin, Staff, Editor, Auditor). Previously, users without the corresponding
manage_*permissions were completely blocked from even viewing these pages. The intended behavior (per #283) is read-only access for all course roles, with only Admin/Staff retaining write access (Editor also retainsmanage_group_configurationsandmanage_library_updates).Changes
constants/permissions.py— AddedCOURSES_VIEW_ADVANCED_SETTINGS,COURSES_VIEW_CERTIFICATES,COURSES_VIEW_GROUP_CONFIGURATIONS, andCOURSES_VIEW_LIBRARY_UPDATESpermission constantsconstants/roles.py— Added all four view permissions to all four course role permission listsengine/config/authz.policy— Added 16 Casbin policy rules (4 permissions × 4 roles)docs/concepts/core_roles_and_permissions/course_roles.rst— Updated permission descriptions and matrix table1.22.0→1.23.0(minor: new backwards-compatible functionality)Manual testing
course_editororcourse_auditorrole on a courseis_user_allowed(user, 'courses.view_advanced_settings', course_scope)returnsTrueis_user_allowed(user, 'courses.manage_advanced_settings', course_scope)returnsFalseview_certificates/manage_certificatesview_group_configurations: auditor gets view=True, manage=False; editor gets both=Trueview_library_updates: auditor gets view=True, manage=False; editor gets both=Truecourse_staffandcourse_adminroles (should have all permissions)Merge checklist:
Notes: This is purely additive — no migrations needed, no breaking changes. Users who previously had
manage_*implicitly also getview_*since their roles receive both. Downstream enforcement changes in openedx-platform depend on this PR being merged and released first.Related to #328, #329, #386, and #387.
AI Usage
Kiro was used in implementing this PR. I guided the process by defining the scope, confirming the approach (single PR, view/manage split pattern), and reviewing each step — Kiro researched the codebase, planned the implementation, generated the permission constants/roles/policies/docs, and ran the full test suite iteratively until all 1301 tests passed. I reviewed the generated code and tests, trimmed redundant test cases after discussing what was already covered by other test layers, and verified the changes end-to-end in a local Tutor environment by assigning users to all four course roles and confirming the correct allow/deny behavior.