Using the $q service in AngularJS to handle promises

Question

I have a service which needs to store some settings data in a DB and at the same time maintain a reference to that in the user'slocalStorage (we want two users logged in to the same account to be able to view them differently). This method looks for an existing previewSession in the localStorage and if not found, creates one and generates a unique URL.

As you can see at the beginning, I'm using the $q service to return a promise and then resolving that promise as and when all operations have finished. There is one use of the $http service and the save call is on a $resource object.

Is this a proper way to handle such a use-case, or could I have done it better?

To summarize the requirements:

1. Method gets called with a settings parameter
2. Method must always return a URL (unless an error occurs on the db level or some such thing)

var PREVIEW_SESSION_KEY_BASE = '<a constant string>'; 
function getPreviewSessionUrl(settings) {
 var deferred = $q.defer();
 $http.get('/' + appCode + '/reservedDomain',
 function(appResponse) {
 $localForage
 .getItem(PREVIEW_SESSION_KEY_BASE + themeId)
 .then(function(data) {
 if (data) {
 deferred.resolve(appResponse + '?<a query key>=' + data);
 } else {
 previewSessionResource.save(
 {},
 {
 settings: settings
 },
 function(previewResponse) {
 $localForage
 .setItem(PREVIEW_SESSION_KEY_BASE + themeId, previewResponse)
 .then(function(data) {
 deferred.resolve(appResponse + '?<a query key>=' + data);
 });
 },
 function(error) {
 deferred.reject(error);
 }
 );
 }
 });
 },
 function(error) {
 deferred.reject(error);
 }
 );
 return deferred.promise;
 }

• Madara's Ghost: Not much of a review, but simply the fact that you have a pyramid of doom style code, implies that you're using promises wrong.
• BadgerBadgerBadgerBadger: True. I completely disregarded chaining of the then calls. Thank you!

Answer 1

Things that could have been better;

• This:

   }, 
   function(error) {
   deferred.reject(error);
   }
   );
  

  could simply be

   }, deferred.reject ); 
  

  reject is a function that takes error as a parameter after all..

• I would have externalized the previewSessionResource.save(call, it makes the reading too hard. You only capture settings and appResponse which you would then provide as parameters.
• If you control the signature of previewSessionResource.save, then I would simply request for settings as an object instead of requesting for { settings: settings } to keep it simple, it does not make sense to wrap a single data point into an object.
• I am also idly wondering if previewSessionResource.save should not be responsible for calling $localForage.setItem(), would you ever call previewSessionResource.save without calling $localForage.setItem()?

On the whole, I think this is okay, it just needs some rejiggering to read better.

• BadgerBadgerBadgerBadger: Thanks @konijn! Yeah, readability is the main issue here. I'll refactor the code, accordingly.