Merge pull request #103 from vippsas/publication-2.3.18

VIPPS-385: Duplicate and missing order issue
vippsas · Jun 4, 2021 · f602ef7 · f602ef7
2 parents 20b6782 + 5ff7854
commit f602ef7
Show file tree

Hide file tree

Showing 6 changed files with 98 additions and 47 deletions.
diff --git a/Controller/Payment/Fallback.php b/Controller/Payment/Fallback.php
@@ -20,26 +20,28 @@
 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;
 use Magento\Framework\Session\SessionManagerInterface;
 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;
 use Vipps\Payment\Api\QuoteRepositoryInterface;
 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,12 +360,28 @@ 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 {
                 $resultRedirect->setPath('checkout/cart', ['_secure' => true]);
             }
         }
     }
+
+    /**
+     * @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
@@ -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
@@ -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
@@ -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
@@ -118,13 +118,19 @@ class TransactionProcessor
      */
     private $resourceConnection;
 
+    /**
+     * @var OrderLocator
+     */
+    private $orderLocator;
+
     /**
      * TransactionProcessor constructor.
      *
      * @param OrderRepositoryInterface $orderRepository
      * @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,19 +507,18 @@ 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 {
                 $connection->beginTransaction();
 
                 $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
@@ -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.*",