-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Fix applying SPECIFIC_PRODUCT
and apply_once_per_order
voucher on draft orders
#16153
Fix applying SPECIFIC_PRODUCT
and apply_once_per_order
voucher on draft orders
#16153
Conversation
SPECIFIC_PRODUCT
voucher on draft ordersSPECIFIC_PRODUCT
and apply_once_per_order
voucher on draft orders
Here is the report for bb1f1f8 (saleor:shopx-872-specific-products-vouchers-in-draft-order-do-not-work) No differences were found.
# saleor.graphql.accountbenchmark account
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
addresses for federation query count 9 9 2
customers query 51 51 0
delete staff members 40 40 0
query staff user 22 22 3
staff create 22 22 3
staff update groups and permissions 35 35 5
users for federation query count 8 8 3
# saleor.graphql.accountbenchmark permission group
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
groups for federation query count 3 3 0
permission group create 24 24 0
permission group delete 29 29 3
permission group query 14 14 0
permission group update 43 43 2
permission group update remove users with manage staff 35 35 3
# saleor.graphql.appbenchmarks app extensions
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
app extensions 15 15 0
app extensions with filter[filter0] 14 14 0
app extensions with filter[filter1] 10 10 0
app extensions with filter[filter2] 14 14 0
app extensions with filter[filter3] 10 10 0
# saleor.graphql.appbenchmarks apps
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
apps for federation query count 9 9 3
apps with tokens and webhooks 9 9 0
# saleor.graphql.attributebenchmark attribute
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
attribute translation 6 6 0
attribute value translation 20 20 0
query attribute 6 6 0
query attributes 8 8 0
# saleor.graphql.channelbenchmark channel
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
channels query 5 5 0
# saleor.graphql.channelmutations channel update
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
channel update mutation remove warehouse 39 39 6
# saleor.graphql.checkoutbenchmark checkout mutations
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
add billing address to checkout 79 79 10
add checkout lines 91 91 14
add checkout lines catalogue discount applies 189 189 15
add checkout lines gift discount applies 154 154 23
add checkout lines multiple catalogue discount applies 204 204 20
add checkout lines order discount applies 167 167 18
add checkout lines with external shipping 193 193 77
add checkout lines with reservations 191 191 68
add delivery to checkout 87 87 14
add shipping to checkout 97 97 17
checkout email update 45 45 2
checkout payment charge 60 60 17
checkout shipping address update 91 91 11
checkout voucher code 104 104 17
complete checkout 248 248 85
complete checkout preorder 239 239 83
complete checkout with digital line 282 282 96
complete checkout with out of stock webhook 250 250 85
complete checkout with single line 250 250 85
create checkout 86 86 9
create checkout with gift promotion 126 126 19
create checkout with order promotion 108 108 9
create checkout with reservations 157 157 28
customer complete checkout 261 261 91
customer complete checkout for cc 232 232 78
update checkout lines 84 84 14
update checkout lines with reservations 197 197 79
# saleor.graphql.checkoutbenchmark checkouts
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
staff multiple checkouts 6 6 0
# saleor.graphql.checkoutbenchmark homepage
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
user checkout details 63 63 6
user checkout details with tax app 85 85 28
# saleor.graphql.discountbenchmark promotion create
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
promotion create 23 23 1
promotion create order promotion 37 37 7
# saleor.graphql.discountbenchmark promotion delete
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
gift promotion delete 25 25 1
promotion delete 30 30 1
# saleor.graphql.discountbenchmark promotion rule create
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
promotion rule create 23 23 1
promotion rule create gift 134 134 7
# saleor.graphql.discountbenchmark promotion rule delete
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
promotion rule delete 23 23 0
# saleor.graphql.discountbenchmark promotion rule update
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
promotion rule update 38 38 0
# saleor.graphql.discountbenchmark promotion update
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
promotion update 16 16 0
# saleor.graphql.discountbenchmark promotions
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
promotions querytest promotions query 9 9 0
# saleor.graphql.discountbenchmark sales
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
sales query with channel slug 9 9 0
sales query without channel slug 7 7 0
# saleor.graphql.discountbenchmark voucher code bulk delete
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
voucher code bulk delete queries 20 20 2
# saleor.graphql.discountbenchmark vouchers
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
vouchers query with channel slug 20 20 0
vouchers query withot channel slug 19 19 0
# saleor.graphql.giftcardbenchmark gift card mutations
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
create never expiry gift card 27 27 4
gift card bulk activate by staff 15 15 3
update gift card 35 35 4
# saleor.graphql.giftcardbenchmark gift card queries
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
filter gift cards by products 14 14 3
filter gift cards by tags 12 12 3
filter gift cards by used by user 13 13 3
query gift card details 14 14 3
query gift cards 12 12 3
# saleor.graphql.orderbenchmark fulfillment
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
fulfillment query 6 6 0
# saleor.graphql.orderbenchmark fulfillment refund and return products
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
fulfillment refund products order lines 61 61 0
fulfillment return products order lines 103 103 7
# saleor.graphql.orderbenchmark order
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
staff multiple draft orders 7 7 0
staff multiple orders 7 7 0
staff order details 57 57 7
user order details 54 54 8
# saleor.graphql.orderbenchmark order bulk create
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
order bulk create 20 20 2
# saleor.graphql.orderbenchmark order fulfill
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
order fulfill 18 18 0
order fulfill with gift cards 18 18 0
# saleor.graphql.orderbenchmark order lines create
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
order lines create 95 95 17
order lines create variants on promotion 154 154 22
# saleor.graphql.pagebenchmark page type
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
page types for federation query count 3 3 0
query page type 18 18 6
query page types 25 25 12
# saleor.graphql.paymentbenchmark payment transactions
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
payment transactions 54 54 20
# saleor.graphql.productbenchmark category
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
categories children 4 4 0
categories for federation query count 3 3 0
category delete 48 48 4
category view 28 28 1
# saleor.graphql.productbenchmark collection
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
collection add products 22 22 5
collection bulk delete 27 27 3
collection view 7 7 0
collections for federation query count 4 4 0
create collection 37 37 5
delete collection 40 40 5
remove products from collection 19 19 5
retrieve collection channel listings 5 5 0
# saleor.graphql.productbenchmark homepage
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
report product sales 10 10 3
retrieve product list 4 4 0
# saleor.graphql.productbenchmark product
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
filter products by attributes 7 7 0
filter products by boolean attributes 14 14 0
filter products by gift card 12 12 1
filter products by numeric attributes 13 13 0
product create 48 48 3
product details 41 41 1
product translations 5 5 0
products for federation query count 6 6 1
products media for federation query count 5 5 0
products types for federation query count 2 2 0
retrieve channel listings 21 21 0
retrieve product attributes 8 8 0
retrieve product images 4 4 0
retrieve product media 4 4 0
retrive products with product types and attributes 7 7 0
update product 71 71 5
# saleor.graphql.productbenchmark product variant channel listing update
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
variant channel listing update 35 35 3
# saleor.graphql.productbenchmark variant
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
product variant bulk create 45 45 1
product variant create 61 61 3
products variants for federation query count 6 6 1
retrieve variant list 34 34 1
update product variant 64 64 7
# saleor.graphql.productbenchmark variant stocks
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
product variants stocks create 28 28 4
product variants stocks create with single webhook called 27 27 4
product variants stocks delete by id 29 29 4
product variants stocks delete by sku 29 29 4
product variants stocks delete with out of stock webhook many calls 25 25 3
product variants stocks update by sku 27 27 4
product variants stocks update byid 27 27 4
query product variants stocks 10 10 0
# saleor.graphql.producttest product sorting attributes
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
sort product not having attribute data 20 20 0
# saleor.graphql.shippingbenchmark shipping methods
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
vouchers query with channel slug 7 7 0
vouchers query without channel slug 7 7 0
# saleor.graphql.shippingbenchmark shipping zones
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
shipping zones query 6 6 0
# saleor.graphql.shopbenchmark homepage
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
retrieve shop 2 2 0
# saleor.graphql.warehousebenchmark stock bulk update
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
stocks bulk update queries count 31 31 5
# saleor.graphql.warehousebenchmark stocks
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
stocks query 5 5 0
# saleor.graphql.webhookbenchmark webhook events
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
webhooks 6 6 0
# saleor.orderbenchmark fetch order prices
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
fetch order prices catalogue discount 50 50 7
fetch order prices multiple catalogue discounts 112 112 21
# saleor.ordertest fetch
test name left count right count duplicate count
------------------------------------------------------------------------ ----------- ----------- ---------------
fetch draft order lines info 20 20 1
|
d8c94ed
to
0e7ee61
Compare
6033aef
to
05edfd2
Compare
62dc458
to
8c9b79f
Compare
Oh! That's a wonderful pull request! Fantastic work! 🥳 |
|
||
|
||
@dataclass | ||
class CheckoutLineInfo: | ||
class CheckoutLineInfo(LineInfo): |
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.
Why not put this class to saleor/core/pricing/interface.py
file?
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.
and move CheckoutInfo
to saleor/core/checkout/interface.py
?
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.
It's checkout specific so I decided to left it in checkout
module, we can discuss this with the team
@@ -73,28 +78,17 @@ def fetch_order_lines(order: "Order") -> list[OrderLineInfo]: | |||
|
|||
|
|||
@dataclass | |||
class DraftOrderLineInfo: | |||
class DraftOrderLineInfo(LineInfo): |
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.
same here, to move this to interfaces.py
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.
It's order specific so I decided to left it in order
module, we can discuss this with the team
def get_voucher_discounts( | ||
self, | ||
): |
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.
def get_voucher_discounts( | |
self, | |
): | |
def get_voucher_discounts( | |
self, | |
) -> List[Union["OrderLineDiscount", "CheckoutLineDiscount"]]: |
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.
I decided to not add the typing here as it was breaking the mypy. I was trying to overload
here, but the result type depends on self
type, so basically to resolve the problem I would have to move those method into DraftOrderLineInfo
and CheckoutLineInfo
, which basically breaks the idea of having this shared interface here. So I decided to not use the typing here, as having those method here in base interface gives more value from me perspective than adding typing.
def get_catalogue_discounts( | ||
self, | ||
): |
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.
def get_catalogue_discounts( | |
self, | |
): | |
def get_catalogue_discounts( | |
self, | |
) -> List[Union["OrderLineDiscount", "CheckoutLineDiscount"]]: |
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.
Same as above
line_info.voucher_code = voucher_info.voucher_code | ||
|
||
|
||
def get_discounted_lines( |
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.
Correct me if I am wrong. get_discounted_lines
is a replacement for custom get_discounted_lines
functions used with Checkout, and Order (saleor.checkout.utils.get_discounted_lines
, saleor.order.utils.get_discounted_lines
). Shouldn't we fully drop these functions?
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.
I will check that, probably it will be possible, but I guess it will need some adjustment. I will verify this and adjust it in another PR, as I'm already working on a similar one with checkout and order adjustment after those changes.
) | ||
) | ||
voucher = order.voucher | ||
if voucher and ( |
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.
In saleor.order.calculations.fetch_order_prices_if_expired
we also call saleor.order.calculations._update_order_discount_for_voucher
. It creates an orderDiscount object. I don't see any condition for specfic_product or apply_once_per_order. It means, that we will propagate the discount over lines and create the additional order.discounts objects. Is it expected? Won't we add a discount twice?
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.
It's not the correct behaviour, we have a ticket for fixing that. I want to narrow the changes here as it's quite big right now. The OrderDiscount
object will have 0
discount amount, so the discount won't be apply twice, but still you are right that the object shouldn't be created. We will fix that in another PR.
53f9b07
to
b9e39ff
Compare
@IKarbowiak note some tests are failing with |
… draft orders (#16153) * Attach voucher to DraftOrderLineInfo if applicable * Add generic LineInfo dataclass * Apply specific_product and apply_onde_per_order voucher on draft order * Include line manual discounts on draft orders * Fix failing test * Add tests for draftOrderCreate mutation * Add tests for draftOrderUpdate * Add tests for fetch_order_prices_if_expired for different voucher types * Add tests for order price recalculations for manual discount and vouchers * Fix failing CI * Add tests for vouchers and manual line discounts for draft order * Adjust TaxableObject.discounts for draft orders * Adjust _update_discount method * Unify create_order_line_discount_objects * Add test for updating OrderLineDiscount object for voucher * Apply code review suggestions * Make product and variant optional on LineInfo
… draft orders (#16153) * Attach voucher to DraftOrderLineInfo if applicable * Add generic LineInfo dataclass * Apply specific_product and apply_onde_per_order voucher on draft order * Include line manual discounts on draft orders * Fix failing test * Add tests for draftOrderCreate mutation * Add tests for draftOrderUpdate * Add tests for fetch_order_prices_if_expired for different voucher types * Add tests for order price recalculations for manual discount and vouchers * Fix failing CI * Add tests for vouchers and manual line discounts for draft order * Adjust TaxableObject.discounts for draft orders * Adjust _update_discount method * Unify create_order_line_discount_objects * Add test for updating OrderLineDiscount object for voucher * Apply code review suggestions * Make product and variant optional on LineInfo
… draft orders (#16153) (#16209) * Attach voucher to DraftOrderLineInfo if applicable * Add generic LineInfo dataclass * Apply specific_product and apply_onde_per_order voucher on draft order * Include line manual discounts on draft orders * Fix failing test * Add tests for draftOrderCreate mutation * Add tests for draftOrderUpdate * Add tests for fetch_order_prices_if_expired for different voucher types * Add tests for order price recalculations for manual discount and vouchers * Fix failing CI * Add tests for vouchers and manual line discounts for draft order * Adjust TaxableObject.discounts for draft orders * Adjust _update_discount method * Unify create_order_line_discount_objects * Add test for updating OrderLineDiscount object for voucher * Apply code review suggestions * Make product and variant optional on LineInfo
… draft orders (saleor#16153) (saleor#16209) * Attach voucher to DraftOrderLineInfo if applicable * Add generic LineInfo dataclass * Apply specific_product and apply_onde_per_order voucher on draft order * Include line manual discounts on draft orders * Fix failing test * Add tests for draftOrderCreate mutation * Add tests for draftOrderUpdate * Add tests for fetch_order_prices_if_expired for different voucher types * Add tests for order price recalculations for manual discount and vouchers * Fix failing CI * Add tests for vouchers and manual line discounts for draft order * Adjust TaxableObject.discounts for draft orders * Adjust _update_discount method * Unify create_order_line_discount_objects * Add test for updating OrderLineDiscount object for voucher * Apply code review suggestions * Make product and variant optional on LineInfo
… draft orders (saleor#16153) (saleor#16209) * Attach voucher to DraftOrderLineInfo if applicable * Add generic LineInfo dataclass * Apply specific_product and apply_onde_per_order voucher on draft order * Include line manual discounts on draft orders * Fix failing test * Add tests for draftOrderCreate mutation * Add tests for draftOrderUpdate * Add tests for fetch_order_prices_if_expired for different voucher types * Add tests for order price recalculations for manual discount and vouchers * Fix failing CI * Add tests for vouchers and manual line discounts for draft order * Adjust TaxableObject.discounts for draft orders * Adjust _update_discount method * Unify create_order_line_discount_objects * Add test for updating OrderLineDiscount object for voucher * Apply code review suggestions * Make product and variant optional on LineInfo
Apply
SPECIFIC_PRODUCT
andapply_once_per_order
voucher on draft orders.Fixes: #15517
Internal issues:
Impact
Docs
Pull Request Checklist
ADDED_IN_X
,PREVIEW_FEATURE
, etc.)