Make product variant attributes required in API #2911
Conversation
|
|
||
| @classmethod | ||
| def clean_input( | ||
| cls, info, instance, input, errors, check_all_attrs_provided=True): |
There was a problem hiding this comment.
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
| @@ -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( | |||
There was a problem hiding this comment.
Can we rename it to check_if... ? or even clean_product_type_attributes
Codecov Report
@@ Coverage Diff @@
## master #2911 +/- ##
==========================================
+ Coverage 89.55% 89.55% +<.01%
==========================================
Files 207 207
Lines 10615 10622 +7
Branches 1027 1030 +3
==========================================
+ Hits 9506 9513 +7
Misses 794 794
Partials 315 315
Continue to review full report at Codecov.
|
tests/api/test_variant.py
Outdated
| attributes: $attributes, | ||
| trackInventory: $trackInventory, | ||
| weight: $weight | ||
| attributes: $attributes | ||
| }) { | ||
| errors { | ||
| field | ||
| message | ||
| } | ||
| productVariant { |
There was a problem hiding this comment.
I think that's unnecessary as well.
| return super(ProductVariantUpdate, cls).clean_input( | ||
| info, instance, input, errors, check_all_attrs_provided=False) | ||
| return super().get_cleaned_input( | ||
| info, instance, input, errors, check_if_all_attrs_provided=False) |
There was a problem hiding this comment.
In such case, one can update ProductVariant and write its attributes partly. I think we should always check if all attrs are provided.
I want to merge this change because...
closes #2879
Screenshots
Pull Request Checklist