Jquery When and Deferred object, broken function flow

Question

I am using $.when and .done to make sure that the close window happens after the data is saved. But, this doesn't seem to work as expected.

The workflow is that, user clicks on a button "Save and Close", which should save the data first, trigger print and close the window. But the save data and close window happens at the same time which makes the print fail.

I have read about when..then and deferred object. Tried to implement it here the following code, sometimes it work but most of the time it would break.

$("#btnSaveAndClose").click(function (event) {
    $.when(zSaveSomeData()).done(function (value) {
        zCloseMyWindow();
    });
});

function zSaveSomeData() {
    return zSaveMasterData(masterdata, function () {  
        return zSaveDetailData();
    });
};

function zSaveMasterData(masterdata, fnAfterSave) {
    return $.ajax({
        type: 'POST',
        contentType: 'application/json',
        url: '/api/masterdata/',
        data: JSON.stringify(masterdata),
        success: function (data) {
            fnAfterSave();
        }
    });
};

function zSaveDetailData() {
    var selectedDataGroups;
    // some logic here

    zSaveDetails(selectedDataGroups);

};

function zSaveDetails(selectedDataGroups) {
    var deferred = $.Deferred();
    $.ajax({
        type: 'POST',
        contentType: 'application/json',
        url: '/api/detaildata/',
        data: JSON.stringify(selectedDataGroups),
        success: function (data) {
            var printableGroupIDs = [];
            $.each(data, function () {
                if (this.IsPrintable)
                    printableGroupIDs.push(this.ID);
            });

            if (printableGroupIDs.length > 0) {
                zPrintGroups(printableGroupIDs);
            }
            deferred.resolve('done');
        }
    });

    zAuditSave();
    return deferred.promise();
};

function zPrintGroups(newGroupIDs) {
    // calls external program to print groups

};

function zCloseWindow() {
    window.close();
};

function zAuditSave() {
    $.ajax({
        type: 'POST',
        contentType: 'application/json',
        url: '/api/audit'
        success: function (data) {

        }
    });
};

Only thing is that the save calls other methods inside to same master and details data. There are couple of ajax calls too. An unusual thing is that after the data is saved, there is a call to VB code that actually triggers a Print. I am so confused on why would close window fire before the other methods are executed. Any help would be appreciated.

@Bergi, thanks for pointing that out, actually that code was left over from my edit. It is not there. I added return to test but did clean the code. — Sri Reddy, Apr 13 '15 at 18:58
`zAuditSave()` is another method that call a webApi to audit save. I will add that method too. — Sri Reddy, Apr 13 '15 at 19:00

score 2 · Answer 1 · edited Apr 14 '15 at 01:09

For me the code is overly divided into functions, with some doing little more than fronting for others.

I would prefer to see the click handler as a comprehensive master routine which sequences three promise-returning functions zSaveMasterData(), zSaveDetails() and zAuditSave(), then closes the window. Thus, some of the current functions will be subsumed by the click handler.

$("#btnSaveAndClose").click(function(event) {
    zSaveMasterData(masterdata).then(function() {
        var selectedDataGroups;
        /* some logic here */
        var detailsSaved = zSaveDetails(selectedDataGroups).then(function(data) {
            var printableGroupIDs = $.map(data, function (obj) {
                return obj.IsPrintable ? obj.ID : null;
            });
            if (printableGroupIDs.length > 0) {
                // calls external program to print groups
            }
        });
        // Here, it is assumed that zSaveDetails() and zAuditSave() can be performed in parallel.
        // If the calls need to be sequential, then the code will be slightly different.            
        return $.when(detailsSaved, zAuditSave());
    }).then(function() {
        window.close();
    });
});

function zSaveMasterData(masterdata) {
    return $.ajax({
        type: 'POST',
        url: '/api/masterdata/',
        contentType: 'application/json',
        data: JSON.stringify(masterdata),
    });
};

