PHP PDO Delete Query

Sorry if this is a duplicate question, but I’m trying to figure out how to create a delete query using PHP PDO and MySQL.

I have a table on a webpage that has an action column with two buttons. One for editing and one for deleting (like this):

image

The edit code redirects the user to another page to edit the user’s information. The edit code is functional, but the delete code doesn’t appear to work.

Here is what I have so far:

<?php

include ('../dbconnect.php');

$id = $_GET['id'];

$stmt = "DELETE FROM users WHERE id = :id";
$stmt->bindParam(':id', $id);

// Delete
if(isset($_POST['btn_delete'])) { 
  $id = $_GET['id']; 
  $stmt->execute();
  header('Location: ../../users.php');
}

?>

My goal is to redirect the user back to the users.php file where the table is. I have a confirmation as well using a Bootstrap Modal (code shown below) that shows when the user clicks the Delete Icon in the users table:

<div class="modal fade" id="deleteModal" tabindex="-1" role="dialog" aria-labelledby="exampleModalLabel" aria-hidden="true">
    <div class="modal-dialog" role="document">
      <div class="modal-content">
        <div class="modal-header">
          <h5 class="modal-title" id="exampleModalLabel">Delete User</h5>
          <button class="close" type="button" data-dismiss="modal" aria-label="Close">
            <span aria-hidden="true">×</span>
          </button>
        </div>
        <div class="modal-body">Are you sure you want to delete this user?</div>
        <div class="modal-footer">
          <button class="btn btn-secondary" type="button" data-dismiss="modal">Cancel</button>
		  <a class="btn btn-danger" href="api/users/delete.php">Delete</a>
        </div>
      </div>
    </div>
  </div>

I currently see these two errors when I run the script:

Notice: Undefined index: id in C:\xampp\htdocs\ccrp\api\users\delete.php on line 5

Fatal error: Uncaught Error: Call to a member function bindParam() on string in C:\xampp\htdocs\ccrp\api\users\delete.php:8 Stack trace: #0 {main} thrown in C:\xampp\htdocs\ccrp\api\users\delete.php on line 8

Am I doing this right? How do I fix it?

  1. $_GET['id'] expects an ID to be in your URL, but there isn’t one. An example would be api/users/delete.php?id=42

  2. You check for $_POST['btn_delete'], but there is no form in your HTML, so there will never be post data. Either make it a form (recommended) or change the check to something else.

@rpkamp I’d rather not use a form for this since the edit code uses one. I want the user to just click the button and confirm they want to delete the user account. I also noticed this line of code twice:

$id = $_GET['id'];

Is it necessary to have it twice?

The request can’t be GET and POST at the same time, it can be one or the other.
A form (via POST) is recommended for delete requests. A simple link with a GET variable can get inadvertantly crawled, and delete all your data.

No, it isn’t.

That doesn’t seem to be a good reason not to use one - can you expand on your thoughts behind that?

You’ll need to call prepare() at some point for your query to work.

To answer the actual question though, droop’s last line is the important one.

$stmt = "DELETE FROM users WHERE id = :id";
$stmt is a string, not a PDOStatement object. You should be executing a $db->prepare() around this string, which will return a PDOStatement object.
(I am assuming you are creating a $db object in your include - if you used a different variable name, obviously use that.)

1 Like

I agree completely that using GET is a pour choice for this especially when it comes to making a DB query.
Is the whole page where you are showing those edit/delete icons a FORM table? If so I would use a different approach. That being said, then as also mentioned you will need to use GET not only in the delete.php url but also your delete icon button and modal call as well so appropriate modal is shown.

If your display table (where icons are) is built with a data set then you can make a separate loop to build modal that matches display, but as you are probably using a query result loop, then the modal should be built within the same loop as display. As display is most likely a table, the modal would be in the same cell as the icon.

Not seeing your icon code I will give following example.
Note: The USER ID defines that target modal, the modal ID and the GET id in the delete path so they all match and are unique.

<button type="button" data-toggle="modal" data-target="#deleteModal_<?php echo $user_id;?>">DeleteImage</button>

<div class="modal fade" id="deleteModal_<?php echo $user_id;?>" role="dialog" aria-labelledby="exampleModalLabel" aria-hidden="true">
  <div class="modal-dialog" role="document">
    <div class="modal-content">
      <div class="modal-header">
        <h5 class="modal-title" id="exampleModalLabel">Delete User</h5>
        <button class="close" type="button" data-dismiss="modal" aria-label="Close">
          <span aria-hidden="true">×</span>
        </button>
      </div>
      <div class="modal-body">Are you sure you want to delete this user?</div>
      <div class="modal-footer">
        <button class="btn btn-secondary" type="button" data-dismiss="modal">Cancel</button>
  		<a class="btn btn-danger" href="api/users/delete.php?id=<?php echo $user_id;?>">Delete</a>
      </div>
    </div>
  </div>
