Updating a product - what can be improved

[darren_azzopardi]

Hi there,

I’ve got a script that displays the product details from a database using an ID it gets from the url.
and looking to see if my questions can be answered or even if they are the right solution…

1. The form will always update inputs that the user might not have updated. is there a better way to update the table without updating everything? Maybe check to see what variable has been changed and if so, submit?

2. Like above the user won’t always need to update the image associated with the product, but my current code to support a ‘no image chosen’ type update is crude and i know this can be handled better…

3. the current code over has holes, like I have an else statement but with no output and my only reason to have it, there is when I remove it, it breaks…

ultimately im asking for what im doing wrong, and then i can go away read more about it and improve it…
Thank you…

<?php
if($_SERVER['REQUEST_METHOD'] == 'POST') {
 
   
    $name_of_book = trim($_POST['name']);  
    $des = trim($_POST['des']);
    $pr = trim($_POST['pr']);
    $cdate = date("Y-m-d");
    $pid = trim($_POST['pid']);
 
 
//it doesnt matter which variable is updated by the user as it will get updated
//!! Can we do it so if the variable is the same as whats in the databate base it doesnt update?   
if($name_of_book || $des || $pr){
 
    //Update in table product
    $stmt = $db_con->prepare("UPDATE product SET name=:en,
    des=:ds, pr=:pr,  cdate=:cd WHERE pid=:id");
 
    $stmt->bindParam(":en", $name_of_book);
    $stmt->bindParam(":ds", $des);
    $stmt->bindParam(":pr", $pr);
    $stmt->bindParam(":cd", $cdate);
    $stmt->bindParam(":id", $pid);
    $stmt->execute();
    $form_feedback = "<p>Details of this book has been updated<p>";
 
   
}
//some times the image wont be updated by the user
//!!(needs to be cleaned up)
if (empty($_FILES['fileToUpload']['name'])) {
    echo '';
}elseif(isset($_FILES["fileToUpload"]["name"])) {
 
        $allowed_ext = array("jpg","png","jpeg","gif");
        $ext = end(explode('.', $_FILES["fileToUpload"]["name"]));
        if(in_array($ext, $allowed_ext)){
                if($_FILES["fileToUpload"]["size"]<500000){
 
                    $name = md5(rand()) . '.' . $ext;
                    $path ="../img/".$name;
                    move_uploaded_file($_FILES["fileToUpload"]["tmp_name"], $path);
                    header("location:upload-v1.php?pid=".$pid."&&file-name=".$name."");
 
                    //Get the id so it knows where to update
                    $pid = trim($_GET['pid']);
                    $pid = filter_var($pid, FILTER_VALIDATE_INT);
 
                    $mg = "../img/".$name;
 
                    //insert in table product
                    $stmt = $db_con->prepare("UPDATE product SET img=:mg WHERE pid=:id");
 
                    $stmt->bindParam(":mg", $mg);
                    $stmt->bindParam(":id", $pid);
                    $stmt->execute();
                }
                }else{
                    echo '<script>alert("not the right image file")</script>';
                }
                }else{
                    echo '<script>alert("Big Image File")</script>';
                }
       
 
        }else{
        //!! A file doesnt have to be selected.
        //echo '<script>alert("Please select file")</script>';
    }
;?>

[astonecipher]

From the code level, you select the row to see what changes if any were made. Then you dynamically build the query to update the fields that were actually changed, or move on if nothing was changed. It’s a high overhead process to do it that way however, especially if you are doing it to multiple rows at a time.

[phdr]

For MySQL, and probably other databases, update queries check if the new value for a column is the same as the existing value and short-circuits the step of saving the data for that column, so you don’t need to -

If you set a column to the value it currently has, MySQL notices this and does not update it.

For any database that doesn’t do this, for the frequency that data is typically updated/edited, just go ahead and include all the columns in the query. You also have an issue with concurrent access to the database table when first SELECTing the data in order to decide what you should update.

Because you are editing existing data, the submitted $_POST field values should be either the existing value or an altered value. The only time an empty value should be submitted is if you want to allow the value to be updated to an empty string in order to clear it. Your update code should validate the $_POST fields using the same rules that you used when you inserted the data, then only execute the update query if there are no validation errors. Aside from the query type, the only other difference between the code for inserting data or updating data is the additional id input, that you should validate before using, for the update code.

For the file handling code, you MUST check if the upload was successful before using any of the uploaded file data. If the total size of the submitted form data exceeds the post_max_size setting, both the $_FILES and $_POST arrays will be empty (this affects the code using the $_POST variables too.) You must detect and handle this condition (setting up an error message for the visitor) before using any of the submitted form data.

You must then check the [‘error’] element in the $_FILES… data. If the error value is a zero (UPLOAD_ERR_OK), you can use the uploaded file information. If the error value is a 4 (UPLOAD_ERR_NO_FILE), it means that no file was selected to be uploaded. Since a file upload is NOT required for the update operation, you would ignore this error value. For all other error values, you need to take an appropriate action. See the php.net documentation for the list of error values. Your current code testing if the [‘name’] element is empty() or isset() doesn’t address if an error occurred. The name element will be set most of the time, even if it is empty, and for some of the possible errors, it will contain a value, even though the upload failed.

Based on the header redirect in the code, your form and form processing code are not on the same page. This results in extra code, on both pages, and results in a poor user experience. Put the form and form processing code on the same page, with the processing code above the start of the html document. Also, every header() redirect needs an exit statement after it to STOP program execution.