function zSaveDetails(selectedDataGroups) {
    return $.ajax({
        type: 'POST',
        contentType: 'application/json',
        url: '/api/detaildata/',
        data: JSON.stringify(selectedDataGroups)
    });
};

function zAuditSave() {
    return $.ajax({
        type: 'POST',
        contentType: 'application/json',
        url: '/api/audit'
    });
};

Note the returns in the three functions with ajax calls. These returns are vital to the sequencing process.

A potentially bigger issue, not addressed in the question (nor in this answer) is how to recover from errors. Presumably, the database will be inconsistent if the sequence of saves was to fail part way through. It may well be better to ditch this client-side sequencing approach in favour of a server-side transaction that the client sees as a single operation.

@Bergi, none of my code is tested and I could easily have made a slip. From memory, `jQuery.map()` differs from `Array.prototype.map()` in that the jQuery version (a) filters out an element on null return, and (b) returns a jQuery-wrapped array (hence the need for .get()). — Roamer-1888, Apr 14 '15 at 00:07
Yeah, it should've been named `concatMap` for its treatment of `null` and array values. But actually [`$.map(arr, fn)`](http://api.jquery.com/jquery.map/) does return arrays, while `$(arr).map(fn)` returns the jquery wrapper. — Bergi, Apr 14 '15 at 00:27
Ah OK, I should have looked it up in the first place. Documentation also says that "within the function, `this` refers to the global (window) object". Whoops, edited. Thanks for the catch. — Roamer-1888, Apr 14 '15 at 01:07

score 0 · Answer 2 · answered Apr 13 '15 at 19:04

The problem here is your code doesn't depend on when fnAfterSave() has completed.

Short answer: don't mix success methods, callbacks, and promises - use one pattern and stick to it - and the easiest pattern to use is promises.

$("#btnSaveAndClose").click(function (event) {
    zSaveSomeData().then(function() { zCloseMyWindow(); });
});

function zSaveSomeData() {
    return zSaveMasterData(masterdata).then(function(data) { zSaveDetailData() });
};

function zSaveMasterData(masterdata) {
    return $.ajax({
        type: 'POST',
        contentType: 'application/json',
        url: '/api/masterdata/',
        data: JSON.stringify(masterdata)
    });

    //remove success callback here as it breaks the chaining
};

score 0 · Answer 3 · edited May 23 '17 at 10:10

0

It seems like your problem is that you are doing asynchronous things inside an ajax success callback. The promise returned by $.ajax still resolves immediately after the response is received - and executes your done callback before the asynchronous zSaveDetailData() has finished.

So, to chain asynchronous actions, always use then. Use it even for synchronous actions, it makes the sequence clear.

Don't use success callbacks when you're working with promises. You also don't need deferreds. You might want to have a look at these generic rules as well, especially that you never must forget to return promises from async functions that you want to await.

$("#btnSaveAndClose").click(function (event) {
    zSaveSomeData().then(zCloseMyWindow);
});

function zSaveSomeData() {
    return zSaveMasterData(masterdata).then(zSaveDetailData);
}

function zSaveMasterData(masterdata) {
    return $.ajax({
        type: 'POST',
        contentType: 'application/json',
        url: '/api/masterdata/',
        data: JSON.stringify(masterdata),
    });
}

function zSaveDetailData() {
    var selectedDataGroups;
    // some logic here

    return zSaveDetails(selectedDataGroups);
//  ^^^^^^
}

function zSaveOrderGroups(selectedDataGroups) {
    return $.ajax({
//  ^^^^^^
        type: 'POST',
        contentType: 'application/json',
        url: '/api/detaildata/',
        data: JSON.stringify(selectedDataGroups)
    }).then(function(data) {
//    ^^^^^^^^^^^^^^^^^^^^^^
        var printableGroupIDs = [];
        $.each(data, function () {
            if (this.IsPrintable)
                 printableGroupIDs.push(this.ID);
        });
        if (printableGroupIDs.length > 0) {
            return zPrintGroups(printableGroupIDs);
//          ^^^^^^
        }
    }).then(zAuditSave);
//    ^^^^^^^^^^^^^^^^^
}

