0

I'm having an issue setting a property in my scope. It does set but only after the second time I click the button that is tied to my controller.

app.controller('ContactsController',  ['$scope','$window','$http','googleService', function($scope, $window, $http, googleService) {

 $scope.login = function () {
  var emails =[];
  var promise =  googleService.login()
      promise.then(function (data) {
        console.log(data[0]);
        gapi.client.load('gmail', 'v1', function() {
            var request = gapi.client.gmail.users.messages.list({'userId' :'me',
            labelIds: ['INBOX'],
            });
            request.execute(function(resp) {
            $scope.emails  = googleService.makeEmails(resp);             
            });
        });                                                                                 
       }

      , function (err) {
        console.log('Failed: ' + err);

      });   
   };
}]);

And here is my method in my service.

this.makeEmails = function(resp){
 var factory = [];
    angular.forEach(resp.messages, function(message){
            var newEmail = gapi.client.gmail.users.messages.get({'userId': 'me','id': message.id ,'format':'metadata', 'metadataHeaders': ['subject','from','to','date']});
                newEmail.execute(function(resp) {
                    var emailw = resp;

                    factory.push(emailw);
                });
        });
        return factory;
 }

So I click the button that is tied to the click() method in the controller and in the network tab I see all the responses come through and when I step through the code factory is being set. I just can't figure out why the first click scope isn't set but the second click sets it.

nicael
  • 18,550
  • 13
  • 57
  • 90
user3298823
  • 302
  • 2
  • 17
  • Most likely has something to do with the digest because it looks like you're not using AngularJS functionality to carry out your async operations. Could you try a manual digest cycle after setting the scope value? – JLRishe Mar 29 '15 at 19:23
  • Do you mean using the $http method and build it out myself rather than use the google provided method? – user3298823 Mar 29 '15 at 19:54
  • No, I mean replace `$scope.emails = googleService.makeEmails(resp);` with [`$scope.$apply(function () { $scope.emails = googleService.makeEmails(resp); });`](http://www.sitepoint.com/understanding-angulars-apply-digest/) – JLRishe Mar 29 '15 at 20:06
  • Yeah, it doesn't like that either. I'm really new to angular so this is confusing! No idea why it would only populate scope on the second click. – user3298823 Mar 29 '15 at 20:32
  • This might help. http://jimhoskins.com/2012/12/17/angularjs-and-apply.html – Nikhil Bhandari Mar 29 '15 at 21:10
  • It looks like its returning factory before the foreach loop even starts. Is there a way to make sure it only returns the variable after the loop finishes? – user3298823 Mar 29 '15 at 21:12
  • @NikhilBhandari I don't think it's with the controller, I think it's with my service. When I step through it, factory gets hit and returned THEN the angular loop starts and adds stuff to factory but then never rereturns the factory. – user3298823 Mar 29 '15 at 21:30

2 Answers2

2

you'll have to use promise/deferred pattern using $q

https://docs.angularjs.org/api/ng/service/$q

something like this

//service
this.makeEmails = function (resp) {
    //create a deferred/ promise object
    var deferred = $q.defer();
    var tasks = [];

    //create an array of promises
    angular.forEach(resp.messages, function (message) {
        tasks.push(function () {
            var deferred = $q.defer();
            var newEmail = gapi.client.gmail.users.messages.get({
                'userId': 'me',
                'id': message.id,
                'format': 'metadata',
                'metadataHeaders': ['subject', 'from', 'to', 'date']
            });
            newEmail.execute(function (resp) {
                deferred.resolve(resp);
            });
        });
    });

    //return the result when all the promises get executed
    $q.all(tasks).then(function(result){
        deferred.resolve(result);
    })
    return deferred;
};

//controller
googleService.makeEmails(resp).then(function(result){
    $scope.emails = result;
});
Nikhil Bhandari
  • 1,584
  • 2
  • 16
  • 34
  • Thanks, I actually ended up breaking it apart and doing that. Though now when it finishes, only the one element gets rendered haha. I guess I need to figure out why the ng-repeat isn't working. – user3298823 Mar 29 '15 at 22:25
2

Important to note: the accepted answer here is using a discouraged practice known as the deferred antipattern. You should avoid doing so as it leads to verbose code that does not handle errors properly.

I strongly urge you to use the following approach, which is cleaner and more robust:

//service
this.makeEmails = function (resp) {
    var promises = resp.messages.map(function (message) {
        return $q(function (resolve) {
            gapi.client.gmail.users.messages.get({
                'userId': 'me',
                'id': message.id,
                'format': 'metadata',
                'metadataHeaders': ['subject', 'from', 'to', 'date']
            }).execute(resolve);
        });
    });

    return $q.all(promises);    
};

//controller
googleService.makeEmails(resp).then(function(result){
    $scope.emails = result;
});
Community
  • 1
  • 1
JLRishe
  • 99,490
  • 19
  • 131
  • 169
  • Lets say I've built objects function, could I return those instead of the response? Can I also build an array of promises with and return that instead? Here's a jfiddle if you want to take a look, it has a few comments but not too many. I'm just not sure how to return the array of objects with these promises and make it work properly. It works now but doesn't update scope until I take a look at it after the login() method is finished. https://jsfiddle.net/bLqujtrv/ – user3298823 Mar 30 '15 at 02:13
  • I agree your method is a little compact but how is it better? Does adding more promises mean an anti pattern? or not using built in function over angular.forEach? – Nikhil Bhandari Mar 30 '15 at 04:50
  • @NikhilBhandari Did you follow the link I provided? Bergi's answer is particularly good. The unnecessary use of `$q.defer()` is what makes it an anti-pattern. – JLRishe Mar 30 '15 at 05:18
  • @user3298823 While you certainly _can_ return an array of promises, the usual thing to do would be to return a single promise for an array of values. Your approach in that fiddle was problematic, and in particular your use of `$scope.$apply()` was completely wrong. Please try this: https://jsfiddle.net/g7y2cbm9/ – JLRishe Mar 30 '15 at 05:48
  • @JLRishe Thanks I'll try this out when I get home later today. I had to look up how you are using .map, that's cool that it will do this code for every element of the array. Never seen that before. – user3298823 Mar 30 '15 at 14:24
  • @NikhilBhandari Thanks! I can't in good conscience upvote an answer that uses practices I recommend against, but I have upvoted two of your other answers that I thought were good. Have a nice day! – JLRishe Mar 30 '15 at 18:24
  • @JLRishe I tried the code and it worked perfectly and helped me to understand how to do this kind of thing in lots of other places! Thanks! – user3298823 Mar 31 '15 at 15:41
  • @JLRishe Thanks for the upvotes, but that was not required. :D – Nikhil Bhandari Mar 31 '15 at 17:35