Checkbox Form to Update Multiple Rows in Database

[MartynLearnsPHP]

Hi.

 

Whilst I keep ploughing through the internet in search of tutorials/examples, can anyone see an obvious flaw in my coding because this is the first time that I have tried to do this and it's doing my noggin' in.

 

In essence, I am trying to create a form where an Admin User, once logged in, will get a list of all the projects associated to the event that they are organising (ie. budgets, expenses, table plan, food, drink, etc.). Once they have selected the project they will get a list of all their attendees/members with a view of which members have been granted permission to contribute to this project and, with checkboxes, edit the list of members by authorising or withdrawing project permissions.

 

My current coding is:

 

The Form to Select the Project (which works just fine) is::

<h3>Update Project Permissions</h3>
<form action="" method="post" class="form-horizontal wb-form wb-new-reg">
	<fieldset>
	<h6>Please select a Project</h6>
	<select name="projectmembers" onChange="showGlobalProject(this.value)">
		  <option value="">Select a Project</option>
		  <option value="budget">Managing Budgets</option>
		  <option value="expense">Managing Expenses</option>
		  <option value="invites">Guest Invites</option>
		  <option value="transport">Transport</option>
		  <option value="food">Food</option>
		  <option value="drink">Drink</option>
		  <option value="tableplan">Table Plan</option>
		  <option value="entertainment">Entertainment</option>
		  <option value="other">Any Other Items</option>
	</select>
</form>
<script src="js/globalprojects.js"></script>
<div id="txtGlobalProjects"></div>

