fix 0024_remove_duplicate_attributes.py (#3989)

* 0024_remove_duplicate_attributes.py handle multiple attribute values with same attribute code

* Use _get_value instead of copying the method

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

@@ -1,19 +1,30 @@
 # Generated by Django 3.2.9 on 2022-01-25 19:01
+import logging
 
 from django.db import migrations
 from django.db.models import CharField, Count, Value
 from django.db.models.functions import Concat
 
+from oscar.core.loading import get_model
+
+# Needed for calling _get_value, the historical model can't be used for that.
+NonHistoricalProductAttributeValue = get_model('catalogue', 'ProductAttributeValue')
+
+
+logger = logging.getLogger(__name__)
+
+
 def remove_duplicate_attributes(apps, schema_editor):
     """
     Removes duplicate attributes that have the same code and product class.
     """
     ProductAttribute = apps.get_model('catalogue', 'ProductAttribute')
+    ProductAttributeValue = apps.get_model('catalogue', 'ProductAttributeValue')
     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(
+    duplicate_attributes = ProductAttribute.objects.filter(product_class__isnull=False).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')
@@ -52,8 +63,32 @@ def remove_duplicate_attributes(apps, schema_editor):
         # 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)
+            for attribute_value in attribute.productattributevalue_set.all():
+                product = attribute_value.product
+
+                # ProductAttributeValue has a unique together constraint on 'product' and 'attribute'.
+                # This means, if the product of the current 'attribute_value' already has a ProductAttributeValue
+                # linked to the 'to_be_used_attribute' attribute, we can't update the attribute on the
+                # 'attribute_value' as this would raise an IntegrityError.
+                to_be_used_attribute_value = to_be_used_attribute.productattributevalue_set.filter(product=product).first()
+                if not to_be_used_attribute_value:
+                    attribute_value.attribute = to_be_used_attribute
+                    attribute_value.save()
+                else:
+                    msg = """Product with ID '%s' had more than one attribute value linked to an attribute
+                    with code '%s'. We've kept the value '%s' and removed the value '%s' as this is the one you
+                    would see in the dashboard when editing the product.
+                    """ % (
+                        product.id,
+                        attribute.code,
+                        NonHistoricalProductAttributeValue._get_value(to_be_used_attribute_value),
+                        NonHistoricalProductAttributeValue._get_value(attribute_value)
+                    )
+                    logger.warning(msg)
+
+            # Once the attribute values have been updated, we can safely remove the attribute instance.
             attribute.delete()
 
 
@@ -66,4 +101,4 @@ class Migration(migrations.Migration):
 
     operations = [
         migrations.RunPython(remove_duplicate_attributes, migrations.RunPython.noop)
-    ]
+    ]

tests/_site/apps/catalogue/migrations/0024_remove_duplicate_attributes.py

@@ -1,19 +1,30 @@
 # Generated by Django 3.2.9 on 2022-01-25 19:01
+import logging
 
 from django.db import migrations
 from django.db.models import CharField, Count, Value
 from django.db.models.functions import Concat
 
+from oscar.core.loading import get_model
+
+# Needed for calling _get_value, the historical model can't be used for that.
+NonHistoricalProductAttributeValue = get_model('catalogue', 'ProductAttributeValue')
+
+
+logger = logging.getLogger(__name__)
+
+
 def remove_duplicate_attributes(apps, schema_editor):
     """
     Removes duplicate attributes that have the same code and product class.
     """
     ProductAttribute = apps.get_model('catalogue', 'ProductAttribute')
+    ProductAttributeValue = apps.get_model('catalogue', 'ProductAttributeValue')
     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(
+    duplicate_attributes = ProductAttribute.objects.filter(product_class__isnull=False).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')
@@ -52,8 +63,32 @@ def remove_duplicate_attributes(apps, schema_editor):
         # 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)
+            for attribute_value in attribute.productattributevalue_set.all():
+                product = attribute_value.product
+
+                # ProductAttributeValue has a unique together constraint on 'product' and 'attribute'.
+                # This means, if the product of the current 'attribute_value' already has a ProductAttributeValue
+                # linked to the 'to_be_used_attribute' attribute, we can't update the attribute on the
+                # 'attribute_value' as this would raise an IntegrityError.
+                to_be_used_attribute_value = to_be_used_attribute.productattributevalue_set.filter(product=product).first()
+                if not to_be_used_attribute_value:
+                    attribute_value.attribute = to_be_used_attribute
+                    attribute_value.save()
+                else:
+                    msg = """Product with ID '%s' had more than one attribute value linked to an attribute
+                    with code '%s'. We've kept the value '%s' and removed the value '%s' as this is the one you
+                    would see in the dashboard when editing the product.
+                    """ % (
+                        product.id,
+                        attribute.code,
+                        NonHistoricalProductAttributeValue._get_value(to_be_used_attribute_value),
+                        NonHistoricalProductAttributeValue._get_value(attribute_value)
+                    )
+                    logger.warning(msg)
+
+            # Once the attribute values have been updated, we can safely remove the attribute instance.
             attribute.delete()
 
 
@@ -66,4 +101,4 @@ class Migration(migrations.Migration):
 
     operations = [
         migrations.RunPython(remove_duplicate_attributes, migrations.RunPython.noop)
-    ]
+    ]