fix: user delete social auth #3693
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@shubham-padia is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
bot
added
front-end
| @@ -126,6 +128,7 @@ class TheComponent extends Component { | |||
| render() { | |||
| const { | |||
| state: { | |||
| auth_type, | |||
There was a problem hiding this comment.
Using snake case here, since first_name, last_name and similar fields are also doing the same for fields fetched from the API
| { isError: isMutationError, isSuccess: updateSuccess }, | ||
| ] = useDeleteAccountMutation() | ||
| const skipPasswordConfirmation = | ||
| auth_type === 'GOOGLE' || auth_type === 'GITHUB' |
There was a problem hiding this comment.
Using string literals here, since I could not find enums being used elsewhere in the frontend codebase
|
Uffizzi Preview |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3693 +/- ##
==========================================
- Coverage 95.92% 95.89% -0.03%
==========================================
Files 1103 1103
Lines 34812 34848 +36
==========================================
+ Hits 33392 33419 +27
- Misses 1420 1429 +9 ☔ View full report in Codecov by Sentry. |
shubham-padia
force-pushed
the
fix/user-delete-social-auth
branch
from
9bd0e3f
to
69626b5
Compare
shubham-padia
changed the title
shubham-padia
force-pushed
the
fix/user-delete-social-auth
branch
from
69626b5
to
0277681
Compare
Not sure why all of these 9 lines of code are not being covered, they should be called by the testing functions 🤔 , except the fail message. I can add tests for the fail message, but not sure if it should be this PR or another one since it is related to the original PR introducing the delete user functionality. https://app.codecov.io/gh/Flagsmith/flagsmith/commit/9bd0e3fba28f33d7c96b6d76c1fcf408e2474e78 (screenshot below) |
|
bump for review |
| # create a couple of users | ||
| email1 = "test1@example.com" | ||
| email2 = "test2@example.com" | ||
| email3 = "test3@example.com" | ||
|
|
There was a problem hiding this comment.
Could you please remove any unnecessary line breaks from the code?
There was a problem hiding this comment.
@shubham-padia are you able to resolve this and we can get this merged?
|
@novakzaballa Replied to your comment. Let me know if there are any other blockers to getting this merged :) ! |
shubham-padia
force-pushed
the
fix/user-delete-social-auth
branch
from
0277681
to
d1fe382
Compare
Fixes Flagsmith#2782. We are enabling passwordless deletions only for Github and Google accounts and not for non email/password auth accounts, since it doesn't feel right to assume the behaviour of the next social auth that might be added.
We are only checking the email match on the frontend, because if user is directly using the API, we can assume they know their stuff enough not to accidentally delete their account. Social auth users with password will also get the email confirmation screen.
shubham-padia
force-pushed
the
fix/user-delete-social-auth
branch
from
d1fe382
to
087e8d7
Compare
Fixes #2782
Thanks for submitting a PR! Please check the boxes below:
pre-committo check lintingdocs/if required so people know about the feature!Changes
We are enabling passwordless deletions only for Github and Google accounts and not for non email/password auth accounts, since it doesn't feel right to assume the behaviour of the next social auth that might be added. It's been a while since I dabbled with React, so please lmk if I should structure the code in a different way.
I'm also just ignoring the password field altogether for social auth as you can see in the API tests. Lmk if there are any concerns there.
How did you test this code?
Delete user with email/password:
email.account.delete.flagsmith.mp4
Delete user with social auth (only tested on google for now):
flagsmith.google.delete.mp4