</div>

So clicking icon brings up corresponding modal, which has corresponding link to delete page.
The weakest point now is your processing that can only rely on GET[‘id’].

OTHER OPTIONS:
IF the display table IS NOT wrapped in a form then I would add form tags around the modal and pass ALL information with POST to delete.php

IF the display table IS wrapped in a form then I would still pass information with POST but in the modal I would replace the link to a submit button that controls that action and the ID… For example:

<input type="submit" class="btn btn-danger" name="Delete_User[<?php echo $user_id;?>]" value="Delete" />

Now on the page where the main form action is pointing to (I will assume it is pointing to edit.php), you add an IF condition looking for $_POST[‘Delete_User’].

if($_SERVER["REQUEST_METHOD"] == "POST" && isset($_POST['Delete_User'])):

As only one of these delete user buttons will be pressed at one time, you need to get the KEY of the button that was pressed. You can get the KEY by searching for the VALUE.

$id = array_search("Delete",$_POST['Delete_User']);

Now you are free to put your query within the IF condition.

if($_SERVER["REQUEST_METHOD"] == "POST" && isset($_POST['Delete_User'])):	
				   
	$id = array_search("Delete",$_POST['Delete_User']);	 
	
	$sql = "DELETE FROM users WHERE id = :id";
	$stmt = $pdo->prepare($sql);
	$stmt->bindParam(':id', $id);
	$stmt->execute();
	header('Location: ../../users.php');
	exit;
endif;

Bottom line, you could call the modal with GET like the example I gave but should pass values to processing with POST. Anyway should get you going.

1 Like

You’re free to use multiple forms on a page.
The reason a form is better is that GET requests should be idempotent. Meaning that when all stays the same I should be able to GET a page as many times as I like and get the same response. The main reason for wanting this is caching.

POST requests on the other are allowed to return a different response on multiple requests and do not have to be idempotent.

True, but you can have a POST request with GET parameters if you want :wink:

1 Like

My general rule, perhaps over-simplified, is that you use GET when you want to retrieve data from your database, and you use POST when you want to update your database. I’m sure I read it somewhere.

1 Like

“Get is for when you don’t mind your buddy leaning over and seeing your stuff on the URL.”

Thanks for this feedback. I always confuse GET with POST. However, that raises a question (just to ease my curiosity): If GET is bad, then why is it used? When is a good time to use it?

My original thought was that the delete did not require user input other than clicking on the icon to delete that particular account. It may be a dumb reason, but when I think of an HTML form, I think of a user filling out a form that requires information to be sent to a server; not just clicking a button.

Yes. In my include, I have a $pdo variable that handles querying my database since I use this variable in my other files. So, I will be using that here as well.

No. The table is not wrapped in a form. So, I presume your second option would be best for me?

Thank you, everyone, for your feedback! Here is my table code in case anyone would like to see it:

<?php

include('nav/head.php');

if($role_id != '1') {
	header('Location: error.php');
	exit();
}

?>

<!DOCTYPE html>
<html lang="en">

