Skip to content
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

Possibility to apply discount on order #6930

Merged
merged 10 commits into from
Mar 5, 2021

Conversation

korycins
Copy link
Member

I want to merge this change because it introduces the possibility to discount the draft orders.

Impact

  • New migrations
  • New/Updated API fields or mutations
  • Deprecated API fields or mutations
  • Removed API types, fields, or mutations
  • Documentation needs to be updated

Pull Request Checklist

  • Privileged queries and mutations are guarded by proper permission checks
  • Database queries are optimized and the number of queries is constant
  • Database migration files are up to date
  • The changes are tested
  • GraphQL schema and type definitions are up to date
  • Changes are mentioned in the changelog

korycins and others added 4 commits February 24, 2021 09:38
* Add order.discounts models and move all bussines voucher logic to use it

* Add OrderDiscount.amount field

* Solve incorrect definition inside the migration file

* Fix tests

* Clean up the code

* Update changelog

* Apply changes after review

* Add note about depreceated discount fields to changelog

* Apply changes after review
…#6862)

* Add order.discounts models and move all bussines voucher logic to use it

* Add OrderDiscount.amount field

* Solve incorrect definition inside the migration file

* Fix tests

* Clean up the code

* Update changelog

* Add mutation responsible for managing the discount assigned to order

* Add mutations for discounting the prices for order lines

* Fix for incorrect calculation of the prices after apply the discount

* Fix tests

* Add missing discount fields in order line

* Add validation before adding new discount to order/orderLine

* Update discount value type for the order line

* Add disscount value to order line

* Use proper type for discount type

* Proper save the changes in discount's reason

* Add order.discounts models and move all bussines voucher logic to use it

* Add OrderDiscount.amount field

* Solve incorrect definition inside the migration file

* Fix tests

* Clean up the code

* Update changelog

* Rename function to apply discount to value

* Add validation to OrderDiscountDelete mutation

* Add order field to the response of the ORderLine discount mutations

* Add tests to cover the order discount mutations

* Apply changes after review

* Add note about depreceated discount fields to changelog

* Add dataloader for order.discount. Add tests to cover the new field in Order type

* Apply changes after review

* Clean up the OrderDiscountsByOrderId dataloader

* Add  benchmark tests for checking the discount section and events

* Clean up changelog after the merge base branch
* Add order.discounts models and move all bussines voucher logic to use it

* Add OrderDiscount.amount field

* Solve incorrect definition inside the migration file

* Fix tests

* Clean up the code

* Update changelog

* Add mutation responsible for managing the discount assigned to order

* Add mutations for discounting the prices for order lines

* Fix for incorrect calculation of the prices after apply the discount

* Fix tests

* Add missing discount fields in order line

* Add validation before adding new discount to order/orderLine

* Update discount value type for the order line

* Add disscount value to order line

* Use proper type for discount type

* Proper save the changes in discount's reason

* Add order.discounts models and move all bussines voucher logic to use it

* Add OrderDiscount.amount field

* Solve incorrect definition inside the migration file

* Fix tests

* Clean up the code

* Update changelog

* Rename function to apply discount to value

* Add validation to OrderDiscountDelete mutation

* Add order field to the response of the ORderLine discount mutations

* Add tests to cover the order discount mutations

* Add order events for discounting order.

* Add order event for each discount action

* Add discount events for order lines

* Trigger discount events only from discount mutations

* Add new event for automatically update the order with discount

* Add to the autmatically discount event info about previous discount values

* Fix incorrect call inside delete order discount mutation

* Extend the mutation tests to check the proper events

* Apply changes after review

* Add note about depreceated discount fields to changelog

* Add dataloader for order.discount. Add tests to cover the new field in Order type

* Apply changes after review

* Add missing dots in OrderEventDiscountObject and OrderDiscount fields description

* Clean up the OrderDiscountsByOrderId dataloader
@korycins korycins self-assigned this Feb 24, 2021
@korycins korycins added feature branch test deployment Deploy test environment for pull request labels Feb 24, 2021
@github-actions github-actions bot temporarily deployed to possibility-to-apply-discount-on-order February 24, 2021 11:31 Inactive
@db-queries
Copy link

db-queries bot commented Feb 24, 2021

Here is the report for 9e338d9 (mirumee:possibility-to-apply-discount-on-order)
Base comparison is ee624ff.

Found 4 differences! (click me)

# saleor.graphql.accountbenchmark account
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  delete staff members                                       	         32	         32	              0
  query staff user                                           	         20	         20	              4
  staff create                                               	         22	         22	              4
  staff update groups and permissions                        	         33	         33	              5

# saleor.graphql.accountbenchmark permission group
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  permission group create                                    	         19	         19	              2
  permission group delete                                    	         20	         20	              3
  permission group query                                     	          7	          7	              0
  permission group update                                    	         33	         33	              1
  permission group update remove users with manage staff     	         27	         27	              3

# saleor.graphql.attributebenchmark attribute
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  query attribute                                            	          7	          7	              2
  query attributes                                           	         11	         11	              3

# saleor.graphql.checkoutbenchmark checkout mutations
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  add billing address to checkout                            	         49	         49	              5
  add shipping to checkout                                   	         64	         64	             10
  checkout email update                                      	         23	         23	              0
  checkout payment charge                                    	         41	         41	             16
  checkout shipping address update                           	         69	         69	             13
  checkout voucher code                                      	         68	         68	             11
- complete checkout                                          	         95	         96	             21
  create checkout                                            	         76	         76	             12
- customer complete checkout                                 	        132	        133	             35
  update checkout lines                                      	         87	         87	             16

# saleor.graphql.checkoutbenchmark homepage
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  user checkout details                                      	         39	         39	              2

# saleor.graphql.discountbenchmark sales
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  sales query with channel slug                              	         16	         16	              0
  sales query withot channel slug                            	         15	         15	              0

# saleor.graphql.discountbenchmark vouchers
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  vouchers query with channel slug                           	         16	         16	              0
  vouchers query withot channel slug                         	         15	         15	              0

# saleor.graphql.menubenchmark homepage
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  retrieve main menu                                         	          5	          5	              0
  retrieve secondary menu                                    	          5	          5	              0

# saleor.graphql.orderbenchmark fulfillment refund and return products
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  fulfillment refund products order lines                    	         46	         46	              2
  fulfillment return products order lines                    	         99	         99	             11

# saleor.graphql.orderbenchmark order
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
+ staff order details                                        	          -	         69	             41
- user order details                                         	         22	         23	              1

# saleor.graphql.pagebenchmark page type
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  query page type                                            	         17	         17	              5
  query page types                                           	         19	         19	              5

# saleor.graphql.productbenchmark category
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  category view                                              	         24	         24	              1

# saleor.graphql.productbenchmark collection
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  collection view                                            	          6	          6	              0
  retrieve collection channel listings                       	          4	          4	              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
  -----------------------------------------------------------	-----------	-----------	---------------
  product details                                            	         26	         26	              0
  retrieve channel listings                                  	         18	         18	              0
  retrieve product attributes                                	          8	          8	              0
  retrive products with product types and attributes         	          6	          6	              0

# saleor.graphql.productbenchmark variant
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  product variant bulk create                                	         59	         59	              2
  retrieve variant list                                      	         24	         24	              0

# saleor.graphql.productbenchmark variant stocks
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  product variants stocks create                             	         22	         22	              5
  product variants stocks delete                             	         19	         19	              5
  product variants stocks update                             	         27	         27	              5

# saleor.graphql.producttest product sorting attributes
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  sort product not having attribute data                     	         23	         23	              0

# saleor.graphql.shippingbenchmark shipping methods
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  vouchers query with channel slug                           	         10	         10	              0
  vouchers query without channel slug                        	          9	          9	              0

# saleor.graphql.shopbenchmark homepage
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  retrieve shop                                              	          2	          2	              0

@codecov
Copy link

codecov bot commented Feb 24, 2021

Codecov Report

Merging #6930 (95e5a0c) into master (ee624ff) will decrease coverage by 0.03%.
The diff coverage is 90.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6930      +/-   ##
==========================================
- Coverage   93.05%   93.01%   -0.04%     
==========================================
  Files         422      423       +1     
  Lines       30604    31126     +522     
  Branches     3129     3162      +33     
