0

I want to update data using Ajax and below is the sample code. The SweetAlert get displaced that it has been updated but it doesn't take effect in the database non thus it gives an error.

Ajax Code

This is the Ajax Script for the form submission when the submit button is clicked.

<script>
               $('#submit').on('click', function(event){
                event.preventDefault();
                var firstname = $('#firstname').val();
                var othername = $('#othername').val();
                var gender = $('#gender').val();
                var id_type = $('#id_type').val();
                var id_number = $('#id_number').val();
                var issue_date = $('#issue_date').val();
                var business_place = $('#business_place').val();
                var food_type = $('#food_type').val();
                var screened = $('#screened').val();
                var sub_metro = $('#sub_metro').val();
                var telephone = $('#telephone').val();
                var get_date = $('#get_date').val();
                var chit_number = $('#chit_number').val();
                var remarks = $('#remarks').val();
                var user_id = $('#user_id').val();
                var vendor_id = $('#vendor_id').val();

if (firstname!="" &&othername!="" && food_type!=""){
            $.ajax({
                url: "action/vendor_update.php",
                type: "POST",
                data: {
                    firstname:firstname, othername:othername, gender:gender, 
id_type:id_type, id_number:id_number, issue_date:issue_date,    
business_place:business_place, food_type:food_type, screened:screened, 
sub_metro:sub_metro, telephone:telephone, get_date:get_date,
                    chit_number:chit_number, remarks:remarks, user_id:user_id, vendor_id:vendor_id,
                },
                cache: false,
                success: function(data){
                    if(data.statusCode=200){
                        $('#dataForm').find('input:text').val('');
                            alert('Updated');
                            })      
                     }
                    else if (data.statusCode=201)
                     {
                            alert('Cannot Update');
                     }
                }
            });
        
        }else{
            alert('All fields are mandatory');
           }
    });

PHP Code ---> action/vendor_update.php

This code is for the php server side for data insertion into the database.

<?
        session_start();
        // include('../includes/session.php');
        include('./includes/connection.php');
        
        $query = $dbh->$prepare("UPDATE vendors SET Firstname=:firstname, Othername=:othername, Telephone=:telephone, Gender=:gender, IdType=:id_type, IdNumber=:id_number, IdDate=:issue_date, BusinessPlace=:business_place, FoodType=:food_type, Notes=:remarks, ScreenStatus=:screened, ChitNumber=:chit_number, UpdatedBy=:user_id WHERE VendorId=:vendor_id");

        $query->execute([
                $firstname = $_POST['firstname'];
                $othername = $_POST['othername'];
                $telephone = $_POST['telephone'];
                $gender = $_POST['gender'];
                $id_type = $_POST['id_type'];
                $id_number = $_POST['id_number'];
                $issue_date = $_POST['issue_date'];
                $business_place = $_POST['business_place'];
                $food_type = $_POST['food_type'];
                $remarks = $_POST['remarks'];
                $screened = $_POST['screened'];
                $chit_number = $_POST['chit_number'];
                $user_id =  $_POST['user_id'];
                $vendor_id = $_POST['vendor_id'];
          ]);
                
        // $query->execute($_POST);

if ($query->execute()){
   echo json_encode(array("statusCode"=>200));
   
} else {
   echo json_encode(array("statusCode"=>500));
}

                

?>
Bigboss
  • 1
  • 4
  • 2
    You are completely defeating the purpose of prepared statements by directly injecting the parameters into the query. See the [proper way to do it](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php). – El_Vanja Mar 24 '21 at 12:45
  • Outside of that, have you tried outputting the query and running it directly against the database to test you have formed it correctly? – El_Vanja Mar 24 '21 at 12:46
  • 2
    As an aside, you could probably save yourself a lot of repetitive code here by learning about jQuery's serialize function - see https://api.jquery.com/serialize/ . Anyway, to answer your actual question we'll need some debugging information. And yes, your code is still vulnerable to SQL injections - simply using prepare() isn't enough, you need to use parameters too. – ADyson Mar 24 '21 at 12:47
  • 3
    Shouldn't that be `if (data.statusCode==200) {` rather than the single = sign? – droopsnoot Mar 24 '21 at 12:49
  • P.S. `I want to delete data` ...well your code runs an `UPDATE` in the SQL. if you want to delete a record, then use a `DELETE` query. Unless you really mean "overwrite" rather than "delete"? Because sending all that form data wouldn't make any sense if you were intending to delete the data immediately. – ADyson Mar 24 '21 at 12:49
  • 1
    **WARNING**: Whenever possible use **prepared statements with placeholder values** to avoid injecting arbitrary data in your queries and creating [SQL injection bugs](https://bobby-tables.com/). These are quite straightforward to do in [`mysqli`](https://php.net/manual/en/mysqli.quickstart.prepared-statements.php) and [PDO](https://php.net/manual/en/pdo.prepared-statements.php) where any user-supplied data is specified with a `?` or `:name` indicator that’s later populated using `bind_param` or `execute` depending on which one you’re using. – tadman Mar 24 '21 at 12:58
  • 1
    Tip: If you're using PDO or `mysqli` you can bind placeholders *directly* to the `$_POST` values, there's absolutely no need for the intermediate variables. This saves a lot of code, reduces bugs, and makes your query vastly safer. – tadman Mar 24 '21 at 12:59
  • I tried with PDO ```?``` but i got the same issue @tadman – Bigboss Mar 24 '21 at 13:08
  • If you're just getting started with PHP and want to build applications, I'd strongly recommend looking at various [development frameworks](https://www.cloudways.com/blog/best-php-frameworks/) to see if you can find one that fits your style and needs. They come in various flavors from lightweight like [Fat-Free Framework](https://fatfreeframework.com/) to far more comprehensive like [Laravel](https://laravel.com/). These give you concrete examples to work from and guidance on how to write your code and organize your project's files. – tadman Mar 24 '21 at 13:08
  • @ADyson Sorry i want to update the data – Bigboss Mar 24 '21 at 13:08
  • If your query fails, the convention is to return HTTP status 500. 201 means [Created](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/201) which is the opposite of what happened here. This should also be set in the response headers. – tadman Mar 24 '21 at 13:09
  • Some other tips: In JavaScript use `let` in preference to `var`, the scoping is much more narrow. You can also omit repetition in your object definitions as `{ x: x, y: y, ... }` can collapse to `{ x, y, ... }` so long as the key and value variable match. Try and structure your code to use iterators whenever possible, like that value extraction could be a simple `map()` on a list of the fields you want defined as `const`. – tadman Mar 24 '21 at 13:11
  • @droopsnoot i just tried ```if (data.statusCode==200){``` it doesn't even run the sweetaler – Bigboss Mar 24 '21 at 13:12
  • @tadman Can you please use my code to elaborate on what you are trying to show me. – Bigboss Mar 24 '21 at 13:14
  • I'm not sure what your problem is, I'm just observing there's a lot of chaff in the way of understanding it. – tadman Mar 24 '21 at 13:15
  • I have change the query to prevent sql injection – Bigboss Mar 24 '21 at 13:17
  • 1
    @tadman, Am reducing the code to make it more readable – Bigboss Mar 24 '21 at 13:17
  • @Bigboss, well, that means that it isn't getting status 200, which means that the query isn't executing without error. So now you need to debug that. With the single = symbol, you were setting the statuscode every time, which is why it always said "success" even though the query didn't run. What is in `$_POST` when it gets to your PHP code when you debug it? If you've changed the code to use prepared statements, perhaps update the post with the new version. – droopsnoot Mar 24 '21 at 13:18
  • You have two `execute()`, a blank one and a second one with variables. And you prepare again after both those executions. Is this mysqli, or PDO? – droopsnoot Mar 24 '21 at 13:20
  • 1
    @Bigboss you're lucky that a lot of people are trying to help you in comments, however, it doesn't work like that on Stackoverflow. Pls, do not expect that someone will elaborate your own code. You can get here some advice and solution for problems, but not code re-writing. I'd suggest implementing some of these advices to make code cleaner, it will help you to identify code problems yourself and also optimize it. – biesior Mar 24 '21 at 13:22
  • @droopsnoot. This is what i get when i run the php alone ``` execute(array($firstname, $othername, $telephone, $gender, $id_type, $id_number, $issue_date, $business_place, $food_type, $remarks, $screened, $chit_number, $user_id, $vendor_id)); if ($query->execute()){ echo json_encode(array("statusCode"=>200)); } else { echo json_encode(array("statusCode"=>201)); } ?> ``` – Bigboss Mar 24 '21 at 13:24
  • @biesior Thanks for the headsup. Really appreciate – Bigboss Mar 24 '21 at 13:25
  • 1
    @Bigboss I don't know what you mean by that - that's the PHP code, not the results of what happens when you run it. It seems to me that some basic debugging should help you here. Are the variables correct in JS before you call the PHP? Are the `$_POST` variables correct when they get to PHP? Does the query run in phpmyadmin if you use the same values? – droopsnoot Mar 24 '21 at 13:27

1 Answers1

1

Here's the cleaned up PHP code:

<?php

// Tip:
// * Use named placeholders
// * Define the query string inside prepare() so you can't "miss" and run
//   the wrong query by accident
// * Use single quotes so you can't interpolate variables by accident
//   and create ugly SQL injection bugs
$query = $dbh->prepare('UPDATE vendors SET Firstname=:firstname, Othername=:othername, Telephone=:telephone, Gender=:gender, IdType=:id_type...');

// Use ONE of:

// A) if you have slight differences
$query->execute([
  'firstname' => $_POST['firstname'],
  'othername' => $_POST['othername'],
  'telephone' => $_POST['telephone'],
  ...
]);

// OR

// B) if you're confident the placeholders match 100% and no clean-up
//    such as trim() is necessary.
$query->execute($_POST);

if ($query->execute()) {
  echo json_encode(array("statusCode"=>200));
} else {
  echo json_encode(array("statusCode"=>500));
}
?>

Note: It's worth noting that code like this does not need to exist, that any decent ORM will make this trivial to do. It's worth exploring what options you have there as this could be so much easier.

tadman
  • 208,517
  • 23
  • 234
  • 262