From de48c3bd454f2fa84d18affb1e0e8f83320bef09 Mon Sep 17 00:00:00 2001 From: KrasnoshchokBohdan Date: Thu, 29 May 2025 00:00:33 +0300 Subject: [PATCH 1/4] magento/magento2#39873: addProductsToCart mutation doesn't work as expect with a given parent_sku - adding new ConfigurableProductPrecursor --- .../Quote/Model/Cart/AddProductsToCart.php | 2 +- .../ConfigurableProductPrecursor.php | 62 +++++++++++++++++++ .../Magento/QuoteGraphQl/etc/graphql/di.xml | 7 +++ 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 app/code/Magento/QuoteGraphQl/Model/CartItem/Precursor/ConfigurableProductPrecursor.php diff --git a/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php b/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php index 7fa7dd961ca5d..3428058c92c84 100644 --- a/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php +++ b/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php @@ -54,7 +54,7 @@ public function execute(string $maskedCartId, array $cartItems): AddProductsToCa $cart = $this->cartRepository->get($cartId); $allErrors = []; - $failedCartItems = $this->addItemsToCart($cart, $cartItems); + $failedCartItems = $this->addItemsToCart($cart, $cartItems);//error here $saveCart = empty($failedCartItems); if (!empty($failedCartItems)) { /* Check if some cart items were successfully added to the cart */ diff --git a/app/code/Magento/QuoteGraphQl/Model/CartItem/Precursor/ConfigurableProductPrecursor.php b/app/code/Magento/QuoteGraphQl/Model/CartItem/Precursor/ConfigurableProductPrecursor.php new file mode 100644 index 0000000000000..a2155550d30d5 --- /dev/null +++ b/app/code/Magento/QuoteGraphQl/Model/CartItem/Precursor/ConfigurableProductPrecursor.php @@ -0,0 +1,62 @@ + $itemData) { + $result[$key] = $itemData; + + if (!empty($itemData['parent_sku'])) { + $parentItemData = [ + 'sku' => $itemData['parent_sku'], + 'quantity' => $itemData['quantity'], + 'parent_sku' => null, + 'selected_options' => $itemData['selected_options'] ?? [], + 'entered_options' => $itemData['entered_options'] ?? [], + ]; + + $result[] = $parentItemData; + } + } + + return $result; + } + + /** + * Return collected errors + * + * @return array + */ + public function getErrors(): array + { + return $this->errors; + } +} diff --git a/app/code/Magento/QuoteGraphQl/etc/graphql/di.xml b/app/code/Magento/QuoteGraphQl/etc/graphql/di.xml index eca1b68354e75..86be288de25e8 100644 --- a/app/code/Magento/QuoteGraphQl/etc/graphql/di.xml +++ b/app/code/Magento/QuoteGraphQl/etc/graphql/di.xml @@ -106,4 +106,11 @@ + + + + Magento\QuoteGraphQl\Model\CartItem\Precursor\ConfigurableProductPrecursor + + + From 287b14eaeb71a64ce11a54de7762d5e2b070f4b5 Mon Sep 17 00:00:00 2001 From: KrasnoshchokBohdan Date: Tue, 3 Jun 2025 17:19:58 +0300 Subject: [PATCH 2/4] magento/magento2#39873: addProductsToCart mutation doesn't work as expect with a given parent_sku - modifying ConfigurableProductPrecursor --- .../ConfigurableProductPrecursor.php | 112 +++++++++++++++--- 1 file changed, 98 insertions(+), 14 deletions(-) diff --git a/app/code/Magento/QuoteGraphQl/Model/CartItem/Precursor/ConfigurableProductPrecursor.php b/app/code/Magento/QuoteGraphQl/Model/CartItem/Precursor/ConfigurableProductPrecursor.php index a2155550d30d5..9a8ae9c189ff9 100644 --- a/app/code/Magento/QuoteGraphQl/Model/CartItem/Precursor/ConfigurableProductPrecursor.php +++ b/app/code/Magento/QuoteGraphQl/Model/CartItem/Precursor/ConfigurableProductPrecursor.php @@ -7,6 +7,10 @@ namespace Magento\QuoteGraphQl\Model\CartItem\Precursor; +use Magento\Catalog\Api\Data\ProductInterface; +use Magento\Catalog\Api\ProductRepositoryInterface; +use Magento\ConfigurableProduct\Model\Product\Type\Configurable; +use Magento\Framework\Exception\NoSuchEntityException; use Magento\GraphQl\Model\Query\ContextInterface; use Magento\QuoteGraphQl\Model\CartItem\PrecursorInterface; @@ -20,6 +24,20 @@ class ConfigurableProductPrecursor implements PrecursorInterface */ private array $errors = []; + /** + * @var ProductRepositoryInterface + */ + private ProductRepositoryInterface $productRepository; + + /** + * @param ProductRepositoryInterface $productRepository + */ + public function __construct( + ProductRepositoryInterface $productRepository + ) { + $this->productRepository = $productRepository; + } + /** * Process cart item data to handle parent_sku for configurable products * @@ -29,25 +47,91 @@ class ConfigurableProductPrecursor implements PrecursorInterface */ public function process(array $cartItemData, ContextInterface $context): array { - $result = []; - - foreach ($cartItemData as $key => $itemData) { - $result[$key] = $itemData; - - if (!empty($itemData['parent_sku'])) { - $parentItemData = [ - 'sku' => $itemData['parent_sku'], - 'quantity' => $itemData['quantity'], - 'parent_sku' => null, - 'selected_options' => $itemData['selected_options'] ?? [], - 'entered_options' => $itemData['entered_options'] ?? [], + $processedCartItemData = []; + + foreach ($cartItemData as $cartItemIndex => $cartItem) { + if (!isset($cartItem['parent_sku'])) { + $processedCartItemData[$cartItemIndex] = $cartItem; + continue; + } + + try { + $childProduct = $this->productRepository->get($cartItem['sku']); + $parentProduct = $this->productRepository->get($cartItem['parent_sku']); + + if ($parentProduct->getTypeId() !== Configurable::TYPE_CODE) { + $this->errors[] = [ + 'message' => sprintf('Product %s is not a configurable product', $cartItem['parent_sku']), + 'code' => 'UNDEFINED' + ]; + $processedCartItemData[$cartItemIndex] = $cartItem; + continue; + } + + $configurableOptions = $this->getConfigurableOptions($parentProduct, $childProduct); + + if (empty($configurableOptions)) { + $this->errors[] = [ + 'message' => sprintf('Could not match child product %s with parent %s', $cartItem['sku'], $cartItem['parent_sku']), + 'code' => 'UNDEFINED' + ]; + $processedCartItemData[$cartItemIndex] = $cartItem; + continue; + } + + $parentCartItem = [ + 'sku' => $cartItem['parent_sku'], + 'quantity' => $cartItem['quantity'], + 'selected_options' => array_merge($configurableOptions, $cartItem['selected_options'] ?? []), + 'entered_options' => $cartItem['entered_options'] ?? [], + 'parent_sku' => null ]; - $result[] = $parentItemData; + $processedCartItemData[] = $parentCartItem; + unset($cartItemData[$cartItemIndex]); + + } catch (NoSuchEntityException $e) { + $this->errors[] = [ + 'message' => $e->getMessage(), + 'code' => 'UNDEFINED' + ]; + $processedCartItemData[$cartItemIndex] = $cartItem; } } - return $result; + return $processedCartItemData; + } + + /** + * Get configurable option IDs for the simple product + * + * @param ProductInterface $parentProduct + * @param ProductInterface $childProduct + * @return array + */ + private function getConfigurableOptions(ProductInterface $parentProduct, ProductInterface $childProduct): array + { + $selectedOptions = []; + + /** @var Configurable $configurableType */ + $configurableType = $parentProduct->getTypeInstance(); + $attributes = $configurableType->getConfigurableAttributes($parentProduct); + + $childProductData = $childProduct->getData(); + + foreach ($attributes as $attribute) { + $attributeId = $attribute->getProductAttribute()->getAttributeId(); + $attributeCode = $attribute->getProductAttribute()->getAttributeCode(); + + if (!isset($childProductData[$attributeCode])) { + continue; + } + + $optionId = $childProductData[$attributeCode]; + $selectedOptions[] = base64_encode("configurable/$attributeId/$optionId"); + } + + return $selectedOptions; } /** From 385a088f85852c045df4a93a97e6c73a3c8d80de Mon Sep 17 00:00:00 2001 From: KrasnoshchokBohdan <111579196+KrasnoshchokBohdan@users.noreply.github.com> Date: Tue, 3 Jun 2025 17:21:15 +0300 Subject: [PATCH 3/4] Update AddProductsToCart.php --- app/code/Magento/Quote/Model/Cart/AddProductsToCart.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php b/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php index 3428058c92c84..7fa7dd961ca5d 100644 --- a/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php +++ b/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php @@ -54,7 +54,7 @@ public function execute(string $maskedCartId, array $cartItems): AddProductsToCa $cart = $this->cartRepository->get($cartId); $allErrors = []; - $failedCartItems = $this->addItemsToCart($cart, $cartItems);//error here + $failedCartItems = $this->addItemsToCart($cart, $cartItems); $saveCart = empty($failedCartItems); if (!empty($failedCartItems)) { /* Check if some cart items were successfully added to the cart */ From 315bf54c0daafc1b72ffa39828ab95652367a561 Mon Sep 17 00:00:00 2001 From: KrasnoshchokBohdan Date: Thu, 12 Jun 2025 11:58:31 +0300 Subject: [PATCH 4/4] magento/magento2#39873: addProductsToCart mutation doesn't work as expect with a given parent_sku - Introduced comprehensive unit tests for ConfigurableProductPrecursor, covering scenarios like valid configurations, non-configurable parents, missing attributes, and missing products. Minor code adjustments were also made for improved readability and array merging in the process method. --- .../ConfigurableProductPrecursor.php | 9 +- .../ConfigurableProductPrecursorTest.php | 265 ++++++++++++++++++ 2 files changed, 270 insertions(+), 4 deletions(-) create mode 100644 app/code/Magento/QuoteGraphQl/Test/Unit/Model/CartItem/Precursor/ConfigurableProductPrecursorTest.php diff --git a/app/code/Magento/QuoteGraphQl/Model/CartItem/Precursor/ConfigurableProductPrecursor.php b/app/code/Magento/QuoteGraphQl/Model/CartItem/Precursor/ConfigurableProductPrecursor.php index 9a8ae9c189ff9..d9eb72e34bc31 100644 --- a/app/code/Magento/QuoteGraphQl/Model/CartItem/Precursor/ConfigurableProductPrecursor.php +++ b/app/code/Magento/QuoteGraphQl/Model/CartItem/Precursor/ConfigurableProductPrecursor.php @@ -42,10 +42,10 @@ public function __construct( * Process cart item data to handle parent_sku for configurable products * * @param array $cartItemData - * @param ContextInterface $context + * @param ContextInterface $_context * @return array */ - public function process(array $cartItemData, ContextInterface $context): array + public function process(array $cartItemData, ContextInterface $_context): array { $processedCartItemData = []; @@ -72,7 +72,8 @@ public function process(array $cartItemData, ContextInterface $context): array if (empty($configurableOptions)) { $this->errors[] = [ - 'message' => sprintf('Could not match child product %s with parent %s', $cartItem['sku'], $cartItem['parent_sku']), + 'message' => sprintf('Could not match child product %s with parent %s', + $cartItem['sku'], $cartItem['parent_sku']), 'code' => 'UNDEFINED' ]; $processedCartItemData[$cartItemIndex] = $cartItem; @@ -82,7 +83,7 @@ public function process(array $cartItemData, ContextInterface $context): array $parentCartItem = [ 'sku' => $cartItem['parent_sku'], 'quantity' => $cartItem['quantity'], - 'selected_options' => array_merge($configurableOptions, $cartItem['selected_options'] ?? []), + 'selected_options' => [...$configurableOptions, ...($cartItem['selected_options'] ?? [])], 'entered_options' => $cartItem['entered_options'] ?? [], 'parent_sku' => null ]; diff --git a/app/code/Magento/QuoteGraphQl/Test/Unit/Model/CartItem/Precursor/ConfigurableProductPrecursorTest.php b/app/code/Magento/QuoteGraphQl/Test/Unit/Model/CartItem/Precursor/ConfigurableProductPrecursorTest.php new file mode 100644 index 0000000000000..9322b4ed66db6 --- /dev/null +++ b/app/code/Magento/QuoteGraphQl/Test/Unit/Model/CartItem/Precursor/ConfigurableProductPrecursorTest.php @@ -0,0 +1,265 @@ +productRepositoryMock = $this->createMock(ProductRepositoryInterface::class); + $this->contextMock = $this->createMock(ContextInterface::class); + $this->precursor = new ConfigurableProductPrecursor($this->productRepositoryMock); + } + + /** + * Test processing cart item without parent SKU + * + * @return void + */ + public function testProcessItemWithoutParentSku(): void + { + $cartItemData = [ + [ + 'sku' => 'simple-1', + 'quantity' => 1, + 'selected_options' => ['option1'], + ] + ]; + + $result = $this->precursor->process($cartItemData, $this->contextMock); + + $this->assertSame($cartItemData, $result); + $this->assertEmpty($this->precursor->getErrors()); + } + + /** + * Test processing valid configurable product + * + * @return void + * @throws Exception + */ + public function testProcessValidConfigurableProduct(): void + { + $childProduct = $this->getMockProduct('simple-1'); + $childProduct->method('getData') + ->willReturn(['color' => '1']); + + $productAttributeMock = $this->createMock(AbstractAttribute::class); + $productAttributeMock->method('getAttributeId')->willReturn('1'); + $productAttributeMock->method('getAttributeCode')->willReturn('color'); + + $attributeMock = $this->getMockBuilder(Attribute::class) + ->disableOriginalConstructor() + ->addMethods(['getProductAttribute']) + ->getMock(); + $attributeMock->method('getProductAttribute')->willReturn($productAttributeMock); + + $attributesCollection = [$attributeMock]; + + $configurableTypeMock = $this->createMock(Configurable::class); + $configurableTypeMock->method('getConfigurableAttributes')->willReturn($attributesCollection); + + $parentProduct = $this->getMockProduct('configurable-1', Configurable::TYPE_CODE); + $parentProduct->method('getTypeInstance')->willReturn($configurableTypeMock); + + $this->productRepositoryMock->expects($this->exactly(2)) + ->method('get') + ->willReturnMap([ + ['simple-1', false, null, false, $childProduct], + ['configurable-1', false, null, false, $parentProduct], + ]); + + $cartItemData = [ + [ + 'sku' => 'simple-1', + 'parent_sku' => 'configurable-1', + 'quantity' => 2, + 'selected_options' => ['existing-option'], + ] + ]; + + $expected = [ + [ + 'sku' => 'configurable-1', + 'quantity' => 2, + 'selected_options' => [base64_encode('configurable/1/1'), 'existing-option'], + 'entered_options' => [], + 'parent_sku' => null + ] + ]; + + $result = $this->precursor->process($cartItemData, $this->contextMock); + + $this->assertEquals($expected, $result); + $this->assertEmpty($this->precursor->getErrors()); + } + + /** + * Test processing with non-configurable parent product + * + * @return void + * @throws Exception + */ + public function testProcessNonConfigurableParent(): void + { + $childProduct = $this->getMockProduct('simple-1'); + $parentProduct = $this->getMockProduct('simple-parent', 'simple'); + + $this->productRepositoryMock->expects($this->exactly(2)) + ->method('get') + ->willReturnMap([ + ['simple-1', false, null, false, $childProduct], + ['simple-parent', false, null, false, $parentProduct], + ]); + + $cartItemData = [ + [ + 'sku' => 'simple-1', + 'parent_sku' => 'simple-parent', + 'quantity' => 1 + ] + ]; + + $result = $this->precursor->process($cartItemData, $this->contextMock); + + $this->assertEquals($cartItemData, $result); + $this->assertCount(1, $this->precursor->getErrors()); + $this->assertStringContainsString('not a configurable product', $this->precursor->getErrors()[0]['message']); + } + + /** + * Test processing with no matching attributes + * + * @return void + * @throws Exception + */ + public function testProcessNoMatchingAttributes(): void + { + $childProduct = $this->getMockProduct('simple-1'); + $childProduct->method('getData') + ->willReturn([]); // Empty array to represent no matching attributes + + $productAttributeMock = $this->createMock(AbstractAttribute::class); + $productAttributeMock->method('getAttributeId')->willReturn('1'); + $productAttributeMock->method('getAttributeCode')->willReturn('color'); + + $attributeMock = $this->getMockBuilder(Attribute::class) + ->disableOriginalConstructor() + ->addMethods(['getProductAttribute']) + ->getMock(); + $attributeMock->method('getProductAttribute')->willReturn($productAttributeMock); + + $attributesCollection = [$attributeMock]; + + $configurableTypeMock = $this->createMock(Configurable::class); + $configurableTypeMock->method('getConfigurableAttributes')->willReturn($attributesCollection); + + $parentProduct = $this->getMockProduct('configurable-1', Configurable::TYPE_CODE); + $parentProduct->method('getTypeInstance')->willReturn($configurableTypeMock); + + $this->productRepositoryMock->expects($this->exactly(2)) + ->method('get') + ->willReturnMap([ + ['simple-1', false, null, false, $childProduct], + ['configurable-1', false, null, false, $parentProduct], + ]); + + $cartItemData = [ + [ + 'sku' => 'simple-1', + 'parent_sku' => 'configurable-1', + 'quantity' => 1 + ] + ]; + + $result = $this->precursor->process($cartItemData, $this->contextMock); + + $this->assertEquals($cartItemData, $result); + $this->assertCount(1, $this->precursor->getErrors()); + $this->assertStringContainsString('Could not match child product', $this->precursor->getErrors()[0]['message']); + } + + /** + * Test processing with product not found exception + * + * @return void + */ + public function testProcessProductNotFound(): void + { + $this->productRepositoryMock->method('get') + ->willThrowException(new NoSuchEntityException(__('Product not found'))); + + $cartItemData = [ + [ + 'sku' => 'unknown', + 'parent_sku' => 'unknown-parent', + 'quantity' => 1 + ] + ]; + + $result = $this->precursor->process($cartItemData, $this->contextMock); + + $this->assertEquals($cartItemData, $result); + $this->assertCount(1, $this->precursor->getErrors()); + $this->assertStringContainsString('Product not found', $this->precursor->getErrors()[0]['message']); + } + + /** + * Create mock product + * + * @param string $sku Product SKU + * @param string $typeId Product type ID + * @return ProductInterface + * @throws Exception + */ + private function getMockProduct(string $sku, string $typeId = 'simple'): ProductInterface + { + $product = $this->getMockBuilder(ProductInterface::class) + ->addMethods(['getData', 'getTypeInstance']) + ->onlyMethods(['getSku', 'getTypeId', 'getId']) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + $product->method('getSku')->willReturn($sku); + $product->method('getTypeId')->willReturn($typeId); + $product->method('getId')->willReturn(rand(1, 100)); + return $product; + } +}