search

bullet-train-co / bullet_train-api

MIT License
4 stars 3 forks source link

Flag users and memberships created by the Application object flow #1

Open ssteffen opened 2 years ago

ssteffen commented 2 years ago

When creating a new Application record to interact with the API layer, Bullet Train creates a user and membership that corresponds to the application. The issue is that when this user is removed from the team, the API will silently fail from then on. While there is a lot of power with this, it would be helpful to be able to programatically identify the user as an "Application" or "Platform" user, and potentially some safety guards around deleting those users without also removing the application.

andrewculver commented 2 years ago

@ssteffen Agreed, these memberships should not be removable.

andrewculver commented 2 years ago

@gazayas I think Membership should throw an exception if someone tries to remove a membership that has platform_agent_of_id and try to figure out how to use CanCanCan to mark those memberships as cannot :destroy.

andrewculver commented 2 years ago

(For context, Membership can't actually be deleted in-app, it can only have it's user_id attribute nullified.)

bborn commented 2 years ago

+1 also if you destroy the user before destroying the application, application#destroy_user will fair with an error undefined method 'destroy' for nil:NilClass (since the user doesn't exist anymore)

gazayas commented 2 years ago

Ah, we never linked https://github.com/bullet-train-co/bullet_train-base/pull/85 here.

I'm also seeing that we can technically re-invite the application to the team once we remove them, which I personally don't feel is in line with the original logic to disallow platform application tombstoning. The membership shows up as archived because we delete the user via the platform application page, but maybe we just shouldn't show the membership at all once it's there.

I'll also look into safeguarding against a NoMethodError when the user isn't present.

@andrewculver, any specific direction you want to take with this one?

gazayas commented 2 years ago

We can potentially at least add a CanCanCan role to the Re-invite to Team button if we want to keep application memberships on record.