Jump to content

PDO update sql form not working


webdeveloper123
Go to solution Solved by Barand,

Recommended Posts

Hi Guys,

I've been at this all day and can't seem to figure it out. I have a customers page, which lists all records on the page (this is only a one table db). Next to each customer I have a Edit and Delete link. I'm working on edit right now but the values won't update in the db. When ever I try to edit values in edit page (form populates just fine), when I press submit I get taken to a page-not-found page which I had done earlier incase in the query string the id entered did not exist in the database. This was working fine. Now I have put the code in for the update query, even though the id exists is still takes me to the page not found page. And If I comment out the block of code that sends me to the page not found page, all I get is reposted to the same form with only "First Name" at the top, nothing at all else on the page. And none of the errors show either, but the errors were all displaying in my insert form. Insert form is fine. Here is my code:

Many thanks

<?php
declare(strict_types = 1);
 ?>


<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<title> Document </title>
</head>
<body>



<?php

include 'includes/db.php'; 
include 'includes/pdofunction.php'; 


$food_choice = ['Burgers', 'Pizza', 'Kebabs',];

$id = $_GET['user_id'] ?? '';


$sql       = "SELECT *  
                FROM customer_details 
               WHERE customer_id = :id;";
$statement = $pdo->prepare($sql);
$statement->execute(['id' => $id]);
$member    = $statement->fetch();

if (!$member) {
      http_response_code(404);
    header('Location: page-not-found.php');
    exit;

}


$customers = [
    'customer_id'  => '',
    'first_name'  => '',
    'last_name'   => '',
    'address' => '',
    'town'  => '',
    'county'   => '',
    'post_code' => '',
    'fav_food' => '',
    'birthdate'  => '',
    'email'   => '',
    'terms' => '',
];        

$errors = [
    'first_name'  => '',
    'last_name'   => '',
    'address' => '',
    'town'  => '',
    'county'   => '',
    'post_code' => '',
    'fav_food' => '',
    'birthdate'  => '',
    'email'   => '',
    'terms' => '',
];          



if ($_SERVER['REQUEST_METHOD'] == 'POST') {

$customers['customer_id'] = $id;
$customers['first_name'] = $_POST['fname'];
$customers['last_name'] = $_POST['lname'];
$customers['address'] =  $_POST['address'];
$customers['town'] =  $_POST['town'];
$customers['county'] =  $_POST['county'];
$customers['post_code'] =  $_POST['postcode'];
$customers['birthdate'] =  $_POST['birthday'];
$customers['email'] = $email = $_POST['email'];
$customers['terms'] = (isset($_POST['terms']) and $_POST['terms'] == true) ? true : false;

 $customers['fav_food']   = $_POST['fav_food'] ?? '';
    $valid   = in_array($customers['fav_food'] , $food_choice);
    $errors['fav_food'] = $valid ? '' : 'Must enter a food type';


$errors['first_name']  = is_text($customers['first_name'], 2, 20)   ? '' : 'Must be 2-20 characters';
$errors['last_name']  = is_text($customers['last_name'], 2, 20)   ? '' : 'Must be 2-20 characters';
$errors['address']  = is_text($customers['address'], 6, 20)   ? '' : 'Must be 6-20 characters';
$errors['town']  = is_text($customers['town'], 3, 20)   ? '' : 'Must be 3-20 characters';
$errors['county']  = is_text($customers['county'], 3, 20)   ? '' : 'Must be 3-20 characters';
$errors['post_code']  = is_text($customers['post_code'], 5, 8)   ? '' : 'Must be 5-8 characters';


if (!check_date($customers['birthdate']) ) {
    $errors['birthdate'] = 'Invalid date';
} else {
    $errors['birthdate'] = '';

}

$errors['email'] = filter_input(INPUT_POST, 'email', FILTER_VALIDATE_EMAIL) ? '' : 'Email not valid';

$errors['terms'] = $customers['terms']  ? '' : 'You must agree to the terms and conditions'; 


 $invalid = implode($errors);                              // Join error messages
    if ($invalid) {                                           // If there are errors
        $message = 'Please correct the following errors:';    // Do not process
    } else {                                                  // Otherwise
        $message = 'Your data was valid';                     // Can process data


         $sql = "UPDATE customer_details SET first_name = :first_name, last_name = :last_name, address = :address, town = :town, county = :county, post_code = :post_code, 
fav_food = :fav_food, birthdate = :birthdate, email = :email, terms = :terms WHERE customer_id = :id;";
          
    

$statement = $pdo->prepare($sql);
$statement->execute($customers);
                   
    }

}


echo $id; 

?>


