Add a way to disable a user

[astashov]

Ticket: https://github.com/CrisisCleanup/crisiscleanup/issues/387

• Added is_disabled boolean field to users table, false by default.
• Added a default_scope, which is is_active: false to User model. It's usually an antipattern, but in this case since we'll have to use it like in 95% of cases, it should be okay.
• Excluded that default scope in Admin::UsersController's actions
• Added the column is_disabled to the User's databable in datatable.js field, and also the checkbox is_disabled to the edit user form.

Testing:

• I've clicked through the site, found those places where we have users list:

• /admin/users

• /worker/my-organization

• /worker/incident/:iid/organizations/:oid

I think that's it. Everywhere there the disabled user was not listed Also

• Made sure after disabling you can't log in anymore.
• You still can send an invitation to signup to the same email, that was disabled, but if you try to signup, it fails silently. Would be nice to improve error messaging there, but for now it's out of scope of this PR.
• You still can see an invitation in /worker/my-organization, even if user was disabled. I guess that's fine, what do you think?

Other that that, it seems to be working, unless I missed something. :)

[seanfisher]

@astashov Is this something you could add a capybara UI test for? In the spec/features folder

[astashov]

@seanfisher Any ideas what to test? Never done the UI tests before.

[astashov]

I haven't added the index on is_disabled - there's not gonna be enough selectivity I think to get any benefit from the index. Is that okay?

[seanfisher]

You'd just write a UI test doing what you've mentioned in your "testing" phase - essentially, emulate clicking through the application and assert that the disabled user isn't there. You can look at some of the other tests for examples

[seanfisher]

As for the index, I would index it. It's going to be part of the WHERE clause every time the User model is used except in the admin portal, where you've deliberately unscoped it. An index makes sense here.

[astashov]

Okay, I've added index to is_deleted. But I still think it doesn't make sense there, due to its low cardinality and selectivity (e.g. here it explains why it could hurt performance: https://www.ibm.com/developerworks/data/library/techarticle/dm-1309cardinal).

[astashov]

I've tried to add Capybara tests for this feature, but unfortunately couldn't make it work - it doesn't load <datatable>s there, by some reason, after page load. Couldn't quickly figure out the issue, so decided to not spend too much time on that, sorry. So I'll pass on Capybara tests.

[aarontitus]

Important: As far as I can see, the ability to delete/deactivate a user is reserved for /admin. This should NOT be the case. While the admin should have this ability, EVERY user must have this ability for EVERY other user in his/her organization. Crisis Cleanup is a flat and decentralized model, and there is no central admin with the capacity or ability to responsibly deactivate users that belong (not to Crisis Cleanup), but one of the participating, partner organizations. This may seem counterintuitive, but it is vital. Crisis Cleanup must deal with a few realities that make this arrangement imperative:

• Crisis Cleanup does not have any volunteers (or at most a few volunteers, most of whom are reading this post). Only the participating organizations have volunteers. We do not have the staff or moral authority to manage other organizations' user base. And I don't want people calling me every time someone leaves their organization, especially once we reach tens of thousands of users.
• Disaster recovery must be flexible. It has high turnover, and the same representatives from an organization rarely interact with Crisis Cleanup in two successive disasters. Roles change hourly. Consequently, Crisis Cleanup has a very flat roles/permissions model by design. It needs to stay that way at almost any cost.

Once we have the ability to deactivate a user, that raises new error handling issues if a deactivated user is re-invited. Ideally, the deactivated user would not be invited, but the inviter and person who deactivated the target user would receive an alert saying, "[Inviter Name] would like to invite [Target User Name], but [Deactivating User Name] deleted this user on [date]. Please coordinate with each other:

• [Inviter User Name]: [Inviter User Email], [Inviter User Phone]
• [Deactivating User Name]: [Deactivating User Email], [Deactivating User Phone]

If you come to the conclusion that [Target User Name] should be readmitted, please follow these instructions: [Instructions for undeleting]."

Please make sure we capture who deactivated a user, so that we have a way to reach out to that user should someone want to reactivate a deactivated user.