The Form which is then displayed (based on the above Project Selection Form and where $q represents the selected project is:

<?php
require 'core/init.php';
$adminid = $user->data() ->id;

if(isset($_GET['q'])) {
	$q = html_entity_decode($_GET['q']);
	echo "<form action='permissionupdate.php' method='post'>";
	if ($q==="budget")
	{
		<h6>Managing Budgets</h6>
		$query = DB::getInstance()->query("SELECT * FROM members WHERE adminid=$adminid");
		foreach($query->results() as $row){
			echo "<input type='checkbox' id='" . $row->id . "' name='budget[]' value='1'";
			if($row->$q==="1") {echo "checked";} else {echo "";} echo ">
			<input type='hidden' name='id[]' value='" .$row->id . "'>
			<input type='hidden' name='token' value='";
			if(isset($token)) {echo $token; } else {echo $token = Token::generate();}
			echo "'>" . $row->member_first_name . " " . $row->member_last_name . "";
		}
	}
	if ($q==="expense")
	{
		<h6>Managing Expenses</h6>
		$query = DB::getInstance()->query("SELECT * FROM members WHERE adminid=$adminid");
		foreach($query->results() as $row){
			echo "<input type='checkbox' id='" . $row->id . "' name='expense[]' value='1'";
			if($row->$q==="1") {echo "checked";} else {echo "";} echo ">
			<input type='hidden' name='id[]' value='" .$row->id . "'>
			<input type='hidden' name='token' value='";
			if(isset($token)) {echo $token; } else {echo $token = Token::generate();}
			echo "'>" . $row->member_first_name . " " . $row->member_last_name . "";
		}
	}
	if ($q==="invites")
	{
		<h6>Managing Budgets</h6>
		$query = DB::getInstance()->query("SELECT * FROM members WHERE adminid=$adminid");
		foreach($query->results() as $row){
			echo "<input type='checkbox' id='" . $row->id . "' name='invites[]' value='1'";
			if($row->$q==="1") {echo "checked";} else {echo "";} echo ">
			<input type='hidden' name='id[]' value='" .$row->id . "'>
			<input type='hidden' name='token' value='";
			if(isset($token)) {echo $token; } else {echo $token = Token::generate();}
			echo "'>" . $row->member_first_name . " " . $row->member_last_name . "";
		}
	}
	if ($q==="transport")
	{
		<h6>Transport</h6>
		$query = DB::getInstance()->query("SELECT * FROM members WHERE adminid=$adminid");
		foreach($query->results() as $row){
			echo "<input type='checkbox' id='" . $row->id . "' name='transport[]' value='1'";
			if($row->$q==="1") {echo "checked";} else {echo "";} echo ">
			<input type='hidden' name='id[]' value='" .$row->id . "'>
			<input type='hidden' name='token' value='";
			if(isset($token)) {echo $token; } else {echo $token = Token::generate();}
			echo "'>" . $row->member_first_name . " " . $row->member_last_name . "";
		}
	}
	if ($q==="food")
	{
		<h6>Food</h6>
		$query = DB::getInstance()->query("SELECT * FROM members WHERE adminid=$adminid");
		foreach($query->results() as $row){
			echo "<input type='checkbox' id='" . $row->id . "' name='food[]' value='1'";
			if($row->$q==="1") {echo "checked";} else {echo "";} echo ">
			<input type='hidden' name='id[]' value='" .$row->id . "'>
			<input type='hidden' name='token' value='";
			if(isset($token)) {echo $token; } else {echo $token = Token::generate();}
			echo "'>" . $row->member_first_name . " " . $row->member_last_name . "";
		}
	}
	if ($q==="drink")
	{
		<h6>Drink</h6>
		$query = DB::getInstance()->query("SELECT * FROM members WHERE adminid=$adminid");
		foreach($query->results() as $row){
			echo "<input type='checkbox' id='" . $row->id . "' name='drink[]' value='1'";
			if($row->$q==="1") {echo "checked";} else {echo "";} echo ">
			<input type='hidden' name='id[]' value='" .$row->id . "'>
			<input type='hidden' name='token' value='";
			if(isset($token)) {echo $token; } else {echo $token = Token::generate();}
			echo "'>" . $row->member_first_name . " " . $row->member_last_name . "";
		}
	}
	if ($q==="tableplan")
	{
		<h6>Table Plan</h6>
		$query = DB::getInstance()->query("SELECT * FROM members WHERE adminid=$adminid");
		foreach($query->results() as $row){
			echo "<input type='checkbox' id='" . $row->id . "' name='tableplan[]' value='1'";
			if($row->$q==="1") {echo "checked";} else {echo "";} echo ">
			<input type='hidden' name='id[]' value='" .$row->id . "'>
			<input type='hidden' name='token' value='";
			if(isset($token)) {echo $token; } else {echo $token = Token::generate();}
			echo "'>" . $row->member_first_name . " " . $row->member_last_name . "";
		}
	}
	if ($q==="entertainment")
	{
		<h6>Entertainment</h6>
		$query = DB::getInstance()->query("SELECT * FROM members WHERE adminid=$adminid");
		foreach($query->results() as $row){
			echo "<input type='checkbox' id='" . $row->id . "' name='entertainment[]' value='1'";
			if($row->$q==="1") {echo "checked";} else {echo "";} echo ">
			<input type='hidden' name='id[]' value='" .$row->id . "'>
			<input type='hidden' name='token' value='";
			if(isset($token)) {echo $token; } else {echo $token = Token::generate();}
			echo "'>" . $row->member_first_name . " " . $row->member_last_name . "";
		}
	}
	if ($q==="other")
	{
		<h6>Any Other Items</h6>
		$query = DB::getInstance()->query("SELECT * FROM members WHERE adminid=$adminid");
		foreach($query->results() as $row){
			echo "<input type='checkbox' id='" . $row->id . "' name='other[]' value='1'";
			if($row->$q==="1") {echo "checked";} else {echo "";} echo ">
			<input type='hidden' name='id[]' value='" .$row->id . "'>
			<input type='hidden' name='token' value='";
			if(isset($token)) {echo $token; } else {echo $token = Token::generate();}
			echo "'>" . $row->member_first_name . " " . $row->member_last_name . "";
		}
	}
	echo "<input type='submit' value='Update' name='project-update'>
	</form>";
}

And my php to process the form (which I guess is where my issue is, is:

if (is_array($_POST['project-update'])){ 
	if(is_array($_POST['id'])) {
	foreach ($_POST['id'] as $index => $id){
		if(Token::check(Input::get('token'))) {
			$db = DB::getInstance();
			
			if(is_array($_POST['budget'])) {		
				$sql = "UPDATE `members` SET `" . $_POST['budget'][$index] . " WHERE `id`='" . $id . "'";
				$memberUpdate = $db->query($sql);
				}
			if(is_array($_POST['expense'])) {		
				$sql = "UPDATE `members` SET `expense`=? WHERE `id` = ?";
				$memberUpdate = $db->query($sql, array($_POST['expense'][$index],$id));
				}
			if(is_array($_POST['invites'])) {		
				$sql = "UPDATE `members` SET `invites`=? WHERE `id` = ?";
				$memberUpdate = $db->query($sql, array($_POST['invites'][$index],$id));
				}
			if(is_array($_POST['transport'])) {		
				$sql = "UPDATE `members` SET `transport`=? WHERE `id` = ?";
				$memberUpdate = $db->query($sql, array($_POST['transport'][$index],$id));
				}
			if(is_array($_POST['food'])) {		
				$sql = "UPDATE `members` SET `food`=? WHERE `id` = ?";
				$memberUpdate = $db->query($sql, array($_POST['food'][$index],$id));
				}
			if(is_array($_POST['drink'])) {		
				$sql = "UPDATE `members` SET `drink`=? WHERE `id` = ?";
				$memberUpdate = $db->query($sql, array($_POST['drink'][$index],$id));
				}
			if(is_array($_POST['tableplan'])) {		
				$sql = "UPDATE `members` SET `tableplan`=? WHERE `id` = ?";
				$memberUpdate = $db->query($sql, array($_POST['tableplan'][$index],$id));
				}
			if(is_array($_POST['entertainment'])) {		
				$sql = "UPDATE `members` SET `entertainment`=? WHERE `id` = ?";
				$memberUpdate = $db->query($sql, array($_POST['entertainment'][$index],$id));
				}
			if(is_array($_POST['other'])) {		
				$sql = "UPDATE `members` SET `other`=? WHERE `id` = ?";
				$memberUpdate = $db->query($sql, array($_POST['other'][$index],$id));
				}
			}
		   Redirect::to('permissionupdate.php');
		}
	}
}

I appreciate that I can cut down a lot of code on the 2nd bit above, but I thought I'd focus on trying to get it to work before doing that.

 

Any help on spotting an obvious mistake or drawing my attention to the bit of code that I should be focusing on would be very much appreciated.

[ginerjm]

One thing that would help you and would help us to understand would be to SEPARATE your html form your php. Don't start your php process with a line of html. Do all your "thinking work", make your decisions, build php vars with any dynamic html generated by the conditions and THEN output your html, seeded with php vars where their content needs to go. You'll end up with a script that can be more easily modified and understood.

 

Second - How is this form being submitted? You have no submit button so I can only guess that your onchange event is doing it but you didn't show us that code. PS - why is your form saying it will be POST'ing and yet your php code looks for a GET input? Plus - you don't allow for the user to make a mistake with his mouse since the onchange is going to take off as soon as the user does anything. (Although I don't see another element on your form that would trigger the onchange since you aren't showing anyplace else to go to.)

 

So now - do some alterations to separate the functions of this script (sending out the select code, retrieving the chosen option, doing something with that option, sending out the new results) to help us see what you are doing. Perhaps it will also help you see what is wrong. For one thing it will make it easier to add some debugging code so you can see how your script progresses thru the different stages. And add a submit button!

[MartynLearnsPHP]

Thanks, ginerjm. I've shared too much unnecessary code.

 

I hope this simplifies things.

 

$q has been acquired from a previous form to determine which project we are updating permissions for (ie. budgets, epenses, invites, transport, etc). So I GET $q to determine which project we are updating.

 

With this project selected, the user gets a list of the Members within the project in order to update whether they have permission to view this project. If I am just looking at the budget project, the relevant part of this form is below:

<?php
require 'core/init.php';
$adminid = $user->data() ->id;

if(isset($_GET['q'])) {
	$q = html_entity_decode($_GET['q']);
	echo "<form action='permissionupdate.php' method='post'>";
	if ($q==="budget")
	{
		<h6>Managing Budgets</h6>
		$query = DB::getInstance()->query("SELECT * FROM members WHERE adminid=$adminid");
		foreach($query->results() as $row){
			echo "<input type='checkbox' id='" . $row->id . "' name='budget[]' value='1'";
			if($row->$q==="1") {echo "checked";} else {echo "";} echo ">
			<input type='hidden' name='id[]' value='" .$row->id . "'>
			<input type='hidden' name='token' value='";
			if(isset($token)) {echo $token; } else {echo $token = Token::generate();}
			echo "'>" . $row->member_first_name . " " . $row->member_last_name . "";
		}
	}
	echo "<input type='submit' value='Update' name='project-update'>
	</form>";
}

This form is then processed with the following php code:

if (is_array($_POST['project-update'])){ 
	if(is_array($_POST['id'])) {
	foreach ($_POST['id'] as $index => $id){
		if(Token::check(Input::get('token'))) {
			$db = DB::getInstance();
			
			if(is_array($_POST['budget'])) {		
				$sql = "UPDATE `members` SET `" . $_POST['budget'][$index] . " WHERE `id`='" . $id . "'";
				$memberUpdate = $db->query($sql);
				}
			////Similar code for expense, invites, etc would be here in case Get_$ had been those values////
				}
			}
		    Redirect::to('permissionupdate.php');
		}
	}
}

I hope I've made things a bit clearer.

[ginerjm]

Nah - too hard to follow. Good luck

[MartynLearnsPHP]

I don't get what's hard to follow. All there is here is a form and the code to process the form.

[Jacques1]

The logic is hard to follow, because you've piled up a large amount of PHPSQLHTMLJavaScript spaghetti code, odd design choices and parts which just don't make sense. Like this:

$sql = "UPDATE `members` SET `" . $_POST['budget'][$index] . " WHERE `id`='" . $id . "'";

That's not a query. It's probably a query fragment and definitely an SQL injection vulnerability.

 

Also, I still have no idea what exactly your problem is (maybe I've missed it). What are we supposed to do now? Try to guess what's on your screen?

 

Quite frankly, I'd scrap the code and start over:

• Your database layout needs rework. Right now, you appearently have a gigantic Excel-style table where you keep all your users, all your projects and all membership information. And whenever there's a new project, you need to add a new column to the table schema. That's not how SQL works. You want one table for the user-related info (name, address, whatever), one table for the projects and one table which assigns users to projects.
• As ginerjm already said, you need to separate the PHP business logic from the HTML stuff. Personally, I'm a big fan of template engines like Twig, because they more or less force you to write clean code. PHP relies entirely on the programmer's discipline (which rarely works) and is actually very bad at generating dynamic HTML
• Start taking security into account. Use prepared statements instead of dumping user input into query strings.
• Checkboxes are tricky, because only the checked ones are actually submitted. Since you rely on an implicit numbering scheme, this may be a problem. There are more robust solutions, but you should first fix the issues above.