Closed Bug 1004242 Opened 11 years ago Closed 11 years ago

Expose FxAccounts resendVerificationEmail to Gaia

Categories

(Firefox OS Graveyard :: FxA, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.0 S1 (9may)

People

(Reporter: spenrose, Assigned: spenrose)

References

Details

Attachments

(1 file, 2 obsolete files)

FxAccountsClient.jsm has a resendVerificationEmail method, which is exposed to FxAccounts.jsm, but needs to be added to FxAccountsManager.jsm and b2g/components/FxAccountsMgtmService.jsm so that Gaia can reach it with a mozFxAccountsContentEvent .
Blocks: 1004251
Attached patch 1004242-expose-resend.patch (obsolete) — Splinter Review
I managed to trigger an appropriate mozFxAccountsContentEvent in Gaia and got the server to respond to a '/recovery_email/status' HTTP request, so this patch "works." I'm a little nervous about the reject() path in FxAccountsManager.jsm, and would love your thoughts on whether it is enough given the throw in its callee. Thanks!
Attachment #8416159 - Flags: feedback?(jparsons)
No longer blocks: 974999
Attached patch 1004242-expose-resend.patch (obsolete) — Splinter Review
Per offline chat, I added a unit test that verifies error handling. Thanks for your consideration!
Attachment #8416159 - Attachment is obsolete: true
Attachment #8416159 - Flags: feedback?(jparsons)
Attachment #8418825 - Flags: review?(jparsons)
Comment on attachment 8418825 [details] [diff] [review] 1004242-expose-resend.patch Review of attachment 8418825 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, Sam! Thanks for adding the test. ::: services/fxaccounts/FxAccountsManager.jsm @@ +254,5 @@ > + return this._fxAccounts.resendVerificationEmail().then( > + (result) => { > + return result; > + }, > + (error) => { I realize more and more how many different formatting styles we have in these files for promises. It's not a problem with this patch, but something to add to the List Of Things To Fix When We Have All The Time In The World. ::: services/fxaccounts/tests/xpcshell/test_manager.js @@ +608,5 @@ > + (success) => { > + do_throw("Unexpected success"); > + }, > + (error) => { > + do_check_eq(error.error, ERROR_SERVER_ERROR); perfect. thank you.
Attachment #8418825 - Flags: review?(jparsons) → review+
Attachment #8418825 - Attachment is obsolete: true
Keywords: checkin-needed
Ryan -- Per your request I ran a try build. The result was a cascade of known issues, mostly timeouts, many of them two years old. I don't see any real problems, but of course I may be missing one. What's the best way forward? https://tbpl.mozilla.org/?tree=Try&rev=f0617c6bb803
Flags: needinfo?(ryanvm)
That run is looking fine so far. We're just getting more picky about requiring Try runs before landing checkin-needed patches. BTW, please give https://wiki.mozilla.org/Sheriffing/How:To:Recommended_Try_Practices a look when you get a chance :)
Flags: needinfo?(ryanvm)
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S1 (9may)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: