0

I am creating a Purchase Orders form/database for a small tool and die shop I work for. I have a form setup with drop down boxes for Company, Customer Name, Tool, and Part. Alongside these dropdowns are text boxes that the user can input new data into the database. Currently I am using a PHP file with about 13 if statements to determine what input boxes are empty and where data needs to be inserted.

Here is part of my PHP file with just 2 of the if statements:

<?php

$servername = "localhost";
$username = "root";
$password = "password";
$dbname = "Purchase_Orders";

$NewCompany=$_POST['NewCompany'];
$NewCustomer=$_POST['NewCustomer'];
$NewTool=$_POST['NewTool'];
$NewPart=$_POST['NewPart'];

$Company=$_POST['Company'];
$Customer=$_POST['Customer'];
$Tool=$_POST['Tool'];



// NewCompany added by itself with NewCustomer, NewTool, and NewPart empty
if (!empty($NewCompany) && empty($NewCustomer) && empty($NewTool) && empty($NewPart)) {

$conn = new mysqli($servername, $username, $password, $dbname);

if ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
}        

$sql = "INSERT INTO Purchase_Orders_dynlist_items (listid,name,value,parent) VALUES ('1', '$NewCompany', '$NewCompany',NULL), ('2', '', '', '$NewCompany'), ('3', '', '', '$NewCompany')";

if ($conn->query($sql) === TRUE) {
    header( 'Location: index.html');
    exit;
} else {
    echo "Error: " . $sql . "<br>" . $conn->error;
}
}


// NewCompany and NewCustomer added with NewTool and NewPart empty
if (!empty($NewCompany) && !empty($NewCustomer) && empty($NewTool) && empty($NewPart)) {

$conn = new mysqli($servername, $username, $password, $dbname);

if ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
}        

$sql = "INSERT INTO Sales_Orders_dynlist_items (listid,name,value,parent) VALUES ('1', '$NewCompany', '$NewCompany',NULL), ('2', '', '', '$NewCompany'), ('2', '$NewCustomer', '$NewCustomer', '$NewCompany'), ('3', '', '', '$NewCompany')";

if ($conn->query($sql) === TRUE) {
    header( 'Location: index.html');
    exit;
} else {
    echo "Error: " . $sql . "<br>" . $conn->error;
}
}

    else {
    echo '<meta http-equiv="refresh" content="0;url=purchase_orders/"/>';
    exit;
}

$conn->close();


?>

So, my question here is can I simplify this code to where it will only input the data when it is not empty. So for instance instead of if (!empty($NewCompany) && empty($NewCustomer) && empty($NewTool) && empty($NewPart)) { could I just have it enter what data is inputted? In other words, instead of having 13 different if statements, only enter data based on what input boxes are not blank.

I was trying something like this:

if (!empty($NewCompany)) {
$conn->query("INSERT INTO Purchase_Orders_dynlist_items (listid,name,value,parent) VALUES ('1', '$NewCompany', '$NewCompany',NULL)");
}

if (!empty($NewCustomer)) {
$conn->query("INSERT INTO Purchase_Orders_dynlist_items (listid,name,value,parent) VALUES ('2', '$NewCustomer', '$NewCustomer', '$NewCompany')");
}

But this was only doing one OR the other. If both of those input fields had data, the data would not be inputted. I am aware that this code is vulnerable to SQL Injection. This is only going to be run/used on our local network, so it's not something I'm overly worried about.

Maybe someone can give me a push in the right direction? Thanks!

6
  • Your code is vulnerable to SQL Injection! You should not run this on a production website. bobby-tables.com Commented Feb 13, 2018 at 15:08
  • Just a hunch: Overwrite the "emtpy" variables with "NULL", that way you simply add this to the database row. Commented Feb 13, 2018 at 15:09
  • Thank you @Cfreak I am aware of this, but this is only going to be run on our local network, so it's not something I was overly concerned about. SQL Injection is something I am curious about, and need to look more into. Commented Feb 13, 2018 at 15:10
  • It doesn't matter if its only on local or not. Visit bobby-tables.com - Look at the examples how to prevent injections. It will take you around 3-5 minutes to learn it and may another 2 minutes to adapt your statement. Once you've done this, simply never write a query with user inputs without preparing it again. Its a really simple and easy topic and it can be done so fast. Its really nothing difficult and should be done at any given time and situation, not regarding the network you're working in. Its good practice and you'll start to do it automatically. Commented Feb 13, 2018 at 15:20
  • why not use a ternary operator? As for your: "This is only going to be run/used on our local network, so it's not something I'm overly worried about." - Even I don't take that chance, internal network or not, or someone I know. Commented Feb 13, 2018 at 15:20

1 Answer 1

1

You could try the following approach, which builds up an array of the data you want to insert and then iterates over this list with a prepared statement and passing each set of data to the insert...

$data = [];
if (!empty($NewCompany)) {
    $data[] = [ '1', $NewCompany, $NewCompany, NULL ];
}
if (!empty($NewCustomer)) {
    $data[] = [ '2', $NewCustomer, $NewCustomer, $NewCompany ];
}
$insert = $conn->prepare("INSERT INTO Purchase_Orders_dynlist_items (listid,name,value,parent) 
    VALUES (?, ?, ?,?)");
if ( $insert === false )    {
    echo "Error:".$conn->error;
    die;
}
foreach ( $data as $item ){
    if ( $insert->bind_param("isss", ...$item) === false )    {
        echo "Error:".$conn->error;
    }
    if ( $insert->execute() === false )    {
        echo "Error:".$conn->error;
    }
}

This allows you to build up various configurations of the data for each item, if there is a standard pattern, you can extract this out to a function...

function addData( $field, &$data )  {
    if ( !empty($field) )   {
        $data[] = [ '1', $field, $field, NULL ];
    }
}

addData($NewCompany, $data );
addData($NewCustomer, $data );
Sign up to request clarification or add additional context in comments.

7 Comments

Alright, this is logical. Now, I have tried your top code, and I can't seem to get it to Insert the data
Is your listid an autoincrement key, as this may be a problem.
No, listid is not an autoincrement key. ID is an autoincrement key in the table.
OK, try the updated code, as I say, I screwed up with the binding bit. ( I'm assuming the first value is an integer - hence the 'isss', if it's a string, then change it to 'ssss' in the bind_param.
I must be missing something :( I have updated, and changed 'isss' to 'ssss' in the bind_param.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.