feat: management command to clean up roles for all users #103
Conversation
OmarIthawi
left a comment
OmarIthawi
left a comment
There was a problem hiding this comment.
Thanks @shadinaif! Looks great!
I've added few suggestions.
| if not user.is_active: | ||
| print('**** User is not active, skipping..') | ||
| continue |
There was a problem hiding this comment.
Inactive role should lose all of its roles to avoid data accidential data breaches.
| if not user.is_active: | ||
| print('**** User is not active, skipping..') | ||
| continue | ||
| if is_system_staff_user(user): |
There was a problem hiding this comment.
Is staff should lose all of its roles to avoid needless permissions appearing on the admin pages.
| invalid_orgs = CourseAccessRole.objects.filter( | ||
| user_id=user_id, | ||
| ).exclude( | ||
| org__in=all_orgs, |
There was a problem hiding this comment.
I think this is case-sensitive, which shouldn't be.
| type=str, | ||
| ) | ||
|
|
||
| def handle(self, *args: list, **options: Dict[str, str]) -> None: |
There was a problem hiding this comment.
Please move the def handle into a clean_roles util function, this would help testing and actual usage in the future by /admin panel actions.
shadinaif
force-pushed
the
shadinaif/roles-cleanup-command
branch
5 times, most recently
from
f8f8273 to
de295c9
Compare
shadinaif
force-pushed
the
shadinaif/roles-cleanup-command
branch
2 times, most recently
from
11d7780 to
4036c95
Compare
shadinaif
force-pushed
the
shadinaif/roles-cleanup-command
branch
from
4036c95 to
31d5e59
Compare
|
I'll get back to this later. Tehreem is working on adding |
feat: management command to clean up roles for all users
Dry run:
Commit:
The command will simply run on all users with roles and perform
update_course_access_roleswith the same roles returned byUserRolesSerializer. It'll also give details about records that cannot be automatically cleaned for any reason