-2

How can I make the database entry code work? Right now nothing shows when I run this code.

 public function hookActionValidateOrder()
    {       
         $products=[]; 
         $ids_product_attribute=[];
         $references=[];
         $stocks= [];

           for($i=0;Context::getContext()->cart->getProducts()[$i]['id_product'];$i++)
           {
                array_push($products,Context::getContext()->cart->getProducts()[$i]['id_product']);
                array_push($ids_product_attribute,Context::getContext()->cart->getProducts()[$i]['id_product_attribute']);
                array_push($references,Context::getContext()->cart->getProducts()[$i]['reference']);
                array_push($stocks,Context::getContext()->cart->getProducts()[$i]['stock_quantity']);

                die(Db::getInstance()->execute("
                INSERT INTO cart_log (products, ids_product_attribute, references, stocks, time)


                VALUES ($products[$i], $ids_product_attribute[$i], $references[$i], $stocks[$i])"));

           }
        //    var_dump($products);
        //    var_dump($ids_product_attribute);
        //    var_dump($references);
        //    var_dump($products);


    }
  • 3
    You don't check for errors. How do you expect to find error if you don't check for them? – John Conde Sep 22 '19 at 12:19
  • The `INSERT` expects 5 columns but the `VALUES` only has 4, `time` is not represented. – Michael Berkowski Sep 22 '19 at 12:20
  • @JohnConde: True. Need have a `try/catch` –  Sep 22 '19 at 12:20
  • 2
    Anytime you get blank output from PHP when you otherwise expect output, you need to view your web server's error log to find fatal error details. Always when developing and testing code, use `error_reporting(E_ALL); ini_set('display_errors', 1);` at the top of your code (or set in php.ini). In production, turn off display_errors. – Michael Berkowski Sep 22 '19 at 12:22
  • 3
    Your code is open to [SQL injection](https://stackoverflow.com/q/332365/2469308) related attacks. Even [`real_escape_string`](https://stackoverflow.com/a/12118602/2469308) cannot secure it completely. Please learn to use [Prepared Statements](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) instead. – Madhur Bhaiya Sep 22 '19 at 12:26
  • I really hope `cart->getProducts()` isn't running another SQL query _every time_ you loop. – Bill Karwin Sep 22 '19 at 12:47

2 Answers2

1

I would write this function this way:

public function hookActionValidateOrder()
{
    $defaults = [
      'id_product' => null,
      'id_product_attribute' => null,
      'reference' => 0,
      'stock_quantity' => 0
    ];
    $stmt = Db::getInstance()->prepare("
        INSERT INTO cart_log
        SET products = :id_product,
            ids_product_attribute = :id_product_attribute,
            references = :reference,
            stocks = :stock_quantity");
    $products = Context::getContext()->cart->getProducts();
    foreach ($products as $product) {
        $values = array_merge($defaults, array_intersect_key($product, $defaults));
        $stmt->execute($values);
    }
}

This example shows usage of query parameters in a prepared statement, so you don't try to interpolate PHP variables directly into your SQL query. Query parameters make it easier to write error-free SQL code, and they protect you from accidental SQL injection errors (also malicious attacks).

I recommend calling your cart->getProducts() once, and save the result in a local variable. I'm not sure what that function does, but I suppose it's running another SQL query. You shouldn't run the same SQL query many times for each loop, that will increase load to your database server.

The business about array_merge(array_intersect_key()) is to make sure the values array has both all of the needed keys, and no other keys besides the needed keys. Then it can be passed as-is to PDOStatement::execute().

I'm using an alternative form for INSERT that is supported by MySQL, using SET column = value ... syntax. I find this makes it easier to make sure I've matched a column to each value and vice-versa.

As mentioned in the comments above, you must enable exceptions in the database connector. I'm assuming your getInstance() function returns a PDO connection, so you can pass the array of parameters to execute(). You need to enable PDO exceptions as described here: https://www.php.net/manual/en/pdo.error-handling.php

If you don't enable exceptions, you should check the return value of prepare() and execute() to see if either is === false, and if so then log the errorInfo() (read the PHP doc I linked to about error handling).

Bill Karwin
  • 499,602
  • 82
  • 638
  • 795
0

I detected two error in your query:

  1. You insert 5 fields, but only 4 values.

  2. In value you don't add '' char if values string

Example: if all fields with data type string

VALUES ('$products[$i]', '$ids_product_attribute[$i]', '$references[$i]', '$stocks[$i]', 'Field 5')
user3783243
  • 5,098
  • 5
  • 16
  • 37
Au Nguyen
  • 663
  • 3
  • 12
  • If the fields are strings, you also have to handle cases where the string value contains a literal `'` character. That would cause an error in the code example you show, if the single-quotes are imbalanced. It's better to use query parameters instead of interpolating PHP variables. Then you don't have to worry about any special characters in the string values. – Bill Karwin Sep 22 '19 at 13:10