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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: FxA, defect)

Description

[Fernando Jiménez Moreno [:ferjm]]

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.

Comment 1

[Sam Penrose]

Attachment: 967008-getAccounts-unverified.patch

Jed this seems to work for me. Could it be this simple?

Comment 2

[Jed Parsons (use needinfo, please) [:jedp, :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.

Comment 3

[Sam Penrose]

Attachment: 967008-getAccounts-return.patch

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.

Comment 4

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

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

Comment 5

[Sam Penrose]

Attachment: 967008-getAccounts-return.patch

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/e75306473cf8

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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