0

The following script is supposed to update my database, the environment column with the selected value for the drop down menu. Right now, the last host is getting a value of 0. What am I doing wrong? It seems like I am not able to track the hosts and the value that was selected.

while($row = mysql_fetch_array($result)){

$host = $row['host'];
$environment = $row['environment'];

echo "<tr><td>" . $host . "</td>
      <td><select name='id[".$host."]'><option value='Null'>Select any</option>
                              <option value='DEV/QA/TEST'>DEV/QA/TEST</option>
                              <option value='PROD/STAGE'>PROD/STAGE</option>
                              </select></td>
                              <td>" . $environment . "</td></tr>"; }


echo "</tbody></table><input type='submit' value='submit'></form>";

if (gettype($_POST['id'])=="array") {

    foreach($_POST['id'] as $host => $val){

            $id_c = $val;
if ($val != 'Null') {
            $query1 = "UPDATE hosts SET environment = '$val' where host='$host'";

            $result1 = mysql_query($query1);

            if($result1 === false) {

            die(mysql_error());
         }

     echo "Environment for Host " .$host. " is updated. <br>";

}}}

Updated with working script.

  • First of all, for god sake, stop using mysql_ functions. It's been deprecated years ago. Second, you're applying the update where sql column is equal to the variable $host, but $host is a variable that belongs to the while that is already closed at that point. As you're using a "dead" local variable from a loop, you're bringing only the last result from that query to your loop. In other words, your foreach is always updating the same $host. – Matheus Loureiro Aug 19 '15 at 16:57
  • I will move to mysqli or POD. In this case, How can I keep track of the hosts and the value selected to reflect in the update statement? – Larry White Aug 19 '15 at 16:59
  • If I were you, I would pass the host's ID as an argument inside id. Instead of the select's name be `id[]`, which is impossible to track its origin, I'd use `id[]`. So in your foreach you can use this: `foreach($_POST['id'] as $hostId => $val)` – Matheus Loureiro Aug 19 '15 at 17:01
  • I am getting the following error when I edit this line. echo "" . $host . " " . $environment . ""; } – Larry White Aug 19 '15 at 17:23
  • [Wed Aug 19 10:19:30 2015] [error] [client 10.144.101.66] PHP Parse error: syntax error, unexpected T_STRING, expecting ',' or ';' in /var/www/html/index.php on line 38 – Larry White Aug 19 '15 at 17:23

1 Answers1

0

Your code is wrong. I'll try to explain what you are currently doing and how you should have code it.

First mistake:

while($row = mysql_fetch_array($result)) {
    echo '<select name="id[]">';
    [...]
}

You are creating one select by host (that's OK). But once it's created, you can't say which select goes for which host. The only way you can tell it is with the order it appears, that's not very reliable.
I'd suggest that you identify your select with the host id (or any identifier you have).
Like this:

while($row = mysql_fetch_array($result)) {
    echo '<select name="id['.$row['host_id'].']">';
    [...]
}

Second mistake:

while($row = mysql_fetch_array($result)) {
    $host = $row['host'];
}
// Here, $host contains the last row

foreach ($_POST['id'] as $val) {
    // UPDATE $host with $val
}

What's happening here: you are updating the last host with every select value you passed via the form.

Here is what you should do:

while($row = mysql_fetch_array($result)) {
    echo '<select name="id['.$row['host_id'].']">';
    [...]
}

foreach ($_POST['id'] as $host_id => $val) {
     // UPDATE hosts SET environment = $val WHERE host_id = $host_id
}

That way you will be updating each host for with each matching value in $_POST['id'].
You can print_r($_POST) to understand its structure.

  • Can you please edit my original post with what the right way it is to do? The following line starts with an echo " so the " inside is not being appreciated. echo "" . $host . " " . $environment . ""; } – Larry White Aug 19 '15 at 17:25