Implement user deletion functionality - #2811
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2811 +/- ##
==========================================
- Coverage 75.32% 75.31% -0.01%
==========================================
Files 525 529 +4
Lines 20143 20164 +21
Branches 1564 1490 -74
==========================================
+ Hits 15172 15187 +15
- Misses 4965 4971 +6
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
jareth-whitney
left a comment
jareth-whitney
left a comment
There was a problem hiding this comment.
Everything looks good, I'm just wondering if we need to handle cascading with other tables when a user is deleted. Perhaps the deactivation filter is enough to get around the cascading, just something to consider. Approved!
| status_code=HTTPStatus.BAD_REQUEST, | ||
| ) | ||
|
|
||
| if actor_external_id and db_user.external_id.lower() == actor_external_id.lower(): |
There was a problem hiding this comment.
Perfect, I was wondering it this would be here. We should definitely stop people from deleting theirselves.
| if db_user is None: | ||
| raise BusinessException(error='User not found.', status_code=HTTPStatus.NOT_FOUND) | ||
|
|
||
| if db_user.status_id != UserStatus.INACTIVE.value: |
There was a problem hiding this comment.
What do we have to do in order to deactivate a user?
| if (savedUser.status_id !== USER_STATUS.INACTIVE.value) | ||
| return dispatch(openNotification({ severity: 'error', text: 'User must be deactivated before deletion.' })); | ||
| if (isSelf) | ||
| return dispatch(openNotification({ severity: 'error', text: 'You cannot delete your own account.' })); |
There was a problem hiding this comment.
Thanks for adding the front-end redundancies, I think it's important to have these checks front and back.
| onClick={handleDeleteUser} | ||
| fullWidth | ||
| loading={isDeletingUser} | ||
| disabled={!canDeleteUser} |
There was a problem hiding this comment.
So the delete user item will appear if you're a super admin, but there are some scenarios where it will still be disabled (like if it's yourself, etc). Makes sense.
| - Added a "Delete User" button to the user details page in the admin view, visible only to super admins. | ||
| - Updated the API to include a DELETE endpoint for users, which checks for appropriate permissions and handles cascading deletions of related data (e.g. role assignments). | ||
| - Added success and error messages to provide feedback on the deletion process. | ||
| - Reworked the user details component to rearrange the existing layout and accommodate the new delete button, while ensuring a clear and intuitive user experience. |
There was a problem hiding this comment.
Should there be any cascades for other tables when a user is deleted? Or does the de-activation filter deal with that? For instance, what happens if someone is a project lead, then is deleted?
|
3 participants
Issue #: 🎟️ DEP-239
Description of changes:
User Guide update ticket (if applicable):