How to correctly chain conditional(?) promises with Q.js
Asked Answered
A

3

16

I've still not quite got a complete understanding of promises so apologies if this is a simple misunderstanding.

I have a function for deleting an item on a page but I have a specific behaviour depending on the state of the page. Psuedo code-wise it's something like this:

Does the page have changes?
    If yes - prompt to save changes first
         If yes - save changes
         If no - exit function
    If no - continue
Prompt to confirm delete
    If yes - delete item and reload data
    If no - exit function

Hopefully that makes sense. Essentially if there are changes, the data must be saved first. Then if the data has been saved, or if there were no changes to begin with, prompt the user to confirm deletion. The problem is I'm using durandal and breeze, and I can't seem to chain the promises they return together correctly.

My function currently looks like this, which I know is wrong, but I'm struggling to work out where to fix it.

if (this.hasChanges()) {
    app.showMessage('Changes must be saved before removing external accounts.  Would you like to save your changes now?', 'Unsaved Changes...', ['Yes', 'No'])
        .then(function (selectedOption) {
             if (selectedOption === 'Yes') {
                 return this.save();
             } else {
                 Q.resolve()
             }
         });
}
app.showMessage('Are you sure you want to delete this item?', 'Delete?', ['Yes', 'No'])
    .then(function (selectedOption) {
        if (selectedOption === 'Yes') {
            item.entityAspect.setDeleted();
            datacontext.saveChanges()
                .then(function () {
                    logger.logNotificationInfo('Item deleted.', '', router.activeInstruction().config.moduleId);
                    Q.resolve(this.refresh(true));
                }.bind(this));
            }
       }.bind(this));

The app.showMessage call from durandal returns a promise, then the this.save returns a promise, and finally the this.refresh also returns a promise.

So I guess I need to check the hasChanges, then if necessary call save, and resolve it. Then after that conditional section has finished resolving, call the second prompt, and then resolve all the promises within that.

I'm sorry I don't think this is super clear, but that's also I think coming from the fact I'm not completely following the chains here.

Any help much appreciated! Thanks.

Apraxia answered 21/10, 2013 at 22:45 Comment(0)
S
11

Kris is correct. You won't need any of the Q.resolve calls.

Btw, returning a promise with resolved value true or false is meaningless in your situation. I fear you are under the mistaken impression that returning false would prevent the chained then() from being called. Not so! A resolved promise with a value of false is still a good promise ... as seen in the following code which triggers the alert message box:

Q(false) // same as Q.resolve(false)
 .then(function () { alert('resolve(false) triggered then()') })

If you want to put the promise in a failed state (and you don't care about the error value), you should return Q.reject().


I don't know what this is in your code but it's going to be nothing but trouble as you execute the inner functions. Capture it in a variable so you don't get lost and struggle with compensating bind(this) logic.


I'm not entirely sure what you're trying to do. It appears that you won't proceed with deleting an item while there are unsaved changes. You'll save unsaved changes if the user OKs that. Then you'll ask the user to confirm the delete. If the user refuses to save pending changes, you should not even begin the delete process.

If I understand correctly, I think you want something like this:

var self = this; // WHAT IS THIS? I don't know but capture it as 'self'

function saveBeforeDeleting() {
  return saveIfNeeded().then(deleteIfConfirmed);
}

function saveIfNeeded() {
  // no need to save; return resolved promise
  if (!self.hasChanges()) return Q();

  var dialogPromise = app.showMessage(
    'Changes must be saved before removing external accounts. '+
    'Would you like to save your changes now?', 
    'Unsaved Changes...', ['Yes', 'No']
  );

  // When the user replies, either save or return a rejected promise
  // (which stops the flow)
  return dialogPromise.then(function (selectedOption) {
    return (selectedOption === 'Yes') ? self.save() : Q.reject();
  });
}

function deleteIfConfirmed() {
  var dialogPromise = app.showMessage(
    'Are you sure you want to delete this item?', 
    'Delete?',
    ['Yes', 'No']
  );

  return dialogPromise.then(function (selectedOption) {
    return (selectedOption === 'Yes') ? deleteIt() : Q.reject();
  });

  function deleteIt() {
     item.entityAspect.setDeleted();
     return datacontext.saveChanges().then(logAndRefresh);
  }

  function logAndRefresh() {
     logger.logNotificationInfo(
       'Item deleted.',
       '', 
       router.activeInstruction().config.moduleId
     );
     return self.refresh(true));
  }
}

Obviously I haven't tested this code. Think of it as inspiration.

Statampere answered 22/10, 2013 at 9:32 Comment(2)
I've marked this as the answer because I basically used all of this code, but everyone's answer was essentially correct as far as I could see.Apraxia
With regards to the this convention, I know the self 'pattern' is how it's supposed to be done, but unfortunately I found a situation where I couldn't get the self and closures to work correctly. Then I fixed it after finding out about bind, and it just stuck because it worked. Not great.Apraxia
C
7

If you throw an error in a promise, the process will jump straight to the first .fail/.catch handler skipping any .thens() in between.

function AbortError() {}

MyClass.prototype.delete = function() {
    var p = Q();
    var self = this;
    if( this.hasChanges() ) {
        p = app.showMessage('...', '...', ['Yes', 'No'])
        .then(function(answer){
            if( answer === "Yes") {
                return self.save(); //I assume save returns a promise
            }
            throw new AbortError();
        });
    }
    return p
    .then(function() {
        return app.showMessage('...', '...', ['Yes', 'No'])
    })
    .then(function(answer) {
        if( answer === "yes") {
            item.entityAspect.setDeleted();
            return datacontext.saveChanges();
        }
        throw new AbortError();
    })
    .then(function(){
        logger.logNotificationInfo('Item deleted.', '', router.activeInstruction().config.moduleId);
        self.refresh(true);
    })
    .fail(function(e){
        //kris please provide typed .catch feature :(
        if( !(e instanceof AbortError) ) {
            throw e;
        }
    });
};
Consumptive answered 22/10, 2013 at 13:23 Comment(0)
P
2

In general you want to create functions to do your work that ALWAYS return a promise, even if that is an immediately resolved one, i.e. "return Q.resolve(someData)".

So I'd try something like the following. Note the extra "return" statements below.

function complexSave() {
   return saveIfNeeded().then(confirmDelete);
}

// returns a promise
function saveIfNeeded() {
  if (this.hasChanges()) {
    return app.showMessage('Changes must be saved before removing external accounts.  Would you like    to  save your changes now?', 'Unsaved Changes...', ['Yes', 'No']).
      then(function (selectedOption) {
         if (selectedOption === 'Yes') {
             return this.save();
         } else {
             return Q.resolve(false)
         }
     });
  else {
    return Q.resolve(false);
  }
}

// returns a promise
function confirmDelete() {
  return app.showMessage('Are you sure you want to delete this item?', 'Delete?', ['Yes', 'No'])
    .then(function (selectedOption) {
       if (selectedOption === 'Yes') {
          item.entityAspect.setDeleted();
          return datacontext.saveChanges()
            .then(function () {
                logger.logNotificationInfo('Item deleted.', '', router.activeInstruction().config.moduleId);
                return Q.resolve(this.refresh(true));
            }.bind(this));
        } else {
          return Q.resolve(false);
        }
   }.bind(this));
}
Pram answered 21/10, 2013 at 23:39 Comment(2)
All of these Q.resolve calls are unnecessary. Any value can be returned from a promise handler and will automatically be wrapped.Unsegregated
horrible promisey pyramid of doomArbiter

© 2022 - 2024 — McMap. All rights reserved.