2
\$\begingroup\$

In my system there are 3 main functionalities:

  1. Add users to database
  2. Delete users from database
  3. Updater users

This is the screenshot in my UI:

enter image description here

When clicking on the button of add new user, a jQuery modal form dialog appears and there is a form to add user details to the database.

When clicking on the images under delete column (see above image), a jQuery confirmation dialog appears, and if confirmation is true, the user should be deleted from the database. In the meantime, a particular table row, which belongs to the deleted user, should disappear from the table with a fadeout effect.

Here I have use jQuery and Ajax with PHP and MySQL for the project first time. How is my jQuery and Ajax code? Are there any security issues or refactoring method in my code?

NOTE: Adding/deleting functionality is working perfectly with the posted code.

jQuery:

$(function(){           

$( "input[type=submit], button" )
    .button()
    .click(function( event ) {
    event.preventDefault();
});


// form validation
var name    = $( "#name" ),
     address = $( "#address" ),
     city   = $( "#city" ),
     all        = $( [] ).add( name ).add( address ).add( city ),
     tips   = $( ".validateTips" ); 

function updateTips( t ) {
    tips
        .text( t )
        .addClass( "ui-state-highlight" );
        setTimeout(function() {
            tips.removeClass( "ui-state-highlight", 1500 ); 
        }, 1500 );
            tips.css("color", "red");   
}           

function checkLength( o, n, min, max ) {
    if ( o.val().length > max || o.val().length < min ) {
        o.addClass( "ui-state-error" );
        updateTips( "Length of " + n + " must be between " + min + " and " + max + "." );
        return false;
    } else {
        return true;
    }
}           

function checkRegexp( o, regexp, n ) {
    if ( !( regexp.test( o.val() ) ) ) {
        o.addClass( "ui-state-error" );
        updateTips( n );
        return false;
    } else {
        return true;
    }
}   



$("#dialog").dialog({
    autoOpen: false,
    height: 'auto',
    width: 350,
    modal: true,
    buttons: {
        "Add New User": function() {
            var bValid = true;
            all.removeClass( "ui-state-error" );        

            bValid = bValid && checkLength( name, "Your Name", 3, 60 );
            bValid = bValid && checkLength( address, "Your Address", 3, 50 );
            bValid = bValid && checkLength( city, "Your City", 3, 40 ); 

            bValid = bValid && checkRegexp( name, /^[a-z -']+$/i, "Your name may consist of A-Z, a-z, 0-9, underscores, begin with a letter." );
            bValid = bValid && checkRegexp( address, /^[a-z -']+$/i, "Your name may consist of A-Z, a-z, 0-9, underscores, begin with a letter." );
            bValid = bValid && checkRegexp( city, /^[a-z -']+$/i, "Your name may consist of A-Z, a-z, 0-9, underscores, begin with a letter." );

            if ( bValid) {      
                $.ajax({
                    type: "POST", // HTTP method POST or GET
                    url: "process.php", //Where to make Ajax calls
                    //dataType:"text", // Data type, HTML, json etc.
                    dataType: 'html',
                    data: { 
                        name: $('#name').val(), 
                        address: $('#address').val(), 
                        city: $('#city').val() 
                    },
                    beforeSend: function(){
                        $('#loading').dialog({
                            height: 57,
                            width: 400,
                            modal: true,
                            position: { my: "center top+20", at: "center top+20", of: window }, 
                            resizable: false,
                            draggable: false,                           
                            dialogClass: 'no-close loading-dialog',
                            hide: {
                                effect: "fade",
                                duration: 500
                            }
                        });
                    },
                    success:function(data){
                        $('#manage_user table > tbody:last').find('tr:first').before(data);

                    },
                    error:function (xhr, ajaxOptions, thrownError){
                        //On error, we alert user
                        alert(thrownError);
                    }, 
                    complete: function(){
                        //alert('update success'); 
                        //$('#loading').hide(); 
                        setTimeout("$('#loading').dialog('close');", 300);
                        $('#success').dialog({
                            height: 57,
                            width: 400,
                            modal: true,
                            position: { my: "center top+20", at: "center top+20", of: window }, 
                            resizable: false,
                            draggable: false,                           
                            dialogClass: 'no-close success-dialog',
                            hide: {
                                effect: "fade",
                                duration: 1000
                            }
                        });
                        setTimeout("$('#success').fadeOut('slow').dialog('close');", 3000);
                    }
                });
                $(this).dialog("close");                    
            } 
        },                  
        Cancel: function() {
            $( this ).dialog( "close" );
        }
    }, 
    close: function() {
        all.val( "" ).removeClass( "ui-state-error" );
    }
});


$( "#FormSubmit" )
    .button()
    .click(function() {
    $( "#dialog" ).dialog( "open" );
    });


$('#filter-value').change(function(){
    var filterValue = $(this).val();
    //console.log(filterValue); 

    $.ajax({
        type: 'post',
        url: 'table.php',
        dataType: 'html',
        data: {filter: filterValue},
        success:function(data){ 
            $('#response').html(data);
            //alert(data); 
        }, 
        error:function (xhr, ajaxOptions, thrownError){
            //On error, we alert user
            alert(thrownError);
        }, 
        complete: function(){
            //alert('update success'); 
        }
    });
});
// set a default value to dropdown 
$('#filter-value').val(5).change();


//##### Send delete Ajax request to process.php #########
$("body").on("click", "#response table td a.del_button", function(e) {
    e.returnValue = false;
    var clickedID = this.id.split('-'); //Split string (Split works as PHP explode)
    var DbNumberID = clickedID[1]; //and get number from array
    var myData = 'recordToDelete='+ DbNumberID; //build a post data structure   
    //console.log(myData); 

    var $tr = $(this).closest('tr'); //here we hold a reference to the clicked tr which will be later used to delete the row


    $("#delete_this_user").dialog({
      resizable: false,
      height:140,
      modal: true,
      buttons: {
         "Yes": function() {
            //$row.remove();
            $(this).dialog( "close" );

                $.ajax({
                    type: "POST", // HTTP method POST or GET
                    url: "process.php", //Where to make Ajax calls
                    dataType:"text", // Data type, HTML, json etc.
                    data:myData, //Form variables
                    beforeSend: function(){
                        $('#loading').dialog({
                            height: 57,
                            width: 400,
                            modal: true,
                            position: { my: "center top+20", at: "center top+20", of: window }, 
                            resizable: false,
                            draggable: false,                           
                            dialogClass: 'no-close loading-dialog',
                            hide: {
                                effect: "fade",
                                duration: 500
                            }
                        });
                    },
                    success:function(response){
                        //on success, hide  element user wants to delete.
                  $tr.find('td').fadeOut(1000,function(){ 
                     $tr.remove();                    
                  });
                    },
                    error:function (xhr, ajaxOptions, thrownError){
                        alert(thrownError);
                    },
                    complete: function(){
                        setTimeout("$('#loading').dialog('close');", 300);
                        $('#success').dialog({
                            height: 57,
                            width: 400,
                            modal: true,
                            position: { my: "center top+20", at: "center top+20", of: window }, 
                            resizable: false,
                            draggable: false,                           
                            dialogClass: 'no-close success-dialog',
                            hide: {
                                effect: "fade",
                                duration: 1000
                            }
                        });
                        setTimeout("$('#success').fadeOut('slow').dialog('close');", 3000);
                    }
                });
         },
         "no": function() {
            $(this).dialog( "close" );
         }
      },
        position: { 
            my: 'top center',
            at: 'top center',
            of: $('#response table')
        }
   });      


});

});

This is the HTML from the index.php page:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
 <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
    <title>User Registration System</title>
    <link type="text/css"  rel="stylesheet" href="css/styles.css"  media="all" />
    <link rel="stylesheet" type="text/css" href="css/jquery-ui-1.10.3.custom.css" />        

    <script type="text/javascript" src="js/jquery-1.9.1.js"></script>
    <script type="text/javascript" src="js/jquery-ui-1.10.3.custom.js"></script>
    <script type="text/javascript" src="js/complete.js" ></script> 

 </head>

 <body>

    <h1>User Management System</h1>

    <div id="popup-msg">
        <div id="loading" title="Loading">
            <img src="images/my-ajax-loader.gif" width="25" />
            <p>Please wait a few seconds.</p>
        </div>  

        <div id="success" title="Hurray!">
            <p>User table is updated.</p>
        </div>
    </div>

    <div id="manage_user">
        <form action="" method="">
            <div id="response"></div>
            <button id="FormSubmit">Add New User</button>
        </form>
    </div>

    <br />
    <div style="margin: 0 20px 20px;">
        <form method="post" action="">
            <select id="filter-value" name="filter">
                <option value="5">5</option>
                <option value="10">10</option>
                <option value="20">20</option>
                <option value="30">30</option>              
            </select>
        </form>
    </div>

    <div id="dialog" class="new_user_dialog_box" title="Add New User">
        <p>Fill this form with your details and click on 'Add New User' button to register.</p>

        <p class="validateTips">All form fields are required.</p>

        <div id="new_user_form">
            <table>
                <tr>
                    <td>Name :</td>
                    <td><input type="text" name="name" value="" id="name" /></td>
                </tr>               
                <tr>
                    <td>Address :</td>
                    <td><input type="text" name="address" value="" id="address" /></td>
                </tr>               
                <tr>
                    <td>City :</td>
                    <td><input type="text" name="city" value="" id="city" /></td>
                </tr>
            </table>
        </div>
    </div>

    <div id="delete_this_user" title="Delete User">
        <p>Are you sure you want to delete this user?</p>
    </div>

 </body>
</html>

This is the PHP code for the table display on index.php:

<?php

//echo '<pre>', print_r($_POST) . '</pre>'; 
//die('I am here');

if ( isset($_POST['filter'])) {
    $filter = $_POST['filter']; 
    echo $filter; 

// include configuration file 
require_once('../test.php');

// make the query 
$q = "SELECT id, name, address, city    FROM users ORDER BY id DESC LIMIT $filter";
// execute the  query 
$r = mysqli_query($dbc, $q); 

// count the users 
$num = mysqli_num_rows($r); 

// define output variable 
$output = '';

// get all registered users 
if ( $num >= 1 ) {

    // print how many users are 
    $output  = "<h2>There are $num registered users</h2>\n";

    // print the table header 
    $output .= "<table>\n";
    $output .= "  <tr>\n";  
    $output .= "    <th><input type='checkbox' class='selectAll' name='selectAll' value='' /> Name</th>\n";
    $output .= "    <th>Address</th>\n";
    $output .= "    <th>City</th>\n";
    $output .= "    <th>Edit</th>\n";
    $output .= "    <th>Delete</th>\n";
    $output .= "  </tr>\n";
    $output .= " <tbody>\n";

    while ($row = mysqli_fetch_array($r, MYSQLI_ASSOC)){
        $id     = $row['id'];
        $name   = $row['name'];
        $address = $row['address'];
        $city       = $row['city'];

        // make the table body 
        $output .= " <tr>\n";
        $output .= "  <td><input type='checkbox' name='' value='' class='' />&nbsp;&nbsp;$name</td>\n";
        $output .= "  <td>$address</td>\n";
        $output .= "  <td>$city</td>\n";
        $output .= "  <td><a href='#' id='edit-$id' class='edit_button'><span class='edit_ico'></span></a></td>\n";
        $output .= "  <td><a href='#' id='delete-$id' class='del_button'><span class='delete_ico'></span></a></td>\n";
        $output .= " </tr>\n";              
    }
} else {
    $output .= "<h2>There is no any registered users so far.</h2>\n"; 
}

// close the table 
$output .= " </tbody>\n";
$output .= "</table>\n";

echo $output;


//close db connection
mysqli_close($dbc);


}

?>

This is the code from the proccess.php page:

<?php

//include db configuration file
include_once("../test.php");

if ( (isset($_POST["name"]) && strlen($_POST["name"]) >= 3 && strlen($_POST["name"]) <= 60) &&
    (isset($_POST["address"]) && strlen($_POST["address"]) >= 3 && strlen($_POST["address"]) <= 50) &&
    (isset($_POST["city"]) && strlen($_POST["city"]) >= 3 && strlen($_POST["city"]) <= 40) ) 
{   //check $_POST["name"] and $_POST["address"] and $_POST["city"] are not empty

    $name   = $_POST["name"];
    $address = $_POST["address"];
    $city   = $_POST["city"];

    $q = "INSERT INTO users ( name, address, city) VALUES 
            ('".$name."','".$address."','".$city."')";
    $r = mysqli_query($dbc, $q); 

    if ( $r ) {
        // make the table row 
        $output  = "<tr>\n";
        $output .= "  <td><input type='checkbox' name='' value='' class='' />&nbsp;&nbsp;$name</td>\n";
        $output .= "  <td>$address</td>\n";
        $output .= "  <td>$city</td>\n";
        $output .= "  <td><span class='edit_ico'></span></td>\n";
        $output .= "  <td><span class='delete_ico'></span></td>\n";
        $output .= "</tr>\n";   

          //$output  = "  <td><input type='checkbox' name='' value='' class='' />&nbsp;&nbsp;$name</td>\n";
        //$output .= "  <td>$address</td>\n";
        //$output .= "  <td>$city</td>\n";
        //$output .= "  <td><span class='edit_ico'></span></td>\n";
        //$output .= "  <td><span class='delete_ico'></span></td>\n";

        echo $output;

    } else {
        echo 'query error';
    }
} elseif( isset($_POST["recordToDelete"]) && 
             strlen($_POST["recordToDelete"]) > 0 && 
             is_numeric($_POST["recordToDelete"])) {    //do we have a delete request? $_POST["recordToDelete"]

    //sanitize post value, PHP filter FILTER_SANITIZE_NUMBER_INT removes all characters except digits, plus and minus sign.
    $idToDelete = filter_var($_POST["recordToDelete"],FILTER_SANITIZE_NUMBER_INT); 

    //try deleting record using the record ID we received from POST
    $q = "DELETE FROM users WHERE id=$idToDelete";
    $r = mysqli_query($dbc, $q); 

    if(mysqli_affected_rows($dbc) > 1 ) {    
        //If mysql delete query was unsuccessful, output error 
        header('HTTP/1.1 500 Could not delete record!');
        exit();
    }
    mysqli_close($dbc); //close db connection

} else {             
    echo "error in post array";
}

?>
\$\endgroup\$
  • \$\begingroup\$ Is this application tested? keep in mind that unit or integration tests help a lot in the refactorization process. \$\endgroup\$ – Tomas Prado Jul 3 '13 at 9:20
  • \$\begingroup\$ Using mysqli_* is good, but better still is using it with prepared statements, which the old mysql_* extension did not support\ \$\endgroup\$ – Elias Van Ootegem Nov 2 '13 at 16:17
1
\$\begingroup\$

I would assess what your needs are in terms of refactoring - am I going to attain any benefit from doing so?

In either case, you will want to tidy your code: for example, you are directly placing data inside a SQL statement without escaping it. Use mysql_real_escape_string to escape the data before putting it into the query for execution.

Second, it might be a good idea to separate your concerns into business logic and view logic, consider using an architecture to help you shape the structure of the application. That way, your file isn't a mixture of SQL and echoing content. Bear in mind, that a lot of developers practice DRY - Don't Repeat Yourself i.e make code as generic as possible without compromise.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.