Urigo / angular-meteor

Angular and Meteor - The perfect stack
https://www.angular-meteor.com/
MIT License
2.36k stars 622 forks source link

(v1.3.4) $auth.currentUser doesn't seem to work #1064

Closed eugenious closed 8 years ago

eugenious commented 8 years ago

I put some comments in a closed issue which I probably shouldn't have done:

968

I'm having trouble getting $auth.currentUser to work, I'm not sure if it's a bug or I'm doing something wrong.

I've copy-pasted here the relevant bits:

Thanks for the great work. I think this is heading in the right direction. If I understand correctly, I should be able to use the $auth service to replace the Meteor global variable. That sounds like a good approach, so here's what I did:

This is what I had previously (it works fine):

    controller: function($scope) {
      $scope.helpers({
        isLoggedIn() {
          return Meteor.userId() !== null;
        }
      });
    }

Using the new $auth service:

    controller: function($scope, $auth) {
      $scope.helpers({
        isLoggedIn() {
          return angular.isDefined($auth.currentUser);
        }
      });
    }

I must be missing something, because I can't get this to work:

By the way, and this might not be related to the issue, I noticed that $scope.$auth.currentUser is returning the correct user (reactively), however I didn't even expected $scope.$auth to be defined at all since I hadn't defined it.

What am I doing wrong?

dotansimha commented 8 years ago

@eugenious Hi, seems like you are right, I think that a bug causes that $auth.currentUser is only exposed to the view (in $scope.$auth object). I am checking it now and will update later.

Urigo commented 8 years ago

@eugenious why would you need to use $auth.currentUser in scope helpers and not just Meteor.user ?

MilosStanic commented 8 years ago

I confirm that $auth.currentUser is only exposed to view, but not to controller. In controller it's undefined. Is this going to be fixed in next release?

eugenious commented 8 years ago

@Urigo I was thinking that using $auth was better than relying on a global Meteor object. (For ease of unit testing, for example). To quote the documentation:

$auth wraps Meteor's API for ease of use with AngularJS applications

So I thought I would try it because further down on the page, the documentation reads:

$auth.currentUser is a property of the $auth service, which contain the current logged in user in every time.

But I couldn't get it to work.

sean-stanley commented 8 years ago

+1 Meteor.user() did work though

DAB0mB commented 8 years ago

Here are three alternatives to do so:

function MyCtrl($scope) {
  $scope.helpers({
    isLoggedIn() {
      return !!Meteor.user();
    }
  });
}

function MyCtrl($scope) {
  $scope.helpers({
    isLoggedIn() {
      return !!$scope.$auth.currentUser;
    }
  });
}

function MyCtrl($reactive, $scope) {
  $reactive(this).attach($scope);

  this.helpers({
    isLoggedIn() {
      return !!this.auth.currentUser();
    }
  });
}

There is no need to define currentUser on $auth service since you can get it using Meteor.user.

Urigo commented 8 years ago

Thanks @DAB0mB

sean-stanley commented 8 years ago

Hang on, can helpers return non cursor values? The documentation says that a helper function must return a MongoDB cursor. Maybe it should be updated if the suggested workarounds are best practice methods.

On Sat, 16 Jan 2016 at 06:04 Uri Goldshtein notifications@github.com wrote:

Thanks @DAB0mB https://github.com/DAB0mB

— Reply to this email directly or view it on GitHub https://github.com/Urigo/angular-meteor/issues/1064#issuecomment-172016995 .

eugenious commented 8 years ago

@DAB0mB, these are all reasonable workarounds, however they are not ideal because:

  1. relies on a global Meteor variable, not great
  2. relies on a $scope.$auth variable, which as far as I can tell, isn't documented and possibly shouldn't even be there. ($auth is a service, not a variable automatically added to the root scope??, see clarifying comment below)
  3. this one is kinda funky

As far as I know, Angular services are not automatically put on the scope, it's up to the application developer to decide how to consume the service, either in a controller, in a directive's link function, within a state resolve function, etc. This means that many of the template examples given in the documentation won't work without a little piece of glue code, something like:

function MyCtrl($scope, $auth) {
  $scope.$auth = $auth;
}

@Urigo If you don't plan to fix this then I think the documentation needs to be updated as it's misleading right now. It says $auth.currentUser is available, but it's not.

MilosStanic commented 8 years ago

@Urigo @DAB0mB I completely agree with @eugenious .

The point of the $auth service is having the universal source of truth when it comes to testing whether user is logged in and what are his/her properties. Therefore, existence of $auth.currentUser available through $auth service is very useful.

Finally, let me quote the docs:

$auth service is a part of angular-meteor-auth package, and it located inside angular-meteor.auth AngularJS module.

This service provides the functionality for authentication for your app.

$auth wraps Meteor's API for ease of use with AngularJS applications, and also provides the ability to access Meteor's Accounts package from the UI. ... $auth.currentUser is a property of the $auth service, which contain the current logged in user in every time.

It automatically changes when the user logs in or logs out, and it can be used from the view or the AngularJS controller/components.

Things should be exactly as the above quoted sections of docs.

Thanks again.

eugenious commented 8 years ago

A small idea, now that I think about it, maybe currentUser shouldn't be a property but rather a function, thus:

$auth.currentUser() is a getter function of the $auth service
MilosStanic commented 8 years ago

In my opinion, it would be better if it were a reactive property. For example, what if the user record contains user's online status. I want to be able to reactively check on that. In fact, there can be both a getter function and a reactive property.

mattiLeBlanc commented 8 years ago

Hi, I might have a related issue that my controller's constructor is not consistent in resolving the currentUser. Some reloads I get a current user, and some it is undefined. It must be a timing issue of the Users subscription but I am using several approaches like promise, Reactive variable and a resolve( that's currentUser dep) on the router. All display the same behaviour.

See console output:

globalController currentuser undefined
profile.information.controller.ts:25 $auth -> undefined
profile.information.controller.ts:36 > currentUser undefined
profile.information.controller.ts:29 $auth wait undefined

My constructor:

  constructor( private $log: angular.ILogService, private $state: angular.ui.IStateService, private $scope: any, private currentUser: any, private toastr: Toastr, $reactive: any, private $auth: any  ) {

    $reactive(this).attach($scope);
    var user: any;
    console.log( '$auth ->', $auth.currentUser);

    $auth.waitForUser().then(() => {
        let currentUser = $auth.currentUser;
        console.log( '$auth wait', currentUser );
      });
    this.autorun( () => {

      user = this.getReactively( 'currentUser' );
    });
    // var user = currentUser;
    console.log('> currentUser', user );
    if ( user ) {
      this.information = user.profile;
      this.information.companyAbn = '123 456 734';
    }
  }

So I am trying multiple things in there so that's why its messy.

Just another reload of the page and check console output:

 globalController currentuser Object {_id: "bATDodgxdCFn6tsBN", emails: Array[1], profile: Object}
profile.information.controller.ts:25 $auth -> undefined
profile.information.controller.ts:36 > currentUser Object {_id: "bATDodgxdCFn6tsBN", emails: Array[1], profile: Object}
profile.information.controller.ts:29 $auth wait undefined

Any idea;s? do I need to apply a window.setTimout 0 for now?

Urigo commented 8 years ago

should be fixed with the new 1.3.6 release. Please re-open and tag me with a reproduction project if it's not.

mattiLeBlanc commented 8 years ago

@urigo new issue, see: https://github.com/Urigo/angular-meteor/issues/1126#issuecomment-187115397

sean-stanley commented 8 years ago

Hi, I found some new stuff for you, i think you're gonna like it, more info at

Cheers, Sean Stanley

mattiLeBlanc commented 8 years ago

is this posted by a bot or is this is real URL?

dapids commented 8 years ago

@mattiLeBlanc,

Guess it's a spambot. :/

Urigo commented 8 years ago

I've edited the message so people won't click it..

Sorry for all the people who wanted massive weight loss...

hanselke commented 7 years ago

Hi,

ive just recently upgraded from meteor 1.2 > 1.5.6 and in the process... have moved all of my bower dependencies as well some meteor packages all to meteor npm.

App was working after the upgrade and i decided to clean up the mess thats caused by multiple ways of getting currentUser now, ie.. Meteor.users(), $q , $scope.currentUser and thought the $auth package was the solution.

Essentially, the problem is after including the $auth controller, $auth.currentUser isnt defined in my angular controllers.

I looked at the angular-meteor docs and see that 1.3.11 lists $auth 0.3.0

Problem: at https://github.com/Urigo/angular-meteor-auth there isnt any 0.3.x tags, only 0.2.x then 1.x. I'm on 1.1.1.

Which version should I be using?