0

I have some code here to update my database based on two columns in the CSV. The CSV file would look like this:

ID Response

1 Hello1

2 Hello2

3 Hello3

In my database I have a table that also contains an id column and an extra column that matches response. The idea is this CSV file is uploaded and will populate the response that matches the ID number.

Basically this: "UPDATE tbl_data SET response = {$response} WHERE id = {$id}"

The form that performs this action look like this:

<form method="post" name="uploadCSV" enctype="multipart/form-data">
    <label>Choose CSV File</label>
    <input type="file" name="csv_file" id="file" accept=".csv" />
    <button type="submit" name="import" class="read-more smaller">Upload</button>
</form>

However, I don't think I've understood how to do this properly, as I get SQL errors, or the form just sits there as if nothing has happened. See code below.

if (isset($_POST["import"])) {

    if($_FILES["csv_file"]["name"]){

        $filename = explode(".", $_FILES["csv_file"]["name"]);

        if(end($filename) == "csv"){

            $handle = fopen($_FILES["csv_file"]["tmp_name"], "r");

            while ($data = fgetcsv($handle)){

                $id = $data[0];
                $response = $data[1];

                $query ="UPDATE tbl_data SET response = {$response} WHERE id = {$id}";
                $update_data = mysqli_query($connection,$query);

                if (!$update_data) {
                    $message = "There was a problem updating your CSV file. If this problem reoccurs, please contact admin";
                    die (mysqli_error($connection));
                }

            }

            fclose($handle);

            header("Location: upload.php?uploaded=1");

        } else {
            $message = "You can only upload a CSV file.";
        }

    } else {
        $message = "Please select a CSV file.";
    }

}

I have the $message to shows the message. but it doesn't show up any of the messages, and the update in the database doesn't appear to take place either.

Is there any errors that I may have overlooked in my code? Or is there a much better way to do this?

Qiniso
  • 2,587
  • 1
  • 24
  • 30
  • "I have the $message variable echo'd to the page but it doesn't show up any of the messages"...have you? where? And this one `$message = "There was a problem updating your CSV file...` will **never** show anyway because you `die()` immediately afterwards which stops the script executing. But you should see the mysqli error in that scenario. Do you ever see that? You mentioned sometimes getting SQL errors, but then didn't tell us what they were. – ADyson Nov 07 '19 at 17:15
  • 1
    Does this answer your question? [Importing CSV data using PHP/MySQL](https://stackoverflow.com/questions/11448307/importing-csv-data-using-php-mysql) – richyen Nov 07 '19 at 17:17
  • I'd suspect it might be something to do with strings though, since `$response` is a string (e.g. "hello") yet you aren't enclosing it in quote marks in the SQL statement. To be honest though you really should use [prepared statements and parameterised queries](https://bobby-tables.com/php) to guard against the possibility of malicious input in the CSV file turning you into a victim of [SQL injection](https://bobby-tables.com/). It will also take care of things like escaping string inputs properly, thus avoiding unexpected syntax errors in the SQL statement. – ADyson Nov 07 '19 at 17:21
  • P.S. the LOAD DATA command with the REPLACE modifier might also work better for you here, and speed things up. Check the question linked by RichYen and also the [MySQL documentation](https://dev.mysql.com/doc/refman/8.0/en/load-data.html) – ADyson Nov 07 '19 at 17:29
  • 1
    I don't see you echoing `$message`. – Funk Forty Niner Nov 07 '19 at 18:52
  • $message is above the form, just forgot to include it in the code bit, apologies. –  Nov 08 '19 at 10:40
  • @ADyson Thanks I'll check it out, I planned to make it a prepared statement, just want to get it working first. PHP is still fairly new to me, which is why I've not gone for object oriented methods for now. –  Nov 08 '19 at 10:42
  • "just want to get it working first"...there's really no point because to make it into a prepared statement etc you have to re-write the query code anyway...so then you have to test it and "get it working" all over again. So you're just creating extra effort for yourself. If you follow the many many examples available online you can get it working properly the first time, and save yourself the hassle of re-doing it. As I mentioned, it can actually reduce the risk of unexpected mistakes anyway, so it should actually make your task easier. – ADyson Nov 08 '19 at 11:18
  • P.S. The use of prepared statements has nothing to do with object-orientedness. There are both procedural and object-oriented versions of all the relevant functions, as shown in the [documentation](https://www.php.net/manual/en/mysqli.prepare.php) e.g. mysqli_prepare(), just the same as mysqli_query(). But OTOH yes I would recommend you use the object-oriented syntax wherever possible going forward - once you get used to it you'll find it makes your code easier to read, maintain and organise in general. And it's generally less verbose, too. – ADyson Nov 08 '19 at 11:21
  • you said "I planned to make it a prepared statement, just want to get it working first. PHP is still fairly new to me, which is why I've not gone for object oriented methods for now"...which implies you didn't use prepared statements because you didn't want to use object-oriented methods. All I'm saying is that not wanting to use object-oriented methods doesn't prevent you from using prepared statements. No offence intended. – ADyson Nov 08 '19 at 12:44
  • "it's really not "extra effort" to just replace about 5 lines of code"...except for all the re-testing effort as well, since you'll have completely changed the query part of the code, which is the most fiddly bit of the whole thing. The least effort would be to simply write it that way to begin with, surely? Again, no offence intended, I just wanted to point out you could make your life easier, that's all. If you don't want any advice to make life easier or make your code better, fine I can go somewhere else. I've been through the same journey as you, so I know what will improve your code. – ADyson Nov 08 '19 at 12:45
  • "When I upload the file it says it's successful but makes no difference to my database one bit."...in that case you need to do some more detailed logging / debugging. Remove (for now) the `header("Location: upload.php?uploaded=1");` so you don't get redirected and lose information. Then add some `var_dump()` commands into each section of your code so you can see the values of significant variables at different points in the process, and see what path the code is actually taking and why. Then you can check how many times it loops, what the values are, what the SQL statements look like, etc. – ADyson Nov 08 '19 at 12:48

1 Answers1

2

Got it working by using the following

if(isset($_POST["importcsv"])){

        $file = $_FILES["csv_file"]["tmp_name"];
        $handle = fopen($file,"r");

        while ($row = fgetcsv($handle)) {

            $id = $row[0];
            $response = $row[1];

            $sql = "UPDATE table SET response = ? WHERE id = ?";
            $update_data_stmt = mysqli_stmt_init($connection);

            if (!mysqli_stmt_prepare($update_data_stmt, $sql)){
                die("Something went wrong with the upload. " . mysqli_error($connection));
            } else {
                mysqli_stmt_bind_param($update_data_stmt, "ss", $response, $id);
                mysqli_stmt_execute($update_data_stmt);
                if ($id == "ID" && $response == "Response"){
                    echo "";
                } else {
                    echo "Lead <b>{$id}</b>'s response was updated to <b>{$response}</b>.</p>";
                }
            }

        }

    }
  • It is a very bad idea to use `die(mysqli_error($conn));` in your code, because it could potentially leak sensitive information. See this post for more explanation: [mysqli or die, does it have to die?](https://stackoverflow.com/a/15320411/1839439) – Dharman Nov 09 '19 at 23:13
  • @Dharman you are correct, thanks for reminding me I left it in. There's a few I've left in for testing but when live they're not necessary! –  Nov 11 '19 at 09:44
  • Better to avoid manual error checking and enable mysqli errors instead with `mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);` – Dharman Nov 11 '19 at 09:44