feat: add advanced settings view permission for course roles#272
feat: add advanced settings view permission for course roles#272bra-i-am wants to merge 4 commits into
Conversation
|
Thanks for the pull request, @bra-i-am! 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. 🔘 Update the status of your PRYour PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate. 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. |
MaferMazu
left a comment
There was a problem hiding this comment.
A quick question: Why did we need to add the " View advanced settings permission"? In the description, you mention consistency, but I would like to understand it more.
The PR looks good to me! Thanks @bra-i-am ✨
Note: Please bump the version (from 1.15 to 1.16) and add the record to the changelog.
8298cd5 to
0f8ce2a
Compare
|
@MaferMazu, @BryanttV, thank you so much for your help reviewing this PR!! ✨ Mafer, the reason to add the
Note CONTEXT This is also done to follow the same pattern already established in the repo for other sections — for example, BTW, I already bumped the version and modified the CHANGELOG. Please let me know if anything else is required. Thanks again! |
0f8ce2a to
61bc401
Compare
| ===== | ||
|
|
||
| * Add ``COURSES_VIEW_ADVANCED_SETTINGS`` permission and assign it to ``course_auditor``, ``course_editor``, ``course_staff``, and ``course_admin`` roles to enable read-only access to Advanced Settings. | ||
| * Assign existing ``COURSES_MANAGE_ADVANCED_SETTINGS`` permission to the ``course_editor`` role to grant full read/write access to Advanced Settings. |
There was a problem hiding this comment.
I removed the approval because I thought you hadn't added the manage_advanced_settings. Details: #288
Is there a place where we say the editor has advanced settings permission? If not, can we remove that permission for the editor role?
|
@bra-i-am thanks for the explanation. As you said, course_editor should be able to edit and view pages and resources. If something in "progress" needs advanced settings, it is okay. He can't access them because the course_editor doesn't have access to manage or view them, but he can edit the rest. Details #288 |
Description
Adds
COURSES_VIEW_ADVANCED_SETTINGSpermission to enable read-only access to advanced settings for thecourse_auditorrole. Also addsCOURSES_MANAGE_ADVANCED_SETTINGStocourse_editorrole for full access.This change supports the read-only access feature for auditors in the Pages and Resources section of frontend-app-authoring MFE, allowing users with
course_auditorrole to view (but not modify) advanced settings.Supporting information
Changes
COURSES_VIEW_ADVANCED_SETTINGSpermissionCOURSES_VIEW_ADVANCED_SETTINGStoCOURSE_AUDITOR_PERMISSIONSandCOURSE_EDITOR_PERMISSIONS; addedCOURSES_MANAGE_ADVANCED_SETTINGStoCOURSE_EDITOR_PERMISSIONSHow to test
course_auditorrole to a user for a specific coursecourse_editorrole to a userMerge checklist
Check off if complete or not applicable: