-
Notifications
You must be signed in to change notification settings - Fork 157
Add roles and departments #582
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: add-permissions-based-on-department
Are you sure you want to change the base?
Add roles and departments #582
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds role management within departments, including CRUD views, templates, drag-and-drop assignment of users to roles, and supporting model, selectors, factories, and tests.
- Introduces Role model and Department–Sequence relationship.
- Adds views, URLs, templates, and JS for managing departments and roles and assigning users to roles.
- Provides test coverage for creating/updating departments and roles and assigning users.
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| back/users/selectors.py | Adds role selector and distinct filters supporting role assignment logic. |
| back/users/models.py | Introduces Role model and Department.sequences relation enabling role-feature associations. |
| back/users/migrations/0045_department_sequences_role.py | Migration creating Role model and Department.sequences field. |
| back/users/factories.py | Adds RoleFactory for test/data setup. |
| back/conftest.py | Registers RoleFactory for pytest fixtures. |
| back/admin/people/views.py | Removes legacy department views superseded by new department_views module. |
| back/admin/people/urls.py | Routes updated to use new department and role views including add/update endpoints. |
| back/admin/people/department_views.py | New views for listing, creating, updating departments/roles and assigning users. |
| back/admin/people/templates/departments.html | Replaces simple list with drag-and-drop UI and user list. |
| back/admin/people/templates/_departments_list.html | Partial rendering departments, roles, and users per role. |
| back/admin/people/templates/department_update.html | Department update form plus roles display. |
| back/admin/people/templates/role_create.html | Role creation form template. |
| back/admin/people/templates/role_update.html | Role update form template. |
| back/admin/people/tests/department_tests.py | Adds tests for department/role CRUD and user-role assignment. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 8 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
cscheng
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The general approach for this functionality is fine. I have a bunch of comments and suggestions at the detail/code level, but in my testing it looks like there are a few things missing (that may be are already planned in a future PR?):
- How is deletion of departments handled?
- How is deletion of roles handled?
- What if a manager accidentally drags and drops a user to the incorrect role? This action cannot be undone.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
is this PR ready to merge and release, together with #572 ? |
|
@compgeniuses not yet. These features are pretty much done, they are sponsored by a company and we are still in talks about the final touches. |
This will allow admins/managers to add users to their own departments in specific roles.