From 3bce9488e1f04b1bde0cd7d072c07e6d85214fa3 Mon Sep 17 00:00:00 2001 From: Mohamed El Mrabet Date: Wed, 15 Apr 2026 05:05:32 +0100 Subject: [PATCH 01/10] Fix GH-40598: addProductsToCart ignores parent_sku for configurable products Two root causes addressed: 1. QuoteConfigurableOptions\SuperAttributeDataProvider - Add parent_sku fallback: when selected_options contain no configurable UIDs, resolve super_attribute data from parent_sku + child sku using ConfigurableProductGraphQl OptionCollection (mirrors the existing logic in ConfigurableProductGraphQl\SuperAttributeDataProvider). - Inject ProductRepositoryInterface, OptionCollection and MetadataPool. - Add module.xml sequence dependency on Magento_ConfigurableProductGraphQl and declare the new composer requirements. 2. Quote\Model\Cart\AddProductsToCart::addItemToCart - When parent_sku is present, load/clone the parent configurable product instead of the child simple product so that Quote::addProduct() invokes the Configurable type instance, which correctly processes super_attribute. - Pre-load parent SKUs in addItemsToCart to keep a single DB round-trip. - Use the child product SKU only for stock quantity display. Tests: 8 unit tests added for SuperAttributeDataProvider covering selected_options path, parent_sku fallback, priority rules, error cases. --- .../Quote/Model/Cart/AddProductsToCart.php | 36 ++- .../BuyRequest/SuperAttributeDataProvider.php | 100 ++++++- .../SuperAttributeDataProviderTest.php | 280 ++++++++++++++++++ .../QuoteConfigurableOptions/composer.json | 2 + .../QuoteConfigurableOptions/etc/module.xml | 7 +- 5 files changed, 404 insertions(+), 21 deletions(-) create mode 100644 app/code/Magento/QuoteConfigurableOptions/Test/Unit/Model/Cart/BuyRequest/SuperAttributeDataProviderTest.php diff --git a/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php b/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php index 7fa7dd961ca5d..d549cc87cb409 100644 --- a/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php +++ b/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php @@ -92,20 +92,24 @@ public function execute(string $maskedCartId, array $cartItems): AddProductsToCa public function addItemsToCart(Quote $cart, array $cartItems): array { $failedCartItems = []; - // add new cart items for preload - $skus = \array_map( - function ($item) { - return $item->getSku(); - }, - $cartItems - ); - $this->productReader->loadProducts($skus, $cart->getStoreId()); + + // Collect all SKUs to pre-load: child SKUs + parent SKUs when present (GH-40598) + $skus = []; + foreach ($cartItems as $cartItem) { + $skus[] = $cartItem->getSku(); + if ($cartItem->getParentSku() !== null) { + $skus[] = $cartItem->getParentSku(); + } + } + $this->productReader->loadProducts(array_unique($skus), $cart->getStoreId()); + foreach ($cartItems as $cartItemPosition => $cartItem) { - $product = $this->productReader->getProductBySku($cartItem->getSku()); + // Use the child product for stock quantity display, regardless of which product is added + $childProduct = $this->productReader->getProductBySku($cartItem->getSku()); $stockItemQuantity = 0.0; - if ($product) { + if ($childProduct) { $stockItem = $this->stockRegistry->getStockItem( - $product->getId(), + $childProduct->getId(), $cart->getStore()->getWebsiteId() ); $stockItemQuantity = $stockItem->getQty() - $stockItem->getMinQty(); @@ -146,13 +150,19 @@ private function addItemToCart( ); } - $productBySku = $this->productReader->getProductBySku($sku); + /* + * GH-40598: When parent_sku is provided the buy request must be processed by the Configurable + * type instance (on the parent product), not by the Simple type instance (on the child). + * We therefore load/clone the parent product when available, falling back to the child SKU. + */ + $resolvedSku = $cartItem->getParentSku() ?? $sku; + $productBySku = $this->productReader->getProductBySku($resolvedSku); $product = isset($productBySku) ? clone $productBySku : null; if (!$product || !$product->isSaleable() || !$product->isAvailable()) { return [ $this->error->create( - __('Could not find a product with SKU "%sku"', ['sku' => $sku])->render(), + __('Could not find a product with SKU "%sku"', ['sku' => $resolvedSku])->render(), $cartItemPosition, $stockItemQuantity ) diff --git a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php index 5254b9d0236a7..ae46869c579e0 100644 --- a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php +++ b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php @@ -7,26 +7,69 @@ namespace Magento\QuoteConfigurableOptions\Model\Cart\BuyRequest; +use Magento\Catalog\Api\Data\ProductInterface; +use Magento\Catalog\Api\ProductRepositoryInterface; +use Magento\ConfigurableProductGraphQl\Model\Options\Collection as OptionCollection; +use Magento\Framework\EntityManager\MetadataPool; use Magento\Framework\Exception\LocalizedException; +use Magento\Framework\Exception\NoSuchEntityException; use Magento\Quote\Model\Cart\BuyRequest\BuyRequestDataProviderInterface; use Magento\Quote\Model\Cart\Data\CartItem; /** * DataProvider for building super attribute options in buy requests + * + * Supports two resolution strategies for configurable products: + * 1. Via selected_options (base64-encoded UIDs) — used by addProductsToCart with selected_options. + * 2. Via parent_sku fallback — used by addProductsToCart with parent_sku + child sku (GH-40598). */ class SuperAttributeDataProvider implements BuyRequestDataProviderInterface { private const OPTION_TYPE = 'configurable'; + /** + * @param ProductRepositoryInterface $productRepository + * @param OptionCollection $optionCollection + * @param MetadataPool $metadataPool + */ + public function __construct( + private readonly ProductRepositoryInterface $productRepository, + private readonly OptionCollection $optionCollection, + private readonly MetadataPool $metadataPool, + ) { + } + /** * @inheritdoc * * @throws LocalizedException */ public function execute(CartItem $cartItem): array + { + $configurableProductData = $this->resolveFromSelectedOptions($cartItem); + + // Fallback: if no configurable UIDs found in selected_options, try resolving via parent_sku + if (empty($configurableProductData) && $cartItem->getParentSku() !== null) { + $configurableProductData = $this->resolveFromParentSku( + $cartItem->getParentSku(), + $cartItem->getSku() + ); + } + + return ['super_attribute' => $configurableProductData]; + } + + /** + * Resolves super_attribute data from base64-encoded selected_options UIDs. + * + * @param CartItem $cartItem + * @return array + * @throws LocalizedException + */ + private function resolveFromSelectedOptions(CartItem $cartItem): array { $configurableProductData = []; - foreach ($cartItem->getSelectedOptions() as $optionData) { + foreach ($cartItem->getSelectedOptions() ?? [] as $optionData) { // phpcs:ignore Magento2.Functions.DiscouragedFunction $optionData = \explode('/', base64_decode($optionData->getId())); @@ -41,7 +84,54 @@ public function execute(CartItem $cartItem): array } } - return ['super_attribute' => $configurableProductData]; + return $configurableProductData; + } + + /** + * Resolves super_attribute data from parent_sku + child sku. + * + * Mirrors the logic from ConfigurableProductGraphQl\SuperAttributeDataProvider, + * enabling addProductsToCart to honour the documented parent_sku field. + * + * @param string $parentSku + * @param string $childSku + * @return array + * @throws LocalizedException + */ + private function resolveFromParentSku(string $parentSku, string $childSku): array + { + try { + $parentProduct = $this->productRepository->get($parentSku); + $childProduct = $this->productRepository->get($childSku); + } catch (NoSuchEntityException) { + throw new LocalizedException(__('Could not find a product with SKU "%1" or "%2".', $parentSku, $childSku)); + } + + $configurableLinks = $parentProduct->getExtensionAttributes()?->getConfigurableProductLinks() ?? []; + if (!in_array($childProduct->getId(), $configurableLinks, strict: true)) { + throw new LocalizedException( + __('The product "%1" is not a variant of "%2".', $childSku, $parentSku) + ); + } + + $linkField = $this->metadataPool->getMetadata(ProductInterface::class)->getLinkField(); + $parentLinkId = (int) $parentProduct->getData($linkField); + + $this->optionCollection->addProductId($parentLinkId); + $options = $this->optionCollection->getAttributesByProductId($parentLinkId); + + $superAttributesData = []; + foreach ($options as $option) { + $code = $option['attribute_code']; + foreach ($option['values'] as $optionValue) { + if ($optionValue['value_index'] === $childProduct->getData($code)) { + $superAttributesData[$option['attribute_id']] = $optionValue['value_index']; + break; + } + } + } + + return $superAttributesData; } /** @@ -52,11 +142,7 @@ public function execute(CartItem $cartItem): array */ private function isProviderApplicable(array $optionData): bool { - if ($optionData[0] !== self::OPTION_TYPE) { - return false; - } - - return true; + return ($optionData[0] ?? null) === self::OPTION_TYPE; } /** diff --git a/app/code/Magento/QuoteConfigurableOptions/Test/Unit/Model/Cart/BuyRequest/SuperAttributeDataProviderTest.php b/app/code/Magento/QuoteConfigurableOptions/Test/Unit/Model/Cart/BuyRequest/SuperAttributeDataProviderTest.php new file mode 100644 index 0000000000000..397ba029065e0 --- /dev/null +++ b/app/code/Magento/QuoteConfigurableOptions/Test/Unit/Model/Cart/BuyRequest/SuperAttributeDataProviderTest.php @@ -0,0 +1,280 @@ +productRepository = $this->createMock(ProductRepositoryInterface::class); + $this->optionCollection = $this->createMock(OptionCollection::class); + $this->metadataPool = $this->createMock(MetadataPool::class); + + $this->provider = new SuperAttributeDataProvider( + $this->productRepository, + $this->optionCollection, + $this->metadataPool, + ); + } + + // ------------------------------------------------------------------------- + // selected_options path (existing behaviour) + // ------------------------------------------------------------------------- + + /** + * When selected_options contain a valid configurable UID, super_attribute is built from them. + */ + public function testExecuteResolvesFromSelectedOptions(): void + { + // configurable/93/57 -> attributeId=93, valueIndex=57 + $uid = base64_encode('configurable/93/57'); + + $selectedOption = $this->createMock(SelectedOption::class); + $selectedOption->method('getId')->willReturn($uid); + + $cartItem = new CartItem('simple-child', 1.0, null, [$selectedOption]); + + $result = $this->provider->execute($cartItem); + + $this->assertSame(['super_attribute' => ['93' => '57']], $result); + } + + /** + * Non-configurable selected_options (e.g. custom-options) are ignored. + */ + public function testExecuteIgnoresNonConfigurableSelectedOptions(): void + { + $uid = base64_encode('custom-option/10/20'); + + $selectedOption = $this->createMock(SelectedOption::class); + $selectedOption->method('getId')->willReturn($uid); + + $cartItem = new CartItem('simple-child', 1.0, null, [$selectedOption]); + + $result = $this->provider->execute($cartItem); + + $this->assertSame(['super_attribute' => []], $result); + } + + /** + * When no selected_options are provided but parent_sku is set, the provider + * falls back to resolving super_attribute via parent_sku + child sku (GH-40598). + */ + public function testExecuteFallsBackToParentSkuWhenNoSelectedOptions(): void + { + $cartItem = new CartItem('ParentItem-Variant', 1.0, 'ParentItem'); + + [$parentMock, $childMock] = $this->buildProductMocks( + parentId: 10, + childId: 42, + configurableLinks: [42], + linkFieldValue: 10, + options: [ + [ + 'attribute_code' => 'color', + 'attribute_id' => 93, + 'values' => [['value_index' => 5]], + ], + ], + childAttributeValue: 5 + ); + + $this->productRepository->method('get')->willReturnMap([ + ['ParentItem', false, null, false, $parentMock], + ['ParentItem-Variant', false, null, false, $childMock], + ]); + + $result = $this->provider->execute($cartItem); + + $this->assertSame(['super_attribute' => [93 => 5]], $result); + } + + /** + * selected_options take priority over parent_sku: if configurable UIDs are present + * the parent_sku fallback must NOT be triggered. + */ + public function testSelectedOptionsTakePriorityOverParentSku(): void + { + $uid = base64_encode('configurable/93/57'); + + $selectedOption = $this->createMock(SelectedOption::class); + $selectedOption->method('getId')->willReturn($uid); + + $cartItem = new CartItem('ParentItem-Variant', 1.0, 'ParentItem', [$selectedOption]); + + // Repository should never be called when selected_options already contain the data + $this->productRepository->expects($this->never())->method('get'); + + $result = $this->provider->execute($cartItem); + + $this->assertSame(['super_attribute' => ['93' => '57']], $result); + } + + /** + * GH-40598: When neither selected_options nor parent_sku are provided, the provider + * returns an empty super_attribute array (simple product behaviour unchanged). + */ + public function testExecuteReturnsEmptyWhenNoSelectedOptionsAndNoParentSku(): void + { + $cartItem = new CartItem('simple-standalone', 1.0); + + $this->productRepository->expects($this->never())->method('get'); + + $result = $this->provider->execute($cartItem); + + $this->assertSame(['super_attribute' => []], $result); + } + + /** + * GH-40598: A LocalizedException is thrown when the child SKU is not a variant + * of the supplied parent_sku. + */ + public function testExecuteThrowsWhenChildIsNotVariantOfParent(): void + { + $cartItem = new CartItem('other-simple', 1.0, 'ParentItem'); + + // getConfigurableProductLinks() is a generated extension attribute method — + // we must use createPartialMockWithReflection to be able to stub it. + $extensionAttributes = $this->createPartialMockWithReflection( + ProductExtensionInterface::class, + ['getConfigurableProductLinks'] + ); + $extensionAttributes->method('getConfigurableProductLinks')->willReturn([99]); // child id 42 not in list + + $parentMock = $this->createMock(Product::class); + $parentMock->method('getId')->willReturn(10); + $parentMock->method('getExtensionAttributes')->willReturn($extensionAttributes); + $parentMock->method('getData')->willReturn(10); + + $childMock = $this->createMock(Product::class); + $childMock->method('getId')->willReturn(42); // 42 != 99 + + $this->productRepository->method('get')->willReturnMap([ + ['ParentItem', false, null, false, $parentMock], + ['other-simple', false, null, false, $childMock], + ]); + + $this->expectException(LocalizedException::class); + $this->expectExceptionMessage('not a variant'); + + $this->provider->execute($cartItem); + } + + /** + * GH-40598: A LocalizedException is thrown when the parent or child product does not exist. + */ + public function testExecuteThrowsWhenProductNotFound(): void + { + $cartItem = new CartItem('ParentItem-Variant', 1.0, 'NonExistentParent'); + + $this->productRepository->method('get') + ->willThrowException(new NoSuchEntityException()); + + $this->expectException(LocalizedException::class); + + $this->provider->execute($cartItem); + } + + /** + * GH-40598: A LocalizedException is thrown when a selected_option UID has wrong format. + */ + public function testExecuteThrowsOnMalformedSelectedOptionUid(): void + { + $uid = base64_encode('configurable/only-two-parts'); // should be 3 parts + + $selectedOption = $this->createMock(SelectedOption::class); + $selectedOption->method('getId')->willReturn($uid); + + $cartItem = new CartItem('simple-child', 1.0, null, [$selectedOption]); + + $this->expectException(LocalizedException::class); + + $this->provider->execute($cartItem); + } + + // ------------------------------------------------------------------------- + // Helpers + // ------------------------------------------------------------------------- + + /** + * Build parent & child product mocks for the parent_sku fallback path. + * + * getConfigurableProductLinks() is a generated extension attribute method — + * we must use createPartialMockWithReflection to be able to stub it. + * + * @param int $parentId + * @param int $childId + * @param int[] $configurableLinks + * @param int $linkFieldValue + * @param array $options + * @param mixed $childAttributeValue + * @return array{0: MockObject, 1: MockObject} + */ + private function buildProductMocks( + int $parentId, + int $childId, + array $configurableLinks, + int $linkFieldValue, + array $options, + mixed $childAttributeValue, + ): array { + $extensionAttributes = $this->createPartialMockWithReflection( + ProductExtensionInterface::class, + ['getConfigurableProductLinks'] + ); + $extensionAttributes->method('getConfigurableProductLinks')->willReturn($configurableLinks); + + $parentMock = $this->createMock(Product::class); + $parentMock->method('getId')->willReturn($parentId); + $parentMock->method('getExtensionAttributes')->willReturn($extensionAttributes); + $parentMock->method('getData')->willReturn($linkFieldValue); + + $childMock = $this->createMock(Product::class); + $childMock->method('getId')->willReturn($childId); + $childMock->method('getData')->willReturn($childAttributeValue); + + $productMetadata = $this->createMock(EntityMetadataInterface::class); + $productMetadata->method('getLinkField')->willReturn('entity_id'); + $this->metadataPool->method('getMetadata') + ->with(ProductInterface::class) + ->willReturn($productMetadata); + + $this->optionCollection->method('getAttributesByProductId') + ->with($linkFieldValue) + ->willReturn($options); + + return [$parentMock, $childMock]; + } +} diff --git a/app/code/Magento/QuoteConfigurableOptions/composer.json b/app/code/Magento/QuoteConfigurableOptions/composer.json index 406da1750a579..8d58455bef819 100644 --- a/app/code/Magento/QuoteConfigurableOptions/composer.json +++ b/app/code/Magento/QuoteConfigurableOptions/composer.json @@ -4,6 +4,8 @@ "require": { "php": "~8.3.0||~8.4.0||~8.5.0", "magento/framework": "*", + "magento/module-catalog": "*", + "magento/module-configurable-product-graph-ql": "*", "magento/module-quote": "*" }, "type": "magento2-module", diff --git a/app/code/Magento/QuoteConfigurableOptions/etc/module.xml b/app/code/Magento/QuoteConfigurableOptions/etc/module.xml index 6cd4bfd702422..028cbdb3ac6d6 100644 --- a/app/code/Magento/QuoteConfigurableOptions/etc/module.xml +++ b/app/code/Magento/QuoteConfigurableOptions/etc/module.xml @@ -6,5 +6,10 @@ */ --> - + + + + + + From a8d98fffe6147cf7d097573e3922f3dda900a88e Mon Sep 17 00:00:00 2001 From: Mohamed El Mrabet Date: Wed, 15 Apr 2026 05:12:58 +0100 Subject: [PATCH 02/10] style: remove verbose PHPDoc from GH-40598 fix --- .../BuyRequest/SuperAttributeDataProvider.php | 48 +------- .../SuperAttributeDataProviderTest.php | 115 +++--------------- 2 files changed, 17 insertions(+), 146 deletions(-) diff --git a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php index ae46869c579e0..c737781017ee4 100644 --- a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php +++ b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php @@ -16,22 +16,10 @@ use Magento\Quote\Model\Cart\BuyRequest\BuyRequestDataProviderInterface; use Magento\Quote\Model\Cart\Data\CartItem; -/** - * DataProvider for building super attribute options in buy requests - * - * Supports two resolution strategies for configurable products: - * 1. Via selected_options (base64-encoded UIDs) — used by addProductsToCart with selected_options. - * 2. Via parent_sku fallback — used by addProductsToCart with parent_sku + child sku (GH-40598). - */ class SuperAttributeDataProvider implements BuyRequestDataProviderInterface { private const OPTION_TYPE = 'configurable'; - /** - * @param ProductRepositoryInterface $productRepository - * @param OptionCollection $optionCollection - * @param MetadataPool $metadataPool - */ public function __construct( private readonly ProductRepositoryInterface $productRepository, private readonly OptionCollection $optionCollection, @@ -41,14 +29,11 @@ public function __construct( /** * @inheritdoc - * - * @throws LocalizedException */ public function execute(CartItem $cartItem): array { $configurableProductData = $this->resolveFromSelectedOptions($cartItem); - // Fallback: if no configurable UIDs found in selected_options, try resolving via parent_sku if (empty($configurableProductData) && $cartItem->getParentSku() !== null) { $configurableProductData = $this->resolveFromParentSku( $cartItem->getParentSku(), @@ -59,13 +44,6 @@ public function execute(CartItem $cartItem): array return ['super_attribute' => $configurableProductData]; } - /** - * Resolves super_attribute data from base64-encoded selected_options UIDs. - * - * @param CartItem $cartItem - * @return array - * @throws LocalizedException - */ private function resolveFromSelectedOptions(CartItem $cartItem): array { $configurableProductData = []; @@ -88,15 +66,7 @@ private function resolveFromSelectedOptions(CartItem $cartItem): array } /** - * Resolves super_attribute data from parent_sku + child sku. - * - * Mirrors the logic from ConfigurableProductGraphQl\SuperAttributeDataProvider, - * enabling addProductsToCart to honour the documented parent_sku field. - * - * @param string $parentSku - * @param string $childSku - * @return array - * @throws LocalizedException + * Resolves super_attribute from parent_sku + child sku (GH-40598 fallback). */ private function resolveFromParentSku(string $parentSku, string $childSku): array { @@ -134,29 +104,15 @@ private function resolveFromParentSku(string $parentSku, string $childSku): arra return $superAttributesData; } - /** - * Checks whether this provider is applicable for the current option - * - * @param array $optionData - * @return bool - */ private function isProviderApplicable(array $optionData): bool { return ($optionData[0] ?? null) === self::OPTION_TYPE; } - /** - * Validates the provided options structure - * - * @param array $optionData - * @throws LocalizedException - */ private function validateInput(array $optionData): void { if (count($optionData) !== 3) { - throw new LocalizedException( - __('Wrong format of the entered option data') - ); + throw new LocalizedException(__('Wrong format of the entered option data')); } } } diff --git a/app/code/Magento/QuoteConfigurableOptions/Test/Unit/Model/Cart/BuyRequest/SuperAttributeDataProviderTest.php b/app/code/Magento/QuoteConfigurableOptions/Test/Unit/Model/Cart/BuyRequest/SuperAttributeDataProviderTest.php index 397ba029065e0..84ca6c01fe9c8 100644 --- a/app/code/Magento/QuoteConfigurableOptions/Test/Unit/Model/Cart/BuyRequest/SuperAttributeDataProviderTest.php +++ b/app/code/Magento/QuoteConfigurableOptions/Test/Unit/Model/Cart/BuyRequest/SuperAttributeDataProviderTest.php @@ -49,65 +49,36 @@ protected function setUp(): void ); } - // ------------------------------------------------------------------------- - // selected_options path (existing behaviour) - // ------------------------------------------------------------------------- - - /** - * When selected_options contain a valid configurable UID, super_attribute is built from them. - */ public function testExecuteResolvesFromSelectedOptions(): void { - // configurable/93/57 -> attributeId=93, valueIndex=57 $uid = base64_encode('configurable/93/57'); - $selectedOption = $this->createMock(SelectedOption::class); $selectedOption->method('getId')->willReturn($uid); - $cartItem = new CartItem('simple-child', 1.0, null, [$selectedOption]); - - $result = $this->provider->execute($cartItem); + $result = $this->provider->execute(new CartItem('simple-child', 1.0, null, [$selectedOption])); $this->assertSame(['super_attribute' => ['93' => '57']], $result); } - /** - * Non-configurable selected_options (e.g. custom-options) are ignored. - */ public function testExecuteIgnoresNonConfigurableSelectedOptions(): void { $uid = base64_encode('custom-option/10/20'); - $selectedOption = $this->createMock(SelectedOption::class); $selectedOption->method('getId')->willReturn($uid); - $cartItem = new CartItem('simple-child', 1.0, null, [$selectedOption]); - - $result = $this->provider->execute($cartItem); + $result = $this->provider->execute(new CartItem('simple-child', 1.0, null, [$selectedOption])); $this->assertSame(['super_attribute' => []], $result); } - /** - * When no selected_options are provided but parent_sku is set, the provider - * falls back to resolving super_attribute via parent_sku + child sku (GH-40598). - */ public function testExecuteFallsBackToParentSkuWhenNoSelectedOptions(): void { - $cartItem = new CartItem('ParentItem-Variant', 1.0, 'ParentItem'); - [$parentMock, $childMock] = $this->buildProductMocks( parentId: 10, childId: 42, configurableLinks: [42], linkFieldValue: 10, - options: [ - [ - 'attribute_code' => 'color', - 'attribute_id' => 93, - 'values' => [['value_index' => 5]], - ], - ], + options: [['attribute_code' => 'color', 'attribute_id' => 93, 'values' => [['value_index' => 5]]]], childAttributeValue: 5 ); @@ -116,62 +87,40 @@ public function testExecuteFallsBackToParentSkuWhenNoSelectedOptions(): void ['ParentItem-Variant', false, null, false, $childMock], ]); - $result = $this->provider->execute($cartItem); + $result = $this->provider->execute(new CartItem('ParentItem-Variant', 1.0, 'ParentItem')); $this->assertSame(['super_attribute' => [93 => 5]], $result); } - /** - * selected_options take priority over parent_sku: if configurable UIDs are present - * the parent_sku fallback must NOT be triggered. - */ public function testSelectedOptionsTakePriorityOverParentSku(): void { $uid = base64_encode('configurable/93/57'); - $selectedOption = $this->createMock(SelectedOption::class); $selectedOption->method('getId')->willReturn($uid); - $cartItem = new CartItem('ParentItem-Variant', 1.0, 'ParentItem', [$selectedOption]); - - // Repository should never be called when selected_options already contain the data $this->productRepository->expects($this->never())->method('get'); - $result = $this->provider->execute($cartItem); + $result = $this->provider->execute(new CartItem('ParentItem-Variant', 1.0, 'ParentItem', [$selectedOption])); $this->assertSame(['super_attribute' => ['93' => '57']], $result); } - /** - * GH-40598: When neither selected_options nor parent_sku are provided, the provider - * returns an empty super_attribute array (simple product behaviour unchanged). - */ public function testExecuteReturnsEmptyWhenNoSelectedOptionsAndNoParentSku(): void { - $cartItem = new CartItem('simple-standalone', 1.0); - $this->productRepository->expects($this->never())->method('get'); - $result = $this->provider->execute($cartItem); + $result = $this->provider->execute(new CartItem('simple-standalone', 1.0)); $this->assertSame(['super_attribute' => []], $result); } - /** - * GH-40598: A LocalizedException is thrown when the child SKU is not a variant - * of the supplied parent_sku. - */ public function testExecuteThrowsWhenChildIsNotVariantOfParent(): void { - $cartItem = new CartItem('other-simple', 1.0, 'ParentItem'); - - // getConfigurableProductLinks() is a generated extension attribute method — - // we must use createPartialMockWithReflection to be able to stub it. $extensionAttributes = $this->createPartialMockWithReflection( ProductExtensionInterface::class, ['getConfigurableProductLinks'] ); - $extensionAttributes->method('getConfigurableProductLinks')->willReturn([99]); // child id 42 not in list + $extensionAttributes->method('getConfigurableProductLinks')->willReturn([99]); $parentMock = $this->createMock(Product::class); $parentMock->method('getId')->willReturn(10); @@ -179,7 +128,7 @@ public function testExecuteThrowsWhenChildIsNotVariantOfParent(): void $parentMock->method('getData')->willReturn(10); $childMock = $this->createMock(Product::class); - $childMock->method('getId')->willReturn(42); // 42 != 99 + $childMock->method('getId')->willReturn(42); $this->productRepository->method('get')->willReturnMap([ ['ParentItem', false, null, false, $parentMock], @@ -189,59 +138,29 @@ public function testExecuteThrowsWhenChildIsNotVariantOfParent(): void $this->expectException(LocalizedException::class); $this->expectExceptionMessage('not a variant'); - $this->provider->execute($cartItem); + $this->provider->execute(new CartItem('other-simple', 1.0, 'ParentItem')); } - /** - * GH-40598: A LocalizedException is thrown when the parent or child product does not exist. - */ public function testExecuteThrowsWhenProductNotFound(): void { - $cartItem = new CartItem('ParentItem-Variant', 1.0, 'NonExistentParent'); - - $this->productRepository->method('get') - ->willThrowException(new NoSuchEntityException()); + $this->productRepository->method('get')->willThrowException(new NoSuchEntityException()); $this->expectException(LocalizedException::class); - $this->provider->execute($cartItem); + $this->provider->execute(new CartItem('ParentItem-Variant', 1.0, 'NonExistentParent')); } - /** - * GH-40598: A LocalizedException is thrown when a selected_option UID has wrong format. - */ public function testExecuteThrowsOnMalformedSelectedOptionUid(): void { - $uid = base64_encode('configurable/only-two-parts'); // should be 3 parts - + $uid = base64_encode('configurable/only-two-parts'); $selectedOption = $this->createMock(SelectedOption::class); $selectedOption->method('getId')->willReturn($uid); - $cartItem = new CartItem('simple-child', 1.0, null, [$selectedOption]); - $this->expectException(LocalizedException::class); - $this->provider->execute($cartItem); + $this->provider->execute(new CartItem('simple-child', 1.0, null, [$selectedOption])); } - // ------------------------------------------------------------------------- - // Helpers - // ------------------------------------------------------------------------- - - /** - * Build parent & child product mocks for the parent_sku fallback path. - * - * getConfigurableProductLinks() is a generated extension attribute method — - * we must use createPartialMockWithReflection to be able to stub it. - * - * @param int $parentId - * @param int $childId - * @param int[] $configurableLinks - * @param int $linkFieldValue - * @param array $options - * @param mixed $childAttributeValue - * @return array{0: MockObject, 1: MockObject} - */ private function buildProductMocks( int $parentId, int $childId, @@ -267,13 +186,9 @@ private function buildProductMocks( $productMetadata = $this->createMock(EntityMetadataInterface::class); $productMetadata->method('getLinkField')->willReturn('entity_id'); - $this->metadataPool->method('getMetadata') - ->with(ProductInterface::class) - ->willReturn($productMetadata); + $this->metadataPool->method('getMetadata')->with(ProductInterface::class)->willReturn($productMetadata); - $this->optionCollection->method('getAttributesByProductId') - ->with($linkFieldValue) - ->willReturn($options); + $this->optionCollection->method('getAttributesByProductId')->with($linkFieldValue)->willReturn($options); return [$parentMock, $childMock]; } From c9afbc756cc0a707b96608a031d697a9ed05bfae Mon Sep 17 00:00:00 2001 From: Mohamed El Mrabet Date: Wed, 15 Apr 2026 05:13:26 +0100 Subject: [PATCH 03/10] style: remove block comment in AddProductsToCart --- app/code/Magento/Quote/Model/Cart/AddProductsToCart.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php b/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php index d549cc87cb409..8b4cd09036d8b 100644 --- a/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php +++ b/app/code/Magento/Quote/Model/Cart/AddProductsToCart.php @@ -150,11 +150,6 @@ private function addItemToCart( ); } - /* - * GH-40598: When parent_sku is provided the buy request must be processed by the Configurable - * type instance (on the parent product), not by the Simple type instance (on the child). - * We therefore load/clone the parent product when available, falling back to the child SKU. - */ $resolvedSku = $cartItem->getParentSku() ?? $sku; $productBySku = $this->productReader->getProductBySku($resolvedSku); $product = isset($productBySku) ? clone $productBySku : null; From 35d14c24522cb8304fc0356ccabd8aec7510dd56 Mon Sep 17 00:00:00 2001 From: Mohamed El Mrabet Date: Wed, 15 Apr 2026 05:14:15 +0100 Subject: [PATCH 04/10] style: restore class-level docblock in SuperAttributeDataProvider --- .../Model/Cart/BuyRequest/SuperAttributeDataProvider.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php index c737781017ee4..56b1bcd28e5d0 100644 --- a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php +++ b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php @@ -16,6 +16,9 @@ use Magento\Quote\Model\Cart\BuyRequest\BuyRequestDataProviderInterface; use Magento\Quote\Model\Cart\Data\CartItem; +/** + * DataProvider for building super attribute options in buy requests + */ class SuperAttributeDataProvider implements BuyRequestDataProviderInterface { private const OPTION_TYPE = 'configurable'; From 9d1a1814694d2b50de38550a50a105abddef4486 Mon Sep 17 00:00:00 2001 From: Mohamed El Mrabet Date: Wed, 15 Apr 2026 05:16:08 +0100 Subject: [PATCH 05/10] style: fix copyright year to 2026 on new test file --- .../Model/Cart/BuyRequest/SuperAttributeDataProviderTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/QuoteConfigurableOptions/Test/Unit/Model/Cart/BuyRequest/SuperAttributeDataProviderTest.php b/app/code/Magento/QuoteConfigurableOptions/Test/Unit/Model/Cart/BuyRequest/SuperAttributeDataProviderTest.php index 84ca6c01fe9c8..47ebc17e1f835 100644 --- a/app/code/Magento/QuoteConfigurableOptions/Test/Unit/Model/Cart/BuyRequest/SuperAttributeDataProviderTest.php +++ b/app/code/Magento/QuoteConfigurableOptions/Test/Unit/Model/Cart/BuyRequest/SuperAttributeDataProviderTest.php @@ -1,6 +1,6 @@ Date: Wed, 15 Apr 2026 05:17:03 +0100 Subject: [PATCH 06/10] style: restore validateInput to original form --- .../Cart/BuyRequest/SuperAttributeDataProvider.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php index 56b1bcd28e5d0..6324d1511f150 100644 --- a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php +++ b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php @@ -112,10 +112,18 @@ private function isProviderApplicable(array $optionData): bool return ($optionData[0] ?? null) === self::OPTION_TYPE; } + /** + * Validates the provided options structure + * + * @param array $optionData + * @throws LocalizedException + */ private function validateInput(array $optionData): void { if (count($optionData) !== 3) { - throw new LocalizedException(__('Wrong format of the entered option data')); + throw new LocalizedException( + __('Wrong format of the entered option data') + ); } } } From 847ea11fc1dce082de62e0e75e9369809b4d2075 Mon Sep 17 00:00:00 2001 From: Mohamed El Mrabet Date: Wed, 15 Apr 2026 05:18:00 +0100 Subject: [PATCH 07/10] style: restore @throws LocalizedException on execute() --- .../Model/Cart/BuyRequest/SuperAttributeDataProvider.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php index 6324d1511f150..61547eaaa90b9 100644 --- a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php +++ b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php @@ -32,6 +32,8 @@ public function __construct( /** * @inheritdoc + * + * @throws LocalizedException */ public function execute(CartItem $cartItem): array { From c2948def50b3fd900508e7c37dd9be855e5bacf0 Mon Sep 17 00:00:00 2001 From: Mohamed El Mrabet Date: Wed, 15 Apr 2026 05:18:56 +0100 Subject: [PATCH 08/10] refactor: inline resolveFromSelectedOptions into execute() --- .../BuyRequest/SuperAttributeDataProvider.php | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php index 61547eaaa90b9..7a2de00e36931 100644 --- a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php +++ b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php @@ -36,20 +36,6 @@ public function __construct( * @throws LocalizedException */ public function execute(CartItem $cartItem): array - { - $configurableProductData = $this->resolveFromSelectedOptions($cartItem); - - if (empty($configurableProductData) && $cartItem->getParentSku() !== null) { - $configurableProductData = $this->resolveFromParentSku( - $cartItem->getParentSku(), - $cartItem->getSku() - ); - } - - return ['super_attribute' => $configurableProductData]; - } - - private function resolveFromSelectedOptions(CartItem $cartItem): array { $configurableProductData = []; foreach ($cartItem->getSelectedOptions() ?? [] as $optionData) { @@ -67,7 +53,14 @@ private function resolveFromSelectedOptions(CartItem $cartItem): array } } - return $configurableProductData; + if (empty($configurableProductData) && $cartItem->getParentSku() !== null) { + $configurableProductData = $this->resolveFromParentSku( + $cartItem->getParentSku(), + $cartItem->getSku() + ); + } + + return ['super_attribute' => $configurableProductData]; } /** From d8f96f5623e42440a9517679bd61856b69cb7f65 Mon Sep 17 00:00:00 2001 From: Mohamed El Mrabet Date: Wed, 15 Apr 2026 05:21:19 +0100 Subject: [PATCH 09/10] style: restore PHPDoc on isProviderApplicable() --- .../Model/Cart/BuyRequest/SuperAttributeDataProvider.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php index 7a2de00e36931..3d657ee6ad7a3 100644 --- a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php +++ b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php @@ -102,6 +102,12 @@ private function resolveFromParentSku(string $parentSku, string $childSku): arra return $superAttributesData; } + /** + * Checks whether this provider is applicable for the current option + * + * @param array $optionData + * @return bool + */ private function isProviderApplicable(array $optionData): bool { return ($optionData[0] ?? null) === self::OPTION_TYPE; From 298ae7c11304b4841f9205436c3efe15477af857 Mon Sep 17 00:00:00 2001 From: Mohamed El Mrabet Date: Wed, 15 Apr 2026 05:22:28 +0100 Subject: [PATCH 10/10] style: restore original isProviderApplicable() body --- .../Model/Cart/BuyRequest/SuperAttributeDataProvider.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php index 3d657ee6ad7a3..7ce9fe33bb312 100644 --- a/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php +++ b/app/code/Magento/QuoteConfigurableOptions/Model/Cart/BuyRequest/SuperAttributeDataProvider.php @@ -110,7 +110,11 @@ private function resolveFromParentSku(string $parentSku, string $childSku): arra */ private function isProviderApplicable(array $optionData): bool { - return ($optionData[0] ?? null) === self::OPTION_TYPE; + if ($optionData[0] !== self::OPTION_TYPE) { + return false; + } + + return true; } /**