Shopping cart with Ajax

Question

I build a cart for an ecommerce website, and I want to know if I did well.

I want to record the changes in the DB and display the information from the DB to the customer. To be sure that what he sees is the good information. I use an ajax request and JSON to do so.

Please give me your comments. I especially want to know is my code safe?

The client side:

    function save_to_db(cartId, newQuantity) {
  $.ajax({
    url: "updateCart.php",
    data: "cartId=" + cartId + "&newQuantity=" + newQuantity + "&memberId=" + <?php echo $memberId?>,
    type: 'post',
    dataType: "json",
    success: function(response) {
      if (response === "Empty cart") {
        $("#total-quantity").text(0);
        $("#total-price").text(0);
        $("#shopping-cart-table").html("<h2><b style='color:red'>Your cart is empty.</b></h2>");
      }

      if (newQuantity == 0) {
        $("#item-container-" + cartId).remove();
      }

      var totalQuantity = 0;
      var totalItemPrice = 0;
      $.each(response, function(key, cartDetails) {
        totalItemPrice = totalItemPrice + parseInt(cartDetails.price) * parseInt(cartDetails.quantity);
        totalQuantity = totalQuantity + parseInt(cartDetails.quantity);

        if (cartDetails.cartId == cartId) {
          $("#input-quantity-" + cartId).val(cartDetails.quantity);
          $("#cart-price-" + cartId).text("$" + cartDetails.price * cartDetails.quantity);
        }
      });

      $("#total-quantity").text(totalQuantity);
      $("#total-price").text(totalItemPrice);
      $("#total-price-foot").text(totalItemPrice);
    }
  });
}

The server side:

    <?php
require 'connectDB.php';

$memberId = $_POST["memberId"];

if($_POST["newQuantity"] == 0){
    $stmt = mysqli_prepare($conn,"DELETE FROM tbl_cart WHERE id = ? AND member_id = ?");

    mysqli_stmt_bind_param($stmt, "ii", $_POST["cartId"], $memberId);
    mysqli_stmt_execute($stmt);
    mysqli_stmt_close($stmt);
}
elseif ($_POST["newQuantity"] == 1 || $_POST["newQuantity"] == -1) {
    $stmt = mysqli_prepare($conn,"UPDATE tbl_cart SET quantity =
        (CASE WHEN quantity + ? > 0 THEN quantity + ? ELSE quantity END)
        WHERE id= ? AND member_id = ?");

        mysqli_stmt_bind_param($stmt, "iiii",
            $_POST["newQuantity"],
            $_POST["newQuantity"],
            $_POST["cartId"],
            $memberId);
        mysqli_stmt_execute($stmt);
        mysqli_stmt_close($stmt);
    }

    if ($stmt = mysqli_prepare($conn,"SELECT tbl_product.id, tbl_product.price, tbl_cart.quantity, tbl_cart.id
        AS cartId FROM tbl_product, tbl_cart WHERE tbl_product.id = tbl_cart.product_id AND tbl_cart.member_id = ?")) {

            mysqli_stmt_bind_param($stmt, "i", $memberId);
            mysqli_stmt_execute($stmt);

            $result = mysqli_stmt_get_result($stmt);
            while ($row = mysqli_fetch_assoc($result)){
                $cartDetails[] = $row;
            }

            mysqli_stmt_close($stmt);
        }


        if(isset($cartDetails)){
            $response = json_encode($cartDetails);
            echo $response;
        }
        else{
            $response = json_encode("Empty cart");
            echo $response;
        }
        ?>

Answer 1

In your action processing script, you should, before doing anything else, validate the input. If anything is outside of the expected range of values, you should immediately return a failure response.

I will also urge you to write your query functions in object-oriented syntax rather than procedural because it is more brief and easier to read (IMO).

Since you are returning json, I recommend that you always return some sensibly keyed objects containing predictable data types for your javascript to handle. You may consider status, message, and data as included keys no matter the outcome, this will make the javascript handling much cleaner.

The following rewrite will exercise reduced variable generation, and less complex & more readable logic and queries.

The action processing script:

if (empty($_POST["memberId"]) ||
    empty($_POST["cartId"]) ||
    !isset($_POST["newQuantity"]) ||
    !ctype_digit($_POST["memberId"]) ||
    !ctype_digit($_POST["cartId"]) ||
    !in_array($_POST["newQuantity"], [-1, 0, 1])
) {
    exit(json_encode(['status' => 'fail',
                      'message' => 'Missing/Invalid request data',
                      'data' => []]));
}

require 'connectDB.php';  // adjust this to OO style instead of procedure

if (!$_POST["newQuantity"]) {
    $query = "DELETE FROM tbl_cart WHERE id = ? AND member_id = ?";
} elseif ($_POST["newQuantity"] == 1) {
    $query = "UPDATE tbl_cart SET quantity = quantity + 1 WHERE id = ? AND member_id = ?";
} else {
    $query = "UPDATE tbl_cart SET quantity = quantity - 1 WHERE quantity > 0 AND id = ? AND member_id = ?";
}
$stmt = $conn->prepare($query);
$stmt->bind_param('ii', $_POST["cartId"], $_POST["memberId"]);
$stmt->execute();
$stmt->close();

$stmt = $conn->prepare($conn, "SELECT p.id, p.price, c.quantity, c.id AS cartId 
                               FROM tbl_product AS p
                               INNER JOIN tbl_cart AS c on p.id = c.product_id
                               WHERE c.member_id = ?");
$stmt->bind_param('i', $_POST["memberId"]);
$stmt->execute();
$result = $stmt->get_result();
exit(json_encode(['status' => 'pass',
                  'message' => 'Shopping cart updated',
                  'data' => $result->fetch_all(MYSQLI_ASSOC)]));

Now you javascript can be adjusted to process these very useful, meaningful, and always-available values (which ALWAYS have the same data type) via:

• response.status (string)
• response.message(string)
• response.data(an empty or multi-dimensional array)

Answer 2

So far what I see is you aren't either casting nor sanitizing your $_POST variables but simply placing them into your SQL statement - it's better you perform some sort of filtering on those arguments (e.g. remove whitespaces, fixing invalid characters like '\' to '\\', etc.) and ensure you are passing numbers (according to your example you'd maybe use intval()), not a string or NULL value (unless that's intended, which I doubt).

In addition, the server-end should separate the database calls from parsing and filtering content - either as separate functions, files and/or both; if you were going the OOP route, then you'd set up a class(es) for either activity and use a layering system like MVC, N-Tier, MVVC.

All of this goes into making your code more reusable and easier to manage plus adds a layer of security via "Abstraction".