rubyforgood / human-essentials

Human Essentials is an inventory management system for diaper, incontinence, and period-supply banks. It supports them in distributing to partners, tracking inventory, and reporting stats and analytics.
https://humanessentials.app
MIT License
436 stars 450 forks source link

replace org_user deactivation with removal of org_user role #4477

Open elasticspoon opened 5 days ago

elasticspoon commented 5 days ago

Resolves #3587

Description

Problem

The issue was that banks could "deactivate" a user (which is essentially a soft delete of the whole User record). However, if that user then went to a Partner they would not be able to invite that user because that user would exist (new User cannot be created) but be soft deleted (existing User is treated like they didn't have permissions).

Solution

Roles got added in https://github.com/rubyforgood/human-essentials/pull/3117. Therefore, the logic around deactivation should address the role not the user.

However, there is a caveat. I attempted to implement logic around deactivating roles themselves in https://github.com/rubyforgood/human-essentials/pull/4392, https://github.com/rubyforgood/human-essentials/pull/4382. But the logic became very complex. We decided (https://github.com/rubyforgood/human-essentials/issues/3587#issuecomment-2138464940) to go for a more architerurally simple solution of removing the deactivation functionality and just deleting the roles.

Drawbacks

There are two main drawbacks with this approach.

Existing Data

What do we do with existing discarded users? Discarded == soft deleted. I can imagine a world where a user was discarded without that meaning to be a "deactivation".

My current idea is that any User that is discarded and only had the org_user role was probably a deactivation and we can reactivate them and remove the org_user role from them. But this requires some more information.

Loss of Granularity

Previously we had 3 states for org_users: active, not an org_user and deactivated (meaning they were a user at one point but not any more).

Now there are only 2 states: user or not. This means we have a bit less information, we can no longer track if someone was once a user but not anymore. Does this matter? Probably not.

Type of change

How Has This Been Tested?

Request + System specs

Screenshots

image

cielf commented 1 day ago

I hope it's going to be a pretty rare case that someone is going to try to sign in after their role has been deleted, so I'm ok with leaving the slightly confusing error message. Not the best place to be using our resources if it's too hairy.