Repository: saleor/saleor
Pull request: Add attribute validation to attributeAssign
closes #5140 
# Impact

* [ ] New migrations
* [ ] New/Updated API fields or mutations
* [ ] Deprecated API fields or mutations
* [ ] Removed API types, fields, or mutations

# Pull Request Checklist

<!-- Please keep this section. It will make maintainer's life easier. -->

* [x] Privileged queries and mutations are guarded by proper permission checks
* [x] Database queries are optimized and the number of queries is constant
* [x] Database migration files are up to date
* [x] The changes are tested
* [x] GraphQL schema and type definitions are up to date
* [x] Changes are mentioned in the changelog


Problem statement:
Assigning a non-existing attribute to a product type crashes

### Steps to reproduce the problem
**Query:**
```gql
mutation AssignAttributesToPT(
  $productTypeID: ID!
  $operations: [AttributeAssignInput]!
) {
  attributeAssign(productTypeId: $productTypeID, operations: $operations) {
    errors {
      field
      message
    }
  }
}
```

**Variables:**
```python
{
  "variables": {
    "productTypeID": "UHJvZHVjdFR5cGU6MTE2",   # <- valid and existing product type ID
    "operations": [
      {
        "id": "QXR0cmlidXRlOjU1NQ==",  # <- valid attribute ID but non-existing
        "type": "PRODUCT"
      }
    ]
  }
}
```

### What I expected to happen
To have a check in the mutation to prevent it from happening.

Hints from issue discussion:

Commit message: Return attributes global id instead of db pk

Patch to review:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 081fb9564a5..e4b6fe6c17d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -88,6 +88,7 @@ All notable, unreleased changes to this project will be documented in this file.
 - Add shipping rate price validation - #5411 by @kswiatek92
 - Remove unused settings and environment variables - #5420 by @maarcingebala
 - Add product price validation - #5413 by @kswiatek92
+- Add attribute validation to attributeAssign - #5423 by @kswiatek92
 
 ## 2.9.0
 
diff --git a/saleor/graphql/core/types/common.py b/saleor/graphql/core/types/common.py
index 23daba25782..4b0e24e2475 100644
--- a/saleor/graphql/core/types/common.py
+++ b/saleor/graphql/core/types/common.py
@@ -73,6 +73,14 @@ class ProductError(Error):
     code = ProductErrorCode(description="The error code.", required=True)
 
 
+class ProductAttributeError(ProductError):
+    attributes = graphene.List(
+        graphene.NonNull(graphene.ID),
+        description="List of attributes IDs which causes the error.",
+        required=False,
+    )
+
+
 class BulkProductError(ProductError):
     index = graphene.Int(
         description="Index of an input list item that caused the error."
diff --git a/saleor/graphql/product/mutations/attributes.py b/saleor/graphql/product/mutations/attributes.py
index 9b5ff1dd63f..cd56c73ebdd 100644
--- a/saleor/graphql/product/mutations/attributes.py
+++ b/saleor/graphql/product/mutations/attributes.py
@@ -10,7 +10,7 @@
 from ....product import AttributeInputType, models
 from ....product.error_codes import ProductErrorCode
 from ...core.mutations import BaseMutation, ModelDeleteMutation, ModelMutation
-from ...core.types.common import ProductError
+from ...core.types.common import ProductAttributeError, ProductError
 from ...core.utils import (
     from_global_id_strict_type,
     validate_slug_and_generate_if_needed,
@@ -300,7 +300,7 @@ class Arguments:
 
     class Meta:
         description = "Assign attributes to a given product type."
-        error_type_class = ProductError
+        error_type_class = ProductAttributeError
         error_type_field = "product_errors"
 
     @classmethod
@@ -382,9 +382,41 @@ def check_product_operations_are_assignable(cls, product_attrs_pks):
                 }
             )
 
+    @classmethod
+    def handle_typed_errors(cls, errors: list, **extra):
+        typed_errors = [
+            cls._meta.error_type_class(  # type: ignore
+                field=e.field,
+                message=e.message,
+                code=code,
+                attributes=params.get("attributes") if params else None,
+            )
+            for e, code, params in errors
+        ]
+        extra.update({cls._meta.error_type_field: typed_errors})  # type: ignore
+        return cls(errors=[e[0] for e in errors], **extra)  # type: ignore
+
     @classmethod
     def clean_operations(cls, product_type, product_attrs_pks, variant_attrs_pks):
         """Ensure the attributes are not already assigned to the product type."""
+        attrs_pk = product_attrs_pks + variant_attrs_pks
+        attributes = models.Attribute.objects.filter(id__in=attrs_pk).values_list(
+            "pk", flat=True
+        )
+        if len(attrs_pk) != len(attributes):
+            invalid_attrs = set(attrs_pk) - set(attributes)
+            invalid_attrs = [
+                graphene.Node.to_global_id("Attribute", pk) for pk in invalid_attrs
+            ]
+            raise ValidationError(
+                {
+                    "operations": ValidationError(
+                        f"Attribute doesn't exist.",
+                        code=ProductErrorCode.NOT_FOUND.value,
+                        params={"attributes": list(invalid_attrs)},
+                    )
+                }
+            )
         cls.check_product_operations_are_assignable(product_attrs_pks)
         cls.check_operations_not_assigned_already(
             product_type, product_attrs_pks, variant_attrs_pks
diff --git a/saleor/graphql/product/resolvers.py b/saleor/graphql/product/resolvers.py
index e48fc6f61f8..e82145826cc 100644
--- a/saleor/graphql/product/resolvers.py
+++ b/saleor/graphql/product/resolvers.py
@@ -13,8 +13,8 @@
     filter_by_period,
     filter_by_query_param,
     get_database_id,
-    get_user_or_service_account_from_context,
     get_nodes,
+    get_user_or_service_account_from_context,
     sort_queryset,
 )
 from .filters import (
diff --git a/saleor/graphql/schema.graphql b/saleor/graphql/schema.graphql
index 64edfb5a781..6de0ee7f52f 100644
--- a/saleor/graphql/schema.graphql
+++ b/saleor/graphql/schema.graphql
@@ -214,7 +214,7 @@ type Attribute implements Node & ObjectWithMetadata {
 type AttributeAssign {
   errors: [Error!]!
   productType: ProductType
-  productErrors: [ProductError!]!
+  productErrors: [ProductAttributeError!]!
 }
 
 input AttributeAssignInput {
@@ -3174,6 +3174,13 @@ type Product implements Node & ObjectWithMetadata {
   translation(languageCode: LanguageCodeEnum!): ProductTranslation
 }
 
+type ProductAttributeError {
+  field: String
+  message: String
+  code: ProductErrorCode!
+  attributes: [ID!]
+}
+
 type ProductBulkDelete {
   errors: [Error!]!
   count: Int!
diff --git a/tests/api/test_attributes.py b/tests/api/test_attributes.py
index 28c94005e1f..98160bb4236 100644
--- a/tests/api/test_attributes.py
+++ b/tests/api/test_attributes.py
@@ -1183,9 +1183,11 @@ def test_resolve_assigned_attribute_without_values(api_client, product_type, pro
 ASSIGN_ATTR_QUERY = """
     mutation assign($productTypeId: ID!, $operations: [AttributeAssignInput]!) {
       attributeAssign(productTypeId: $productTypeId, operations: $operations) {
-        errors {
+        productErrors {
           field
+          code
           message
+          attributes
         }
         productType {
           id
@@ -1229,7 +1231,7 @@ def test_assign_attributes_to_product_type(
             query, variables, permissions=[permission_manage_products]
         )
     )["data"]["attributeAssign"]
-    assert not content["errors"], "Should have succeeded"
+    assert not content["productErrors"], "Should have succeeded"
 
     assert content["productType"]["id"] == product_type_global_id
     assert len(content["productType"]["productAttributes"]) == len(
@@ -1252,6 +1254,27 @@ def test_assign_attributes_to_product_type(
     assert found_variant_attrs_ids == variant_attributes_ids
 
 
+def test_assign_non_existing_attributes_to_product_type(
+    staff_api_client, permission_manage_products, attribute_list
+):
+    product_type = ProductType.objects.create(name="Default Type", has_variants=True)
+    product_type_global_id = graphene.Node.to_global_id("ProductType", product_type.pk)
+
+    query = ASSIGN_ATTR_QUERY
+    attribute_id = graphene.Node.to_global_id("Attribute", "555")
+    operations = [{"type": "PRODUCT", "id": attribute_id}]
+    variables = {"productTypeId": product_type_global_id, "operations": operations}
+
+    response = staff_api_client.post_graphql(
+        query, variables, permissions=[permission_manage_products]
+    )
+    content = get_graphql_content(response)
+    content = content["data"]["attributeAssign"]
+    assert content["productErrors"][0]["code"] == ProductErrorCode.NOT_FOUND.name
+    assert content["productErrors"][0]["field"] == "operations"
+    assert content["productErrors"][0]["attributes"] == [attribute_id]
+
+
 def test_assign_variant_attribute_to_product_type_with_disabled_variants(
     staff_api_client,
     permission_manage_products,
@@ -1277,12 +1300,15 @@ def test_assign_variant_attribute_to_product_type_with_disabled_variants(
     content = get_graphql_content(staff_api_client.post_graphql(query, variables))[
         "data"
     ]["attributeAssign"]
-    assert content["errors"] == [
-        {
-            "field": "operations",
-            "message": "Variants are disabled in this product type.",
-        }
-    ]
+    assert content["productErrors"][0]["field"] == "operations"
+    assert (
+        content["productErrors"][0]["message"]
+        == "Variants are disabled in this product type."
+    )
+    assert (
+        content["productErrors"][0]["code"]
+        == ProductErrorCode.ATTRIBUTE_VARIANTS_DISABLED.name
+    )
 
 
 def test_assign_variant_attribute_having_unsupported_input_type(
@@ -1310,15 +1336,16 @@ def test_assign_variant_attribute_having_unsupported_input_type(
     content = get_graphql_content(staff_api_client.post_graphql(query, variables))[
         "data"
     ]["attributeAssign"]
-    assert content["errors"] == [
-        {
-            "field": "operations",
-            "message": (
-                "Attributes having for input types ['multiselect'] cannot be assigned "
-                "as variant attributes"
-            ),
-        }
-    ]
+    assert content["productErrors"][0]["field"] == "operations"
+    assert (
+        content["productErrors"][0]["message"]
+        == "Attributes having for input types ['multiselect'] "
+        "cannot be assigned as variant attributes"
+    )
+    assert (
+        content["productErrors"][0]["code"]
+        == ProductErrorCode.ATTRIBUTE_CANNOT_BE_ASSIGNED.name
+    )
 
 
 @pytest.mark.parametrize(
@@ -1365,12 +1392,15 @@ def test_assign_attribute_to_product_type_having_already_that_attribute(
     content = get_graphql_content(staff_api_client.post_graphql(query, variables))[
         "data"
     ]["attributeAssign"]
-    assert content["errors"] == [
-        {
-            "field": "operations",
-            "message": "Color (color) have already been assigned to this product type.",
-        }
-    ]
+    assert content["productErrors"][0]["field"] == "operations"
+    assert (
+        content["productErrors"][0]["message"]
+        == "Color (color) have already been assigned to this product type."
+    )
+    assert (
+        content["productErrors"][0]["code"]
+        == ProductErrorCode.ATTRIBUTE_ALREADY_ASSIGNED.name
+    )
 
 
 UNASSIGN_ATTR_QUERY = """


Code review comments:
diff_hunk: @@ -382,9 +382,41 @@ def check_product_operations_are_assignable(cls, product_attrs_pks):
                 }
             )
 
+    @classmethod
+    def handle_typed_errors(cls, errors: list, **extra):
+        typed_errors = [
+            cls._meta.error_type_class(  # type: ignore
+                field=e.field,
+                message=e.message,
+                code=code,
+                attributes=params.get("attributes") if params else None,
+            )
+            for e, code, params in errors
+        ]
+        extra.update({cls._meta.error_type_field: typed_errors})  # type: ignore
+        return cls(errors=[e[0] for e in errors], **extra)  # type: ignore
+
     @classmethod
     def clean_operations(cls, product_type, product_attrs_pks, variant_attrs_pks):
         """Ensure the attributes are not already assigned to the product type."""
+        attrs_pk = product_attrs_pks + variant_attrs_pks
+        attributes = models.Attribute.objects.filter(id__in=attrs_pk).values_list(
+            "pk", flat=True
+        )
+        if len(attrs_pk) != len(attributes):
+            invalid_attrs = set(attrs_pk) - set(attributes)
+            invalid_attrs = [
+                graphene.Node.to_global_id("Attribute", pk) for pk in invalid_attrs
+            ]
+            raise ValidationError(
+                {
+                    "operations": ValidationError(
+                        f"Attribute doesn't exist.",
+                        code=ProductErrorCode.NOT_FOUND.value,
path: saleor/graphql/product/mutations/attributes.py
text: @user1:
You don't need `.value`, it's done automatically in `get_error_code_from_error`.