<form action="edit.php" method="post">
  <label for="fname">First name:</label><br>
  <input type="text" id="fname" name="fname" value="<?= htmlspecialchars($member['first_name']) ?>"><br>
    <span class="error"><?= $errors['first_name'] ?></span><br>
  
  <label for="lname">Last name:</label><br>
  <input type="text" id="lname" name="lname" value="<?= htmlspecialchars($member['last_name']) ?>"><br>
   <span class="error"><?= $errors['last_name'] ?></span><br>

  <label for="address">Address</label><br>
  <input type="text" id="address" name="address" value="<?= htmlspecialchars($member['address']) ?>"><br>
<span class="error"><?= $errors['address'] ?></span><br>
  
  <label for="town">Town</label><br>
  <input type="text" id="town" name="town" value="<?= htmlspecialchars($member['town']) ?>"><br>
  <span class="error"><?= $errors['town'] ?></span><br>
  
  <label for="county">County</label><br>
  <input type="text" id="county" name="county" value="<?= htmlspecialchars($member['county']) ?>"><br>
   <span class="error"><?= $errors['county'] ?></span><br>
  
  <label for="postcode">Post Code</label><br>
  <input type="text" id="postcode" name="postcode" value="<?= htmlspecialchars($member['post_code']) ?>"><br><br>
  <span class="error"><?= $errors['post_code'] ?></span><br>










  <label for="food">What is your favourite food?</label>
<?php foreach ($food_choice as $option) { ?> <br> 
       <input type="radio" name="fav_food"
            value="<?= $option ?>" 
            <?= ($member['fav_food'] == $option) ? 'checked' : '' ?>> <?= $option ?> 
  <?php } ?>
             <br> <span class="error"><?= $errors['fav_food'] ?></span><br>







  
  
  <label for="birthday">Birthday:</label>
  <input type="date" id="birthday" name="birthday" value="<?= htmlspecialchars($member['birthdate']) ?>"><br><br>
  <span class="error"><?= $errors['birthdate'] ?></span><br>
  
  <label for="email">Email</label><br>
  <input type="text" id="email" name="email" value="<?= htmlspecialchars($member['email']) ?>"><br><br>
  <span class="error"><?= $errors['email'] ?></span><br>


  <input type="checkbox" id="terms" name="terms" value="true" <?= $member['terms'] ? 'checked' : '' ?>>
  <label for="terms">I agree to the terms.</label><br><br>
     <span class="error"><?= $errors['terms'] ?></span><br>

    <input type="submit" value="Submit">  
</form>


</body>
</html>

 

Link to comment
Share on other sites

$statement = $pdo->prepare($sql);
$statement->execute(['id' => $id]);
$member    = $statement->fetch();

