Ability to create order from dashboard #1971

mad-anne · 2018-03-23T15:00:21Z

I want to merge this change because it adds an ability to create order from dashboard. Those orders have DRAFT status and can be edited until confirmed by staff users. Draft status allows to add new products, change shipping, modify discount, assign customer or user email and edit shipping and billing address. Confirmed draft order changes status to UNFULFILLED and customer can be notified (optionally) about it.

Closes #1125

TODO's

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]

Draft order details dashboard page.

Order header for draft orders is Order draft #<number>. You can edit shipping, discount and discount amount by clicking on their names/values. You can edit customer and billing/shipping address by clicking Edit button next to them. Billing address contains Same as shipping address note if not differs from shipping address (but you can still edit it). Create order button is highlighted as the most important action.

Edit voucher modal.

You can choose any voucher to apply to an order without validation if it is correct for current state - it will be done while saving draft order. If some voucher is already applied, additional Remove voucher from order action appears in modal.

Edit customer modal.

You can set any existing user as a customer in a draft order (order is invisible until created) or just set an email, but only one option is accepted. If some customer is already set, additional Remove customer from order action appears in modal. Order can have no customer set - then customer is simple a Guest.

NyanKiyoshi · 2018-03-23T15:11:13Z

templates/dashboard/order/detail.html

+      {% if order.is_draft %}
+        <li>
+          <a href="#base-modal" data-href="{% url "dashboard:draft-order-delete" order_pk=order.pk %}" class="modal-trigger-custom">
+            {% trans "Remove order" context "Order detail action" %}


Would "Delete draft" be more descriptive?

NyanKiyoshi · 2018-03-23T15:14:28Z

templates/dashboard/order/modal/remove_order.html

+{% endblock %}
+
+{% block title %}
+{% trans "Remove order" context "Modal remove order title" %}


Same here. Maybe "Delete draft order".

You are right, it will be better - I will change it. But I will leave Remove word, as we use it more widely in dashboard.

codecov · 2018-03-23T15:14:55Z

Codecov Report

Merging #1971 into master will increase coverage by 0.06%.
The diff coverage is 81.17%.

@@            Coverage Diff             @@
##           master    #1971      +/-   ##
==========================================
+ Coverage   85.01%   85.08%   +0.06%     
==========================================
  Files         166      167       +1     
  Lines        6974     7420     +446     
  Branches      697      772      +75     
==========================================
+ Hits         5929     6313     +384     
- Misses        853      884      +31     
- Partials      192      223      +31

Impacted Files	Coverage Δ
saleor/dashboard/customer/urls.py	`100% <ø> (ø)`	⬆️
saleor/cart/utils.py	`97.05% <ø> (ø)`	⬆️
saleor/dashboard/discount/urls.py	`100% <ø> (ø)`	⬆️
saleor/discount/models.py	`84.53% <ø> (-0.16%)`	⬇️
saleor/dashboard/order/urls.py	`100% <ø> (ø)`	⬆️
saleor/dashboard/views.py	`48.27% <0%> (+1.4%)`	⬆️
saleor/account/views.py	`73.97% <100%> (ø)`	⬆️
saleor/shipping/models.py	`96.55% <100%> (+4.24%)`	⬆️
saleor/checkout/views/summary.py	`44.32% <100%> (+0.57%)`	⬆️
saleor/checkout/utils.py	`100% <100%> (ø)`
... and 18 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 40a5539...60febc4. Read the comment docs.

NyanKiyoshi · 2018-03-23T15:51:08Z

By any chance, would you have some time to put a task list/ todo list on your WiP PR? That would really help to know what bugs, what missing things and issues are known.

Since I also worked on my own implementation of this feature, it would be quite nice to work on yours!

Discount: fixed or percentage

What about allowing the staff user to chose between percentage or fixed discount? It would really be useful and needed in real use for the staff member. I do acknowledge it would require to put a flag for that, probably in the Order model in the current design.

We could calculate the fixed value in OrderEditDiscountForm through prices.discount.percentage_discount.

Optional shipping method field

What about IRL orders? If we don't have to ship an order that have products that require shipping? Maybe we should put the field as optional.

Removing selected customer

We can't switch between a selected customer to a guest (can't remove the selected value).

Optional shipping address

