Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Settings to allow users to delete their account #166

Merged
merged 19 commits into from
Jul 21, 2023
Merged

Settings to allow users to delete their account #166

merged 19 commits into from
Jul 21, 2023

Conversation

dadukhankevin
Copy link
Contributor

Allows users to hard delete their own account.
Closes #137.
image
image

@dadukhankevin dadukhankevin added the UI Design owner: Tim label Jul 5, 2023
@dadukhankevin dadukhankevin marked this pull request as draft July 10, 2023 03:29
@hahn-kev
Copy link
Collaborator

@dadukhankevin is this ready to review? It's still a draft PR so it looks like you're still working on it.

@dadukhankevin
Copy link
Contributor Author

@hahn-kev I thought it was ready for review but I guess I somehow missed pushing one file from the office computer to the branch... I'm going to rewrite that (it shouldn't take long) it should be ready to go, I just converted it to a draft so that it wouldn't accidentally be merged

@dadukhankevin dadukhankevin marked this pull request as ready for review July 14, 2023 02:43
hahn-kev
hahn-kev requested changes Jul 14, 2023
View reviewed changes
Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good, though I'd reuse as much of the code as you can from when you created the feature for admins to delete users. That way you don't need to duplicate so much (mutation client and server side, modal ect).

backend/LexBoxApi/GraphQL/UserMutations.cs Outdated Show resolved Hide resolved
frontend/src/routes/(authenticated)/user/DeleteAccountModal.svelte Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once we modify the existing delete user mutation we can delete this modal and just use the same one we use on the admin dashboard (moving that one into a shared location like lib/components)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dadukhankevin I believe this file is unused right?

hahn-kev
hahn-kev requested changes Jul 19, 2023
View reviewed changes
Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, there's a minor issue with the deleted notification though. Also you can probably remove the old user specific dialog.

frontend/src/routes/(authenticated)/user/+page.svelte Outdated Show resolved Hide resolved
frontend/src/routes/(authenticated)/user/DeleteAccountModal.svelte Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dadukhankevin I believe this file is unused right?

hahn-kev
hahn-kev reviewed Jul 20, 2023
View reviewed changes
backend/LexBoxApi/GraphQL/LexMutations.cs Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you checked in a merge conflict for this file

@hahn-kev hahn-kev merged commit 8350316 into sillsdev:develop Jul 21, 2023
3 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hahn-kev hahn-kev hahn-kev requested changes

Assignees
No one assigned
Labels
UI Design owner: Tim
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Let users delete their account
2 participants
@dadukhankevin @hahn-kev