Support dedicated /me/security UI to change email address

[daledupreez]

Changes proposed in this Pull Request

This PR may well be too big, but it takes on a number of closely related tasks to expand the work in #43527:

• It updates the state and data layers for user settings as follows:
  • Settings updates now support an error handler callback
  • There is a new isFetchingUserSettings() selector (and the related wiring) so fetches and updates can be reflected in the UI
  • The isPendingEmailChange() selector has been updated to force a boolean return value.
• It refactors the email logic currently used in /me/account into a dedicated component
  • At the same time, it does _not_refactor the existing code to use Redux, so it has plenty of bifurcation depending on whether the container is supplying a UserSettings object or expecting the component to rely on the Redux state.
• It adds a new component to /me/security/account-email that is only responsible for updating the account email address, and fits the UX for the other menu items listed under /me/security.

Testing instructions

There is a LOT to test here.

Regression Tests for /me/account

Our main goal here is to make sure that the existing email modification UI continues to work. Make sure the following work:

• Navigate to /me/account and update the email address field.
  • Make sure that invalid addresses result in an error being shown, and the submit button being disabled.
  • Make sure that if the email address is the same as your starting email address, the submit button is disabled.
  • Enter a valid email address and submit the updated email address.
• Ensure that after you've submitted the change, a pending change warning appears and you can't modify the field directly.
• Click on the "Cancel" option for the pending change, and verify that the form updates and reverts to the original email address, and you can update and submit the email address again.
  It may also be worth verifying an email to check that the UI updates appropriately -- this is existing behaviour, but I don't believe I touched it in this case.

Testing the email option in /me/security

Using a local Calypso version, or by using the calypso.live UI for this branch, try the following:

• Navigate to /me/security.
• Verify that clicking on the "Account Email" entry takes you to /me/security/account-email, and that clicking the "Back" button gets you back to the menu. Click on the "Account Email" entry again.
• In the dedicated email UI, verify that the following occur:
  • When the email address in the text box matches your initial email address, the "Update" button is disabled.
  • When the email address in the text box is invalid, an appropriate error should be displayed, and the "Update" button should be disabled.
  • When you type in a valid email address, the "Update" button should be enabled.
• Type in a valid email address for yourself, and click on the "Update" button.
• The button text should change to "Updating..." and should be disabled while that text displays. Once the update completes, the button should remain disabled with the original "Update" text. In addition, the email field should be disabled and should show a prominent notice indicating that the email change is still pending.
• Click on the "Back" button.
• Verify that the "Account Email" entry shows a warning icon and the text: "There is an unconfirmed change of your email address to email@example.com."
• Click on the "Account Email" entry, and verify that the pending email change is displayed.
• Click on the "Cancel" text/link in the email warning, and verify that the email field reverts to your original email address.
• Click on the "Back" button, and verify that the main security menu shows a check-mark and your original email address.

[matticbot]

Test live: https://calypso.live/?branch=try/security-account-email-ui

[matticbot]

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Webpack Runtime (~95 bytes added 📈 [gzipped])

name      parsed_size           gzip_size
manifest       +261 B  (+0.1%)      +95 B  (+0.2%)

Webpack runtime for loading modules. It is included in the HTML page as an inline script. Is downloaded and parsed every time the app is loaded.

App Entrypoints (~61 bytes added 📈 [gzipped])

name        parsed_size           gzip_size
entry-main       +394 B  (+0.0%)      +61 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~7238 bytes added 📈 [gzipped])

name                   parsed_size           gzip_size
security                   +9956 B  (+2.1%)    +3019 B  (+2.5%)
account                    +5515 B  (+1.5%)    +2087 B  (+2.1%)
concierge                   +957 B  (+0.3%)     +242 B  (+0.3%)
site-blocks                 +936 B  (+0.3%)     +236 B  (+0.3%)
purchases                   +936 B  (+0.1%)     +238 B  (+0.1%)
privacy                     +936 B  (+0.4%)     +236 B  (+0.3%)
notification-settings       +936 B  (+0.3%)     +236 B  (+0.3%)
me                          +936 B  (+0.4%)     +236 B  (+0.3%)
help                        +936 B  (+0.2%)     +236 B  (+0.2%)
happychat                   +936 B  (+0.3%)     +236 B  (+0.3%)
account-close               +936 B  (+0.3%)     +236 B  (+0.3%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~1771 bytes added 📈 [gzipped])

