Improve order fulfillment process #1777

mad-anne · 2018-02-12T17:40:12Z

I want to merge this change because it simplifies order related logic. DeliveryGroup model is removed with all logic related to it and all order lines are placed directly into Order. Fulfillment model is introduced and it represents single shipment of many order lines in given quantity. There may be many fulfillments in an order and each order line may be in many fulfillments.

Closes #1767, closes #1850

Pull Request Checklist

Privileged views and APIs are guarded by proper permission checks.
All visible strings are translated with proper context.
All data-formatting is locale-aware (dates, numbers, and so on).
Database queries are optimized and the number of queries is constant.
The changes are tested.
The code is documented (docstrings, project documentation).
Python code quality checks pass: pycodestyle, pydocstyle, pylint.
JavaScript code quality checks pass: eslint.

[Edit]

Order details dashboard page.

Fulfillment view.

Cancel fulfillment modal.

Order details storefront.

codecov · 2018-02-20T14:02:31Z

Codecov Report

Merging #1777 into master will decrease coverage by 0.17%.
The diff coverage is 60.49%.

@@            Coverage Diff             @@
##           master    #1777      +/-   ##
==========================================
- Coverage   83.52%   83.34%   -0.18%     
==========================================
  Files         145      145              
  Lines        6250     6179      -71     
  Branches      664      652      -12     
==========================================
- Hits         5220     5150      -70     
- Misses        856      858       +2     
+ Partials      174      171       -3

Impacted Files	Coverage Δ
saleor/dashboard/order/filters.py	`88.88% <ø> (+2.68%)`	⬆️
saleor/dashboard/order/urls.py	`100% <ø> (ø)`	⬆️
saleor/core/analytics.py	`74.19% <0%> (+2.31%)`	⬆️
saleor/dashboard/order/utils.py	`100% <100%> (ø)`	⬆️
saleor/account/views.py	`73.97% <100%> (ø)`	⬆️
saleor/product/utils.py	`87.39% <100%> (ø)`	⬆️
saleor/checkout/core.py	`92.59% <100%> (-0.07%)`	⬇️
saleor/dashboard/views.py	`46.87% <33.33%> (+0.2%)`	⬆️
saleor/dashboard/order/views.py	`53.58% <33.92%> (-2.53%)`	⬇️
saleor/dashboard/order/forms.py	`73.95% <58.69%> (-4.23%)`	⬇️
... and 4 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 ec5502c...3dbfb79. Read the comment docs.

codecov-io · 2018-02-22T10:37:27Z

Codecov Report

❗ No coverage uploaded for pull request base (master@9627bf5). Click here to learn what that means.
The diff coverage is 57.75%.

@@            Coverage Diff            @@
##             master    #1777   +/-   ##
=========================================
  Coverage          ?   83.94%           
=========================================
  Files             ?      151           
  Lines             ?     6427           
  Branches          ?      665           
=========================================
  Hits              ?     5395           
  Misses            ?      857           
  Partials          ?      175

Impacted Files	Coverage Δ
saleor/dashboard/order/urls.py	`100% <ø> (ø)`
saleor/dashboard/order/filters.py	`88.88% <ø> (ø)`
saleor/core/analytics.py	`74.19% <0%> (ø)`
saleor/product/utils.py	`87.23% <100%> (ø)`
saleor/checkout/core.py	`92.8% <100%> (ø)`
saleor/dashboard/order/utils.py	`100% <100%> (ø)`
saleor/account/views.py	`73.97% <100%> (ø)`
saleor/dashboard/views.py	`46.87% <33.33%> (ø)`
saleor/dashboard/order/views.py	`58.77% <34.28%> (ø)`
saleor/order/emails.py	`81.81% <38.46%> (ø)`
... and 4 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 9627bf5...c9da395. Read the comment docs.

patrys · 2018-02-27T12:04:13Z

docs/architecture/orders.rst


-There are three possible delivery group statuses:
+There are two possible fulfillment statuses:


Shouldn't there exist a third status to indicate which groups are already dispatched?

I came from the assumption, that we consider fulfilled group as dispatched. This is why user also is asked for tracking number, when fulfilling the items.

How does that work with packing slips? The usual flow for a warehouse (where packing slips are used) is:

dispatcher creates a shipment group (fulfillment)

warehouse prints a packing slip

warehouse collects and packages items

warehouse dispatches the order (this is usually the moment you know the tracking number)

For now we could leave this as it is, cause it's quite simple. We can discuss further changes in the flow and open new PR if necessary.

patrys · 2018-02-27T12:05:54Z

saleor/order/migrations/0034_auto_20180210_0422.py

+            model_name='order',
+            name='last_status_change',
+        ),
+        migrations.RunPython(assign_order_to_lines, migrations.RunPython.noop)


Can we please move the RunPython action to a separate migration file?

rafalp · 2018-03-02T13:29:39Z

@mad-anne could you please update your PR's description to mention that it also fixes the #1850 ? Thanks!

maarcingebala · 2018-03-07T10:42:03Z

saleor/order/models.py