If we set (or create) a billing address, for example on a guest, we should default the shipping address to the billing address to avoid the staff user having to enter twice the same address.

Can't set order as paid (capture amount)

We can't mark an order as paid or capture amount on order (especially if the customer is a guest, as the users can't pay the order from their account).

NyanKiyoshi · 2018-03-25T06:42:52Z

saleor/order/__init__.py

    UNFULFILLED = 'unfulfilled'
    PARTIALLY_FULFILLED = 'partially fulfilled'
    FULFILLED = 'fulfilled'
    CANCELED = 'canceled'

    CHOICES = [
+        (DRAFT, pgettext_lazy('order status', 'Draft')),
        (UNFULFILLED, pgettext_lazy('order status', 'Unfulfilled')),


Should we profit from the fact that we are editing the order process, to make contexts more descriptive? To explain to the translators what trigger each status.

This is a clever idea, maybe the statuses' contexts could be more descriptive.

mad-anne · 2018-03-26T08:59:18Z

I can list some TODO's, but I will provide an exact list when we decide what exactly should be done in this PR. I listed some problems that I met in related issue #1125, as you saw.

Percentage discounts are a really good idea, but there are few problems when it comes to handle them properly. One thing that I plan to do for now is to recalculate an order total value every time the order state changes (e. g. products, shipping or a discount). So with percentage discount we should hold this information in an order. I would move this feature to separate PR.

Optional shipping method and removing customer (this one will be added for purpose of changing between an existing and unregistered user) are related to IRL orders and we have not supported this until now. We must decide if we want to.

I think that I could provide optional/default shipping address in some way in this PR. That's a good idea.

Marking order as paid is one of the features we plan to add in the future.

Thank you for your support with this feature 🙂 Your feedback is really helpful.

NyanKiyoshi · 2018-03-26T09:02:57Z

Sounds great for me! Thanks for the answer!

Pacu2

Tests missing

Pacu2 · 2018-03-30T07:06:58Z

saleor/checkout/utils.py

+    """Calculate discount value for a voucher of product or category type."""
+    if voucher.type == VoucherType.PRODUCT:
+        prices = [
+            item[1] for item in get_product_variants_and_prices(


Can we unpack here? I've seen that get_product_variants_and_prices returns 2 el tuple, variant and variant_price

Pacu2 · 2018-03-30T07:09:08Z

saleor/dashboard/customer/views.py

+            Q(email__icontains=search_query))
+
+    users = [
+        {'id': user.pk, 'text': user.ajax_label} for user in queryset]


I think you may use queryset.values() here, instead of putting dict manually

That's a good idea, but ajax_label is property, not database field.

Pacu2 · 2018-03-30T07:10:56Z

saleor/dashboard/order/forms.py

+
+
+class CreateOrderFromDraftForm(forms.ModelForm):
+    """Save draft order as a ready to fulfill."""


I think we can mark as ready to fulfill, not save, also an article before ready is not necessary

Pacu2 · 2018-03-30T07:12:55Z

saleor/dashboard/order/forms.py

+            shipping_address = self.instance.shipping_address
+            if (
+                method and shipping_address and
+                method.country_code != ANY_COUNTRY and


I think introducing variable for this would improve readability, that's lots of ands for simple if

Yes, I'll move it to variable 🙂

Pacu2 · 2018-03-30T07:14:45Z

saleor/dashboard/order/forms.py

+        model = Order
+        fields = []
+
+    def __init__(self, *args, **kwargs):


Pacu2 · 2018-03-30T07:15:51Z

saleor/dashboard/order/forms.py

+
+        if customer:
+            if (
+                customer.default_billing_address ==


Why are we doing this? Maybe some comment would be helpful, for one going back to this after some time

Cause if we set billing and shipping address in an order to default user addresses before, we want to unset this addresses when removing customer. But if custom changes were made, we leave it, as it might have been intended. I'll add a comment or maybe move whole logic of removing customer to separate function, marked with docstring.

Pacu2 · 2018-03-30T07:26:40Z

saleor/order/models.py

-        return self.status != OrderStatus.CANCELED
+        return self.status not in {OrderStatus.CANCELED, OrderStatus.DRAFT}
+
+    def can_mark_as_paid(self):


This will trigger new query when fired, I've seen its used only in template once, can we pass it from the view instead? I think payments should be already prefetched on order detail view

True, such helpers are very convenient, but those that perform database queries should be rather avoided.

Ok, I'll fix it.

Pacu2 · 2018-03-30T07:28:26Z

saleor/order/utils.py

    """Recalculate and assign total price of order.

    Total price is a sum of items in order and order shipping price minus
    discount amount.
    """
    prices = [line.get_total() for line in order]
    total = sum(prices, order.shipping_price)
+    # discount amount can't be greater than order total
+    order.discount_amount = min(
+        discount_amount or order.discount_amount or


I'd assign it to some sort of variable and then pass it to min() function, it would be more readable

Pacu2 · 2018-04-03T06:54:07Z

docs/architecture/orders.rst

@@ -34,3 +34,5 @@ Order status is based on statuses of its fulfillments. There are four possible s

 - ``CANCELED``
    Order has been canceled. Every fulfillment (if there is any) has ``CANCELED`` status. Order doesn't require further actions by a shop operator.
+
+There is also ``DRAFT`` status, used for orders created from dashboard.


It's not exactly for all orders created from the dashboard, but only for those which were not published.

Pacu2 · 2018-04-03T06:59:50Z

saleor/order/models.py

+    def drafts(self):
+        return self.filter(status=OrderStatus.DRAFT)
+
+    def to_ship(self):


I think we've started moving logic out of Object Managers in favour of utils helpers
eg. product/utils.py

We used to have heavy functions in managers with a lot of logic. But when you only have to prepare a specific queryset, then this is a good place.

Pacu2 · 2018-04-03T07:03:41Z

tests/conftest.py

@@ -52,8 +52,26 @@ def cart(db):  # pylint: disable=W0613


 @pytest.fixture
-def customer_user(db):  # pylint: disable=W0613
-    return User.objects.create_user('test@example.com', 'password')
+def default_billing_address(db):  # pylint: disable=W0613


This might be used as any address, not exactly default billing, for the purpose of creating customer_user, and as I'm aware - it is.
I think that address is a better fit, as and advantage we won't end up with several:
address = default_billing_address in tests, to fit everything within 80 chars

Pacu2 · 2018-04-04T11:38:36Z

tests/dashboard/test_order.py

@@ -1,15 +1,20 @@
 from decimal import Decimal
-from unittest.mock import patch
+from unittest.mock import MagicMock, Mock, patch


I think MagicMock is unused

maarcingebala · 2018-04-04T11:56:02Z

saleor/order/utils.py

@@ -59,6 +59,10 @@ def recalculate_order(order, **kwargs):
    Voucher discount amount is recalculated by default. To avoid this, pass
    update_voucher_discount argument set to False.
    """
+    # do not use prefetched order lines, cause they might have changed
+    if hasattr(order, '_prefetched_objects_cache'):


This is quite uncommon, wouldn't order.refresh_from_db() do the trick (if it fetches related objects which I'm not sure of)? Anyway, it's very intriguing that we have to remove prefetched data.

Unhopefully, refresh_from_db doesn't work. Don't have any other ideas right now.

mad-anne added the in progress label Mar 23, 2018

mad-anne added this to the 2018.03 milestone Mar 23, 2018

mad-anne self-assigned this Mar 23, 2018

NyanKiyoshi reviewed Mar 23, 2018

View reviewed changes

NyanKiyoshi reviewed Mar 25, 2018

View reviewed changes

mad-anne force-pushed the create-order branch from 0cfca8e to 8875b58 Compare March 26, 2018 07:15

NyanKiyoshi mentioned this pull request Mar 26, 2018

Add ability to create order from dashboard #1125

Closed

6 tasks

mad-anne force-pushed the create-order branch 4 times, most recently from ce1f6f5 to 94baaf5 Compare March 28, 2018 09:55

Pacu2 reviewed Mar 30, 2018

View reviewed changes

NyanKiyoshi mentioned this pull request Mar 30, 2018

Add notes to customers #2013

Closed

Pacu2 reviewed Apr 3, 2018

View reviewed changes

mad-anne force-pushed the create-order branch 3 times, most recently from 3b9d79a to 844170d Compare April 4, 2018 09:14

Pacu2 reviewed Apr 4, 2018

View reviewed changes

maarcingebala reviewed Apr 4, 2018

View reviewed changes

mad-anne added 6 commits April 4, 2018 15:55

Create order view

8811c83

Confirm draft order modal action

206007d

Display form non_field_errors in modals

4e87577

Test confirming draft order view

f337b47

Enable order edition only if draft

4a783e4

Connect an order to a user account or an email

8b5d528

mad-anne added 26 commits April 4, 2018 15:55

Remove customer from order action

c3b874c

Improve order statuses translations contexts

e424209

Skip sending emails in order when no user email

e1a85b8

Hide print and cancel order options in draft

fda131f

More descriptive namings

06a5eb1

Highlight create order action

e962edc

Add remove action to modals

55124ac

Remove unused base confirm delete modal

49cb961

Red non_field_errors in modals

8f2523d

Marking order as paid

ff18a93

Add guides to docs about draft orders

284770e

Improve ajax_label naming

8e40e6f

Make modals raw again

96be970

Show invoices for all not draft orders

6190c14

Add OrderQuerySet

9046c6d

UI improvements

2c7ea97

Code improvements after review

12d18a2

Tests for order discounts

41876a3

Add missing tests

ade329b

Decorate recalculate_order to recalculate voucher

fa012f4

Test order views and utils logic

3da3adf

Load prefetched lines again in recalculate_order

cf4e145

Remove unused imports

fad32d9

Hide actions bar below payments

3a5ede7

Get order lines directly in recalculate order

7ef88d2

Optimize order details page

60febc4

mad-anne force-pushed the create-order branch from 49fcd9d to 60febc4 Compare April 4, 2018 14:46

maarcingebala merged commit c9e2d94 into saleor:master Apr 4, 2018

mad-anne deleted the create-order branch April 5, 2018 06:51

mad-anne removed the in progress label Apr 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ability to create order from dashboard #1971

Ability to create order from dashboard #1971

mad-anne commented Mar 23, 2018 •

edited

NyanKiyoshi Mar 23, 2018

NyanKiyoshi Mar 23, 2018

mad-anne Mar 23, 2018 •

edited

codecov bot commented Mar 23, 2018 •

edited

NyanKiyoshi commented Mar 23, 2018 •

edited

NyanKiyoshi Mar 25, 2018 •

edited

maarcingebala Mar 26, 2018

mad-anne commented Mar 26, 2018 •

edited

NyanKiyoshi commented Mar 26, 2018

Pacu2 left a comment

Pacu2 Mar 30, 2018

Pacu2 Mar 30, 2018

mad-anne Apr 3, 2018

Pacu2 Mar 30, 2018

Pacu2 Mar 30, 2018

mad-anne Apr 3, 2018

Pacu2 Mar 30, 2018

Pacu2 Mar 30, 2018

mad-anne Apr 3, 2018

Pacu2 Mar 30, 2018

maarcingebala Mar 30, 2018

mad-anne Apr 3, 2018

Pacu2 Mar 30, 2018

Pacu2 Apr 3, 2018

Pacu2 Apr 3, 2018

maarcingebala Apr 3, 2018

Pacu2 Apr 3, 2018

Pacu2 Apr 4, 2018

maarcingebala Apr 4, 2018

mad-anne Apr 4, 2018



		class CreateOrderFromDraftForm(forms.ModelForm):
		"""Save draft order as a ready to fulfill."""

Ability to create order from dashboard #1971

Ability to create order from dashboard #1971

Conversation

mad-anne commented Mar 23, 2018 • edited

TODO's

Pull Request Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mad-anne Mar 23, 2018 • edited

Choose a reason for hiding this comment

codecov bot commented Mar 23, 2018 • edited

Codecov Report

NyanKiyoshi commented Mar 23, 2018 • edited

Discount: fixed or percentage

Optional shipping method field

Removing selected customer

Optional shipping address

Can't set order as paid (capture amount)

NyanKiyoshi Mar 25, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mad-anne commented Mar 26, 2018 • edited

NyanKiyoshi commented Mar 26, 2018

Pacu2 left a 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

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mad-anne commented Mar 23, 2018 •

edited

mad-anne Mar 23, 2018 •

edited

codecov bot commented Mar 23, 2018 •

edited

NyanKiyoshi commented Mar 23, 2018 •

edited

NyanKiyoshi Mar 25, 2018 •

edited

mad-anne commented Mar 26, 2018 •

edited