Deactivated user behavior

[orenyk]

What happens when you deactivate a user and they try to create a new account (e.g. is uniqueness on netID enforced)?

Write tests that cover all situations including deactivate / reactivate, deactivate / create new, etc. All of these tests will define the behavior of how we manage the deletion (if we even need it) and will be helpful.

OR

We can remove this feature since it's unclear when you should be deleting user accounts since their netID doesn't change and we can just ban them if necessary.

[orenyk]

Also, see what happens when you soft-delete a user - are all of their reservations deleted? Reservations do not have soft-deletion, so it may not propagate.

The fact that we haven't encountered this before suggests that this feature is not often used, so option B above is probably fine (but I'm not 100% sure)

[dgoerger]

cross-reference #557 and #526. An admin may wish to remove a user if it was falsely created (e.g. entered the wrong NetID, account belongs to no-one), but that's a step further than deactivation and I don't think it's the worst if we don't support that in the views.

I feel like the behavior shouldn't be to delete reservations when deactivating a user---what happens when we deactivate the equipment model, or object? We should definitely not be deleting records automatically like that, in my opinion.

[orenyk]

Agreed. I'm just not sure that we need soft deletion of users; if a user is created erroneously, we can simply delete them (using ActiveAdmin, for example). If we create a user account with errors but we want to keep them for record-keeping purposes, we can just ban them. This is true in any other case where deactivation would be used.

In terms of associated reservations, I agree that we shouldn't delete associated reservations; this issue was created largely because we weren't sure what the default was. If we fix the "Deleted User" issue from #526 such that a missing user doesn't break the show reservation view, then user deletion should have no consequences from a reservation perspective.

[squidgetx]

Fun fact: if you deactivate your own account... nothing happens.

Edit: yeah, it looks like deactivation just doesn't work at all. I can sign out and sign into CAS and still use the app as normal, even after downgrading my account to patron

[squidgetx]

I think we should support deactivation of users for cases like #557. Deactivated users should functionally behave like banned users: users can log in and look at the catalog if they want to but otherwise cannot do anything. They won't be able to make a new account since their account still exists in the db, and this way reservation history will be preserved as well. (We shouldn't be deleting associated reservations, which is what currently happens)

[orenyk]

Is there a reason why we wouldn't just ban them then?

[squidgetx]

I think the only reason is semantics; that a staff member would treat a banned user differently from a deactivated one. But the difference is slight, it may just be better to remove deactivation if we decide that's not a strong enough difference

[orenyk]

Hmmm, I see your point, but I'm not sure it's worth the complexity. Also, once people graduate, I feel like the number of cases where they try to use reservations / interact with the staff is so low that it's basically irrelevant.

[squidgetx]

That's fine, it'll be cleaner to remove soft-deletion of users anyway

[squidgetx]

Should we change all the instances of deactivate and reactivate buttons in the views to ban or hard-delete?

edit: almost definitely ban. users should not be able to be easily deleted (#526)

[orenyk]

Definitely ban, we shouldn't really be able to hard delete anything from the admin role in my mind.

[squidgetx]

Should we remove the destroy method from the controller as well, then?