From aeb3d09b9e29205ed7d7d2e6f639975171aa0cc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paulius=20Stuk=C5=A1ys?= Date: Thu, 27 Aug 2026 12:11:27 +0300 Subject: [PATCH] BRD-1225: stop dropping products and variants with an empty SKU Shops do not require a SKU (PrestaShop reference, Shopify sku are optional), so products without one were skipped in sync and missing from search. Product, ProductVariant and ProductBuilder now accept an empty sku. The PrestaShop adapters read a null or missing sku as '' instead of throwing. id stays required. --- src/Adapters/PrestaShopAdapter.php | 2 +- src/Adapters/PrestaShopAdapterV2.php | 5 +- src/Adapters/README.md | 4 +- .../ValueObjects/BulkOperations/Product.php | 15 ---- .../BulkOperations/ProductBuilder.php | 12 +-- .../BulkOperations/ProductVariant.php | 15 ---- tests/Adapters/PrestaShopAdapterTest.php | 71 ++++++++++++++-- tests/Adapters/PrestaShopAdapterV2Test.php | 83 +++++++++++++++++++ .../BulkOperations/ProductBuilderTest.php | 10 +-- .../BulkOperations/ProductTest.php | 44 ++++++---- .../BulkOperations/ProductVariantTest.php | 10 +-- 11 files changed, 192 insertions(+), 79 deletions(-) diff --git a/src/Adapters/PrestaShopAdapter.php b/src/Adapters/PrestaShopAdapter.php index 4ba5003..4d49b93 100644 --- a/src/Adapters/PrestaShopAdapter.php +++ b/src/Adapters/PrestaShopAdapter.php @@ -58,7 +58,7 @@ private function transformProduct(array $product): array { $result = [ 'id' => $this->getRequiredField($product, 'remoteId'), - 'sku' => $this->getRequiredField($product, 'sku'), + 'sku' => (string) ($product['sku'] ?? ''), 'price' => $this->getRequiredField($product, 'price'), 'basePrice' => $this->getRequiredField($product, 'basePrice'), 'priceTaxExcluded' => $this->getRequiredField($product, 'priceTaxExcluded'), diff --git a/src/Adapters/PrestaShopAdapterV2.php b/src/Adapters/PrestaShopAdapterV2.php index 27d0034..ef056df 100644 --- a/src/Adapters/PrestaShopAdapterV2.php +++ b/src/Adapters/PrestaShopAdapterV2.php @@ -81,7 +81,7 @@ public function transform(array $prestaShopData): array public function transformProduct(array $product): Product { $id = $this->getRequiredField($product, 'remoteId'); - $sku = $this->getRequiredField($product, 'sku'); + $sku = (string) ($product['sku'] ?? ''); $pricing = new ProductPricing( $this->extractPrice($product, 'price'), @@ -180,9 +180,6 @@ public function transformVariant(array $variant, string $locale): ProductVariant } $sku = (string) ($variant['sku'] ?? ''); - if ($sku === '') { - throw new ValidationException("Variant 'sku' is required"); - } $pricing = new ProductPricing( $this->extractPrice($variant, 'price'), diff --git a/src/Adapters/README.md b/src/Adapters/README.md index 0923b74..a4f0f02 100644 --- a/src/Adapters/README.md +++ b/src/Adapters/README.md @@ -75,7 +75,7 @@ $result = $adapter->transform($prestaShopData); | PrestaShop Field | BradSearch Field | Notes | | ---------------------- | -------------------------------- | ----------------------- | | `remoteId` | `id` | Required | -| `sku` | `sku` | Required | +| `sku` | `sku` | Optional, `''` if none | | `localizedNames` | `name` (+ locale suffixes) | Multi-locale support | | `brand.localizedNames` | `brand` (+ locale suffixes) | Multi-locale support | | `productUrl` | `productUrl` (+ locale suffixes) | Multi-locale support | @@ -135,7 +135,7 @@ PrestaShop variants are transformed to match BradSearch requirements: The adapter validates input data and throws `ValidationException` for: -- Missing required fields (`remoteId`, `sku`) +- Missing required fields (`remoteId`) - Invalid data structure - Missing product array diff --git a/src/V2/ValueObjects/BulkOperations/Product.php b/src/V2/ValueObjects/BulkOperations/Product.php index d0802d4..660d5ba 100644 --- a/src/V2/ValueObjects/BulkOperations/Product.php +++ b/src/V2/ValueObjects/BulkOperations/Product.php @@ -36,7 +36,6 @@ public function __construct( public array $additionalFields = [] ) { $this->validateId($id); - $this->validateSku($sku); } /** @@ -239,18 +238,4 @@ private function validateId(string $id): void ); } } - - /** - * @throws InvalidArgumentException - */ - private function validateSku(string $sku): void - { - if (trim($sku) === '') { - throw new InvalidArgumentException( - 'The product SKU cannot be empty.', - 'sku', - $sku - ); - } - } } diff --git a/src/V2/ValueObjects/BulkOperations/ProductBuilder.php b/src/V2/ValueObjects/BulkOperations/ProductBuilder.php index fe23aa6..cdc9b53 100644 --- a/src/V2/ValueObjects/BulkOperations/ProductBuilder.php +++ b/src/V2/ValueObjects/BulkOperations/ProductBuilder.php @@ -17,7 +17,7 @@ final class ProductBuilder { private ?string $id = null; - private ?string $sku = null; + private string $sku = ''; private ?ProductPricing $pricing = null; private ?ImageUrl $imageUrl = null; private ?bool $inStock = null; @@ -149,14 +149,6 @@ public function build(): Product ); } - if ($this->sku === null) { - throw new InvalidArgumentException( - 'Product SKU is required.', - 'sku', - null - ); - } - if ($this->pricing === null) { throw new InvalidArgumentException( 'Product pricing is required.', @@ -190,7 +182,7 @@ public function build(): Product public function reset(): self { $this->id = null; - $this->sku = null; + $this->sku = ''; $this->pricing = null; $this->imageUrl = null; $this->inStock = null; diff --git a/src/V2/ValueObjects/BulkOperations/ProductVariant.php b/src/V2/ValueObjects/BulkOperations/ProductVariant.php index 7e7faa2..161c743 100644 --- a/src/V2/ValueObjects/BulkOperations/ProductVariant.php +++ b/src/V2/ValueObjects/BulkOperations/ProductVariant.php @@ -34,7 +34,6 @@ public function __construct( public array $attrs = [] ) { $this->validateId($id); - $this->validateSku($sku); $this->validateProductUrl($productUrl); } @@ -173,20 +172,6 @@ private function validateId(string $id): void } } - /** - * @throws InvalidArgumentException - */ - private function validateSku(string $sku): void - { - if (trim($sku) === '') { - throw new InvalidArgumentException( - 'The variant SKU cannot be empty.', - 'sku', - $sku - ); - } - } - /** * @throws InvalidArgumentException */ diff --git a/tests/Adapters/PrestaShopAdapterTest.php b/tests/Adapters/PrestaShopAdapterTest.php index cd0f0c6..d9c1f43 100644 --- a/tests/Adapters/PrestaShopAdapterTest.php +++ b/tests/Adapters/PrestaShopAdapterTest.php @@ -456,20 +456,79 @@ public function testTransformProductWithMissingSku(): void 'products' => [ [ 'remoteId' => '1807', - // Missing sku + 'price' => '99.99', + 'basePrice' => '99.99', + 'priceTaxExcluded' => '82.64', + 'basePriceTaxExcluded' => '82.64', 'localizedNames' => [ 'en-US' => 'Test Product' - ] + ], + 'categories' => [], + 'variants' => [] ] ] ]; $result = $this->adapter->transform($prestaShopData); - $this->assertCount(0, $result['products']); - $this->assertCount(1, $result['errors']); - $this->assertEquals('transformation_error', $result['errors'][0]['type']); - $this->assertEquals("Required field 'sku' is missing from PrestaShop data", $result['errors'][0]['message']); + $this->assertCount(0, $result['errors']); + $product = $this->getProductFromResult($result); + $this->assertEquals('1807', $product['id']); + $this->assertSame('', $product['sku']); + } + + public function testTransformProductWithNullSku(): void + { + $prestaShopData = [ + 'products' => [ + [ + 'remoteId' => '1807', + 'sku' => null, + 'price' => '99.99', + 'basePrice' => '99.99', + 'priceTaxExcluded' => '82.64', + 'basePriceTaxExcluded' => '82.64', + 'localizedNames' => [ + 'en-US' => 'Test Product' + ], + 'categories' => [], + 'variants' => [] + ] + ] + ]; + + $result = $this->adapter->transform($prestaShopData); + + $this->assertCount(0, $result['errors']); + $product = $this->getProductFromResult($result); + $this->assertSame('', $product['sku']); + } + + public function testTransformProductWithEmptySku(): void + { + $prestaShopData = [ + 'products' => [ + [ + 'remoteId' => '1807', + 'sku' => '', + 'price' => '99.99', + 'basePrice' => '99.99', + 'priceTaxExcluded' => '82.64', + 'basePriceTaxExcluded' => '82.64', + 'localizedNames' => [ + 'en-US' => 'Test Product' + ], + 'categories' => [], + 'variants' => [] + ] + ] + ]; + + $result = $this->adapter->transform($prestaShopData); + + $this->assertCount(0, $result['errors']); + $product = $this->getProductFromResult($result); + $this->assertSame('', $product['sku']); } public function testTransformVariantWithoutRemoteId(): void diff --git a/tests/Adapters/PrestaShopAdapterV2Test.php b/tests/Adapters/PrestaShopAdapterV2Test.php index 60fad47..b993b07 100644 --- a/tests/Adapters/PrestaShopAdapterV2Test.php +++ b/tests/Adapters/PrestaShopAdapterV2Test.php @@ -446,6 +446,63 @@ public function testTransformVariantMissingRemoteId(): void $this->adapter->transformVariant($variant, 'en-US'); } + public function testTransformProductWithNullSku(): void + { + $product = $this->getMinimalProductData('1807', 'SKU-123'); + $product['sku'] = null; + + $result = $this->adapter->transformProduct($product); + + $this->assertEquals('1807', $result->id); + $this->assertSame('', $result->sku); + $this->assertSame('', $result->jsonSerialize()['sku']); + } + + public function testTransformProductWithEmptySku(): void + { + $product = $this->getMinimalProductData('1807', ''); + + $result = $this->adapter->transformProduct($product); + + $this->assertEquals('1807', $result->id); + $this->assertSame('', $result->sku); + $this->assertSame('', $result->jsonSerialize()['sku']); + } + + public function testTransformProductWithMissingSku(): void + { + $product = $this->getMinimalProductData('1807', 'SKU-123'); + unset($product['sku']); + + $result = $this->adapter->transform(['products' => [$product]]); + + $this->assertCount(0, $result['errors']); + $this->assertCount(1, $result['products']); + $this->assertSame('', $result['products'][0]->sku); + } + + public function testTransformVariantWithEmptySku(): void + { + $variant = $this->getMinimalVariantData(); + $variant['sku'] = ''; + + $result = $this->adapter->transformVariant($variant, 'en-US'); + + $this->assertEquals('12345', $result->id); + $this->assertSame('', $result->sku); + } + + public function testTransformVariantWithMissingSku(): void + { + $variant = $this->getMinimalVariantData(); + unset($variant['sku']); + + $result = $this->adapter->transformVariant($variant, 'en-US'); + + $this->assertSame('', $result->sku); + $this->assertSame('', $result->jsonSerialize()['sku']); + } + public function testTransformProductWithMissingRequiredFields(): void { $prestaShopData = [ @@ -1143,6 +1200,32 @@ private function getMinimalValidProduct(): array ]; } + /** + * Helper method to get minimal variant data. + * + * @return array + */ + private function getMinimalVariantData(): array + { + return [ + 'remoteId' => '12345', + 'sku' => 'VARIANT-SKU', + 'price' => 29.99, + 'basePrice' => 39.99, + 'priceTaxExcluded' => 24.79, + 'basePriceTaxExcluded' => 33.05, + 'productUrl' => [ + 'localizedValues' => [ + 'en-US' => 'http://example.com/variant', + ], + ], + 'imageUrl' => [ + 'small' => 'http://example.com/small.jpg', + 'medium' => 'http://example.com/medium.jpg', + ], + ]; + } + /** * Helper method to get minimal product data. * diff --git a/tests/V2/ValueObjects/BulkOperations/ProductBuilderTest.php b/tests/V2/ValueObjects/BulkOperations/ProductBuilderTest.php index b2288b8..1646608 100644 --- a/tests/V2/ValueObjects/BulkOperations/ProductBuilderTest.php +++ b/tests/V2/ValueObjects/BulkOperations/ProductBuilderTest.php @@ -194,18 +194,16 @@ public function testThrowsExceptionForMissingId(): void ->build(); } - public function testThrowsExceptionForMissingSku(): void + public function testBuildWithoutSkuDefaultsToEmptyString(): void { $builder = new ProductBuilder(); - - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Product SKU is required.'); - - $builder + $product = $builder ->id(self::PRODUCT_ID) ->pricing($this->createPricing()) ->imageUrl($this->createImageUrl()) ->build(); + + $this->assertSame('', $product->sku); } public function testThrowsExceptionForMissingPricing(): void diff --git a/tests/V2/ValueObjects/BulkOperations/ProductTest.php b/tests/V2/ValueObjects/BulkOperations/ProductTest.php index 6824647..85eab16 100644 --- a/tests/V2/ValueObjects/BulkOperations/ProductTest.php +++ b/tests/V2/ValueObjects/BulkOperations/ProductTest.php @@ -354,30 +354,45 @@ public function testThrowsExceptionForWhitespaceOnlyId(): void ); } - public function testThrowsExceptionForEmptySku(): void + public function testAcceptsEmptySku(): void { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('The product SKU cannot be empty.'); - - new Product( + $product = new Product( self::PRODUCT_ID, '', $this->createPricing(), $this->createImageUrl() ); + + $this->assertSame('', $product->sku); + $this->assertSame('', $product->jsonSerialize()['sku']); } - public function testThrowsExceptionForWhitespaceOnlySku(): void + public function testAcceptsWhitespaceOnlySkuUnchanged(): void { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('The product SKU cannot be empty.'); - - new Product( + $product = new Product( self::PRODUCT_ID, ' ', $this->createPricing(), $this->createImageUrl() ); + + $this->assertSame(' ', $product->sku); + } + + public function testFromArrayWithoutSkuDefaultsToEmptyString(): void + { + $product = Product::fromArray([ + 'id' => self::PRODUCT_ID, + 'price' => self::PRICE, + 'basePrice' => self::BASE_PRICE, + 'priceTaxExcluded' => self::PRICE_TAX_EXCLUDED, + 'basePriceTaxExcluded' => self::BASE_PRICE_TAX_EXCLUDED, + 'imageUrl' => ['small' => self::SMALL_IMAGE, 'medium' => self::MEDIUM_IMAGE], + ]); + + $this->assertSame(self::PRODUCT_ID, $product->id); + $this->assertSame('', $product->sku); + $this->assertSame('', $product->jsonSerialize()['sku']); } public function testExceptionContainsArgumentName(): void @@ -406,14 +421,13 @@ public function testWithIdValidatesNewValue(): void $product->withId(''); } - public function testWithSkuValidatesNewValue(): void + public function testWithSkuAcceptsEmptyValue(): void { $product = $this->createProduct(); + $newProduct = $product->withSku(''); - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('The product SKU cannot be empty.'); - - $product->withSku(''); + $this->assertSame(self::SKU, $product->sku); + $this->assertSame('', $newProduct->sku); } public function testJsonSerializeMatchesDarboDrabuziaiExample(): void diff --git a/tests/V2/ValueObjects/BulkOperations/ProductVariantTest.php b/tests/V2/ValueObjects/BulkOperations/ProductVariantTest.php index 0c267f3..9f20a5d 100644 --- a/tests/V2/ValueObjects/BulkOperations/ProductVariantTest.php +++ b/tests/V2/ValueObjects/BulkOperations/ProductVariantTest.php @@ -283,18 +283,18 @@ public function testThrowsExceptionForWhitespaceOnlyId(): void ); } - public function testThrowsExceptionForEmptySku(): void + public function testAcceptsEmptySku(): void { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('The variant SKU cannot be empty.'); - - new ProductVariant( + $variant = new ProductVariant( self::VARIANT_ID, '', $this->createPricing(), self::PRODUCT_URL, $this->createImageUrl() ); + + $this->assertSame('', $variant->sku); + $this->assertSame('', $variant->jsonSerialize()['sku']); } public function testThrowsExceptionForEmptyProductUrl(): void