Minor fixes for the regular payment workflow (#216)

* Some fixes for the regular payment workflow

Should hopefully be cherry pick-able out into its own PR. Fixes for a few things that were problematic while getting stuff ready for refunds:
- Zero-value baskets caused errors on checkout - the code was expecting to redirect, we need to send status back to the React app instead
- import_product and update_product_data commands both didn't manage product versions correctly - for update_product_data, this fix is in the underlying API call that gets queued
- refactored PendingOrder._get_or_create a bit to check to make sure the lines and versions match, or fail (and to move checking into a separate function to keep Ruff happy)
- Added a BasketItem inline so you can see what's in the basket in Django Admin more easily

* Additional fixes

- Fix some tests with payment APIs
- Add hard system link to Orders - we're doing this in Basket anyway

payments/admin.py

@@ -9,6 +9,30 @@
 from payments import models
 
 
+class BasketItemInline(admin.TabularInline):
+    """Inline editor for lines"""
+
+    model = models.BasketItem
+    list_display = [
+        "product",
+        "quantity",
+        "base_price",
+        "discounted_price",
+        "tax_money",
+        "total_price",
+    ]
+    readonly_fields = [
+        "product",
+        "quantity",
+        "base_price",
+        "discounted_price",
+        "tax_money",
+        "total_price",
+    ]
+    min_num = 0
+    extra = 0
+
+
 @admin.register(models.Basket)
 class BasketAdmin(VersionAdmin):
     """Admin for Basket"""
@@ -17,6 +41,9 @@ class BasketAdmin(VersionAdmin):
     search_fields = ["user__email", "user__username"]
     list_display = ["id", "user"]
     raw_id_fields = ("user",)
+    inlines = [
+        BasketItemInline,
+    ]
 
 
 @admin.register(models.BasketItem)

payments/api.py

@@ -62,10 +62,8 @@
     REDEMPTION_TYPE_ONE_TIME_PER_USER,
     REDEMPTION_TYPE_UNLIMITED,
     REFUND_SUCCESS_STATES,
-    USER_MSG_TYPE_PAYMENT_ACCEPTED_NOVALUE,
     ZERO_PAYMENT_DATA,
 )
-from unified_ecommerce.utils import redirect_with_user_message
 from users.api import determine_user_location, get_flagged_countries
 
 log = logging.getLogger(__name__)
@@ -76,12 +74,12 @@ def generate_checkout_payload(request, system):
     """Generate the payload to send to the payment gateway."""
     basket = Basket.establish_basket(request, system)
 
+    log.debug("established basket has %s lines", basket.basket_items.count())
+
     # Notes for future implementation: this used to check for
-    # * ~~Blocked products (by country)~~ (now handled in the basket_add hook)
     # * Re-purchases of the same product
     # * Purchasing a product that is expired
     # These are all cleared for now, but will need to go back here later.
-    # For completeness, this is also where discounts were checked for validity.
 
     order = PendingOrder.create_from_basket(basket)
     total_price = 0
@@ -121,13 +119,6 @@ def generate_checkout_payload(request, system):
             )
             return {
                 "no_checkout": True,
-                "response": redirect_with_user_message(
-                    reverse("cart"),
-                    {
-                        "type": USER_MSG_TYPE_PAYMENT_ACCEPTED_NOVALUE,
-                        "run": order.lines.first().purchased_object.course.title,
-                    },
-                ),
             }
 
     callback_uri = request.build_absolute_uri(reverse("v0:checkout-result-callback"))

payments/api_test.py

@@ -49,7 +49,7 @@
     WebhookBasketAction,
     WebhookOrder,
 )
-from system_meta.factories import ProductFactory
+from system_meta.factories import IntegratedSystemFactory, ProductFactory
 from system_meta.models import IntegratedSystem
 from unified_ecommerce.constants import (
     DISCOUNT_TYPE_DOLLARS_OFF,
@@ -664,6 +664,10 @@ def test_pre_sale_webhook(mocker, user, products):
 def test_post_sale_webhook_multisystem(mocker, fulfilled_complete_order, source):
     """Test fire the post-sale webhook with an order with lines from >1 system."""
 
+    # Create an integrated system out of band to make sure this is actually
+    # getting called correctly.
+    _ = IntegratedSystemFactory.create()
+
     with reversion.create_revision():
         product = ProductFactory.create()
 
@@ -688,6 +692,9 @@ def test_post_sale_webhook_multisystem(mocker, fulfilled_complete_order, source)
             ],
         )
 
