From b499c3e9976aea03e156025868dd4be48366b595 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 12:45:10 -0400 Subject: [PATCH 01/21] add anonymous_id property to basket, make user nullable and add a checkconstraint that forces a basket to have either one, not both --- .../migrations/0040_basket_anonymous_id.py | 42 +++++++++++++++++++ ecommerce/models.py | 16 ++++++- 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 ecommerce/migrations/0040_basket_anonymous_id.py diff --git a/ecommerce/migrations/0040_basket_anonymous_id.py b/ecommerce/migrations/0040_basket_anonymous_id.py new file mode 100644 index 0000000000..2c74626e3f --- /dev/null +++ b/ecommerce/migrations/0040_basket_anonymous_id.py @@ -0,0 +1,42 @@ +# Generated by Django 5.2.15 on 2026-07-30 16:37 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("ecommerce", "0039_add_b2b_gsheet_index_to_discount"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AddField( + model_name="basket", + name="anonymous_id", + field=models.UUIDField(blank=True, db_index=True, null=True, unique=True), + ), + migrations.AlterField( + model_name="basket", + name="user", + field=models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="basket", + to=settings.AUTH_USER_MODEL, + ), + ), + migrations.AddConstraint( + model_name="basket", + constraint=models.CheckConstraint( + condition=models.Q( + models.Q(("anonymous_id__isnull", True), ("user__isnull", False)), + models.Q(("anonymous_id__isnull", False), ("user__isnull", True)), + _connector="OR", + ), + name="basket_user_xor_anonymous_id", + ), + ), + ] diff --git a/ecommerce/models.py b/ecommerce/models.py index b6abb1c310..8fa98a5b9e 100644 --- a/ecommerce/models.py +++ b/ecommerce/models.py @@ -116,7 +116,21 @@ def __str__(self): class Basket(TimestampedModel): """Represents a User's basket.""" - user = models.OneToOneField(User, on_delete=models.CASCADE, related_name="basket") + anonymous_id = models.UUIDField(null=True, blank=True, unique=True, db_index=True) + user = models.OneToOneField( + User, on_delete=models.CASCADE, related_name="basket", null=True, blank=True + ) + + class Meta: + constraints = [ + models.CheckConstraint( + condition=( + models.Q(user__isnull=False, anonymous_id__isnull=True) + | models.Q(user__isnull=True, anonymous_id__isnull=False) + ), + name="basket_user_xor_anonymous_id", + ) + ] def has_user_blocked_products(self, user): """Return true if any of the courses in the basket block user's country""" From 3bd23db23b149016874e1aa4a97b5845effd2372 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 12:52:50 -0400 Subject: [PATCH 02/21] add is_anonymous --- ecommerce/models.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ecommerce/models.py b/ecommerce/models.py index 8fa98a5b9e..7e2b32926d 100644 --- a/ecommerce/models.py +++ b/ecommerce/models.py @@ -132,6 +132,11 @@ class Meta: ) ] + @property + def is_anonymous(self): + """Return True if this basket belongs to an anonymous (unauthenticated) user.""" + return self.user_id is None + def has_user_blocked_products(self, user): """Return true if any of the courses in the basket block user's country""" basket_items = self.basket_items.prefetch_related("product") From 23ec9731066beb288dd92206eaeac7b6371eec43 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 13:42:48 -0400 Subject: [PATCH 03/21] add get_anonymous_basket_id function --- ecommerce/api.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/ecommerce/api.py b/ecommerce/api.py index e89ef1ce01..fadaced376 100644 --- a/ecommerce/api.py +++ b/ecommerce/api.py @@ -451,6 +451,30 @@ def establish_basket(request, *, no_delay=False): return basket +ANONYMOUS_BASKET_SESSION_KEY = "anonymous_basket_id" + + +def get_anonymous_basket_id(request, *, create=False): + """ + Get the anonymous basket id stored in the request's session, minting one + if requested and none exists yet. + + Kwargs: + create (bool): mint and store a new id in the session if one isn't + already present. Only pass True from call sites that are about to + write to the basket - minting an id writes to the session, which + forces a Set-Cookie header and defeats caching for anonymous page + views that don't need one. + """ + anonymous_id = request.session.get(ANONYMOUS_BASKET_SESSION_KEY) + + if anonymous_id is None and create: + anonymous_id = str(uuid.uuid4()) + request.session[ANONYMOUS_BASKET_SESSION_KEY] = anonymous_id + + return anonymous_id + + def refund_order(*, order_id: int = None, reference_number: str = None, **kwargs): # noqa: RUF013 """ A function that performs refund for a given order id From 9a2b04b434eaf524834dc75a4c80d47cc617dfa6 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 13:48:17 -0400 Subject: [PATCH 04/21] add establish_basket_for_request --- ecommerce/api.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/ecommerce/api.py b/ecommerce/api.py index fadaced376..ed20050520 100644 --- a/ecommerce/api.py +++ b/ecommerce/api.py @@ -475,6 +475,20 @@ def get_anonymous_basket_id(request, *, create=False): return anonymous_id +def establish_basket_for_request(request): + """ + Get or create the basket for the current request, whether the requester + is authenticated or anonymous. + """ + if request.user.is_authenticated: + return establish_basket(request) + + anonymous_id = get_anonymous_basket_id(request, create=True) + basket, _ = Basket.objects.get_or_create(anonymous_id=anonymous_id) + + return basket + + def refund_order(*, order_id: int = None, reference_number: str = None, **kwargs): # noqa: RUF013 """ A function that performs refund for a given order id From 5381de253ffa1754baa281d5891b9bfd9a5f1c04 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 14:23:38 -0400 Subject: [PATCH 05/21] add claim_anonymous_basket --- ecommerce/api.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/ecommerce/api.py b/ecommerce/api.py index ed20050520..7826bfe7ab 100644 --- a/ecommerce/api.py +++ b/ecommerce/api.py @@ -489,6 +489,42 @@ def establish_basket_for_request(request): return basket +def claim_anonymous_basket(request): + """ + Convert the anonymous basket identified by the current session into a + basket for the now-authenticated request.user. + + If request.user already has a basket, it is discarded in favor of the + anonymous basket - the anonymous basket reflects what was just shown on + the cart page, and merging would silently change the price the user saw. + + Returns the claimed basket, or None if there's no anonymous basket to + claim (e.g. an expired session). + """ + anonymous_id = get_anonymous_basket_id(request, create=False) + if anonymous_id is None: + return None + + with transaction.atomic(): + try: + anon_basket = Basket.objects.select_for_update().get( + anonymous_id=anonymous_id + ) + except Basket.DoesNotExist: + return None + + Basket.objects.filter(user=request.user).exclude(pk=anon_basket.pk).delete() + + anon_basket.user = request.user + anon_basket.anonymous_id = None + anon_basket.save(update_fields=["user", "anonymous_id"]) + + del request.session[ANONYMOUS_BASKET_SESSION_KEY] + apply_user_discounts(request) + + return anon_basket + + def refund_order(*, order_id: int = None, reference_number: str = None, **kwargs): # noqa: RUF013 """ A function that performs refund for a given order id From df835d0a22b582518f49284dc4a080d3ea804fc0 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 14:41:56 -0400 Subject: [PATCH 06/21] add a for_update arg to establish_basket_for_request and use it in add_to_cart --- ecommerce/api.py | 16 ++++++++++++---- ecommerce/views/legacy/__init__.py | 4 +--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/ecommerce/api.py b/ecommerce/api.py index 7826bfe7ab..bb4365729b 100644 --- a/ecommerce/api.py +++ b/ecommerce/api.py @@ -475,16 +475,24 @@ def get_anonymous_basket_id(request, *, create=False): return anonymous_id -def establish_basket_for_request(request): +def establish_basket_for_request(request, *, for_update=False): """ Get or create the basket for the current request, whether the requester is authenticated or anonymous. + + Kwargs: + for_update (bool): re-fetch the basket with select_for_update() so it's + locked for the remainder of the caller's transaction. Pass True + when the caller is about to mutate basket contents. """ if request.user.is_authenticated: - return establish_basket(request) + basket = establish_basket(request) + else: + anonymous_id = get_anonymous_basket_id(request, create=True) + basket, _ = Basket.objects.get_or_create(anonymous_id=anonymous_id) - anonymous_id = get_anonymous_basket_id(request, create=True) - basket, _ = Basket.objects.get_or_create(anonymous_id=anonymous_id) + if for_update: + basket = Basket.objects.select_for_update().get(pk=basket.pk) return basket diff --git a/ecommerce/views/legacy/__init__.py b/ecommerce/views/legacy/__init__.py index 2f6b6cc435..316adce5f3 100644 --- a/ecommerce/views/legacy/__init__.py +++ b/ecommerce/views/legacy/__init__.py @@ -687,9 +687,7 @@ def redeem_discount(self, request): def add_to_cart(self, request): """Add product to the cart""" with transaction.atomic(): - basket, _ = Basket.objects.select_for_update().get_or_create( - user=self.request.user - ) + basket = api.establish_basket_for_request(request, for_update=True) # Check if multiple cart items feature is enabled allow_multiple_items = getattr( From ce10eb1f090c0b710d017170e4ce21d2ecead00c Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 14:58:10 -0400 Subject: [PATCH 07/21] modify the cart endpoint to return the anonymous basket if appropriate --- ecommerce/views/legacy/__init__.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/ecommerce/views/legacy/__init__.py b/ecommerce/views/legacy/__init__.py index 316adce5f3..431a476372 100644 --- a/ecommerce/views/legacy/__init__.py +++ b/ecommerce/views/legacy/__init__.py @@ -764,9 +764,17 @@ def cart(self, request): """ Returns the current cart, with the product info embedded. """ - try: - basket = Basket.objects.filter(user=request.user).get() - except ObjectDoesNotExist: + if request.user.is_authenticated: + basket = Basket.objects.filter(user=request.user).first() + else: + anonymous_id = api.get_anonymous_basket_id(request, create=False) + basket = ( + Basket.objects.filter(anonymous_id=anonymous_id).first() + if anonymous_id + else None + ) + + if basket is None: return Response("No basket", status=status.HTTP_406_NOT_ACCEPTABLE) if not basket.get_products(): @@ -774,7 +782,8 @@ def cart(self, request): "No product in basket", status=status.HTTP_406_NOT_ACCEPTABLE ) - api.apply_user_discounts(request) + if request.user.is_authenticated: + api.apply_user_discounts(request) return Response(BasketWithProductSerializer(basket).data) From 77eb0cb96d40459053ac474cee9a281d103d9dbf Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 15:11:00 -0400 Subject: [PATCH 08/21] get item count from anonymous baskets --- ecommerce/views/legacy/__init__.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ecommerce/views/legacy/__init__.py b/ecommerce/views/legacy/__init__.py index 431a476372..2e4b56014d 100644 --- a/ecommerce/views/legacy/__init__.py +++ b/ecommerce/views/legacy/__init__.py @@ -794,9 +794,17 @@ def cart(self, request): url_name="basket_items_count", ) def basket_items_count(self, request): - basket, _ = Basket.objects.get_or_create(user=request.user) + if request.user.is_authenticated: + basket, _ = Basket.objects.get_or_create(user=request.user) + else: + anonymous_id = api.get_anonymous_basket_id(request, create=False) + basket = ( + Basket.objects.filter(anonymous_id=anonymous_id).first() + if anonymous_id + else None + ) - return Response(basket.basket_items.count()) + return Response(basket.basket_items.count() if basket else 0) @method_decorator(csrf_exempt, name="dispatch") From 18c521f34aa975df76c5be65a3150074f296d952 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 15:17:15 -0400 Subject: [PATCH 09/21] update checkoutproductview to use new basket function --- ecommerce/views/legacy/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ecommerce/views/legacy/__init__.py b/ecommerce/views/legacy/__init__.py index 2e4b56014d..524894cae7 100644 --- a/ecommerce/views/legacy/__init__.py +++ b/ecommerce/views/legacy/__init__.py @@ -978,9 +978,7 @@ class CheckoutProductView(LoginRequiredMixin, RedirectView): def get_redirect_url(self, *args, **kwargs): """Populate the basket before redirecting""" with transaction.atomic(): - basket, _ = Basket.objects.select_for_update().get_or_create( - user=self.request.user - ) + basket = api.establish_basket_for_request(self.request, for_update=True) basket.basket_items.all().delete() BasketDiscount.objects.filter(redeemed_basket=basket).delete() From 7342ccbc683d263a53eebcd4938d66b28a3b5251 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 15:26:25 -0400 Subject: [PATCH 10/21] set permissions to allow anonymous access --- ecommerce/views/legacy/__init__.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ecommerce/views/legacy/__init__.py b/ecommerce/views/legacy/__init__.py index 524894cae7..d41181db05 100644 --- a/ecommerce/views/legacy/__init__.py +++ b/ecommerce/views/legacy/__init__.py @@ -26,7 +26,7 @@ from rest_framework.decorators import action from rest_framework.exceptions import ParseError from rest_framework.generics import ListCreateAPIView, RetrieveAPIView -from rest_framework.permissions import IsAdminUser, IsAuthenticated +from rest_framework.permissions import AllowAny, IsAdminUser, IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView from rest_framework.viewsets import ( @@ -587,6 +587,12 @@ class CheckoutApiViewSet(ViewSet): authentication_classes = (SessionAuthentication, TokenAuthentication) permission_classes = (IsAuthenticated,) + def get_permissions(self): + """Allow anonymous access to everything except discount redemption""" + if self.action == "redeem_discount": + return [IsAuthenticated()] + return [AllowAny()] + @extend_schema( request=RedeemDiscountRequestSerializer, responses={200: RedeemDiscountResponseSerializer}, @@ -970,7 +976,7 @@ def post(self, request, *args, **kwargs): # noqa: ARG002 return Response(status=status.HTTP_200_OK) -class CheckoutProductView(LoginRequiredMixin, RedirectView): +class CheckoutProductView(RedirectView): """View to add products to the cart and proceed to the checkout page""" pattern_name = "cart" From c7fe90d5d9a25a99647b24e50cd3a9bb4bfa36e6 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 15:59:08 -0400 Subject: [PATCH 11/21] add anonymous checkout view --- ecommerce/urls.py | 6 ++++++ ecommerce/views/legacy/__init__.py | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/ecommerce/urls.py b/ecommerce/urls.py index b5b08e4053..cbc0e69311 100644 --- a/ecommerce/urls.py +++ b/ecommerce/urls.py @@ -3,6 +3,7 @@ from ecommerce.admin import AdminRefundOrderView from ecommerce.views.legacy import ( AllProductViewSet, + AnonymousCheckoutView, BackofficeCallbackView, BasketDiscountViewSet, BasketItemViewSet, @@ -92,6 +93,11 @@ CheckoutInterstitialView.as_view(), name="checkout_interstitial_page", ), + re_path( + r"^checkout/anonymous/?$", + AnonymousCheckoutView.as_view(), + name="checkout-anonymous", + ), path( "api/orders/receipt//", OrderReceiptView.as_view(), diff --git a/ecommerce/views/legacy/__init__.py b/ecommerce/views/legacy/__init__.py index d41181db05..03b7eea21e 100644 --- a/ecommerce/views/legacy/__init__.py +++ b/ecommerce/views/legacy/__init__.py @@ -1018,6 +1018,16 @@ def get_redirect_url(self, *args, **kwargs): return super().get_redirect_url(*args, **kwargs) +class AnonymousCheckoutView(LoginRequiredMixin, RedirectView): + """Claim the anonymous basket for the now-authenticated user, then proceed to checkout""" + + pattern_name = "checkout_interstitial_page" + + def get_redirect_url(self, *args, **kwargs): + api.claim_anonymous_basket(self.request) + return super().get_redirect_url(*args, **kwargs) + + class CheckoutInterstitialView(LoginRequiredMixin, TemplateView): template_name = "checkout_interstitial.html" From b5f84c7768492e0507c31119a3cd95e5058c76e6 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 16:03:22 -0400 Subject: [PATCH 12/21] update apisix routing --- config/apisix/apisix.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/config/apisix/apisix.yaml b/config/apisix/apisix.yaml index c4e45c4dbc..36afa1a646 100644 --- a/config/apisix/apisix.yaml +++ b/config/apisix/apisix.yaml @@ -64,8 +64,8 @@ routes: - "/admin/login*" - id: 3 - name: "app-cart" - desc: "Require login for cart so session is established." + name: "app-checkout-anonymous" + desc: "Require login for the anonymous basket claim/checkout endpoint so a session is established." priority: 5 upstream_id: 1 plugins: @@ -88,8 +88,8 @@ routes: set: Content-Security-Policy: frame-ancestors 'self' ${{OPENEDX_API_BASE_URL}} uris: - - "/cart" - - "/cart/" + - "/checkout/anonymous" + - "/checkout/anonymous/" #END From 3155a2744dda82d98920d051e70ed247f241a089 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 16:37:52 -0400 Subject: [PATCH 13/21] pass isauthenticated and if not, send the user to /checkout/anonymous --- .../public/src/components/OrderSummaryCard.js | 18 ++++++++++++------ .../pages/checkout/OrderReceiptPage.js | 1 + 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/frontend/public/src/components/OrderSummaryCard.js b/frontend/public/src/components/OrderSummaryCard.js index 911e26ea18..74f9de4153 100644 --- a/frontend/public/src/components/OrderSummaryCard.js +++ b/frontend/public/src/components/OrderSummaryCard.js @@ -15,7 +15,8 @@ type Props = { refunds: Array, addDiscount?: Function, discountCode: string, - cardTitle?: string + cardTitle?: string, + isAuthenticated: boolean } type FormValues = { @@ -124,6 +125,10 @@ export class OrderSummaryCard extends React.Component { ) } + getCheckoutUrl() { + return this.props.isAuthenticated ? "/checkout/to_payment" : "/checkout/anonymous/" + } + handlePlaceOrder = async () => { const formik = this.formikRef.current const { discounts } = this.props @@ -134,7 +139,7 @@ export class OrderSummaryCard extends React.Component { discounts.length > 0 && (!formik || !formik.values.couponCode || !formik.values.couponCode.trim()) ) { - window.location = "/checkout/to_payment" + window.location = this.getCheckoutUrl() return } @@ -145,7 +150,7 @@ export class OrderSummaryCard extends React.Component { await formik.submitForm() } else { // No coupon code, proceed directly to payment - window.location = "/checkout/to_payment" + window.location = this.getCheckoutUrl() } } @@ -166,7 +171,8 @@ export class OrderSummaryCard extends React.Component { addDiscount, discountCode, cardTitle, - refunds + refunds, + isAuthenticated } = this.props const fmtPrice = formatLocalePrice(totalPrice) @@ -203,7 +209,7 @@ export class OrderSummaryCard extends React.Component { - {!orderFulfilled ? ( + {!orderFulfilled && isAuthenticated ? ( { if (this.state.submittingPlaceOrder) { // Redirect only if there were no errors and this was from Place Order button - window.location = "/checkout/to_payment" + window.location = this.getCheckoutUrl() } } diff --git a/frontend/public/src/containers/pages/checkout/OrderReceiptPage.js b/frontend/public/src/containers/pages/checkout/OrderReceiptPage.js index 5b1bdfdfb4..66361d101f 100644 --- a/frontend/public/src/containers/pages/checkout/OrderReceiptPage.js +++ b/frontend/public/src/containers/pages/checkout/OrderReceiptPage.js @@ -68,6 +68,7 @@ export class OrderReceiptPage extends React.Component { refunds={orderReceipt.refunds} cardTitle={`Order Number: ${orderReceipt.reference_number} `} discountCode="" + isAuthenticated={true} /> ) : null } From 2b02ed756556f3c78458b49f2efe9cad3fc5d96d Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 16:41:11 -0400 Subject: [PATCH 14/21] check authentication on cartpage --- .../src/containers/pages/checkout/CartPage.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/frontend/public/src/containers/pages/checkout/CartPage.js b/frontend/public/src/containers/pages/checkout/CartPage.js index a55c8ee25d..b97bfbe050 100644 --- a/frontend/public/src/containers/pages/checkout/CartPage.js +++ b/frontend/public/src/containers/pages/checkout/CartPage.js @@ -11,6 +11,7 @@ import { createStructuredSelector } from "reselect" import { pathOr } from "ramda" import type { BasketItem, Discount } from "../../../flow/cartTypes" +import type { CurrentUser } from "../../../flow/authTypes" import Loader from "../../../components/Loader" import { CartItemCard } from "../../../components/CartItemCard" @@ -25,6 +26,7 @@ import { discountSelector, applyDiscountCodeMutation } from "../../../lib/queries/cart" +import { currentUserSelector } from "../../../lib/queries/users" import type { RouterHistory } from "react-router" import { isSuccessResponse } from "../../../lib/util" @@ -39,7 +41,8 @@ type Props = { isLoading: boolean, applyDiscountCode: (code: string) => Promise, addUserNotification: Function, - forceRequest: Function + forceRequest: Function, + currentUser: ?CurrentUser } type CartState = { @@ -98,7 +101,7 @@ export class CartPage extends React.Component { } renderOrderSummaryCard() { - const { totalPrice, discountedPrice, discounts } = this.props + const { totalPrice, discountedPrice, discounts, currentUser } = this.props const refunds = [] return ( @@ -110,12 +113,18 @@ export class CartPage extends React.Component { refunds={refunds} addDiscount={this.addDiscount.bind(this)} discountCode={this.state.discountCode} + isAuthenticated={Boolean(currentUser && currentUser.is_authenticated)} /> ) } renderFinancialAssistanceOffer() { - const { cartItems, discounts } = this.props + const { cartItems, discounts, currentUser } = this.props + + if (!currentUser || !currentUser.is_authenticated) { + return null + } + let userFlexiblePriceExists = false // Check if there are any discounts, and if those discounts are for flexible pricing. if ( @@ -199,7 +208,8 @@ const mapStateToProps = createStructuredSelector({ totalPrice: totalPriceSelector, discountedPrice: discountedPriceSelector, discounts: discountSelector, - isLoading: pathOr(true, ["queries", cartQueryKey, "isPending"]) + isLoading: pathOr(true, ["queries", cartQueryKey, "isPending"]), + currentUser: currentUserSelector }) const mapDispatchToProps = { From 7d6884c77ee80ea853d6f6de3674ec36d5bcf6a7 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 16:43:21 -0400 Subject: [PATCH 15/21] get cart item count for all users --- frontend/public/src/containers/App.js | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/frontend/public/src/containers/App.js b/frontend/public/src/containers/App.js index ee946beda5..66a9440e30 100644 --- a/frontend/public/src/containers/App.js +++ b/frontend/public/src/containers/App.js @@ -123,7 +123,7 @@ export class App extends React.Component { !this.isLearnerRecordsPage() && (
)} @@ -214,16 +214,10 @@ const mapDispatchToProps = { addUserNotification } -const mapPropsToConfig = props => { - const queries = [users.currentUserQuery()] - - // Add cart query for authenticated users - if (props.currentUser && props.currentUser.is_authenticated) { - queries.push(cartItemsCountQuery()) - } - - return queries -} +const mapPropsToConfig = () => [ + users.currentUserQuery(), + cartItemsCountQuery() +] export default compose( connect(mapStateToProps, mapDispatchToProps), connectRequest(mapPropsToConfig) From 6bcffb487cdee2eebe3415e18f6fcfdfeffe39be Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 17:08:04 -0400 Subject: [PATCH 16/21] unit tests --- ecommerce/api_test.py | 158 ++++++++++++++ ecommerce/models_test.py | 30 +++ ecommerce/views/legacy/views_test.py | 195 ++++++++++++++++++ .../src/components/OrderSummaryCard_test.js | 92 +++++++++ frontend/public/src/containers/App_test.js | 7 +- .../pages/checkout/CartPage_test.js | 97 +++++++++ 6 files changed, 576 insertions(+), 3 deletions(-) create mode 100644 frontend/public/src/components/OrderSummaryCard_test.js create mode 100644 frontend/public/src/containers/pages/checkout/CartPage_test.js diff --git a/ecommerce/api_test.py b/ecommerce/api_test.py index 93f2a602ea..624858f0be 100644 --- a/ecommerce/api_test.py +++ b/ecommerce/api_test.py @@ -1,6 +1,7 @@ """Tests for Ecommerce api""" import random +import uuid from datetime import datetime, timedelta from zoneinfo import ZoneInfo @@ -9,6 +10,7 @@ import reversion from CyberSource.rest import ApiException from django.conf import settings +from django.contrib.auth.models import AnonymousUser from django.contrib.contenttypes.models import ContentType from django.test import RequestFactory from django.urls import reverse @@ -24,13 +26,17 @@ ProgramFactory, ) from ecommerce.api import ( + ANONYMOUS_BASKET_SESSION_KEY, apply_discount_to_basket, check_and_process_pending_orders_for_resolution, check_for_duplicate_discount_redemptions, + claim_anonymous_basket, create_verified_program_course_run_enrollment, create_verified_program_discount, establish_basket, + establish_basket_for_request, generate_checkout_payload, + get_anonymous_basket_id, get_auto_apply_discounts_for_basket, process_cybersource_payment_response, refund_order, @@ -1231,3 +1237,155 @@ def test_establish_basket_calls_create_user(mocker, no_delay): establish_basket(request) assert not expected_run_mock.called + + +def test_get_anonymous_basket_id_no_create_does_not_write_session(): + """Test that create=False never mints or writes an id into the session""" + request = RequestFactory().get("/") + request.session = {} + + result = get_anonymous_basket_id(request, create=False) + + assert result is None + assert ANONYMOUS_BASKET_SESSION_KEY not in request.session + + +def test_get_anonymous_basket_id_create_mints_and_is_idempotent(): + """Test that create=True mints an id once and reuses it on subsequent calls""" + request = RequestFactory().get("/") + request.session = {} + + first_id = get_anonymous_basket_id(request, create=True) + + assert first_id is not None + assert request.session[ANONYMOUS_BASKET_SESSION_KEY] == first_id + + second_id = get_anonymous_basket_id(request, create=True) + + assert second_id == first_id + + +def test_establish_basket_for_request_authenticated(user): + """Test that an authenticated request dispatches to establish_basket""" + request = RequestFactory().get("/") + request.session = {} + request.user = user + + basket = establish_basket_for_request(request) + + assert basket.user_id == user.id + assert basket.is_anonymous is False + + +def test_establish_basket_for_request_anonymous_creates_basket(): + """Test that an anonymous request creates a basket keyed by the session's anonymous id""" + request = RequestFactory().get("/") + request.session = {} + request.user = AnonymousUser() + + basket = establish_basket_for_request(request) + + assert basket.is_anonymous is True + assert str(basket.anonymous_id) == request.session[ANONYMOUS_BASKET_SESSION_KEY] + + # A second call with the same session should return the same basket + same_basket = establish_basket_for_request(request) + assert same_basket.id == basket.id + + +def test_establish_basket_for_request_for_update_locks_basket(mocker): + """Test that for_update=True re-fetches the basket with select_for_update""" + request = RequestFactory().get("/") + request.session = {} + request.user = AnonymousUser() + + select_for_update_spy = mocker.spy(Basket.objects, "select_for_update") + + basket = establish_basket_for_request(request, for_update=True) + + select_for_update_spy.assert_called_once() + assert basket.is_anonymous is True + + +def test_claim_anonymous_basket_no_session_returns_none(user): + """Test that there's nothing to claim if the session has no anonymous basket id""" + request = RequestFactory().get("/") + request.session = {} + request.user = user + + assert claim_anonymous_basket(request) is None + + +def test_claim_anonymous_basket_missing_basket_row_returns_none(user): + """Test a stale session id (e.g. the basket was culled) returns None rather than raising""" + request = RequestFactory().get("/") + request.session = {ANONYMOUS_BASKET_SESSION_KEY: str(uuid.uuid4())} + request.user = user + + assert claim_anonymous_basket(request) is None + + +def test_claim_anonymous_basket_claims_and_pops_session(user): + """Test the normal claim path: basket is reassigned, anonymous_id cleared, session popped""" + anon_request = RequestFactory().get("/") + anon_request.session = {} + anon_request.user = AnonymousUser() + anon_basket = establish_basket_for_request(anon_request) + + claim_request = RequestFactory().get("/") + claim_request.session = anon_request.session + claim_request.user = user + + claimed = claim_anonymous_basket(claim_request) + + assert claimed.id == anon_basket.id + assert claimed.user_id == user.id + assert claimed.anonymous_id is None + assert ANONYMOUS_BASKET_SESSION_KEY not in claim_request.session + + +def test_claim_anonymous_basket_discards_existing_user_basket(user): + """Test that an existing basket for the user is discarded in favor of the anonymous one""" + existing_basket = Basket.objects.create(user=user) + + anon_request = RequestFactory().get("/") + anon_request.session = {} + anon_request.user = AnonymousUser() + anon_basket = establish_basket_for_request(anon_request) + + claim_request = RequestFactory().get("/") + claim_request.session = anon_request.session + claim_request.user = user + + claimed = claim_anonymous_basket(claim_request) + + assert claimed.id == anon_basket.id + assert not Basket.objects.filter(pk=existing_basket.pk).exists() + + +def test_claim_anonymous_basket_applies_user_discount_after_conversion(user): + """Test that a pre-assigned user discount is applied once the basket is claimed""" + product = ProductFactory.create() + discount = UnlimitedUseDiscountFactory.create() + UserDiscount.objects.create(discount=discount, user=user) + + anon_request = RequestFactory().get("/") + anon_request.session = {} + anon_request.user = AnonymousUser() + anon_basket = establish_basket_for_request(anon_request) + BasketItem.objects.create(basket=anon_basket, product=product) + + assert BasketDiscount.objects.filter(redeemed_basket=anon_basket).count() == 0 + + claim_request = RequestFactory().get("/") + claim_request.session = anon_request.session + claim_request.user = user + + claimed = claim_anonymous_basket(claim_request) + + assert ( + BasketDiscount.objects.filter( + redeemed_basket=claimed, redeemed_discount=discount + ).count() + == 1 + ) diff --git a/ecommerce/models_test.py b/ecommerce/models_test.py index 37b412e8a3..413854253d 100644 --- a/ecommerce/models_test.py +++ b/ecommerce/models_test.py @@ -1,4 +1,5 @@ import random +import uuid from datetime import timedelta from decimal import Decimal @@ -265,6 +266,35 @@ def test_basket_order_equivalency(user, basket, unlimited_discount): assert basket.compare_to_order(order) is False +def test_basket_is_anonymous(): + """Test that is_anonymous reflects whether the basket has a user or an anonymous_id""" + user_basket = BasketFactory.create() + anonymous_basket = BasketFactory.create(user=None, anonymous_id=uuid.uuid4()) + + assert user_basket.is_anonymous is False + assert anonymous_basket.is_anonymous is True + + +def test_basket_requires_exactly_one_of_user_or_anonymous_id(): + """Test the CheckConstraint rejects baskets with both or neither of user/anonymous_id set""" + user = UserFactory.create() + + with pytest.raises(IntegrityError), transaction.atomic(): + Basket.objects.create(user=user, anonymous_id=uuid.uuid4()) + + with pytest.raises(IntegrityError), transaction.atomic(): + Basket.objects.create(user=None, anonymous_id=None) + + +def test_compare_to_order_anonymous_basket(user): + """Test that an anonymous basket never compares equal to an order""" + anonymous_basket = BasketFactory.create(user=None, anonymous_id=uuid.uuid4()) + order = Order(purchaser=user, state=OrderStatus.FULFILLED, total_price_paid=10) + order.save() + + assert anonymous_basket.compare_to_order(order) is False + + def test_product_delete_protection_inactive(): """Test that deleting product(s) instead de-activates it""" single_product = ProductFactory.create() diff --git a/ecommerce/views/legacy/views_test.py b/ecommerce/views/legacy/views_test.py index 4c3b1e6a84..2053083fea 100644 --- a/ecommerce/views/legacy/views_test.py +++ b/ecommerce/views/legacy/views_test.py @@ -1,17 +1,20 @@ import operator as op import random +import uuid from datetime import datetime, timedelta from zoneinfo import ZoneInfo import freezegun import pytest import reversion +from django.conf import settings from django.forms.models import model_to_dict from django.test import Client, RequestFactory from django.urls import reverse from mitol.common.utils.datetime import now_in_utc from mitol.payment_gateway.api import PaymentGateway from rest_framework import status +from rest_framework.test import APIClient from reversion.models import Version from b2b.factories import ContractPageFactory @@ -71,6 +74,21 @@ pytestmark = [pytest.mark.django_db] +def set_anonymous_basket_session(client, anonymous_id): + """ + Seed a test client's session with an anonymous_basket_id. + + SESSION_ENGINE is signed_cookies, so session.save() only updates the + in-memory session_key - it doesn't rewrite the client's cookie jar the + way it would for a server-side session backend. The cookie has to be set + manually or the modified session never reaches the next request. + """ + session = client.session + session["anonymous_basket_id"] = str(anonymous_id) + session.save() + client.cookies[settings.SESSION_COOKIE_NAME] = session.session_key + + @pytest.fixture def products(): with reversion.create_revision(): @@ -990,6 +1008,183 @@ def test_add_to_cart_does_not_trigger_hubspot_for_duplicate_product( mock_sync.assert_not_called() +def test_add_to_cart_anonymous_creates_basket(): + """An anonymous caller can add a product to a new anonymous basket""" + client = APIClient() + product = ProductFactory.create() + + resp = client.post( + reverse("checkout_api-add_to_cart"), + data={"product_id": product.id}, + ) + + assert resp.status_code == status.HTTP_200_OK + + basket = Basket.objects.get(anonymous_id__isnull=False) + assert basket.basket_items.count() == 1 + assert basket.basket_items.first().product == product + assert client.session["anonymous_basket_id"] == str(basket.anonymous_id) + + +@pytest.mark.parametrize( + "cart_exists, cart_empty, expected_status, expected_message", # noqa: PT006 + [ + (False, True, status.HTTP_406_NOT_ACCEPTABLE, "No basket"), + (True, True, status.HTTP_406_NOT_ACCEPTABLE, "No product in basket"), + (True, False, status.HTTP_200_OK, ""), + ], +) +def test_checkout_cart_anonymous( + cart_exists, cart_empty, expected_status, expected_message +): + """Verifies cart/ behaves the same way for anonymous users as for authenticated ones""" + client = APIClient() + + # An authenticated user's basket must never leak to an anonymous caller + other_basket = BasketFactory.create() + BasketItemFactory.create(basket=other_basket) + + basket = None + if cart_exists: + anonymous_id = uuid.uuid4() + basket = Basket.objects.create(anonymous_id=anonymous_id) + set_anonymous_basket_session(client, anonymous_id) + + if basket and not cart_empty: + BasketItemFactory.create(basket=basket) + + resp = client.get(reverse("checkout_api-cart")) + assert resp.status_code == expected_status + + if cart_empty: + assert resp.data == expected_message + else: + assert_drf_json_equal(resp.json(), BasketWithProductSerializer(basket).data) + + +def test_checkout_cart_anonymous_no_session_does_not_leak_other_baskets(): + """An anonymous caller with no session id must never see another user's basket""" + client = APIClient() + + other_basket = BasketFactory.create() + BasketItemFactory.create(basket=other_basket) + + resp = client.get(reverse("checkout_api-cart")) + + assert resp.status_code == status.HTTP_406_NOT_ACCEPTABLE + assert resp.data == "No basket" + + +def test_basket_items_count_authenticated(user, user_drf_client): + """Authenticated basket item count reflects the user's own basket""" + basket = BasketFactory.create(user=user) + BasketItemFactory.create_batch(2, basket=basket) + + resp = user_drf_client.get(reverse("checkout_api-basket_items_count")) + + assert resp.status_code == status.HTTP_200_OK + assert resp.json() == 2 + + +def test_basket_items_count_anonymous_no_session_returns_zero(): + """An anonymous caller with no session yet gets zero, not an error, and no leak""" + client = APIClient() + + other_basket = BasketFactory.create() + BasketItemFactory.create(basket=other_basket) + + resp = client.get(reverse("checkout_api-basket_items_count")) + + assert resp.status_code == status.HTTP_200_OK + assert resp.json() == 0 + + +def test_basket_items_count_anonymous_with_items(): + """An anonymous caller with an established basket gets the real count""" + client = APIClient() + anonymous_id = uuid.uuid4() + basket = Basket.objects.create(anonymous_id=anonymous_id) + BasketItemFactory.create_batch(3, basket=basket) + + set_anonymous_basket_session(client, anonymous_id) + + resp = client.get(reverse("checkout_api-basket_items_count")) + + assert resp.status_code == status.HTTP_200_OK + assert resp.json() == 3 + + +def test_redeem_discount_anonymous_forbidden(): + """Anonymous users cannot redeem discount codes, unlike the other checkout actions""" + client = APIClient() + + resp = client.post( + reverse("checkout_api-redeem_discount"), {"discount": "SOMECODE"} + ) + + assert resp.status_code == status.HTTP_403_FORBIDDEN + + +def test_checkout_product_anonymous(): + """CheckoutProductView is reachable anonymously and populates an anonymous basket""" + client = Client() + product = ProductFactory.create() + + resp = client.get(reverse("checkout-product"), {"product_id": product.id}) + + assert resp.status_code == 302 + assert resp.url == reverse("cart") + + basket = Basket.objects.get(anonymous_id__isnull=False) + assert [item.product for item in basket.basket_items.all()] == [product] + + +def test_anonymous_checkout_view_requires_login(): + """AnonymousCheckoutView is defense-in-depth protected behind LoginRequiredMixin""" + client = Client() + + resp = client.get(reverse("checkout-anonymous")) + + assert resp.status_code == 302 + assert resp.url.startswith(reverse("gateway-login")) + + +def test_anonymous_checkout_view_claims_basket_and_redirects(user): + """ + The anonymous_basket_id must survive the login round trip so that the + basket set up before authentication can be claimed once the user logs in. + """ + client = Client() + product = ProductFactory.create() + + resp = client.post( + reverse("checkout_api-add_to_cart"), data={"product_id": product.id} + ) + assert resp.status_code == status.HTTP_200_OK + + anon_basket = Basket.objects.get(anonymous_id__isnull=False) + session_anon_id = client.session["anonymous_basket_id"] + + # Force login with a non-remote backend: real APISIX-authenticated sessions + # carry a header on every subsequent request that keeps a + # RemoteUserBackend-authenticated session alive, but this test client + # doesn't send that header, so using that backend here would cause + # ApisixUserMiddleware to immediately invalidate the session again. + client.force_login(user, backend="django.contrib.auth.backends.ModelBackend") + + assert client.session["anonymous_basket_id"] == session_anon_id + + resp2 = client.get(reverse("checkout-anonymous")) + + assert resp2.status_code == 302 + assert resp2.url == reverse("checkout_interstitial_page") + + anon_basket.refresh_from_db() + assert anon_basket.user_id == user.id + assert anon_basket.anonymous_id is None + assert "anonymous_basket_id" not in client.session + + def test_discount_rest_api(admin_drf_client, user_drf_client): """ Checks that the admin REST API is only accessible by an admin diff --git a/frontend/public/src/components/OrderSummaryCard_test.js b/frontend/public/src/components/OrderSummaryCard_test.js new file mode 100644 index 0000000000..765c342733 --- /dev/null +++ b/frontend/public/src/components/OrderSummaryCard_test.js @@ -0,0 +1,92 @@ +// @flow +import React from "react" +import sinon from "sinon" +import { shallow } from "enzyme" +import { assert } from "chai" + +import { OrderSummaryCard } from "./OrderSummaryCard" +import ApplyCouponForm from "./forms/ApplyCouponForm" + +describe("OrderSummaryCard", () => { + let sandbox + + const baseProps = { + totalPrice: 100, + orderFulfilled: false, + discountedPrice: 100, + discounts: [], + refunds: [], + discountCode: "" + } + + beforeEach(() => { + sandbox = sinon.createSandbox() + }) + + afterEach(() => { + sandbox.restore() + }) + + it("does not render the coupon form when logged out", () => { + const wrapper = shallow( + + ) + assert.isFalse(wrapper.find(ApplyCouponForm).exists()) + }) + + it("renders the coupon form when logged in", () => { + const wrapper = shallow( + + ) + assert.isTrue(wrapper.find(ApplyCouponForm).exists()) + }) + + it("still hides the coupon form when logged in but the order is fulfilled", () => { + const wrapper = shallow( + + ) + assert.isFalse(wrapper.find(ApplyCouponForm).exists()) + }) + + it("returns the anonymous checkout url when logged out", () => { + const wrapper = shallow( + + ) + assert.equal(wrapper.instance().getCheckoutUrl(), "/checkout/anonymous/") + }) + + it("returns the authenticated checkout url when logged in", () => { + const wrapper = shallow( + + ) + assert.equal(wrapper.instance().getCheckoutUrl(), "/checkout/to_payment") + }) + + it("redirects to the anonymous checkout url when placing an order while logged out", async () => { + const originalDescriptor = Object.getOwnPropertyDescriptor( + window, + "location" + ) + Object.defineProperty(window, "location", { + writable: true, + configurable: true, + value: { href: "" } + }) + + try { + const wrapper = shallow( + + ) + await wrapper.instance().handlePlaceOrder() + + assert.equal(window.location, "/checkout/anonymous/") + } finally { + // $FlowFixMe - originalDescriptor is always defined for window.location + Object.defineProperty(window, "location", originalDescriptor) + } + }) +}) diff --git a/frontend/public/src/containers/App_test.js b/frontend/public/src/containers/App_test.js index 3bbacce4ab..0c58f2e4fe 100644 --- a/frontend/public/src/containers/App_test.js +++ b/frontend/public/src/containers/App_test.js @@ -98,7 +98,7 @@ describe("Top-level App", () => { sinon.assert.calledOnce(removeStoredUserMessageStub) }) - it("does not call cartItemsCountQuery for unauthenticated users", async () => { + it("calls cartItemsCountQuery for unauthenticated users too", async () => { helper.handleRequestStub.returns(anonymousUser) await renderPage() // Should call /api/users/me to get user data @@ -107,8 +107,9 @@ describe("Top-level App", () => { "/api/v0/users/current_user/", "GET" ) - // Should NOT call the cart items count API for unauthenticated users - sinon.assert.neverCalledWith( + // Should also call the cart items count API for unauthenticated users, + // so the header badge works for anonymous carts + sinon.assert.calledWith( helper.handleRequestStub, "/api/checkout/basket_items_count/", "GET" diff --git a/frontend/public/src/containers/pages/checkout/CartPage_test.js b/frontend/public/src/containers/pages/checkout/CartPage_test.js new file mode 100644 index 0000000000..0ad44a24f6 --- /dev/null +++ b/frontend/public/src/containers/pages/checkout/CartPage_test.js @@ -0,0 +1,97 @@ +// @flow +import { assert } from "chai" + +import CartPage, { CartPage as InnerCartPage } from "./CartPage" +import IntegrationTestHelper from "../../../util/integration_test_helper" + +describe("CartPage", () => { + let helper, renderPage + + const anonymousUser = { + id: null, + username: "", + email: null, + legal_address: null, + user_profile: null, + is_anonymous: true, + is_authenticated: false, + is_staff: false, + is_superuser: false, + grants: [], + is_active: false + } + + const loggedInUser = { + ...anonymousUser, + id: 1, + username: "test", + email: "test@example.com", + is_anonymous: false, + is_authenticated: true, + is_active: true + } + + const cartItem = { + product: { + id: 1, + price: "100.00", + description: "test product", + purchasable_object: { + course: { + page: { + financial_assistance_form_url: "https://example.com/fa" + } + } + } + } + } + + beforeEach(() => { + helper = new IntegrationTestHelper() + + renderPage = helper.configureShallowRenderer(CartPage, InnerCartPage, { + entities: { + cartItems: [cartItem], + totalPrice: 100, + discountedPrice: 100, + discounts: [], + currentUser: loggedInUser + }, + queries: { + cartItems: { + isPending: false + } + } + }) + }) + + afterEach(() => { + helper.cleanup() + }) + + it("shows the financial assistance offer link when the user is authenticated", async () => { + const { inner } = await renderPage() + assert.isOk(inner.instance().renderFinancialAssistanceOffer()) + }) + + it("suppresses the financial assistance offer link when the user is logged out", async () => { + const { inner } = await renderPage({ + entities: { currentUser: anonymousUser } + }) + assert.isNull(inner.instance().renderFinancialAssistanceOffer()) + }) + + it("passes isAuthenticated=true down to OrderSummaryCard when logged in", async () => { + const { inner } = await renderPage() + const summaryCard = inner.find("OrderSummaryCard") + assert.isTrue(summaryCard.prop("isAuthenticated")) + }) + + it("passes isAuthenticated=false down to OrderSummaryCard when logged out", async () => { + const { inner } = await renderPage({ + entities: { currentUser: anonymousUser } + }) + const summaryCard = inner.find("OrderSummaryCard") + assert.isFalse(summaryCard.prop("isAuthenticated")) + }) +}) From 64b5f31c057b8f0535285d3c3621788919c76d87 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 17:15:11 -0400 Subject: [PATCH 17/21] add basket cleanup task --- ecommerce/api.py | 13 +++++++++++++ ecommerce/api_test.py | 24 ++++++++++++++++++++++++ ecommerce/tasks.py | 7 +++++++ ecommerce/tasks_test.py | 10 ++++++++++ main/settings.py | 10 ++++++++++ 5 files changed, 64 insertions(+) diff --git a/ecommerce/api.py b/ecommerce/api.py index bb4365729b..a37f0c79f2 100644 --- a/ecommerce/api.py +++ b/ecommerce/api.py @@ -2,6 +2,7 @@ import logging import uuid +from datetime import timedelta from decimal import Decimal from urllib.parse import urljoin @@ -533,6 +534,18 @@ def claim_anonymous_basket(request): return anon_basket +def cull_anonymous_baskets(): + """ + Delete anonymous baskets that haven't been touched in a while (abandoned + carts). A basket's anonymous_id is only reachable via its session cookie, + so once that cookie could plausibly have expired there's no way for a + basket to ever be claimed - it's safe to remove. + """ + cutoff = now_in_utc() - timedelta(seconds=settings.ANONYMOUS_BASKET_CULL_AGE) + + Basket.objects.filter(anonymous_id__isnull=False, updated_on__lt=cutoff).delete() + + def refund_order(*, order_id: int = None, reference_number: str = None, **kwargs): # noqa: RUF013 """ A function that performs refund for a given order id diff --git a/ecommerce/api_test.py b/ecommerce/api_test.py index 624858f0be..3da5d270aa 100644 --- a/ecommerce/api_test.py +++ b/ecommerce/api_test.py @@ -33,6 +33,7 @@ claim_anonymous_basket, create_verified_program_course_run_enrollment, create_verified_program_discount, + cull_anonymous_baskets, establish_basket, establish_basket_for_request, generate_checkout_payload, @@ -1389,3 +1390,26 @@ def test_claim_anonymous_basket_applies_user_discount_after_conversion(user): ).count() == 1 ) + + +def test_cull_anonymous_baskets(settings, user): + """Test that only anonymous baskets older than the cutoff are removed""" + settings.ANONYMOUS_BASKET_CULL_AGE = 100 + + old_anon_basket = Basket.objects.create(anonymous_id=uuid.uuid4()) + Basket.objects.filter(pk=old_anon_basket.pk).update( + updated_on=now_in_utc() - timedelta(seconds=200) + ) + + recent_anon_basket = Basket.objects.create(anonymous_id=uuid.uuid4()) + + old_user_basket = Basket.objects.create(user=user) + Basket.objects.filter(pk=old_user_basket.pk).update( + updated_on=now_in_utc() - timedelta(seconds=200) + ) + + cull_anonymous_baskets() + + assert not Basket.objects.filter(pk=old_anon_basket.pk).exists() + assert Basket.objects.filter(pk=recent_anon_basket.pk).exists() + assert Basket.objects.filter(pk=old_user_basket.pk).exists() diff --git a/ecommerce/tasks.py b/ecommerce/tasks.py index 13902c1f83..5183f97f94 100644 --- a/ecommerce/tasks.py +++ b/ecommerce/tasks.py @@ -60,3 +60,10 @@ def perform_check_for_duplicate_discount_redemptions(): from ecommerce.api import check_for_duplicate_discount_redemptions check_for_duplicate_discount_redemptions() + + +@app.task(acks_late=True) +def perform_cull_anonymous_baskets(): + from ecommerce.api import cull_anonymous_baskets + + cull_anonymous_baskets() diff --git a/ecommerce/tasks_test.py b/ecommerce/tasks_test.py index 0d17cb1971..e814d08641 100644 --- a/ecommerce/tasks_test.py +++ b/ecommerce/tasks_test.py @@ -4,6 +4,7 @@ from ecommerce.factories import ProductFactory from ecommerce.serializers.serializers_test import create_order_receipt from ecommerce.tasks import ( + perform_cull_anonymous_baskets, perform_downgrade_from_order, perform_unenrollment_from_order, ) @@ -15,6 +16,15 @@ def products(): return ProductFactory.create_batch(5) +def test_perform_cull_anonymous_baskets_calls_api(mocker): + """The task should just delegate to the api function""" + mock_cull = mocker.patch("ecommerce.api.cull_anonymous_baskets") + + perform_cull_anonymous_baskets() + + mock_cull.assert_called_once() + + @pytest.mark.skip_nplusone_check def test_delayed_order_receipt_sends_email( # noqa: PLR0913 settings, mocker, user, products, user_client, django_capture_on_commit_callbacks diff --git a/main/settings.py b/main/settings.py index 76b74a8c78..ec5a939316 100644 --- a/main/settings.py +++ b/main/settings.py @@ -334,6 +334,12 @@ SESSION_ENGINE = "django.contrib.sessions.backends.signed_cookies" +ANONYMOUS_BASKET_CULL_AGE = get_int( + name="ANONYMOUS_BASKET_CULL_AGE", + default=global_settings.SESSION_COOKIE_AGE, + description="Seconds of inactivity after which an anonymous (unclaimed) basket is deleted", +) + MITXONLINE_NEW_USER_LOGIN_URL = get_string( name="MITXONLINE_NEW_USER_LOGIN_URL", default="http://mitxonline.odl.local:8013/create-profile", @@ -1056,6 +1062,10 @@ offset=timedelta(seconds=B2B_GSHEETS_UPDATE_OFFSET), ), }, + "cull-anonymous-baskets": { + "task": "ecommerce.tasks.perform_cull_anonymous_baskets", + "schedule": crontab(minute=0, hour=4), + }, } # django cache back-ends From 06acd5bee22fa9d0a8e9d426c33f7e98039605c2 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 17:18:27 -0400 Subject: [PATCH 18/21] regenerate openapi --- openapi/specs/v0.yaml | 4 ++-- openapi/specs/v1.yaml | 4 ++-- openapi/specs/v2.yaml | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index 930ff7039d..208cf19296 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -4474,6 +4474,7 @@ components: readOnly: true user: type: integer + nullable: true basket_items: type: array items: @@ -4482,7 +4483,6 @@ components: required: - basket_items - id - - user BasketDiscountDetail: type: object description: BasketDiscount model serializer @@ -4518,6 +4518,7 @@ components: readOnly: true user: type: integer + nullable: true basket_items: type: array items: @@ -4562,7 +4563,6 @@ components: - discounts - id - total_price - - user BlankEnum: enum: - '' diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 1c8dc24f80..8c396d919d 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -4474,6 +4474,7 @@ components: readOnly: true user: type: integer + nullable: true basket_items: type: array items: @@ -4482,7 +4483,6 @@ components: required: - basket_items - id - - user BasketDiscountDetail: type: object description: BasketDiscount model serializer @@ -4518,6 +4518,7 @@ components: readOnly: true user: type: integer + nullable: true basket_items: type: array items: @@ -4562,7 +4563,6 @@ components: - discounts - id - total_price - - user BlankEnum: enum: - '' diff --git a/openapi/specs/v2.yaml b/openapi/specs/v2.yaml index c9f2a2a7ee..33486b8b47 100644 --- a/openapi/specs/v2.yaml +++ b/openapi/specs/v2.yaml @@ -4474,6 +4474,7 @@ components: readOnly: true user: type: integer + nullable: true basket_items: type: array items: @@ -4482,7 +4483,6 @@ components: required: - basket_items - id - - user BasketDiscountDetail: type: object description: BasketDiscount model serializer @@ -4518,6 +4518,7 @@ components: readOnly: true user: type: integer + nullable: true basket_items: type: array items: @@ -4562,7 +4563,6 @@ components: - discounts - id - total_price - - user BlankEnum: enum: - '' From ed54f26698ba9c22909e747122cf2c11e86bc1f2 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Jul 2026 17:19:39 -0400 Subject: [PATCH 19/21] fmt --- frontend/public/src/components/OrderSummaryCard.js | 4 +++- frontend/public/src/containers/App.js | 5 +---- .../public/src/containers/pages/checkout/CartPage_test.js | 4 ++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/frontend/public/src/components/OrderSummaryCard.js b/frontend/public/src/components/OrderSummaryCard.js index 74f9de4153..50b94215f2 100644 --- a/frontend/public/src/components/OrderSummaryCard.js +++ b/frontend/public/src/components/OrderSummaryCard.js @@ -126,7 +126,9 @@ export class OrderSummaryCard extends React.Component { } getCheckoutUrl() { - return this.props.isAuthenticated ? "/checkout/to_payment" : "/checkout/anonymous/" + return this.props.isAuthenticated ? + "/checkout/to_payment" : + "/checkout/anonymous/" } handlePlaceOrder = async () => { diff --git a/frontend/public/src/containers/App.js b/frontend/public/src/containers/App.js index 66a9440e30..a172f8221e 100644 --- a/frontend/public/src/containers/App.js +++ b/frontend/public/src/containers/App.js @@ -214,10 +214,7 @@ const mapDispatchToProps = { addUserNotification } -const mapPropsToConfig = () => [ - users.currentUserQuery(), - cartItemsCountQuery() -] +const mapPropsToConfig = () => [users.currentUserQuery(), cartItemsCountQuery()] export default compose( connect(mapStateToProps, mapDispatchToProps), connectRequest(mapPropsToConfig) diff --git a/frontend/public/src/containers/pages/checkout/CartPage_test.js b/frontend/public/src/containers/pages/checkout/CartPage_test.js index 0ad44a24f6..6b52a84430 100644 --- a/frontend/public/src/containers/pages/checkout/CartPage_test.js +++ b/frontend/public/src/containers/pages/checkout/CartPage_test.js @@ -34,8 +34,8 @@ describe("CartPage", () => { const cartItem = { product: { id: 1, - price: "100.00", - description: "test product", + price: "100.00", + description: "test product", purchasable_object: { course: { page: { From 5a6a6c4a49931acddc74a8fa600cae1192a748fc Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Fri, 31 Jul 2026 13:58:07 -0400 Subject: [PATCH 20/21] allow clearing of the anonymous basket --- ecommerce/views/v0/__init__.py | 102 ++++++++++++++++++------------- ecommerce/views/v0/views_test.py | 81 ++++++++++++++++++++++++ openapi/specs/v0.yaml | 4 ++ openapi/specs/v1.yaml | 4 ++ openapi/specs/v2.yaml | 4 ++ 5 files changed, 154 insertions(+), 41 deletions(-) diff --git a/ecommerce/views/v0/__init__.py b/ecommerce/views/v0/__init__.py index 98c2c28d17..7ecdb11e71 100644 --- a/ecommerce/views/v0/__init__.py +++ b/ecommerce/views/v0/__init__.py @@ -8,6 +8,7 @@ from django.contrib.auth import get_user_model from django.contrib.auth.mixins import LoginRequiredMixin from django.core.exceptions import ObjectDoesNotExist +from django.db import transaction from django.db.models import Count, Q from django.http import Http404 from django.shortcuts import redirect @@ -27,7 +28,7 @@ from rest_framework.exceptions import ParseError from rest_framework.generics import RetrieveAPIView from rest_framework.pagination import LimitOffsetPagination -from rest_framework.permissions import IsAdminUser, IsAuthenticated +from rest_framework.permissions import AllowAny, IsAdminUser, IsAuthenticated from rest_framework.response import Response from rest_framework.viewsets import ModelViewSet, ReadOnlyModelViewSet from rest_framework_extensions.mixins import NestedViewSetMixin @@ -45,6 +46,7 @@ from ecommerce.api import ( apply_discount_to_basket, establish_basket, + establish_basket_for_request, generate_checkout_payload, generate_discount_code, get_auto_apply_discounts_for_basket, @@ -225,7 +227,6 @@ def _create_basket_from_product( Returns: Response: HTTP response """ - basket = establish_basket(request) quantity = request.data.get("quantity", 1) checkout = request.data.get("checkout", False) @@ -236,49 +237,68 @@ def _create_basket_from_product( {"error": "Product not found"}, status=status.HTTP_404_NOT_FOUND ) - # FUTURE: This is where the basket_add hook was called. + with transaction.atomic(): + basket = establish_basket_for_request(request, for_update=True) - (_, created) = BasketItem.objects.update_or_create( - basket=basket, product=product, defaults={"quantity": quantity} - ) + # FUTURE: This is where the basket_add hook was called. - sync_hubspot_cart_add( - request.user, - product, - is_uai=( - is_product_courserun(product) - and is_uai_course_run(product.purchasable_object) + (_, created) = BasketItem.objects.update_or_create( + basket=basket, product=product, defaults={"quantity": quantity} ) - or (is_product_program(product) and is_uai_program(product.purchasable_object)), - ) - - existing_basket_discounts = [bd.redeemed_discount for bd in basket.discounts.all()] - discounts_to_apply = [ - *existing_basket_discounts, - *list(get_auto_apply_discounts_for_basket(basket.id).all()), - ] - - # Clear the discounts that are in the basket - we retained whatever was - # already applied above and will re-apply so the codes get re-checked. (So, - # if a code has now expired, you don't get it anymore.) - BasketDiscount.objects.filter(redeemed_basket=basket).delete() - for discount in discounts_to_apply: - apply_discount_to_basket(basket, discount, allow_finaid=True) + if request.user.is_authenticated: + sync_hubspot_cart_add( + request.user, + product, + is_uai=( + is_product_courserun(product) + and is_uai_course_run(product.purchasable_object) + ) + or ( + is_product_program(product) + and is_uai_program(product.purchasable_object) + ), + ) - # Order matters - apply the code supplied last so we can always attach a - # better-value discount by hand if we want. (Also, turn off finaid flag here.) - if discount_code: - try: - supplied_discount = Discount.objects.get(discount_code=discount_code) - apply_discount_to_basket(basket, supplied_discount) - except Discount.DoesNotExist: - pass + # Discounts (including auto-applied financial assistance) are only + # ever computed against a real user, and shouldn't show up at all for + # a logged-out cart - so this whole step is skipped for anonymous + # baskets rather than run against a basket with no user to check. + existing_basket_discounts = [ + bd.redeemed_discount for bd in basket.discounts.all() + ] + discounts_to_apply = [ + *existing_basket_discounts, + *list(get_auto_apply_discounts_for_basket(basket.id).all()), + ] + + # Clear the discounts that are in the basket - we retained whatever was + # already applied above and will re-apply so the codes get re-checked. (So, + # if a code has now expired, you don't get it anymore.) + BasketDiscount.objects.filter(redeemed_basket=basket).delete() + + for discount in discounts_to_apply: + apply_discount_to_basket(basket, discount, allow_finaid=True) + + # Order matters - apply the code supplied last so we can always attach a + # better-value discount by hand if we want. (Also, turn off finaid flag here.) + if discount_code: + try: + supplied_discount = Discount.objects.get( + discount_code=discount_code + ) + apply_discount_to_basket(basket, supplied_discount) + except Discount.DoesNotExist: + pass basket.refresh_from_db() if checkout: - return redirect("checkout_interstitial_page") + return redirect( + "checkout_interstitial_page" + if request.user.is_authenticated + else "checkout-anonymous" + ) return Response( BasketWithProductSerializer(basket).data, @@ -300,7 +320,7 @@ def _create_basket_from_product( ], ) @api_view(["POST"]) -@permission_classes((IsAuthenticated,)) +@permission_classes((AllowAny,)) def create_basket_from_product(request, product_id: int): """Run _create_basket_from_product.""" @@ -440,7 +460,7 @@ def create_basket_with_products(request): responses={204: OpenApiResponse(description="Basket cleared successfully")}, ) @api_view(["DELETE"]) -@permission_classes([IsAuthenticated]) +@permission_classes([AllowAny]) def clear_basket(request): """ Clear the basket for the current user. @@ -451,9 +471,9 @@ def clear_basket(request): Returns: Response: HTTP response """ - basket = establish_basket(request) - - basket.delete() + with transaction.atomic(): + basket = establish_basket_for_request(request, for_update=True) + basket.delete() return Response(None, status=status.HTTP_204_NO_CONTENT) diff --git a/ecommerce/views/v0/views_test.py b/ecommerce/views/v0/views_test.py index 02f8cd18d3..4091c6a33e 100644 --- a/ecommerce/views/v0/views_test.py +++ b/ecommerce/views/v0/views_test.py @@ -11,6 +11,7 @@ import pytest import reversion from django.forms.models import model_to_dict +from django.test import Client from django.urls import reverse from mitol.common.utils.datetime import now_in_utc from reversion.models import Version @@ -523,6 +524,86 @@ def test_create_basket_with_product( # noqa: PLR0913 ) +# These four tests use transaction=True rather than the default django_db +# marker. The views under test call select_for_update(), which requires an +# active transaction.atomic() block in the view itself - the default +# django_db marker silently masks a missing atomic() block by wrapping the +# whole test in its own outer transaction, so this is the only way to +# actually exercise (and catch regressions in) that requirement. +@pytest.mark.django_db(transaction=True, serialized_rollback=True) +def test_create_basket_from_product_anonymous(mocker): + """ + Test that an anonymous caller can create a basket via create_from_product, + without triggering hubspot sync or picking up auto-applied discounts. + """ + mock_sync = mocker.patch("ecommerce.views.v0.sync_hubspot_cart_add") + product = ProductFactory.create() + UnlimitedUseDiscountFactory.create(automatic=True) + + client = Client() + url = reverse( + "v0:baskets_api-create_from_product", + kwargs={"product_id": product.id}, + ) + + response = client.post(url) + + assert response.status_code == 201 + + basket = Basket.objects.get(id=response.data["id"]) + assert basket.is_anonymous is True + assert basket.basket_items.count() == 1 + assert basket.discounts.count() == 0 + mock_sync.assert_not_called() + + +@pytest.mark.django_db(transaction=True, serialized_rollback=True) +def test_create_basket_from_product_anonymous_checkout_redirect(mocker): + """Test that checkout=True redirects an anonymous caller to the anonymous checkout flow""" + mocker.patch("ecommerce.views.v0.sync_hubspot_cart_add") + product = ProductFactory.create() + + client = Client() + url = reverse( + "v0:baskets_api-create_from_product", + kwargs={"product_id": product.id}, + ) + + response = client.post(url, {"checkout": True}) + + assert response.status_code == 302 + assert response.url == reverse("checkout-anonymous") + + +@pytest.mark.django_db(transaction=True, serialized_rollback=True) +def test_clear_basket_anonymous(): + """Test that an anonymous caller can clear their own anonymous basket""" + client = Client() + product = ProductFactory.create() + + create_url = reverse( + "v0:baskets_api-create_from_product", + kwargs={"product_id": product.id}, + ) + response = client.post(create_url) + basket_id = response.data["id"] + + clear_response = client.delete(reverse("v0:baskets_api-clear_basket")) + + assert clear_response.status_code == 204 + assert not Basket.objects.filter(id=basket_id).exists() + + +@pytest.mark.django_db(transaction=True, serialized_rollback=True) +def test_clear_basket_anonymous_with_no_basket_yet(): + """Test that clearing with no prior basket is a harmless no-op""" + client = Client() + + response = client.delete(reverse("v0:baskets_api-clear_basket")) + + assert response.status_code == 204 + + @pytest.mark.parametrize( ["try_flex_pricing_discount", "try_whitespace"], # noqa: PT006 [ diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index 208cf19296..76678f0a96 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -1072,6 +1072,8 @@ paths: description: Clears the basket for the current user. tags: - baskets + security: + - {} responses: '204': description: Basket cleared successfully @@ -1088,6 +1090,8 @@ paths: required: true tags: - baskets + security: + - {} responses: '200': content: diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 8c396d919d..9c8efa21e1 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -1072,6 +1072,8 @@ paths: description: Clears the basket for the current user. tags: - baskets + security: + - {} responses: '204': description: Basket cleared successfully @@ -1088,6 +1090,8 @@ paths: required: true tags: - baskets + security: + - {} responses: '200': content: diff --git a/openapi/specs/v2.yaml b/openapi/specs/v2.yaml index 33486b8b47..05c9028ef0 100644 --- a/openapi/specs/v2.yaml +++ b/openapi/specs/v2.yaml @@ -1072,6 +1072,8 @@ paths: description: Clears the basket for the current user. tags: - baskets + security: + - {} responses: '204': description: Basket cleared successfully @@ -1088,6 +1090,8 @@ paths: required: true tags: - baskets + security: + - {} responses: '200': content: From 59fc63313887585f26e2a58942c4d9fa53bf3a6e Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Fri, 31 Jul 2026 15:14:16 -0400 Subject: [PATCH 21/21] pass anonymous basket id because we can't derive it from the session cross domain without keycloak --- drf_lint_baseline.json | 44 ++++++-------- ecommerce/serializers/__init__.py | 1 + ecommerce/serializers/v0/__init__.py | 1 + main/middleware.py | 45 +++++++++++++++ main/middleware_test.py | 86 +++++++++++++++++++++++++++- main/settings.py | 1 + openapi/specs/v0.yaml | 4 ++ openapi/specs/v1.yaml | 4 ++ openapi/specs/v2.yaml | 4 ++ 9 files changed, 161 insertions(+), 29 deletions(-) diff --git a/drf_lint_baseline.json b/drf_lint_baseline.json index baad7c7f54..95d29a0f41 100644 --- a/drf_lint_baseline.json +++ b/drf_lint_baseline.json @@ -20,11 +20,11 @@ "courses/serializers/v1/programs.py:346:17:ORM001", "courses/serializers/v1/programs.py:368:16:ORM001", "courses/serializers/v2/courses.py:272:17:ORM002", + "courses/serializers/v2/courses.py:337:18:ORM001", "courses/serializers/v2/departments.py:35:40:ORM002", "courses/serializers/v2/departments.py:49:42:ORM002", "courses/serializers/v2/programs.py:385:53:ORM002", "courses/serializers/v2/programs.py:495:12:ORM002", - "courses/serializers/v2/programs.py:596:12:ORM002", "courses/serializers/v3/courses.py:111:14:ORM001", "courses/serializers/v3/courses.py:55:12:ORM002", "courses/serializers/v3/programs.py:55:22:ORM001", @@ -37,14 +37,14 @@ "ecommerce/serializers/__init__.py:326:20:ORM002", "ecommerce/serializers/__init__.py:331:31:ORM002", "ecommerce/serializers/__init__.py:346:31:ORM002", - "ecommerce/serializers/__init__.py:417:24:ORM002", - "ecommerce/serializers/__init__.py:436:12:ORM001", - "ecommerce/serializers/__init__.py:460:22:ORM002", - "ecommerce/serializers/__init__.py:507:22:ORM002", - "ecommerce/serializers/__init__.py:571:20:ORM002", - "ecommerce/serializers/__init__.py:704:22:ORM002", - "ecommerce/serializers/__init__.py:821:28:ORM002", - "ecommerce/serializers/__init__.py:891:28:ORM002", + "ecommerce/serializers/__init__.py:418:24:ORM002", + "ecommerce/serializers/__init__.py:437:12:ORM001", + "ecommerce/serializers/__init__.py:461:22:ORM002", + "ecommerce/serializers/__init__.py:508:22:ORM002", + "ecommerce/serializers/__init__.py:572:20:ORM002", + "ecommerce/serializers/__init__.py:705:22:ORM002", + "ecommerce/serializers/__init__.py:822:28:ORM002", + "ecommerce/serializers/__init__.py:892:28:ORM002", "ecommerce/serializers/v0/__init__.py:279:17:ORM001", "ecommerce/serializers/v0/__init__.py:281:18:ORM001", "ecommerce/serializers/v0/__init__.py:282:18:ORM001", @@ -54,14 +54,14 @@ "ecommerce/serializers/v0/__init__.py:408:20:ORM002", "ecommerce/serializers/v0/__init__.py:414:35:ORM002", "ecommerce/serializers/v0/__init__.py:430:31:ORM002", - "ecommerce/serializers/v0/__init__.py:502:24:ORM002", - "ecommerce/serializers/v0/__init__.py:521:12:ORM001", - "ecommerce/serializers/v0/__init__.py:545:22:ORM002", - "ecommerce/serializers/v0/__init__.py:592:22:ORM002", - "ecommerce/serializers/v0/__init__.py:657:20:ORM002", - "ecommerce/serializers/v0/__init__.py:807:22:ORM002", + "ecommerce/serializers/v0/__init__.py:503:24:ORM002", + "ecommerce/serializers/v0/__init__.py:522:12:ORM001", + "ecommerce/serializers/v0/__init__.py:546:22:ORM002", + "ecommerce/serializers/v0/__init__.py:593:22:ORM002", + "ecommerce/serializers/v0/__init__.py:658:20:ORM002", + "ecommerce/serializers/v0/__init__.py:808:22:ORM002", "ecommerce/serializers/v0/__init__.py:83:28:ORM002", - "ecommerce/serializers/v0/__init__.py:951:28:ORM002", + "ecommerce/serializers/v0/__init__.py:952:28:ORM002", "flexiblepricing/serializers.py:129:38:ORM001", "flexiblepricing/serializers.py:132:34:ORM001", "flexiblepricing/serializers.py:147:34:ORM001", @@ -71,18 +71,6 @@ "flexiblepricing/serializers.py:207:31:ORM001", "flexiblepricing/serializers.py:212:16:ORM001", "flexiblepricing/serializers.py:216:16:ORM001", - "hubspot_sync/serializers.py:170:22:ORM002", - "hubspot_sync/serializers.py:171:22:ORM001", - "hubspot_sync/serializers.py:183:25:ORM002", - "hubspot_sync/serializers.py:186:33:ORM002", - "hubspot_sync/serializers.py:193:21:ORM002", - "hubspot_sync/serializers.py:312:33:ORM001", - "hubspot_sync/serializers.py:323:36:ORM001", - "hubspot_sync/serializers.py:324:33:ORM001", - "hubspot_sync/serializers.py:335:36:ORM001", - "hubspot_sync/serializers.py:64:22:ORM002", - "hubspot_sync/serializers.py:76:31:ORM002", - "hubspot_sync/serializers.py:80:31:ORM002", "users/serializers.py:209:16:ORM001", "users/serializers.py:254:20:ORM001", "users/serializers.py:301:19:ORM001", diff --git a/ecommerce/serializers/__init__.py b/ecommerce/serializers/__init__.py index ede1fed06c..1fcfb157bb 100644 --- a/ecommerce/serializers/__init__.py +++ b/ecommerce/serializers/__init__.py @@ -362,6 +362,7 @@ class Meta: fields = [ "id", "user", + "anonymous_id", "basket_items", "total_price", "discounted_price", diff --git a/ecommerce/serializers/v0/__init__.py b/ecommerce/serializers/v0/__init__.py index dc10628858..8a96f79079 100644 --- a/ecommerce/serializers/v0/__init__.py +++ b/ecommerce/serializers/v0/__init__.py @@ -446,6 +446,7 @@ class Meta: fields = [ "id", "user", + "anonymous_id", "basket_items", "total_price", "discounted_price", diff --git a/main/middleware.py b/main/middleware.py index b7acc4a52a..43879b2e45 100644 --- a/main/middleware.py +++ b/main/middleware.py @@ -1,14 +1,18 @@ """Common mitx_online middleware""" import logging +import uuid from urllib.parse import urlparse from django.conf import settings +from django.http import HttpResponseRedirect from django.middleware.csrf import CsrfViewMiddleware from django.utils.deprecation import MiddlewareMixin log = logging.getLogger(__name__) +ANONYMOUS_BASKET_HANDOFF_PARAM = "anonymous_basket_id" + class CachelessAPIMiddleware(MiddlewareMixin): """Add Cache-Control header to API responses""" @@ -25,6 +29,47 @@ def process_response(self, request, response): return response +class AnonymousBasketHandoffMiddleware(MiddlewareMixin): + """ + Adopt an anonymous_basket_id passed as a query parameter into this + request's own session, then redirect to the same URL with the parameter + stripped. + + An anonymous basket's session cookie is host-only, and MIT's shared + mit.edu domain can't be used to widen it (institution-wide cookie size + limits). Learn's frontend proxies basket API calls through a different + subdomain than the one that serves mitxonline's own pages, so the + cookie set during those API calls never reaches this domain on its own - + the id has to be handed off explicitly through the URL instead. + """ + + def process_request(self, request): + basket_id = request.GET.get(ANONYMOUS_BASKET_HANDOFF_PARAM) + if not basket_id: + return None + + if not request.user.is_authenticated and not request.session.get( + "anonymous_basket_id" + ): + try: + uuid.UUID(basket_id) + except ValueError: + log.warning( + "Ignoring malformed anonymous_basket_id query param: %s", + basket_id, + ) + else: + request.session["anonymous_basket_id"] = basket_id + + query_params = request.GET.copy() + del query_params[ANONYMOUS_BASKET_HANDOFF_PARAM] + redirect_url = request.path + if query_params: + redirect_url = f"{redirect_url}?{query_params.urlencode()}" + + return HttpResponseRedirect(redirect_url) + + class HostBasedCSRFMiddleware(CsrfViewMiddleware): """ CSRF middleware that changes the response cookie's domain property diff --git a/main/middleware_test.py b/main/middleware_test.py index b4c1799b39..a265297056 100644 --- a/main/middleware_test.py +++ b/main/middleware_test.py @@ -1,7 +1,13 @@ +import uuid + import pytest +from django.contrib.auth.models import AnonymousUser from django.http import HttpResponse -from main.middleware import HostBasedCSRFMiddleware +from main.middleware import AnonymousBasketHandoffMiddleware, HostBasedCSRFMiddleware +from users.factories import UserFactory + +pytestmark = [pytest.mark.django_db] @pytest.mark.parametrize( @@ -54,6 +60,84 @@ def test_host_based_csrf_middleware(mocker, rf, settings, host, expected_domain) ) +def test_anonymous_basket_handoff_no_param_is_a_noop(mocker, rf): + """Test that a request with no handoff param passes straight through""" + get_response = mocker.MagicMock() + middleware = AnonymousBasketHandoffMiddleware(get_response) + + request = rf.get("/cart/") + request.session = {} + request.user = AnonymousUser() + + assert middleware.process_request(request) is None + + +def test_anonymous_basket_handoff_adopts_valid_id(mocker, rf): + """Test that a valid handoff id is adopted into the session and the param is stripped""" + get_response = mocker.MagicMock() + middleware = AnonymousBasketHandoffMiddleware(get_response) + + anon_id = str(uuid.uuid4()) + request = rf.get(f"/cart/?anonymous_basket_id={anon_id}&other=1") + request.session = {} + request.user = AnonymousUser() + + response = middleware.process_request(request) + + assert response.status_code == 302 + assert response.url == "/cart/?other=1" + assert request.session["anonymous_basket_id"] == anon_id + + +def test_anonymous_basket_handoff_ignores_malformed_id(mocker, rf): + """Test that a malformed id is not stored, but the param is still stripped""" + get_response = mocker.MagicMock() + middleware = AnonymousBasketHandoffMiddleware(get_response) + + request = rf.get("/cart/?anonymous_basket_id=not-a-uuid") + request.session = {} + request.user = AnonymousUser() + + response = middleware.process_request(request) + + assert response.status_code == 302 + assert response.url == "/cart/" + assert "anonymous_basket_id" not in request.session + + +def test_anonymous_basket_handoff_does_not_overwrite_existing_session(mocker, rf): + """Test that an id already established in this session takes precedence""" + get_response = mocker.MagicMock() + middleware = AnonymousBasketHandoffMiddleware(get_response) + + existing_id = str(uuid.uuid4()) + incoming_id = str(uuid.uuid4()) + request = rf.get(f"/cart/?anonymous_basket_id={incoming_id}") + request.session = {"anonymous_basket_id": existing_id} + request.user = AnonymousUser() + + response = middleware.process_request(request) + + assert response.status_code == 302 + assert request.session["anonymous_basket_id"] == existing_id + + +def test_anonymous_basket_handoff_skips_session_write_when_authenticated(mocker, rf): + """Test that an authenticated request never has its session mutated by this middleware""" + get_response = mocker.MagicMock() + middleware = AnonymousBasketHandoffMiddleware(get_response) + + anon_id = str(uuid.uuid4()) + request = rf.get(f"/cart/?anonymous_basket_id={anon_id}") + request.session = {} + request.user = UserFactory.create() + + response = middleware.process_request(request) + + assert response.status_code == 302 + assert "anonymous_basket_id" not in request.session + + def test_host_based_csrf_middleware_no_referer(mocker, rf, settings): """Test that middleware handles missing referer header gracefully.""" settings.CSRF_COOKIE_NAME = "csrf_mitxonline" diff --git a/main/settings.py b/main/settings.py index ec5a939316..eb42911337 100644 --- a/main/settings.py +++ b/main/settings.py @@ -304,6 +304,7 @@ "django.middleware.common.CommonMiddleware", "django.contrib.auth.middleware.AuthenticationMiddleware", "mitol.apigateway.middleware.ApisixUserMiddleware", + "main.middleware.AnonymousBasketHandoffMiddleware", "django.middleware.csrf.CsrfViewMiddleware", "main.middleware.HostBasedCSRFMiddleware", "django.contrib.messages.middleware.MessageMiddleware", diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index 76678f0a96..f040c12c90 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -4523,6 +4523,10 @@ components: user: type: integer nullable: true + anonymous_id: + type: string + format: uuid + nullable: true basket_items: type: array items: diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 9c8efa21e1..0ec7915da0 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -4523,6 +4523,10 @@ components: user: type: integer nullable: true + anonymous_id: + type: string + format: uuid + nullable: true basket_items: type: array items: diff --git a/openapi/specs/v2.yaml b/openapi/specs/v2.yaml index 05c9028ef0..60a9383ec8 100644 --- a/openapi/specs/v2.yaml +++ b/openapi/specs/v2.yaml @@ -4523,6 +4523,10 @@ components: user: type: integer nullable: true + anonymous_id: + type: string + format: uuid + nullable: true basket_items: type: array items: