hoodie-plugin-angularjs icon indicating copy to clipboard operation
hoodie-plugin-angularjs copied to clipboard

Username should be cleared on signout

Open ndarilek opened this issue 11 years ago • 9 comments

Currently the code reads:

  hoodie.account.on(eventName, function (username) {
    $rootScope.$apply(function () {
      service.username = username;
    });

My understanding of Hoodie is that hoodie.account.username is defined if a user is logged in, but the signout event is called with the username of the signing-out user. So once you are signed in, there is no way to clear hoodieAccount.username. I'm guessing this code should check for the signout event and set hoodieAccount.username to null in that case.

I'm fairly new to Hoodie, so I may be wrong. I currently have an Angular app that, once I'm logged in, I can't log out of until I reload and get a fresh scope. Things react fine when I log in and username is assigned an initial value, but it never clears even when hoodie:signout fires.

ndarilek avatar Jul 16 '14 01:07 ndarilek

I can confirm this issue. It basically makes it impossible to completely sign out without a page refresh.

davidpfahler avatar Jul 16 '14 16:07 davidpfahler

Ah, didn't get a notification about the other PR for this issue.

I don't really care which gets merged, but this fix is definitely needed ASAP. I too can confirm that clearing username on signout makes my app work as expected.

Thanks!

ndarilek avatar Jul 18 '14 15:07 ndarilek

@ndarilek Ha! Seems like better communication would have saved you some work. Sorry about that. Sometimes GitHub works in mysterious ways.

The real problem is, however, that I cannot run tests. I'm grateful we have this plugin, don't get me wrong, but the repo and process is kind of a mess. Why is there a Gulpfile and a Gruntfile. Either way, non of which worked for me. /cc @elmarburke @robinboehm

davidpfahler avatar Jul 19 '14 09:07 davidpfahler

Hey guys,

great work here! Sorry for the problems with our build system. We removed grunt and now using gulp for everything. ddda91d588ea1fc241e9a5e0b791fb6e1379beec Finished that now.

Is this issue solved by #37 or #38? May you want to create tests for this case together.

<3

robinboehm avatar Jul 22 '14 11:07 robinboehm

@robinboehm #37 is this issue. #38 would fix it, but I cannot test, because running tests is still broken in many ways, please see PR for details.

I will write a test for this change as soon as tests work again. Thanks in advance.

davidpfahler avatar Jul 22 '14 14:07 davidpfahler

Oki, I'll take a look at it as soon as possible.

Robin Böhm

Send from mobile

On 22.07.2014, at 16:15, David Pfahler [email protected] wrote:

@robinboehm #37 is this issue. #38 would fix it, but I cannot test, because running tests is still broken in many ways, please see PR for details.

I will write a test for this change as soon as tests work again. Thanks in advance.

— Reply to this email directly or view it on GitHub.

robinboehm avatar Jul 22 '14 14:07 robinboehm

In progress

elmarburke avatar Aug 04 '14 16:08 elmarburke

Please add @davidpfahler commit for fixing this issue...

:+1:

idanb11 avatar Feb 03 '15 12:02 idanb11

As soon as someone* wrote a test for it.

*) it's on my todo list since august… Feel free to commit

elmarburke avatar Feb 04 '15 14:02 elmarburke