Restaurant system with JavaScript connecting to PHP

Question

I'm making a web application for a restaurant kind of type. The idea is to administrate the orders and customers with the functions such as:

• Make a new order
• Delete a order
• View all the orders
• Make a new customer
• Delete a customer
• View all the customers
• Finance overview

I'm doing this by separate PHP files. Here's the example for inserting a new order:

<?php

require_once "database.php";

if (!empty($_POST['customer_id'])) {
    $customer_id = $_POST['customer_id'];
    $order_info = $_POST['order_info'];
    $location = $_POST['location'];

    Database::getInstance()->insert_order($customer_id, $order_info, $location);
    exit;
} else {
    die();
}

This goes together with the Database class:

<?php

class Database extends mysqli
{

    // single instance of self shared among all instances
    private static $instance = null;

    private $databaseHost = "";
    private $databaseUser = "";
    private $databasePassword = "";
    private $databaseName = "";

    public static function getInstance() {
        if (!self::$instance instanceof self) {
            self::$instance = new self;
        }
        return self::$instance;
    }

    public function __clone() {
        trigger_error('Clone is not allowed.', E_USER_ERROR);
    }

    public function __wakeup() {
        trigger_error('Deserializing is not allowed.', E_USER_ERROR);
    }

    // private constructor
    function __construct() {
        parent::__construct($this->databaseHost, $this->databaseUser, $this->databasePassword, $this->databaseName);
        if (mysqli_connect_error()) {
            exit('Connect Error (' . mysqli_connect_errno() . ') ' . mysqli_connect_error());
        }
        parent::set_charset('utf-8');
    }


    function get_simple_data() {

        $query = "SELECT customers.first_name,
                         customers.last_name,
                         orders.location,
                         orders.id,
                         orders.total_price,
                         customers.email_adress

                  FROM customers

                  INNER JOIN orders ON customers.id = orders.customer_id
                  ORDER BY orders.id DESC";


        return $this->query($query);
    }

    function get_all_data_by_order_id($order_id) {
        $query = "SELECT customers.first_name,
                         customers.last_name,
                         customers.email_adress,
                         customers.customer_info,

                         orders.order_info,
                         orders.total_price,
                         orders.location,
                         orders.created

                         FROM customers
                 INNER JOIN orders ON customers.id = orders.customer_id

                 WHERE orders.id = {$order_id}";

        return $this->query($query);
    }

    function get_orders_by_customer_id($customer_id) {
        $query = "SELECT id, order_info, location, created FROM orders WHERE customer_id=" . $customer_id;
        return $this->query($query);
    }

    function get_order_items_by_order_id($order_id) {
        $query = "SELECT `products`.`name`, `orders-items`.`quantity` FROM `orders-items`\n" . "INNER JOIN `products`ON `orders-items`.`products_id` = `products`.`id`\n" . "WHERE order_id=" . $order_id;

        return $this->query($query);
    }

