• fix: add postgres triggers to remove deleted users from user_links #12117 👈
• fix: do not query user_link for deleted accounts #12112
• chore: add database test fixture to insert non-unique linked_ids #12111
• main

This stack of pull requests is managed by Graphite.Learn more about stacking.

Join@Emyrk and the rest of your teammates onGraphite

Why are we removing the ability to undelete a user?

@kylecarbs

Why are we removing the ability to undelete a user?

We never exposed this via the CLI or the API. It only exists in the DB query.

However, un-deleting a user is more work than just the query that I removed. It can now collide with existing users, theiruser_links were not correct, and now deleted.

Essentially undeleting a user can cause issues (if it can succeed at all), so we never supported it. And we should remove the query that will very likely fail in prod. I fear someone would just add a query param, use the existing query, merge it (because we do not test deleted users very much in tests). Then almost immediately it'll cause issues in an actual database.

I know we never supported it, but it feels odd to keep the soft delete and areal delete. Can we change it to completely do one or the other?

I know we never supported it, but it feels odd to keep the soft delete and areal delete. Can we change it to completely do one or the other?

We do not support hard deletes at all on most resources. For auditing purposes, we need to keep these objects around.

@Emyrk I'm confused why weshould then on users. It seems like a fork in the road. It seems better to either leave this and not permit undeleting a user (e.g. actually blocking it), or making it work instead of allowing it for a single resource type.

I do not totally follow.

The product feature set is unchanged. We previously only supported soft deleting users from the API. I just made the DB query reflect this, so nowyou cannot un-delete users even from a DB query. So just trying to prevent developer error.

Theuser_links cleanup is because a deleted user should not be able to authenticate into Coder anymore. This becomes problematic if this is allowed because then a single github user can have multiple accounts linked to it. Maybe this is ok? Just felt strange from a data POV, so I implemented this to clean it up in the same way we do api keys.

2. Is it alright if we continue to useUpdateDeleted instead ofSoftDelete? Does my rationale make sense in this context?

How about I return the name toUpdateUserDeletedByID but always make it dodeleted = true? So there is no argument to ever revert the deleted state?

That sounds good to me!