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

Introduce a dataloader for available quantity #5707

Merged
merged 6 commits into from
Jun 10, 2020
Merged

Introduce a dataloader for available quantity #5707

merged 6 commits into from
Jun 10, 2020

Conversation

patrys
Copy link
Member

@patrys patrys commented May 27, 2020

It's not very readable but we want to return the maximum available in any single shipping zone which means the loader needs to operate on several tables at once.

TODO:

  • Figure out how to handle variant.track_inventory, it seems wasteful to calculate things for products that don't care about stock, maybe we should rethink requiring a stock record to exist?

New flow for products without inventory tracking:

  • If staff user create an available product without inventory tracking, the product is available to buy in any shipping zone and max quantity is settings.MAX_CHECKOUT_LINE_QUANTITY.

  • Stocks with any quantity for a product without inventory tracking are needed when any staff user want to create fulfillment with this product. (Fulfillment don't decrease quantity for a product without inventory tracking)

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

Copy link
Member

@NyanKiyoshi NyanKiyoshi left a comment

Choose a reason for hiding this comment

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

I think docstrings could be nice before a wall of code

@lgtm-com
Copy link

lgtm-com bot commented May 27, 2020

This pull request fixes 1 alert when merging 511c735b3b1c649143b89a0466dd1ca2bcb6dd85 into ea8e38c - view on LGTM.com

fixed alerts:

  • 1 for Superclass attribute shadows subclass method

@lgtm-com
Copy link

lgtm-com bot commented Jun 8, 2020

This pull request fixes 1 alert when merging aa27c88c8462c5eca87ac4f34045798657e4ca27 into b91b6ff - view on LGTM.com

fixed alerts:

  • 1 for Superclass attribute shadows subclass method

patrys and others added 4 commits June 9, 2020 12:28
It's not very readable but we want to return the maximum available in any single shipping zone which means the loader needs to operate on several tables at once.

TODO:
- Figure out how to handle variant.track_inventory, it seems wasteful to calculate things for products that don't care about stock, maybe we should rethink requiring a stock record to exist?
@fowczarek fowczarek self-assigned this Jun 9, 2020
@codecov
Copy link

codecov bot commented Jun 9, 2020

Codecov Report

Merging #5707 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5707      +/-   ##
==========================================
+ Coverage   91.87%   91.90%   +0.03%     
==========================================
  Files         321      324       +3     
  Lines       20064    20164     +100     
  Branches     1836     1858      +22     
==========================================
+ Hits        18434    18532      +98     
- Misses       1197     1198       +1     
- Partials      433      434       +1     
Impacted Files Coverage Δ
saleor/graphql/core/dataloaders.py 94.44% <100.00%> (+0.32%) ⬆️
saleor/graphql/product/types/products.py 87.17% <100.00%> (+0.25%) ⬆️
saleor/graphql/warehouse/dataloaders.py 100.00% <100.00%> (ø)
saleor/product/utils/availability.py 97.95% <100.00%> (-0.09%) ⬇️
saleor/warehouse/availability.py 100.00% <100.00%> (ø)
saleor/graphql/menu/types.py 93.33% <0.00%> (-0.79%) ⬇️
saleor/graphql/shop/types.py 96.92% <0.00%> (-0.02%) ⬇️
saleor/graphql/page/dataloaders.py 100.00% <0.00%> (ø)
saleor/graphql/menu/dataloaders.py 100.00% <0.00%> (ø)

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 5a6d2a7...92815c4. Read the comment docs.

@db-queries
Copy link

db-queries bot commented Jun 9, 2020

Here is the report for 92815c4 (patrys/saleor @ quantity-available-dataloader)
Base comparison is 5a6d2a7.

**Found 6 differences!** (click me)

# saleor.graphql.accountbenchmark 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

# saleor.graphql.accountbenchmark permission group
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  permission group create                                    	         21	         21	              3
  permission group delete                                    	         22	         22	              4
  permission group query                                     	          8	          8	              0
  permission group update                                    	         36	         36	              2
  permission group update remove users with manage staff     	         31	         31	              4

# saleor.graphql.checkoutbenchmark checkout mutations
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  add billing address to checkout                            	         50	         50	             26
  add shipping to checkout                                   	         37	         37	             11
  checkout email update                                      	         28	         28	             13
  checkout payment charge                                    	         27	         27	              6
  checkout shipping address update                           	         33	         33	              8
  checkout voucher code                                      	         57	         57	             31
  complete checkout                                          	         73	         73	             19
+ create checkout                                            	        134	        130	             65
+ update checkout lines                                      	        101	         97	             45

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

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

# saleor.graphql.orderbenchmark order
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  user order details                                         	         17	         17	              2

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

# saleor.graphql.productbenchmark collection
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  collection view                                            	         17	         17	              0

# saleor.graphql.productbenchmark homepage
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  featured products list                                     	         14	         14	              0
  retrieve product list                                      	          5	          5	              0

# saleor.graphql.productbenchmark product
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
+ product details                                            	         17	         16	              0
  retrieve product attributes                                	          7	          7	              0

# saleor.graphql.productbenchmark variant
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  product variant bulk create                                	         48	         48	              2
+ retrieve variant list                                      	         30	         18	              2

# saleor.graphql.productbenchmark 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

# 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.shopbenchmark homepage
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  retrieve shop                                              	          2	          2	              0

@lgtm-com
Copy link

lgtm-com bot commented Jun 9, 2020

This pull request introduces 1 alert and fixes 1 when merging 811d9e19b9b155f71ee5943fc09a14a7f12cce5d into 5a6d2a7 - view on LGTM.com

new alerts:

  • 1 for Unused import

fixed alerts:

  • 1 for Superclass attribute shadows subclass method

@lgtm-com
Copy link

lgtm-com bot commented Jun 9, 2020

This pull request fixes 1 alert when merging b44dad1 into 5a6d2a7 - view on LGTM.com

fixed alerts:

  • 1 for Superclass attribute shadows subclass method

@fowczarek fowczarek requested a review from a team June 9, 2020 12:23
@fowczarek fowczarek marked this pull request as ready for review June 9, 2020 12:23
@patrys patrys requested a review from NyanKiyoshi June 9, 2020 14:44
@NyanKiyoshi NyanKiyoshi removed their request for review June 9, 2020 15:38
@fowczarek fowczarek requested review from maarcingebala and removed request for fowczarek June 10, 2020 08:45
@lgtm-com
Copy link

lgtm-com bot commented Jun 10, 2020

This pull request fixes 1 alert when merging 92815c4 into 89c7f35 - view on LGTM.com

fixed alerts:

  • 1 for Superclass attribute shadows subclass method

@maarcingebala maarcingebala merged commit e96a962 into saleor:master Jun 10, 2020
@patrys patrys deleted the quantity-available-dataloader branch June 10, 2020 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants