New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change filed type to choice field of tax rate in product #3478
Change filed type to choice field of tax rate in product #3478
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3478 +/- ##
=========================================
Coverage ? 89.75%
=========================================
Files ? 237
Lines ? 12950
Branches ? 1313
=========================================
Hits ? 11623
Misses ? 922
Partials ? 405
Continue to review full report at Codecov.
|
saleor/graphql/schema.graphql
Outdated
@@ -1448,6 +1448,33 @@ enum ProductOrderField { | |||
DATE | |||
} | |||
|
|||
enum ProductTaxRate { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have the TaxRateType
in the schema. The difference is that TaxRateType
is implemented explicitly by us, while this one is generated by Graphene after introducing choices
in tax_rate
field (I guess). We should only keep one of them in the schema. I think the TaxRateType
would be easier to drop but the question is how can we use the ProductTaxRate
in other places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it good now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now! We're changing the type here and since we keep the same name of the field, we cannot deprecate it. This means that we have to pass this PR to @dominik-zeglen to fix references to taxRate
field in the Dashboard 2.0.
Front end reports full okayness. |
@@ -286,6 +286,7 @@ class Product(CountableDjangoObjectType): | |||
Money, | |||
description=dedent("""The product's base price (without any discounts | |||
applied).""")) | |||
tax_rate = TaxRateType(description='A type of tax rate.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The a type of...
is sounding weird to me.
What do you think of: The tax rate type.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welp @maarcingebala just merged it 😆
I want to merge this change because resolve #2829
Screenshots
Pull Request Checklist