Additional kwargs, better efficiency for get_valid_coupon_versions qu…

…ery (#243)

ecommerce/api.py

@@ -10,7 +10,7 @@
 import uuid
 
 from django.conf import settings
-from django.db.models import Q, Max, F, Count
+from django.db.models import Q, Max, F, Count, Subquery
 from django.db import transaction
 from rest_framework.exceptions import ValidationError
 
@@ -153,7 +153,9 @@ def latest_coupon_version(coupon):
     return coupon.versions.order_by("-created_on").first()
 
 
-def get_valid_coupon_versions(product, user, auto_only=False, code=None):
+def get_valid_coupon_versions(
+    product, user, auto_only=False, code=None, full_discount=False, company=None
+):  # pylint:disable=too-many-arguments
     """
     Given a list of coupon ids, determine which of them are valid based on payment version dates and redemptions.
 
@@ -162,85 +164,83 @@ def get_valid_coupon_versions(product, user, auto_only=False, code=None):
         user (User): User of coupons
         auto_only (bool): Whether or not to filter by automatic=True
         code (str): A coupon code to filter by
+        full_discount (bool): If true, only include 100% off coupons
+        company (Company): a company to filter by
 
     Returns:
         list of CouponVersion: CouponVersion objects sorted by discount, highest first.
     """
-    valid_coupons = []
     now = now_in_utc()
 
-    with transaction.atomic():
-        # Get the ids of the latest coupon versions
-        product_coupon_query = CouponEligibility.objects.select_related(
-            "coupon"
-        ).filter(product=product, coupon__enabled=True)
-        if code:
-            product_coupon_query = product_coupon_query.filter(coupon__coupon_code=code)
-
-        cv_latest = (
-            CouponVersion.objects.select_related("coupon")
-            .filter(coupon__in=product_coupon_query.values_list("coupon", flat=True))
-            .order_by("coupon", "-created_on")
-            .distinct("coupon")
-            .values_list("pk")
-        )
-
-        # filter by expiration and activation dates
-        query = (
-            CouponVersion.objects.select_related("payment_version")
-            .filter(pk__in=cv_latest)
-            .filter(
-                Q(payment_version__expiration_date__gte=now)
-                | Q(payment_version__expiration_date__isnull=True)
-            )
-            .filter(
-                Q(payment_version__activation_date__lte=now)
-                | Q(payment_version__activation_date__isnull=True)
-            )
+    # Get enabled coupons eligible for the product
+    product_coupon_subquery = CouponEligibility.objects.select_related("coupon").filter(
+        product=product, coupon__enabled=True
+    )
+    if code:
+        product_coupon_subquery = product_coupon_subquery.filter(
+            coupon__coupon_code=code
         )
 
-        if auto_only:
-            query = query.filter(payment_version__automatic=True)
+    # Get the latest versions for product coupons
+    coupon_version_subquery = CouponVersion.objects.filter(
+        coupon__in=Subquery(product_coupon_subquery.values_list("coupon", flat=True))
+    )
 
-        # filter by redemption counts
-        for coupon_version in query:
-            redemptions_global = (
-                CouponRedemption.objects.select_related("coupon_version", "order")
-                .filter(coupon_version=coupon_version)
-                .filter(order__status=Order.FULFILLED)
-            )
-            redemptions_user = redemptions_global.filter(order__purchaser=user)
-            if (
-                coupon_version.payment_version.max_redemptions
-                > redemptions_global.count()
-                and coupon_version.payment_version.max_redemptions_per_user
-                > redemptions_user.count()
-            ):
-                valid_coupons.append(coupon_version)
-        return sorted(
-            valid_coupons, key=lambda x: x.payment_version.amount, reverse=True
+    if full_discount:
+        coupon_version_subquery = coupon_version_subquery.filter(
+            payment_version__amount=decimal.Decimal(1)
         )
 
+    if auto_only:
+        coupon_version_subquery = coupon_version_subquery.filter(
+            payment_version__automatic=True
+        )
 
-def best_coupon_for_basket(basket, auto_only=False, code=None):
-    """
-    Get the best eligible coupon for the basket.
-    Assumes that the basket only contains one item/product.
+    if company is not None:
+        coupon_version_subquery = coupon_version_subquery.filter(
+            payment_version__company=company
+        )
 
-    Args:
-        basket (Basket): the basket Object
-        auto_only (bool): Only retrieve `automatic` Coupons
-        code (str): A coupon code to filter by
+    coupon_version_subquery = coupon_version_subquery.order_by(
+        "coupon", "-created_on"
+    ).distinct("coupon")
 
-    Returns:
-        CouponVersion: the CouponVersion with the highest discount, or None
-    """
-    basket_item = basket.basketitems.first()
-    if basket_item:
-        return best_coupon_for_product(
-            basket_item.product, basket.user, auto_only=auto_only, code=code
+    # Exclude versions with too many redemptions or active dates outside of today.
+    query = (
+        CouponVersion.objects.select_related("coupon", "payment_version")
+        .filter(pk__in=Subquery(coupon_version_subquery.values_list("pk", flat=True)))
+        .filter(
+            Q(payment_version__expiration_date__gte=now)
+            | Q(payment_version__expiration_date__isnull=True)
         )
-    return None
+        .filter(
+            Q(payment_version__activation_date__lte=now)
+            | Q(payment_version__activation_date__isnull=True)
+        )
+        .annotate(
+            user_redemptions=(
+                Count(
+                    "couponredemption",
+                    filter=(
+                        Q(couponredemption__order__purchaser=user)
+                        & Q(couponredemption__order__status=Order.FULFILLED)
+                    ),
+                )
+            )
+        )
+        .annotate(
+            global_redemptions=(
+                Count(
+                    "couponredemption",
+                    filter=(Q(couponredemption__order__status=Order.FULFILLED)),
+                )
+            )
+        )
+        .exclude(user_redemptions__gte=F("payment_version__max_redemptions_per_user"))
+        .exclude(global_redemptions__gte=F("payment_version__max_redemptions"))
+    )
+
+    return query.order_by("-payment_version__amount")
 
 
 def best_coupon_for_product(product, user, auto_only=False, code=None):

ecommerce/api_test.py

@@ -22,7 +22,6 @@
     ISO_8601_FORMAT,
     make_reference_id,
     redeem_coupon,
-    best_coupon_for_basket,
     get_new_order_by_reference_number,
     get_product_price,
     get_product_version_price_with_discount,
@@ -37,7 +36,6 @@
 from ecommerce.exceptions import EcommerceException, ParseException
 from ecommerce.factories import (
     BasketFactory,
-    BasketItemFactory,
     CouponRedemptionFactory,
     CouponSelectionFactory,
     CouponVersionFactory,
@@ -223,10 +221,12 @@ def test_get_valid_coupon_versions(basket_and_coupons, auto_only):
     """
     Verify that the correct valid CouponPaymentVersions are returned for a list of coupons
     """
-    best_versions = get_valid_coupon_versions(
-        basket_and_coupons.basket_item.product,
-        basket_and_coupons.basket_item.basket.user,
-        auto_only,
+    best_versions = list(
+        get_valid_coupon_versions(
+            basket_and_coupons.basket_item.product,
+            basket_and_coupons.basket_item.basket.user,
+            auto_only,
+        )
     )
     expected_versions = [basket_and_coupons.coupongroup_worst.coupon_version]
     if not auto_only:
@@ -246,13 +246,39 @@ def test_get_valid_coupon_versions_bad_dates(basket_and_coupons):
     civ_best.expiration_date = today - timedelta(days=1)
     civ_best.save()
 
-    best_versions = get_valid_coupon_versions(
-        basket_and_coupons.basket_item.product,
-        basket_and_coupons.basket_item.basket.user,
+    best_versions = list(
+        get_valid_coupon_versions(
+            basket_and_coupons.basket_item.product,
+            basket_and_coupons.basket_item.basket.user,
+        )
     )
     assert best_versions == []
 
 
+def test_get_valid_coupon_versions_full_discount(basket_and_coupons):
+    """Verify that only 100% coupons are returned if full_discount kwarg is True"""
+    assert list(
+        get_valid_coupon_versions(
+            basket_and_coupons.basket_item.product,
+            basket_and_coupons.basket_item.basket.user,
+            full_discount=True,
+        )
+    ) == [basket_and_coupons.coupongroup_best.coupon_version]
+    assert basket_and_coupons.coupongroup_best.payment_version.amount == Decimal(1.0)
+
+
+def test_get_valid_coupon_versions_by_company(basket_and_coupons):
+    """Verify that valid coupons are filtered by company"""
+    company = basket_and_coupons.coupongroup_worst.payment_version.company
+    assert list(
+        get_valid_coupon_versions(
+            basket_and_coupons.basket_item.product,
+            basket_and_coupons.basket_item.basket.user,
+            company=company,
+        )
+    ) == [basket_and_coupons.coupongroup_worst.coupon_version]
+
+
 @pytest.mark.parametrize("order_status", [Order.FULFILLED, Order.FAILED])
 def test_get_valid_coupon_versions_over_redeemed(basket_and_coupons, order_status):
     """
@@ -276,9 +302,11 @@ def test_get_valid_coupon_versions_over_redeemed(basket_and_coupons, order_statu
         ),
     )
 
-    best_versions = get_valid_coupon_versions(
-        basket_and_coupons.basket_item.product,
-        basket_and_coupons.basket_item.basket.user,
+    best_versions = list(
+        get_valid_coupon_versions(
+            basket_and_coupons.basket_item.product,
+            basket_and_coupons.basket_item.basket.user,
+        )
     )
     if order_status == Order.FULFILLED:
         assert best_versions == []
@@ -289,65 +317,6 @@ def test_get_valid_coupon_versions_over_redeemed(basket_and_coupons, order_statu
         ]
 
 
-@pytest.mark.parametrize("auto_only", [True, False])
-def test_get_best_coupon_for_basket(basket_and_coupons, auto_only):
-    """
-    Verify that the CouponPaymentVersion with the best price is returned for a bucket based on auto filter
-    """
-    best_cv = best_coupon_for_basket(
-        basket_and_coupons.basket_item.basket, auto_only=auto_only
-    )
-    if auto_only:
-        assert best_cv == basket_and_coupons.coupongroup_worst.coupon_version
-    else:
-        assert best_cv == basket_and_coupons.coupongroup_best.coupon_version
-
-
-@pytest.mark.parametrize("code", ["WORST", None])
-def test_get_best_coupon_for_basket_by_code(basket_and_coupons, code):
-    """
-    Verify that the CouponPaymentVersion with the best price is returned for a bucket based on coupon code
-    """
-    best_cv = best_coupon_for_basket(
-        basket_and_coupons.basket_item.basket, auto_only=False, code=code
-    )
-    if code:
-        assert best_cv == basket_and_coupons.coupongroup_worst.coupon_version
-    else:
-        assert best_cv == basket_and_coupons.coupongroup_best.coupon_version
-
-
-def test_get_best_coupon_for_basket_empty_basket():
-    """
-    Verify that the best_coupon_version() returns None if the basket has no product
-    """
-    assert best_coupon_for_basket(BasketFactory()) is None
-
-
-def test_get_best_coupon_for_basket_no_coupons():
-    """
-    Verify that best_coupon_version() returns None if the product has no coupons
-    """
-    basket_item = BasketItemFactory()
-    ProductVersionFactory(product=basket_item.product, price=Decimal(25.00))
-    assert best_coupon_for_basket(basket_item.basket) is None
-
-
-def test_get_best_coupon_for_basket_no_valid_coupons(basket_and_coupons):
-    """
-    Verify that best_coupon_version() returns None if the product coupons are invalid
-    """
-    today = now_in_utc()
-    civ_worst = basket_and_coupons.coupongroup_worst.coupon_version.payment_version
-    civ_worst.activation_date = today + timedelta(days=1)
-    civ_worst.save()
-
-    assert (
-        best_coupon_for_basket(basket_and_coupons.basket_item.basket, code="WORST")
-        is None
-    )
-
-
 def test_latest_coupon_version(basket_and_coupons):
     """
     Verify that the most recent coupon version is returned

ecommerce/conftest.py

@@ -58,7 +58,7 @@ def basket_and_coupons():
         payment=payment_best, amount=Decimal("0.50")
     )
     # Coupon payment for best coupon, more recent than previous so takes precedence
-    civ_best = CouponPaymentVersionFactory(payment=payment_best, amount=Decimal("0.40"))
+    civ_best = CouponPaymentVersionFactory(payment=payment_best, amount=Decimal("1.00"))
 
     # Coupon version for worst coupon
     cv_worst = CouponVersionFactory(payment_version=civ_worst, coupon=coupon_worst)

ecommerce/serializers.py

@@ -544,18 +544,28 @@ def create(self, validated_data):
                 payment_type=validated_data.get("payment_type"),
                 payment_transaction=validated_data.get("payment_transaction"),
             )
-            for coupon in range(validated_data.get("num_coupon_codes")):
-                coupon = models.Coupon.objects.create(
+
+            eligibilities = []
+
+            coupons = [
+                models.Coupon(
                     coupon_code=validated_data.get("coupon_code", uuid4().hex),
                     payment=payment,
                 )
-                models.CouponVersion.objects.create(
-                    coupon=coupon, payment_version=payment_version
-                )
-                for product_id in validated_data.get("product_ids"):
-                    models.CouponEligibility.objects.create(
-                        coupon=coupon, product_id=product_id
-                    )
+                for _ in range(validated_data.get("num_coupon_codes"))
+            ]
+            coupon_objs = models.Coupon.objects.bulk_create(coupons)
+            versions = [
+                models.CouponVersion(coupon=obj, payment_version=payment_version)
+                for obj in coupon_objs
+            ]
+            eligibilities = [
+                models.CouponEligibility(coupon=obj, product_id=product_id)
+                for obj in coupon_objs
+                for product_id in validated_data.get("product_ids")
+            ]
+            models.CouponVersion.objects.bulk_create(versions)
+            models.CouponEligibility.objects.bulk_create(eligibilities)
             return payment_version
 
 

ecommerce/serializers_test.py

@@ -2,6 +2,8 @@
 Tests for ecommerce serializers
 """
 # pylint: disable=unused-argument, redefined-outer-name
+from decimal import Decimal
+
 import pytest
 
 from mitxpro.test_utils import any_instance_of
@@ -227,8 +229,9 @@ def test_serialize_coupon_payment_version_serializer(basket_and_coupons):
         assert serializer.data.get(attr) == getattr(
             basket_and_coupons.coupongroup_best.payment_version, attr
         ).strftime(datetime_format)
-    assert serializer.data.get("amount") == "{0:.2}".format(
-        basket_and_coupons.coupongroup_best.payment_version.amount
+    assert (
+        Decimal(serializer.data.get("amount"))
+        == basket_and_coupons.coupongroup_best.payment_version.amount
     )