0

I'm trying to use ajax to post to a controller when a checkbox is clicked. It should update an existing object. My two problems are:

  • I cannot successfully call the controller action method
  • Only the first item in a list has the click event

Hopefully someone can help out

On click event with ajax

    $('#checkboxID').on('click', function () {
            $.ajax({
                url: '@Url.Action("Attending", "GuestBookings")',
                type: 'POST',
                data: JSON.stringify({
                    "cID": $(this).val('cID'), "eID": $(this).val('eID'), "check": $(this).is(':checked') ? 1 : 0
                }),
                success: function(result) {
                    alert("Succeeded post");
                },
                error: function(result) {
                    alert("Failed to post");
                }
            });
        });

Controller HttpPost

    [HttpPost]
        [ValidateAntiForgeryToken]
        public async Task<IActionResult> Attending(string cID, string eID, string check)
        {
            if (ModelState.IsValid)
            {
                var guest = await _context.Guests
                                     .Where(g => g.CustomerId.ToString() == cID && g.EventId.ToString() == eID).FirstOrDefaultAsync();
                if (check == "0")
                {
                    guest.Attended = false;
                }
                else
                {
                    guest.Attended = true;
                }
                try
                {
                    _context.Update(guest);
                    await _context.SaveChangesAsync();
                }
                catch (DbUpdateConcurrencyException)
                {

                }
            }
            return Json(true);
        }

Checkbox

    <tbody>
            @foreach (var item in Model.GuestsBooked)
            {
                <tr>
                    <td>
                        @Html.DisplayFor(modelItem => item.Surname)
                    </td>
                    <td>
                        @Html.DisplayFor(modelItem => item.FirstName)
                    </td>
                    <td>
                        @Html.DisplayFor(modelItem => item.Email)
                    </td>
                    <td>
                        @Html.CheckBoxFor(modelItem => item.Attended, new { @id = "checkboxID", @cID = item.Id, @eID = Model.Id })
                    </td>
                    <td>
                        @Html.ActionLink("Details", "Details", "Customers", new { id = item.Id }) |
                        @Html.ActionLink("Unbook", "Delete", "GuestBookings", new { id = item.Id, eventId = Model.Id }) |
                    </td>
                </tr>
            }
        </tbody>
