Inserting data based on date and select boxes

SMT · August 25, 2016, 6:21am

Am selecting some persons from form and setting a start and end date. While loop is working fine where the limit is set on date but foreach loop is not inserting the data. I have checked the ids of persons from check boxes are fetching perfectly just issue with insert query

<?php
include ('db.php'); 
		$date_start = $_POST['date_start'];
		$date_end = $_POST['date_end'];
		$ids = $_POST['chk'];
		$shift = $_POST['shiftTime'];
	$conn = mysql_connect($servername,$database_username,$database_password);
if(!empty($date_start)){
//$date_change = DATE_FORMAT(NOW(),'%m-%d-%Y');
echo "Start date: $date_start"."<br>";
echo '<pre>';
print_r($_POST);
echo '</pre>';
$i=0;

while ($date_start<=$date_end){

echo "<br>".$i;
    $date_start = date ("Y-m-d", strtotime("+1 day", strtotime($date_start)));
	foreach ($ids as $val) {
	$new_date = $date_start;
	$sql1=mysql_fetch_assoc(mysql_query("Select * FROM user_shift_test WHERE userid='$val' "),$conn);
		echo $e_id=$sql1['id'];
		echo $e_name=$sql1['name'];
		echo $e_dayoff=$sql1['dayoff'];
	$query = mysql_query("INSERT INTO users_new_test (index,userid,id,name,shift,date_start,date_end,dayoff,updatetime) VALUES ('NULL','$ids[i]','$e_id','$e_name','$shift','$new_date','$date_end','$e_dayoff',now())");

}}
}
echo "<br>Successfully Inserted";
}
else{
	echo "Nothing selected";
	echo '<pre>';
	print_r($_POST);
	echo '</pre>';
}
?>

SpacePhoenix · August 25, 2016, 7:11am

You’re a sitting duck for SQL Injection attacks as you’re sticking user submitted data into the database without having at the very minimum validated the data submitted by the user. Also your code will be broken on any server running PHP version 7 or newer as the mysql_* was removed from PHP version 7.

You should now be using either the mysqli_* extension (note the “i” in there) or PDO and should always be using prepared statements

SMT · August 25, 2016, 7:36am

Its an old database where i have to apply this and they still have php5 and also still using mysql. So i can’t change that

Mittineague · August 25, 2016, 7:47am

Both mysqli and PDO are are able to work with MySQL databases.

Both were and are available to use with PHP version 5

So not only can you change, you should, now while you’re working with this code, before you need to later on.

SMT · August 25, 2016, 7:48am

Can you help me changing it ?

felgall · August 25, 2016, 7:50am

They were told to stop using the old mysql_ back in mid 2013. The mySQLi and PDO replacements were introduced in PHP5 in mid 2004. The conversion to mySQLi or PDO ought to have been started by 2007 (as the new replacements were well and truly proved by then as being way superior) so you have been really slack in maintaining the code.

Which version of PHP5 are you using - the only one to still have any support at all is 5.6 - all the earlier versions are now security holes.

SMT · August 25, 2016, 7:55am

am new here.
php 5.2
First i have to make it run then i will change it

Mittineague · August 25, 2016, 8:21am

I personally have grown to have a preference for PDO mainly because I sometimes work with databases other than MySQL.

PDO can be a bit scary if you aren’t used to OOP syntax, but mysqli procedural syntax isn’t that much different than mysql which I consider having reached its End Of Life

The connect takes the database name

// $conn = mysql_connect($servername,$database_username,$database_password);
$conn = mysqli_connect($servername,$database_username,$database_password,$database_name);

and query requires the connection handle as the first argument (as opposed to an optional second)

// $sql1=mysql_fetch_assoc(mysql_query("Select * FROM user_shift_test WHERE userid='$val' "),$conn);
$sql1=mysqli_fetch_assoc(mysqli_query($conn,"Select * FROM user_shift_test WHERE userid='$val' "));

// $query = mysql_query("INSERT INTO users_new_test (index,userid,id,name,shift,date_start,date_end,dayoff,updatetime) VALUES ('NULL','$ids[i]','$e_id','$e_name','$shift','$new_date','$date_end','$e_dayoff',now())");
$query = mysqli_query($conn,"INSERT INTO users_new_test (index,userid,id,name,shift,date_start,date_end,dayoff,updatetime) VALUES ('NULL','$ids[i]','$e_id','$e_name','$shift','$new_date','$date_end','$e_dayoff',now())");

The above don’t demonstrate the use of prepared statements, which could most likely simplify your input cleanup code and improve the code in the process.

felgall · August 25, 2016, 8:32am

upgrade immediately to PHP 7 - there is little point in upgrading to 5.6 when the version you currently have has been a security hole for so many years.

droopsnoot · August 25, 2016, 9:41am

In this query:

$query = mysql_query("INSERT INTO users_new_test (index,userid,id,name,shift,date_start,date_end,dayoff,updatetime) VALUES ('NULL','$ids[i]','$e_id','$e_name','$shift','$new_date','$date_end','$e_dayoff',now())");