+        if len(order_info.lines) == 0:
+            continue
+
         webhook_data = WebhookBase(
             type=PAYMENT_HOOK_ACTION_POST_SALE,
             system_slug=system.slug,

payments/factories.py

@@ -40,6 +40,7 @@ class OrderFactory(DjangoModelFactory):
 
     total_price_paid = fuzzy.FuzzyDecimal(10.00, 10.00)
     purchaser = SubFactory(UserFactory)
+    integrated_system = SubFactory(IntegratedSystemFactory)
 
     class Meta:
         """Meta options for BasketFactory"""

payments/migrations/0015_order_integrated_system.py

@@ -0,0 +1,25 @@
+# Generated by Django 4.2.18 on 2025-02-06 20:23
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("system_meta", "0009_product_details_url"),
+        ("payments", "0014_alter_discount_payment_type"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="order",
+            name="integrated_system",
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.PROTECT,
+                related_name="+",
+                to="system_meta.integratedsystem",
+            ),
+        ),
+    ]

payments/models.py

@@ -654,6 +654,13 @@ class STATE:
         ]
 
     state = models.CharField(default=STATE.PENDING, choices=STATE.choices)
+    integrated_system = models.ForeignKey(
+        IntegratedSystem,
+        on_delete=models.PROTECT,
+        related_name="+",
+        null=True,
+        blank=True,
+    )
     purchaser = models.ForeignKey(
         settings.AUTH_USER_MODEL,
         on_delete=models.CASCADE,
@@ -871,6 +878,40 @@ def delete_redeemed_discounts(self):
 class PendingOrder(Order):
     """An order that is pending payment"""
 
+    def _process_basket_products(self, basket):
+        """Process the basket products."""
+
+        products = basket.get_products()
+        # Get the details from each Product.
+        product_versions = [
+            Version.objects.get_for_object(product).first() for product in products
+        ]
+
+        if len(product_versions) == 0:
+            log.error(
+                (
+                    "PendingOrder._get_or_create: %s products are there "
+                    "but no versions?"
+                ),
+                len(products),
+            )
+            msg = "No product versions found"
+            raise ValidationError(msg)
+
+        if len(product_versions) != len(products):
+            log.error(
+                (
+                    "PendingOrder._get_or_create: %s products are there "
+                    "but only %s versions?"
+                ),
+                len(products),
+                len(product_versions),
+            )
+            msg = "Product versions don't match lines added"
+            raise ValidationError(msg)
+
+        return product_versions
+
     @transaction.atomic
     def _get_or_create(self, basket: Basket):
         """
@@ -887,88 +928,77 @@ def _get_or_create(self, basket: Basket):
         Returns:
             PendingOrder: the retrieved or created PendingOrder.
         """
-        try:
-            products = basket.get_products()
-            # Get the details from each Product.
-            product_versions = [
-                Version.objects.get_for_object(product).first() for product in products
-            ]
-
-            # Get or create a PendingOrder
-            # TODO: we prefetched the discounts here
-            orders = Order.objects.select_for_update().filter(
-                lines__product_version__in=product_versions,
+        product_versions = self._process_basket_products(self, basket)
+
+        # Get or create a PendingOrder
+        orders = Order.objects.select_for_update().filter(
+            lines__product_version__in=product_versions,
+            state=Order.STATE.PENDING,
+            purchaser=basket.user,
+            integrated_system=basket.integrated_system,
+        )
+        # Previously, multiple PendingOrders could be created for a single user
+        # for the same product, if multiple exist, grab the first.
+        if orders:
+            order = orders.first()
+            # TODO: this should clear discounts from the order here
+
+            order.refresh_from_db()
+        else:
+            order = Order.objects.create(
                 state=Order.STATE.PENDING,
                 purchaser=basket.user,
+                integrated_system=basket.integrated_system,
+                total_price_paid=0,
             )
-            # Previously, multiple PendingOrders could be created for a single user
-            # for the same product, if multiple exist, grab the first.
-            if orders:
-                order = orders.first()
-                # TODO: this should clear discounts from the order here
-
-                order.refresh_from_db()
-            else:
-                order = Order.objects.create(
-                    state=Order.STATE.PENDING,
-                    purchaser=basket.user,
-                    total_price_paid=0,
-                )
 
-            # TODO: Apply any discounts to the PendingOrder
+        # TODO: Apply any discounts to the PendingOrder
 
-            # Manually set these so that we get updated data if it changes
-            # (this re-uses a PendingOrder if it exists, so it might now be wrong)
-            order.tax_rate = basket.tax_rate
-            order.purchaser_ip = basket.user_ip
-            order.purchaser_taxable_country_code = (
-                basket.user_taxable_country_code
-                if basket.user_taxable_country_code
-                else ""
-            )
-            order.purchaser_taxable_geolocation_type = (
-                basket.user_taxable_geolocation_type
+        # Manually set these so that we get updated data if it changes
+        # (this re-uses a PendingOrder if it exists, so it might now be wrong)
+        order.tax_rate = basket.tax_rate
+        order.purchaser_ip = basket.user_ip
+        order.purchaser_taxable_country_code = (
+            basket.user_taxable_country_code if basket.user_taxable_country_code else ""
+        )
+        order.purchaser_taxable_geolocation_type = basket.user_taxable_geolocation_type
+        order.purchaser_blockable_country_code = (
+            basket.user_blockable_country_code
+            if basket.user_blockable_country_code
+            else ""
+        )
+        order.purchaser_blockable_geolocation_type = (
+            basket.user_blockable_geolocation_type
+        )
+        order.save()
+
+        # Create or get Line for each product.
+        # Calculate the Order total based on Lines and discount.
+        total = Decimal(0)
+        used_discounts = []
+        for product_version in product_versions:
+            basket_item = basket.basket_items.get(
+                product=product_version.field_dict["id"]
             )
-            order.purchaser_blockable_country_code = (
-                basket.user_blockable_country_code
-                if basket.user_blockable_country_code
-                else ""
+            line, created = order.lines.get_or_create(
+                order=order,
+                product_version=product_version,
+                defaults={
+                    "quantity": 1,
+                    "discounted_price": basket_item.discounted_price,
+                },
             )
-            order.purchaser_blockable_geolocation_type = (
-                basket.user_blockable_geolocation_type
+            used_discounts.append(basket_item.best_discount_for_item_from_basket)
+            total += line.total_price_money
+            log.debug(
+                "%s line %s product %s",
+                ("Created" if created else "Updated"),
+                line,
+                product_version.field_dict["sku"],
             )
-            order.save()
-
-            # Create or get Line for each product.
-            # Calculate the Order total based on Lines and discount.
-            total = Decimal(0)
-            used_discounts = []
-            for product_version in product_versions:
-                basket_item = basket.basket_items.get(
-                    product=product_version.field_dict["id"]
-                )
-                line, created = order.lines.get_or_create(
-                    order=order,
-                    product_version=product_version,
-                    defaults={
-                        "quantity": 1,
-                        "discounted_price": basket_item.discounted_price,
-                    },
-                )
-                used_discounts.append(basket_item.best_discount_for_item_from_basket)
-                total += line.total_price_money
-                log.debug(
-                    "%s line %s product %s",
-                    ("Created" if created else "Updated"),
-                    line,
-                    product_version.field_dict["sku"],
-                )
-                line.save()
+            line.save()
 
-            order.total_price_paid = total
-
-        except Exception:  # pylint: disable=broad-except  # noqa: BLE001
-            order.state = Order.STATE.ERRORED
+        order.total_price_paid = total
 
         order.save()
 
@@ -1259,6 +1289,14 @@ def product(self) -> Product:
             self.product_version,
         )
 
+    @staticmethod
+    def from_product(product: Product, **kwargs):
+        """Create a Line for the most recent version of the product."""
+
+        kwargs["product_version"] = Version.objects.get_for_object(product).first()
+
+        return Line(**kwargs)
+
     def __str__(self):
         """Return string version of the line."""
         return f"{self.product_version}"

payments/views/v0/__init__.py

@@ -407,13 +407,16 @@ def start_checkout(request, system_slug: str):
 
     if (
         "country_blocked" in payload
-        or "no_checkout" in payload
         or "purchased_same_courserun" in payload
         or "purchased_non_upgradeable_courserun" in payload
         or "invalid_discounts" in payload
     ):
         return Response(payload["response"], status=status.HTTP_406_NOT_ACCEPTABLE)
 
+    if "no_checkout" in payload:
+        # The user doesn't have to check out - we're done here.
+        return Response(payload, status=status.HTTP_201_CREATED)
+
     return Response(CyberSourceCheckoutSerializer(payload).data)