Canceling an account doesn't work

[yashdavisgupta]

Expected action:
When canceling an account on /users/edit using the button account should be removed and client should be redirected to home page
Actual action:
User isn't removed and client is redirected to /users which isn't a real page, resulting in a 404

[lequytra]

I took a look at this. I think the wrong redirection is an easy fix but we have a problem with associations between users and messages so right now deleting a user throws an error because of InvalidForeignKey error.
InvalidForeignKey (PG::ForeignKeyViolation: ERROR: update or delete on table "users" violates foreign key constraint "fk_rails_bb310b7e83" on table "rides") DETAIL: Key (id)=(1) is still referenced from table "rides".
@AlexanderOtavka

[YourFin]

I think this and #52 fundamentally stem from not having a consistent methodology on what to do with dangling user keys. Our options here (on a per-foreign key basis) are as follows:

• Cascading deletes: Everything that references the user gets deleted
• Reverse the relation: move the reference to the user. Effectively the same outcome as cascading deletes, and potentially means that we need to do garbage collection on afflicted fields
• Allow null references: Conceptualize a dummy user with a unique id, and move all references to said user upon delete
• Don't actually delete the user: Change the way our code works so that we can do "soft deletes", potentially deleting information in some columns but not actually freeing the id. Devise has some documentation on this.

We are going to need one of the last two solutions for the case where a driver deletes their account, as the riders on their rides should still be able to see that they took a ride. If we decide that riders should still be able to see who they took rides with, we have to implement soft deletes (or denormalize our historical records, but ick and that's hard in rails).

My guess is that soft deletes, even if we get rid of all identifying information, will be probably easier to implement than a null user as it does not seem to stray as far from the well beaten path. However, even if we delete all the columns from deleted users except their ride id, it would still be possible to reconstruct who a rider is if you know a couple rides they took by looking for common ids, which does raise potential data privacy problems.

This was a bit of a brain dump, so please bother me if anything doesn't make sense.

[yashdavisgupta]

@YourFin I think your comment makes perfect sense. I think the soft delete is the way to go as it will sometimes be important to know which user did something (like posted a message or was a driver). As far as data privacy, I'm sure there is something to be said for that, but at the end of the day this is a social website (users interacting with other users) so there will be some record of users using the site even if we delete everything. I would take the route of Reddit on this, user generated data can be deleted and user references can be deleted, but the fact that they existed can't. Further, in the case of rides, if a driver offers a ride and deletes their account and passengers join it we should allow this ride to exist but remove the driver. Same for passengers: in the case of a canceled account, passengers need to be removed from all rides.

[AlexanderOtavka]

I prefer deleting the user fully and nullifying or deleting all related models. They only relations that would need to be made optional are the created_by properties of messages and rides, and there isn't a lot of code that relies on those being non-null. We could safely destroy seat assignments and notification subscriptions, since those are just join tables.