How do I update a row of data in mysql using PHP?

Question

I'm attempting to create a form where the user can update their profile details but it just doesn't seem to work.

I'm quite the beginner in server side programming so I'm piecing together code from different tutorials viz. from http://www.codingcage.com/2015/04/php-login-and-registration-script-with.html

The class.user.php file, which originally only had the code for login, and signup. I copied the signup function and changed some stuff to update instead:

public function update($id,$uname,$umob,$uaddr,$uacc,$upass) {
    try {
        $upass = password_hash($upass, PASSWORD_DEFAULT);

        $stmt = $this->conn->prepare(
            "UPDATE users
                SET
                    id       = :id, 
                    name     = :uname, 
                    mobile   = :umob,
                    address  = :uaddr,
                    accNo    = :uacc, 
                    password = :upass
              WHERE id = :id"
        );

        $stmt->bindParam(":id", $id);
        $stmt->bindParam(":upass", $upass);
        $stmt->bindParam(":uacc", $uacc);                                         
        $stmt->bindParam(":uname", $uname);
        $stmt->bindParam(":uaddr", $uaddr); 
        $stmt->bindParam(":umob", $umob);

        $stmt->execute();   

        return $stmt;   
    }
    catch(PDOException $e) {
        echo $e->getMessage();
    }               
}

and in view_account.php: (edit 3, whole file including code corrections by @e_i_pi):

<?php
ini_set("error_log", "/path/to/error.log");
    require_once("session.php");

    require_once("class.user.php");

    $auth_user = new USER();

    $stmt = $auth_user->runQuery("SELECT * FROM users WHERE consumer-no=:cno");

    $userRow = $stmt->fetch(PDO::FETCH_ASSOC);

    if(!$session->is_loggedin()){
        // session no set redirects to login page
        $session->redirect('index.php');
    }

    if(isset($_POST['submit']) && $_POST['submit'] === 'save') {
        $uname = strip_tags($_POST['full-name']);
        $umob = strip_tags($_POST['mobile']);
        $uaddr = strip_tags($_POST['addr']);
        $uacc = strip_tags($_POST['bank-acc']);
        $id = strip_tags($_POST['id']);
        $upass = strip_tags($_POST['password']);


        if($uname=="") {
            $signuperror[] = "Please Enter Your Full Name!";    
        }
        else if($umob=="")  {
            $signuperror[] = "Please Enter Your Mobile No.!";   
        }
        else if($uaddr=="") {
            $signuperror[] = 'Please Enter Your Address!';
        }
        else if($upass=="") {
            $signuperror[] = "Please Enter a Password!";
        }
        else if(strlen($upass) < 6) {
            $signuperror[] = "Password must be atleast 6 characters";   
        }
        else {
            try {
// I commented out these for some weird reason I can't even rememebr
//                $stmt = $auth_user->runQuery("SELECT id FROM users WHERE id=:id");
//                $stmt->execute(array(':id'=>$id));
//                $row = $stmt->fetch(PDO::FETCH_ASSOC);
                $auth_user->update($id,$uname,$umob,$uaddr,$uacc,$upass);
            }
            catch(PDOException $e) {
                echo $e->getMessage();
            }
        }   
    }

?>
<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Gas Booking</title>
    <link rel="stylesheet" href="style.css">
</head>
<body>
    <header>
        <h1>gas booking</h1>
        <nav>
            <ul>
                <li><a href="index.php">home</a></li>
                <li><a href="booking.php">booking</a></li>
                <li><a href="payment.php">payment</a></li>
                <li><a href="ticket.php">ticket</a></li>
                <li><a href="view_account.php">view account</a></li>
                <li><a href="user-bank.php">bank</a></li>
                <li><a href="logout.php?logout=true">logout</a></li>
            </ul>
        </nav>
    </header>
    <div class="content">
        <h2>Edit Your Profile Details</h2>

        <form method="post" action="view_account.php">
            <input type="hidden" id="id" name="id" value="<?php echo $_SESSION['id']; ?>">
            <label for="full-name" class="input-info">
                <div class="label">full name</div>
                <input type="text" id="full-name" name="full-name" value="<?php echo $_SESSION['name']; ?>">
            </label>
            <label for="mobile" class="input-info">
                <div class="label">mobile number</div>
                <input type="text" id="mobile" name="mobile" value="<?php echo $_SESSION['mob']; ?>">
            </label>
            <label for="addr" class="input-info">
                <div class="label">address</div>
                <input id="addr" name="addr" value="<?php echo $_SESSION['addr']; ?>">
            </label>
            <label for="bank-acc" class="input-info">
                <div class="label">bank account number</div>
                <input type="text" id="bank-acc" name="bank-acc" value="<?php echo $_SESSION['accNo']; ?>">
            </label>
            <hr>
            <label for="password" class="input-info">
                <div class="label">password</div>
                <input type="password" id="password" name="password">
            </label>
            <button type="submit" name="submit" value="save">
                Save Changes
            </button>
        </form>     
    </div>
