Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'authenticated' event fires multiple times #44

Closed
max-mapper opened this issue Apr 16, 2013 · 10 comments
Closed

'authenticated' event fires multiple times #44

max-mapper opened this issue Apr 16, 2013 · 10 comments

Comments

@max-mapper
Copy link
Contributor

when I have a hoodie.account.on('authenticated' listener it calls the callback 3 times when I do hoodie.account.authenticate

@gr2m
Copy link
Member

gr2m commented Apr 19, 2013

Hey Max, I'm gonna look into this after front trends, sorry it takes so long

@gr2m
Copy link
Member

gr2m commented Apr 20, 2013

btw, there is also a signin event. Could you describe your use case? Maybe there's a better way, but I'll look into this anyway

@max-mapper
Copy link
Contributor Author

here is the code I have:

hoodie.account.authenticate().then(function noop(){}, isLoggedOut)

hoodie.account.on('authenticated', isLoggedIn)
hoodie.account.on('signout', isLoggedOut)

this works okay but it would be better if .on('authenticated') only fired
once and not 3 times. maybe there is a .on('unauthenticated') or a
.on('signin') ?

On Sat, Apr 20, 2013 at 9:22 AM, Gregor Martynus
notifications@github.comwrote:

btw, there is also a signin event. Could you describe your use case?
Maybe there's a better way, but I'll look into this anyway


Reply to this email directly or view it on GitHubhttps://github.com//issues/44#issuecomment-16706683
.

@gr2m
Copy link
Member

gr2m commented Apr 20, 2013

here's a list of account events (please give feedback if you'd suggest a name change, now is the time ;-):

  • authenticated
  • cleanup
  • error:unauthenticated
  • password_reset:error
  • passwordreset
  • signin:anonymous
  • signin
  • signout
  • signout
  • signup:anonymous
  • signup

I'd do the following:

hoodie.account.authenticate().then(isLoggedIn, isLoggedOut) 
hoodie.account.on('signin', isLoggedIn)
hoodie.account.on('signout', isLoggedOut)

The reason why you get the authenticated event multiple times is that Hoodie does run hoodie.account.authenticate() itself on startup. But I wonder why you get it 3 times, not twice

@max-mapper
Copy link
Contributor Author

ah so my dilemma is:

when the app starts up it will trigger .on('authenticated') a bunch of
times so I don't use these:

  • hoodie.account.on('authenticated', isLoggedIn)
  • hoodie.account.authenticate().then(isLoggedIn, isLoggedOut)

but if I rely on .signIn/.signOut it will only trigger when the user
signsIn and not when they are already signed in (when the page first loads)

so, this is why I do this (now with more comments):

// check if we are authenticated, but ignore the success callback because
hoodie.account.on('authenticated') is already going to fire
// if we aren't authenticated this is the only way AFAIK that I can get a
callback to fire so I can show the 'you are signed out' UI
hoodie.account.authenticate().then(function noop(){}, isLoggedOut)

// ideally this would only fire once. I have had to change my code so that
it assumes it fires multiple times though
hoodie.account.on('authenticated', isLoggedIn)

// this catches when the user clicks the signout button, but IMO a nicer
solution would be to have hoodie.account.on('unauthenticated') or something
that pairs better with .on('authenticated'). The reason being that I have
no reason to use hoodie.account.on('signin') here since
.on('authenticated') will fire when you sign in AND when you are already
signed in (which is that I want)
hoodie.account.on('signout', isLoggedOut)

@gr2m
Copy link
Member

gr2m commented Apr 20, 2013

your comment again for better readablity, dunno what's happening with @github


ah so my dilemma is:

when the app starts up it will trigger .on('authenticated') a bunch of
times so I don't use these:

  • hoodie.account.on('authenticated', isLoggedIn)
  • hoodie.account.authenticate().then(isLoggedIn, isLoggedOut)

but if I rely on .signIn/.signOut it will only trigger when the user
signsIn and not when they are already signed in (when the page first loads)

so, this is why I do this (now with more comments):

// check if we are authenticated, but ignore the success callback because
hoodie.account.on('authenticated') is already going to fire
// if we aren't authenticated this is the only way AFAIK that I can get a
callback to fire so I can show the 'you are signed out' UI
hoodie.account.authenticate().then(function noop(){}, isLoggedOut)

// ideally this would only fire once. I have had to change my code so that
it assumes it fires multiple times though
hoodie.account.on('authenticated', isLoggedIn)

// this catches when the user clicks the signout button, but IMO a nicer
solution would be to have hoodie.account.on('unauthenticated') or something
that pairs better with .on('authenticated'). The reason being that I have
no reason to use hoodie.account.on('signin') here since
.on('authenticated') will fire when you sign in AND when you are already
signed in (which is that I want)
hoodie.account.on('signout', isLoggedOut)

@max-mapper
Copy link
Contributor Author

ah weird, that was written from gmail, usually that works

@gr2m
Copy link
Member

gr2m commented Apr 20, 2013

The authenticate() method is only to also check the user session that needs to be valid for the sync.

The unauthenticated event (currently called error:unauthenticated) does not only get fired when a user signes out, but also when for whatever reason the backend said that the session is not valid anymore, e.g. session timeout. In this case, the user is still signed in, but his/her data is not synced to remote.

So, my suggestion would be, simply check if the user is currently logged in without checking the session:

if (hoodie.account.username) {
   // user is logged in
} else { 
   // user is not logged in
}

and then, handle only the events when a user intentionally signes in or out, with

hoodie.account.on('signin', isLoggedIn)
hoodie.account.on('signout', isLoggedOut)

And then, there is the case when a user is signedIn as hoodie.account.username is set, but the session is not valid anymore. To handle that event, you can subscribe to

hoodie.account.on('error:unauthenticated', handleUnauthenticated)

does that make sense?

@max-mapper
Copy link
Contributor Author

Ahhhhh, I see. I'll fix my code. Thanks! BTW I stole my code from a readme
somewhere so maybe putting these two lines next to eachother in the 'hello
world' examples would help clarify:

var hoodie  = new Hoodie()
console.log(hoodie.account.username)

@gr2m
Copy link
Member

gr2m commented Apr 20, 2013

Yeah, great idea, cheers! I've made an extra ticket to clarify things in general.

@gr2m gr2m closed this as completed Apr 20, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants