Merge pull request #9405 from PierreRambaud/fix/boom-6127

Error when adding product in cart or editing quantity
PrestaShop · Aug 24, 2018 · c2a71fd · c2a71fd
2 parents a4490ec + 372ba58
commit c2a71fd
Show file tree

Hide file tree

Showing 11 changed files with 1,066 additions and 174 deletions.
diff --git a/admin-dev/themes/default/public/bundle.js b/admin-dev/themes/default/public/bundle.js
diff --git a/admin-dev/themes/default/webpack.config.js b/admin-dev/themes/default/webpack.config.js
@@ -22,11 +22,11 @@
  * @license   https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0)
  * International Registered Trademark & Property of PrestaShop SA
  */
-var path = require('path');
-var webpack = require('webpack');
-var ExtractTextPlugin = require('extract-text-webpack-plugin');
+const path = require('path');
+const webpack = require('webpack');
+const ExtractTextPlugin = require('extract-text-webpack-plugin');
 
-module.exports = {
+const config = {
   entry: [
     './js/theme.js'
   ],
@@ -75,6 +75,11 @@ module.exports = {
   },
   plugins: [
     new ExtractTextPlugin('theme.css'),
+  ]
+};
+
+if (process.env.NODE_ENV === 'production') {
+  config.plugins.push(
     new webpack.optimize.UglifyJsPlugin({
       sourceMap: false,
       compress: {
@@ -89,5 +94,9 @@ module.exports = {
         comments: false
       }
     })
-  ]
-};
+  );
+} else {
+  config.plugins.push(new webpack.HotModuleReplacementPlugin());
+}
+
+module.exports = config;
diff --git a/admin-dev/themes/new-theme/webpack.config.js b/admin-dev/themes/new-theme/webpack.config.js
@@ -22,12 +22,13 @@
  * @license   https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0)
  * International Registered Trademark & Property of PrestaShop SA
  */
+
 const path = require('path');
 const webpack = require('webpack');
 const ExtractTextPlugin = require("extract-text-webpack-plugin");
 const keepLicense = require('uglify-save-license');
 
-let config = {
+const config = {
   entry: {
     main: [
       'prestakit/dist/js/prestashop-ui-kit.js',
@@ -227,9 +228,7 @@ if (process.env.NODE_ENV === 'production') {
     })
   );
 } else {
-  config.plugins.push(
-    new webpack.HotModuleReplacementPlugin()
-  );
+  config.plugins.push(new webpack.HotModuleReplacementPlugin());
   config.entry.stock.push('webpack/hot/only-dev-server');
   config.entry.stock.push('webpack-dev-server/client?http://localhost:8080');
 }

diff --git a/classes/Cart.php b/classes/Cart.php
@@ -1250,7 +1250,12 @@ public function getProductQuantity($idProduct, $idProductAttribute = 0, $idCusto
      */
     public function containsProduct($id_product, $id_product_attribute = 0, $id_customization = 0, $id_address_delivery = 0)
     {
-        $result = $this->getProductQuantity($id_product, $id_product_attribute, $id_customization, $id_address_delivery);
+        $result = $this->getProductQuantity(
+            $id_product,
+            $id_product_attribute,
+            $id_customization,
+            $id_address_delivery
+        );
 
         if (empty($result['quantity'])) {
             return false;
@@ -1285,11 +1290,16 @@ public function updateQty(
         }
 
         if (Context::getContext()->customer->id) {
-            if ($id_address_delivery == 0 && (int) $this->id_address_delivery) { // The $id_address_delivery is null, use the cart delivery address
+            if ($id_address_delivery == 0 && (int) $this->id_address_delivery) {
+                // The $id_address_delivery is null, use the cart delivery address
                 $id_address_delivery = $this->id_address_delivery;
-            } elseif ($id_address_delivery == 0) { // The $id_address_delivery is null, get the default customer address
-                $id_address_delivery = (int) Address::getFirstCustomerAddressId((int) Context::getContext()->customer->id);
-            } elseif (!Customer::customerHasAddress(Context::getContext()->customer->id, $id_address_delivery)) { // The $id_address_delivery must be linked with customer
+            } elseif ($id_address_delivery == 0) {
+                // The $id_address_delivery is null, get the default customer address
+                $id_address_delivery = (int) Address::getFirstCustomerAddressId(
+                    (int) Context::getContext()->customer->id
+                );
+            } elseif (!Customer::customerHasAddress(Context::getContext()->customer->id, $id_address_delivery)) {
+                // The $id_address_delivery must be linked with customer
                 $id_address_delivery = 0;
             }
         }
@@ -1343,85 +1353,102 @@ public function updateQty(
 
         if ((int) $quantity <= 0) {
             return $this->deleteProduct($id_product, $id_product_attribute, (int) $id_customization);
-        } elseif (!$product->available_for_order
-                || (Configuration::isCatalogMode() && !defined('_PS_ADMIN_DIR_'))
+        }
+
+        if (!$product->available_for_order
+            || (
+                Configuration::isCatalogMode()
+                && !defined('_PS_ADMIN_DIR_')
+            )
         ) {
             return false;
-        } else {
-            /* Check if the product is already in the cart */
-            $cartProductQuantity = $this->getProductQuantity($id_product, $id_product_attribute, (int) $id_customization, (int) $id_address_delivery);
+        }
 
-            /* Update quantity if product already exist */
-            if (!empty($cartProductQuantity['quantity'])) {
-                $productQuantity = Product::getQuantity($id_product, $id_product_attribute, null, $this);
-                $availableOutOfStock = Product::isAvailableWhenOutOfStock($product->out_of_stock);
+        /* Check if the product is already in the cart */
+        $cartProductQuantity = $this->getProductQuantity(
+            $id_product,
+            $id_product_attribute,
+            (int) $id_customization,
+            (int) $id_address_delivery
+        );
 
-                if ($operator == 'up') {
-                    $updateQuantity = '+ ' . $quantity;
-                    $newProductQuantity = $productQuantity - $quantity;
+        /* Update quantity if product already exist */
+        if (!empty($cartProductQuantity['quantity'])) {
+            $productQuantity = Product::getQuantity($id_product, $id_product_attribute, null, $this);
+            $availableOutOfStock = Product::isAvailableWhenOutOfStock($product->out_of_stock);
 
-                    if ($newProductQuantity < 0 && !$availableOutOfStock && !$skipAvailabilityCheckOutOfStock) {
-                        return false;
-                    }
-                } elseif ($operator == 'down') {
-                    $cartFirstLevelProductQuantity = $this->getProductQuantity((int) $id_product, (int) $id_product_attribute, $id_customization);
-                    $updateQuantity = '- ' . $quantity;
-                    $newProductQuantity = $productQuantity + $quantity;
+            if ($operator == 'up') {
+                $updateQuantity = '+ ' . $quantity;
+                $newProductQuantity = $productQuantity - $quantity;
 
-                    if ($cartFirstLevelProductQuantity['quantity'] <= 1) {
-                        return $this->deleteProduct((int) $id_product, (int) $id_product_attribute, (int) $id_customization);
-                    }
-                } else {
+                if ($newProductQuantity < 0 && !$availableOutOfStock && !$skipAvailabilityCheckOutOfStock) {
                     return false;
                 }
-                Db::getInstance()->execute(
-                    'UPDATE `' . _DB_PREFIX_ . 'cart_product`
+            } elseif ($operator == 'down') {
+                $cartFirstLevelProductQuantity = $this->getProductQuantity(
+                    (int) $id_product,
+                    (int) $id_product_attribute,
+                    $id_customization
+                );
+                $updateQuantity = '- ' . $quantity;
+                $newProductQuantity = $productQuantity + $quantity;
+
+                if ($cartFirstLevelProductQuantity['quantity'] <= 1
+                    || $cartProductQuantity['quantity'] - $quantity <= 0
+                ) {
+                    return $this->deleteProduct((int) $id_product, (int) $id_product_attribute, (int) $id_customization);
+                }
+            } else {
+                return false;
+            }
+
+            Db::getInstance()->execute(
+                'UPDATE `' . _DB_PREFIX_ . 'cart_product`
                     SET `quantity` = `quantity` ' . $updateQuantity . '
                     WHERE `id_product` = ' . (int) $id_product .
-                    ' AND `id_customization` = ' . (int) $id_customization .
-                    (!empty($id_product_attribute) ? ' AND `id_product_attribute` = ' . (int) $id_product_attribute : '') . '
+                ' AND `id_customization` = ' . (int) $id_customization .
+                (!empty($id_product_attribute) ? ' AND `id_product_attribute` = ' . (int) $id_product_attribute : '') . '
                     AND `id_cart` = ' . (int) $this->id . (Configuration::get('PS_ALLOW_MULTISHIPPING') && $this->isMultiAddressDelivery() ? ' AND `id_address_delivery` = ' . (int) $id_address_delivery : '') . '
                     LIMIT 1'
-                );
-            } elseif ($operator == 'up') {
-                /* Add product to the cart */
+            );
+        } elseif ($operator == 'up') {
+            /* Add product to the cart */
 
-                $sql = 'SELECT stock.out_of_stock, IFNULL(stock.quantity, 0) as quantity
+            $sql = 'SELECT stock.out_of_stock, IFNULL(stock.quantity, 0) as quantity
                         FROM ' . _DB_PREFIX_ . 'product p
                         ' . Product::sqlStock('p', $id_product_attribute, true, $shop) . '
                         WHERE p.id_product = ' . $id_product;
 
-                $result2 = Db::getInstance()->getRow($sql);
+            $result2 = Db::getInstance()->getRow($sql);
 
-                // Quantity for product pack
-                if (Pack::isPack($id_product)) {
-                    $result2['quantity'] = Pack::getQuantity($id_product, $id_product_attribute, null, $this);
-                }
+            // Quantity for product pack
+            if (Pack::isPack($id_product)) {
+                $result2['quantity'] = Pack::getQuantity($id_product, $id_product_attribute, null, $this);
+            }
 
-                if (!Product::isAvailableWhenOutOfStock((int) $result2['out_of_stock']) && !$skipAvailabilityCheckOutOfStock) {
-                    if ((int) $quantity > $result2['quantity']) {
-                        return false;
-                    }
+            if (!Product::isAvailableWhenOutOfStock((int) $result2['out_of_stock']) && !$skipAvailabilityCheckOutOfStock) {
+                if ((int) $quantity > $result2['quantity']) {
+                    return false;
                 }
+            }
 
-                if ((int) $quantity < $minimal_quantity) {
-                    return -1;
-                }
+            if ((int) $quantity < $minimal_quantity) {
+                return -1;
+            }
 
-                $result_add = Db::getInstance()->insert('cart_product', array(
-                    'id_product' => (int) $id_product,
-                    'id_product_attribute' => (int) $id_product_attribute,
-                    'id_cart' => (int) $this->id,
-                    'id_address_delivery' => (int) $id_address_delivery,
-                    'id_shop' => $shop->id,
-                    'quantity' => (int) $quantity,
-                    'date_add' => date('Y-m-d H:i:s'),
-                    'id_customization' => (int) $id_customization,
-                ));
-
-                if (!$result_add) {
-                    return false;
-                }
+            $result_add = Db::getInstance()->insert('cart_product', array(
+                'id_product' => (int) $id_product,
+                'id_product_attribute' => (int) $id_product_attribute,
+                'id_cart' => (int) $this->id,
+                'id_address_delivery' => (int) $id_address_delivery,
+                'id_shop' => $shop->id,
+                'quantity' => (int) $quantity,
+                'date_add' => date('Y-m-d H:i:s'),
+                'id_customization' => (int) $id_customization,
+            ));
+
+            if (!$result_add) {
+                return false;
             }
         }
 
@@ -1436,10 +1463,17 @@ public function updateQty(
         }
 
         if ($product->customizable) {
-            return $this->_updateCustomizationQuantity((int) $quantity, (int) $id_customization, (int) $id_product, (int) $id_product_attribute, (int) $id_address_delivery, $operator);
-        } else {
-            return true;
+            return $this->_updateCustomizationQuantity(
+                (int) $quantity,
+                (int) $id_customization,
+                (int) $id_product,
+                (int) $id_product_attribute,
+                (int) $id_address_delivery,
+                $operator
+            );
         }
+
+        return true;
     }
 
     /**

diff --git a/tests/Unit/Core/Cart/Adding/Product/AddStandardProductTest.php b/tests/Unit/Core/Cart/Adding/Product/AddStandardProductTest.php
@@ -80,4 +80,75 @@ public function testProductCanBeAddedInCartIfMoreThanStockButAvailableWhenOutOfS
         Configuration::set('PS_ORDER_OUT_OF_STOCK', $oldOrderOutOfStock);
     }
 
+    /**
+     * @dataProvider updateQuantitiesProvider
+     */
+    public function testNumberOfProductsInCartIsReportedCorrectlyWhenUpdatingTheirQuantityOnce(
+        $quantity,
+        $operator,
+        $expected,
+        $quantityExpected
+    ) {
+        $product = $this->getProductFromFixtureId(1);
+        $result = $this->cart->updateQty(
+            $quantity,
+            $product->id,
+            $id_product_attribute = null,
+            $id_customization = false,
+            $operator
+        );
+        $cartProductQuantity = $this->cart->getProductQuantity(
+            $product->id,
+            $id_product_attribute,
+            (int) $id_customization,
+            $id_address_delivery = 0
+        );
+
+        $this->assertEquals($expected, $result);
+        $this->assertEquals($quantityExpected, $cartProductQuantity['quantity']);
+    }
+
+    public function updateQuantitiesProvider()
+    {
+        return [
+            [1, 'up', true, 1],
+            [2, 'up', true, 2],
+            [2, 'down', true, 0],
+            [0, 'down', true, 0],
+        ];
+    }
+
+    /**
+     * @dataProvider multipleUpdateQuantitiesProvider
+     */
+    public function testNumberOfProductsInCartIsReportedCorrectlyWhenUpdatingTheirQuantityTwice($first, $second)
+    {
+        list($quantity, $operator, $expected, $quantityExpected) = $first;
+        $this->testNumberOfProductsInCartIsReportedCorrectlyWhenUpdatingTheirQuantityOnce(
+            $quantity,
+            $operator,
+            $expected,
+            $quantityExpected
+        );
+
+        list($quantity, $operator, $expected, $quantityExpected) = $second;
+        $this->testNumberOfProductsInCartIsReportedCorrectlyWhenUpdatingTheirQuantityOnce(
+            $quantity,
+            $operator,
+            $expected,
+            $quantityExpected
+        );
+    }
+
+    public function multipleUpdateQuantitiesProvider()
+    {
+        return [
+            [[1, 'up', true, 1], [1, 'up', true, 2]],
+            [[2, 'up', true, 2], [2, 'down', true, 0]],
+            [[2, 'down', true, 0], [2, 'up', true, 2]],
+            [[0, 'down', true, 0], [1, 'nothing', true, 0]],
+            [[1, 'down', true, 0], [1, 'nothing', true, 0]],
+            [[1, 'up', true, 1], [10, 'nothing', false, 1]],
+        ];
+    }
 }
diff --git a/tests/phpunit.xml b/tests/phpunit.xml
@@ -1,4 +1,5 @@
-<phpunit bootstrap="bootstrap.php">
+<phpunit bootstrap="bootstrap.php"
+         color="true">
     <php>
         <server name="KERNEL_CLASS" value="AppKernel" />
         <env name="SYMFONY_ENV" value="test"/>
@@ -23,4 +24,12 @@
             <directory>Integration</directory>
         </testsuite>
     </testsuites>
+
+    <filter>
+        <whitelist>
+            <directory suffix=".php">../src</directory>
+            <directory suffix=".php">../classes</directory>
+            <directory suffix=".php">../controllers</directory>
+        </whitelist>
+    </filter>
 </phpunit>