Repository: saleor/saleor
Pull request: Match orders with a new user
closes [#5398](https://github.com/mirumee/saleor/issues/5398)
<!-- Please mention all relevant issue numbers. -->


# Impact

* [ ] New migrations
* [ ] New/Updated API fields or mutations
* [ ] Deprecated API fields or mutations
* [ ] Removed API types, fields, or mutations

# Pull Request Checklist

<!-- Please keep this section. It will make maintainer's life easier. -->

* [X] Privileged queries and mutations are guarded by proper permission checks
* [X] Database queries are optimized and the number of queries is constant
* [X] Database migration files are up to date
* [X] The changes are tested
* [X] GraphQL schema and type definitions are up to date
* [X] Changes are mentioned in the changelog


Problem statement:
Match orders with new customer accounts
### What I'm trying to achieve
> Affect order to customer.

Case: 
Customer create an account and validité this account only after passing and order.
Order is not affect to this customer (**user_id** empty in **order_order** table)

### Describe a proposed solution
Modify mutation **orderUdapte** and add into **OrderUpdateInput**, **user_id field**.

Thanks

Hints from issue discussion:
Hey, correct me if I'm wrong - you want to be able to match existing orders with a user account? 

Example case - as a customer I placed an order with email "customer@example.com" (but I don't have any account in Saleor). Later I created an account for email "customer@example.com" and I don't see the order that I placed in the past. You want to be able to update the old order to match it with my new account?
Yes !!! :-)

Thierry BOULOGNE
06 43 18 31 19

> Le 27 mars 2020 à 12:21, Marcin Gębala <notifications@github.com> a écrit :
> 
> Hey, correct me if I'm wrong - you want to be able to match existing orders with a user account?
> 
> Example case - as a customer I placed an order with email "customer@example.com" (but I don't have any account in Saleor). Later I created an account for email "customer@example.com" and I don't see the order that I placed in the past. You want to be able to update the old order to match it with my new account?
> 
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub, or unsubscribe.

OK, so I think this should happen automatically when you create a new account. We used to have it in the "old" Saleor (in Django views) and should be added to the API as well. 
I mean we don't need a special mutation for that, instead, the mutation to create a new account (or confirm it, if there is email confirmation enabled) should look for orders with the email the account was created for and match it.
@maarcingebala on Account activation could be great.
OK, I'll add it to our backlog and we'll try to take this issue as soon as we can.
thanks
### Proposed solution

So I suggest that we add a function that looks for orders for the given email address and matches them with a new user. This function would be used in API mutations:
- `accountRegister` mutation - when email confirmation for new accounts is disabled 
- `confirmAccount` - when it's enabled.

Corner cases to consider:
- user can change the email address once the account is active - should we also match orders in this case? Edit: changing the email should require users to confirm the new address. If we implement the matching logic in `confirmAccount` mutation it should be also used when confirming new email addresses.

Regarding the corner case... email should be link to an activation.
It's mean,  that user need to activate (confirm) he is the owner of the email. Don't know how saleor manage this...

So no match on email change, only on activation :-).

Commit message: Add unit tests

Patch to review:
diff --git a/saleor/graphql/account/mutations/account.py b/saleor/graphql/account/mutations/account.py
index 11513557c5d..da791f27cd9 100644
--- a/saleor/graphql/account/mutations/account.py
+++ b/saleor/graphql/account/mutations/account.py
@@ -8,6 +8,7 @@
 from ....account.utils import create_jwt_token, decode_jwt_token
 from ....checkout import AddressType
 from ....core.utils.url import validate_storefront_url
+from ....order.utils import match_orders_with_new_user
 from ...account.enums import AddressTypeEnum
 from ...account.types import Address, AddressInput, User
 from ...core.mutations import BaseMutation, ModelDeleteMutation, ModelMutation
@@ -88,6 +89,7 @@ def save(cls, info, user, cleaned_input):
             emails.send_account_confirmation_email(user, cleaned_input["redirect_url"])
         else:
             user.save()
+            match_orders_with_new_user(user)
         account_events.customer_account_created_event(user=user)
         info.context.extensions.customer_created(customer=user)
 
diff --git a/saleor/graphql/account/mutations/base.py b/saleor/graphql/account/mutations/base.py
index 5a0c5ed6720..ba3e4e51d6d 100644
--- a/saleor/graphql/account/mutations/base.py
+++ b/saleor/graphql/account/mutations/base.py
@@ -13,6 +13,7 @@
 from ....account.error_codes import AccountErrorCode
 from ....core.permissions import AccountPermissions
 from ....core.utils.url import validate_storefront_url
+from ....order.utils import match_orders_with_new_user
 from ...account.i18n import I18nMixin
 from ...account.types import Address, AddressInput, User
 from ...core.mutations import (
@@ -186,6 +187,7 @@ def perform_mutation(cls, _root, info, **data):
 
         user.is_active = True
         user.save(update_fields=["is_active"])
+        match_orders_with_new_user(user)
 
         return ConfirmAccount()
 
diff --git a/saleor/order/utils.py b/saleor/order/utils.py
index 9f3c93c8cda..824f2d15111 100644
--- a/saleor/order/utils.py
+++ b/saleor/order/utils.py
@@ -5,6 +5,7 @@
 from django.utils import timezone
 from prices import Money, TaxedMoney
 
+from ..account.models import User
 from ..core.taxes import zero_money
 from ..core.weight import zero_weight
 from ..discount.models import NotApplicable, Voucher, VoucherType
@@ -333,3 +334,8 @@ def get_voucher_discount_for_order(order: Order) -> Money:
     if order.voucher.type == VoucherType.SPECIFIC_PRODUCT:
         return get_products_voucher_discount_for_order(order.voucher)
     raise NotImplementedError("Unknown discount type")
+
+
+def match_orders_with_new_user(user: User) -> None:
+    """Match Orders with a new user."""
+    Order.objects.filter(user_email=user.email, user=None).update(user=user)
diff --git a/tests/api/test_account.py b/tests/api/test_account.py
index 9931b60532f..69cb1c62a86 100644
--- a/tests/api/test_account.py
+++ b/tests/api/test_account.py
@@ -607,7 +607,10 @@ def test_user_with_cancelled_fulfillments(
     ENABLE_ACCOUNT_CONFIRMATION_BY_EMAIL=True, ALLOWED_CLIENT_HOSTS=["localhost"]
 )
 @patch("saleor.account.emails._send_account_confirmation_email")
-def test_customer_register(send_account_confirmation_email_mock, api_client):
+@patch("saleor.graphql.account.mutations.account.match_orders_with_new_user")
+def test_customer_register(
+    match_orders_with_new_user_mock, send_account_confirmation_email_mock, api_client
+):
     email = "customer@example.com"
     variables = {
         "email": email,
@@ -633,18 +636,21 @@ def test_customer_register(send_account_confirmation_email_mock, api_client):
     customer_creation_event = account_events.CustomerEvent.objects.get()
     assert customer_creation_event.type == account_events.CustomerEvents.ACCOUNT_CREATED
     assert customer_creation_event.user == new_user
+    assert match_orders_with_new_user_mock.call_count == 0
 
 
 @override_settings(ENABLE_ACCOUNT_CONFIRMATION_BY_EMAIL=False)
 @patch("saleor.account.emails._send_account_confirmation_email")
+@patch("saleor.graphql.account.mutations.account.match_orders_with_new_user")
 def test_customer_register_disabled_email_confirmation(
-    send_account_confirmation_email_mock, api_client
+    match_orders_with_new_user_mock, send_account_confirmation_email_mock, api_client
 ):
     variables = {"email": "customer@example.com", "password": "Password"}
     response = api_client.post_graphql(ACCOUNT_REGISTER_MUTATION, variables)
     errors = response.json()["data"]["accountRegister"]["errors"]
     assert errors == []
     assert send_account_confirmation_email_mock.delay.call_count == 0
+    assert match_orders_with_new_user_mock.call_count == 1
 
 
 @override_settings(ENABLE_ACCOUNT_CONFIRMATION_BY_EMAIL=True)
diff --git a/tests/test_order_utils.py b/tests/test_order_utils.py
index 01c75dddbc1..799a883e4e2 100644
--- a/tests/test_order_utils.py
+++ b/tests/test_order_utils.py
@@ -1,8 +1,8 @@
 import pytest
 
 from saleor.order.events import OrderEvents
-from saleor.order.models import OrderEvent
-from saleor.order.utils import change_order_line_quantity
+from saleor.order.models import Order, OrderEvent
+from saleor.order.utils import change_order_line_quantity, match_orders_with_new_user
 
 
 @pytest.mark.parametrize(
@@ -44,3 +44,20 @@ def test_change_quantity_generates_proper_event(
             {"quantity": expected_quantity, "line_pk": line.pk, "item": str(line)}
         ]
     }
+
+
+def test_match_orders_with_new_user(customer_user):
+    def get_order_user_by_customer_email(email):
+        return Order.objects.filter(user_email=email).first().user
+
+    customer_email = customer_user.email
+    address = customer_user.default_billing_address.get_copy()
+    Order.objects.create(
+        billing_address=address, user=None, user_email=customer_email,
+    )
+
+    assert get_order_user_by_customer_email(customer_email) is None
+
+    match_orders_with_new_user(customer_user)
+
+    assert get_order_user_by_customer_email(customer_email) == customer_user


Code review comments:
diff_hunk: @@ -633,18 +636,21 @@ def test_customer_register(send_account_confirmation_email_mock, api_client):
     customer_creation_event = account_events.CustomerEvent.objects.get()
     assert customer_creation_event.type == account_events.CustomerEvents.ACCOUNT_CREATED
     assert customer_creation_event.user == new_user
+    assert match_orders_with_new_user_mock.call_count == 0
 
 
 @override_settings(ENABLE_ACCOUNT_CONFIRMATION_BY_EMAIL=False)
 @patch("saleor.account.emails._send_account_confirmation_email")
+@patch("saleor.graphql.account.mutations.account.match_orders_with_new_user")
 def test_customer_register_disabled_email_confirmation(
-    send_account_confirmation_email_mock, api_client
+    match_orders_with_new_user_mock, send_account_confirmation_email_mock, api_client
 ):
     variables = {"email": "customer@example.com", "password": "Password"}
     response = api_client.post_graphql(ACCOUNT_REGISTER_MUTATION, variables)
     errors = response.json()["data"]["accountRegister"]["errors"]
     assert errors == []
     assert send_account_confirmation_email_mock.delay.call_count == 0
+    assert match_orders_with_new_user_mock.call_count == 1
path: tests/api/test_account.py
text: @user1:
We should use `assert_called_once_with` instead of call_count.

diff_hunk: @@ -44,3 +44,20 @@ def test_change_quantity_generates_proper_event(
             {"quantity": expected_quantity, "line_pk": line.pk, "item": str(line)}
         ]
     }
+
+
+def test_match_orders_with_new_user(customer_user):
+    def get_order_user_by_customer_email(email):
+        return Order.objects.filter(user_email=email).first().user
+
+    customer_email = customer_user.email
+    address = customer_user.default_billing_address.get_copy()
+    Order.objects.create(
+        billing_address=address, user=None, user_email=customer_email,
+    )
+
+    assert get_order_user_by_customer_email(customer_email) is None
+
+    match_orders_with_new_user(customer_user)
+
+    assert get_order_user_by_customer_email(customer_email) == customer_user
path: tests/test_order_utils.py
text: @user1:
```suggestion
    order = Order.objects.create(
        billing_address=address, user=None, user_email=customer_email,
    )

    match_orders_with_new_user(customer_user)
    order.refresh_from_db()
    assert order.user == customer_user
```

I think this test is more readable.

diff_hunk: @@ -633,18 +636,21 @@ def test_customer_register(send_account_confirmation_email_mock, api_client):
     customer_creation_event = account_events.CustomerEvent.objects.get()
     assert customer_creation_event.type == account_events.CustomerEvents.ACCOUNT_CREATED
     assert customer_creation_event.user == new_user
+    assert match_orders_with_new_user_mock.call_count == 0
path: tests/api/test_account.py
text: @user1:
We should use `assert_not_called()` instead of call_count.