Skip to content

Commit 0c9b158

Browse files
nfourtythreenfourtythree
authored
Merge pull request #4269 from craftcms/nathaniel/com-540-5x-the-sku-is-not-editable-if-an-automatic-format-is-used
[5.x] Fix variants not saving it the SKU auto format caused duplicates
2 parents 70f6688 + d0d3370 commit 0c9b158

4 files changed

Lines changed: 168 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22

33
## Unreleased
44

5+
- 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))
56
- Fixed a PHP error that occurred when marking an inventory transfer as pending. ([#4267](https://github.com/craftcms/commerce/issues/4267))
67
- Fixed a bug where variants' sort order wasn’t being respected when reordering with disabled variants. ([#4270](https://github.com/craftcms/commerce/issues/4270))
78

9+
810
## 5.6.1.1 - 2026-03-27
911

1012
- Fixed a bug where PDF Link Duration didn’t save. ([#4265](https://github.com/craftcms/commerce/issues/4265))

‎src/elements/Product.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use craft\commerce\base\HasStoreInterface;
1515
use craft\commerce\base\StoreTrait;
1616
use craft\commerce\behaviors\CurrencyAttributeBehavior;
17+
use craft\commerce\db\Table;
1718
use craft\commerce\elements\actions\CreateDiscount;
1819
use craft\commerce\elements\actions\CreateSale;
1920
use craft\commerce\elements\conditions\products\ProductCondition;
@@ -51,6 +52,7 @@
5152
use craft\helpers\ElementHelper;
5253
use craft\helpers\Html;
5354
use craft\helpers\Json;
55+
use craft\helpers\Sequence;
5456
use craft\helpers\StringHelper;
5557
use craft\helpers\UrlHelper;
5658
use craft\models\FieldLayout;
@@ -1783,6 +1785,34 @@ public function beforeValidate(): bool
17831785
Craft::error('Craft Commerce could not generate the supplied SKU format: ' . $e->getMessage(), __METHOD__);
17841786
$variant->sku = '';
17851787
}
1788+
1789+
if ($variant->sku) {
1790+
$skuExistsQuery = function(string $sku, ?int $id) {
1791+
$query = (new Query())
1792+
->select(['sku'])
1793+
->from(Table::PURCHASABLES)
1794+
->where(['sku' => $sku]);
1795+
1796+
// Make sure it isn't for the purchasable we are currently saving
1797+
if ($id) {
1798+
$query->andWhere(['not', ['id' => $id]]);
1799+
}
1800+
1801+
return $query;
1802+
};
1803+
1804+
// Ensure there isn't a clash with an existing SKU when using auto formats
1805+
if ($skuExistsQuery($variant->sku, $variant->id)->exists()) {
1806+
// If there is a clash, we need to append a number to the end.
1807+
$baseSku = $variant->sku;
1808+
do {
1809+
$seq = Sequence::next('sku::' . $baseSku);
1810+
$newSku = $baseSku . '-' . $seq;
1811+
} while ($skuExistsQuery($newSku, $variant->id)->exists());
1812+
1813+
$variant->sku = $newSku;
1814+
}
1815+
}
17861816
}
17871817
}
17881818

‎src/elements/Variant.php‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
use craft\helpers\Db;
4343
use craft\helpers\ElementHelper;
4444
use craft\helpers\Html;
45+
use craft\helpers\Sequence;
4546
use craft\helpers\UrlHelper;
4647
use craft\models\FieldLayout;
4748
use Throwable;
@@ -725,6 +726,32 @@ public function updateSku(Product $product): void
725726
$language = Craft::$app->language;
726727
Craft::$app->language = $this->getSite()->language;
727728
$this->sku = Craft::$app->getView()->renderObjectTemplate($type->skuFormat, $this);
729+
730+
$skuExistsQuery = function(string $sku, ?int $id) {
731+
$query = (new Query())
732+
->select(['sku'])
733+
->from(Table::PURCHASABLES)
734+
->where(['sku' => $sku]);
735+
736+
// Make sure it isn't for the purchasable we are currently saving
737+
if ($id) {
738+
$query->andWhere(['not', ['id' => $id]]);
739+
}
740+
741+
return $query;
742+
};
743+
744+
// Ensure there isn't a clash with an existing SKU when using auto formats
745+
if ($skuExistsQuery($this->getSku(), $this->id)->exists()) {
746+
// If there is a clash, we need to append a number to the end.
747+
do {
748+
$seq = Sequence::next('sku::' . $this->sku);
749+
$newSku = $this->sku . '-' . $seq;
750+
} while ($skuExistsQuery($newSku, $this->id)->exists());
751+
752+
$this->sku = $newSku;
753+
}
754+
728755
Craft::$app->language = $language;
729756
}
730757
}

