PHP Form must be submitted twice to update checkbox

Question

I'm still relatively new to PHP. I'm trying to build a privacy settings page for members to opt out of automatic emails for triggered events (i.e. private message notification). I want the checkbox set automatically based on the database setting. As of now, the form does update the database correctly, but the checkbox status does not show the correct setting unless the Submit button is pressed twice, or the page is reloaded. Setting would be '0' for unchecked, '1' for checked. I'd love to use Ajax or jQuery to handle this, but I don't know those at all.

privacysettings.php

<?php
  $id = "";
  $pm_mail_able = "";
  $pm_email = "";

  if (isset($_GET['id'])) {
    $id = preg_replace('#[^0-9]#i', '', $_GET['id']); // filter everything but numbers
  } else if (isset($_SESSION['idx'])) {
    $id = $logOptions_id;
  } else {
    header("location: index.php");
    exit();
  }

  //query to get checkbox status
  $sql = mysql_query("SELECT * FROM members WHERE id='$id'");

  while($row = mysql_fetch_array($sql)){
    $pm_mail_able = $row['pm_mail_able'];
  }

  switch ($pm_mail_able) {
    case 0:
      $pm_setting = NULL;
      break;
    case 1:
      $pm_setting = "checked=\"checked\"";
      break;
  }

  if(isset($_GET['pm_email']) && !empty($_GET['pm_email'])) {
    $updateqry = mysql_query("UPDATE members SET pm_mail_able='1' WHERE id='$id'");
  } else {
    $updateqry = mysql_query("UPDATE members SET pm_mail_able='0' WHERE id='$id'");
  }

?>

<html>
    Email Notifications<br />

    <form name="testform" method="get" action="PvResult.php">
        When a friend sends me a private message
        <input type="checkbox" name="pm_email" value="on"<?php echo $pm_setting;?> />
        <br /><br />
        <input type="submit" value="Submit" />
    </form>
</html>

PvResult.php

<?php

  $url = 'http://www.mywebsite.com';

  //If the form isn't submitted, redirect to the form
  if(!isset($_GET['Submit']))
    header('Location: '.$url.'/privacysettings.php');

  //Redirect to the correct location based on form input
  $pm_email = $_GET['pm_email'];
  $url .= '/privacysettings.php?pm_email='.$pm_email;

  header('Location: '.$url);
?>

from where do you get $id? keep a space after value="on" in checkbox field /> — Pradeep Sanjaya, Jul 01 '12 at 03:42
@King Skippus - i declared it at the top of the form. left it out for brevity, but just edited it in. As I said, the database *IS* correctly updating on first hit of the submit button. The problem is the checkbox is not updating. — Veloj, Jul 01 '12 at 03:44
I see your edit, but this still won't work. $id will always be "", so your query will probably always fail because it will always end with ...WHERE id=''. How are you passing it into the script? From a $_GET or $_POST variable? From $_COOKIE or $_SESSION? I know you can't really be setting it to "" at the top of your script. I know you don't think it's important to share those kind of details, but it really is. — King Skippus, Jul 01 '12 at 03:45
@Pradeep - added the space where you suggest, still same issue. Addressed the $id above, but it is pulled from a Sessions and/or cookie when the user logs in. — Veloj, Jul 01 '12 at 03:48
@King - I edited in where the $id value is set. This same code is in all of the other pages for the site and have had no issues with using $id elsewhere. — Veloj, Jul 01 '12 at 03:51

score 2 · Answer 1 · edited May 23 '17 at 11:58

Okay, hopefully this won't just answer your question, but give you a few best practices you might want to consider.

You can combine these two scripts into one relatively easily. Also, I'd highly suggest using a POST instead of GET; GET is very limited and is not intended to submit data like you're using it. If you're going to be changing data in a back-end store, using GET will bite you. Maybe not today, maybe not tomorrow, but it will, trust me.

You really should consider moving to PDO instead of the mysql_ functions. PDO is a lot better in handling parameterized queries, which you really should have here for better security, and it's more portable if someday you want to move to a different database system.

I'm still a little hazy on how your app is getting the $id. Most apps get it from a $_SESSION variable, making sure that the user has successfully validated a login. If you're not doing that, please do. You might want to thoroughly digest this article, it's got a lot of juicy best practices regarding authentication and "remember me"-type functionality.

Here's a bit of a rewrite. I haven't actually tested it, but it should give you a pretty good idea on where to go with your immediate needs. If it throws any errors (remember the disclaimer: I haven't actually tested it!), let me know and I'll try to debug it.

<?php
$message = '';
$pm_setting = '';
$id = 0;

// Put your $id retrieval logic here.  It should look something like:
if (isset($_SESSION['id'])) {
    $id = $_SESSION['id'];
    if (!preg_match('/^\\d{1,10}$/', $id) > 0) {
        // Someone is trying to hack your site.
        header("location: scum.php");
        exit();
    }
    $id = intval($id);
}
// Quick security note: You might want to read up on a topic called
// session hijacking if you want to ensure your site is secure and
// this $id isn't spoofed.

if (isset($_POST['Submit'])) {
    // The form is being submitted.  We don't need to read the current
    // pm_mail_able setting from the database because we're going to
    // overwrite it anyway.
    if ($id > 0) {
        $pm_mail_able = 0;
        if (isset($_POST['pm_email']) && $_POST['pm_email'] === 'on') {
            $pm_mail_able = 1;
            $pm_setting = 'checked ';
        }
        $query = 'UPDATE members SET pm_mail_able='.$pm_mail_able.
            ' WHERE id = '.$id;
        mysql_query($query);
        // Another quick security note: You REALLY need to consider
        // updating to PDO so that you can bind these parameters
        // instead. The mysql_ functions are probably going to be
        // deprecated soon anyway.

        if (mysql_affected_rows($query) > 0)
            $message = '<p style="color: #00a000;">Settings saved!</p>';
        else
            $message = '<p style="color: #a00000;">User id not valid.</p>';
    }
    else
        $message = '<p style="color: #a00000;">User id not valid.</p>';
}

else {
    // This is the first load of the form, we need to just display it
    // with the existing setting.
    if ($id > 0) {
        $query = mysql_query('SELECT * FROM members WHERE id = '.$id);
        if (($row = mysql_fetch_array($query, MYSQL_ASSOC)) !== FALSE)
            if ($row['pm_mail_able'] === 1) $pm_setting = 'checked ';
    }
}

?>
<html>
    <body>
        <?= $message ?>
        <!-- Without action parameter, form submitted to this script. -->
        <form name="testform" method="post">
            E-mail notifications<br />
            <input type="checkbox" name="pm_email" value="on" <?= $pm_setting ?>/>
            When a friend sends me a private message
            <br /><br />
            <input type="submit" value="Submit" />
        </form>
    </body>
</html>

I originally was using just one form AND was using POST instead of GET. Found some forums online with the similar issue and evolved to the current files, which resolved all but the checkbox issue. I will give your revision a shot and let you know what happens. Thanks for sticking with this. — Veloj, Jul 01 '12 at 04:41
By the way, I just noticed that I used the short expression syntax of = $expr ?>. If you prefer, you can use . Some php installations may not have the short expression syntax enabled, but I think most do. It's always on in recent versions of php. http://www.php.net/manual/en/ini.core.php#ini.short-open-tag — King Skippus, Jul 01 '12 at 05:00

Marcio Mazzucato · Answer 2 · 2012-07-01T04:45:33.050

1

Try to do these settings and see if it will work:

1) You need to add an space between "on" and "checked=checked"

<input type="checkbox" name="pm_email" value="on" <?php echo $pm_setting;?> />

2) You have to reference the submit button by its name, not its value

<input type="submit" name="Submit" value="Send" />

3) When the setting is "0", set $pm_setting as a empty string, instead of NULL

case 0:
    $pm_setting = '';

4) Maybe there is some problem with $_GET['pm_email'] and the else is always being executed

5) If the things work when you press the Submit button twice, it means that the form is passing some GET var that make the code work, so try to discover what var is this

edited Jul 01 '12 at 04:45

answered Jul 01 '12 at 03:47

Marcio Mazzucato

7,849
6
60
75

did both but still same issue, and further the submit button now says "Submit Query" by default. – Veloj Jul 01 '12 at 03:57
@Veloj, Are you sure that the field's name is `pm_mail_able` in database? – Marcio Mazzucato Jul 01 '12 at 04:01
positive. I am verifying after using the form that the values are changing between 0 and 1 as desired. Could the issue be the order of when the updateqry is called? – Veloj Jul 01 '12 at 04:05
@Veloj, It's possible. And it's possible too that the code is always updating to "0", have you checked it? – Marcio Mazzucato Jul 01 '12 at 04:20
it's definitely going between 1 and 0 and 1, etc. I refresh MYSQLAdmin after each submit. It's only changing in the DB when a different checkbox value is sent. It's not even changing when I hit the submit button for the second time with the same value, as it shouldn't. – Veloj Jul 01 '12 at 04:25
@Veloj, Try to `echo` the vars `$pm_mail_able` and `$pm_setting` after the `switch` and tell me their contents. – Marcio Mazzucato Jul 01 '12 at 04:32
Marcio - echoes result as follows (by condition): initially unchecked = "0" for both 1st and 2nd times hitting submit. initially checked = "1, checked="checked" " for both times hitting submit. which is what i would expect. – Veloj Jul 01 '12 at 04:47
@Veloj, I've done some local tests and they worked. Your problem should be some small detail, so you have to analyse it with big attention. Additionally, see the tips of "King Skippus", they will help you to get some good practices. – Marcio Mazzucato Jul 01 '12 at 05:29

PHP Form must be submitted twice to update checkbox

2 Answers2