From 19845ce4e0c8fa2c67d98246ca872cf956f29696 Mon Sep 17 00:00:00 2001 From: Luke Rodgers Date: Fri, 14 Jul 2023 12:17:00 +0100 Subject: [PATCH 1/4] Add `CancelOrderWithDeletedProductTest.php` To cover scenarios where the source items table is cleaned up upon delete, versus not --- .../CancelOrderWithDeletedProductTest.php | 232 ++++++++++++++++++ 1 file changed, 232 insertions(+) create mode 100644 dev/MagentoTests/Integration/Cancel/CancelOrderWithDeletedProductTest.php diff --git a/dev/MagentoTests/Integration/Cancel/CancelOrderWithDeletedProductTest.php b/dev/MagentoTests/Integration/Cancel/CancelOrderWithDeletedProductTest.php new file mode 100644 index 0000000..e19deb6 --- /dev/null +++ b/dev/MagentoTests/Integration/Cancel/CancelOrderWithDeletedProductTest.php @@ -0,0 +1,232 @@ +objectManager = Bootstrap::getObjectManager(); + $this->clearQueueProcessor = $this->objectManager->get(ClearQueueProcessor::class); + $this->productRepository = $this->objectManager->get(ProductRepositoryInterface::class); + $this->productRepository->cleanCache(); + $this->getSourceItemsBySku = $this->objectManager->get(GetSourceItemsBySkuInterface::class); + $this->consumerFactory = $this->objectManager->get(ConsumerFactory::class); + $this->registry = $this->objectManager->get(Registry::class); + } + + /** + * @magentoConfigFixture default/cataloginventory/options/synchronize_with_catalog 0 + */ + public function testCancelOrderWithDeletedProductAndNotSyncWithCatalog() + { + $sku = uniqid('product_will_be_deleted'); + + /** + * Create a product with 5 qty + */ + $this->productFixture = new ProductFixture( + ProductBuilder::aSimpleProduct() + ->withSku($sku) + ->withPrice(10) + ->withStockQty(5) + ->withIsInStock(true) + ->build() + ); + + /** + * Create a customer and login + */ + $this->customerFixture = new CustomerFixture(CustomerBuilder::aCustomer()->withAddresses( + AddressBuilder::anAddress()->asDefaultBilling(), + AddressBuilder::anAddress()->asDefaultShipping() + )->build()); + $this->customerFixture->login(); + + /** + * Order 1 qty + */ + $checkout = CustomerCheckout::fromCart( + CartBuilder::forCurrentSession() + ->withSimpleProduct( + $this->productFixture->getSku(), + 1 + ) + ->build() + ); + + $order = $checkout + ->withPaymentMethodCode('checkmo') + ->placeOrder(); + $this->assertGreaterThan(0, strlen($order->getIncrementId()), 'the order does not have a valid increment_id'); + $this->assertIsNumeric($order->getId(), 'the order does not have an entity_id'); + + /** + * Ensure consumers are cleared before next steps + */ + $this->clearQueueProcessor->execute('inventory.source.items.cleanup'); + + /** + * Delete the product + */ + $origVal = $this->registry->registry('isSecureArea'); + $this->registry->unregister('isSecureArea'); + $this->registry->register('isSecureArea', true); + $product = $this->productRepository->deleteById($sku); + $this->registry->unregister('isSecureArea'); + $this->registry->register('isSecureArea', $origVal); + + try { + $this->productRepository->getById($sku); + $this->fail('This product should not be loadable: ' . $sku); + } catch (\Magento\Framework\Exception\NoSuchEntityException $e) { + // This exception is expected to happen + } + + /** + * cataloginventory/options/synchronize_with_catalog=0 + * + * Verify the source items still exist after the delete + */ + $sourceItems = $this->getSourceItemsBySku->execute($sku); + self::assertNotEmpty($sourceItems); + + /** + * Cancel the order + */ + $this->assertTrue($order->canCancel(), 'Cannot cancel the order'); + $order->cancel(); + $this->assertEquals('canceled', $order->getStatus(), 'The order was not cancelled'); + } + + /** + * @magentoConfigFixture default/cataloginventory/options/synchronize_with_catalog 1 + */ + public function testCancelOrderWithDeletedProductAndSyncWithCatalog() + { + $sku = uniqid('product_will_be_deleted'); + + /** + * Create a product with 5 qty + */ + $this->productFixture = new ProductFixture( + ProductBuilder::aSimpleProduct() + ->withSku($sku) + ->withPrice(10) + ->withStockQty(5) + ->withIsInStock(true) + ->build() + ); + + /** + * Create a customer and login + */ + $this->customerFixture = new CustomerFixture(CustomerBuilder::aCustomer()->withAddresses( + AddressBuilder::anAddress()->asDefaultBilling(), + AddressBuilder::anAddress()->asDefaultShipping() + )->build()); + $this->customerFixture->login(); + + /** + * Order 1 qty + */ + $checkout = CustomerCheckout::fromCart( + CartBuilder::forCurrentSession() + ->withSimpleProduct( + $this->productFixture->getSku(), + 1 + ) + ->build() + ); + + $order = $checkout + ->withPaymentMethodCode('checkmo') + ->placeOrder(); + $this->assertGreaterThan(0, strlen($order->getIncrementId()), 'the order does not have a valid increment_id'); + $this->assertIsNumeric($order->getId(), 'the order does not have an entity_id'); + + /** + * Ensure consumers are cleared before next steps + */ + $this->clearQueueProcessor->execute('inventory.source.items.cleanup'); + + /** + * Delete the product + */ + $origVal = $this->registry->registry('isSecureArea'); + $this->registry->unregister('isSecureArea'); + $this->registry->register('isSecureArea', true); + $product = $this->productRepository->deleteById($sku); + $this->registry->unregister('isSecureArea'); + $this->registry->register('isSecureArea', $origVal); + + try { + $this->productRepository->getById($sku); + $this->fail('This product should not be loadable: ' . $sku); + } catch (\Magento\Framework\Exception\NoSuchEntityException $e) { + // This exception is expected to happen + } + + /** + * Process the source items cleanup consumers and verify the deletions worked, because + * cataloginventory/options/synchronize_with_catalog=1 + */ + $consumer = $this->consumerFactory->get('inventory.source.items.cleanup'); + $consumer->process(1); + + $sourceItems = $this->getSourceItemsBySku->execute($sku); + self::assertEmpty($sourceItems); + + /** + * Cancel the order + */ + $this->assertTrue($order->canCancel(), 'Cannot cancel the order'); + $order->cancel(); + $this->assertEquals('canceled', $order->getStatus(), 'The order was not cancelled'); + } +} From 6fbdc32c3f5ac8264b8a31893f47d8797382ce72 Mon Sep 17 00:00:00 2001 From: Luke Rodgers Date: Fri, 14 Jul 2023 12:28:58 +0100 Subject: [PATCH 2/4] Backport `ClearQueueProcessor` logic --- .../CancelOrderWithDeletedProductTest.php | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/dev/MagentoTests/Integration/Cancel/CancelOrderWithDeletedProductTest.php b/dev/MagentoTests/Integration/Cancel/CancelOrderWithDeletedProductTest.php index e19deb6..b64dfc5 100644 --- a/dev/MagentoTests/Integration/Cancel/CancelOrderWithDeletedProductTest.php +++ b/dev/MagentoTests/Integration/Cancel/CancelOrderWithDeletedProductTest.php @@ -2,6 +2,9 @@ declare(strict_types=1); namespace Ampersand\DisableStockReservation\Test\Integration\Cancel; +use Magento\Framework\MessageQueue\QueueRepository; +use Magento\Framework\MessageQueue\Consumer\ConfigInterface as ConsumerConfig; +use Magento\Framework\MessageQueue\Consumer\Config\ConsumerConfigItemInterface; use Magento\Framework\Registry; use Magento\Catalog\Api\ProductRepositoryInterface; use Magento\Framework\ObjectManagerInterface; @@ -9,7 +12,6 @@ use Magento\Framework\MessageQueue\ConsumerFactory; use Magento\InventorySales\Model\GetStockBySalesChannelCache; use Magento\TestFramework\Helper\Bootstrap; -use Magento\TestFramework\MessageQueue\ClearQueueProcessor; use TddWizard\Fixtures\Catalog\ProductBuilder; use TddWizard\Fixtures\Catalog\ProductFixture; use TddWizard\Fixtures\Sales\ShipmentBuilder; @@ -26,9 +28,6 @@ class CancelOrderWithDeletedProduct extends TestCase /** @var ObjectManagerInterface */ private $objectManager; - /** @var ClearQueueProcessor */ - private $clearQueueProcessor; - /** @var GetSourceItemsBySkuInterface */ private $getSourceItemsBySku; @@ -37,7 +36,13 @@ class CancelOrderWithDeletedProduct extends TestCase /** @var ProductRepositoryInterface */ private $productRepository; - + + /** @var ConsumerConfig*/ + private $consumerConfig; + + /** @var QueueRepository */ + private $queueRepository; + /** @var Registry */ private $registry; @@ -52,12 +57,13 @@ public function setUp(): void parent::setUp(); $this->objectManager = Bootstrap::getObjectManager(); - $this->clearQueueProcessor = $this->objectManager->get(ClearQueueProcessor::class); $this->productRepository = $this->objectManager->get(ProductRepositoryInterface::class); $this->productRepository->cleanCache(); $this->getSourceItemsBySku = $this->objectManager->get(GetSourceItemsBySkuInterface::class); $this->consumerFactory = $this->objectManager->get(ConsumerFactory::class); $this->registry = $this->objectManager->get(Registry::class); + $this->consumerConfig = $this->objectManager->get(ConsumerConfig::class); + $this->queueRepository = $this->objectManager->get(QueueRepository::class); } /** @@ -106,11 +112,6 @@ public function testCancelOrderWithDeletedProductAndNotSyncWithCatalog() $this->assertGreaterThan(0, strlen($order->getIncrementId()), 'the order does not have a valid increment_id'); $this->assertIsNumeric($order->getId(), 'the order does not have an entity_id'); - /** - * Ensure consumers are cleared before next steps - */ - $this->clearQueueProcessor->execute('inventory.source.items.cleanup'); - /** * Delete the product */ @@ -192,9 +193,16 @@ public function testCancelOrderWithDeletedProductAndSyncWithCatalog() /** * Ensure consumers are cleared before next steps + * + * Backported logic from \Magento\TestFramework\MessageQueue\ClearQueueProcessor */ - $this->clearQueueProcessor->execute('inventory.source.items.cleanup'); - + /** @var ConsumerConfigItemInterface $consumerConfig */ + $consumerConfig = $this->consumerConfig->getConsumer('inventory.source.items.cleanup'); + $queue = $this->queueRepository->get($consumerConfig->getConnection(), $consumerConfig->getQueue()); + while ($message = $queue->dequeue()) { + $queue->acknowledge($message); + } + /** * Delete the product */ From 9a95e102e57672d0fa7b8be5a0578678ff3d0538 Mon Sep 17 00:00:00 2001 From: Luke Rodgers Date: Fri, 14 Jul 2023 12:29:36 +0100 Subject: [PATCH 3/4] Fix indexing error deleting products When the indexers are set to realtime (like in tests) and you delete a product you try to index no items here which throws an error --- src/Service/ExecuteSourceDeductionForItems.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Service/ExecuteSourceDeductionForItems.php b/src/Service/ExecuteSourceDeductionForItems.php index dec4d8f..303f97d 100644 --- a/src/Service/ExecuteSourceDeductionForItems.php +++ b/src/Service/ExecuteSourceDeductionForItems.php @@ -169,6 +169,8 @@ public function executeSourceDeductionForItems(OrderItem $orderItem, array $item $itemsIds = $this->product->getProductsIdsBySkus($itemsSkus); $itemsIds = array_values(array_map('intval', $itemsIds)); - $this->priceIndexer->reindexList($itemsIds); + if (!empty($itemsIds)) { + $this->priceIndexer->reindexList($itemsIds); + } } } From 9cbfbb7ca0bfcb2850e57f123df6b943c54d48bb Mon Sep 17 00:00:00 2001 From: Luke Rodgers Date: Fri, 14 Jul 2023 12:47:36 +0100 Subject: [PATCH 4/4] Fix order cancellation when source items have been removed --- src/Observer/CancelOrderItemObserver.php | 2 +- src/Service/ExecuteSourceDeductionForItems.php | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Observer/CancelOrderItemObserver.php b/src/Observer/CancelOrderItemObserver.php index 7e96f1d..77f4c14 100644 --- a/src/Observer/CancelOrderItemObserver.php +++ b/src/Observer/CancelOrderItemObserver.php @@ -52,6 +52,6 @@ public function execute(EventObserver $observer): void return; } - $this->executeSourceDeductionForItems->executeSourceDeductionForItems($orderItem, $itemsToCancel); + $this->executeSourceDeductionForItems->executeSourceDeductionForItems($orderItem, $itemsToCancel, true); } } diff --git a/src/Service/ExecuteSourceDeductionForItems.php b/src/Service/ExecuteSourceDeductionForItems.php index 303f97d..071202c 100644 --- a/src/Service/ExecuteSourceDeductionForItems.php +++ b/src/Service/ExecuteSourceDeductionForItems.php @@ -113,9 +113,10 @@ public function __construct( /** * @param OrderItem $orderItem * @param array $itemsToCancel + * @param bool $graceful * @throws NoSuchEntityException */ - public function executeSourceDeductionForItems(OrderItem $orderItem, array $itemsToCancel) + public function executeSourceDeductionForItems(OrderItem $orderItem, array $itemsToCancel, bool $graceful = false) { $order = $orderItem->getOrder(); @@ -163,7 +164,13 @@ public function executeSourceDeductionForItems(OrderItem $orderItem, array $item 'salesEvent' => $salesEvent ]); - $this->sourceDeductionService->execute($sourceDeductionRequest); + try { + $this->sourceDeductionService->execute($sourceDeductionRequest); + } catch (NoSuchEntityException $noSuchEntityException) { + if (!$graceful) { + throw $noSuchEntityException; + } + } } }