Page MenuHomePhabricator
Differential D6349

[web] Allow account deletion for SIWE accounts
ClosedPublic
Actions

Authored by atul on Jan 23 2023, 9:55 AM.
Tags
None
Referenced Files
F3281848: D6349.diff
Sat, Nov 16, 10:17 AM
Unknown Object (File)
Tue, Nov 12, 12:26 AM
Unknown Object (File)
Tue, Nov 12, 12:25 AM
Unknown Object (File)
Tue, Nov 12, 12:25 AM
Unknown Object (File)
Fri, Nov 8, 2:57 AM
Unknown Object (File)
Fri, Nov 8, 2:57 AM
Unknown Object (File)
Fri, Nov 8, 2:57 AM
Unknown Object (File)
Fri, Nov 8, 2:57 AM
View All Files
Subscribers
None

Details

Reviewers
ashoat
tomek
marcin
Commits
rCOMMcb1fb3254d87: [web] Allow account deletion for SIWE accounts
Summary

Context: https://linear.app/comm/issue/ENG-2586/do-not-require-password-when-deleting-siwe-account-on-web

Basically D6099, but for web instead of native.

Test Plan
  1. Able to successfully delete SIWE account:

5fceb3.png (978×3 px, 331 KB)

38e782.png (632×1 px, 49 KB)

7d6526.png (978×3 px, 306 KB)

  1. "Traditional" account deletion continues to work as expected:

Diff Detail

Repository
rCOMM Comm
Branch
arcpatch-D6349 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul created this revision.Jan 23 2023, 9:55 AM
atul published this revision for review.Jan 23 2023, 10:02 AM
atul edited the test plan for this revision. (Show Details)
atul added a reviewer: marcin.
Harbormaster completed remote builds in B15547: Diff 21208.Jan 23 2023, 10:10 AM
ashoat requested changes to this revision.Jan 23 2023, 10:29 AM

The second screenshot you included was not something you changed. Can you update this diff to have the screenshot of the interface you actually changed?

web/settings/account-delete-modal.react.js
169 ↗(On Diff #21208)

This invariant is still here. There's no point to the conditional below. Pretty sure you'll crash if this is triggered

This revision now requires changes to proceed.Jan 23 2023, 10:29 AM
atul added a comment.Jan 23 2023, 10:36 AM
In D6349#190075, @ashoat wrote:

The second screenshot you included was not something you changed. Can you update this diff to have the screenshot of the interface you actually changed?

Yeah meant to attach the following:

38e782.png (632×1 px, 49 KB)

(before)

53afc3.png (798×1 px, 68 KB)

atul updated this revision to Diff 21219.Jan 23 2023, 10:39 AM

address feedback

atul edited the test plan for this revision. (Show Details)Jan 23 2023, 10:40 AM
ashoat accepted this revision.Jan 23 2023, 10:42 AM
This revision is now accepted and ready to land.Jan 23 2023, 10:42 AM
atul added inline comments.Jan 23 2023, 10:49 AM
web/settings/account-delete-modal.react.js
169 ↗(On Diff #21208)

Again, not sure how I missed that... crap

Harbormaster failed remote builds in B15558: Diff 21219!Jan 23 2023, 10:49 AM
Harbormaster completed remote builds in B15558: Diff 21219.Jan 23 2023, 11:01 AM
atul updated this revision to Diff 21232.Jan 23 2023, 2:14 PM

rebase before landing

atul updated this revision to Diff 21233.Jan 23 2023, 2:20 PM

fix

atul added inline comments.Jan 23 2023, 2:24 PM
web/settings/account-delete-modal.react.js
76–81

Tested with "traditional" account creation, had to modify this invariant to handle falsiness of empty string.

Harbormaster failed remote builds in B15569: Diff 21233!Jan 23 2023, 2:29 PM
Harbormaster completed remote builds in B15568: Diff 21232.
Harbormaster completed remote builds in B15569: Diff 21233.Jan 23 2023, 2:41 PM
atul updated this revision to Diff 21252.Jan 24 2023, 9:52 AM

land

This revision was landed with ongoing or failed builds.Jan 24 2023, 9:53 AM
Closed by commit rCOMMcb1fb3254d87: [web] Allow account deletion for SIWE accounts (authored by atul). · Explain Why
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMMcb1fb3254d87: [web] Allow account deletion for SIWE accounts.
Harbormaster completed remote builds in B15587: Diff 21252.Jan 24 2023, 10:07 AM

Revision Contents
Changeset List

PathSize
web/
settings/
69 lines

Diff 21233

View Options

web/settings/account-delete-modal.react.js

Loading...