What do you intend by $ids[i] ? I see that you set $i to have a value of zero, but you would need to reference that as $ids[$i] in any case, and is it correct that you never increment that value?

Your “Successfully inserted” message needs looking at too - it’s a bit misleading as it doesn’t look at whether the data was successfully inserted or not.

And echo what they all said about changing to PDO. Once you get it all working with the old-style calls, you won’t want to risk disturbing working code. It doesn’t have to be all that different, you can start by just using the new calls, then move to prepared statements and so on.

SMT · August 25, 2016, 10:12am

am incrementing it at the end of the loop i might missed it here

droopsnoot · August 25, 2016, 10:54am

So we’re trying to figure out what’s wrong with the code you posted, but that’s not your actual code. That will be challenging.

SMT · August 25, 2016, 10:55am

Changing it to mysqli worked for me. Thanks

SMT · August 25, 2016, 10:57am

Thanks its working after changing it to mysqli

<?php
ini_set("display_startup_errors", 1);
ini_set("display_errors", 1);

/* Reports for either E_ERROR | E_WARNING | E_NOTICE  | Any Error*/
error_reporting(E_ALL);
include ('db.php'); 
		$date_start = $_POST['date_start'];
		$date_end = $_POST['date_end'];
		$chkarray = $_POST['chk'];
		$shift = $_POST['shiftTime'];
if(!empty($date_start)){
//$date_change = DATE_FORMAT(NOW(),'%m-%d-%Y');
echo "Start date: $date_start"."<br>";
echo '<pre>';
print_r($_POST);
echo '</pre>';
$i=0;

while ($date_start<=$date_end){
	echo "<br>".$i."<br>".$date_start."<br>";
	 foreach ($chkarray as $val) {  
		echo $val."  ";
	$conn = mysqli_connect($servername,$database_username,$database_password,$database_name);
	$new_date = $date_start;
	//$sql1=mysql_fetch_assoc(mysql_query("Select * FROM user_shift_test WHERE userid='$val' ",$conn));
	$sql1=mysqli_fetch_assoc(mysqli_query($conn,"Select * FROM user_shift_test WHERE userid='$val' "));
	echo $e_id=$sql1['id'];
	echo $e_name=$sql1['name'];
	echo $e_dayoff=$sql1['dayoff'];
	$query = mysqli_query($conn,"INSERT INTO users_new_test (userid,id,name,shift,date_start,date_end,dayoff,updatetime) VALUES ('$val','$e_id','$e_name','$shift','$new_date','$date_end','$e_dayoff',now())");
if($sql1){
echo "Shukar :P";
}
else{echo "ERROR!!!!<br>";}

}$date_start = date ("Y-m-d", strtotime("+1 day", strtotime($date_start)));
$i++;
}
echo "<br>Successfully Inserted";
}//first if closing
else{
	echo "Nothing selected";
	echo '<pre>';
	print_r($_POST);
	echo '</pre>';
}
    ?>

droopsnoot · August 25, 2016, 11:03am

You could improve this:

$sql1=mysqli_fetch_assoc(mysqli_query($conn,"Select * FROM user_shift_test WHERE userid='$val' "));

by changing it to this:

$sql1=mysqli_fetch_assoc(mysqli_query($conn,"Select id,name,dayoff FROM user_shift_test WHERE userid='$val' "));

As you only use those three columns, there is no need to fetch all of them. Unless of course those are the only three columns in that table.

Good on you for having a go with mysqli, now take a look at prepared statements for your two queries - as you have a list of values for $val in your retrieval query, and for all the fields in your insert query, it would be an ideal place to prepare the queries before the loop, and simply bind the parameters to them inside the loop prior to execution.

Also you connect to the database inside your loop, which is a bit inefficient - if you have fifty items in the $chkarray array, it’s going to open fifty connections. Move it to before the ‘foreach’ and it will only connect the once. Debatable how much difference it makes in reality, but it’s better practice IMO.

SMT · August 25, 2016, 11:06am

Am good at mysqli i will try it. And i will get more fields from database later thats why getting all fields

felgall · August 25, 2016, 10:27pm

So add the individual fields as you need them - using * creates a security hole in the database.

SMT · September 1, 2016, 5:20am

Thanks for the help i have tried it. And you are right it doesn’t make any difference but as you said it will be good practice to make a connection outside the loop.

SMT · September 1, 2016, 5:20am

Oh yeah. you got a point. Thanks

system · December 1, 2016, 12:21pm

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.

Topic		Replies	Views
Insert data to database where data from while loop PHP	3	8227	October 8, 2014
While loop's issue PHP	4	477	October 8, 2014
Php input boxes PHP	15	957	October 8, 2014
Problem in saving data from while loop and select option PHP	18	3363	October 8, 2014
How to perform different update queries based on user selection using php PHP	16	1120	May 19, 2017

Inserting data based on date and select boxes

Related topics