diff --git a/CHANGELOG-7.3.md b/CHANGELOG-7.3.md index 98fc2f1d7b..dcba3b2b01 100644 --- 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 \ No newline at end of file diff --git a/source/Application/Controller/OrderController.php b/source/Application/Controller/OrderController.php index 6e8f9ee311..46809632b2 100644 --- 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 index b8dd03ffad..26ff3c9555 100644 --- 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 index d7996d1db0..9baeef59bc 100644 --- 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 index cf781807eb..c7b65391b3 100644 --- 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' => '', ]; } - }