Checkout (page) backend

Question

I am developing an e-commerce site and am confused about this.

1. Customers come on the site and add some products to the cart.
2. Customers click on the checkout link if already logged in, then will go to the "thanks" page (this is your order number, total price, etc.).

I want to insert the total price into Order table.

<?php
session_start();
include('../includes/dbfunctions.php');
if(!isset($_SESSION["id"]))
{
header("location:order_login.php");
}
if(isset($_SESSION['id']))
{
$id=$_SESSION["id"];
$email=$_SESSION["email"];
$name=$_SESSION['name'];
$contact=$_SESSION["contact"];
$mysqli->query("insert into     orders(user_id,user_name,contact_no,email)values('$id','$name','$contact','$email')");

$result=$mysqli->query("select order_id from orders where user_id='$id'   ORDER BY order_id desc limit 1"); 
$obj=$result->fetch_object();
$order_id=$obj->order_id;
}
?>
<form action="#" method="post" enctype="multipart/form-data">
<div class="table-responsive">
  <?php
        //Display of cart

        if(isset($_SESSION["products"]))
        {
            $total = 0;
            $qty=0;
            ?>



        <?php   
        foreach ($_SESSION["products"] as $cart_itm)
        {   
            $product_qty = $cart_itm["qty"];
            $product_code=$cart_itm["product_code"];
            $image=$cart_itm["pic"];
            $product_name=$cart_itm["name"];

            echo '<img src="../image/product/'.$image.'"width="120px" height="94px">';
            echo 'ucwords($product_name)';
            echo 'ucwords($product_code)';
            echo 'ucwords($product_qty)';
            $subtotal = ($cart_itm["price"]*$product_qty);
            $total = ($total + $subtotal);          
            echo '$subtotal';
            $qty=($product_qty+$qty);

            $mysqli->query("insert into order_details(user_id,img,product_name,product_code,qty,sub_total,order_id)values('$id','$image','$product_name','$product_code','$product_qty','$subtotal',$order_id)");


            }
            ?>
</form>

Answer 1

I see two obvious problems:

1. Your MySQLi queries are vulnerable to sql-injection since you are constructing the queries using string interpolation. You should use properly parameterized queries with ? placeholders instead.

2. The business logic is heavily intertwined with the HTML generation. You would be better off handling the input first (using PHP more like a programming language), writing the order to the database, then when the transaction has successfully completed, proceed to display the output (using PHP as a template language).

Answer 2

That is very poorly written code and requires major changes. I have following comments about your code:

• Your code is more like a script than a code of application.
• You have mixed up forms, your processing and your logic.
• You should separate your UI code and your processing at the least.
• Your checkout logic should be separated in different tasks and they should be either functions or methods of a class so that your code can be reused.

For example, you are often going to require order id of the user. I would, for the least, create a following function:

function getLastOrderIDForUser($user_id) {

    $result = $mysqli->query("select order_id from orders where user_id='{$user_id}' ORDER BY order_id desc limit 1"); 
    if($result) {
        $obj = $result->fetch_object();
        $order_id = $obj->order_id;    
    }
    else {
        return false;
    }

}

Answer 3

200_success covered the important things, I just have a couple of smaller points about code style and best practices:

• prepared statements are necessary for security, in your case they may also speed things up, as you can prepare the query once and then execute multiple times with different values.
• I don't get the logic behind your orders table. Why does it only contain user data, but no data for orders? It seems a little odd, but if this makes sense for your db design, ignore this.
• it's good practice to die after a redirect, as clients do not have to follow it. In this case, it doesn't result in vulnerable code, as you check the session id again afterwards, but still, your code may change.
• the standard says to have absolute paths for the location header field.
• don't check conditions twice. if (cond) is true, you do not need to check if (!cond) as well.
• your indentation is off, which makes your code harder to read.
• your spacing is also off. Sometimes you have spaces around =, sometimes you don't. The spacing in your queries also seems rather random.
• you have unnecessary newlines.
• SQL keywords should be all uppercase, which makes it easier to see what part of the query is keywords and what is data.
• don't shorten variable names. item isn't that much longer than itm, but more readable and easier to remember. Same with qty and quantity.
• your variable names could be more concrete. eg total what? total price? total amount of products? What's the difference between qty and product_qty? Is the first not the quantity of products but of something else?
• you have unneeded parentheses, eg here: $qty=($product_qty+$qty);.