<head>

  <meta charset="utf-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
  <meta name="description" content="">
  <meta name="author" content="">

  <title>CCRP | Users</title>

  <?php include('nav/header.php') ?>

          <!-- Page Heading -->
          <h1 class="h3 mb-2 text-gray-800">Users List</h1><br>
          <!-- <p class="mb-4">DataTables is a third party plugin that is used to generate the demo table below. For more information about DataTables, please visit the <a target="_blank" href="https://datatables.net">official DataTables documentation</a>.</p> -->

          <!-- DataTables Example -->
          <div class="card shadow mb-4">
            <div class="card-header py-3">
              <h6 class="m-0 font-weight-bold text-primary">Add, Edit or Remove User Accounts</h6>
            </div>
            <div class="card-body">
			<a class="btn btn-success" href="user_new.php"><i class="fa fa-user-plus"></i>&nbsp Add New User</a>
			<br><br>
              <div class="table-responsive">
                <table class="table table-bordered" id="dataTable" width="100%" cellspacing="0">
                  <thead>
                    <tr>
                      <th>User ID</th>
                      <th>User Role</th>
                      <th>First Name</th>
                      <th>Last Name</th>
                      <th>Email Address</th>
                      <th>Username</th>
					  <th>Action</th>
                    </tr>
                  </thead>
				  <?php
				  
				  	$stmt = $pdo->prepare("SELECT id, role_id, first_name, last_name, email, username FROM users");
					$stmt->execute();
					while($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
				  
				  ?>
				  	<tr>
						<td><?php print($row['id']) ?></td>
						<td>
						
						<?php 
						
						if ($row['role_id'] === '1') {
							echo 'Administrator';
						} elseif ($row['role_id'] === '2') {
							echo 'Operator';
						}
						
						?>
						
						</td>
						<td><?php print($row['first_name']) ?></td>
						<td><?php print($row['last_name']) ?></td>
						<td><?php print($row['email']) ?></td>
						<td><?php print($row['username']) ?></td>
						<td>							
							<a href="user_edit.php?edit_id=<?php print($row['id']); ?>"><i class="fa fa-user-edit"></i></a>
							<a href="#" data-toggle="modal" data-target="#deleteModal">
                  				<i style="color: #a40000;"class="fas fa-trash fa-sm fa-fw mr-2"></i>
                			</a>							
						</td>
					</tr>
					<?php } ?>			  
                </table>
              </div>
            </div>
          </div>

        </div>
        <!-- /.container-fluid -->

      </div>
      <!-- End of Main Content -->

	  <!-- Delete Modal-->
  <div class="modal fade" id="deleteModal" tabindex="-1" role="dialog" aria-labelledby="exampleModalLabel" aria-hidden="true">
    <div class="modal-dialog" role="document">
      <div class="modal-content">
        <div class="modal-header">
          <h5 class="modal-title" id="exampleModalLabel">Delete User</h5>
          <button class="close" type="button" data-dismiss="modal" aria-label="Close">
            <span aria-hidden="true">×</span>
          </button>
        </div>
        <div class="modal-body">Are you sure you want to delete this user?</div>
        <div class="modal-footer">
          <button class="btn btn-secondary" type="button" data-dismiss="modal">Cancel</button>
		  <a class="btn btn-danger" href="api/users/delete.php">Delete</a>
        </div>
      </div>
    </div>
  </div>
	  
	  <?php include('nav/footer.php'); ?>

</html>

It’s all about context. In and of itself, GET is not bad. It works wonderfully for any request that doesn’t change anything in your database. It is however not a nice fit for requests that do have to change something in the database, like adding something, or changing something, or deleting something.

The reason is that GET requests can be cached, so if two people perform the same action, the second one may receive the response the first person got. Which is totally weird for an edit or delete. POST requests on the other hand may never be cached, so then you don’t have that problem.

In terms of CQS a GET request is a query, while a POST request is a command.

2 Likes

@rpkamp That makes more sense. Thanks!

Sure, echo $row[‘id’] in the link to the modal name, the id of the modal and as the KEY of the delete button in your popup form. None of this is tested. Just roughly placed things into your page.

Assuming this page is users.php

<?php

include('nav/head.php');

if($role_id != '1') {
	header('Location: error.php');
	exit();
}

if($_SERVER["REQUEST_METHOD"] == "POST" && isset($_POST['Delete_User'])):	
				   
	$id = array_search("Delete",$_POST['Delete_User']);	 
	
	$sql = "DELETE FROM users WHERE id = :id";
	$stmt = $pdo->prepare($sql);
	$stmt->bindParam(':id', $id);
	$stmt->execute();
	header('Location: users.php');
	exit;
endif;	
?>

<!DOCTYPE html>
<html lang="en">