if (!$member) {
      http_response_code(404);
    header('Location: page-not-found.php');
    exit;

The above code should have a check on the $statement value to see if your query actually runs.  And you could also add a check on the number of records returned before you try to process one.  And just what does that response code thingie doing for you?  You are producing your own custom error page so WTF code?

And is the not found page located in the same folder as your running script? or in the list of locations to search?

Edited by ginerjm
Link to comment
Share on other sites

Well you have to change that echo.  Of course I could also ask just why you are trying to echo that result array?

Can you pare down your posting to just the parts that are giving you the problem.  Way too much stuff here to make sense of what you're asking us to look at.  LIke  where is the submit that causes your error? And the form that contains that submit.

Edited by ginerjm
Link to comment
Share on other sites

remove the action='...' attribute from the <form ...> tag to get the form to submit to the same page it is on AND automatically propagate any existing get parameters in the url. by specifying the URL in the action attribute with just the page name, there is no longer any user_id on the end of the url.

before you go on, you should lay out the code on your page 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

since you want to initially populate the form fields with the existing data, then if there are any user/validation errors in the form processing code, populate them with the submitted form data, i recommend that you copy and trim the $_POST data into the existing $member array, using one single line of code. you would also want to only query for and fetch the existing data if the form has never been submitted (the $member array will be empty), so that you don't keep replacing the values the user has taken the time to enter, with the original values.

here's a laundry list of issues with this code -

  1. the $_GET[user_id] is a requirement for this code to work. if it's not set or it doesn't contain a valid integer > 0, that's an error and you should setup and display an error message on this page that a required input is not present/valid, and not even attempt to run the SELECT query. had there been code to do this, you would have been getting an error message that would have alerted you that the form wasn't including the existing $_GET['user_id'] in the url.
  2. likewise, if the SELECT query doesn't match any data, which would mean that either there's a programming mistake or that the row of data was deleted after the edit link was produced, you should you should setup and display generic message, on this page, for the user that's there's no data to edit.
  3. hopefully, when you are making the database connection you are setting the error mode to exceptions (along with setting the character set to match your database tables, setting emulated prepared queries to false, and setting the default fetch mode to assoc.)
  4. don't unnecessarily write out things for every possible form field. forget the $customers array variable, just reuse the $member variable as described above.
  5. just set the $errors to be an empty array and only put entries into it for actual errors. you can then just test if it is empty() or not empty() at any point to find if there are no errors or errors.
  6. there's no need for any else{} logic clearing an element in the $errors array, since you will only be setting an element in the $errors array if there was an error.
  7. all the input data you use in the post method form processing should come via post data. to get the customer_id/user_id into that code, pass it via a hidden form field.
  8. if 'required' inputs are empty, after being trimmed, that's a different problem from them being a wrong length or having a wrong format. you should first validate that required fields are not empty strings, before performing any other validation.
  9. at least the email column should be defined as a unique index. you need error handling for any insert/update query to catch and handle duplicate (or out of range errors.) the catch logic would test the error number and if it is for anything that the visitor can correct, setup a message telling them what exactly was wrong with the data that they submitted. for all other error numbers, just re-throw the exception and let php handle it.
  10. after the end of all the post method form processing logic, if there are no errors, redirect to the exact same url of the current page to cause a get request for that page. this will prevent the browser from trying to resubmit the form data.
  11. if you want to display a one-time success message, store it in a session variable, then test, display, and clear that session variable at the appropriate location in the html document.
  12. don't share the same variable for the error and the success message. at the point of displaying any error message in the html document, test if the $errors array is not empty.

 

Edited by mac_gyver
Link to comment
Share on other sites

1 hour ago, webdeveloper123 said:

What's wrong with that. I had 2 options, either print an error message on the same page of forward to custom error page, and It was in the book so I thought it was quite good 

a http 404 error is used if a requested web page doesn't exist. if a query doesn't match a 'required' input value, that's not a use for a http 404 error page.

a query that doesn't match a required value is either due to - a programming mistake (which is the current cause), the matching row of data was deleted, or something is feeding your code their own input value that doesn't exist.

when you are learning, developing, and debugging code/queries, your code/system should tell you why it is failing. does using a http 404 error page do that? doesn't that mean that you should instead be setting up and displaying a helpful error message?

Edited by mac_gyver
Link to comment
Share on other sites

43 minutes ago, webdeveloper123 said:

the GET variable is fine,

that's not what i stated to do. if your code does not have validation logic for all inputs and error handling for all the statements that can fail with an error, you will forever be wondering why it doesn't work.

the advice we give you is to help you make your code - more secure, provide a good user experience, be simple, general-purpose, maintainable, and through validation and error handling, will either work or it will tell you - display/log, the reason why it isn't working.

Link to comment
Share on other sites

9 minutes ago, mac_gyver said:

a query that doesn't match a required value is either due to - a programming mistake (which is the current cause), the matching row of data was deleted, or something is feeding your code their own input value that doesn't exist.

+ "the matching row of data never existed"

Not getting a matching row is not a database error - in fact it may often be the desired result.

Link to comment
Share on other sites

I changed it to this, to give the error on the page rather than re-directing


if (!$member) {
   
 $errorMember = 'That Id was not found';
} else {
    $errorMember = '';
}

Than I echo that $errorMember variable just above the html form

 

But I still can't seem to spot the programming mistake your talking about mac_gyver. This is one of my first shots at PDO, so your defiantly saying the error (or at least one of them) lies in the query?

Link to comment
Share on other sites

this is the programming mistake -

18 hours ago, mac_gyver said:

remove the action='...' attribute from the <form ...> tag to get the form to submit to the same page it is on AND automatically propagate any existing get parameters in the url. by specifying the URL in the action attribute with just the page name, there is no longer any user_id on the end of the url.

along with the fact that you are not validating that get input before trying to use it with an sql query.

Link to comment
Share on other sites

ok I got rid of the action attribute but now it's giving me even more errors. I will have a go at validating get input before using it in the query. Its now giving me an 

Fatal error: Uncaught Error: Call to undefined function check_date() in /var/www/vhosts/

On my date function which is in an include file. This is the function:

function check_date($input, $format='m/d/Y')
{
    $date = DateTime::createFromFormat($format, $input);
    return ($date && $date->format($format) === $input);
}

Can you help please?

Link to comment
Share on other sites

1 hour ago, webdeveloper123 said:

It's this date function now, I don't know what's wrong with it, I haven't changed it

What does $customers['birthdate'] contain when you have a problem?

I notice your location is London but your default input format in your function is US format.

Link to comment
Share on other sites

6 minutes ago, Barand said:

I notice your location is London but your default input format in your function is US format.

You said I shouldn't change it on the other thread when you gave me that code.

And you said previously on another thread you should use DATE sql type for dates and store in default format (Y-m-d) 

Edited by webdeveloper123
Link to comment
Share on other sites

1 minute ago, webdeveloper123 said:

You said I shouldn't it on the other thread when you gave me that code.

And you said previously on another thread you should use DATE sql type for dates and store in default format (Y-m-d)

You asked if you should change it because you were outputting to the database in Y-m-d format. I told you that $format was defining the expected input format.

2 minutes ago, webdeveloper123 said:

On my form the date is in UK format

You are using an INPUT type="date" therefore it will display the date according to your locale. However it will be sent in the POST data as Y-m-d, therefore your expected input format is "Y-m-d".

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.