feat: add PoC permission and role #209
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @dwong2708! 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. |
mphilbrick211
added
the
mao-onboarding
| # Course Policies | ||
|
|
||
| # Course Staff Permissions | ||
| p, role^course_staff, act^courses.manage_advanced_settings, course^*, allow |
There was a problem hiding this comment.
I feel like I'm still getting my bearings in this repository, but course is introduced as a scope namespace here. I believe we will need to create a custom Scope model for this namespace, like what was done with ContentLibraryScope. The other option would be to use the global scope *, but I'm not sure what the implications would be there.
Also, if we want admins and superusers to be authorized without explicitly being given the course_staff role, we would want to look at changing this conditional in is_admin_or_superuser_check.
There was a problem hiding this comment.
You are right, the Scope class is needed for the internal logic.
Also good catch on the is_admin_or_superuser_check
rodmgwgu
left a comment
rodmgwgu
left a comment
There was a problem hiding this comment.
Adding some initial comments, I'm still investigating the error you are seeing.
openedx_authz/tests/stubs/models.py
Outdated
|
|
||
| class CourseOverview(models.Model): | ||
| """ | ||
| Model for storing and caching basic information about a course. |
There was a problem hiding this comment.
Please speciffy that this is a model stub
openedx_authz/tests/stubs/models.py
Outdated
| start_date = models.DateTimeField(null=True) | ||
| end_date = models.DateTimeField(null=True) | ||
|
|
||
| advertised_start = models.TextField(null=True) |
There was a problem hiding this comment.
We don't need to replicate the whole model, only the fields relevant for testing
openedx_authz/tests/stubs/models.py
Outdated
| """ | ||
|
|
||
| class Meta: | ||
| app_label = "course_overviews" |
There was a problem hiding this comment.
Removing this solves the "'stubs.CourseOverview', which is either not installed, or is abstract." error. But there are other errors with tests after that one is gone.
There was a problem hiding this comment.
The other errors relate to the table not existing, I think you need to recreate the migraitons
5 participants
Resolves: #203
As part of implementing PoC permissions and roles for the advanced settings section, this PR is intended to introduce only the definitions of the mentioned roles and permissions.
Merge checklist:
Check off if complete or not applicable: