OXDEV-8393 Alert user of basket changes before confirming order

OXID-eSales · Dec 10, 2024 · 5d92cf0 · 5d92cf0
1 parent 6d32d3f
commit 5d92cf0
Show file tree

Hide file tree

Showing 5 changed files with 125 additions and 40 deletions.
diff --git a/CHANGELOG-7.3.md b/CHANGELOG-7.3.md
@@ -4,10 +4,10 @@
 
 ### Added
 - PHPUnit v11 support
-- Category detail page codeception test
 
 ### Fixed
 - Shop ID resolution considers SSL language URLs
+- Add to basket does not force a refresh of order confirmation step [#0007254](https://bugs.oxid-esales.com/view.php?id=7254)
 
 ### Removed
 - PHPUnit v10 support
diff --git a/source/Application/Controller/OrderController.php b/source/Application/Controller/OrderController.php
@@ -7,9 +7,12 @@
 
 namespace OxidEsales\EshopCommunity\Application\Controller;
 
+use OxidEsales\Eshop\Application\Model\Order;
 use OxidEsales\Eshop\Core\Registry;
 use OxidEsales\Eshop\Core\UtilsObject;
 use OxidEsales\Eshop\Application\Model\BasketContentMarkGenerator;
+use OxidEsales\EshopCommunity\Core\Di\ContainerFacade;
+use Psr\Log\LoggerInterface;
 
 /**
  * Order manager. Arranges user ordering data, checks/validates
@@ -114,7 +117,7 @@ class OrderController extends \OxidEsales\Eshop\Application\Controller\FrontendC
     public function init()
     {
         // disabling performance control variable
-        \OxidEsales\Eshop\Core\Registry::getConfig()->setConfigParam('bl_perfCalcVatOnlyForBasketOrder', false);
+        Registry::getConfig()->setConfigParam('bl_perfCalcVatOnlyForBasketOrder', false);
 
         // recalc basket cause of payment stuff
         if ($oBasket = $this->getBasket()) {
@@ -138,10 +141,10 @@ public function render()
     {
         if ($this->getIsOrderStep()) {
             $oBasket = $this->getBasket();
-            $myConfig = \OxidEsales\Eshop\Core\Registry::getConfig();
+            $myConfig = Registry::getConfig();
+            $session = Registry::getSession();
 
             if ($myConfig->getConfigParam('blPsBasketReservationEnabled')) {
-                $session = \OxidEsales\Eshop\Core\Registry::getSession();
                 $session->getBasketReservations()->renewExpiration();
                 if (!$oBasket || ($oBasket && !$oBasket->getProductsCount())) {
                     Registry::getUtils()->redirect($myConfig->getShopHomeUrl() . 'cl=basket', true, 302);
@@ -163,11 +166,12 @@ public function render()
             }
         }
 
+        $this->_aViewData['basketHash'] = $this->getBasketSummaryHash();
         parent::render();
 
         // reload blocker
-        if (!Registry::getSession()->getVariable('sess_challenge')) {
-            Registry::getSession()->setVariable('sess_challenge', $this->getUtilsObjectInstance()->generateUID());
+        if (!$session->getVariable('sess_challenge')) {
+            $session->setVariable('sess_challenge', $this->getUtilsObjectInstance()->generateUID());
         }
 
         return $this->_sThisTemplate;
@@ -186,44 +190,53 @@ public function render()
      */
     public function execute()
     {
-        $session = \OxidEsales\Eshop\Core\Registry::getSession();
+        $session = Registry::getSession();
         if (!$session->checkSessionChallenge()) {
-            return;
+            return null;
         }
 
         if (!$this->validateTermsAndConditions()) {
             $this->_blConfirmAGBError = 1;
 
-            return;
+            return null;
         }
 
-        // additional check if we really really have a user now
-        $oUser = $this->getUser();
-        if (!$oUser) {
+        $user = $this->getUser();
+        if (!$user) {
             return 'user';
         }
 
-        // get basket contents
-        $oBasket = $session->getBasket();
-        if ($oBasket->getProductsCount()) {
+        $basket = $session->getBasket();
+        $requestBasketHash = Registry::getRequest()->getRequestParameter('basketHash');
+        if ($requestBasketHash === null) {
+            ContainerFacade::get(LoggerInterface::class)->warning('Pricing and payments verification can not'
+                . ' be performed, the basketSummaryHash parameter was not sent with request data.');
+        } elseif ($requestBasketHash !== $this->getBasketSummaryHash()) {
+            $redirect = $basket->getProductsCount() === 0 ? 'basket' : 'order';
+            Registry::getUtilsView()->addErrorToDisplay('BASKET_ITEMS_CHANGED_ERROR', false, true, '', $redirect);
+
+            return $redirect;
+        }
+
+        if ($basket->getProductsCount()) {
             try {
-                $oOrder = oxNew(\OxidEsales\Eshop\Application\Model\Order::class);
+                $order = oxNew(Order::class);
 
                 //finalizing ordering process (validating, storing order into DB, executing payment, setting status ...)
-                $iSuccess = $oOrder->finalizeOrder($oBasket, $oUser);
+                $success = $order->finalizeOrder($basket, $user);
 
                 // performing special actions after user finishes order (assignment to special user groups)
-                $oUser->onOrderExecute($oBasket, $iSuccess);
+                $user->onOrderExecute($basket, $success);
 
                 // proceeding to next view
-                return $this->getNextStep($iSuccess);
-            } catch (\OxidEsales\Eshop\Core\Exception\OutOfStockException $oEx) {
-                $oEx->setDestination('basket');
-                Registry::getUtilsView()->addErrorToDisplay($oEx, false, true, 'basket');
-            } catch (\OxidEsales\Eshop\Core\Exception\NoArticleException $oEx) {
-                Registry::getUtilsView()->addErrorToDisplay($oEx);
-            } catch (\OxidEsales\Eshop\Core\Exception\ArticleInputException $oEx) {
-                Registry::getUtilsView()->addErrorToDisplay($oEx);
+                return $this->getNextStep($success);
+            } catch (\OxidEsales\Eshop\Core\Exception\OutOfStockException $exception) {
+                $exception->setDestination('basket');
+                Registry::getUtilsView()->addErrorToDisplay($exception, false, true, 'basket');
+            } catch (\OxidEsales\Eshop\Core\Exception\NoArticleException $exception) {
+                Registry::getUtilsView()->addErrorToDisplay($exception);
+            } catch (\OxidEsales\Eshop\Core\Exception\ArticleInputException $exception) {
+                Registry::getUtilsView()->addErrorToDisplay($exception);
             }
         }
     }
@@ -248,8 +261,8 @@ public function getPayment()
             if (
                 $sPaymentid && $oPayment->load($sPaymentid) &&
                 $oPayment->isValidPayment(
-                    \OxidEsales\Eshop\Core\Registry::getSession()->getVariable('dynvalue'),
-                    \OxidEsales\Eshop\Core\Registry::getConfig()->getShopId(),
+                    Registry::getSession()->getVariable('dynvalue'),
+                    Registry::getConfig()->getShopId(),
                     $oUser,
                     $oBasket->getPriceForPayment(),
                     Registry::getSession()->getVariable('sShipSet')
@@ -271,7 +284,7 @@ public function getBasket()
     {
         if ($this->_oBasket === null) {
             $this->_oBasket = false;
-            $session = \OxidEsales\Eshop\Core\Registry::getSession();
+            $session = Registry::getSession();
             if ($oBasket = $session->getBasket()) {
                 $this->_oBasket = $oBasket;
             }
@@ -333,7 +346,7 @@ public function getDelAddress()
     {
         if ($this->_oDelAddress === null) {
             $this->_oDelAddress = false;
-            $oOrder = oxNew(\OxidEsales\Eshop\Application\Model\Order::class);
+            $oOrder = oxNew(Order::class);
             $this->_oDelAddress = $oOrder->getDelAddressInfo();
         }
 
@@ -369,7 +382,7 @@ public function isConfirmAGBActive()
     {
         if ($this->_blConfirmAGB === null) {
             $this->_blConfirmAGB = false;
-            $this->_blConfirmAGB = \OxidEsales\Eshop\Core\Registry::getConfig()->getConfigParam('blConfirmAGB');
+            $this->_blConfirmAGB = Registry::getConfig()->getConfigParam('blConfirmAGB');
         }
 
         return $this->_blConfirmAGB;
@@ -394,7 +407,7 @@ public function showOrderButtonOnTop()
     {
         if ($this->_blShowOrderButtonOnTop === null) {
             $this->_blShowOrderButtonOnTop = false;
-            $this->_blShowOrderButtonOnTop = \OxidEsales\Eshop\Core\Registry::getConfig()->getConfigParam('blShowOrderButtonOnTop');
+            $this->_blShowOrderButtonOnTop = Registry::getConfig()->getConfigParam('blShowOrderButtonOnTop');
         }
 
         return $this->_blShowOrderButtonOnTop;
@@ -483,6 +496,11 @@ public function getBasketContentMarkGenerator()
         return oxNew(BasketContentMarkGenerator::class, $this->getBasket());
     }
 
+    private function getBasketSummaryHash(): string
+    {
+        return md5(json_encode($this->getBasket()->getBasketSummary()));
+    }
+
     /**
      * Returns next order step. If ordering was sucessfull - returns string "thankyou" (possible
      * additional parameters), otherwise - returns string "payment" with additional
@@ -498,24 +516,24 @@ protected function getNextStep($iSuccess)
 
         //little trick with switch for multiple cases
         switch (true) {
-            case ($iSuccess === \OxidEsales\Eshop\Application\Model\Order::ORDER_STATE_MAILINGERROR):
+            case ($iSuccess === Order::ORDER_STATE_MAILINGERROR):
                 $sNextStep = 'thankyou?mailerror=1';
                 break;
-            case ($iSuccess === \OxidEsales\Eshop\Application\Model\Order::ORDER_STATE_INVALIDDELADDRESSCHANGED):
+            case ($iSuccess === Order::ORDER_STATE_INVALIDDELADDRESSCHANGED):
                 $sNextStep = 'order?iAddressError=1';
                 break;
-            case ($iSuccess === \OxidEsales\Eshop\Application\Model\Order::ORDER_STATE_BELOWMINPRICE):
+            case ($iSuccess === Order::ORDER_STATE_BELOWMINPRICE):
                 $sNextStep = 'order';
                 break;
-            case ($iSuccess === \OxidEsales\Eshop\Application\Model\Order::ORDER_STATE_VOUCHERERROR):
+            case ($iSuccess === Order::ORDER_STATE_VOUCHERERROR):
                 $sNextStep = 'basket';
                 break;
-            case ($iSuccess === \OxidEsales\Eshop\Application\Model\Order::ORDER_STATE_PAYMENTERROR):
+            case ($iSuccess === Order::ORDER_STATE_PAYMENTERROR):
                 // no authentication, kick back to payment methods
                 Registry::getSession()->setVariable('payerror', 2);
                 $sNextStep = 'payment?payerror=2';
                 break;
-            case ($iSuccess === \OxidEsales\Eshop\Application\Model\Order::ORDER_STATE_ORDEREXISTS):
+            case ($iSuccess === Order::ORDER_STATE_ORDEREXISTS):
                 break; // reload blocker activ
             case (is_numeric($iSuccess) && $iSuccess > 3):
                 Registry::getSession()->setVariable('payerror', $iSuccess);
@@ -542,7 +560,7 @@ protected function getNextStep($iSuccess)
     protected function validateTermsAndConditions()
     {
         $blValid = true;
-        $oConfig = \OxidEsales\Eshop\Core\Registry::getConfig();
+        $oConfig = Registry::getConfig();
 
         if ($oConfig->getConfigParam('blConfirmAGB') && !Registry::getRequest()->getRequestEscapedParameter('ord_agb')) {
             $blValid = false;

diff --git a/source/Application/translations/de/lang.php b/source/Application/translations/de/lang.php
@@ -54,6 +54,7 @@
 'BARGAIN'                                                     => 'Schnäppchen',
 'BARGAIN_PRODUCTS'                                            => 'Die besten Schnäppchen des Shops',
 'BASKET_EMPTY'                                                => 'Der Warenkorb ist leer.',
+'BASKET_ITEMS_CHANGED_ERROR'                                  => 'Die Warenkorbartikel wurden geändert.',
 'BIC'                                                         => 'BIC',
 'BILLING_ADDRESS'                                             => 'Rechnungsadresse',
 'BILLING_SHIPPING_SETTINGS'                                   => 'Rechnungs- und Lieferadressen',

diff --git a/source/Application/translations/en/lang.php b/source/Application/translations/en/lang.php
@@ -54,6 +54,7 @@
 'BARGAIN'                                                     => 'Bargain',
 'BARGAIN_PRODUCTS'                                            => 'Bargain products',
 'BASKET_EMPTY'                                                => 'The shopping cart is empty.',
+'BASKET_ITEMS_CHANGED_ERROR'                                  => 'The shopping cart items have been changed.',
 'BIC'                                                         => 'BIC',
 'BILLING_ADDRESS'                                             => 'Billing address',
 'BILLING_SHIPPING_SETTINGS'                                   => 'Billing and shipping addresses',

diff --git a/tests/Codeception/Acceptance/CheckoutProcessCest.php b/tests/Codeception/Acceptance/CheckoutProcessCest.php
@@ -826,6 +826,72 @@ public function checkOrderStepChangedAddress(AcceptanceTester $I): void
         $orderCheckout->submitOrderSuccessfully();
     }
 
+    public function testMultipleTabsBasketConsistencyAndOrderSubmission(AcceptanceTester $I): void
+    {
+        $I->wantToTest('basket consistency and order submission when products are added from multiple tabs.');
+
+        $basket = new Basket($I);
+        $homePage = $I->openShop();
+        $userData = $this->getExistingUserData();
+        $homePage->loginUser($userData['userLoginName'], $userData['userPassword']);
+
+        $product1 = $this->getProductData('1000');
+        $basket->addProductToBasket($product1['OXID'], 1);
+
+        $orderCheckout = $basket->openMiniBasket()->openCheckout()->goToNextStep();
+
+        $I->openNewTab();
+        $I->openShop();
+        $product2 = $this->getProductData('1001');
+        $basket->addProductToBasket($product2['OXID'], 1);
+        $basket->openMiniBasket()->openCheckout()->goToNextStep();
+        $I->closeTab();
+
+        $orderCheckout->submitOrder();
+
+        $I->see($product1['OXTITLE_1']);
+        $I->see($product2['OXTITLE_1']);
+        $I->see(Translator::translate('BASKET_ITEMS_CHANGED_ERROR'));
+
+        $orderCheckout->submitOrderSuccessfully();
+    }
+
+    public function testOrderSubmissionWithEmptyBasketAfterUpdateInAnotherTab(AcceptanceTester $I): void
+    {
+        $I->wantToTest('order submission with empty basket in other tab');
+        $basket = new Basket($I);
+        $homePage = $I->openShop();
+
+        $userData = $this->getExistingUserData();
+        $homePage->loginUser($userData['userLoginName'], $userData['userPassword']);
+
+        $product1 = $this->getProductData('1000');
+        $basket->addProductToBasket($product1['OXID'], 1);
+
+        $orderCheckout = $basket->openMiniBasket()->openCheckout()->goToNextStep();
+
+        $I->openNewTab();
+        $I->openShop();
+        $basket->openMiniBasket()->openBasketDisplay()->updateProductAmount(0);
+        $I->closeTab();
+
+        $I->see($product1['OXTITLE_1']);
+
+        $orderCheckout->submitOrder();
+
+        $I->see(Translator::translate('BASKET_ITEMS_CHANGED_ERROR'));
+        $I->see(Translator::translate('BASKET_EMPTY'));
+
+        $I->reloadPage();
+
+        $I->dontSee(Translator::translate('BASKET_ITEMS_CHANGED_ERROR'));
+    }
+
+    private function getProductData(string $productID): array
+    {
+        return Fixtures::get('product-' . $productID);
+    }
+
     private function getExistingUserData(): array
     {
         return Fixtures::get('existingUser');
@@ -870,5 +936,4 @@ private function getUserAddressFormData(): array
             'faxNr' => '',
         ];
     }
-
 }