[5.x] Fix variants not saving it the SKU auto format caused duplicates

CHANGELOG.md

@@ -2,8 +2,10 @@
 
 ## Unreleased
 
+- Fixed a bug where variants weren’t being saved for product types with a Variant SKU Format that could cause duplicate SKUs. ([#4249](https://github.com/craftcms/commerce/issues/4249))
 - Fixed a PHP error that occurred when marking an inventory transfer as pending. ([#4267](https://github.com/craftcms/commerce/issues/4267))
 
+
 ## 5.6.1.1 - 2026-03-27
 
 - Fixed a bug where PDF Link Duration didn’t save. ([#4265](https://github.com/craftcms/commerce/issues/4265))

src/elements/Product.php

@@ -14,6 +14,7 @@
 use craft\commerce\base\HasStoreInterface;
 use craft\commerce\base\StoreTrait;
 use craft\commerce\behaviors\CurrencyAttributeBehavior;
+use craft\commerce\db\Table;
 use craft\commerce\elements\actions\CreateDiscount;
 use craft\commerce\elements\actions\CreateSale;
 use craft\commerce\elements\conditions\products\ProductCondition;
@@ -50,6 +51,7 @@
 use craft\helpers\ElementHelper;
 use craft\helpers\Html;
 use craft\helpers\Json;
+use craft\helpers\Sequence;
 use craft\helpers\StringHelper;
 use craft\helpers\UrlHelper;
 use craft\models\FieldLayout;
@@ -1778,6 +1780,34 @@ public function beforeValidate(): bool
                     Craft::error('Craft Commerce could not generate the supplied SKU format: ' . $e->getMessage(), __METHOD__);
                     $variant->sku = '';
                 }
+
+                if ($variant->sku) {
+                    $skuExistsQuery = function(string $sku, ?int $id) {
+                        $query = (new Query())
+                            ->select(['sku'])
+                            ->from(Table::PURCHASABLES)
+                            ->where(['sku' => $sku]);
+
+                        // Make sure it isn't for the purchasable we are currently saving
+                        if ($id) {
+                            $query->andWhere(['not', ['id' => $id]]);
+                        }
+
+                        return $query;
+                    };
+
+                    // Ensure there isn't a clash with an existing SKU when using auto formats
+                    if ($skuExistsQuery($variant->sku, $variant->id)->exists()) {
+                        // If there is a clash, we need to append a number to the end.
+                        $baseSku = $variant->sku;
+                        do {
+                            $seq = Sequence::next('sku::' . $baseSku);
+                            $newSku = $baseSku . '-' . $seq;
+                        } while ($skuExistsQuery($newSku, $variant->id)->exists());
+
+                        $variant->sku = $newSku;
+                    }
+                }
             }
         }
 

src/elements/Variant.php

@@ -42,6 +42,7 @@
 use craft\helpers\Db;
 use craft\helpers\ElementHelper;
 use craft\helpers\Html;
+use craft\helpers\Sequence;
 use craft\helpers\UrlHelper;
 use craft\models\FieldLayout;
 use Throwable;
@@ -725,6 +726,32 @@ public function updateSku(Product $product): void
             $language = Craft::$app->language;
             Craft::$app->language = $this->getSite()->language;
             $this->sku = Craft::$app->getView()->renderObjectTemplate($type->skuFormat, $this);
+
+            $skuExistsQuery = function(string $sku, ?int $id) {
+                $query = (new Query())
+                    ->select(['sku'])
+                    ->from(Table::PURCHASABLES)
+                    ->where(['sku' => $sku]);
+
+                // Make sure it isn't for the purchasable we are currently saving
+                if ($id) {
+                    $query->andWhere(['not', ['id' => $id]]);
+                }
+
+                return $query;
+            };
+
+            // Ensure there isn't a clash with an existing SKU when using auto formats
+            if ($skuExistsQuery($this->getSku(), $this->id)->exists()) {
+                // If there is a clash, we need to append a number to the end.
+                do {
+                    $seq = Sequence::next('sku::' . $this->sku);
+                    $newSku = $this->sku . '-' . $seq;
+                } while ($skuExistsQuery($newSku, $this->id)->exists());
+
+                $this->sku = $newSku;
+            }
+
             Craft::$app->language = $language;
         }
     }

tests/unit/elements/product/ProductTest.php

@@ -11,9 +11,12 @@
 use craft\commerce\db\Table;
 use craft\commerce\elements\Product;
 use craft\commerce\elements\Variant;
+use craft\commerce\Plugin;
 use craft\db\Query;
+use craft\db\Table as CraftTable;
 use craftcommercetests\fixtures\ProductFixture;
 use DateTime;
+use ReflectionClass;
 
 /**
  * ProductTest
@@ -357,4 +360,110 @@ public function testSaveProductAndVariants(): void
         // Remove the product
         \Craft::$app->getElements()->deleteElementById($product->id, Product::class, null, true);
     }
+
+    /**
+     * @group Product
+     */
+    public function testSkuFormatGeneratesSkuWhenEmpty(): void
+    {
+        $this->setProductTypeSkuFormat(2001, 'generated-sku-from-format');
+
+        $product = new Product();
+        $product->title = 'SKU Format Test Product';
+        $product->typeId = 2001;
+        $product->enabled = false;
+
+        $variant = new Variant();
+        $variant->title = 'Test Variant';
+        // SKU intentionally not set — should be generated from skuFormat
+
+        $product->setVariants([$variant]);
+        $product->validate();
+
+        self::assertEquals('generated-sku-from-format', $variant->sku);
+
+        $this->setProductTypeSkuFormat(2001, null);
+    }
+
+    /**
+     * @group Product
+     */
+    public function testSkuFormatDeduplicatesWhenCollisionExists(): void
+    {
+        // Fixture variant 'rad-hood' already exists in the PURCHASABLES table.
+        // Reset the sequence so the suffix is always predictable (-1).
+        $this->resetSkuSequence('rad-hood');
+        $this->setProductTypeSkuFormat(2001, 'rad-hood');
+
+        $product = new Product();
+        $product->title = 'Collision Test Product';
+        $product->typeId = 2001;
+        $product->enabled = false;
+
+        $variant = new Variant();
+        $variant->title = 'Test Variant';
+        // No SKU — format generates 'rad-hood' which collides with the fixture variant
+
+        $product->setVariants([$variant]);
+        $product->validate();
+
+        self::assertEquals('rad-hood-1', $variant->sku);
+
+        $this->setProductTypeSkuFormat(2001, null);
+    }
+
+    /**
+     * @group Product
+     */
+    public function testSkuFormatDeduplicatesMultipleVariantsWithCollision(): void
+    {
+        // Fixture variant 'hct-white' already exists in the PURCHASABLES table.
+        // Reset the sequence so suffixes are always predictable (-1, -2).
+        $this->resetSkuSequence('hct-white');
+        $this->setProductTypeSkuFormat(2001, 'hct-white');
+
+        $product = new Product();
+        $product->title = 'Multi-Variant Collision Test';
+        $product->typeId = 2001;
+        $product->enabled = false;
+
+        $variant1 = new Variant();
+        $variant1->title = 'Variant One';
+
+        $variant2 = new Variant();
+        $variant2->title = 'Variant Two';
+
+        $product->setVariants([$variant1, $variant2]);
+        $product->validate();
+
+        self::assertEquals('hct-white-1', $variant1->sku);
+        self::assertEquals('hct-white-2', $variant2->sku);
+
+        $this->setProductTypeSkuFormat(2001, null);
+    }
+
+    private function resetSkuSequence(string $baseSku): void
+    {
+        \Craft::$app->getDb()->createCommand()
+            ->delete(CraftTable::SEQUENCES, ['name' => 'sku::' . $baseSku])
+            ->execute();
+    }
+
+    private function setProductTypeSkuFormat(int $typeId, ?string $skuFormat): void
+    {
+        $productTypesService = Plugin::getInstance()->getProductTypes();
+        // Ensure types are loaded into cache
+        $productTypesService->getAllProductTypes();
+
+        $reflection = new ReflectionClass($productTypesService);
+        $prop = $reflection->getProperty('_allProductTypes');
+        $prop->setAccessible(true);
+
+        foreach ($prop->getValue($productTypesService) as $type) {
+            if ($type->id === $typeId) {
+                $type->skuFormat = $skuFormat;
+                return;
+            }
+        }
+    }
 }