From 3be6d282d94f8a4ac56c703ae07c4505744261c7 Mon Sep 17 00:00:00 2001 From: Luke Rodgers Date: Fri, 4 Aug 2023 17:30:22 +0100 Subject: [PATCH 1/3] Test `back_to_stock` on refunds handles `is_in_stock` correctly In some cases if a product has moved to `is_in_stock=0` and a refund is issued and returns the stock, the `is_in_stock=1` flag is not being set so the product remains unsaleable. This is detailed in https://github.com/AmpersandHQ/magento2-disable-stock-reservation/pull/92 These test cases cover REST api use cases, as well as some magento integration tests to cover both `back_to_stock=true` and `return_to_stock_items` on the rest api --- composer.json | 2 +- dev/MagentoTests/Helper/IntegrationHelper.php | 90 +++++++ .../MultipleSourceInventoryTest.php | 14 +- ...sBackInStockWhenRefundedControllerTest.php | 222 ++++++++++++++++++ dev/README.MD | 6 +- dev/tests/acceptance/CheckoutCest.php | 181 ++++++++++++++ 6 files changed, 500 insertions(+), 15 deletions(-) create mode 100644 dev/MagentoTests/Helper/IntegrationHelper.php create mode 100644 dev/MagentoTests/Integration/Refund/ProductGoesBackInStockWhenRefundedControllerTest.php diff --git a/composer.json b/composer.json index 8d0a027..12b3f53 100644 --- a/composer.json +++ b/composer.json @@ -52,7 +52,7 @@ "vendor/bin/mtest 'php bin/magento --version'" ], "docker-run-codeception": [ - "URL=\"http://0.0.0.0:1234/\" MYSQL_USER=\"root\" MYSQL_HOST=\"0.0.0.0\" MYSQL_DB=\"magento\" MYSQL_PORT=\"1235\" ./dev/run-codeception.sh" + "TEST_GROUP=$TEST_GROUP URL=\"http://0.0.0.0:1234/\" MYSQL_USER=\"root\" MYSQL_HOST=\"0.0.0.0\" MYSQL_DB=\"magento\" MYSQL_PORT=\"1235\" ./dev/run-codeception.sh" ], "docker-run-unit-tests": [ "vendor/bin/mtest 'vendor/bin/phpunit -c /var/www/html/dev/tests/unit/phpunit.xml.dist --testsuite Unit --debug'" diff --git a/dev/MagentoTests/Helper/IntegrationHelper.php b/dev/MagentoTests/Helper/IntegrationHelper.php new file mode 100644 index 0000000..e122555 --- /dev/null +++ b/dev/MagentoTests/Helper/IntegrationHelper.php @@ -0,0 +1,90 @@ +get(StockRegistryStorage::class); + $storage->clean(); + } + } + + public static function clearSalesChannelCache() + { + if (class_exists(GetStockBySalesChannelCache::class)) { + $getStockBySalesChannelCache = Bootstrap::getObjectManager()->get(GetStockBySalesChannelCache::class); + $ref = new \ReflectionObject($getStockBySalesChannelCache); + try { + $refProperty = $ref->getProperty('channelCodes'); + } catch (\ReflectionException $exception) { + $refProperty = $ref->getParentClass()->getProperty('channelCodes'); + } + $refProperty->setAccessible(true); + $refProperty->setValue($getStockBySalesChannelCache, []); + } + } + + public static function clearGetStockItemDataCache() + { + if (class_exists(GetStockItemDataCache::class) && class_exists(CacheStorage::class)) { + $cacheStorage = Bootstrap::getObjectManager()->get(CacheStorage::class); + if (method_exists($cacheStorage, '_resetState')) { + $cacheStorage->_resetState(); + } else { + $ref = new \ReflectionObject($cacheStorage); + try { + $refProperty = $ref->getProperty('cachedItemData'); + } catch (\ReflectionException $exception) { + $refProperty = $ref->getParentClass()->getProperty('cachedItemData'); + } + $refProperty->setAccessible(true); + $refProperty->setValue($cacheStorage, [[]]); + } + } elseif (class_exists(GetStockItemDataCache::class)) { + $getStockItemDataCache = Bootstrap::getObjectManager()->get(GetStockItemDataCache::class); + $ref = new \ReflectionObject($getStockItemDataCache); + try { + $refProperty = $ref->getProperty('stockItemData'); + } catch (\ReflectionException $exception) { + $refProperty = $ref->getParentClass()->getProperty('stockItemData'); + } + $refProperty->setAccessible(true); + $refProperty->setValue($getStockItemDataCache, []); + } + } + + public static function clearGetLegacyStockItemCache() + { + if (class_exists(GetLegacyStockItemCache::class)) { + $object = Bootstrap::getObjectManager()->get(GetLegacyStockItemCache::class); + $ref = new \ReflectionObject($object); + try { + $refProperty = $ref->getProperty('legacyStockItemsBySku'); + } catch (\ReflectionException $exception) { + if (!$ref->getParentClass()) { + return; + } + $refProperty = $ref->getParentClass()->getProperty('legacyStockItemsBySku'); + } + $refProperty->setAccessible(true); + $refProperty->setValue($object, []); + } + } +} \ No newline at end of file diff --git a/dev/MagentoTests/Integration/MultipleSourceInventoryTest.php b/dev/MagentoTests/Integration/MultipleSourceInventoryTest.php index 6e461ee..4c9865c 100644 --- a/dev/MagentoTests/Integration/MultipleSourceInventoryTest.php +++ b/dev/MagentoTests/Integration/MultipleSourceInventoryTest.php @@ -30,6 +30,8 @@ use PHPUnit\Framework\TestCase; use Magento\InventorySalesApi\Api\StockResolverInterface; use Magento\InventorySales\Model\GetStockBySalesChannelCache; +require_once __DIR__ . '/../Helper/IntegrationHelper.php'; +use Ampersand\DisableStockReservation\Test\Helper\IntegrationHelper as TestHelper; class MultipleSourceInventoryTest extends TestCase { @@ -219,17 +221,7 @@ private function setStockItemConfigIsDecimal(string $sku, int $stockId): void */ private function clearSalesChannelCache(): void { - if (class_exists(GetStockBySalesChannelCache::class)) { - $getStockBySalesChannelCache = $this->objectManager->get(GetStockBySalesChannelCache::class); - $ref = new \ReflectionObject($getStockBySalesChannelCache); - try { - $refProperty = $ref->getProperty('channelCodes'); - } catch (\ReflectionException $exception) { - $refProperty = $ref->getParentClass()->getProperty('channelCodes'); - } - $refProperty->setAccessible(true); - $refProperty->setValue($getStockBySalesChannelCache, []); - } + TestHelper::clearCaches(); $stockId = $this->objectManager->get(StockResolverInterface::class) ->execute(SalesChannelInterface::TYPE_WEBSITE, 'eu_website') diff --git a/dev/MagentoTests/Integration/Refund/ProductGoesBackInStockWhenRefundedControllerTest.php b/dev/MagentoTests/Integration/Refund/ProductGoesBackInStockWhenRefundedControllerTest.php new file mode 100644 index 0000000..7ee34f2 --- /dev/null +++ b/dev/MagentoTests/Integration/Refund/ProductGoesBackInStockWhenRefundedControllerTest.php @@ -0,0 +1,222 @@ +customerFixture = new CustomerFixture(CustomerBuilder::aCustomer()->withAddresses( + AddressBuilder::anAddress()->asDefaultBilling(), + AddressBuilder::anAddress()->asDefaultShipping() + )->build()); + $this->customerFixture->login(); + + /** + * Create a product with 5 qty + */ + $this->productFixture = new ProductFixture( + ProductBuilder::aSimpleProduct() + ->withPrice(10) + ->withStockQty(5) + ->withIsInStock(true) + ->build() + ); + + $stockItem = $this->getStockItem($this->productFixture->getSku()); + $this->assertTrue($stockItem->getIsInStock(), 'Product should be created in stock'); + $this->assertEquals(5, $stockItem->getQty(), 'Product should be created qty=5'); + $this->assertEquals( + 5, + $this->getSource($this->productFixture->getSku())->getQuantity(), + 'The product source should have qty=0 when it is created' + ); + + /** + * Order 5 qty + */ + $checkout = CustomerCheckout::fromCart( + CartBuilder::forCurrentSession() + ->withSimpleProduct( + $this->productFixture->getSku(), + 5 + ) + ->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'); + + $this->assertEquals( + 0, + $this->getSource($this->productFixture->getSku())->getQuantity(), + 'The product source should have qty=0 after the order' + ); + $stockItem = $this->getStockItem($this->productFixture->getSku()); + $this->assertEquals(0, $stockItem->getQty(), 'Product should go qty=0 after purchase'); + $this->assertFalse($stockItem->getIsInStock(), 'Product should go is_in_stock=0 after purchase'); + + /** + * Invoice and ship + */ + $this->assertTrue($order->canInvoice(), 'The order cannot have an invoice created'); + $invoice = InvoiceBuilder::forOrder($order)->build(); + $this->assertTrue($order->canShip(), 'The order cannot be shipped'); + $shipment = ShipmentBuilder::forOrder($order)->build(); + $this->assertGreaterThan(0, strlen($shipment->getIncrementId()), 'the shipment does not have a valid increment_id'); + $order->save(); + + /** + * Create a credit memo with return_to_stock (aka setBacKToStock) + */ + $this->assertTrue($order->canCreditmemo(), 'The order cannot have a credit memo created'); + + /** @var $item \Magento\Sales\Model\Order\Item */ + $item = $order->getAllItems()[0]; + $this->getRequest()->setParam( + 'order_id', + $order->getId() + ); + $payload = [ + 'do_offline' => 1, + 'comment_text' => '', + 'shipping_amount' => 0, + 'adjustment_positive' => 0, + 'adjustment_negative' => 0 + ]; + if ($backToStock) { + $payload['items'] = [ + $item->getId() => [ + 'back_to_stock' => $backToStock, + 'qty' => 5 + ] + ]; + } + + $this->getRequest()->setPostValue( + 'creditmemo', + $payload + ); + $this->getRequest()->setMethod('POST'); + $this->dispatch('backend/sales/order_creditmemo/save'); + $this->assertEquals(1, count($this->getSessionMessages()), 'We should only have 1 session message'); + $this->assertSessionMessages( + $this->equalTo( + ['You created the credit memo.'], + \Magento\Framework\Message\MessageInterface::TYPE_SUCCESS + ) + ); + + $stockItem = $this->getStockItem($this->productFixture->getSku()); + $this->assertEquals( + $expectedStockData['is_in_stock'], + $stockItem->getIsInStock(), + 'is_in_stock does not match expected' + ); + $this->assertEquals( + $expectedStockData['qty'], + $stockItem->getQty(), + 'qty does not match expected' + ); + + $this->assertEquals( + $expectedStockData['qty'], + $this->getSource($this->productFixture->getSku())->getQuantity() + ); + } + + /** + * @param $sku + * @return mixed|null + */ + private function getSource($sku) + { + /** @var \Magento\InventoryApi\Api\GetSourceItemsBySkuInterface $getStockItems */ + $getSourceItemsBySku = Bootstrap::getObjectManager()->get(\Magento\InventoryApi\Api\GetSourceItemsBySkuInterface::class); + $sources = $getSourceItemsBySku->execute($this->productFixture->getSku()); + $this->assertIsArray($sources); + $this->assertCount(1, $sources); + $source = array_pop($sources); + return $source; + } + + /** + * @param $sku + * @return \Magento\CatalogInventory\Api\Data\StockItemInterface\ + */ + private function getStockItem($sku) + { + TestHelper::clearCaches(); + $registry = Bootstrap::getObjectManager()->create(\Magento\CatalogInventory\Model\StockRegistry::class); + /** @var \Magento\CatalogInventory\Api\Data\StockItemInterface $stockItem */ + $stockItem = $registry->getStockItemBySku($sku); + $this->assertGreaterThan(0, strlen($stockItem->getItemId())); + + TestHelper::clearCaches(); + return $stockItem; + } + + public function productBackInStockHandlingDataProvider(): array + { + return [ + 'back_to_stock=true returns items' => [ + 'back_to_stock' => '1', + 'expected_stock_data' => [ + 'is_in_stock' => true, + 'qty' => 5 + ] + ], + 'back_to_stock=false does not return items' => [ + 'back_to_stock' => '0', + 'expected_stock_data' => [ + 'is_in_stock' => false, + 'qty' => 0 + ] + ], + ]; + } + + /** + * @return void + */ + protected function tearDown(): void + { + if (isset($this->productFixture)) { + $this->productFixture->rollback(); + } + if (isset($this->customerFixture)) { + $this->customerFixture->rollback();; + } + } +} diff --git a/dev/README.MD b/dev/README.MD index b6fe172..04922b0 100644 --- a/dev/README.MD +++ b/dev/README.MD @@ -18,8 +18,8 @@ composer docker-configure-magento Run the tests ``` -composer docker-run-unit-tests # unit tests -composer docker-run-integration-tests # integration tests -composer docker-run-codeception # codeception rest api tests +composer docker-run-unit-tests # unit tests +composer docker-run-integration-tests # integration tests +TEST_GROUP=2-4-5 composer docker-run-codeception # codeception rest api tests ``` diff --git a/dev/tests/acceptance/CheckoutCest.php b/dev/tests/acceptance/CheckoutCest.php index 23c8655..22d5a8a 100644 --- a/dev/tests/acceptance/CheckoutCest.php +++ b/dev/tests/acceptance/CheckoutCest.php @@ -310,4 +310,185 @@ public function refundOnShippedOrderDoesNotAffectQuantityWhenNotReturnedToStock( $refundedQty = $I->grabFromDatabase('cataloginventory_stock_item', 'qty', ['product_id' => $productId]); $I->assertEquals(95, $refundedQty, 'The quantity should remain at 95 when not returned'); } + + /** + * @link https://github.com/AmpersandHQ/magento2-disable-stock-reservation/pull/92 + * + * @depends stockIsReturnedWhenOrderIsCancelled + * @param Step\Acceptance\Magento $I + */ + public function productGoesBackInStockWhenOrderIsRefunded(Step\Acceptance\Magento $I) + { + if (in_array(getenv('TEST_GROUP'), ['2-4-4', '2-4-3'])) { + //https://github.com/magento/inventory/commit/1d5bc9daa517e41577d533c58be5f42251270366 + // Inventory changes done through API not reflect on the PDP page + $I->markTestSkipped('Known Rest API only failure in this version of magento, covered with integration test'); + } + + $productId = $I->createSimpleProduct('amp_stock_returns_in_stock_on_refund',1); + $I->assertEquals( + 1, + $I->grabFromDatabase('cataloginventory_stock_item', 'qty', ['product_id' => $productId]), + 'Product has not started with qty=1' + ); + $I->assertEquals( + 1, + $I->grabFromDatabase('cataloginventory_stock_item', 'is_in_stock', ['product_id' => $productId]), + 'Product has not started with is_in_stock=1' + ); + + $cartId = $I->getGuestQuote(); + $I->addSimpleProductToQuote($cartId, 'amp_stock_returns_in_stock_on_refund', 1); + $orderId = $I->completeGuestCheckout($cartId); + + $I->assertEquals( + 0, + $I->grabFromDatabase('cataloginventory_stock_item', 'qty', ['product_id' => $productId]), + 'Product did not go qty=0 after an order' + ); + $I->assertEquals( + 0, + $I->grabFromDatabase('cataloginventory_stock_item', 'is_in_stock', ['product_id' => $productId]), + 'Product did not go is_in_stock=0 after an order' + ); + + $I->amBearerAuthenticated(Step\Acceptance\Magento::ACCESS_TOKEN); + $I->sendPOSTAndVerifyResponseCodeIs200("V1/order/{$orderId}/invoice", json_encode([ + "capture" => true, + "notify" => false + ])); + $I->sendPOSTAndVerifyResponseCodeIs200("V1/order/{$orderId}/ship"); + + $I->assertEquals( + 0, + $I->grabFromDatabase('cataloginventory_stock_item', 'qty', ['product_id' => $productId]), + 'Product did not stay qty=0 after invoicing and shipping' + ); + $I->assertEquals( + 0, + $I->grabFromDatabase('cataloginventory_stock_item', 'is_in_stock', ['product_id' => $productId]), + 'Product did not stay is_in_stock=0 after invoicing and shipping' + ); + + $orderItemId = $I->grabFromDatabase('sales_order_item', 'item_id', ['order_id' => $orderId]); + + $I->sendPOSTAndVerifyResponseCodeIs200("V1/order/{$orderId}/refund", json_encode([ + "items" => [ + [ + "order_item_id" => $orderItemId, + "qty" => 1 + ] + ], + "notify" => false, + "arguments" => [ + "shipping_amount" => 0, + "adjustment_positive" => 0, + "adjustment_negative" => 0, + "extension_attributes" => [ + "return_to_stock_items" => [ + $orderItemId + ] + ] + ] + ])); + + $I->assertEquals( + 1, + $I->grabFromDatabase('cataloginventory_stock_item', 'is_in_stock', ['product_id' => $productId]), + 'Product did not go to is_in_stock=1 after a refund' + ); + $I->assertEquals( + 1, + $I->grabFromDatabase('cataloginventory_stock_item', 'qty', ['product_id' => $productId]), + 'Product did not go to qty=1 after a refund' + ); + } + + public function productStockNotAmendedWhenOrderIsRefundedAndNoBackToStockSet(Step\Acceptance\Magento $I) + { + if (in_array(getenv('TEST_GROUP'), ['2-4-4', '2-4-3'])) { + //https://github.com/magento/inventory/commit/1d5bc9daa517e41577d533c58be5f42251270366 + // Inventory changes done through API not reflect on the PDP page + $I->markTestSkipped('Known failure in this version of magento'); + } + + $productId = $I->createSimpleProduct('amp_no_stock_returns_in_stock_on_refund',1); + $I->assertEquals( + 1, + $I->grabFromDatabase('cataloginventory_stock_item', 'qty', ['product_id' => $productId]), + 'Product has not started with qty=1' + ); + $I->assertEquals( + 1, + $I->grabFromDatabase('cataloginventory_stock_item', 'is_in_stock', ['product_id' => $productId]), + 'Product has not started with is_in_stock=1' + ); + + $cartId = $I->getGuestQuote(); + $I->addSimpleProductToQuote($cartId, 'amp_no_stock_returns_in_stock_on_refund', 1); + $orderId = $I->completeGuestCheckout($cartId); + + $I->assertEquals( + 0, + $I->grabFromDatabase('cataloginventory_stock_item', 'qty', ['product_id' => $productId]), + 'Product did not go qty=0 after an order' + ); + $I->assertEquals( + 0, + $I->grabFromDatabase('cataloginventory_stock_item', 'is_in_stock', ['product_id' => $productId]), + 'Product did not go is_in_stock=0 after an order' + ); + + $I->amBearerAuthenticated(Step\Acceptance\Magento::ACCESS_TOKEN); + $I->sendPOSTAndVerifyResponseCodeIs200("V1/order/{$orderId}/invoice", json_encode([ + "capture" => true, + "notify" => false + ])); + $I->sendPOSTAndVerifyResponseCodeIs200("V1/order/{$orderId}/ship"); + + $I->assertEquals( + 0, + $I->grabFromDatabase('cataloginventory_stock_item', 'qty', ['product_id' => $productId]), + 'Product did not stay qty=0 after invoicing and shipping' + ); + $I->assertEquals( + 0, + $I->grabFromDatabase('cataloginventory_stock_item', 'is_in_stock', ['product_id' => $productId]), + 'Product did not stay is_in_stock=0 after invoicing and shipping' + ); + + $orderItemId = $I->grabFromDatabase('sales_order_item', 'item_id', ['order_id' => $orderId]); + + // Do not return to stock, the products should remail OOS + $I->sendPOSTAndVerifyResponseCodeIs200("V1/order/{$orderId}/refund", json_encode([ + "items" => [ + [ + "order_item_id" => $orderItemId, + "qty" => 1 + ] + ], + "notify" => false, + "arguments" => [ + "shipping_amount" => 0, + "adjustment_positive" => 0, + "adjustment_negative" => 0, + /*"extension_attributes" => [ + "return_to_stock_items" => [ + $orderItemId + ] + ]*/ + ] + ])); + + $I->assertEquals( + 0, + $I->grabFromDatabase('cataloginventory_stock_item', 'is_in_stock', ['product_id' => $productId]), + 'Product did not stay is_in_stock=0 after a refund' + ); + $I->assertEquals( + 0, + $I->grabFromDatabase('cataloginventory_stock_item', 'qty', ['product_id' => $productId]), + 'Product did not stay qty=0 after a refund' + ); + } } From b1a1c7b3a21d782b8cae4ed4fbf7c8d917ecb6d5 Mon Sep 17 00:00:00 2001 From: Mark Fischmann Date: Tue, 13 Dec 2022 21:51:27 +0100 Subject: [PATCH 2/3] Change a class in the refund observer to use the module's own SourceDeductionService instead of the native one. This causes refunds to not change stock status in cataloginventory_stock_item, cataloginventory_stock_status and inventory_source_item if the product goes from "out of stock" to "in stock" following a refund. --- .../RestoreSourceItemQuantityOnRefundObserver.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Observer/RestoreSourceItemQuantityOnRefundObserver.php b/src/Observer/RestoreSourceItemQuantityOnRefundObserver.php index 8e8be77..4221577 100644 --- a/src/Observer/RestoreSourceItemQuantityOnRefundObserver.php +++ b/src/Observer/RestoreSourceItemQuantityOnRefundObserver.php @@ -20,7 +20,7 @@ use Magento\InventorySalesApi\Model\StockByWebsiteIdResolverInterface; use Magento\InventorySourceDeductionApi\Model\ItemToDeductFactory; use Magento\InventorySourceDeductionApi\Model\SourceDeductionRequestFactory; -use Magento\InventorySourceDeductionApi\Model\SourceDeductionService; +use Magento\InventorySourceDeductionApi\Model\SourceDeductionServiceInterface; use Magento\Sales\Model\OrderRepository; class RestoreSourceItemQuantityOnRefundObserver implements ObserverInterface @@ -76,7 +76,7 @@ class RestoreSourceItemQuantityOnRefundObserver implements ObserverInterface private $getSalesChannelForOrder; /** - * @var SourceDeductionService + * @var SourceDeductionServiceInterface */ private $sourceDeductionService; @@ -108,7 +108,7 @@ class RestoreSourceItemQuantityOnRefundObserver implements ObserverInterface * @param SourceDeductionRequestFactory $sourceDeductionRequestFactory * @param SalesEventExtensionFactory $salesEventExtensionFactory * @param GetSalesChannelForOrderFactory $getSalesChannelForOrderFactory - * @param SourceDeductionService $sourceDeductionService + * @param SourceDeductionServiceInterface $sourceDeductionService * @param GetSourceItemsBySkuInterface $getSourceItemsBySku * @param SalesEventInterfaceFactory $salesEventFactory * @param ItemToDeductFactory $itemToDeductFactory @@ -124,7 +124,7 @@ public function __construct( SourceDeductionRequestFactory $sourceDeductionRequestFactory, SalesEventExtensionFactory $salesEventExtensionFactory, GetSalesChannelForOrderFactory $getSalesChannelForOrderFactory, - SourceDeductionService $sourceDeductionService, + SourceDeductionServiceInterface $sourceDeductionService, GetSourceItemsBySkuInterface $getSourceItemsBySku, SalesEventInterfaceFactory $salesEventFactory, ItemToDeductFactory $itemToDeductFactory From 77d7d911c1b760d45b7ccf10111cab4e76037775 Mon Sep 17 00:00:00 2001 From: Luke Rodgers Date: Mon, 7 Aug 2023 16:42:09 +0100 Subject: [PATCH 3/3] Fix code styling --- dev/MagentoTests/Helper/IntegrationHelper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/MagentoTests/Helper/IntegrationHelper.php b/dev/MagentoTests/Helper/IntegrationHelper.php index e122555..3b6a0e6 100644 --- a/dev/MagentoTests/Helper/IntegrationHelper.php +++ b/dev/MagentoTests/Helper/IntegrationHelper.php @@ -87,4 +87,4 @@ public static function clearGetLegacyStockItemCache() $refProperty->setValue($object, []); } } -} \ No newline at end of file +}