</body>
</html>

and my table is as follows:

--
-- Table structure for table `users`
--

CREATE TABLE IF NOT EXISTS `users` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `consumerNo` varchar(15) NOT NULL,
  `password` varchar(255) NOT NULL,
  `accNo` varchar(255) NOT NULL,
  `name` varchar(255) NOT NULL,
  `address` varchar(255) NOT NULL,
  `mobile` bigint(10) NOT NULL,
  `balance` bigint(10) NOT NULL,
  `joining_date` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
  PRIMARY KEY (`id`)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;

I'm sure I've done something stupid. I'd really appreciate pointing me in the right direction, I sat with it till 5:00am and am feeling frustrated with myself.

The connection with the db is working, classes are properly included. Let me know if you need more information. Thank you!

The project can be downloaded here: https://www.dropbox.com/s/9v69m18l82n1t46/gas.zip?dl=0. Warning the code's kind of a mess.

How are you defining the user $id? Is it in $_SESSION, or is it in a hidden field in the form? Can you show your form html — Darren
– Darren, Commented Feb 24, 2017 at 0:19
"doesn't seem to work" isn't very informative. An error message may be helpful. Or telling us which parts are successful (e.g. echoing variables shows the correct stuff). — mickmackusa
– mickmackusa ♦, Commented Feb 24, 2017 at 0:27
Be careful about the binding order. You should bind in the order that the the placeholders appear in the query. — Anthony Rutledge
– Anthony Rutledge, Commented Feb 24, 2017 at 1:33
@AnthonyRutledge you only need to bind in order if you are using ? characters as parameter placeholders. If you are using :paramaterName placeholders then it becomes a key:value pairing — e_i_pi
– e_i_pi, Commented Feb 24, 2017 at 2:50
I'm used to using libraries where I bind using an array of key / types object pairs. Never had a problem. If it makes you feel safer doing them in order, go for it, but it's just homeopathic — e_i_pi
– e_i_pi, Commented Feb 24, 2017 at 9:08

Darren · Accepted Answer · 2017-02-24 01:29:15Z

8

Update

You seem to be doing the following in view-account.php:

try {
    $auth_user->update($id,$uname,$umob,$uaddr,$uacc,$upass);
} catch(PDOException $e) {
    echo $e->getMessage();
}

Yet you're already try/catch'ing within your update() method. I assume it never gets to this as an error in your if/elseif/elseif/else/etc checks is picked up. Could you modify it to look like this for testing purposes:

$errors = [];
if ($uname == "") {
    $errors[] = "Please Enter Your Full Name!";
}
if ($umob == "") {
    $errors[] = "Please Enter Your Mobile No.!";
}
if ($uaddr == "") {
    $errors[] = 'Please Enter Your Address!';
}
if ($upass == "") {
    $errors[] = "Please Enter a Password!";
}
if (strlen($upass) < 6) {
    $errors[] = "Password must be atleast 6 characters";
}
// check errors
if (!empty($errors)) {
    print_r($errors);
    return false;
}
// otherwise try the query:
$obj = $auth_user->update($id, $uname, $umob, $uaddr, $uacc, $upass);

and let us know what comes up!

I assume you'd have an error thrown, something along the lines of;

SQLSTATE[HY093]: Invalid parameter number

This is because you're trying to bind on :id twice. You have to remember that a users ID is unique and should never change (right?).

Modify your query to look like this:

$stmt = $this->conn->prepare(
    "UPDATE users
     SET
         name = :uname, 
         mobile = :umob,
         address = :uaddr,
         accNo = :uacc, 
         password = :upass
      WHERE id = :id"
);

Notes

You're best to modify your "password change" functionality to have the user confirm their password (if(PASSWORD == PASSWORD_REPEAT) { .... SET PASSWORD...)
Don't pass the user ID inside the form. It's insecure. Since it's in $_SESSION already, simply access it like that within your view-account.php file!
- Why the above you ask? Simple. If I inspect your <form... element, I could easily modify that hidden input to be some other users ID, allowing me to change their passwords/information/etc.
- And since it looks like you're dealing with "Bank" related information, I'd suggest doing this asap! Imagine If I could change "Barack Obama's" bank password and access his account.
Your form also doesn't have any action attribute...so it does nothing.. Best change that to your view-account.php page
I suggest removing your use of strip_tags(). It could ruin some fields (i.e. passwords). You're also already binding/preparing your statements (Props to you on that, good work!)
While we're at it, you might want to look at your view-account.php file, It could be modified to stop the use of all those if / elseif / elseif / else statements. You're essentially checking all your fields and if it fails, you're adding an error message to an array, but if it passes you're running the query, this is bad practice. You should look at doing something similar to (pseudo code):

$errors = [];
if (!check_fields()) {
    $errors[] = THE FIELD ERROR MESSAGE;
}

// now check if your errors are empty or not
if(!empty($errors)) {
    // this means we have errors in the form.
    // return the errors array to the front end and handle it appropriately.
    return $errors;
}

