1

I am trying to have a list of registered people show from my SQL db.

I have a table that each column outputs into but where I am running into a problem is with the checkbox that will be updated to show whether the person has paid or not. I will be checking the box not the customer.

So I have run into 2 issues so far:

1) When I submit the update to save the checked box into it's column in the SQL db, it does not show the box being checked unless I refresh the page where it then goes and gets the updated (newly stored) value of checked.

2) When I am trying to uncheck the box and try to save I get an error that says my paymentStatus array is and undefined index since there is no value assigned to it.

I have been going around in circles on this and I can't imagine it's really this hard so I am thinking there must be a better way of accomplishing what I am trying to do.

Here is my current code: http://pastebin.com/bgAtYqbL

Thanks for any help.


Full code:

<?php
    require_once ('functions.php');
    require_once ('includes/cred.php');

    $query = $conn->prepare("SELECT * FROM sponsors");
    //$query->bindParam(":idnum", "1");
    $query->execute();
    $notPaid = $query->fetchAll();


    if (isset($_REQUEST["sponsorUpdate"])) {
        //Create Sessions
        $array = $_POST;
        foreach ($array as $key => $value) {
            $_SESSION[$key] = $value;
        }

        if(isset($_POST['paymentStatus'])) {
            $payment = $_POST['paymentStatus'];
            foreach($payment as $value) {
                $stmt = $conn->prepare("UPDATE sponsors SET paid=1 WHERE id = $value");
                $stmt->execute();
            }
        } else {
            $payment = $_POST['id'];
            foreach($payment as $value) {
                $stmt = $conn->prepare("UPDATE sponsors SET paid=0 WHERE id = $value");
                $stmt->execute();
            }
        }
    }

?>
<html>

<head>
</head>
<body>
<form action="<?php $_SERVER['PHP_SELF']; ?>" method="post">
<table border="1" cellspacing="0" cellpadding="10">
    <tr>
        <th>First Name</th>
        <th>Last Name</th>
        <th>Company Name</th>
        <th>Email Address</th>
        <th>Phone Number</th>
        <th>Address</th>
        <th>City</th>
        <th>State</th>
        <th>Zipcode</th>
        <th>Sponsor Level</th>
        <th>Payment Received</th>
    </tr>
    <?php 

            foreach ($notPaid as $row) {
            $phone = preg_replace('/(\d{3})(\d{3})(\d{4})/', '($1) $2-$3', $row['phone']);
        ?>
    <tr>
        <td><?php echo $row['fname']; ?></td>
        <td><?php echo $row['lname']; ?></td>
        <td><?php echo $row['cname']; ?></td>
        <td><?php echo $row['email']; ?></td>
        <td><?php echo $phone; ?></td>
        <td><?php echo $row['address']; ?></td>
        <td><?php echo $row['city']; ?></td>
        <td><?php echo $row['state']; ?></td>
        <td><?php echo $row['zip']; ?></td>
        <td><?php echo $row['sponsorLevel']; ?></td>
        <td><input name="paymentStatus[]" type="checkbox" value="<?php echo $row['id']; ?>" <?php if ($row['paid'] == 1) echo 'checked="checked"'; ?></td>
    </tr>
    <?php
     } ?>
</table>
    <input type="submit" name="sponsorUpdate" value="Save Changes" />
</form>
</body>
</html>
Funk Forty Niner
  • 74,450
  • 15
  • 68
  • 141
  • 2
    **By building SQL statements with outside variables, you are leaving yourself open to SQL injection attacks.** Also, any input data with single quotes in it, like a name of "O'Malley", will blow up your SQL query. Please learn about using parametrized queries, preferably with the PDO module, to protect your web app. [This question](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) has many examples in detail. You can also see http://bobby-tables.com/php for alternatives and explanation of the danger. – Andy Lester Apr 04 '14 at 15:06
  • That actually does look like the PDO module. The code is just missing the `bindParam` statement to bind the `$value` variable to a parameter. http://www.php.net/manual/en/pdostatement.bindparam.php – Cypher Apr 04 '14 at 16:08
  • 1
    My original implementation of this includes the bindParam statement, however, in an attempt to just test if this is working I have included basic variables. Currently this is all being developed locally so injection is not a concern as I am still in the early stages and trying to get the basic functionality working. I would however like the original question to be answered, not looking for help securing this. – user2908959 Apr 04 '14 at 16:19

1 Answers1

0

1) When I submit the update to save the checked box into it's column in the SQL db, it does not show the box being checked unless I refresh the page where it then goes and gets the updated (newly stored) value of checked.

This is because on each page load, you are doing a SELECT first, then an UPDATE. Switch the order of this. UPDATE your database with any new user-input (if any), and then SELECT to display those potentially updated changes to the user.

2) When I am trying to uncheck the box and try to save I get an error that says my paymentStatus array is and undefined index since there is no value assigned to it.

This is to be expected. If you look at the HTTP request in something like Firebug, you'll see that an unchecked checkbox is not sent with the request. That means $_POST['paymentStatus'] is not going to be set. You'll need to account for that in your code when attempting to save the value of that checkbox to your database.

If you want to test to see if the checkbox has been checked, something like this should do the job:

$paymentStatus = isset($_POST['paymentStatus']);

The variable $paymentStatus would then contain a true or false value, depending on whether or not the checkbox was checked when the form was sent. This is because if it WAS checked, the name of the input would be sent with the request with a value of on. If it was NOT checked, it just wouldn't be sent at all and wouldn't show up in $_POST.

A quick example demonstrating the above:

if (isset($_REQUEST["sponsorUpdate"])) 
{
    // if the paymentStatus parameter is sent with the HTTP request
    // the $paymentStatus variable will be set to 1 which indicates that
    // the checkbox was checked.  If it was not checked, it will have a
    // value of 0.

    $paymentStatus = isset($_POST['paymentStatus']) ? 1 : 0;

    // always test your input before putting data into a database
    // the bindParam() method should take care of this for us, but it's
    // good practice anyway

    if ($paymentStatus === 0 || $paymentStatus === 1)
    {
        $stmt = $conn->prepare("UPDATE sponsors SET paid=?");
        $stmt->bindParam(1, $paymentStatus);
        $stmt->execute();

        // test for SQL errors ...
    }
}

There is one final problem I see with your solution, and it's that you are relying on two things that aren't going to help you with regard to your checkboxes:

  1. The value of a GET or POST parameter associated with a checkbox is always sent to the server with the value of 'on' if it is checked, or just not sent at all if it is not checked. The value property is not sent, so your server will never get the ID of the row you want to update.

  2. Since nothing is sent to the server to indicate that a checkbox is not checked, you aren't going to be able to use that input as your key to loop through when saving all of your form inputs. You are going to need to re-think your design, unfortunately.

Community
  • 1
  • 1
Cypher
  • 2,608
  • 5
  • 27
  • 41
  • Thanks Cypher, 1 makes sense. For 2, I guess that is my actual question is how do I account for that. I understand what it's doing but I have not come up with a good solution for it. – user2908959 Apr 04 '14 at 16:22
  • I added a bit more about the second part of your question. Let me know if you need further clarification. – Cypher Apr 04 '14 at 16:27
  • Setting this works but only gets me as far as my original issue. If the box is checked all is fine, it runs the code within my if statement. There is still an issue of undefined index within the else statement. There will potentially be multiple boxes checked at the same time so I need it to be able to run through all iterations and update their row. I am not sure how to give it the associated row id without the undefined index? – user2908959 Apr 04 '14 at 16:40
  • I'm not sure I understand your comment entirely. However, I have updated the answer to give a demonstration of how I go about dealing with checkboxes in forms. – Cypher Apr 04 '14 at 16:49
  • `There is still an issue of undefined index within the else statement.` If you attempt to access an index of an array that doesn't exist, you'll get a PHP error. You'll need to test for that using `isset()` or `array_key_exists()`. You can even use the `filter_input()` function on superglobals to retrieve values without triggering that error. – Cypher Apr 04 '14 at 16:52
  • I tried the newly added code you provided but it does not work - I assume it has to do with the $value variable not actually being given a value anywhere. More detailed description of my previous comment: My page will output all registered users into a table. One of the fields for each registered user will be the paid checkbox. I want to be able to check multiple different users as paid at the same time then hit the update button to save them all. Do you not have to have a way to identify which iteration the loop is currently on for it to know which record it's updating? – user2908959 Apr 04 '14 at 17:03
  • You beat me to it, I was about to mention changing $value to $paymentStatus. The 2nd part of the previous comment still stands though, when I have more than one user and I check the box for one of them they are all checked. – user2908959 Apr 04 '14 at 17:07
  • @user2908959 Good catch. :-) That's actually quite a different question, because you can't have multiple form fields with the same name. Well, you *can*, but if you have multiple checkboxes with the name 'paymentStatus', PHP will only have one value for `$_POST['paymentStatus']`. There are some tricks out there that you can use, but there are browser issues. You may have to re-think your solution, or get creative. – Cypher Apr 04 '14 at 17:12
  • Right, that's why in my original code I had paymentStatus as an array that used the foreach to give it an id to assign the SQL update to. – user2908959 Apr 04 '14 at 17:15
  • @user2908959 Well, that's the source of another problem that I should probably mention in the answer. – Cypher Apr 04 '14 at 17:26