harryprs
  • 3
  • 1
  • You are not passing your validation token to the controller, see the accepted answer to the following question: https://stackoverflow.com/questions/4074199/jquery-ajax-calls-and-the-html-antiforgerytoken – Isma Dec 23 '18 at 23:23
  • You are creating multiple checkboxes with the same id. make checkboxID the class: @Html.CheckBoxFor(modelItem => item.Attended, new {@class = "checkboxID" } and then use $('.checkboxID').on('click', function () { .... this should fix issue 2. – grayson Dec 23 '18 at 23:33

1 Answers1

0

I see a few issues with your code.

First of all, In your view code, you are rendering the table rows inside a loop. Inside the table rows, you are using the CheckBoxFor helper method to render and bind the checkbox from the Attended property value. But you are using the overload where you are passing an anonymous object as the html attributes and there you are passing the Id attribute value as checkboxID. This means it will render checkboxID as the Id for all the checkboxes. Id values should be unique. If you have same Id value for more than one element, that is invalid HTML. This will also cause your jQuery click event to only work for the first item from the collection (the checkbox rendered in the first table row).

Let's fix this first. You can use another attribute for wiring up your jQuery click event handler. For example, here I am adding an attribute called ajaxupdate instead of the Id

@Html.CheckBoxFor(modelItem => item.Attended, 
                                 new {  ajaxupdate="true", 
                                        @cID = item.Id, @eID = Model.Id })

With this change, it will render the HTML markup for the checkbox element like this(some attributes are omitted for readability). The important part here is the presence of ajaxupdate attribute.

<input ajaxupdate="true" checked="checked" 
       cid="1" eid="2" name="item.Active" type="checkbox" >

Now to wire up the click event handler, we can use this data attribute selector.

$('[ajaxupdate="true"]').on('click', function () {
   // to do : code for ajax call
});

Once we make this fix, we will find that the click event is atleast executing the event handler code. But you will notice that the ajax calls are getting a 400 bad request response from the server. This is happening because you have your action method decorated with the ValidateAntiForgeryToken attribute. When an action method is decorated with this, the framework will check the submitted request data and if it does not find a valid anti forgery token, it will be considered a bad request and a 400 response will be sent back.

To fix this problem, you can explicitly read the value of __RequestVerificationToken hidden input (Generated by the @Html.AntiForgeryToken() method call in your view) and send that in your ajax request headers.

var t = $("input[name='__RequestVerificationToken']").val();

$.ajax({  url: '@Url.Action("Attending", "Home")',
          type: 'POST',
          headers:
                {
                    "RequestVerificationToken": t
                },
        //existing code for ajax
       })

Another option is to remove the ValidateAntiForgeryToken. But taking a hit on security.'

Now with the above step we must have fixed our 400 response problem. Now you probably will get a 500 Internal server error because you are getting some null reference error because your action method parameters are not properly bound with the values you expected and your LINQ query might be returning you a null object and you are trying to update Attended property value of that object without first checking whether it is a NULL object or not.

Let's first fix the client side code. Inside the click event handler of the checkbox, the this expression represents the checkbox element which was clicked. In your razor code, you are setting the cID and eID values as attributes to the element. So you should be reading those with attr method. Also you do not need to send the JSON string version. Simply pass the JavaScript object as the data property.

So the final cleaned up version of client side code would be

$(function () {

    $('[ajaxupdate="true"]').on('click', function () {
        var t = $("input[name='__RequestVerificationToken']").val();
        var $this = $(this);
        $.ajax({
            url: '@Url.Action("Attending", "GuestBookings")',
            type: 'POST',
            headers:
            {
                "RequestVerificationToken": t
            },
            data: {
                "cID": $this.attr('cID'),
                "eID": $this.attr('eID'),
                "check": $this.is(':checked') ? 1 : 0
            },
            success: function(result) {
                alert("Succeeded post");
                console.log(result);
            },
            error: function(xhr,status,errorThrown) {
                alert("Failed to post");
                console.log(errorThrown);
            }
        });
    });

});

Another suggestion I have is to use the correct types on your server side code. You are using string type for your parameters and then doing a ToString() call on your numerical properties to do comparison (with the predicate in your Where clause). Instead of doing that, use the appropriate numeric type as your parameter.

public async Task<IActionResult> Attending(int cID, int eID, int check)
{
    var guest = await _context.Guests
                              .Where(g => g.CustomerId == cID 
                                       && g.EventId == eID)
                              .FirstOrDefaultAsync();
    if(guest!=null)
    {
       guest.IsActive = check == 0;
       try
       {
           _context.Update(guest);
           await _context.SaveChangesAsync();
       }
       catch (Exception ex)
       {
          // Make sure to log the exception and not leak the ToString version to client.
          return Json(new { status = false, message = ex.ToString() });
       }
       return Json(new { status = true, message = "Updated" });
    }
    return Json(new { status = false, message = "Customer not found!" });
}

In the above code, I am returning a JSON object with a status and message property back to the client. For the exception use case, I am sending the ToString() version of the exception in the message property. You should do that only if you want to see that in the client side (mostly during your development). When you ship it to production, you do not want to leak this information to the client. Instead log it.

Inside the success handler of the ajax call, you may check the status or message property as needed.

success: function(result) {
    alert(result.message);                   
},
Shyju
  • 214,206
  • 104
  • 411
  • 497
  • This has done it! My first time using ajax in this way and I've learned a lot from this. I thought the action method would be wrong, I didn't think using stringify would cause the problems it did though. Thanks! – harryprs Dec 24 '18 at 10:58