1

I'm setting a page where there are a couple of configurations.

So I want the page to bring stuff from the database with a SELECT query, show it to the user and for the user to be able to update it and save it into the database with an UPDATE.

So I have the select query:

$buscarTemplates = " SELECT mailNombre, mailAsunto, mailMensaje, mailDetalle
                                 FROM configMailsUsuarios

And I show the list of available options with a while loop:

$mostrarTemplates = mysqli_query($conectar,$buscarTemplates);
    while ($mails=mysqli_fetch_array($mostrarTemplates)) {

        echo '<h4>'.$mails['mailDetalle'].'</h4>';
        echo '<div class="form-group">
            <input type="text" name="mailNombre" value="'.$mails['mailNombre'].'" readonly>
            <label for="mailAsunto">Asunto:</label>
            <input type="text" name="mailAsunto" value="'.$mails['mailAsunto'].'">
            </div>';
        echo '<textarea class="form-control" name="mailMensaje" rows="4">'.$mails['mailMensaje'].'</textarea>';             
    }

When the user hits the submit button:

 if(isset($_POST['submit'])) { 
    $itemsFetched = mysqli_num_rows($mostrarTemplates);
    for ($i=0; $i < $itemsFetched; $i++) { 
        $mailNombre = $_POST['mailNombre'];
        $mailAsunto = $_POST['mailAsunto'];
        $mailMensaje = $_POST['mailMensaje'];
            $guardarTemplates = "UPDATE configMailsUsuarios
                                 SET mailAsunto = '$mailAsunto',
                                     mailMensaje = '$mailMensaje' 
                                 WHERE mailNombre = '$mailNombre'
                                ";
            if (mysqli_query($conectar,$guardarTemplates)) {
                echo 'Configuraciones guardadas exitosamente.<br>';
                echo '<a href="templateMails.php"><input type="submit" name="submit" class="btn btn-danger btn-xs" value="Recargar la página"></input></a><br>';
            } else {
                echo 'Oops!';
            }               
    }

The problem is that there are two items in the fetched list, and only the second one gets saved.

This is what it is currently in the database table "configMailsUsuarios":

mail ID --- mailNombre ------ mailAsunto ----- mailMensaje -- mailDetalle
1 --------- mailBienvenida -- AvisoA --------- mensajeA ----- XXY
1 --------- mailMorosos ----- AvisoB --------- mensajeB ----- YYX

So, I've tried printing out what the query actually returned inside the for loop with echo $guardarTemplates.'<br>'; And the result shows that the loop runs twice (as expected), with the new information typed by the user, but the exact same query, that just takes the second item in the list:

UPDATE configMailsUsuarios SET mailAsunto = 'Aviso2', mailMensaje = 'whatever' WHERE mailNombre = 'mailMorosos' 
UPDATE configMailsUsuarios SET mailAsunto = 'Aviso2', mailMensaje = 'whatever' WHERE mailNombre = 'mailMorosos' 

The expected result should be taking the first row and updating it, and then the second ones:

UPDATE configMailsUsuarios SET mailAsunto = 'Aviso1', mailMensaje = 'blah blah' WHERE mailNombre = 'mailBienvenida' 
UPDATE configMailsUsuarios SET mailAsunto = 'Aviso2', mailMensaje = 'whatever' WHERE mailNombre = 'mailMorosos' 

What am I doing wrong?

Rosamunda
  • 14,620
  • 10
  • 40
  • 70

1 Answers1

1
        <input type="text" name="mailNombre[]" value="'.$mails['mailNombre'].'" readonly>
        <input type="text" name="mailAsunto[]" value="'.$mails['mailAsunto'].'">

Those should be array, otherwise the first gets overwritten by the last one.

Then you have to change your script a little, so it accesses the correct index of your $_POST-array, you have the for-loop already set up so that is good, but you should define it by the number of elements in your request or show us what data is in $itemsFetched = mysqli_num_rows($mostrarTemplates);.

You should check out How can I prevent SQL injection in PHP? As you are not using prepared statements your script is vulnerable.

In a foreach-loop you could access the fields like this:

if(isset($_POST['submit'])){
    foreach ($_POST['mailNombre'] as $key => $value) {

        $stmt = mysqli_prepare($link, "UPDATE configMailsUsuarios 
                                       SET mailAsunto = ?, mailMensaje = ? 
                                       WHERE mailNombre = ?");
        mysqli_stmt_bind_param($stmt, "sss", $mailNombre, $mailAsunto, $mailMensaje);

        $mailNombre = $_POST['mailNombre'][$key];
        $mailAsunto = $_POST['mailAsunto'][$key];
        $mailMensaje = $_POST['mailMensaje'][$key];

        mysqli_stmt_execute($stmt);
    }
}
Community
  • 1
  • 1
Philipp
  • 2,787
  • 2
  • 25
  • 27
  • I think I should change it to a foreach loop instead? Each one for each array?? And yes, it's just a proof of concept, having added any security yet – Rosamunda Jul 30 '16 at 20:55
  • one foreach is enough, added an example. You only need it for the keys to access the fields correctly – Philipp Jul 30 '16 at 20:59