Make attribute codes unique per product class (#3823)

src/oscar/apps/catalogue/abstract_models.py

@@ -870,6 +870,7 @@ class AbstractProductAttribute(models.Model):
         ordering = ['code']
         verbose_name = _('Product attribute')
         verbose_name_plural = _('Product attributes')
+        unique_together = ('code', 'product_class')
 
     @property
     def is_option(self):

src/oscar/apps/catalogue/migrations/0024_remove_duplicate_attributes.py

@@ -0,0 +1,69 @@
+# Generated by Django 3.2.9 on 2022-01-25 19:01
+
+from django.db import migrations
+from django.db.models import CharField, Count, Value
+from django.db.models.functions import Concat
+
+def remove_duplicate_attributes(apps, schema_editor):
+    """
+    Removes duplicate attributes that have the same code and product class.
+    """
+    ProductAttribute = apps.get_model('catalogue', 'ProductAttribute')
+    ProductClass = apps.get_model("catalogue", "ProductClass")
+
+    # Instead of iterating over all attributes, we concat the code and product class pk
+    # with a "|" so we can find duplicate attributes in one query.
+    duplicate_attributes = ProductAttribute.objects.annotate(
+        code_and_product_class=Concat('code', Value('|'), 'product_class__pk', output_field=CharField())
+    ).values('code_and_product_class').annotate(
+        same_code_count=Count('code_and_product_class')
+    ).filter(same_code_count__gt=1)
+
+    for attribute in duplicate_attributes:
+        attribute_code, product_class_pk = attribute["code_and_product_class"].split("|")
+        product_class = ProductClass.objects.get(pk=product_class_pk)
+        attributes = ProductAttribute.objects.filter(
+            code=attribute_code,
+            product_class=product_class
+        )
+        used_attributes = attributes.filter(productattributevalue__isnull=False)
+        used_attribute_count = used_attributes.distinct().count()
+
+        # In most cases, the used attributes count will be one or zero as
+        # the dashboard will always show one attribute. If the used attribute
+        # count is one, we exclude this from attributes and remove the others.
+        # If it's zero, we pick the last created and delete others.
+        if used_attribute_count == 1:
+            attributes.exclude(pk=used_attributes.first().pk).delete()
+            continue
+        elif used_attribute_count == 0:
+            attributes.exclude(pk=attributes.last().pk).delete()
+            continue
+
+        # If we found multiple attributes that have values linked to them,
+        # we must move them to one attribute and then delete the others.
+        # We can only do this if the value_types are all the same!
+        ASSERTION_MESSAGE = """Duplicate attribute found with code: %s but different types!
+        You could fix this by renaming the duplicate codes or by matching all types to one
+        type and update the attribute values accordingly for their new type. After that you can
+        re-run the migration.""" % attribute_code
+        assert used_attributes.values("type").distinct().count() == 1, ASSERTION_MESSAGE
+
+        # Choose one attribute that will be used to move to and others to be deleted.
+        to_be_used_attribute = used_attributes.first()
+        to_be_deleted_attributes = used_attributes.exclude(pk=to_be_used_attribute.pk)
+        for attribute in to_be_deleted_attributes:
+            attribute.productattributevalue_set.all().update(attribute=to_be_used_attribute)
+            attribute.delete()
+
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('catalogue', '0023_auto_20210824_1414'),
+    ]
+
+    operations = [
+        migrations.RunPython(remove_duplicate_attributes, migrations.RunPython.noop)
+    ]

src/oscar/apps/catalogue/migrations/0025_attribute_code_uniquetogether_constraint.py

@@ -0,0 +1,17 @@
+# Generated by Django 3.2.9 on 2022-01-25 20:17
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('catalogue', '0024_remove_duplicate_attributes'),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name='productattribute',
+            unique_together={('code', 'product_class')},
+        ),
+    ]

tests/integration/catalogue/test_attributes.py

@@ -4,7 +4,7 @@ from django.core.exceptions import ValidationError
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.test import TestCase
 
-from oscar.apps.catalogue.models import Product, ProductAttribute
+from oscar.apps.catalogue.models import Product, ProductAttribute, ProductClass
 from oscar.test import factories
 
 
@@ -44,6 +44,17 @@ class TestContainer(TestCase):
         product.attr.refresh()
         assert product.attr.a1 == "v2"
 
+    def test_attribute_code_uniquenesss(self):
+        product_class = factories.ProductClassFactory()
+        attribute1 = ProductAttribute.objects.create(name='a1', code='a1', product_class=product_class)
+        attribute1.full_clean()
+        attribute2 = ProductAttribute.objects.create(name='a1', code='a1', product_class=product_class)
+        with self.assertRaises(ValidationError):
+            attribute2.full_clean()
+        another_product_class = ProductClass.objects.create(name="another product class")
+        attribute3 = ProductAttribute.objects.create(name='a1', code='a1', product_class=another_product_class)
+        attribute3.full_clean()
+
 
 class TestBooleanAttributes(TestCase):