name                                             parsed_size           gzip_size
async-load-layout-community-translator-launcher      +1142 B  (+6.0%)     +262 B  (+4.3%)
async-load-reader-site-stream                         +957 B  (+3.7%)     +250 B  (+3.2%)
async-load-reader-sidebar                             +957 B  (+1.5%)     +255 B  (+1.6%)
async-load-reader-search-stream                       +957 B  (+1.1%)     +250 B  (+1.1%)
async-load-reader-following-manage                    +957 B  (+0.9%)     +250 B  (+0.8%)
async-load-reader-feed-stream                         +957 B  (+3.7%)     +250 B  (+3.3%)
async-load-design-blocks                              +957 B  (+0.0%)     +254 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

[klimeryk]

Thanks for improving this abandoned, but super important section in Calypso! 🥇

As expected with a big refactor of an old code, some issues crept in, but overall I love the direction.

One thing that could use still improvement is loading/read error handling, as it seems the UI is just permanently stuck in loading state if the backend returns an error (there's not even an error notice). For updates, it seems much better, though.

I've also noticed a bit confusing state when the account email component is loaded:

When the state of the email is being fetched from the backend, the button actually changes to Updating... - same as if I actually pressed it to update. And, as mentioned above, if there's an error when loading the state, the button is stuck in the Updating... state.

[klimeryk]

I think there might be still something wrong with this logic (or the way we handle the data in Redux/flux?). There's a different behaviour still for the empty email case.

In /me/account, when the field is empty, there's an explicit error message shown:

However, in account-email, there is no error message:

The field is highlighted as error, though - and you can't submit the form... unless you go to a different view and come back:

😁

[daledupreez]

This was basically the same underlying logic issue around '' being falsey that I worked on in 6e6d760, but I didn't catch the problem in the validation checks. This should be fixed now.

[daledupreez]

Thanks for the thorough testing, @klimeryk! 😍😍💖💖💖

I've addressed almost all of the issues you flagged. The one area that's still a bit suspect overall is clearer handling for successful/failed requests to the server. I believe that other state management logic uses distinct _SUCCESS and _FAILURE actions for this purpose, but I am not sure how much bigger that would make this PR. 😐 (Even the code in this PR would benefit from being able to detect successful saves in particular.)

I also want to note that the existing /me/account page has reasonably poor error handling too! So this definitely counts as an improvement. 😁

[klimeryk]

Nice, thanks for the updates! The validation is indeed working much better now. And I agree that the error handling there is indeed a bit bare-bones. If it means the PR would get bigger, I think it's fine to leave it for a follow-up one. At least now there's no error - the update button just hangs in the "Updating..." state. But refreshing or going back and again to the view fixes it.

Speaking of which, I noticed one more bug: if you change the email to an invalid value in me/security/account-email, you get a validation error and the update button is inactive. But if you click the Back button and go back to the view, the value remains the same (invalid), but there's no validation error and you can submit the form. I have not tested if the backend will actually accept it (hopefully not :D). I'm guessing it's related to that whole mess with Redux and flux stores. Up to you if you want to fix it. Looks like there is backend validation so it should not be a blocker.

[daledupreez]

Thanks for taking another look, @klimeryk!

I did spend some time on the state-related work yesterday, and I came away realising that it needs some serious work to support good error handling for fetches and saves. Pretty much anything I do at the moment is somewhere between a work-around and a hack. 😬

I'll take another look at the weird cases you tripped over, and will likely wrap this PR with those fixes.

[flootr]

@daledupreez are you still working on this PR? As a FYI I started working on removing lib/user-settings as part of our reduxification efforts (#24162) and will very likely touch a few of the files you worked an, I can even see some overlap with the work you did here.

[daledupreez]

Thanks for checking, @flootr!
For now, this is something I want to pick up, but I got stuck on the fact that I didn't have good state handling for settings. I think you should go ahead and get the state changes done -- I'll try to pick this work up once the cleanup has been done (which should actually make my work much simpler!).

[github-actions]

This PR has been marked as stale due to lack of activity within the last 30 days.