[MAGE] add delete user option #1577
Conversation
There was a problem hiding this comment.
@vincanger nice! Looks great, couple of small comments and we can merge!
| @@ -154,7 +159,7 @@ entity SocialLogin {=psl | |||
| providerId String | |||
|
|
|||
| userId Int | |||
| user User @relation(fields: [userId], references: [id]) | |||
| user User @relation(fields: [userId], references: [id], onDelete: Cascade) | |||
There was a problem hiding this comment.
Thought: once projects are deleted, we will never know they existed, so our number of created projects will become smaller. Would be cool if they only thing that is left is this idea of "there was a project that was deleted". But to be honest I am not sure myself how to implement that simply, so probably best to just ignore it, doesn't matter.
There was a problem hiding this comment.
this isnt for projects. when the user is deleted it deletes the social login entity, but leaves their projects in the DB.
There was a problem hiding this comment.
I think it's fine to leave the generated apps in the database in case they've been shared. there is no connection whatsoever to the user that created it.
There was a problem hiding this comment.
I think it would still be good if they can delete their projects, since they were authored by them.
I would suggest on of the following:
- Delete all the project they created, and that is it, simple.
- Give them choice -> they can delete also the projects, or they can leave them.
There was a problem hiding this comment.
ok I've gone ahead and deleted all user-relevant info leaving just the project shell :)
There was a problem hiding this comment.
@vincanger looking good! I approved, but just wanted to check, what does it look like if somebody tries to open a project of deleted user via its URL? Is it broken, or does it look somewhat reasonable? Would be good if it isn't a total mess, but instead it said something like This project was deleted.
Also,
Aha, ok it will have Might be better if we set |
Good catch! I didn't want to change the name and description properties to optional. so I just made the delete function update the status to "deleted", and then check for that on the client and show: |
There was a problem hiding this comment.
LGTM: I left a small note regarding how "delete" could be more "clean cody", but since we dropped the ball anyway for statuses regarding that, then we don't need to fix it now, we can fix it all later, so all good.
NOTE: It is a bit tricky when formatting is all mixed up with changes, it makes it really hard for me to figure out which are actual changes.
Ways to go about this, when we have a codebase like this where files are not yet formatted, is to e.g. turn of formatting in your editor and do it without, and then do separate PR which does the formatting. Or, maybe at least a separate commit (although that is also tricky to review if there is more than one non-formatting commit).
Or you could maybe do formatting, but then comment in the PR on the changes that are relevant, so I know what to focus on.
| @@ -244,7 +252,16 @@ export const ResultPage = () => { | |||
| </> | |||
| )} | |||
|
|
|||
| <Logs logs={logs} status={currentStatus.status} onRetry={retry} /> | |||
| {appGenerationResult?.project.status.includes("deleted") ? ( | |||
There was a problem hiding this comment.
This "deleted" here, would be great to have it imported from somewhere, so we don't have duplication. But this project in general is not very careful about this, so if all the other statuses are handled badly like this then we can postpone this refactoring to be refactored all together.
Description
Adds delete account functionality to the UserPage, and another couple small improvements
Select what type of change this PR introduces:
Update Waspc ChangeLog and version if needed
If you did a bug fix, new feature, or breaking change, that affects waspc, make sure you satisfy the following: