Jump to content

PHP MySQL update query not working if not all form fields completed


johnc81
Go to solution Solved by mac_gyver,

Recommended Posts

Hello,

I have a PHP form which I am posting to update an existing record. This is a sales order form and will be updated with data as it becomes available. The form will have missing data when it is posted (input / select fields left empty). The problem is that unless I complete all the fields, the record is not updating. 

Am I missing something? All the fields I have left empty are allowed NULLs in the table. They are a mixture of INT, DATE and VARCHAR data types. I have a mix of input=text and select dropdown fields in my form.

Please can someone let me know how this process normally works because I assumed it would just fill in what it could and leave the rest as NULL?

Many Thanks,

J

Link to comment
Share on other sites

once the form has been submitted, except for unchecked checkbox/radio fields, all the fields will be set and they will be a string data type, regardless of what value they contain. empty always-set fields will be an empty string. unless you have code setting the values to null and are using those values in the sql query statement in such as way that the null value will get used as is, e.g. not quoted, or you are dynamically building the sql query statement and leaving the fields out, or are explicitly using the DEFAULT keyword for the value, nulls or the defined default value won't get used.

you will need to post all the code we would needed, form and form processing, to reproduce the problem, along with your database table definition for anyone here to directly help.

Link to comment
Share on other sites

Hi,

Thank you for your response. I will post the code below.

This is the form page, test.php. When I complete the mandatory fields and leave the remaining ones empty, I get the message to say the record has been created, but there is no row in the database. I do not receive any errors.

<?php
ERROR_REPORTING(E_ALL);
ini_set('display_errors', 1);

include 'db/connect.php';
include 'includes/rowcolours.php';

session_start();
if(isset($_SESSION['orderIDx'])) {
	$orderIDx = $_SESSION['orderIDx'];
	$orderActivex = $_SESSION['orderActivex'];
} else {
	$orderIDx = 0;
	$orderActivex = 0;
}
session_destroy();

// Create a new sales order header
if(isset($_POST['createsalesorder'])) {
	foreach($_POST as $key=>$value) {
		$$key = $value;
	}
	
	if (isset($_POST['orderPaid'])) {
		$orderPaidx = 1;
	} else {
		$orderPaidx = 0;
	}

	if($shopID == '' || $orderReference == '' || $orderDate == '' || $customerName == '') {
		header('Location: test.php?flag=2');
	} else {
		$createsalesorder = "INSERT INTO tbl_order (shopID, orderReference, orderDate, customerName, customerAddress, countryID, paymentMethodID, orderPaid, orderPickDate, orderDespatchDate, shipperID, orderShippingReference, orderShippingCost, orderActive) VALUES ('$shopID', '$orderReference', '$orderDate', '$customerName', '$customerAddress', '$countryID', '$paymentMethodID', '$orderPaidx', '$orderPickDate', '$orderDespatchDate', '$shipperID', '$orderShippingReference', '$orderShippingCost', 1)";
		mysqli_query($conn,$createsalesorder);
		$lastID = $conn->insert_id;
		session_start();
		$_SESSION['orderIDx'] = $lastID;
		$_SESSION['orderActivex'] = 1;

		if($createsalesorder){
			header('Location: test.php?flag=1');
		} else {
			print 'ERROR : ('. $mysqli->errno .') '. $mysqli->error;
		}
	}
}
?>

<!DOCTYPE html>
<html lang="en">
<head>
	<title>Manage Sales Orders</title>
	<link rel="stylesheet" type="text/css" href="css/style.css" />
</head>
<body>
	<?php include 'includes/navbar.php'; 
		echo '<div><h1>Manage Sales Orders</h1></div>';
		
		if(isset($_GET['flag'])) {
			if ($_GET['flag'] == 1) { echo '<div class="flagresponse">Sales order created</div>'; }
			if ($_GET['flag'] == 2) { echo '<div class="flagresponse">Missing data. Record not created</div>'; }
			if ($_GET['flag'] == 3) { echo '<div class="flagresponse">Sales order updated</div>'; }
			if ($_GET['flag'] == 4) { echo '<div class="flagresponse">Sales order deleted</div>'; }
			if ($_GET['flag'] == 5) { echo '<div class="flagresponse">Sales order line created</div>'; }
			if ($_GET['flag'] == 6) { echo '<div class="flagresponse">Missing data. Record not updated</div>'; }
			if ($_GET['flag'] == 7) { echo '<div class="flagresponse">Sales order line updated</div>'; }
			if ($_GET['flag'] == 8) { echo '<div class="flagresponse">Sales order line deleted</div>'; }
			if ($_GET['flag'] == 9) { echo '<div class="flagresponse">Sales order closed</div>'; }
		}
	?>
	
	<?php if($orderIDx == 0) {
		echo '<h2>Create New Sales Order</h2>
		<form name="createsalesorder" action="" enctype="multipart/data" method="POST" id="createsalesorder">
			<div class="divTable">
				<div class="divTableBody">
					<div class="divTableHeader">
						<div class="div250px"><h3>Shop</h3></div>
						<div class="div250px"><h3>Order Ref</h3></div>
						<div class="div150px"><h3>Order Date</h3></div>
					</div>
				</div>
			</div>
			<div class="divTable">
				<div class="divTableBody">
					<div class="divTableRow" style="background-color: '.$colourrow1.'">
						<div class="div250px">
							<select name="shopID" class="select250px" required>
								<option name="" value="">Select...</option>';
									$sql = "SELECT shopID, shopName FROM tbl_shops";
									$result = $conn->query($sql);
									if ($result->num_rows > 0) {
										while($row = $result->fetch_assoc()) {	
											echo '<option value="'.$row['shopID'].'">'.$row['shopName'].'</option>'; 
										}
									} else {
										echo '0 Results';
									}
							echo '</select>
						</div>
						<div class="div250px"><input id="input250px" type="text" name="orderReference" id="orderReference" required /></div>
						<div class="div150px"><input id="input150px" type="date" name="orderDate" id="orderDate" required /></div>
					</div>
				</div>
			</div>
			<p />
			<div class="divTable">
				<div class="divTableBody">
					<div class="divTableHeader">
						<div class="div250px"><h3>Customer Name</h3></div>
						<div class="div600px"><h3>Address</h3></div>
						<div class="div250px"><h3>Country</h3></div>
						<div class="div250px"><h3>Payment Method</h3></div>
						<div class="div100px"><h3 style="text-align: center;">Paid</h3></div>
						<div class="div150px"><h3>Pick Date</h3></div>
					</div>
				</div>
			</div>
			<div class="divTable">
				<div class="divTableBody">
					<div class="divTableRow" style="background-color: '.$colourrow1.'">
						<div class="div250px"><input id="input250px" type="text" name="customerName" id="customerName" required /></div>
						<div class="div600px"><input id="input600px" type="text" name="customerAddress" id="customerAddress" /></div>
						<div class="div250px">
							<select name="countryID" class="select250px">
								<option name="" value="">Select...</option>';
									$sql = "SELECT countryID, countryName FROM tbl_countries";
									$result = $conn->query($sql);
									if ($result->num_rows > 0) {
										while($row = $result->fetch_assoc()) {	
											echo '<option value="'.$row['countryID'].'">'.$row['countryName'].'</option>'; 
										}
									} else {
										echo '0 Results';
									}
							echo '</select>
						</div>
						<div class="div250px">
							<select name="paymentMethodID" class="select250px">
								<option name="" value="">Select...</option>';
									$sql = "SELECT paymentMethodID, paymentMethodName FROM tbl_paymentmethods";
									$result = $conn->query($sql);
									if ($result->num_rows > 0) {
										while($row = $result->fetch_assoc()) {	
											echo '<option value="'.$row['paymentMethodID'].'">'.$row['paymentMethodName'].'</option>'; 
										}
									} else {
										echo '0 Results';
									}
							echo '</select>
						</div>
						<div class="div100px"><input id="input100px" class="checkbox" type="checkbox" value="1" name="orderPaid" id="orderPaid" /></div>
						<div class="div150px"><input id="input150px" type="date" name="orderPickDate" id="orderPickDate" /></div>
					</div>
				</div>
			</div>
			<p />
			<div class="divTable">
				<div class="divTableBody">
					<div class="divTableHeader">
						<div class="div250px"><h3>Shipper</h3></div>
						<div class="div250px"><h3>Shipper Ref</h3></div>
						<div class="div150px"><h3>Shipping Cost</h3></div>
						<div class="div150px"><h3>Despatch Date</h3></div>
						<div class="div100px"><h3>Create</h3></div>
					</div>
				</div>
			</div>
		
			<div class="divTable">
				<div class="divTableBody">
					<div class="divTableRow" style="background-color: '.$colourrow1.'">
						<div class="div250px">
							<select name="shipperID" class="select250px">
								<option name="" value="">Select...</option>';
									$sql = "SELECT shipperID, shipperName FROM tbl_shippers";
									$result = $conn->query($sql);
									if ($result->num_rows > 0) {
										while($row = $result->fetch_assoc()) {	
											echo '<option value="'.$row['shipperID'].'">'.$row['shipperName'].'</option>'; 
										}
									} else {
										echo '0 Results';
									}
							echo '</select>
						</div>
						<div class="div250px"><input id="input250px" type="text" name="orderShippingReference" id="orderShippingReference" /></div>
						<div class="div150px"><input id="input150px" type="text" name="orderShippingCost" id="orderShippingCost" /></div>
						<div class="div150px"><input id="input150px" type="date" name="orderDespatchDate" id="orderDespatchDate" /></div>
                        <div class="div100px"><input type="submit" value="Create" name="createsalesorder" id="submitbtn" /></div>
					</div>
				</div>
			</div>
		</form>';
	} ?>
</body>
</html>

 

There are 4 includes, firstly connect.php

<?php
$servername = "localhost";
$username = "USERNAME";
$password = "PASSWORD";
$dbname = "testdb";

// Create connection
$conn = new mysqli($servername, $username, $password, $dbname);
// Check connection
if ($conn->connect_error) {
  die("Connection failed: " . $conn->connect_error);
}
?>

 

Second, rowcolours.php

<?php
	$colourrow1 = "#dddddd";
	$colourrow2 = "#efefef";
	$colour = '';
?>

 

Third, navbar.php

<?php
	echo '<div id="navbar">
		<div><a id="navbar" href="index.php">Home</a></div>
		</div>';
?>

 

Fourth, the CSS, style.css

body {
	font-family: arial,verdana,helvetica,sans-serif;
	font-size: 10pt;
}

h1 {
	font-size: 14pt;
	text-align: left;
	margin: 10px 0px 4px 0px;
}

h2 {
	font-size: 11pt;
	text-align: left;
	margin: 20px 0px 4px 0px;
}

h3 {
	font-size: 10pt;
	text-align: left;
	margin: 5px 0px 2px 0px;
}

label {
	font-size: 10pt;
	font-weight: bold;
}

input,
select {
	font-family: "Lucida Sans Typewriter", "Lucida Console", monaco, "Bitstream Vera Sans Mono", monospace;
	width: 400px;
	height: 20px;
	margin: 2px 0px;
	-moz-box-sizing: border-box;
	-webkit-box-sizing: border-box;
	box-sizing: border-box;
}

.select250px {
	font-family: "Lucida Sans Typewriter", "Lucida Console", monaco, "Bitstream Vera Sans Mono", monospace;
	width: 250px;
	height: 20px;
	margin: 2px 0px;
	-moz-box-sizing: border-box;
	-webkit-box-sizing: border-box;
	box-sizing: border-box;
}

.select600px {
	font-family: "Lucida Sans Typewriter", "Lucida Console", monaco, "Bitstream Vera Sans Mono", monospace;
	width: 600px;
	height: 20px;
	margin: 2px 0px;
	-moz-box-sizing: border-box;
	-webkit-box-sizing: border-box;
	box-sizing: border-box;
}

.checkbox {
	padding: 0px;
	margin: 0px;
	position: relative;
    vertical-align: middle;
	width: 14px;
	height: 14px;
}

input#submitbtn {
	width: 100px;
}

input#submitbtn200px {
	width: 200px;
}

button {
	width: 100px;
}

input#sml {
	width: 50px;
}

input#med {
	width: 100px;
}

td#formlabel {
	width: 100px;
}

td#inputfield {
	width: 500px;
}

div#navbar {
	width: 100%;
	background-color: #333333;
	padding: 10px;
}

a:link#navbar, a:visited#navbar {
	background-color: #f44336;
	font-size: 14pt;
	color: white;
	padding: 14px 25px;
	text-align: center;
	text-decoration: none;
	display: inline-block;
	margin-right: 10px;
}

a:hover#navbar, a:active#navbar {
	background-color: red;
}

.grid {
  display: flex;
  width: 100%;
  flex-wrap: wrap;
}

.content {
  color: #333333;
  background-color: #dddddd;
  font-weight: bold;
  text-align: center;
  height: 150px;
  padding: 10px;
  width: 150px;
  border: 10px solid #333333;
  border-radius: 7px;
  margin: 10px;
}

#blackbox:hover {
	opacity: 0.6;
	transition: 0.3s;
}

#blackbox {
	width: 100%;
	height: 100%;
	opacity: 1;
	transition: 0.3s;
}

.formlabel {
  width:100px;
  display: inline-flex;
}

.divTable{
	display: table;
	padding: 0 0 0 10px;
}

.divTableHeader {
	display: table-row;
	background-color: #333333;
	color: #ffffff;
}

.divTableRow {
	display: table-row;
	padding: 10px;
}

.divTableFooter {
	display: table-row;
	background-color: #333333;
	color: #ffffff;
}

.divTableBody {
	display: table-row-group;
}

.div100px {
	width: 100px;
	display: table-cell;
	padding: 2px 5px 2px 5px;
}

input#input100px {
	width: 100px;
}

.div150px {
	width: 150px;
	display: table-cell;
	padding: 2px 5px 2px 5px;
}
input#input150px {
	width: 150px;
}

.div200px {
	width: 200px;
	display: table-cell;
	padding: 2px 5px 2px 5px;
}

input#input200px {
	width: 200px;
}

.div250px {
	width: 250px;
	display: table-cell;
	padding: 2px 5px 2px 5px;
}
input#input250px {
	width: 250px;
}

.div300px {
	width: 300px;
	display: table-cell;
	padding: 2px 5px 2px 5px;
}
input#input300px {
	width: 300px;
}

.div400px {
	width: 400px;
	display: table-cell;
	padding: 2px 5px 2px 5px;
}

input#input400px {
	width: 400px;
}

.div600px {
	width: 600px;
	display: table-cell;
	padding: 2px 5px 2px 5px;
}

input#input600px {
	width: 600px;
}

.flagresponse {
	color: #dd0000;
	font-size: 10pt;
	font-weight: bold;
	padding: 5px 0 5px 0;
}

hr {
    border: 0;
    height: 1px;
    background: #777777;
    background-image: linear-gradient(to right, #ccc, #777, #ccc);
	margin: 20px 0 20px 0;
}

.hrreport {
    border: 0;
    height: 1px;
    background: #777777;
    background-image: linear-gradient(to right, #ccc, #777, #ccc);
	margin: 4px 0 4px 0;
}

.hrreporttotal {
    border: 0;
    height: 2px;
    background: #777777;
    background-image: linear-gradient(to right, #ccc, #777, #ccc);
	margin: 4px 0 4px 0;
}

h2.bgcolour {
	background-color: #cccccc;
	padding: 5px 5px 5px 10px;
}

.closebutton {
	color: #f44336;
	font-weight: bold;
}

.txtright {
	text-align: right;
}

.boldfont {
	font-weight: bold;
}

 

The structure for the sales order table, tbl_order. The columns that are not required in the initial record create are all set to allow NULL

DROP TABLE IF EXISTS `tbl_order`;
CREATE TABLE IF NOT EXISTS `tbl_order` (
  `orderID` int(11) NOT NULL AUTO_INCREMENT,
  `shopID` int(11) NOT NULL,
  `orderReference` varchar(80) NOT NULL,
  `orderDate` date NOT NULL,
  `customerName` varchar(80) NOT NULL,
  `customerAddress` text,
  `countryID` int(11) DEFAULT NULL,
  `paymentMethodID` int(11) DEFAULT NULL,
  `orderPaid` tinyint(1) DEFAULT NULL,
  `orderPickDate` date DEFAULT NULL,
  `orderDespatchDate` date DEFAULT NULL,
  `shipperID` int(11) DEFAULT NULL,
  `orderShippingReference` varchar(80) DEFAULT NULL,
  `orderShippingCost` decimal(7,2) DEFAULT NULL,
  `orderActive` tinyint(1) NOT NULL,
  PRIMARY KEY (`orderID`)
) ENGINE=InnoDB AUTO_INCREMENT=1 DEFAULT CHARSET=utf8;

 

The shops table, tbl_shops

DROP TABLE IF EXISTS `tbl_shops`;
CREATE TABLE IF NOT EXISTS `tbl_shops` (
  `shopID` int(11) NOT NULL AUTO_INCREMENT,
  `shopName` varchar(45) NOT NULL,
  `shopActive` tinyint(1) NOT NULL,
  PRIMARY KEY (`shopID`)
) ENGINE=InnoDB AUTO_INCREMENT=3 DEFAULT CHARSET=utf8;

--
-- Dumping data for table `tbl_shops`
--

INSERT INTO `tbl_shops` (`shopID`, `shopName`, `shopActive`) VALUES
(1, 'Ebay', 1),
(2, 'Etsy', 1);

 

The Country table, tbl_countries

DROP TABLE IF EXISTS `tbl_countries`;
CREATE TABLE IF NOT EXISTS `tbl_countries` (
  `countryID` int(11) NOT NULL AUTO_INCREMENT,
  `countryName` varchar(50) NOT NULL,
  `twoCharCountryCode` char(2) NOT NULL,
  `threeCharCountryCode` char(3) NOT NULL,
  `countryActive` tinyint(1) NOT NULL,
  PRIMARY KEY (`countryID`)
) ENGINE=InnoDB AUTO_INCREMENT=4 DEFAULT CHARSET=utf8;

--
-- Dumping data for table `tbl_countries`
--

INSERT INTO `tbl_countries` (`countryID`, `countryName`, `twoCharCountryCode`, `threeCharCountryCode`, `countryActive`) VALUES
(1, 'United Kingdom', 'GB', 'GBR', 1),
(2, 'Denmark', 'DK', 'DNK', 1),
(3, 'France', 'FR', 'FRA', 1);

 

The payment methods table, tbl_paymentmethods

DROP TABLE IF EXISTS `tbl_paymentmethods`;
CREATE TABLE IF NOT EXISTS `tbl_paymentmethods` (
  `paymentMethodID` int(11) NOT NULL AUTO_INCREMENT,
  `paymentMethodName` varchar(45) NOT NULL,
  `paymentMethodActive` tinyint(1) NOT NULL,
  PRIMARY KEY (`paymentMethodID`)
) ENGINE=InnoDB AUTO_INCREMENT=4 DEFAULT CHARSET=utf8;

--
-- Dumping data for table `tbl_paymentmethods`
--

INSERT INTO `tbl_paymentmethods` (`paymentMethodID`, `paymentMethodName`, `paymentMethodActive`) VALUES
(1, 'PayPal', 1),
(2, 'Bank Transfer', 1),
(3, 'Cash', 1);

 

The shippers table, tbl_shippers

DROP TABLE IF EXISTS `tbl_shippers`;
CREATE TABLE IF NOT EXISTS `tbl_shippers` (
  `shipperID` int(11) NOT NULL AUTO_INCREMENT,
  `shipperName` varchar(45) NOT NULL,
  `shipperActive` tinyint(1) NOT NULL,
  PRIMARY KEY (`shipperID`)
) ENGINE=InnoDB AUTO_INCREMENT=6 DEFAULT CHARSET=utf8;

--
-- Dumping data for table `tbl_shippers`
--

INSERT INTO `tbl_shippers` (`shipperID`, `shipperName`, `shipperActive`) VALUES
(1, 'Royal Mail', 1),
(2, 'Evri', 1),
(3, 'DPD', 1),
(4, 'DHL', 1),
(5, 'FedEx', 1);

 

I think this is everything to be able to recreate the scenario above. Any support with this would be really appreciated. I have been struggling with this for a couple of days and am out of ideas. I have tried using if statements for each field being posted to see if it is empty and then setting default values. I have tried using mysqli and PDO but nothing seems to work.

Thanks,

J

Link to comment
Share on other sites

  • Solution

the main problem with the current code is that it doesn't have any 'working' error handling for the insert query. the error handling you do have for that query is testing if the sql query statement, which is a string, is a true value, which it always will be. instead, use exceptions for database statement error handling and only catch and handle the exception for user recoverable errors, such as when inserting/updating duplicate or out of range user submitted data values. in all other cases, simply let php catch and handle the exception, where php will use its error related settings to control what happens with the actual error information (database statement errors will 'automatically' get displayed/logged the same as php errors.) you would then remove any existing database error handling conditional logic in your code (for the connection and insert query), since it will no longer get execution upon an error.

if you are familiar with the PDO extension, use it. it is much simpler and more modern than the mysqli extension, especially when dealing with prepared queries, which you should be using, since they provide protection against sql special characters in data values, for all data types, from breaking the sql query syntax.

code for any page should be laid out in this general order -

  1. initialization.
  2. post method form processing.
  3. get method business logic - get/produce data needed to display the page.
  4. html document.

your code generally follows this, but it has things like two session_start() statements, that needs to be cleaned up.

the post method form processing should -

  1. detect if a post method form has been submitted before referencing any of the form data.
  2. keep the form data as a set in a php array variable, then operate on elements in this array variable throughout the rest of the code.
  3. trim all the input data, mainly so that you can detect if it consists of all white-space characters.
  4. validate inputs, storing validation errors in an array using the field name as the array index.
  5. after the end of the validation logic, if there are no errors, use the form data.
  6. after using the form data, if there are no errors, perform a redirect to the exact same url of the current page to cause a get request for that page.
  7. any redirect needs an exit/die statement after it to stop code execution.
  8. to display a one-time success message, store it in a session variable, then test, display, and clear the session variable at the appropriate location in the html document.
  9. if there are errors at step #5 or #6 on this list, the code would continue on to display the html document, where you would display any errors and redisplay the form, populating the form field values with any existing data.
  10. since there won't be any existing data values the first time the form is displayed, you need to address this at the point of using the values in the form. php's null coalescing operator ?? is a good choice to use here.
  11. any external, dynamic, unknown value output in a html context should have htmlentities() applied to it to help prevent cross site scripting.

here's a laundry list of addition things for the posted code -

  1. the php error related settings should be in the php.ini on your system
  2. use 'require' for things your code must have for it to work
  3. creating a sales order is an administrative activity. you should have a login and user permission system to control access to this operation
  4. don't copy variables to other variables for nothing, just use the original variables
  5. don't use multiple variables to indicate something when one will work, e.g. orderIDx/orderActivex
  6. the session can hold data from different parts of your application. don't completely destroy the session, only unset specific things when needed. since you will be directly testing/using the session variable, instead of copying them to other variables, you won't be destroying or unsetting session variables for the part of the code you have shown.
  7. don't blindly loop over external data. hackers can cause 1000's of post variables to be submitted to your code. instead, only operate on expected post entries. you should actually be using a data-driven design, where you have an array that defines the expected fields, their validation, and processing, that you would loop over to dynamically do this operation.
  8. don't use variable variables, especially on external data. this allows hackers to set ANY of your program variables to anything they want.
  9. don't redirect around on your site, accepting values in the url to control what gets displayed as the result of performing an operation. this opens your site to phishing attacks.
  10. if you have a need for using numerical values to indicate which of multiple states something is in, use defined constants with meaningful names, so that anyone reading the code can tell what the values mean
  11. in the validation code, setup a unique and helpful error message for each validation error for each input, i.e. don't make the user guess what was wrong with the submitted data
  12. if any of the fields/columns must be unique, define them as unique indexes in the database table, then test in the database exception error handling if the query produced a duplicative index error number (1062 if i remember correctly.) for all other error numbers, just re-throw the exception and let php handle it.
  13. an empty action="" attribute is actually not valid html5. to cause the form to submit to the same page, leave out the entire action attribute
  14. don't put name or id attributes into the markup unless they are used
  15. you should validate the resulting web pages at validator.w3.org
  16. <option tags don't have name attributes
  17. you can put php variables directly into over-all double-quoted strings, without all the extra concatenation dots and quotes.
  18. just about every SELECT query should have an ORDER BY ... term so that the rows in the result set are in a desired order
  19. don't echo static html. just drop out of php 'mode'
  20. when you build and output the <option lists, output the selected attribute for the option that matches the existing form data
  21. when you output the checkbox field, you would output the checked attribute if that field is set in the existing form data
  22. the point where you are using - echo '0 Results'; won't display anything because it is inside the <select></select> tags. if there are no option choices, you should probably not even output the markup for the entire field, and output the message instead
  23. when conditional logic 'failure' code is much shorter then the 'success' code, invert the condition being tested and put the 'failure' code first. this will make your code clearer and cleaner

 

Link to comment
Share on other sites

Hi,

Thank you for the detailed explanation. There is actually a lot more code to this page, but I cut it out for the purposes of showing the issue. For example, the session variable orderIDx is used to get the order, but orderActiveX is used to determine if it is open or closed and displays differently (editable or view mode). I haven't included a login because this website will never go online, it will be run locally and only by me.

I tried to use PDO, but I couldn't get anything to work with that, hence why I decided to stick with mysqli. I would have preferred PDO but I every example I look at only does it a different way and I have no idea what is correct :(

I am not sure what this means: "your code generally follows this, but it has things like two session_start() statements, that needs to be cleaned up." I start a session once the form has been posted, but unless I start another one after the page reloads, how do I get the use the session variables?

I am also not sure how to change the code to see any errors coming from the form. It doesn't create the record in MySQL, so I guess there is something wrong and an error should be displayed?

Is there a tutorial or something I can look at to show what good looks like because I am learning all this by following stuff online, but clearly it is not good.

I just want to be able to see what the issue is and why it won't post so I can start trying to fix that.

Thanks,

J

Link to comment
Share on other sites

Hi,

I am trying to work through the post method form processing you have above, using PDO:

// detect if a post method form has been submitted before referencing any of the form data.
if(isset($_POST['createsalesorder'])) {
	
	// keep the form data as a set in a php array variable, then operate on elements in this array variable throughout the rest of the code.
	$data = [
		'shopID' => $_POST['shopID'],
		'orderReference' => $_POST['orderReference'],
		'orderDate' => $_POST['orderDate'],
		'customerName' => $_POST['customerName'],
		'customerAddress' => $_POST['customerAddress'],
		'countryID' => $_POST['countryID'],
		'paymentMethodID' => $_POST['paymentMethodID'],
		'orderPaid' => 0,
		'orderPickDate' => $_POST['orderPickDate'],
		'orderDespatchDate' => $_POST['orderDespatchDate'],
		'shipperID' => $_POST['shipperID'],
		'orderShippingReference' => $_POST['orderShippingReference'],
		'orderShippingCost' => $_POST['orderShippingCost'],
		'orderActive' => 1
	];
	
	$createsalesorder = "INSERT INTO tbl_order (shopID, orderReference, orderDate, customerName, customerAddress, countryID, paymentMethodID, orderPaid, orderPickDate, orderDespatchDate, shipperID, orderShippingReference, orderShippingCost, orderActive) VALUES (:shopID, :orderReference, :orderDate, :customerName, :customerAddress, :countryID, :paymentMethodID, :orderPaid, :orderPickDate, :orderDespatchDate, :shipperID, :orderShippingReference, :orderShippingCost, :orderActive)";
	$stmt= $pdo->prepare($createsalesorder);
	$stmt->execute($data);

 

I think the above covers points 1 & 2. I have no idea how to implement points 3 & 4? I am also not sure how to handle checkbox (I just put a default value in). I am also getting an error when posting as well because countryID is an INT, but it thinks it is a string. How do I validate that?

Thanks,

J

Link to comment
Share on other sites

the following is an example, with one 'email' type field, showing the organizational and post method form processing/form points that were made -

<?php

// initialization
session_start();

$post = []; // array to hold a trimmed working copy of the form data
$errors = []; // array to hold user/validation errors

// post method form processing, detect if a post method form has been submitted
if($_SERVER['REQUEST_METHOD'] === 'POST')
{
	// trim all the data at once
	$post = array_map('trim',$_POST); // if any input is an array, use a recursive trim call-back function here instead of php's trim
	
	// validate inputs
	if($post['email'] === '')
	{
		$errors['email'] = "Email is required.";
	}
	else if(!filter_var($post['email'], FILTER_VALIDATE_EMAIL))
	{
		$errors['email'] = "Email format is not valid.";
	}
	
	// if no errors, use the form data
	if(empty($errors))
	{
		// use the data in $post here...
	}
	
	// if no errors, success
	if(empty($errors))
	{
		$_SESSION['success_message'] = "Some success message.";
		die(header("Refresh:0"));
	}
}

// html document starts here...
?>

<?php
// output and clear any success message
if(isset($_SESSION['success_message']))
{
	echo "<p>{$_SESSION['success_message']}</p>";
	unset($_SESSION['success_message']);
}
?>

<?php
// output any errors
if(!empty($errors))
{
	echo "<p>".implode('<br>',$errors)."</p>";
}
?>

<?php
// output the form, repopulating any field values with the existing form data
?>
<form method='post'>
<label>Email: <input type='email' name='email' value='<?=htmlentities($post['email']??'',ENT_QUOTES)?>'></label><br>
<input type='submit'>
</form>

 

1 hour ago, johnc81 said:

I am also not sure how to handle checkbox

your original logic, checking if it is set and assigning one value if it is, and another if it is not, is okay. i wouldn't make a new name/change the name for it when doing so, and you can use php's ternary operator to simplify the logic -

// condition checkbox input
$post['orderPaid'] = isset($post['orderPaid']) ? 1 : 0;

 

1 hour ago, johnc81 said:

I am also getting an error when posting as well because countryID is an INT, but it thinks it is a string. How do I validate that?

by definition, all external data are string data types, regardless of what value they contain. if the 1st value='' option is selected, you will get an empty string in the php code. if one of the other options is selected, you will get a string containing the numerical value. as long as the string value only contains numerical characters, it should be handled okay by php and by the database. to actually validate that the non-empty submitted value is one of the permitted choices, i would make an array of the data from the tbl_countries select query, with the id as the array indexes, and the element values being the country names, then either test if the array entry corresponding to the submitted value isset() or testing if the submitted value is in the array of the array keys, using in_array() and array_keys().

Link to comment
Share on other sites

2 hours ago, johnc81 said:

I start a session once the form has been posted, but unless I start another one after the page reloads, how do I get the use the session variables?

the posted code has a session_start near the top, then another one inside the post method form processing, right before assigning the last insert id to a session variable. the one near the top is all you need and in fact the second one will be producing a php error, which is probably being discarded when the redirect occurs. i would set php's output_buffering to off in the php.ini, so that you will see any non-fatal php errors or debugging output from your code prior to a redirect.

Link to comment
Share on other sites

Hi,

Thank you very much for your help and detailed explanations. I still need to work through the rest of your list (and will do that over the next day or so), but now I have it posting correctly. I would really appreciate you having a quick look at the code to see if there are any problems with what I have now:

<?php
ERROR_REPORTING(E_ALL);

require 'db/connect.php';
require 'db/pdoconnect.php';
require 'includes/rowcolours.php';

session_start();

$errors = [];

if(isset($_SESSION['orderIDx'])) {
	$orderIDx = $_SESSION['orderIDx'];
	$orderActivex = $_SESSION['orderActivex'];
} else {
	$orderIDx = 0;
	$orderActivex = 0;
}

// Create sales order form submitted
if(isset($_POST['createsalesorder'])) {
	
	// Create array of data
	$data = [
 	'shopID' 					=> trim($_POST['shopID']) == '' ? null : $_POST['shopID'],
	'orderReference' 			=> trim($_POST['orderReference']) == '' ? null : $_POST['orderReference'],
	'orderDate' 				=> trim($_POST['orderDate']) == '' ? null : $_POST['orderDate'],
	'customerName' 				=> trim($_POST['customerName']) == '' ? null : $_POST['customerName'],
	'customerAddress' 			=> trim($_POST['customerAddress']) == '' ? null : $_POST['customerAddress'],
	'countryID' 				=> trim($_POST['countryID']) == '' ? null : $_POST['countryID'],
	'paymentMethodID' 			=> trim($_POST['paymentMethodID']) == '' ? null : $_POST['paymentMethodID'],
	'orderPaid' 				=> isset($_POST['orderPaid']) ? 1 : 0,
	'orderPickDate' 			=> trim($_POST['orderPickDate']) == '' ? null : $_POST['orderPickDate'],
	'orderDespatchDate' 		=> trim($_POST['orderDespatchDate']) == '' ? null : $_POST['orderDespatchDate'],
	'shipperID' 				=> trim($_POST['shipperID']) == '' ? null : $_POST['shipperID'],
	'orderShippingReference' 	=> trim($_POST['orderShippingReference']) == '' ? null : $_POST['orderShippingReference'],
	'orderShippingCost' 		=> trim($_POST['orderShippingCost']) == '' ? null : $_POST['orderShippingCost'],
	'orderActive' 				=> 1,
	];
	
	// Check for errors
	if($_POST['shopID'] === '' || $_POST['orderReference'] === '' || $_POST['orderDate'] === '' || $_POST['customerName'] === '') {
		$errors['data'] = 'Missing data. Record not created';
	}
	
	// Post form if no errors
	if(empty($errors)) {
		$createsalesorder = "INSERT INTO tbl_order (shopID, orderReference, orderDate, customerName, customerAddress, countryID, paymentMethodID, orderPaid, orderPickDate, orderDespatchDate, shipperID, orderShippingReference, orderShippingCost, orderActive) VALUES (:shopID, :orderReference, :orderDate, :customerName, :customerAddress, :countryID, :paymentMethodID, :orderPaid, :orderPickDate, :orderDespatchDate, :shipperID, :orderShippingReference, :orderShippingCost, :orderActive)";
		$stmt= $pdo->prepare($createsalesorder);
		$stmt->execute($data);
	}
	
	// Set session variables and refresh
	if(empty($errors)) {
		$lastID = $pdo->lastInsertId();
		$_SESSION['orderIDx'] = $lastID;
		$_SESSION['orderActivex'] = 1;
		$_SESSION['success_message'] = "Sales order created";
		die(header("Refresh:0"));
	}
}
?>

<!DOCTYPE html>
<html lang="en">
<head>
	<title>Manage Sales Orders</title>
	<link rel="stylesheet" type="text/css" href="css/style.css" />
</head>
<body>
	<?php require 'includes/navbar.php'; 
		echo '<div><h1>Manage Sales Orders</h1></div>';
	?>
	
	<?php
		// output any errors
		if(!empty($errors)) {
			echo "<p>".implode('<br>',$errors)."</p>";
		}
		
		// Output success message
		if(isset($_SESSION['success_message'])) {
			echo "<p>{$_SESSION['success_message']}</p>";
			unset($_SESSION['success_message']);
		}
?>

 

Even though I am only using this offline, I would be interested to know if the above would be enough to avoid SQL injection?

Thanks,

J

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.