Updates the discount redemption cleanup code to be more resilient (#1748

)

* Moved when the order discounts are cleaned up, made the test more robust

* changed the methodology up again

ecommerce/models.py

@@ -655,10 +655,12 @@ def _get_or_create(
         # for the same product, if multiple exist, grab the first.
         if orders:
             order = orders.first()
-
+            # Clear discounts except for the most recent one
+            # If there aren't any discounts in the basket, clear them all
             for old_discount in order.discounts.all():
                 old_discount.delete()
 
+            order.refresh_from_db()
         else:
             order = Order.objects.create(
                 state=Order.STATE.PENDING,
@@ -694,6 +696,7 @@ def _get_or_create(
         order.total_price_paid = total
 
         order.save()
+
         return order
 
     @classmethod

ecommerce/models_test.py

@@ -423,7 +423,12 @@ def test_pending_order_is_reused(basket):
     assert Order.objects.filter(state=Order.STATE.PENDING).count() == 1
 
 
-def test_pending_order_is_reused_but_discounts_cleared(basket, unlimited_discount):
+@pytest.mark.parametrize(
+    "apply_discount", ["to_order", "to_basket", "to_second_basket"]
+)
+def test_pending_order_is_reused_but_discounts_cleared(
+    basket, unlimited_discount, apply_discount
+):
     """
     If a pending order is reused and had discounts, then we want those discounts
     to clear.
@@ -434,30 +439,67 @@ def test_pending_order_is_reused_but_discounts_cleared(basket, unlimited_discoun
 
     basket_item = BasketItem(product=product, basket=basket, quantity=1)
     basket_item.save()
+
+    if apply_discount == "to_basket":
+        basket_discount = BasketDiscount(
+            redemption_date=now_in_utc(),
+            redeemed_by=basket.user,
+            redeemed_discount=unlimited_discount,
+            redeemed_basket=basket,
+        )
+        basket_discount.save()
+        basket.refresh_from_db()
+
     order = PendingOrder.create_from_basket(basket)
     order.save()
     assert Order.objects.filter(state=Order.STATE.PENDING).count() == 1
 
-    redemption = DiscountRedemption(
-        redeemed_discount=unlimited_discount,
-        redemption_date=now_in_utc(),
-        redeemed_order=order,
-        redeemed_by=order.purchaser,
-    )
-    redemption.save()
+    if apply_discount == "to_order":
+        redemption = DiscountRedemption(
+            redeemed_discount=unlimited_discount,
+            redemption_date=now_in_utc(),
+            redeemed_order=order,
+            redeemed_by=order.purchaser,
+        )
+        redemption.save()
 
     order.refresh_from_db()
-    assert order.discounts.count() == 1
+    if apply_discount != "to_second_basket":
+        assert order.discounts.count() == 1
+    else:
+        assert order.discounts.count() == 0
+
+    if apply_discount == "to_second_basket":
+        a_different_discount = UnlimitedUseDiscountFactory.create()
+
+        basket_discount = BasketDiscount(
+            redemption_date=now_in_utc(),
+            redeemed_by=basket.user,
+            redeemed_discount=a_different_discount,
+            redeemed_basket=basket,
+        )
+        basket_discount.save()
 
     order = PendingOrder.create_from_basket(basket)
     order.save()
     order.refresh_from_db()
 
     # Verify that the existing PendingOrder is reused and a duplicate is not created.
     # This is to ensure that we also reuse the HubSpot Deal associated with Orders.
-    # Also ensure the discounts aren't reattached to the order
+    # Also ensure the discounts aren't reattached to the order if we just attached
+    # the discount to the order - if it's in the basket, it should be reattached, but we should only get one
     assert Order.objects.filter(state=Order.STATE.PENDING).count() == 1
-    assert order.discounts.count() == 0
+    if apply_discount == "to_order":
+        assert order.discounts.count() == 0
+    else:
+        order.discounts.count() == 1
+
+        # A different discount gets attached if there's a second basket so make
+        # sure the right one is there
+        if apply_discount == "to_second_basket":
+            assert order.discounts.first().redeemed_discount == a_different_discount
+        else:
+            assert order.discounts.first().redeemed_discount == unlimited_discount
 
 
 def test_new_pending_order_is_created_if_product_is_different():