Merge pull request #13407 from saleor/13338-simplify-product-attribut…

…e-relation

Simplify Product - Attribute relation

CHANGELOG.md

@@ -43,6 +43,7 @@ All notable, unreleased changes to this project will be documented in this file.
 - Added validation for timestamp comparison #14025 by @ritanjandawn
 - Page -> Attributes refactor. The goal is to simplify the attribute models. The current attribute model relations are complex and really hard to understand. - #13621
 - `requirements.txt` and `requirements_dev.txt` were dropped in favor of only supporting `poetry` - #14611 by @patrys
+- Change the Attribute - Product relation to decrease code complexity and make it easier to understand the relations - #13407 by @aniav
 
 # 3.17.0
 

saleor/attribute/migrations/0038_remove_assignedproductattribute.py

@@ -0,0 +1,96 @@
+# Generated by Django 3.2.20 on 2023-07-11 16:10
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("attribute", "0037_remove_assignedpageattribute"),
+    ]
+
+    state_operations = [
+        migrations.RemoveField(
+            model_name="attributeproduct",
+            name="assigned_products",
+        ),
+        migrations.RemoveField(
+            model_name="assignedproductattributevalue",
+            name="assignment",
+        ),
+        migrations.DeleteModel(
+            name="AssignedProductAttribute",
+        ),
+        migrations.AlterUniqueTogether(
+            name="assignedproductattributevalue",
+            unique_together={("value", "product")},
+        ),
+        migrations.AlterField(
+            model_name="assignedproductattributevalue",
+            name="product",
+            field=models.ForeignKey(
+                db_index=False,
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name="attributevalues",
+                to="product.product",
+            ),
+        ),
+    ]
+
+    operations = [
+        # Set FKs to allow null values before marking their models/fields deleted
+        migrations.AlterField(
+            model_name="assignedproductattributevalue",
+            name="assignment",
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.SET_NULL,
+                related_name="productvalueassignment",
+                to="attribute.AssignedProductAttribute",
+                null=True,
+            ),
+        ),
+        migrations.AlterField(
+            model_name="assignedproductattribute",
+            name="product",
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.SET_NULL,
+                related_name="attributes",
+                to="product.Product",
+                null=True,
+            ),
+        ),
+        migrations.AlterField(
+            model_name="assignedproductattribute",
+            name="assignment",
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.SET_NULL,
+                related_name="productassignments",
+                to="attribute.AttributeProduct",
+                null=True,
+            ),
+        ),
+        # Remove FK constraints so that the DB doesn't try to truncate the references
+        migrations.RunSQL(
+            """
+            ALTER TABLE attribute_assignedproductattribute
+            DROP CONSTRAINT IF EXISTS
+            attribute_assignedpr_assignment_id_f65ddd91_fk_attribute;
+
+
+            ALTER TABLE attribute_assignedproductattribute
+            DROP CONSTRAINT IF EXISTS
+            attribute_assignedpr_product_id_c267932a_fk_product_p;
+
+
+            ALTER TABLE attribute_assignedproductattributevalue
+            DROP CONSTRAINT IF EXISTS
+            attribute_assignedpa_assignment_id_6863be0a_fk_attribute;
+
+            ALTER TABLE attribute_assignedproductattributevalue
+            DROP CONSTRAINT IF EXISTS
+            attribute_assignedproduc_value_id_assignment_id_ad6f5a87_uniq;
+            """,
+            reverse_sql=migrations.RunSQL.noop,
+        ),
+        migrations.SeparateDatabaseAndState(state_operations=state_operations),
+    ]

saleor/attribute/models/__init__.py

@@ -5,11 +5,7 @@
     AttributeValueTranslation,
 )
 from .page import AssignedPageAttributeValue, AttributePage
