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.

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

Account Settings: better error messaging for change email flow when address is already in use #9384

Open
thehenrybyrd opened this Issue Mar 8, 2019 · 7 comments

Comments

4 participants
@thehenrybyrd
Copy link

thehenrybyrd commented Mar 8, 2019

Expected behavior

While logged into a WordPress.com account on the app, I try to update my email address to one that's used on a different WordPress.com account.
I expect to get an error which explains the email address is already being used on another account.

Actual behavior

I get the error "Could not save your account settings".

nochange

Steps to reproduce the behavior

You'll need two WordPress.com accounts to test.

  1. Logged into account #1, go to Me > Account Settings and tap Email.
  2. Change the email address to the email address associated with account #2.
  3. See the "Could not save your account settings" error.

User report, Xiaomi Redmi Note 4X, WP Android 11.6 - #1855172-zen
I also tested on WPiOS 11.9 with similar results, reported in wordpress-mobile/WordPress-iOS#11214

@rachelmcr

This comment has been minimized.

Copy link
Member

rachelmcr commented Mar 8, 2019

Note that if the email address change fails in this case, the API returns a nice error message we could use:

{"code":400,"headers":[{"name":"Content-Type","value":"application\/json"}],"body":{"error":"invalid_input","message":"That e-mail address is already being used."}}

@rachelmcr rachelmcr added this to To Do in Groundskeeping via automation Mar 8, 2019

@jtj9817

This comment has been minimized.

Copy link

jtj9817 commented Mar 8, 2019

Mind if I take a look at this issue?

@designsimply designsimply moved this from To Do to Prioritized Android in Groundskeeping Mar 15, 2019

@rachelmcr

This comment has been minimized.

Copy link
Member

rachelmcr commented Mar 19, 2019

Mind if I take a look at this issue?

Hi, sorry I missed your comment earlier — you're welcome to take a look! You may be interested in looking at how this was solved on the iOS side of things for inspiration: wordpress-mobile/WordPress-iOS#11229

@jtj9817

This comment has been minimized.

Copy link

jtj9817 commented Mar 20, 2019

@rachelmcr Thank you very much! I have hit a dead end for finding a solution to this problem but with that PR I might be able to solve it properly.

@jtj9817

This comment has been minimized.

Copy link

jtj9817 commented Mar 22, 2019

@rachelmcr I followed the logic of the solution in the PR that you posted. Since WordPress uses FluxC for handling network requests, it has a specific function for returning corresponding error messages. The problem is, since I'm using a development build (Oauth2), it's returning an error that my client is unauthorized to change the email (which might be true). So unless this fix is shipped in the production release, there's no way to verify if it's returning the correct error. Here's the screenshot:

Now here's a simpler solution, to just use another string that displays the appropriate code but there's no guarantee that all errors will be only because of invalid email:

@rachelmcr

This comment has been minimized.

Copy link
Member

rachelmcr commented Mar 25, 2019

Thanks for the update! Sorry for not catching earlier that this would be a tricky one to work on.

Since WordPress uses FluxC for handling network requests, it has a specific function for returning corresponding error messages.

This is definitely the approach we want to take here, since there could be other reasons for a failed update (e.g. entering an invalid email address). Are you comfortable opening a PR to make that change despite not being able to verify the error message in your dev build? During the PR review, your reviewer can use the production keys to test the change and verify the message that's returned.

(And thank you for raising this here and in the WordPress Slack! It has sparked discussion among the release team about how this could be improved for anyone working on developing the apps.)

@jtj9817

This comment has been minimized.

Copy link

jtj9817 commented Mar 28, 2019

@rachelmcr I would push a PR later today for this one, thank you for encouraging it!

@aforcier aforcier self-assigned this Apr 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.