‎tests/unit/elements/product/ProductTest.php‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,12 @@
1111
use craft\commerce\db\Table;
1212
use craft\commerce\elements\Product;
1313
use craft\commerce\elements\Variant;
14+
use craft\commerce\Plugin;
1415
use craft\db\Query;
16+
use craft\db\Table as CraftTable;
1517
use craftcommercetests\fixtures\ProductFixture;
1618
use DateTime;
19+
use ReflectionClass;
1720

1821
/**
1922
* ProductTest
@@ -357,4 +360,110 @@ public function testSaveProductAndVariants(): void
357360
// Remove the product
358361
\Craft::$app->getElements()->deleteElementById($product->id, Product::class, null, true);
359362
}
363+
364+
/**
365+
* @group Product
366+
*/
367+
public function testSkuFormatGeneratesSkuWhenEmpty(): void
368+
{
369+
$this->setProductTypeSkuFormat(2001, 'generated-sku-from-format');
370+
371+
$product = new Product();
372+
$product->title = 'SKU Format Test Product';
373+
$product->typeId = 2001;
374+
$product->enabled = false;
375+
376+
$variant = new Variant();
377+
$variant->title = 'Test Variant';
378+
// SKU intentionally not set — should be generated from skuFormat
379+
380+
$product->setVariants([$variant]);
381+
$product->validate();
382+
383+
self::assertEquals('generated-sku-from-format', $variant->sku);
384+
385+
$this->setProductTypeSkuFormat(2001, null);
386+
}
387+
388+
/**
389+
* @group Product
390+
*/
391+
public function testSkuFormatDeduplicatesWhenCollisionExists(): void
392+
{
393+
// Fixture variant 'rad-hood' already exists in the PURCHASABLES table.
394+
// Reset the sequence so the suffix is always predictable (-1).
395+
$this->resetSkuSequence('rad-hood');
396+
$this->setProductTypeSkuFormat(2001, 'rad-hood');
397+
398+
$product = new Product();
399+
$product->title = 'Collision Test Product';
400+
$product->typeId = 2001;
401+
$product->enabled = false;
402+
403+
$variant = new Variant();
404+
$variant->title = 'Test Variant';
405+
// No SKU — format generates 'rad-hood' which collides with the fixture variant
406+
407+
$product->setVariants([$variant]);
408+
$product->validate();
409+
410+
self::assertEquals('rad-hood-1', $variant->sku);
411+
412+
$this->setProductTypeSkuFormat(2001, null);
413+
}
414+
415+
/**
416+
* @group Product
417+
*/
418+
public function testSkuFormatDeduplicatesMultipleVariantsWithCollision(): void
419+
{
420+
// Fixture variant 'hct-white' already exists in the PURCHASABLES table.
421+
// Reset the sequence so suffixes are always predictable (-1, -2).
422+
$this->resetSkuSequence('hct-white');
423+
$this->setProductTypeSkuFormat(2001, 'hct-white');
424+
425+
$product = new Product();
426+
$product->title = 'Multi-Variant Collision Test';
427+
$product->typeId = 2001;
428+
$product->enabled = false;
429+
430+
$variant1 = new Variant();
431+
$variant1->title = 'Variant One';
432+
433+
$variant2 = new Variant();
434+
$variant2->title = 'Variant Two';
435+
436+
$product->setVariants([$variant1, $variant2]);
437+
$product->validate();
438+
439+
self::assertEquals('hct-white-1', $variant1->sku);
440+
self::assertEquals('hct-white-2', $variant2->sku);
441+
442+
$this->setProductTypeSkuFormat(2001, null);
443+
}
444+
445+
private function resetSkuSequence(string $baseSku): void
446+
{
447+
\Craft::$app->getDb()->createCommand()
448+
->delete(CraftTable::SEQUENCES, ['name' => 'sku::' . $baseSku])
449+
->execute();
450+
}
451+
452+
private function setProductTypeSkuFormat(int $typeId, ?string $skuFormat): void
453+
{
454+
$productTypesService = Plugin::getInstance()->getProductTypes();
455+
// Ensure types are loaded into cache
456+
$productTypesService->getAllProductTypes();
457+
458+
$reflection = new ReflectionClass($productTypesService);
459+
$prop = $reflection->getProperty('_allProductTypes');
460+
$prop->setAccessible(true);
461+
462+
foreach ($prop->getValue($productTypesService) as $type) {
463+
if ($type->id === $typeId) {
464+
$type->skuFormat = $skuFormat;
465+
return;
466+
}
467+
}
468+
}
360469
}

0 commit comments

Comments
 (0)