feat: new permissions added to the roles and permissions matrix - #215
Conversation
|
Thanks for the pull request, @jesusbalderramawgu! 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. |
|
@jesusbalderramawgu thank you for this, please make sure the linter passes |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #215 +/- ##
=======================================
Coverage 97.64% 97.65%
=======================================
Files 68 68
Lines 1702 1665 -37
Branches 447 408 -39
=======================================
- Hits 1662 1626 -36
+ Misses 40 39 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
you are right, fixed. Thanks @dcoa! |
dcoa
left a comment
There was a problem hiding this comment.
In general, looks good. I suggested some improvements in description
| { | ||
| key: CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_ADVANCED_SETTINGS, | ||
| resource: 'course_advanced_certificates', | ||
| description: 'Access and view the Advanced Settings page in Studio. This covers a wide range of technical course configurations, including proctoring, timed exams, LTI tools, enrollment limits, and custom display options.', |
There was a problem hiding this comment.
| description: 'Access and view the Advanced Settings page in Studio. This covers a wide range of technical course configurations, including proctoring, timed exams, LTI tools, enrollment limits, and custom display options.', | |
| description: 'Access the Advanced Settings page in Studio. This covers a wide range of technical course configurations, including proctoring, timed exams, LTI tools, enrollment limits, and custom display options.', |
Then in edit description we can use a simplified version to avoid being repetitive
description: 'Edit technical course configurations in the Advanced Settings page in Studio.',What do you think?
There was a problem hiding this comment.
Agree, I have updated the texts. Thank you!
| { | ||
| key: CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_CERTIFICATES, | ||
| resource: 'course_advanced_certificates', | ||
| description: 'See the course certificate settings, including certificate design and eligibility criteria.', |
There was a problem hiding this comment.
Following the same reasoning:
| description: 'See the course certificate settings, including certificate design and eligibility criteria.', | |
| description: 'See the course certificate settings.', |
dcoa
left a comment
There was a problem hiding this comment.
LGTM, just a small change left
| CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_SCHEDULE_AND_DETAILS, | ||
| CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_ADVANCED_SETTINGS, | ||
| CONTENT_COURSE_PERMISSIONS.VIEW_COURSE_CERTIFICATES, | ||
|
|
There was a problem hiding this comment.
Can we remove this extra space, please?
Description
New permissions added to the roles and permissions matrix
this PR closes these four issues :
openedx/openedx-authz#393
openedx/openedx-authz#396
openedx/openedx-authz#399
openedx/openedx-authz#402
Screenshots with the new permissions