<head>

  <meta charset="utf-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
  <meta name="description" content="">
  <meta name="author" content="">

  <title>CCRP | Users</title>

  <?php include('nav/header.php') ?>

          <!-- Page Heading -->
          <h1 class="h3 mb-2 text-gray-800">Users List</h1><br>
          <!-- <p class="mb-4">DataTables is a third party plugin that is used to generate the demo table below. For more information about DataTables, please visit the <a target="_blank" href="https://datatables.net">official DataTables documentation</a>.</p> -->

          <!-- DataTables Example -->
          <div class="card shadow mb-4">
            <div class="card-header py-3">
              <h6 class="m-0 font-weight-bold text-primary">Add, Edit or Remove User Accounts</h6>
            </div>
            <div class="card-body">
			<a class="btn btn-success" href="user_new.php"><i class="fa fa-user-plus"></i>&nbsp Add New User</a>
			<br><br>
              <div class="table-responsive">
                <table class="table table-bordered" id="dataTable" width="100%" cellspacing="0">
                  <thead>
                    <tr>
                      <th>User ID</th>
                      <th>User Role</th>
                      <th>First Name</th>
                      <th>Last Name</th>
                      <th>Email Address</th>
                      <th>Username</th>
					  <th>Action</th>
                    </tr>
                  </thead>
				  <?php
				  
				  	$stmt = $pdo->prepare("SELECT id, role_id, first_name, last_name, email, username FROM users");
					$stmt->execute();
					while($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
				  
				  ?>
				  	<tr>
						<td><?php print($row['id']) ?></td>
						<td>
						
						<?php 
						
						if ($row['role_id'] === '1') {
							echo 'Administrator';
						} elseif ($row['role_id'] === '2') {
							echo 'Operator';
						}
						
						?>
						
						</td>
						<td><?php print($row['first_name']) ?></td>
						<td><?php print($row['last_name']) ?></td>
						<td><?php print($row['email']) ?></td>
						<td><?php print($row['username']) ?></td>
						<td>							
							<a href="user_edit.php?edit_id=<?php print($row['id']); ?>"><i class="fa fa-user-edit"></i></a>
							<a href="#" data-toggle="modal" data-target="#deleteModal_<?php echo $row['id'];?>">
                  				<i style="color: #a40000;"class="fas fa-trash fa-sm fa-fw mr-2"></i>
                			</a>
							<div class="modal fade" id="deleteModal_<?php echo $row['id'];?>" role="dialog" aria-labelledby="exampleModalLabel" aria-hidden="true">
							  <div class="modal-dialog" role="document">
							    <div class="modal-content">
							      <div class="modal-header">
							        <h5 class="modal-title" id="exampleModalLabel">Delete User</h5>
							        <button class="close" type="button" data-dismiss="modal" aria-label="Close">
							          <span aria-hidden="true">×</span>
							        </button>
							      </div>
							      <div class="modal-body">Are you sure you want to delete this user?</div>
							      <div class="modal-footer">
							        <button class="btn btn-secondary" type="button" data-dismiss="modal">Cancel</button>
									<form action="users.php" method="post">
									<input type="submit" class="btn btn-danger" name="Delete_User[<?php echo $row['id'];?>]" value="Delete" />
									</form>
							      </div>
							    </div>
							  </div>
							</div>							
						</td>
					</tr>
					<?php } ?>			  
                </table>
              </div>
            </div>
          </div>

        </div>
        <!-- /.container-fluid -->

      </div>
      <!-- End of Main Content -->

	  
	  <?php include('nav/footer.php'); ?>

</html>

@Drummin Would it be possible to put this code in it’s own page or is that not recommended?

The delete processing code? Sure not a problem. Just change the action in the form.

I have my reasons why I always do processing on the same page but you don’t have to.
First I do not have queries within html so I build an array of data before anything is ever sent to browser.
I can use this array many times on the page as needed examples might be emailing or viewing any ONE row of data or counting information from the data, or as in this case I would check that the ID being passed is a valid ID before allowing it to be used in a query. These kinds of things can be done with this data before it is used within html to display content. Added benefits are also easily displaying processing messages for example user did not input data correctly or a number doesn’t match, while also keeping POST values in the form until all has passed processing checks, so you can do much more than check if a form input is empty but also check that what has been entered is in fact entered correctly… Hey, that’s just my way of doing things. To each their own.

1 Like

You’ll probably want to add some logic just before the query is attempted that checks whether the person trying to delete users actually is authorised to do so.

2 Likes

@SpacePhoenix I already have this in place. The code is written to where only admins can view users info and delete accounts.

@Drummin That appeared to do the trick. Thanks!

It’s not always bad, just for some things, and delete queries is one of those things.
It’s fine to use GET for many things, for example, search queries, identifying pages in pagination, or pages in general, to name a few.

My post did imply that you can’t GET parameters on a POST request, as @rpkamp points out, that is incorrect. I do sometimes use GET on POST requests to give instructions to the controller.

You need to re-think carefully about what is (or can be) user input, and what isn’t (can’t be).
Just becuase you “hard-coded” a value into and input or button, the URL variable in an action or link, that does not necesarily mean that same value will be there when the request is submitted.
Altering a URL variable is as simple as going to the browser’s address bar and re-typing it to what you want, ready to be picked up by the $_GET in your script.
But POST isn’t much safer either. The values in your form can be edited to whatever in any browser with “Dev Tools”, or the whole form could be spoofed and actioned to your processor remotely.
Basiaclly, trust nothing that comes back from the client side.
Validate, sanitise, prepare any query, escape any output, where data from the client is concerned.

1 Like

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