function zPrintGroups(newGroupIDs) {
    // calls external program to print groups
}

function zCloseWindow() {
    window.close();
}

function zAuditSave() {
    return $.ajax({
//  ^^^^^^
        type: 'POST',
        contentType: 'application/json',
        url: '/api/audit'
    });
}

edited May 23 '17 at 10:10

Community

1
1

answered Apr 13 '15 at 19:06

Bergi

630,263
148
957
1,375

I tried to refactor the code to use just one pattern i.e. `promises`. Still `zCloseWindow()` is called before `zPrintGroups()`. I tried to debug, and found that the control hits the line `return zSaveDetails(selectedDataGroups);` and then when I step into, the ajax with in `zSaveDetails()` is executed to save the data and then the control goes straight to `zCloseWindow()` and close the window hence `zPrintGroups()` method in `zSaveDetails()` is never fired. – Sri Reddy Apr 13 '15 at 22:14
Maybe you've got `.then(zCloseWindow())` instead of `.then(zCloseWindow)`? – Bergi Apr 13 '15 at 22:15
I cross checked, it is `zSaveSomeData().then(zCloseWindow);`. I also, checked all other method calls, and they look good. – Sri Reddy Apr 13 '15 at 22:19
Can you put together a demo somewhere maybe? I'm pretty confident this should work. – Bergi Apr 13 '15 at 22:22
Sure, i will try to create something demoable. Not sure how to do ajax call though. But, let me try. – Sri Reddy Apr 13 '15 at 22:29
I created a demo with simple mockjax calls and it works as explained by you. But the code looks similar to actual code, I noticed that `zCloseWindow()` is not called until data is saved (ajax POST) in `zSaveDetails(selectedDataGroups)`. Data is saved and then control goes `zCloseWindow()` method before executing anything in then section. – Sri Reddy Apr 14 '15 at 20:56
Here is the link to my code: http://codepad.org/x6aZq4gE I have removed the unwanted code from this link. The problem workflow starts with `$("#btnSaveAndCloseOrders").click()` event. Hope this is not too overwhelming code. – Sri Reddy Apr 14 '15 at 21:22
Inside your `zSaveOrderDiagnosesConceptsAndOrderData` function you are still calling `zSaveConceptData()` with a callback, while it returns a promise. That callback `function () { return zSaveOrderGroups(selectedOrderGroups); }` is never called, it should probably be a callback to a `.then()` call. – Bergi Apr 14 '15 at 21:44
good catch. But, in my workflow that code was never hit but i have modified the code but still `zSaveOrderGroups(selectedOrderGroups)` is fired and data is saved and then it directly goes to `function zCloseOrderWindow()`. And then returns back to execute the code `function zPrintOrderGroups(newOrderGroupIDs)'. But by then, the windows is already closed hence no code is executed. Here is the modified code: http://codepad.org/k9BLwlgU – Sri Reddy Apr 15 '15 at 15:26
Uh, you haven't modified that callback code there? Regardless, I cannot explain that behavior. Maybe the `window.location = ` assignment in `function zPrintOrderGroups` confuses someone? – Bergi Apr 15 '15 at 16:11
Grrr, I found the issue why it didn't execute the rest of the methods in the chain. It is due to the older version of jquery library that they use. With 1.6.2, after the first promise, it ignores the rest of the chain within the `zSaveOrderDiagnosesConceptsAndOrderData ()` and goes back to 'zCleanUp()' followed by `zCloseOrderWindow()`. Bad luck that they can't upgrade to higher version yet. Any ideas how to resolve this? – Sri Reddy Apr 15 '15 at 20:41
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/75331/discussion-between-kuul13-and-bergi). – Sri Reddy Apr 15 '15 at 21:28
@kuul13: In such old jquery versions, you still can use `.pipe()` instead of `.then()`. But you really should consider upgrading. – Bergi Apr 15 '15 at 23:51

Jquery When and Deferred object, broken function flow

3 Answers3