Closed Bug 967008 Opened 11 years ago Closed 11 years ago

getAccounts should return the account info right away even if the account isn't verified

Categories

(Firefox OS Graveyard :: FxA, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.0 S1 (9may)

People

(Reporter: ferjm, Assigned: spenrose)

References

Details

Attachments

(1 file, 2 obsolete files)

Right now, if the account in the device is not verified, we check with the server the account status before returning the account information. We should return the account information right away and check the verification status in the background.
Assignee: nobody → ferjmoreno
Blocks: 955951
Blocks: 974096
No longer blocks: 955951
Jed this seems to work for me. Could it be this simple?
Attachment #8395897 - Flags: feedback?(jparsons)
Comment on attachment 8395897 [details] [diff] [review]
967008-getAccounts-unverified.patch

This certainly seems like it will do what comment 0 is asking.  So f+ on the direction.

If there are no longer any functions waiting on promises from verificationStatus(), maybe it shouldn't return anything.  In which case, verificationStatus() will want some reworking to make it simpler.

In the case of errors in verificationStatus(), such as 'offline', would they now just get logged?  (Again, no promises to reject with _error().)

Also, don't forget to update the tests.
Attachment #8395897 - Flags: feedback?(jparsons) → feedback+
Attached patch 967008-getAccounts-return.patch (obsolete) — Splinter Review
Per feedback I converted verificationStatus() to have no return value; errors are now logged. I updated three of the affected tests; the fourth did not seem worth saving.
Assignee: ferjmoreno → spenrose
Attachment #8395897 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8396741 - Flags: review?(jparsons)
No longer blocks: 949053
Comment on attachment 8396741 [details] [diff] [review]
967008-getAccounts-return.patch

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

Thanks, Sam.  Looks good to me. 

I wonder whether there couldn't be a better name than 'verificationStatus' for this function now.  Maybe 'checkVerificationStatus' or 'updateVerificationStatus' or something.

If you can think of a great name, go ahead and change it.  But it's ok by me if it goes in like this.

r=me
Attachment #8396741 - Flags: review?(jparsons) → review+
Attachment #8397141 - Flags: review+
Attachment #8396741 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e75306473cf8
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.5 S1 (9may)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: