Skip to content
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

C2: Deleting users in 2 steps #5097

Closed
ywarnier opened this issue Jan 18, 2024 · 2 comments
Closed

C2: Deleting users in 2 steps #5097

ywarnier opened this issue Jan 18, 2024 · 2 comments

Comments

@ywarnier
Copy link
Member

ywarnier commented Jan 18, 2024

It happens more than one would hope that admins erroneously delete users. This is an irreversible process (you then have to restore a database backup of previous days, deleting all that happened on the portal since then). A better way to reduce this kind of issues would be to have a buffer, a "trash can" where users can still be recovered for a while.

This, however, means that we need a status for a "deleted" user. Currently, we use the user.active field to determine whether a user is active or not. This field is defined in the Entity as Boolean, which will have to be changed to int so we can use "-2" as a value for "deleted". The User::isActive() method will return an int (-2 for deleted, -1 for expired, 0 for inactive and 1 for active).

This action is applied across multi URLs. If the user is "deleted" from the system (which normally is not an action taken at the URL level unless the user was created there), the user.active field is set to -1 and disappears from all lists (except the "trashcan" page users list), so no change is needed at the resource_link level.

To do:

  • update entity and migration to set "active" as int instead of bool
  • update entity so isActive() returns true on value 1, and false otherwise
  • update entity setActive() and getActive() to support int param/return value instead of bool
  • update all code correspondingly (make sure we use isActive() instead of getActive())
  • in UserRepository, update deleteUser() to take a second parameter bool $destroy = false that only really deletes the user when the param is true, otherwise it just does $user->setActive(-2);
  • in all users lists, make sure the user does not appear if he's been deleted (where active != -2)
  • create a new tab in admin users list to see the deleted users (only select those users where active = -2), where there are only 3 options: edit, restore and delete (which really deletes the user then, calling $userRepository->deleteUser($user, true)).

This issue is related to #5212.

@ywarnier
Copy link
Member Author

ywarnier commented Mar 7, 2024

I've updated the description above because I had missed the fact that -1 was already used for "automatically disabled" (or "expired account").

christianbeeznest added a commit that referenced this issue Mar 7, 2024
User: Add deleting users in 2 steps - refs #5097
@ywarnier
Copy link
Member Author

ywarnier commented Mar 7, 2024

Totally approved 👍

@ywarnier ywarnier closed this as completed Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants