Exception: attributes are not cleaned prior to save for ProductVariantCreate

onecrayon commented 3 years ago

What I'm trying to achieve

I am trying to implicitly set Attribute values for a singleton ProductVariant, per the GraphQL schema docs (running 3.0.0a16).

Steps to reproduce the problem

Through GraphQL:

Create a DROPDOWN Attribute with no values assigned
Create a ProductType that uses the attribute
Create a Product that uses the ProductType
Create a ProductVariant that uses the ProductType but assigns a string value implicitly to the attribute

For instance:

mutation {
    productVariantCreate (input: {
      sku: "my-sku", 
      product: "PRODUCTID", 
      attributes: [
        {id: "ATTRIBUTEID", values: ["New Value"]}
      ]
    }) {
        productVariant {
            id
        }
        productErrors {
            field,
            message,
        }
    }
}

What I expected to happen

I expect to have the variant created, since that follows the provided schema for ProductVariantCreateInput. What actually happens is that Saleor throws an exception:

{
  "errors": [
    {
      "message": "'str' object has no attribute 'input_type'",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "productVariantCreate"
      ],
      "extensions": {
        "exception": {
          "code": "AttributeError",
          "stacktrace": [
            "Traceback (most recent call last):",
            "  File \"/usr/local/lib/python3.9/site-packages/promise/promise.py\", line 489, in _resolve_from_executor",
            "    executor(resolve, reject)",
            "  File \"/usr/local/lib/python3.9/site-packages/promise/promise.py\", line 756, in executor",
            "    return resolve(f(*args, **kwargs))",
            "  File \"/usr/local/lib/python3.9/site-packages/graphql/execution/middleware.py\", line 75, in make_it_promise",
            "    return next(*args, **kwargs)",
            "  File \"/app/saleor/graphql/core/mutations.py\", line 297, in mutate",
            "    response = cls.perform_mutation(root, info, **data)",
            "  File \"/app/saleor/graphql/core/mutations.py\", line 500, in perform_mutation",
            "    cls.save(info, instance, cleaned_input)",
            "  File \"/usr/local/lib/python3.9/contextlib.py\", line 79, in inner",
            "    return func(*args, **kwds)",
            "  File \"/app/saleor/graphql/product/mutations/products.py\", line 896, in save",
            "    AttributeAssignmentMixin.save(instance, attributes)",
            "  File \"/app/saleor/graphql/attribute/utils.py\", line 377, in save",
            "    if attribute.input_type == AttributeInputType.FILE:",
            "AttributeError: 'str' object has no attribute 'input_type'"
          ]
        }
      }
    }
  ],
  "data": {
    "productVariantCreate": null
  }
}

I looked into the Saleor source to see if I could figure out how to fix this, but I'm unfamiliar with how Saleor data flows from GraphQL so I'm having a hard time identifying where the change needs to be placed. For whatever reason /app/saleor/graphql/attribute/utils.py line 377 is receiving exactly the same dict input as I am passing to the GraphQL input (it's not getting passed through clean_attributes the way the documentation comment states it must).

System information

Operating system: unmodified local Saleor Docker container Saleor version: 3.0.0-a16

IKarbowiak commented 3 years ago

Hi @onecrayon,

Thank you for reporting the issue. The problem is that you are creating productVariant on Product which shouldn't have variants. Variants can be only assigned to products that are assigned to ProductTypes with hasVariants field set to True. So please update ProductType hasVariants field with productTypeUpdate mutation, and try again. From our site, we'll update productVariantCreate with validation for checking if a variant on the specified product can be created.

onecrayon commented 3 years ago

@IKarbowiak I was originally setting hasVariants to True on the ProductType, but this resulted in it being set as a Configurable product instead of a Simple product. However, creating a single ProductVariant against a Simple product is exactly what your dashboard does. I extracted this process straight from that GraphQL before I went about coding my migration logic, because the documentation was basically non-existent for how to handle this and that seemed like a good example.

So if I'm understanding correctly from poking around in the Dashboard GraphQL calls a bit more:

For a Configurable product, you can set attributes on individual ProductVariants, and the Product can contain multiple ProductVariants.
For a Simple product, you still need a ProductVariant, but the only information you associate with it is the SKU (so that you can set cost-per-channel information for that variant). In this case, all attributes must be defined when creating the Product entry instead.

Is that accurate? I would really love more documentation about exactly what each toggle does and what steps are necessary to create a well-formed product (for instance, I only figured out the relationship between hasVariants and configurable/simple products through trial and error, and I had to snoop on the Dashboard's GraphQL calls to get the full flow necessary to create a new product).

onecrayon commented 3 years ago

@IKarbowiak Wanted to mention: the other thing that had me convinced I needed to be setting attributes on the ProductVariant instead of the Product even for Simple products is that the ProductVariantCreate throws a validation error if the attributes property is not included in the input (which seemed nonsensical if attributes are also disallowed).

maarcingebala commented 3 years ago

Clickup task: https://app.clickup.com/t/2549495/SALEOR-2873

saleor / saleor