// otherwise we can try the query here now!
try {
    // YOUR SQL UPDATE QUERY
} .....`

edited Feb 24, 2017 at 1:29

answered Feb 24, 2017 at 0:28

Darren

13.1k4 gold badges44 silver badges81 bronze badges

Sign up to request clarification or add additional context in comments.

11 Comments

carpenumidium Over a year ago

I'm not getting any errors, that's the (not so) funny part. And yep. Guilty about the careless form build. I put the id in a hidden field to have something to match the logged in user with. Bad practices all around from my part, I'm kinda (totally) flailing in the dark at the moment. Thanks for the heads-up though.

Darren Over a year ago

@carpenumidium What is $auth_user? I don't see you instantiating it anywhere? Ps, don't stress man, we all have to start somewhere and learn from our mistakes! Just keep going :). Lets sort this out!

carpenumidium Over a year ago

I've updated the post with the whole php for view-account.php. $auth_user is user to instantiate the USER class from class.user.php. Thanks mate :)

carpenumidium Over a year ago

I'm following this tutorial codingcage.com/2015/04/… and its forms didn't have an action attribute for forms. I'll try out your suggestions.

Darren Over a year ago

@carpenumidium That'd be because they form submit goes to the page it's on (where the processing is done), but because this is 2 separate files, you need to specify where the action needs to take place :-)

|

e_i_pi · Accepted Answer · 2017-02-24 00:45:27Z

4

Righto, you have a few problems with things not matching up etc., which is to be expected if you are starting out.

Let's start with the HTML form. There are two issues here:

The form has no action property, so it doesn't get submitted anywhere
The submit button is given a specific name and no value. (While some will consider this okay, maybe we can try a different approach which is a little more sensible)

I would suggest your HTML form be changed to this:

<form method="post" action="view-account.php">
    <input type="hidden" id="id" name="id" value="<?php echo $_SESSION['id']; ?>">
    <label for="full-name" class="input-info">
        <div class="label">full name</div>
        <input type="text" id="full-name" name="full-name" value="<?php echo $_SESSION['name']; ?>">
    </label>
    <label for="mobile" class="input-info">
        <div class="label">mobile number</div>
        <input type="text" id="mobile" name="mobile" value="<?php echo $_SESSION['mob']; ?>">
    </label>
    <label for="addr" class="input-info">
        <div class="label">address</div>
        <input id="addr" name="addr" value="<?php echo $_SESSION['addr']; ?>">
    </label>
    <label for="bank-acc" class="input-info">
        <div class="label">bank account number</div>
        <input type="text" id="bank-acc" name="bank-acc" value="<?php echo $_SESSION['accNo']; ?>">
    </label>
    <hr>
    <label for="password" class="input-info">
        <div class="label">password</div>
        <input type="password" id="password" name="password">
    </label>
    <button type="submit" name="submit" value="save">
        Save Changes
    </button>
</form>

Now, once this form is submitted to view-account.php, we want to make sure that the submit button is "save" mode, so we change the first line of view-account.php to this:

if(isset($_POST['submit']) && $_POST['submit'] === 'save') {

This approach means we can have different submit buttons on the same form - we may in future want actions for save, delete, archive, etc.

Lastly, I notice that the id field in your database table is declared AUTOINCREMENT. Great, exactly what you want, database id fields are internal unique identifiers that we let the database determine (in 99% of cases - there are edge cases where we like to define our own UIDs). This means that there is a problem with your UPDATE statement. You cannot update an auto-incremented field. In your class.user.php file, change the declaration of $stmt to this:

$stmt = $this->conn->prepare(
    "UPDATE users
    SET
        name = :uname, 
        mobile = :umob,
        address = :uaddr,
        accNo = :uacc, 
        password = :upass
    WHERE id = :id"
);

This should fix your code issues, I think I got everything. BUT, there may be other problems. If your code still does not work, I would suggest checking your error logs. If you're not sure where they are, either check your php.ini file to see what the error log location is, or override the default location by putting this at the top of the page you're trying to debug:

ini_set("error_log", "/path/to/error.log");

answered Feb 24, 2017 at 0:45

e_i_pi

4,8605 gold badges32 silver badges47 bronze badges

3 Comments

carpenumidium Over a year ago

Hi, thank you for the post! I tried out your amendments and in the error logs it's saying [24-Feb-2017 01:57:42 Europe/Berlin] PHP Notice: Undefined index: id in C:\xampp\htdocs\gas\view_account.php on line 81 which is where the html form begins.

DCR Over a year ago

do <?php print_r($_SESSION); ?> just before the form in the html section and see if $_SESSION['id'] is defined

e_i_pi Over a year ago

What @DCR said, the array key id doesn't exist in the $_SESSION global variable. Perhaps in session.php the session isn't starting, maybe there was never that key in the variable in the first place. This is where the art of debugging is practiced :)

Collectives™ on Stack Overflow

How do I update a row of data in mysql using PHP?

2 Answers 2

Update

Notes

11 Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Update

Notes

11 Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related