0

I am trying to filter/match a list of returned IDs to a list of JSON data records, but I am struggling with (I believe) my promises and method chaining.

I can get the functions to work, except for when I add step 3 below. Then it resolves without the matching data (the function does carry on and eventually return the correct matching data, but by that time my method has already completed).

This is how it is supposed to work:

  1. (getCompanyBrandProfileIDs) First my method gets a brandProfileID linked to the current user.
  2. (getBrandProfiles) Then it takes the brandProfileID and get all brandProfiles linked to the specific brandProfile.
  3. (getKeywordProfiles) Then it SHOULD take the returned brandProfiles, and get the matching keywordProfile for each brandProfile. It is an array of objects containing a brand_profile_id and and id.

This is my main method:

this.getCompanyBrandProfileIDs = function () {

  var brandProfileIDsToReturn = $q.defer();

  GetUserAccessService.returnBrandProfileID().then(function (brandProfileID) {

      console.log(brandProfileID);

      getBrandProfiles(brandProfileID).then(function (brandProfiles) {

          console.log(JSON.stringify(brandProfiles));

          var keywordProfilesArray = [];

          getKeywordProfiles(brandProfiles).then(function (keywordProfiles) {

              keywordProfilesArray = keywordProfiles;

              console.log(JSON.stringify(keywordProfilesArray));

              //brandProfileIDsToReturn.resolve(keywordProfilesArray);

          });

          brandProfileIDsToReturn.resolve(keywordProfilesArray);

      });

  });

  return brandProfileIDsToReturn.promise;

};

This is the getBrandProfiles method:

function getBrandProfiles(brandProfileID) {

  var getBrandProfilesLinkedToCompany = $q.defer();

  pullSocialMediaData('keyword_profile_brand_profiles.json?brand_profile_id=' + brandProfileID).then(function (brandProfiles) {

      var brandProfilesArray = [];

      brandProfiles.forEach(function (profile) {

          brandProfilesArray.push({ id: profile.id, name: profile.name });

      });

      getBrandProfilesLinkedToCompany.resolve(brandProfilesArray);

  });

  return getBrandProfilesLinkedToCompany.promise;

}

This is the getKeywordProfiles method:

function getKeywordProfiles(brandProfiles) {

      var keywordProfilesToReturn = $q.defer();

      var brandProfilesArray = brandProfiles;

      var array = [];

      brandProfilesArray.forEach(function (profile) {
          findKeywordProfile(profile.id).then(function (keywordID) {

              array.push(keywordID);
          });
          keywordProfilesToReturn.resolve(array);
      })

      return keywordProfilesToReturn.promise;

  }

This is a helper method for getKeywordProfiles:

function findKeywordProfile(brandProfileID) {

  var keywordProfileID = $q.defer();

  pullSocialMediaData('list_keyword_profiles.json').then(function (data) {

      var keywordProfileInstance = data.filter(function (keyword) {
          return keyword.brand_profile_id === brandProfileID;
      });

      keywordProfileID.resolve(keywordProfileInstance[0].id);

  });

  return keywordProfileID.promise;
}

I would appreciate your assistance!

onmyway
  • 1,435
  • 3
  • 29
  • 53

2 Answers2

1

You are resolving the brandProfileIDsToReturn too soon. In this code: when you resolve the promise the then callback will not have been called, so keywordProfilesArray is guaranteed to be empty.

this.getCompanyBrandProfileIDs = function () {
  var brandProfileIDsToReturn = $q.defer();
  GetUserAccessService.returnBrandProfileID().then(function (brandProfileID) {
      console.log(brandProfileID);
      getBrandProfiles(brandProfileID).then(function (brandProfiles) {
          console.log(JSON.stringify(brandProfiles));
          var keywordProfilesArray = [];
          getKeywordProfiles(brandProfiles).then(function (keywordProfiles) {
              keywordProfilesArray = keywordProfiles;
              console.log(JSON.stringify(keywordProfilesArray));
              //brandProfileIDsToReturn.resolve(keywordProfilesArray);
          });
          brandProfileIDsToReturn.resolve(keywordProfilesArray);
      });
  });
  return brandProfileIDsToReturn.promise;
};

