From 5ff7854665f445b95adf21ea683725fe34ec8588 Mon Sep 17 00:00:00 2001 From: Volodymyr Klymenko Date: Thu, 3 Jun 2021 18:17:18 +0300 Subject: [PATCH] VIPPS-385: Duplicate and missing order issue --- Controller/Payment/Fallback.php | 72 +++++++++++++++++++++++---------- Model/Quote/CancelFacade.php | 16 ++++++-- Model/QuoteRepository.php | 10 +++-- Model/ResourceModel/Quote.php | 12 ++++-- Model/TransactionProcessor.php | 33 ++++++++------- composer.json | 2 +- 6 files changed, 98 insertions(+), 47 deletions(-) diff --git a/Controller/Payment/Fallback.php b/Controller/Payment/Fallback.php index f75e49f..b6352fa 100755 --- a/Controller/Payment/Fallback.php +++ b/Controller/Payment/Fallback.php @@ -20,12 +20,12 @@ use Magento\Framework\App\Action\Action; use Magento\Framework\App\Action\Context; use Magento\Framework\App\CsrfAwareActionInterface; -use Magento\Framework\App\Request\InvalidRequestException; use Magento\Framework\App\RequestInterface; +use Magento\Framework\App\Request\InvalidRequestException; use Magento\Framework\App\ResponseInterface; -use Magento\Framework\Controller\Result\Redirect; use Magento\Framework\Controller\ResultFactory; use Magento\Framework\Controller\ResultInterface; +use Magento\Framework\Controller\Result\Redirect; use Magento\Framework\Exception\CouldNotSaveException; use Magento\Framework\Exception\LocalizedException; use Magento\Framework\Exception\NoSuchEntityException; @@ -33,6 +33,7 @@ use Magento\Payment\Gateway\ConfigInterface; use Magento\Quote\Api\CartRepositoryInterface; use Magento\Quote\Model\Quote; +use Magento\Sales\Api\Data\OrderInterface; use Magento\Sales\Api\OrderManagementInterface; use Psr\Log\LoggerInterface; use Vipps\Payment\Api\Data\QuoteInterface; @@ -40,6 +41,7 @@ use Vipps\Payment\Gateway\Command\PaymentDetailsProvider; use Vipps\Payment\Gateway\Transaction\Transaction; use Vipps\Payment\Model\Gdpr\Compliance; +use Vipps\Payment\Model\OrderLocator; use Vipps\Payment\Model\TransactionProcessor; /** * Class Fallback @@ -98,6 +100,16 @@ class Fallback extends Action implements CsrfAwareActionInterface */ private $config; + /** + * @var OrderLocator + */ + private $orderLocator; + + /** + * @var OrderInterface|null + */ + private $order; + /** * Fallback constructor. * @@ -108,6 +120,7 @@ class Fallback extends Action implements CsrfAwareActionInterface * @param CartRepositoryInterface $cartRepository * @param QuoteRepositoryInterface $vippsQuoteRepository * @param OrderManagementInterface $orderManagement + * @param OrderLocator $orderLocator * @param Compliance $compliance * @param LoggerInterface $logger * @param ConfigInterface $config @@ -122,6 +135,7 @@ public function __construct( CartRepositoryInterface $cartRepository, QuoteRepositoryInterface $vippsQuoteRepository, OrderManagementInterface $orderManagement, + OrderLocator $orderLocator, Compliance $compliance, LoggerInterface $logger, ConfigInterface $config @@ -134,6 +148,7 @@ public function __construct( $this->vippsQuoteRepository = $vippsQuoteRepository; $this->gdprCompliance = $compliance; $this->orderManagement = $orderManagement; + $this->orderLocator = $orderLocator; $this->logger = $logger; $this->config = $config; } @@ -166,16 +181,18 @@ public function execute() } finally { $vippsQuote = $this->getVippsQuote(true); $cartPersistence = $this->config->getValue('cancellation_cart_persistence'); - $transactionWasCancelled = $transaction && $transaction->transactionWasCancelled(); + $quoteCouldBeRestored = $transaction + && ($transaction->transactionWasCancelled() || $transaction->isTransactionExpired()); + $order = $this->getOrder(); - if ($transactionWasCancelled && $cartPersistence) { + if ($quoteCouldBeRestored && $cartPersistence) { $this->restoreQuote($vippsQuote); - } elseif ($vippsQuote->getOrderId()) { - $this->storeLastOrder($vippsQuote); + } elseif ($order) { + $this->storeLastOrder($order); } if (isset($e)) { - if (!$cartPersistence && $this->getVippsQuote()->getOrderId()) { + if (!$cartPersistence && $order) { $resultRedirect->setPath('checkout/onepage/failure', ['_secure' => true]); } else { $resultRedirect->setPath('checkout/cart', ['_secure' => true]); @@ -224,22 +241,17 @@ private function getVippsQuote($forceReload = false) } /** - * Method to update Checkout session with Last Placed Order - * Or restore quote if order was not placed (ex. Express Checkout) + * Method store order info to checkout session */ - private function storeLastOrder(QuoteInterface $vippsQuote) + private function storeLastOrder(OrderInterface $order) { - if ($vippsQuote->getOrderId()) { - $this->checkoutSession - ->clearStorage() - ->setLastQuoteId($vippsQuote->getQuoteId()) - ->setLastSuccessQuoteId($vippsQuote->getQuoteId()) - ->setLastOrderId($vippsQuote->getOrderId()) - ->setLastRealOrderId($vippsQuote->getReservedOrderId()) - ->setLastOrderStatus( - $this->orderManagement->getStatus($vippsQuote->getOrderId()) - ); - } + $this->checkoutSession + ->clearStorage() + ->setLastQuoteId($order->getQuoteId()) + ->setLastSuccessQuoteId($order->getQuoteId()) + ->setLastOrderId($order->getEntityId()) + ->setLastRealOrderId($order->getIncrementId()) + ->setLastOrderStatus($order->getStatus()); } /** @@ -348,7 +360,7 @@ private function defineRedirectPath(Redirect $resultRedirect, Transaction $trans if ($transaction->isTransactionReserved() || $transaction->isTransactionCaptured()) { $resultRedirect->setPath('checkout/onepage/success', ['_secure' => true]); } else { - $orderId = $this->getVippsQuote() ? $this->getVippsQuote()->getOrderId() : null; + $orderId = $this->getOrder() ? $this->getOrder()->getEntityId() : null; if (!$this->isCartPersistent() && $orderId) { $resultRedirect->setPath('checkout/onepage/failure', ['_secure' => true]); } else { @@ -356,4 +368,20 @@ private function defineRedirectPath(Redirect $resultRedirect, Transaction $trans } } } + + /** + * @param false $forceReload + * + * @return OrderInterface|null + * @throws NoSuchEntityException + */ + private function getOrder($forceReload = false) + { + if (null === $this->order || $forceReload) { + $vippsQuote = $this->getVippsQuote($forceReload); + $this->order = $this->orderLocator->get($vippsQuote->getReservedOrderId()); + } + + return $this->order; + } } diff --git a/Model/Quote/CancelFacade.php b/Model/Quote/CancelFacade.php index f82653c..a0d5350 100644 --- a/Model/Quote/CancelFacade.php +++ b/Model/Quote/CancelFacade.php @@ -26,6 +26,7 @@ use Vipps\Payment\Api\Data\QuoteInterface; use Vipps\Payment\Api\Data\QuoteStatusInterface; use Vipps\Payment\Api\Quote\CancelFacadeInterface; +use Vipps\Payment\Model\OrderLocator; use Vipps\Payment\Model\Quote; use Vipps\Payment\Model\QuoteRepository; @@ -70,6 +71,11 @@ class CancelFacade implements CancelFacadeInterface */ private $logger; + /** + * @var OrderLocator + */ + private $orderLocator; + /** * CancelFacade constructor. * @@ -79,6 +85,8 @@ class CancelFacade implements CancelFacadeInterface * @param AttemptManagement $attemptManagement * @param CartRepositoryInterface $cartRepository * @param OrderRepositoryInterface $orderRepository + * @param OrderLocator $orderLocator + * @param LoggerInterface $logger */ public function __construct( CommandManagerInterface $commandManager, @@ -87,6 +95,7 @@ public function __construct( AttemptManagement $attemptManagement, CartRepositoryInterface $cartRepository, OrderRepositoryInterface $orderRepository, + OrderLocator $orderLocator, LoggerInterface $logger ) { $this->commandManager = $commandManager; @@ -95,6 +104,7 @@ public function __construct( $this->attemptManagement = $attemptManagement; $this->cartRepository = $cartRepository; $this->orderRepository = $orderRepository; + $this->orderLocator = $orderLocator; $this->logger = $logger; } @@ -106,10 +116,10 @@ public function __construct( public function cancel(QuoteInterface $vippsQuote) { try { - if ($vippsQuote->getOrderId()) { + $order = $this->orderLocator->get($vippsQuote->getReservedOrderId()); + if ($order) { /** @var Order $order */ - $order = $this->orderRepository->get($vippsQuote->getOrderId()); - $this->orderManagement->cancel($vippsQuote->getOrderId()); + $this->orderManagement->cancel($order->getEntityId()); $this->commandManager->cancel($order->getPayment()); } else { /** @var \Magento\Quote\Model\Quote $quote */ diff --git a/Model/QuoteRepository.php b/Model/QuoteRepository.php index 1efcdbf..361af30 100644 --- a/Model/QuoteRepository.php +++ b/Model/QuoteRepository.php @@ -128,17 +128,19 @@ public function loadByQuote($quoteId): QuoteInterface * Load monitoring quote by quote. * * @param $quoteId + * @param string $status * - * @return Quote + * @return QuoteInterface * @throws NoSuchEntityException + * @throws \Magento\Framework\Exception\LocalizedException */ - public function loadNewByQuote($quoteId): QuoteInterface + public function loadNewByQuote($quoteId, $status = QuoteInterface::STATUS_NEW): QuoteInterface { $vippsQuote = $this->quoteFactory->create(); - $this->quoteResource->loadNewByQuote($vippsQuote, $quoteId, 'quote_id'); + $this->quoteResource->loadNewByQuote($vippsQuote, $quoteId, 'quote_id', $status); if (!$vippsQuote->getId()) { - throw NoSuchEntityException::singleField('quote_id', $quoteId); + throw NoSuchEntityException::doubleField('quote_id', $quoteId, 'status', $status); } return $vippsQuote; diff --git a/Model/ResourceModel/Quote.php b/Model/ResourceModel/Quote.php index 5568915..7985b18 100644 --- a/Model/ResourceModel/Quote.php +++ b/Model/ResourceModel/Quote.php @@ -50,12 +50,17 @@ protected function _construct() //@codingStandardsIgnoreLine * @param AbstractModel $object * @param $value * @param null $field + * @param string $status * * @return $this * @throws LocalizedException */ - public function loadNewByQuote(AbstractModel $object, $value, $field = null) - { + public function loadNewByQuote( + AbstractModel $object, + $value, + $field = null, + $status = QuoteInterface::STATUS_NEW + ) { $object->beforeLoad($value, $field); if ($field === null) { $field = $this->getIdFieldName(); @@ -67,7 +72,8 @@ public function loadNewByQuote(AbstractModel $object, $value, $field = null) $select = $connection->select() ->from($this->getMainTable()) ->where("$field = ?", $value) - ->where('status = ?', QuoteInterface::STATUS_NEW) + ->where('status = ?', $status) + ->order($this->getIdFieldName() . ' DESC') ->limit(1); $data = $connection->fetchRow($select); diff --git a/Model/TransactionProcessor.php b/Model/TransactionProcessor.php index f4d908f..49c8875 100644 --- a/Model/TransactionProcessor.php +++ b/Model/TransactionProcessor.php @@ -118,6 +118,11 @@ class TransactionProcessor */ private $resourceConnection; + /** + * @var OrderLocator + */ + private $orderLocator; + /** * TransactionProcessor constructor. * @@ -125,6 +130,7 @@ class TransactionProcessor * @param CartRepositoryInterface $cartRepository * @param CartManagementInterface $cartManagement * @param QuoteLocator $quoteLocator + * @param OrderLocator $orderLocator * @param Processor $processor * @param QuoteUpdater $quoteUpdater * @param LockManager $lockManager @@ -142,6 +148,7 @@ public function __construct( CartRepositoryInterface $cartRepository, CartManagementInterface $cartManagement, QuoteLocator $quoteLocator, + OrderLocator $orderLocator, Processor $processor, QuoteUpdater $quoteUpdater, LockManager $lockManager, @@ -156,6 +163,7 @@ public function __construct( $this->cartRepository = $cartRepository; $this->cartManagement = $cartManagement; $this->quoteLocator = $quoteLocator; + $this->orderLocator = $orderLocator; $this->processor = $processor; $this->quoteUpdater = $quoteUpdater; $this->lockManager = $lockManager; @@ -189,9 +197,6 @@ public function process(QuoteInterface $vippsQuote) $vippsQuote->getReservedOrderId() ); - // reload quote because it could be changed by another process - $vippsQuote = $this->quoteManagement->reload($vippsQuote); - if ($transaction->transactionWasCancelled() || $transaction->transactionWasVoided()) { $this->processCancelledTransaction($vippsQuote); } elseif ($transaction->isTransactionReserved()) { @@ -215,8 +220,9 @@ public function process(QuoteInterface $vippsQuote) */ private function processCancelledTransaction(QuoteInterface $vippsQuote) { - if ($vippsQuote->getOrderId()) { - $this->cancelOrder($vippsQuote->getOrderId()); + $order = $this->orderLocator->get($vippsQuote->getReservedOrderId()); + if ($order) { + $this->cancelOrder($order); } $vippsQuote->setStatus(QuoteStatusInterface::STATUS_CANCELED); @@ -236,9 +242,8 @@ private function processCancelledTransaction(QuoteInterface $vippsQuote) */ private function processReservedTransaction(QuoteInterface $vippsQuote, Transaction $transaction) { - if ($vippsQuote->getOrderId()) { - $order = $this->orderRepository->get($vippsQuote->getOrderId()); - } else { + $order = $this->orderLocator->get($vippsQuote->getReservedOrderId()); + if (!$order) { $order = $this->placeOrder($vippsQuote, $transaction); } @@ -262,8 +267,9 @@ private function processReservedTransaction(QuoteInterface $vippsQuote, Transact */ private function processExpiredTransaction(QuoteInterface $vippsQuote) { - if ($vippsQuote->getOrderId()) { - $this->orderManagement->cancel($vippsQuote->getOrderId()); + $order = $this->orderLocator->get($vippsQuote->getReservedOrderId()); + if ($order) { + $this->cancelOrder($order); } $vippsQuote->setStatus(QuoteStatusInterface::STATUS_EXPIRED); @@ -501,11 +507,10 @@ private function notify(OrderInterface $order) * * @throws \Exception */ - private function cancelOrder($orderId): void + private function cancelOrder($order): void { - $order = $this->orderRepository->get($orderId); if ($order->getState() === Order::STATE_NEW) { - $this->orderManagement->cancel($orderId); + $this->orderManagement->cancel($order->getEntityId()); } else { $connection = $this->resourceConnection->getConnection(); try { @@ -513,7 +518,7 @@ private function cancelOrder($orderId): void $order->setState(Order::STATE_NEW); $this->orderRepository->save($order); - $this->orderManagement->cancel($orderId); + $this->orderManagement->cancel($order->getEntityId()); $connection->commit(); } catch (\Exception $e) { diff --git a/composer.json b/composer.json index aead310..ce46ebc 100644 --- a/composer.json +++ b/composer.json @@ -3,7 +3,7 @@ "type": "magento2-module", "description": "Vipps Payment Method", "license": "proprietary", - "version": "2.3.17", + "version": "2.3.18", "require": { "magento/framework": "102.0.*", "magento/module-sales": "102.0.*",