==========================================
+ Hits        28478    28953     +475     
- Misses       1494     1531      +37     
- Partials      632      642      +10     
Impacted Files Coverage Δ
saleor/graphql/order/filters.py 84.93% <ø> (ø)
saleor/graphql/order/mutations/draft_orders.py 95.66% <66.66%> (ø)
saleor/plugins/avatax/__init__.py 94.75% <66.66%> (+0.04%) ⬆️
saleor/graphql/order/types.py 87.82% <68.86%> (-4.11%) ⬇️
saleor/order/events.py 81.48% <82.92%> (+0.40%) ⬆️
saleor/order/utils.py 92.09% <93.43%> (+0.49%) ⬆️
saleor/graphql/order/mutations/discount_order.py 97.47% <97.47%> (ø)
saleor/checkout/complete_checkout.py 89.56% <100.00%> (+0.13%) ⬆️
saleor/discount/__init__.py 100.00% <100.00%> (ø)
saleor/discount/models.py 93.41% <100.00%> (+0.46%) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee624ff...9e338d9. Read the comment docs.

@github-actions github-actions bot temporarily deployed to possibility-to-apply-discount-on-order February 24, 2021 12:37 Inactive
@github-actions github-actions bot temporarily deployed to possibility-to-apply-discount-on-order February 26, 2021 08:23 Inactive
@github-actions github-actions bot temporarily deployed to possibility-to-apply-discount-on-order February 26, 2021 08:26 Inactive
@korycins korycins marked this pull request as ready for review March 1, 2021 07:34
@korycins korycins marked this pull request as draft March 1, 2021 08:09
@korycins korycins marked this pull request as ready for review March 2, 2021 11:01
@@ -366,6 +365,20 @@ def _create_order(
status=status,
channel=checkout.channel,
)
if checkout.discount:
# store voucher as a fixed value as it this the simplest solution for now.
# This will be solved when we refactor the voucher logic to use .discounts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a ClickUp task for this? Maybe it would useful to link it here.

Copy link
Member Author

@korycins korycins Mar 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will include this github thread to the task after I will analyze all requirements for this.

saleor/graphql/order/mutations/discount_order.py Outdated Show resolved Hide resolved
saleor/graphql/order/mutations/discount_order.py Outdated Show resolved Hide resolved
saleor/graphql/order/mutations/discount_order.py Outdated Show resolved Hide resolved
saleor/graphql/order/types.py Outdated Show resolved Hide resolved
saleor/graphql/order/types.py Outdated Show resolved Hide resolved
@@ -548,7 +754,7 @@ def resolve_lines(root: models.Order, info):
@staticmethod
@permission_required(OrderPermissions.MANAGE_ORDERS)
def resolve_events(root: models.Order, _info):
return root.events.all().order_by("pk")
return root.events.prefetch_related("user").all().order_by("pk")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, the OrderEvent type should have a data loader for users, I think the simplest one would do the job:

class UserByIdLoader(DataLoader):
    context_key = "user_by_id"

    def batch_load(self, keys):
        users = User.objects.in_bulk(keys)
        return [users.get(user_id) for user_id in keys]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already discovered the issue with the missing dataloader here and in OrderEvent type. I believe @fowczarek already created the tasks for this as the changes require more work to do it properly. . I've added here the prefetch as I discover that we are generating huge amount of queries by calling order events. But the loaders here should be added in separate PR.

return discount_parameters


def order_discount_event(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would consider moving these to a new file saleor/order/events/discounts.py but if you don't have time let's skip it for now. Later we could try to refactor other events.

saleor/order/utils.py Outdated Show resolved Hide resolved

return decorator


def get_voucher_discount_assigned_to_order(order: Order):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I would consider if we can move all purely discount-related functions to saleor/order/utils/discounts.py.

* update the way of calculating the order discount

* Apply changes after review
CHANGELOG.md Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to possibility-to-apply-discount-on-order March 5, 2021 10:14 Inactive
@maarcingebala maarcingebala merged commit 326a5b8 into master Mar 5, 2021
@maarcingebala maarcingebala deleted the possibility-to-apply-discount-on-order branch March 5, 2021 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test deployment Deploy test environment for pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants