diff --git a/source/Application/Controller/OrderController.php b/source/Application/Controller/OrderController.php index 7c463bedef..2ac9033c34 100644 --- a/source/Application/Controller/OrderController.php +++ b/source/Application/Controller/OrderController.php @@ -205,15 +205,14 @@ public function execute() if (!$user) { return 'user'; } - $basket = $session->getBasket(); + $requestBasketSummaryHash = Registry::getRequest()->getRequestParameter('basketSummaryHash'); - if ($requestBasketSummaryHash === null) { - ContainerFacade::get(LoggerInterface::class)->warning('Pricing and payments verification can not' - . ' be performed, the basketSummaryHash parameter was not sent with request data.'); + if (!$requestBasketSummaryHash) { + $this->notifyIfBasketSummaryValidationIsNotPossible(); } elseif ($requestBasketSummaryHash !== $this->getBasketSummaryHash()) { $redirect = $basket->getProductsCount() === 0 ? 'basket' : 'order'; - Registry::getUtilsView()->addErrorToDisplay('BASKET_ITEMS_CHANGED_ERROR', false, true, '', $redirect); + $this->addBasketSummaryValidationError($redirect); return $redirect; } @@ -496,11 +495,6 @@ 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 @@ -590,4 +584,30 @@ protected function getUtilsObjectInstance() { return Registry::getUtilsObject(); } + + private function getBasketSummaryHash(): string + { + return md5(json_encode($this->getBasket()->getBasketSummary())); + } + + private function notifyIfBasketSummaryValidationIsNotPossible(): void + { + ContainerFacade::get(LoggerInterface::class) + ->warning( + 'Pricing and payments verification can not be performed, ' . + 'the basketSummaryHash parameter was not sent with request data.' + ); + } + + private function addBasketSummaryValidationError(string $controller): void + { + Registry::getUtilsView() + ->addErrorToDisplay( + 'BASKET_ITEMS_CHANGED_ERROR', + false, + true, + '', + $controller + ); + } } diff --git a/tests/Codeception/Acceptance/CheckoutProcessCest.php b/tests/Codeception/Acceptance/CheckoutProcessCest.php index 1ff68c1cca..1da42bae2a 100644 --- a/tests/Codeception/Acceptance/CheckoutProcessCest.php +++ b/tests/Codeception/Acceptance/CheckoutProcessCest.php @@ -826,71 +826,77 @@ public function checkOrderStepChangedAddress(AcceptanceTester $I): void $orderCheckout->submitOrderSuccessfully(); } - public function testMultipleTabsBasketConsistencyAndOrderSubmission(AcceptanceTester $I): void + public function testWarningAboutBasketChanges(AcceptanceTester $I): void { - $I->wantToTest('basket consistency and order submission when products are added from multiple tabs.'); + $I->wantToTest('user sees warning before submitting an order if his cart was modified from another session'); + $user = Fixtures::get('existingUser'); + $product1 = Fixtures::get('product-1000'); + $product2 = Fixtures::get('product-1001'); + $I->amGoingTo('add a product and go through till the last order step'); + $I->openShop() + ->loginUser( + $user['userLoginName'], + $user['userPassword'] + ); $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->amGoingTo('add one more product to existing basket in another session (browser tab)'); $I->openNewTab(); $I->openShop(); - $product2 = $this->getProductData('1001'); - $basket->addProductToBasket($product2['OXID'], 1); + (new Basket($I))->addProductToBasket($product2['OXID'], 1); + + $I->amGoingTo('go back to the initial session (tab)'); $I->closeTab(); + $I->amGoingTo('try to submit an order and make sure I see the warning'); $orderCheckout->submitOrder(); - $I->see($product1['OXTITLE_1']); $I->see($product2['OXTITLE_1']); $I->see(Translator::translate('BASKET_ITEMS_CHANGED_ERROR')); + $I->amGoingTo('confirm that order can be submitted after the warning was shown'); $orderCheckout->submitOrderSuccessfully(); } - public function testOrderSubmissionWithEmptyBasketAfterUpdateInAnotherTab(AcceptanceTester $I): void + public function testWarningAboutBasketChangesWithEmptyBasket(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); + $I->wantToTest('card-modified-warning-message behaviour when cart was emptied from another session'); + $user = Fixtures::get('existingUser'); + $product = Fixtures::get('product-1000'); + $I->amGoingTo('add a product and go through till the last order step'); + $I->openShop() + ->loginUser( + $user['userLoginName'], + $user['userPassword'] + ); + $basket = new Basket($I); + $basket->addProductToBasket($product['OXID'], 1); $orderCheckout = $basket->openMiniBasket()->openCheckout()->goToNextStep(); + $I->amGoingTo('remove that product from basket in another session (browser tab)'); $I->openNewTab(); $I->openShop(); - $basket->openMiniBasket()->openBasketDisplay()->updateProductAmount(0); - $I->closeTab(); + (new Basket($I))->openMiniBasket() + ->openBasketDisplay() + ->updateProductAmount(0); - $I->see($product1['OXTITLE_1']); + $I->amGoingTo('go back to the initial session (tab)'); + $I->closeTab(); + $I->amGoingTo('try to submit an order and make sure I see the warning'); $orderCheckout->submitOrder(); - $I->see(Translator::translate('BASKET_ITEMS_CHANGED_ERROR')); $I->see(Translator::translate('BASKET_EMPTY')); + $I->amGoingTo('confirm that warning is gone after page reload'); $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'); diff --git a/tests/Integration/Application/Controller/OrderControllerTest.php b/tests/Integration/Application/Controller/OrderControllerTest.php new file mode 100644 index 0000000000..4a9c380ac1 --- /dev/null +++ b/tests/Integration/Application/Controller/OrderControllerTest.php @@ -0,0 +1,136 @@ +prepareUserStub(); + $this->prepareBasketMock(); + $this->stubSession(); + unset($_SESSION['Errors']); + } + + public function testExecuteWithBasketMissingSummaryHashParameterWillLogAnError(): void + { + $logger = $this->createMock(LoggerInterface::class); + $this->injectLoggerMockIntoContainer($logger); + + $logger->expects($this->once()) + ->method('warning') + ->with($this->stringContains($this->basketSummaryHashParameter)); + + oxNew(OrderController::class)->execute(); + } + + public function testExecuteWithWrongBasketSummaryHashParameterAndEmptyBasketWillRedirectAndAddError(): void + { + $_GET[$this->basketSummaryHashParameter] = 'some-invalid-hash'; + $this->basket->method('getProductsCount')->willReturn(0); + + $redirect = oxNew(OrderController::class)->execute(); + + $this->assertEquals('basket', $redirect); + $this->assertNotEmpty($_SESSION['Errors']); + } + + public function testExecuteWithWrongBasketSummaryHashParameterAndNonEmptyBasketWillRedirectAndAddError(): void + { + $_GET[$this->basketSummaryHashParameter] = 'some-invalid-hash'; + $this->basket->method('getProductsCount')->willReturn(123); + + $redirect = oxNew(OrderController::class)->execute(); + + $this->assertEquals('order', $redirect); + $this->assertNotEmpty($_SESSION['Errors']); + } + + private function prepareUserStub(): void + { + Registry::getConfig()->setConfigParam('blEnableIntangibleProdAgreement', false); + $user = oxNew('oxUser'); + $user->oxuser__oxusername = new Field('some-user-name', Field::T_RAW); + $user->oxuser__oxpassword = new Field('some-user-pass', Field::T_RAW); + $user->save(); + + $this->userId = $user->getId(); + } + + private function stubSession(): void + { + $session = $this->createPartialMock( + Session::class, + [ + 'checkSessionChallenge', + 'getVariable', + 'getBasket', + ] + ); + $session->method('checkSessionChallenge') + ->willReturn(true); + $session->method('getBasket') + ->willReturn($this->basket); + $session->method('getVariable') + ->willReturnMap( + [ + ['login-token', null], + ['usr', $this->userId], + ] + ); + Registry::set(Session::class, $session); + } + + private function prepareBasketMock(): void + { + $this->basket = $this->createPartialMock( + Basket::class, + ['getProductsCount'] + ); + } + + private function injectLoggerMockIntoContainer(LoggerInterface $logger): void + { + $this->createContainer(); + $this->useNonVfsProjectConfigurationDirectory(); + $this->container->set(LoggerInterface::class, $logger); + $this->container->autowire(LoggerInterface::class, LoggerInterface::class); + $this->attachContainerToContainerFactory(); + } + + private function useNonVfsProjectConfigurationDirectory(): void + { + $this->container->get(ContextInterface::class) + ->setProjectConfigurationDirectory( + ContainerFacade::get(ContextInterface::class) + ->getProjectConfigurationDirectory() + ); + } +}