Negative weight validation #5564
Conversation
|
Here is the report for 94308f0 (mirumee/saleor @ negative_weight_validation) No differences were found. (click me)
# api.benchmark account
test name left count right count duplicate count
----------------------------------------------------------- ----------- ----------- ---------------
delete staff members 32 32 0
query staff user 21 21 4
staff create 24 24 5
staff update groups and permissions 36 36 6
# api.benchmark category
test name left count right count duplicate count
----------------------------------------------------------- ----------- ----------- ---------------
category view 18 18 1
# api.benchmark checkout mutations
test name left count right count duplicate count
----------------------------------------------------------- ----------- ----------- ---------------
add billing address to checkout 52 52 26
add shipping to checkout 40 40 12
checkout email update 29 29 13
checkout payment charge 23 23 5
checkout shipping address update 35 35 8
checkout voucher code 59 59 31
complete checkout 74 74 19
create checkout 140 140 75
update checkout lines 101 101 50
# api.benchmark collection
test name left count right count duplicate count
----------------------------------------------------------- ----------- ----------- ---------------
collection view 17 17 0
# api.benchmark homepage
test name left count right count duplicate count
----------------------------------------------------------- ----------- ----------- ---------------
featured products list 14 14 0
retrieve main menu 8 8 0
retrieve product list 5 5 0
retrieve secondary menu 8 8 0
retrieve shop 2 2 0
user checkout details 52 52 26
# api.benchmark order
test name left count right count duplicate count
----------------------------------------------------------- ----------- ----------- ---------------
user order details 17 17 2
# api.benchmark permission group
test name left count right count duplicate count
----------------------------------------------------------- ----------- ----------- ---------------
permission group create 21 21 3
permission group delete 21 21 4
permission group query 8 8 0
permission group update 36 36 2
permission group update remove users with manage staff 31 31 4
# api.benchmark product
test name left count right count duplicate count
----------------------------------------------------------- ----------- ----------- ---------------
product details 19 19 2
retrieve product attributes 7 7 0
# api.benchmark variant
test name left count right count duplicate count
----------------------------------------------------------- ----------- ----------- ---------------
product variant bulk create 48 48 2
retrieve variant list 23 23 6
# api.benchmark variant stocks
test name left count right count duplicate count
----------------------------------------------------------- ----------- ----------- ---------------
product variants stocks create 23 23 5
product variants stocks delete 20 20 5
product variants stocks update 28 28 5
# api product sorting attributes
test name left count right count duplicate count
----------------------------------------------------------- ----------- ----------- ---------------
sort product not having attribute data 20 20 0 |
Codecov Report
@@ Coverage Diff @@
## master #5564 +/- ##
=======================================
Coverage 91.68% 91.68%
=======================================
Files 303 303
Lines 19776 19789 +13
Branches 1835 1840 +5
=======================================
+ Hits 18131 18144 +13
Misses 1209 1209
Partials 436 436
Continue to review full report at Codecov.
|
fowczarek
requested review from
IKarbowiak,
d-wysocki,
maarcingebala and
tomaszszymanski129
| @@ -802,6 +802,18 @@ def clean_attributes( | |||
| @classmethod | |||
| def clean_input(cls, info, instance, data): | |||
| cleaned_input = super().clean_input(info, instance, data) | |||
|
|
|||
| weight = cleaned_input.get("weight") | |||
| if weight and weight.value < 0: | |||
There was a problem hiding this comment.
btw. can something have 0? if not that would have to be <=
There was a problem hiding this comment.
We talk about it yesterday, and in some cases, you want to set products (e.g. Post Cards) Weight to 0.
There was a problem hiding this comment.
Well, post cards have some not-null weight but I guess there might be someone that sells virtual goods among physical products so that makes sense anyway.
| weight = cleaned_input.get("weight") | ||
| if weight and weight.value < 0: | ||
| raise ValidationError( | ||
| { | ||
| "weight": ValidationError( | ||
| "Product variant can't have negative weight.", | ||
| code=ProductErrorCode.INVALID.value, | ||
| ) | ||
| } | ||
| ) | ||
|
|
There was a problem hiding this comment.
can be extracted to something like validate_weight and used here and there as there are already 3 places with the same code
There was a problem hiding this comment.
But errors are different(Solution with rise errors outside the function do not increase code readability. We also can rise errors in function but we should pass error information in parameters, I think it isn't good solution)
Second, think is that we have more duplicated logic in clean_input in ProductVariantCreate, ProductTypeCreate, and ProcutCreate, We should consider refactor it globally instead of having some logic duplicated and some logic hidden in functions.
I want to merge this change because adding negative weight validation and fix 0 value in weight scalar.
Impact
Pull Request Checklist