+
+    @property
+    def quantity_fulfilled(self):
+        lines = FulfillmentLine.objects.filter(


Isn't the logic here too much for a property? I would expect from a property to perform rather simple logic based on model fields. Here we're performing a database query. I'm afraid that sooner or later this will lead to duplicated queries or someone will call it in a template (performing queries from templates isn't a good practice). Wouldn't it be better to denormalize this value and store it in the database? @patrys What's your view on that?

👍 from my end. Executing queries in a property adds a hidden cost to something that looks innocent plus there is no way to optimize it.

As for the logic if FulfillmentLine objects for the order are already prefetched then iterating over lines.all() and filtering in Python is cheaper (as it won't touch the database) than executing a filter().

True, if we did the filtering in Python then such logic would be acceptable for a property in my opinion. I assume that anyone using this will have to ensure proper prefetching.

mad-anne added the in progress label Feb 12, 2018

mad-anne added this to the 2018.02 milestone Feb 12, 2018

mad-anne self-assigned this Feb 12, 2018

mad-anne force-pushed the order-fulfillment branch 2 times, most recently from 2ee01cd to 31f7f46 Compare February 20, 2018 11:28

mad-anne force-pushed the order-fulfillment branch 2 times, most recently from 5b05ada to 0850e76 Compare February 21, 2018 13:39

mad-anne force-pushed the order-fulfillment branch from e337d73 to f4e170d Compare February 22, 2018 10:47

mad-anne requested review from akjanik, patrys, rafalp, maarcingebala and dominik-zeglen February 27, 2018 11:11

patrys reviewed Feb 27, 2018

View reviewed changes

mad-anne force-pushed the order-fulfillment branch 2 times, most recently from 889a28d to dc6610a Compare March 2, 2018 11:37

akjanik mentioned this pull request Mar 2, 2018

Random order generator should also randomly set shipping price #1850

Closed

maarcingebala added the merge-conflict label Mar 5, 2018

mad-anne force-pushed the order-fulfillment branch 2 times, most recently from 6cafb3a to 669a38d Compare March 7, 2018 09:14

maarcingebala modified the milestones: 2018.02, 2018.03 Mar 7, 2018

maarcingebala removed the merge-conflict label Mar 7, 2018

maarcingebala reviewed Mar 7, 2018

View reviewed changes

mad-anne force-pushed the order-fulfillment branch from 22f5d06 to ff947ad Compare March 7, 2018 15:01

mad-anne mentioned this pull request Mar 12, 2018

"Cancel Shipment" option confusing for an unshipped order. #1889

Closed

mad-anne added 24 commits March 13, 2018 12:48

Add and edit tracking number actions

72ef579

Do not count canceled quantity in quantity_fulfilled

ff674d0

Packing slips per fulfillment

062552d

Make migrations reversible

b76c91a

Move RunPython to separate migration file

290d025

Send fulfillment confirmation email to user

572bd5b

Test updating order status

defcb5e

Send shipping update email

2ec575f

Fix migrations after rebase

da0e02c

Fix failing tests

356da59

Hide packing slips for canceled fulfillment

5222a61

Merge order price summary to order card

608d077

Improve order details dashboard view

b07d47d

Fit fulfill order view to order details

6d69d4a

Improve displaying fulfillment quantity

628ea65

Unify order details styling

3558226

Fix test

cc7b011

Save quantity_fulfilled to database

d1b3252

Fix tests and styling

2c5d07c

Test changing quantity_fulfilled

d1db1a0

Improve edit fulfillment tracking modal

83aa3fe

Improve order details storefront page

d284220

Fix migrations

ad9cef5

Omit canceled fulfillments in order details

c9da395

mad-anne force-pushed the order-fulfillment branch from 5580b38 to c9da395 Compare March 13, 2018 12:00

maarcingebala merged commit ecd86ad into saleor:master Mar 13, 2018

maarcingebala removed the in progress label Mar 13, 2018

maarcingebala mentioned this pull request Mar 13, 2018

Be able to create an order from dashboard #1904

Closed

mad-anne deleted the order-fulfillment branch March 19, 2018 14:22

mad-anne mentioned this pull request Mar 23, 2018

Remove partition logic from checkout and cart #1969

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve order fulfillment process #1777

Improve order fulfillment process #1777

mad-anne commented Feb 12, 2018 •

edited by maarcingebala

codecov bot commented Feb 20, 2018 •

edited

codecov-io commented Feb 22, 2018 •

edited by codecov bot

patrys Feb 27, 2018

mad-anne Feb 27, 2018

patrys Feb 27, 2018

mad-anne Feb 28, 2018

patrys Feb 27, 2018

rafalp commented Mar 2, 2018

maarcingebala Mar 7, 2018

patrys Mar 7, 2018

maarcingebala Mar 7, 2018


		There are three possible delivery group statuses:
		There are two possible fulfillment statuses:

Improve order fulfillment process #1777

Improve order fulfillment process #1777

Conversation

mad-anne commented Feb 12, 2018 • edited by maarcingebala

Pull Request Checklist

codecov bot commented Feb 20, 2018 • edited

Codecov Report

codecov-io commented Feb 22, 2018 • edited by codecov bot

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafalp commented Mar 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mad-anne commented Feb 12, 2018 •

edited by maarcingebala

codecov bot commented Feb 20, 2018 •

edited

codecov-io commented Feb 22, 2018 •

edited by codecov bot