Insert or Update duplicating data

[Adamhumbug]

Hi All,

I have an insert/update that i am using in several places across my site, it works fine everywhere apart from on one page.

I am clearly missing something but cant for the life of me work it out.

My php:

if ($_SERVER['REQUEST_METHOD']=='POST') {
    $jobId = $_SESSION['current_job_id'];
    $qty = $_POST['drinkItemQty'];
    // prepare insert query
    $stmt = $conn->prepare("INSERT INTO ssm_drink_order (drink_qty, job_id, drink_id) 
                            VALUES (?,?,?) 
                            ON DUPLICATE KEY UPDATE 
                            drink_qty = VALUES(drink_qty)"
                        ); 
    
    
    foreach ($_POST['drinkItemId'] as $k => $diid) {

        if ($qty[$k] > 0) {

            $stmt->bind_param("iii", $qty[$k], $jobId, $diid);

            $stmt->execute();
        }

        if ($qty[$k] < 1) {

            $stmt1 =$conn->prepare("DELETE FROM ssm_drink_order WHERE job_id = ?");
            $stmt1->bind_param('i', $jobId);
            $stmt1->execute();
        }
    }   
}

the page html

<tbody>
    <tr>
        <th style="width:70%;" class="text-center">Drink Item</th>
        <th class="text-center">Quantity</th>
    </tr>

    <tr>
        <td>
            House Gin
            <input name="drinkItemId[]" type="hidden" value="2">
        </td>
        <td class="text-center">
            <input name="drinkItemQty[]" type="number" value="999" class="text-center">
        </td>
    </tr>
    <tr>
        <td>
            House Brandy
            <input name="drinkItemId[]" type="hidden" value="4">
        </td>
        <td class="text-center">
            <input name="drinkItemQty[]" type="number" value="" class="text-center">
        </td>
    </tr>
    <tr>
        <td>
            House Vodka
            <input name="drinkItemId[]" type="hidden" value="1">
        </td>
        <td class="text-center">
            <input name="drinkItemQty[]" type="number" value="" class="text-center">
        </td>
    </tr>
    <tr>
        <td>
            House Whiskey
            <input name="drinkItemId[]" type="hidden" value="3">
        </td>
        <td class="text-center">
            <input name="drinkItemQty[]" type="number" value="" class="text-center">
        </td>
    </tr>
</tbody>

Any help is as ever greatly appreciated

[ginerjm]

What have you done to debug this?  Any echos?  Any dumps of the vars in use during the query processing?  Anything?

The html is of no use to us if you simply dump the $_POST array to begin with.

[Adamhumbug]

var_dump of $_POST gives me

array(3) { ["money_update_button"]=> string(0) "" ["drinkItemId"]=> array(4) { [0]=> string(1) "2" [1]=> string(1) "4" [2]=> string(1) "1" [3]=> string(1) "3" } ["drinkItemQty"]=> array(4) { [0]=> string(3) "999" [1]=> string(0) "" [2]=> string(0) "" [3]=> string(0) "" } }

 

[Adamhumbug]

with the following

<?php

ini_set('display_errors', 1);
ini_set('display_startup_errors', 1);
error_reporting(E_ALL);

if (session_status() == PHP_SESSION_NONE) {
  session_start();
}

if (!isset($_SESSION['user_id'])){
    header("location: index.php");
}
mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
include '../_includes/dbconn.php';  
 
if ($_SERVER['REQUEST_METHOD']=='POST') {
    $jobId = $_SESSION['current_job_id'];
    $qty = $_POST['drinkItemQty'];
    // prepare insert query
    $stmt = $conn->prepare("INSERT INTO ssm_drink_order (drink_qty, job_id, drink_id) 
                            VALUES (?,?,?) 
                            ON DUPLICATE KEY UPDATE 
                            drink_qty = VALUES(drink_qty)"
                        ); 
    
    
    foreach ($_POST['drinkItemId'] as $k => $diid) {

        if ($qty[$k] > 0) {

            $stmt->bind_param("iii", $qty[$k], $jobId, $diid);

            $stmt->execute();
        }

        if ($qty[$k] < 1) {

            $stmt1 =$conn->prepare("DELETE FROM ssm_drink_order WHERE job_id = ?");
            $stmt1->bind_param('i', $jobId);
            $stmt1->execute();
        }
    }   
}

header("location: ../order-drink.php");

This is now removing everything from the database that was put in with the insert - effectively regardless of the quantity it thinks it is zero

 

var_dump($qty[$k]) gives me string(3) "100" string(0) "" string(0) "" string(0) ""

Edited January 7, 2020 by Adamhumbug

[ginerjm]

Ok - so you use that info to follow the rest of your php code and see what it does with the input.  Echo out any query statement that you build and try to execute.  Check the results of your query.  

What exactly is happening.  You say you have a problem but what is it?

[ginerjm]

I don't like either of your queries.  Do you know that they run without error?

[Adamhumbug]

3 minutes ago, ginerjm said:

I don't like either of your queries.  Do you know that they run without error?

i have run them in phpMyAdmin and i have mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);

i dont see any errors.

Its confusing me as the rest of my pages use this method.

[ginerjm]

Your form processing logic is flawed.  The data proves it.

You have 4 ids incoming but only one quantity.  Your logic does an update with the first pair of id/qty.  Then the remaining ids are processed but the qty < 1 so a delete is what you are asking for.

You should be checking the two input arrays and ensure that each key of your first array has a non-empty value in the second array.  That's called good programming practice.

[ginerjm]

PS - A better way to dump your arrays would be to do this:

	echo "POST is: <pre>" . print_r($_POST,true) . "</pre>";
	

[mac_gyver]

if you prepare the DELETE query, once, before the start of the loop, using the same conditions that the INSERT ... query is using to identify which row to operate on, you may see the mistake in your logic.

btw - each header() redirect needs an exit; statement after it to stop program execution. your current login check code is still running all the code on the page even is someone is not logged in.

[Adamhumbug]

27 minutes ago, mac_gyver said:

if you prepare the DELETE query, once, before the start of the loop, using the same conditions that the INSERT ... query is using to identify which row to operate on, you may see the mistake in your logic.

btw - each header() redirect needs an exit; statement after it to stop program execution. your current login check code is still running all the code on the page even is someone is not logged in.

thanks for the tip with the exit, i wasnt aware of that.

I am struggling a little to follow what you mean in your first line.  My understanding not your explanation.

[mac_gyver]

what is the overall goal of this code? if the quantity is greater-than zero, insert/update a row matching a job_id/item_id, otherwise, delete a row matching a job_id/item_id? is this what the code/queries are doing now?

note: a drink is a type/category of an item. your database table should be general-purpose and handle any type of item in an order. the table is for recording orders and should be named as such. also, the columns should just be named job_id, item_id, and quantity (or some abbreviation of those.)

also, if you use the item_id as the form field's array index value, you only need one set of form fields and no extra variables when you process the form data.

[Adamhumbug]

9 hours ago, mac_gyver said:

what is the overall goal of this code? if the quantity is greater-than zero, insert/update a row matching a job_id/item_id, otherwise, delete a row matching a job_id/item_id? is this what the code/queries are doing now?

note: a drink is a type/category of an item. your database table should be general-purpose and handle any type of item in an order. the table is for recording orders and should be named as such. also, the columns should just be named job_id, item_id, and quantity (or some abbreviation of those.)

also, if you use the item_id as the form field's array index value, you only need one set of form fields and no extra variables when you process the form data.

The goal is:

The user enters a qty of each product that they would like to order.

Click the submit button and the items are added to the order (submitted to the database)

If they need to amend the order, they can change the values and they will be updated.  

Its not doing anything complicated at all.

If there is a better way for me to be doing the whole thing i would appreciate any help as i am using this code in many places and just getting to grips with how it all works.

[Barand]

If it helps, this is how I would do it...

<?php
require 'db_inc.php';
$db = pdoConnect('humbug');

//
//   PROCESS POST DATA
//
if ($_SERVER['REQUEST_METHOD']=='POST') {
    $jobid = trim($_POST['jobid']);
    
    if (!empty($jobid)) {
        $placholders = [];
        $values = [];
        
        foreach ($_POST['qty'] as $did => $qty) {
            $placeholders[] = "(?,?,?)";
            array_push($values, $jobid, $did, intval($qty));
        }
        // single multiple-insert query
        $stmt = $db->prepare("INSERT IGNORE INTO ssm_drink_order 
                                  (job_id, drink_id, drink_qty)
                                  VALUES " . join(',', $placeholders) . 
                              "ON DUPLICATE KEY
                                  UPDATE drink_qty = VALUES(drink_qty) 
                             ");
        $stmt->execute($values);
        
        // clear zero qty ordere
        $db->exec("DELETE FROM ssm_drink_order WHERE drink_qty = 0");
    }
    
    // reload the form
    header("Location:?jobid=");
    exit;
}

//
//  PROCESS GET DATA
//
$jobid = $_GET['jobid'] ?? '';
$formdata = '';

    $stmt = $db->prepare("SELECT d.drink_id
                                 , d.drink_name
                                 , o.drink_qty
                            FROM ssm_drink d 
                                     LEFT JOIN
                                 ssm_drink_order o ON d.drink_id = o.drink_id
                                                   AND o.job_id = ?
                            ORDER BY d.drink_id
                            ");
    $stmt->execute([$jobid]);
    foreach ($stmt as $d) {
        $formdata .= "<label>{$d['drink_name']}</label>
                      <input type='number' name='qty[{$d['drink_id']}]' value={$d['drink_qty']}>
                      <br>
                     ";
    }
    

function jobOptions(PDO $db, $current)
{
    $res = $db->query("SELECT job_id, job_name FROM ssm_job");
    $opts = "<option value=''> - Choose Job - </option>\n";
    foreach ($res as $r) {
        $sel = $r['job_id'] == $current ? 'selected' : '';
        $opts .= "<option $sel value='{$r['job_id']}'>{$r['job_name']}</option>\n";
    }
    return $opts;
}  
?>
<!DOCTYPE html>
<html>
<head>
<title>Example Drinks Order</title>
<link rel="stylesheet" href="https://www.w3schools.com/w3css/4/w3.css">
<style type="text/css">
    label {color: #2DABE1; font-weight: 600; width: 250px; display: inline-block; }
    input[type='number']  { padding-right: 16px; text-align: right;}
</style>
</head>
<body>
    <header class="w3-blue w3-container w3-padding">
        <h1>Example Drinks Order</h1>
    </header>
    
    <form method="GET">
        <fieldset class="w3-content w3-border w3-padding w3-margin">
            <label>Job</label>
            <select class="w3-input w3-border" name="jobid" onchange="this.form.submit()">
                <?=jobOptions($db, $jobid)?>
            </select>
        </fieldset>
    </form>
    
    <form method="POST">
        <input type="hidden" name="jobid" value="<?=$jobid?>">
        <fieldset class="w3-content w3-border w3-padding w3-margin">
            <legend>Drinks Quantities</legend>
            <?=$formdata?>
            <br><br>
            <label>&nbsp;</label>
            <input type="submit" name="btnSub" value="Update Order">
        </fieldset>
    </form>
        
</body>
</html>

Note:  Assumes ssm_drink_order is

CREATE TABLE `ssm_drink_order` (
  `job_id` int(11) NOT NULL,
  `drink_id` int(11) NOT NULL,
  `drink_qty` int(11) DEFAULT NULL,
  PRIMARY KEY (`job_id`,`drink_id`)
)

or, at least, that (job_id, drink_id) is defined as UNIQUE

[Adamhumbug]

39 minutes ago, Barand said:

If it helps, this is how I would do it...

<?php
require 'db_inc.php';
$db = pdoConnect('humbug');

//
//   PROCESS POST DATA
//
if ($_SERVER['REQUEST_METHOD']=='POST') {
    $jobid = trim($_POST['jobid']);
    
    if (!empty($jobid)) {
        $placholders = [];
        $values = [];
        
        foreach ($_POST['qty'] as $did => $qty) {
            $placeholders[] = "(?,?,?)";
            array_push($values, $jobid, $did, intval($qty));
        }
        // single multiple-insert query
        $stmt = $db->prepare("INSERT IGNORE INTO ssm_drink_order 
                                  (job_id, drink_id, drink_qty)
                                  VALUES " . join(',', $placeholders) . 
                              "ON DUPLICATE KEY
                                  UPDATE drink_qty = VALUES(drink_qty) 
                             ");
        $stmt->execute($values);
        
        // clear zero qty ordere
        $db->exec("DELETE FROM ssm_drink_order WHERE drink_qty = 0");
    }
    
    // reload the form
    header("Location:?jobid=");
    exit;
}

//
//  PROCESS GET DATA
//
$jobid = $_GET['jobid'] ?? '';
$formdata = '';

    $stmt = $db->prepare("SELECT d.drink_id
                                 , d.drink_name
                                 , o.drink_qty
                            FROM ssm_drink d 
                                     LEFT JOIN
                                 ssm_drink_order o ON d.drink_id = o.drink_id
                                                   AND o.job_id = ?
                            ORDER BY d.drink_id
                            ");
    $stmt->execute([$jobid]);
    foreach ($stmt as $d) {
        $formdata .= "<label>{$d['drink_name']}</label>
                      <input type='number' name='qty[{$d['drink_id']}]' value={$d['drink_qty']}>
                      <br>
                     ";
    }
    

function jobOptions(PDO $db, $current)
{
    $res = $db->query("SELECT job_id, job_name FROM ssm_job");
    $opts = "<option value=''> - Choose Job - </option>\n";
    foreach ($res as $r) {
        $sel = $r['job_id'] == $current ? 'selected' : '';
        $opts .= "<option $sel value='{$r['job_id']}'>{$r['job_name']}</option>\n";
    }
    return $opts;
}  
?>
<!DOCTYPE html>
<html>
<head>
<title>Example Drinks Order</title>
<link rel="stylesheet" href="https://www.w3schools.com/w3css/4/w3.css">
<style type="text/css">
    label {color: #2DABE1; font-weight: 600; width: 250px; display: inline-block; }
    input[type='number']  { padding-right: 16px; text-align: right;}
</style>
</head>
<body>
    <header class="w3-blue w3-container w3-padding">
        <h1>Example Drinks Order</h1>
    </header>
    
    <form method="GET">
        <fieldset class="w3-content w3-border w3-padding w3-margin">
            <label>Job</label>
            <select class="w3-input w3-border" name="jobid" onchange="this.form.submit()">
                <?=jobOptions($db, $jobid)?>
            </select>
        </fieldset>
    </form>
    
    <form method="POST">
        <input type="hidden" name="jobid" value="<?=$jobid?>">
        <fieldset class="w3-content w3-border w3-padding w3-margin">
            <legend>Drinks Quantities</legend>
            <?=$formdata?>
            <br><br>
            <label>&nbsp;</label>
            <input type="submit" name="btnSub" value="Update Order">
        </fieldset>
    </form>
        
</body>
</html>

Note:  Assumes ssm_drink_order is

CREATE TABLE `ssm_drink_order` (
  `job_id` int(11) NOT NULL,
  `drink_id` int(11) NOT NULL,
  `drink_qty` int(11) DEFAULT NULL,
  PRIMARY KEY (`job_id`,`drink_id`)
)

or, at least, that (job_id, drink_id) is defined as UNIQUE

Thanks Barand, i will have a look at this and see if i can make it work for me.  My table structure is currently

CREATE TABLE `ssm_drink_order` (
  `surrogate_id` int(11) NOT NULL,
  `drink_id` int(11) NOT NULL,
  `drink_qty` int(11) NOT NULL,
  `job_id` int(11) NOT NULL
) ;

ALTER TABLE `ssm_drink_order`
  ADD PRIMARY KEY (`surrogate_id`),
  ADD UNIQUE KEY `drink_id` (`drink_id`),
  ADD UNIQUE KEY `job_id` (`job_id`);

 

[Barand]

You certainly need to change those key definitions.

Unique drink id means you cannot have the same drink id for multiple jobs, and similarly for unique job id.

It is the combination that needs to be unique.

 

[Adamhumbug]

35 minutes ago, Barand said:

You certainly need to change those key definitions.

Unique drink id means you cannot have the same drink id for multiple jobs, and similarly for unique job id.

It is the combination that needs to be unique.

 

Thank you for this pointer - food for thought re: the rest of my tables.

[Adamhumbug]

The following is giving me the results that i want and also means that the submit and update actions can be the same.

<?php

ini_set('display_errors', 1);
ini_set('display_startup_errors', 1);
error_reporting(E_ALL);

if (session_status() == PHP_SESSION_NONE) {
  session_start();
}

if (!isset($_SESSION['user_id'])){
    header("location: index.php");
}
mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
include '../_includes/dbconn.php';  
 
if ($_SERVER['REQUEST_METHOD']=='POST') {
    $jobId = $_SESSION['current_job_id'];
    $qty = $_POST['drinkItemQty'];


    // prepare insert query
    $stmt = $conn->prepare("INSERT IGNORE INTO ssm_drink_order (job_id, drink_id, drink_qty) VALUES (?,?,?) ON DUPLICATE KEY
                                  UPDATE drink_qty = VALUES(drink_qty) ");
    
    
    foreach ($_POST['drinkItemId'] as $k => $diid) {

        if ($qty[$k] > 0) {

            $stmt->bind_param("iii", $jobId, $diid, $qty[$k]);

            $stmt->execute();
        }
        if ($qty[$k] < 1) {

            $stmt1 =$conn->prepare("DELETE FROM ssm_drink_order WHERE job_id = ? and drink_id = ?");
            $stmt1->bind_param('ii', $jobId, $diid);
            $stmt1->execute();
        }
    }   
        


        
    
    
}

header("location: ../order-drink.php");