-from .product import (
-    AssignedProductAttribute,
-    AssignedProductAttributeValue,
-    AttributeProduct,
-)
+from .product import AssignedProductAttributeValue, AttributeProduct
 from .product_variant import (
     AssignedVariantAttribute,
     AssignedVariantAttributeValue,
@@ -23,7 +19,6 @@
     "AttributeValueTranslation",
     "AssignedPageAttributeValue",
     "AttributePage",
-    "AssignedProductAttribute",
     "AssignedProductAttributeValue",
     "AttributeProduct",
     "AssignedVariantAttribute",

saleor/attribute/models/base.py

@@ -21,6 +21,8 @@
 
 
 class BaseAssignedAttribute(models.Model):
+    # TODO: stop using this class in new code
+    # See: https://github.com/saleor/saleor/issues/12881
     class Meta:
         abstract = True
 
@@ -312,9 +314,7 @@ def get_translation_context(self):
                 elif assigned_product_attribute_value := (
                     attribute_value.productvalueassignment.first()
                 ):
-                    if product_id := (
-                        assigned_product_attribute_value.assignment.product_id
-                    ):
+                    if product_id := assigned_product_attribute_value.product_id:
                         context["product_id"] = product_id
             elif attribute.type == AttributeType.PAGE_TYPE:
                 if assigned_page_attribute_value := (

saleor/attribute/models/product.py

@@ -3,7 +3,7 @@
 
 from ...core.models import SortableModel
 from ...product.models import Product, ProductType
-from .base import AssociatedAttributeManager, AttributeValue, BaseAssignedAttribute
+from .base import AssociatedAttributeManager
 
 
 class AssignedProductAttributeValue(SortableModel):
@@ -12,50 +12,24 @@ class AssignedProductAttributeValue(SortableModel):
         on_delete=models.CASCADE,
         related_name="productvalueassignment",
     )
-    assignment = models.ForeignKey(
-        "AssignedProductAttribute",
-        on_delete=models.CASCADE,
-        related_name="productvalueassignment",
-    )
-
     product = models.ForeignKey(
         Product,
         related_name="attributevalues",
         on_delete=models.CASCADE,
-        null=True,
-        blank=True,
+        null=False,
+        blank=False,
         db_index=False,
     )
 
     class Meta:
-        unique_together = (("value", "assignment"),)
+        unique_together = (("value", "product"),)
         ordering = ("sort_order", "pk")
         indexes = [
             BTreeIndex(fields=["product"], name="assignedprodattrval_product_idx")
         ]
 
     def get_ordering_queryset(self):
-        return self.assignment.productvalueassignment.all()
-
-
-class AssignedProductAttribute(BaseAssignedAttribute):
-    """Associate a product type attribute and selected values to a given product."""
-
-    product = models.ForeignKey(
-        Product, related_name="attributes", on_delete=models.CASCADE
-    )
-    assignment = models.ForeignKey(
-        "AttributeProduct", on_delete=models.CASCADE, related_name="productassignments"
-    )
-    values = models.ManyToManyField(
-        AttributeValue,
-        blank=True,
-        related_name="productassignments",
-        through=AssignedProductAttributeValue,
-    )
-
-    class Meta:
-        unique_together = (("product", "assignment"),)
+        return self.product.attributevalues.all()
 
 
 class AttributeProduct(SortableModel):
@@ -65,13 +39,6 @@ class AttributeProduct(SortableModel):
     product_type = models.ForeignKey(
         ProductType, related_name="attributeproduct", on_delete=models.CASCADE
     )
-    assigned_products = models.ManyToManyField(
-        Product,
-        blank=True,
-        through=AssignedProductAttribute,
-        through_fields=("assignment", "product"),
-        related_name="attributesrelated",
-    )
 
     objects = AssociatedAttributeManager()
 

saleor/attribute/tasks.py

@@ -26,6 +26,6 @@ def update_associated_products_search_vector(attribute_value_pk: int):
         Exists(instance.variantassignments.filter(variant_id=OuterRef("id")))
     )
     Product.objects.filter(
-        Q(Exists(instance.productassignments.filter(product_id=OuterRef("id"))))
+        Q(Exists(instance.productvalueassignment.filter(product_id=OuterRef("id"))))
         | Q(Exists(variants.filter(product_id=OuterRef("id"))))
     ).update(search_index_dirty=True)

saleor/attribute/tests/model_helpers.py

@@ -1,10 +1,13 @@
 from django.db.models.expressions import Exists, OuterRef
 
 from ...page.models import Page
+from ...product.models import Product
 from ..models import (
     AssignedPageAttributeValue,
+    AssignedProductAttributeValue,
     Attribute,
     AttributePage,
+    AttributeProduct,
     AttributeValue,
 )
 
@@ -24,3 +27,36 @@ def get_page_attribute_values(page: Page, attribute: Attribute):
     return values.filter(
         Exists(assigned_values.filter(value_id=OuterRef("id"))),
     ).order_by("pagevalueassignment__sort_order")
+
+
+def get_product_attributes(product: Product):
+    """Get product attributes filtered by product_type.
+
+    ProductType defines which attributes can be assigned to a product and
+    we have to filter out the attributes on the instance by the ones attached to the
+    product_type.
+    """
+    product_attributes = AttributeProduct.objects.filter(
+        product_type_id=product.product_type_id
+    )
+    return Attribute.objects.filter(
+        Exists(product_attributes.filter(attribute_id=OuterRef("id")))
+    ).order_by("attributeproduct__sort_order")
+
+
+def get_product_attribute_values(product: Product, attribute: Attribute):
+    """Get values assigned to a product.
+
+    Note: this doesn't filter out attributes that might have been unassigned from the
+    product type.
+    """
+    assigned_values = AssignedProductAttributeValue.objects.filter(
+        product_id=product.pk
+    )
+
+    return AttributeValue.objects.filter(
+        Exists(
+            assigned_values.filter(value_id=OuterRef("id")),
+        ),
+        attribute_id=attribute.pk,
+    ).order_by("productvalueassignment__sort_order")