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
– Sean, Commented Jul 28, 2013 at 3: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
– Amelia, Commented Jul 28, 2013 at 3:47
@Sean, I'm not sure how the userTypeId value would be the same if I'm iterating through each of the <tr> tags, even if they have the same id. — Mico Francisco
– Mico Francisco, Commented Jul 28, 2013 at 8: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
– Mico Francisco, Commented Jul 28, 2013 at 8:26
Although technically it might work to select the child of the <tr> with the id of userTypeId (stackoverflow.com/a/7017308/689579), I would not recommend it. Using the same id multiple times in a document, even if in different <tr> tags, is invalid html. w3.org/TR/html401/struct/global.html#h-7.5.2 / 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
– Sean, Commented Jul 28, 2013 at 15:14

Jackson Ray Hamilton · Accepted Answer · 2013-07-28 03:58:34Z

4

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.

answered Jul 28, 2013 at 3:58

Jackson Ray Hamilton

9,5766 gold badges54 silver badges81 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Mico Francisco Over a year ago

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.

Jackson Ray Hamilton Over a year ago

Maybe try: var defer = $.when.apply($, jqXHRs); defer.then(function() { ... }); (stackoverflow.com/questions/9865586/… see Andy's answer)

Community · Accepted Answer · 2017-05-23 12:14:22Z

0

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.

edited May 23, 2017 at 12:14

CommunityBot

11 silver badge

answered Jul 28, 2013 at 3:43

Karl Anderson

34.9k12 gold badges69 silver badges83 bronze badges

Comments

lemieuxster · Accepted Answer · 2013-07-28 03:47:01Z

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, 2013 at 3:47

lemieuxster

1,0817 silver badges14 bronze badges

2 Comments

Mico Francisco Over a year ago

It gets more clear now, however, regarding #3, would it matter if each <tr> 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 <tr>. Is it right or I'm missing something?

lemieuxster Over a year ago

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.

Collectives™ on Stack Overflow

jQuery AJAX solution inside each() loop

3 Answers 3

2 Comments

Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related