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

RESOLVED FIXED in 2.0 S1 (9may)

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ferjm, Assigned: spenrose)

Tracking

unspecified
2.0 S1 (9may)
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
(Reporter)

Updated

5 years ago
Assignee: nobody → ferjmoreno
(Assignee)

Updated

5 years ago
Blocks: 955951
(Assignee)

Updated

5 years ago
Blocks: 974096
No longer blocks: 955951
(Assignee)

Comment 1

5 years ago
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+
(Assignee)

Comment 3

5 years ago
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)
(Assignee)

Updated

5 years ago
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+
(Assignee)

Comment 5

5 years ago
Attachment #8397141 - Flags: review+
(Assignee)

Updated

5 years ago
Attachment #8396741 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e75306473cf8
Status: ASSIGNED → RESOLVED
Last Resolved: 5 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.