981210 - FxAccountsManager.jsm and consumers: change "accountId" attribute to "email"

Status: RESOLVED FIXED

(Firefox :: Firefox Accounts, defect)

Description

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Right now, when I login using the fxa test client, the FxAccountsIACHelper.openFlow() callback gets:

{"email": "[e-mail]", "success": true, "verified": true}

but if I then do FxAccountsIACHelper.getAccounts(), the response is:

{"accountId": "[e-mail]", "verified": true}

We should use accountId everywhere for consistency.

Comment 1

[Sam Penrose]

Jed and I chatted about this and noticed that the UI refers to "email" and the value of the field must be a orutable email, so we're inclined to go with "email." Thoughts?

Comment 2

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Sounds great!

Comment 3

[Sam Penrose]

This comes down to a single attribute name in services/fxaccounts/FxAccountsManager.jsm which gets copied all over the place. So it's a straightforward find-and-replace for both gecko and gaia, but I'm a little concerned about coordinating the paired patches. Jared, how would you feel about tweaking the consumers of FxAccountsIACHelper to look for email and fall back to accountId? Then I could update gecko safely. At the same time I'd like to move all this to nice-to-have. So a new gaia bug to that effect would block 974121 and this bug, and this one would block 974096. Once this bug lands, we could then remove the fallback.

Comment 4

[Sam Penrose]

Per IRC with Jared, moving this to nice-to-have.

Comment 5

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Hey Sam - sorry for the lack of response here. I'll fix this inside the settings patch (we can skip opening another bug). Would you mind opening a bug to update the system app?

Comment 6

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

doh, forgot ni

Comment 7

[Sam Penrose]

Attachment: 981210-accountId-to-email.patch

This is a pure search-and-replace, except for the bit in FxAccountsMgmtService.jsm to catch accountId. Once this patch lands, we can take accountId out of Gaia, and then remove that kludge. Thanks Jed!

Comment 8

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Comment on attachment 8414771 [details] [diff] [review]
981210-accountId-to-email.patch

Review of attachment 8414771 [details] [diff] [review]:
-----------------------------------------------------------------

That looks good to me.  I searched services/ and b2g/ and don't find any other instances that need changing.

Thanks, Sam!

Comment 9

[Sam Penrose]

Attachment: 981210-accountId-to-email.patch

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d05f273a2d1d

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d05f273a2d1d