Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

FxA AccountObserver.onAuthenticated: what is considered a newAccount? Update docs #4069

Closed
mcomella opened this issue Aug 9, 2019 · 4 comments
Closed

FxA AccountObserver.onAuthenticated: what is considered a newAccount? Update docs #4069

mcomella opened this issue Aug 9, 2019 · 4 comments
Assignees
@jonalmeida
Labels
📖 documentation Writing code documentation, wiki articles, .. ❓ question Questions, help, support
Milestone
9.0.0 📷

Comments

@mcomella
Copy link
Contributor

mcomella commented Aug 9, 2019 •
edited by data-sync-user
Loading

In 7.0.0, the AccountObserver interface added a newAccount parameter to the onAuthenticated method. However, I'm not sure what's considered a newAccount. The docs say,

@param newAccount True if this is a new account that was authenticated.

Is a new account an account that:

  • has never been added to this app before? (i.e. it's not new if you log out and log back in)
  • wasn't logged out but onAuthenticated is called? (e.g. on app start or when authenticating after re-auth is needed)
  • if the user has signed into a second account on the device (if that's even possible)?

We should clarify what a newAccount is and update the docs.

@grigoryk Any thoughts?

┆Issue is synchronized with this Jira Task
mcomella added a commit to mcomella/firefox-tv that referenced this issue Aug 9, 2019
@mcomella
Issue mozilla-mobile#2591: Fix breaking change in AccountObserver.onA…
8ee3523 
…uthenticated signature.

The `newAccount` parameter was added. I don't know what a new account is
so I filed mozilla-mobile/android-components#4069.
That being said, it doesn't seem like it'd affect our code so I didn't
make any changes for it.
@mcomella mcomella mentioned this issue Aug 9, 2019
Merged
4 tasks
@jonalmeida jonalmeida added the ❓ question Questions, help, support label Aug 9, 2019
@jonalmeida
Copy link
Contributor

jonalmeida commented Aug 9, 2019 •
edited
Loading

When the app is cold booted, the account is re-authenticated so the the onAuthenticated callback is fired. There are some use cases where you want to be notified when an account was authenticated only on the first time that an account was signed into.

If you logged into an account (newAccount is true), logged out, logged in again with the same or another account, you would get newAccount as true.

  • has never been added to this app before? (i.e. it's not new if you log out and log back in)

Any new login is considered a new account.

  • wasn't logged out but onAuthenticated is called? (e.g. on app start or when authenticating after re-auth is needed)

It won't fire for this.

  • has never been added to this app before? (i.e. it's not new if you log out and log back in)

This case isn't really possible, but if you have a fennec install and you re-authenticated that account in a new browser (e.g. Fenix, Fire TV), then technically that's a new account added to the app.

dnarcese pushed a commit to mozilla-mobile/firefox-tv that referenced this issue Aug 9, 2019
@mcomella @dnarcese
Issue #2591: Fix breaking change in AccountObserver.onAuthenticated s…
e601342 
…ignature.

The `newAccount` parameter was added. I don't know what a new account is
so I filed mozilla-mobile/android-components#4069.
That being said, it doesn't seem like it'd affect our code so I didn't
make any changes for it.
@mcomella mcomella changed the title FxA AccountObserver.onAuthenticated: what is considered a newAccount? FxA AccountObserver.onAuthenticated: what is considered a newAccount? Update docs Aug 9, 2019
@mcomella mcomella added the 📖 documentation Writing code documentation, wiki articles, .. label Aug 9, 2019
@mcomella
Copy link
Contributor Author

mcomella commented Aug 9, 2019

Thanks for the explanation, Jonathan. I think the action item here is to update the documentation.

bors bot pushed a commit that referenced this issue Aug 14, 2019
@MickeyMoz @jonalmeida
Merge #4122
cca2831 
4122: Closes #4069: Update docs for newAccount param r=jonalmeida a=jonalmeida



Co-authored-by: Jonathan Almeida <[email protected]>
bors bot pushed a commit that referenced this issue Aug 14, 2019
@MickeyMoz @jonalmeida
Merge #4122
9ad814d 
4122: Closes #4069: Update docs for newAccount param r=jonalmeida a=jonalmeida



Co-authored-by: Jonathan Almeida <[email protected]>
@bors bors bot closed this as completed in 2a43a1e Aug 14, 2019
@pocmo pocmo added this to the 9.0.0 📷 milestone Aug 19, 2019
@jonalmeida
Copy link
Contributor

Re-opening since the variable name is still misleading so we should rename that too.

@jonalmeida jonalmeida reopened this Aug 28, 2019
@jonalmeida jonalmeida self-assigned this Aug 28, 2019
@jonalmeida jonalmeida modified the milestones: 9.0.0 📷, 11.0.0 🍀 Aug 28, 2019
@jonalmeida
Copy link
Contributor

Closing again since this was completed in another PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jonalmeida jonalmeida

Labels
📖 documentation Writing code documentation, wiki articles, .. ❓ question Questions, help, support
Projects
None yet
Milestone
9.0.0 📷
Development

No branches or pull requests
3 participants
@pocmo @mcomella @jonalmeida