as per the white paper, we will require password users to enter their password in order to delete their account.
Depends on D12325
Details
Details
- usingCommServicesAccessToken = false; no change to the UI
- usingCommServicesAccessToken = true, password user; prompted for password, if incorrect, identity account deletion fails and alert is displayed, but user is not logged out. if correct, account successfully deleted and user is logged out
- usingCommServicesAccessToken = true, wallet user; no change to the UI, account deletion successful
- usingCommServicesAccessToken = true, password user, keyserver offline; prompted for password, if correct identity account deletion succeeds but keyserver account deletion fails. user is logged out and deleteAccountActionTypes.success is dispatched
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- delete
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
adding @inka as a blocking reviewer
referenced this commit for the UI changes: https://github.com/CommE2E/comm/commit/ba9a43c03a7c0f95ba7c71963452b2c98c0e9e8c?diff=split&w=0
| lib/actions/user-actions.js | ||
|---|---|---|
| 335 ↗ | (On Diff #41071) | we await this promise first to make sure that the user isn't logged out if identity account deletion fails. callKeyserverDeleteAccount below calls setNewSession if it succeeds, which logs the user out |
| 336–356 ↗ | (On Diff #41071) | if usingCSAT and keyserver account deletion fails, we should still dispatch deleteAccountActionTypes.success. otherwise, the user would be visibly logged in but unable to connect to a keyserver since they wouldn't have any keys in DynamoDB keyservers will have "zombie" accounts if keyserver account deletion fails but identity account deletion succeeds. we'll need some way to clean them up. this might be handled by ENG-7838 or ENG-5918 (cc @kamil ) |
| 349–351 ↗ | (On Diff #41071) | in the pre-usingCSAT world, we don't want to dispatch deleteAccountActionTypes.success if keyserver account deletion fails |
| lib/actions/user-actions.js | ||
|---|---|---|
| 357–363 ↗ | (On Diff #41071) | it seems like the only reducer that checks the payload on deleteAccountActionTypes.success is reduceCurrentUserInfo, which sets currentUserInfo in redux to the value of currentUserInfo in the payload if it's different than the existing state. i think setting it to null is correct, but want to double check (cc @tomek, @inka ) |
| lib/actions/user-actions.js | ||
|---|---|---|
| 317–335 ↗ | (On Diff #41071) | There's really no point to declaring this IIFE |
| 358 ↗ | (On Diff #41071) | This changed but I think it's okay |
| native/profile/delete-account.react.js | ||
| 92 ↗ | (On Diff #41071) | We never capitalize like this. It should always be "Password required". But separately, can we make the error cases and language match what we had previously? |
| lib/actions/user-actions.js | ||
|---|---|---|
| 317–335 ↗ | (On Diff #41071) | Right, I missed a return after the await identityClient.deletePasswordUser(password) |
| lib/actions/user-actions.js | ||
|---|---|---|
| 317–335 ↗ | (On Diff #41071) | I don't think we want to return after the await identityClient.deletePasswordUser(password), but I think I understand your general feedback about not using an IIFE |
| native/profile/delete-account.react.js | ||
|---|---|---|
| 132 ↗ | (On Diff #41117) | Realized we're missing autoCapitalize="none" here, can you add it in a follow-up diff? |