Repository: saleor/saleor
Pull request: Make product variant attributes required in API
I want to merge this change because...

closes #2879

### Screenshots

<!-- If your changes affect the UI, providing "before" and "after" screenshots will
greatly reduce the amount of work needed to review your work. -->

### Pull Request Checklist

<!-- Please keep this section. It will make maintainer's life easier. -->

1. [x] Privileged views and APIs are guarded by proper permission checks.
1. [x] All visible strings are translated with proper context.
1. [x] All data-formatting is locale-aware (dates, numbers, and so on).
1. [x] Database queries are optimized and the number of queries is constant.
1. [x] The changes are tested.
1. [x] The code is documented (docstrings, project documentation).
1. [x] GraphQL schema and type definitions are up to date.


Problem statement:
User can create a variant without attributes
Currently user is able to create a variant with partly set attributes or without attributes at all. 

### What I expected to happen
All attributes set in product type should be required in variant creating form.

Hints from issue discussion:

Commit message: Make product variant attributes required in API

Patch to review:
diff --git a/saleor/graphql/product/mutations/products.py b/saleor/graphql/product/mutations/products.py
index c9dd01e67a3..77baa4e24e4 100644
--- a/saleor/graphql/product/mutations/products.py
+++ b/saleor/graphql/product/mutations/products.py
@@ -332,7 +332,19 @@ class Meta:
         model = models.ProductVariant
 
     @classmethod
-    def clean_input(cls, info, instance, input, errors):
+    def check_all_product_type_attributes_provided(
+            cls, attributes_qs, attributes_input, errors):
+        product_type_attr_slugs = {attr.slug for attr in attributes_qs}
+        attributes_input = {attr.slug for attr in attributes_input}
+        missing_attributes = product_type_attr_slugs - attributes_input
+        if missing_attributes:
+            cls.add_error(
+                errors, 'attributes',
+                'Missing attributes: %s' % ', '.join(missing_attributes))
+
+    @classmethod
+    def clean_input(
+            cls, info, instance, input, errors, check_all_attrs_provided=True):
         cleaned_input = super().clean_input(info, instance, input, errors)
 
         # Attributes are provided as list of `AttributeValueInput` objects.
@@ -348,6 +360,9 @@ def clean_input(cls, info, instance, input, errors):
         if attributes and product_type:
             try:
                 qs = product_type.variant_attributes.prefetch_related('values')
+                if check_all_attrs_provided:
+                    cls.check_all_product_type_attributes_provided(
+                        qs, attributes, errors)
                 attributes = attributes_to_hstore(attributes, qs)
             except ValueError as e:
                 cls.add_error(errors, 'attributes', str(e))
@@ -378,6 +393,11 @@ class Meta:
         description = 'Updates an existing variant for product'
         model = models.ProductVariant
 
+    @classmethod
+    def clean_input(cls, info, instance, input, errors):
+        return super(ProductVariantUpdate, cls).clean_input(
+            info, instance, input, errors, check_all_attrs_provided=False)
+
 
 class ProductVariantDelete(ModelDeleteMutation):
     class Arguments:
diff --git a/tests/api/test_variant.py b/tests/api/test_variant.py
index 3d2a832bd03..eaf41d0d4f1 100644
--- a/tests/api/test_variant.py
+++ b/tests/api/test_variant.py
@@ -150,6 +150,94 @@ def test_create_variant(admin_api_client, product, product_type):
     assert data['weight']['value'] == weight
 
 
+def test_create_product_variant_not_all_attributes(
+        admin_api_client, product, product_type, color_attribute):
+    query = """
+            mutation createVariant (
+                $productId: ID!,
+                $sku: String!,
+                $priceOverride: Decimal,
+                $costPrice: Decimal,
+                $quantity: Int!,
+                $attributes: [AttributeValueInput],
+                $weight: WeightScalar,
+                $trackInventory: Boolean!) {
+                    productVariantCreate(
+                        input: {
+                            product: $productId,
+                            sku: $sku,
+                            priceOverride: $priceOverride,
+                            costPrice: $costPrice,
+                            quantity: $quantity,
+                            attributes: $attributes,
+                            trackInventory: $trackInventory,
+                            weight: $weight
+                        }) {
+                        errors {
+                            field
+                            message
+                        }
+                        productVariant {
+                            name
+                            sku
+                            attributes {
+                                attribute {
+                                    slug
+                                }
+                                value {
+                                    slug
+                                }
+                            }
+                            quantity
+                            priceOverride {
+                                currency
+                                amount
+                                localized
+                            }
+                            costPrice {
+                                currency
+                                amount
+                                localized
+                            }
+                            weight {
+                                value
+                                unit
+                            }
+                        }
+                    }
+                }
+
+        """
+    product_id = graphene.Node.to_global_id('Product', product.pk)
+    sku = "1"
+    price_override = 1.32
+    cost_price = 3.22
+    quantity = 10
+    weight = 10.22
+    variant_slug = product_type.variant_attributes.first().slug
+    variant_value = 'test-value'
+    product_type.variant_attributes.add(color_attribute)
+
+    variables = json.dumps({
+        'productId': product_id,
+        'sku': sku,
+        'quantity': quantity,
+        'costPrice': cost_price,
+        'priceOverride': price_override,
+        'weight': weight,
+        'attributes': [
+            {'slug': variant_slug, 'value': variant_value}],
+        'trackInventory': True})
+    response = admin_api_client.post(
+        reverse('api'), {'query': query, 'variables': variables})
+    content = get_graphql_content(response)
+    assert 'errors' not in content
+    assert 'errors' in content['data']['productVariantCreate']
+    assert content['data']['productVariantCreate']['errors'] == [{
+        'field': 'attributes', 'message': 'Missing attributes: color'}]
+    assert not product.variants.filter(sku=sku).exists()
+
+
 def test_update_product_variant(admin_api_client, product):
     query = """
         mutation updateVariant (


Code review comments:
diff_hunk: @@ -332,7 +332,19 @@ class Meta:
         model = models.ProductVariant
 
     @classmethod
-    def clean_input(cls, info, instance, input, errors):
+    def check_all_product_type_attributes_provided(
+            cls, attributes_qs, attributes_input, errors):
+        product_type_attr_slugs = {attr.slug for attr in attributes_qs}
+        attributes_input = {attr.slug for attr in attributes_input}
+        missing_attributes = product_type_attr_slugs - attributes_input
+        if missing_attributes:
+            cls.add_error(
+                errors, 'attributes',
+                'Missing attributes: %s' % ', '.join(missing_attributes))
+
+    @classmethod
+    def clean_input(
+            cls, info, instance, input, errors, check_all_attrs_provided=True):
path: saleor/graphql/product/mutations/products.py
text: @user1:
Could we rename it to `check_if_...`? It would be more natural
Also, is there any case when we shouldn't check if all attrs are provided? 
I wonder if this variable is really necessary or we should always check if all product's attributes are present

diff_hunk: @@ -332,7 +332,19 @@ class Meta:
         model = models.ProductVariant
 
     @classmethod
-    def clean_input(cls, info, instance, input, errors):
+    def check_all_product_type_attributes_provided(
path: saleor/graphql/product/mutations/products.py
text: @user1:
Can we rename it to `check_if...` ? or even `clean_product_type_attributes`