feat: add courseaccessrole events #309
Conversation
Zacharis278
force-pushed
the
zhancock/course-access-role-events
branch
from
91059b7
to
603f0d2
Compare
| @@ -331,6 +331,24 @@ class ExamAttemptData: | |||
| requesting_user = attr.ib(type=UserData, default=None) | |||
|
|
|||
|
|
|||
| @attr.s(frozen=True) | |||
| class CourseAccessRoleData: | |||
There was a problem hiding this comment.
Reading the ManageStudentsPermissionData class defined below makes me wonder if we should refrain from using the word "role" in this class name. I know you've talked to more people about the roles + permissions work though, so I trust your judgement!
There was a problem hiding this comment.
thanks. Yup the conclusion we came to was to define this as edx-platform exists today rather than trying to guess the future. I kept the name specific to the LMS model so it's use is clear instead of a ambiguous half-measure that might be reusable in the future.
There's no longer a near term path to the permission changes getting merged as we had originally wanted to account for. I'm not sure manage students will even end up being the correct permission when that the time does come to use permissions over roles.
There was a problem hiding this comment.
Got it, appreciate the explanation! This looks good then 👍
|
thanks @felipemontoya. I don't have write access to merge this. Is that something you can do or do I need to ping a specific maintainer? |
Description: Creates new data class and events for publishing information about the creation or removal of an LMS CourseAccessRole. We are not planning to account for the roles and permissions changes at this point, events would be based on how roles currently work.
This will be consumed by edx-exams to sync access to instructor tools and actions on the API.
Author concerns: These events will be used instead those added by #267. I plan to remove those in a separate PR since there is no plan to use those event definitions or supporting data classes.
Merge checklist:
Post merge:
finished.