Simply moving the resolve() call inside the then callback should fix it and in fact you have that line commented out, so uncomment it and remove the other resolve:

this.getCompanyBrandProfileIDs = function () {
  var brandProfileIDsToReturn = $q.defer();
  GetUserAccessService.returnBrandProfileID().then(function (brandProfileID) {
      console.log(brandProfileID);
      getBrandProfiles(brandProfileID).then(function (brandProfiles) {
          console.log(JSON.stringify(brandProfiles));
          var keywordProfilesArray = [];
          getKeywordProfiles(brandProfiles).then(function (keywordProfiles) {
              keywordProfilesArray = keywordProfiles;
              console.log(JSON.stringify(keywordProfilesArray));
              brandProfileIDsToReturn.resolve(keywordProfilesArray);
          });
      });
  });
  return brandProfileIDsToReturn.promise;
};

However you can probably simplify the code a lot if you stop using $q.defer(). Your functions already return promises so just return the promises they use and stop trying to create additional promises. I think this is equivalent to the previous code except it returns the promises directly, and I removed the log messages, and that means the getKeywordProfiles call simplifies down to a callback that just calls the function so you can pass the function directly:

this.getCompanyBrandProfileIDs = function () {
  return GetUserAccessService.returnBrandProfileID().then(function (brandProfileID) {
      return getBrandProfiles(brandProfileID).then(getKeywordProfiles);
      });
  });
};

and then you can simplify it further by extracting the inner .then:

this.getCompanyBrandProfileIDs = function () {
  return GetUserAccessService.returnBrandProfileID()
  .then(getBrandProfiles)
  .then(getKeywordProfiles);
};

The getKeywordProfiles() function also needs to avoid resolving its promise until all of the findKeywordProfile() calls have resolved. Return a promise for the array of promises and when they resolve the promise will complete to an array of values:

function getKeywordProfiles(brandProfilesArray) {
      var array = [];
      brandProfilesArray.forEach(function (profile) {
          array.push(findKeywordProfile(profile.id));
      })
      return $q.all(array);
  }

To clarify my comments about $q, there are some cases where you need to create a promise from scratch using it, but they're fairly uncommon. Anything that happens asynchronously in Angular already returns a promise, and the great thing about promises is that they chain together, so when you have one promise calling .then() or .catch() will return a new one. Also the .then() callback can either return a value which resolves the new promise, or can return a promise which will only resolve the new promise when it, itself resolves. So just keep chaining the .then() calls together and each will wait for the previous one to complete.

$q is still useful though: $q.all() if you want to wait for a bunch of promises to all resolve, $q.race() if you have a bunch of promises and only one needs to resolve, $q.resolve(value) can also be useful as sometimes you just want a promise that will resolve immediately so you can hang a chain of other async functions off it.

Also it is safe to keep a promise around even long after it has resolved and you can still call .then() on it: this is useful if you have asynchronous initialisation code and events that may or may not be triggered before the initialisation has completed. No need to do if(isInitialised) when you can just do initPromise.then(...)

Duncan
  • 92,073
  • 11
  • 122
  • 156
  • Wow. I cannot believe how much you simplified the code. Thank you! Can you perhaps give me insight re not using $q? Does Angular manage the promises automatically? – onmyway Feb 27 '17 at 13:04
  • 1
    I added some more comments about `$q` but search around and there are some great articles on how to use promises effectively. – Duncan Feb 27 '17 at 13:54
  • Thank you for the additional insight - it is really valuable! – onmyway Feb 27 '17 at 15:49
1

In getKeywordProfiles function you need resolve it when array loop finished.

  function getKeywordProfiles(brandProfiles) {

      var keywordProfilesToReturn = $q.defer();

      var brandProfilesArray = brandProfiles;

      var array = [];

      brandProfilesArray.forEach(function (profile) {
          findKeywordProfile(profile.id).then(function (keywordID) {

              array.push(keywordID);
          });
          //--
          //keywordProfilesToReturn.resolve(array);
      })
      //++
      keywordProfilesToReturn.resolve(array);
      return keywordProfilesToReturn.promise;
  }

Info: I think you need to create an profileIdArray push all brandProfileID and send to your findKeywordProfile function. It will be more useful.

hurricane
  • 6,521
  • 2
  • 34
  • 44