jQuery AJAX solution inside each() loop

Question

I have noticed that I'm having problems when I'm using AJAX in jQuery inside a .each() loop. Only the first record in my database are being updated when the script executes.

Here's my script:

function save(){
            var _userTypeId;
            var _userTypeName;
            var _isDeleted;
            var request;

            $("tr.recUserType").each(function(){
                $this = $(this);
                _userTypeId = $this.find("#userTypeId").html();
                _userTypeName = $this.find("#userTypeName").val();
                _isDeleted = $this.find("#isDeleted").val();

                request = $.ajax({
                    url: "save.php",
                    type: "POST",
                    data: {userTypeId: _userTypeId, userTypeName: _userTypeName, isDeleted: _isDeleted}
                });
            });

            request.done(function(){
                document.location.reload();
            });

            request.fail(function(){
                alert("Request Failed!");
            });
        }

And the contents of save.php:

<?php
include_once "globals.php";

dbConnect();

$isExisting = mysql_query("SELECT COUNT(userTypeId) AS userCount FROM userType WHERE userTypeId='".$_POST['userTypeId']."';");

$result = mysql_fetch_array($isExisting);

//original: if(!$result['userCount'] = 0) <-- This was a logical error
if($result['userCount'] != 0)
    mysql_query("UPDATE userType SET userTypeName='".$_POST['userTypeName']."', isDeleted='".$_POST['isDeleted']."' WHERE userTypeId='".$_POST['userTypeId']."';");
else
    mysql_query("INSERT INTO userType VALUES('', '".$_POST['userTypeName']."', '".$_POST['isDeleted']."');");

echo mysql_error();

dbClose();
?>

I have read that I have the option to use synchronous instead of asynchronous, but I have also read it is not a good practice.

So how do I actually get this done asynchronously and fix the problem?

You are using the same `userTypeId` - `_userTypeId = $this.find("#userTypeId").html();` for each loop so you are only going to update 1 row. — Sean, Jul 28 '13 at 03:43
Allow me to just `POST userTypeName=Lolinjection&isDeleted=1&userTypeId=' OR 1 = 1; --` and explain to you how SQL injection is bad. Use a library that actually helps protect against this, or at the very least sanitize your inputs. — Amelia, Jul 28 '13 at 03:47
@Sean, I'm not sure how the userTypeId value would be the same if I'm iterating through each of the tags, even if they have the same id. — Mico Francisco, Jul 28 '13 at 08:24
@Hiroto, I do know that my code is still prone to SQLi, and I still have to do some validations for that. Thanks for the reminder though. — Mico Francisco, Jul 28 '13 at 08:26
Although technically it might work to select the child of the `` with the id of `userTypeId` (http://stackoverflow.com/a/7017308/689579), I would not recommend it. Using the same id multiple times in a document, even if in different `` tags, is invalid html. http://www.w3.org/TR/html401/struct/global.html#h-7.5.2 / http://www.w3.org/TR/html5/dom.html#the-id-attribute `The id attribute assigns a **unique** identifier to an element` / `The id attribute specifies its element's unique identifier (ID)`. These would be better as classes. — Sean, Jul 28 '13 at 15:14
@Sean, I see your point there. I would try to modify the code, but as of now, it's working. My error was with the save.php file, I'll just edit the post. Thanks. — Mico Francisco, Jul 29 '13 at 04:26

score 4 · Accepted Answer · answered Jul 28 '13 at 03:58

jQuery's $.ajax() returns a jQuery XMLHttpRequest Object (a "jqXHR"). You are storing this object into your "response" variable.

Your problem is scope. You are storing all N of your jqXHRs into the same "request" variable. By the end of your loop, "request" is only pointing to the last jqXHR, and thus .done() will only be called when your LAST request completes.

As Karl Anderson pointed out, you should store ALL of your jqXHRs into an array, and then execute a single callback once ALL of them have [asynchronously] completed.

var XHRs = [];

// ...

$("tr.recUserType").each(function() {
    $this = $(this);
    _userTypeId = $this.find("#userTypeId").html();
    _userTypeName = $this.find("#userTypeName").val();
    _isDeleted = $this.find("#isDeleted").val();

    XHRs.push($.ajax({
        url: "save.php",
        type: "POST",
        data: {userTypeId: _userTypeId, userTypeName: _userTypeName, isDeleted: _isDeleted}
    }));
});

$.when(XHRs).then(function() {
    document.location.reload();
});

Also, avoid the delicious temptation to use $.ajax()'s "async: false". The browser will be forced to hang until a request completes, which is bad. You can pretty much always accomplish a $.ajax() call asynchronously; it may require some craftiness, but will definitely be for the better.

For some reason, it still does not work even after I store the jqXHR objects inside an array. I got the concept of what you're talking about, just not sure why it does not work. — Mico Francisco, Jul 28 '13 at 08:22
Maybe try: var defer = $.when.apply($, jqXHRs); defer.then(function() { ... }); (http://stackoverflow.com/questions/9865586/jquery-when-troubleshooting-with-variable-number-of-arguments see Andy's answer) — Jackson, Jul 29 '13 at 04:23

score 0 · Answer 2 · edited May 23 '17 at 12:14

Your issue is in this block of code:

request.done(function(){
    document.location.reload();
});

Since the actions are asynchronous, this line is being executed before the save is completing, thus after the first save does complete, it executes the done logic.

You need to create an array of deferred objects that you can then wait until they are all executed before proceeding forward with your logic using the jQuery .when()/.then() functions.

Here is a previous StackOverflow question that details how to setup this situation. Please read the accepted answer of jQuery when each is completede trigger function.

score 0 · Answer 3 · answered Jul 28 '13 at 03:47

0

There are a couple things happening here.

By re-using the request variable in the loop (scoped outside the loop), every iteration of the loop assigns a new object to request, overwriting what was there. This means you are only setting the response handlers for the last iteration.
Your request.done method reloads the page, which in practice will halt the other requests and...
You are looking up the same userTypeId for each iteration of the loop, as mentioned by @Sean

answered Jul 28 '13 at 03:47

lemieuxster

1,081
7
14

It gets more clear now, however, regarding #3, would it matter if each tag has the same id which is userTypeId? It won't be the same userTypeId for each iteration because I would be iterating through each . Is it right or I'm missing something? – Mico Francisco Jul 28 '13 at 07:19
since you are using `#userTypeId` it is looking for elements on the page with the `id="userTypeId"` of which there should only be one (in terms of best practice). But since you are scoping the lookup to the parent tr it may work out in this case, but the code could be made cleaner using classes or something different. – lemieuxster Jul 29 '13 at 19:27

jQuery AJAX solution inside each() loop

3 Answers3

Linked