Skip to content

Commit

Permalink
OXDEV-8393 Alert user of basket changes before confirming order
Browse files Browse the repository at this point in the history
  • Loading branch information
@AshrafOxid
AshrafOxid committed Dec 18, 2024
1 parent 5a2023e commit 161a34f
Show file tree
Hide file tree
Showing 6 changed files with 286 additions and 39 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-7.3.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
### Changed
- Raised minimum required version of Symfony components to 6.4
- Set the default value of blSkipDebitOldBankInfo to true
- 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
Expand Down
114 changes: 76 additions & 38 deletions source/Application/Controller/OrderController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()) {
Expand All @@ -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);
Expand All @@ -163,11 +166,12 @@ public function render()
}
}

$this->_aViewData['basketSummaryHash'] = $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;
Expand All @@ -186,44 +190,52 @@ 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';
}
$basket = $session->getBasket();

$requestBasketSummaryHash = Registry::getRequest()->getRequestParameter('basketSummaryHash');
if (!$requestBasketSummaryHash) {
$this->notifyIfBasketSummaryValidationIsNotPossible();
} elseif ($requestBasketSummaryHash !== $this->getBasketSummaryHash()) {
$redirect = $basket->getProductsCount() === 0 ? 'basket' : 'order';
$this->addBasketSummaryValidationError($redirect);

return $redirect;
}

// get basket contents
$oBasket = $session->getBasket();
if ($oBasket->getProductsCount()) {
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);
}
}
}
Expand All @@ -248,8 +260,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')
Expand All @@ -271,7 +283,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;
}
Expand Down Expand Up @@ -333,7 +345,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();
}

Expand Down Expand Up @@ -369,7 +381,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;
Expand All @@ -394,7 +406,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;
Expand Down Expand Up @@ -498,24 +510,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);
Expand All @@ -542,7 +554,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;
Expand Down Expand Up @@ -572,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
);
}
}
1 change: 1 addition & 0 deletions source/Application/translations/de/lang.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions source/Application/translations/en/lang.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
72 changes: 71 additions & 1 deletion tests/Codeception/Acceptance/CheckoutProcessCest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,77 @@ public function checkOrderStepChangedAddress(AcceptanceTester $I): void
$orderCheckout->submitOrderSuccessfully();
}

public function testWarningAboutBasketChanges(AcceptanceTester $I): void
{
$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);
$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();
(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 testWarningAboutBasketChangesWithEmptyBasket(AcceptanceTester $I): void
{
$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();
(new Basket($I))->openMiniBasket()
->openBasketDisplay()
->updateProductAmount(0);

$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 getExistingUserData(): array
{
return Fixtures::get('existingUser');
Expand Down Expand Up @@ -870,5 +941,4 @@ private function getUserAddressFormData(): array
'faxNr' => '',
];
}

}
Loading

0 comments on commit 161a34f

Please sign in to comment.