    function insert_customer($first_name, $last_name, $email_adress, $customer_info = "") {
        $firstName = mysql_real_escape_string($$first_name);
        $lastName = mysql_real_escape_string($last_name);
        $emailAdress = mysql_real_escape_string($email_adress);
        $customerInfo = mysql_real_escape_string($customer_info);
        $query = "INSERT INTO customers
         (id, first_name, last_name, email_adress, customer_info)
          VALUES (null, '$firstName', '$lastName', '$emailAdress', '$customerInfo'";

        $this->query($query);
    }

    function insert_order($customer_id, $order_info, $location) {
        $query = "INSERT INTO orders (id, customer_id, order_info, total_price, location, created) VALUES (NULL, '$customer_id', '$order_info', NULL, '$location', NOW())";
        $this->query($query);
    }

    function get_names() {
        $query = "SELECT customers.id,
                         customers.first_name,
                         customers.last_name

                  FROM customers";

        return $this->query($query);
    }

    function delete_order($order_id) {
        $query = "DELETE FROM orders WHERE id={$order_id}";
        $this->query($query);
    }

    function delete_all_order_items($order_id) {
        $query = "DELETE FROM" . "`orders-items` WHERE `order_id`={$order_id}";
        $this->query($query);
    }
}

All of the PHP files for each function looks like each other, just with other functions. They're all referred to the Database class.

For the purpose of making the page look nice by not letting it look like the page is refreshing, I'm using jQuery AJAX to load and give function to all the elements within the page.

$(document).ready(function () {

    var orderListScreen = $("#list"),
        newOrderScreen = $("#new-order"),
        availableProductsScreen = $("#available-products"),
        orderInformationScreen = $("#order-information"),
        selectedOrderInformation = $("#selected-order-information"),
        financeOverview = $("#overview");


    $("#new-order-btn").click(function () {
        new_order();
    });
    $("#delete-order-btn").click(function () {
        delete_order();
    });

    $("#list tbody tr").click(function () {
        select_row(this);
    });

    $("#submit-order").click(function () {
        submit_order(false);
    });

    function delete_order() {
        if (confirm("Are you sure you want to delete this order?")) {
            if (newOrderScreen.is(":visible")) {
                clear_form("#new-order-form");

                switchScreen(orderListScreen, newOrderScreen);
                switchScreen(orderInformationScreen, availableProductsScreen);

                $("#new-order-btn").prop("disabled", false);
                $("#delete-order-btn").prop("disabled", true);
            } else {
                $.ajax({
                    type: 'post',
                    url: "includes/functions/delete-order.php",
                    data: "order_id=" + $(".selectedRow").attr("data-order-index"),

                    success: function (data) {
                        $(".selectedRow").remove();
                        $("#delete-order-btn").prop("disabled", true);
                        switchScreen(financeOverview, selectedOrderInformation);
                    }

                });

            }
        }
    }

    function select_row(row) {
        var item = $(row);

        // remove previous selectedRow before setting new selectedRow
        item.parent().find(".selectedRow").removeClass("selectedRow");

        // set new selected row
        item.addClass("selectedRow");

        $("#delete-order-btn").prop("disabled", false);
        $.ajax({
            url: "includes/functions/order.php",
            type: "get",
            data: {order_id: item.attr("data-order-index")},
            success: function (data) {
                if (!selectedOrderInformation.is(":visible")) {
                    switchScreen(selectedOrderInformation, financeOverview);
                }
                selectedOrderInformation.html(data);
            }
        });
    }

    function new_order() {
        switchScreen(newOrderScreen, orderListScreen);
        switchScreen(availableProductsScreen, orderInformationScreen);

        $("#new-order-btn").prop("disabled", true);
        $("#delete-order-btn").prop("disabled", false);
        $("select[name=\"customer_id\"]").focus();
    }


    function submit_order(makeNew) {
        $("#new-order-form").submit(function (e) {
            e.preventDefault();
            $.ajax({
                type: 'post',
                url: "includes/functions/insert-order.php",
                data: $("#new-order-form").serialize(),
                success: function () {
                    clear_form("#new-order-form");
                    if (!makeNew) {
                        switchScreen(orderListScreen, newOrderScreen);
                        switchScreen(orderInformationScreen, availableProductsScreen);
                    }
                }
            });
            return false;
        });
    }

    function clear_form(form) {
        var $form = $(form);
        $form.find("input[type=\"text\"], textarea").val("").find("select").val("0");
    }


    function switchScreen(toActivate, toDeactivate) {
        var $toActivate = $(toActivate);
        var $toDeactivate = $(toDeactivate);

        if ($toDeactivate.is(":visible")) {
            $toDeactivate.hide();
            $toActivate.show();
        }
    }

});

It works great, but there are a few problems:

• The code looks very messy. I have to type out everything myself what the function is about, such as: new_order, delete_order, submit_order etc... Is there any way this can be seperated in classes or something?

• When executing the PHP codes via AJAX, it loads very slow into the document. It's loading even longer when I'm trying to do 2 queries at once.

• The PHP also looks very messy.

Notes

• When the list of orders is loaded into the document by the get_simple_data function in the Database class, its given a custom attribute: the order_id. This is used for loading the specific order data, such as what items are in the order.

---

On selecting a order, with only 2 orders in the list and only one has products in it here are some reaction times:

Answer 1

It seems like you have your work cut out..

It seems that get_orders_by_customer_id get_order_items_by_order_id delete_order and delete_all_order_items are vulnerable to SQL Injection.

Furthermore, if orders.id is an auto-increment number, then it seems a simple loop with ajax call in the console can empty your entire orders table. Probably not what you want. It might be more prudent to set a status flag to cancelled so that you recover more easily instead of just deleting data.

Finally, given that you do not have full protection for SQL Injection, I am assuming you will also have to read up on Cross Site Scripting.

When you wire the listeners, you can simply provide the functions you wrote, so that this

$("#new-order-btn").click(function () {
    new_order();
});
$("#delete-order-btn").click(function () {
    delete_order();
});

$("#list tbody tr").click(function () {
    select_row(this);
});

$("#submit-order").click(function () {
    submit_order(false);
});

becomes

$("#new-order-btn").click(new_order);
$("#delete-order-btn").click(delete_order);
//Undefined will translate to falsey, so `submit_order` will work
$("#submit-order").click(submit_order);
//You could do something with `bind` here
$("#list tbody tr").click(function () {
    select_row(this);
});

Which brings me to the point that JavaScript should really be written in lowerCamelCase so new_order -> newOrder, submit_order -> submitOrder etc.

Other than that, your JavaScript code is clean.

Answer 2

Just a few notes on your JavaScript code that were not brought up by other answers:

1. Namespace your functions. Right now they are all under this anonymous function, that's really messy. Instead separate your functions, create a namespace for your UI operations, for your utils, for your api calls, etc.

2. Avoid using jQuery selectors. They are slower than alternatives, as jQuery has to search the DOM to find the element, and the worse part is that selectors make your code VERY unreadable. Instead define an object (say Button) and do something like delete-order-button = new Button("some label"). Then you can do delete-order-buttor.click(...) which is much better than using the selector.

3. If you're going to use selectors, avoid using single selectors. $("#submit-order") looks shaky. Your code will break if there is a modification in your app that adds another item with id submit-order. If you're going to selectors do something like $("#button-container").find("#submit-order") that way you know you'll be getting the right #submit-order. Anyway I stay away from selectors, they make the code hard to follow and hard to maintain.

4. Define objects! Objects are good and make your code easier to follow! So for your use case you should probably define a Order object that will represent each order and handle things like updating the UI and making api calls. You could separate it even more by having a UI_Order and a UI_controller, but I don't think it's necessary for your use case.

5. Define your ajax calls in a separate file. Create a js file (say api.js) and place it in its own namespace. Inside that file define all your api calls as functions with meaningful names. For example,

   $.ajax({
       type: 'post',
       url: "includes/functions/delete-order.php",
       data: "order_id=" + $(".selectedRow").attr("data-order-index"),
       success: function (data) {
           $(".selectedRow").remove();
           $("#delete-order-btn").prop("disabled", true);
           switchScreen(financeOverview, selectedOrderInformation);
       }
   });
   

Would look much better like this:

myApp.api.deleteOrder = function (order_id) {
   return $.ajax({
                type: 'post',
                url: myApp.const.DELETE_ORDER_URL,
                data: { order_id: order_id }
            });
};

Then in your Order UI object you can do something like:

delete-order-button.click($.proxy(function () {
   myApp.api.deleteOrder(this.id).done($.proxy(function () {
      // Update UI to .remove this object
   }, this));
}, this));

Anyway, these are some of the things that came to mind as I read your code. For your current use case your code will work fine, but it most definitely won't scale. In the end you have to remember that code is simply a way of communicating, so you should strive to do it in the most organized and readable way possible.

I hope that helps!