Address review feedback on anonymous checkout

frontends/api/src/mitxonline/hooks/baskets/index.ts

@@ -1,7 +1,6 @@
 import { basketQueries } from "./queries"
 import { useMutation, useQueryClient } from "@tanstack/react-query"
 import { basketsApi } from "../../clients"
-import { useUserIsAuthenticated } from "../../../hooks/user"
 import type { BasketWithProduct } from "@mitodl/mitxonline-api-axios/v2"
 
 /**
@@ -10,7 +9,6 @@ import type { BasketWithProduct } from "@mitodl/mitxonline-api-axios/v2"
  */
 const useAddToBasket = () => {
   const queryClient = useQueryClient()
-  const isAuthenticated = useUserIsAuthenticated()
   return useMutation({
     mutationFn: async (productId: number): Promise<BasketWithProduct> => {
       const response = await basketsApi.basketsCreateFromProductCreate({
@@ -19,16 +17,10 @@ const useAddToBasket = () => {
       return response.data
     },
     onSuccess: async () => {
-      // Invalidate checkout query to ensure fresh data - but only once
-      // authenticated. basketState hits the checkout-payload endpoint, which
-      // creates a real order against the basket's purchaser; an anonymous
-      // basket has no purchaser, so this has to wait until after the
-      // anonymous-checkout conversion step, not fire on every cart add.
-      if (isAuthenticated) {
-        queryClient.invalidateQueries({
-          queryKey: basketQueries.basketState().queryKey,
-        })
-      }
+      // Invalidate checkout query to ensure fresh data
+      queryClient.invalidateQueries({
+        queryKey: basketQueries.basketState().queryKey,
+      })
     },
   })
 }
@@ -38,17 +30,14 @@ const useAddToBasket = () => {
  */
 const useClearBasket = () => {
   const queryClient = useQueryClient()
-  const isAuthenticated = useUserIsAuthenticated()
   return useMutation({
     mutationFn: async (): Promise<void> => {
       await basketsApi.basketsClearDestroy()
     },
     onSuccess: () => {
-      if (isAuthenticated) {
-        queryClient.invalidateQueries({
-          queryKey: basketQueries.basketState().queryKey,
-        })
-      }
+      queryClient.invalidateQueries({
+        queryKey: basketQueries.basketState().queryKey,
+      })
     },
   })
 }

frontends/api/src/mitxonline/test-utils/factories/baskets.ts

@@ -0,0 +1,42 @@
+import { faker } from "@faker-js/faker/locale/en"
+import invariant from "tiny-invariant"
+import type { BasketWithProduct } from "@mitodl/mitxonline-api-axios/v2"
+
+const isSet = (value: unknown) => value !== null && value !== undefined
+
+/**
+ * A basket owned by a signed-in user. See `anonymousBasket` for the other case:
+ * `user` and `anonymous_id` are mutually exclusive, and MITx Online enforces
+ * that with a database constraint (`basket_user_xor_anonymous_id`).
+ */
+const basket = (
+  overrides: Partial<BasketWithProduct> = {},
+): BasketWithProduct => {
+  const merged: BasketWithProduct = {
+    id: faker.number.int({ min: 1 }),
+    user: faker.number.int({ min: 1 }),
+    anonymous_id: null,
+    basket_items: [],
+    total_price: 0,
+    discounted_price: 0,
+    discounts: [],
+    ...overrides,
+  }
+  invariant(
+    isSet(merged.user) !== isSet(merged.anonymous_id),
+    "A basket has exactly one of `user` and `anonymous_id`. Use basket() for a signed-in user, anonymousBasket() otherwise.",
+  )
+  return merged
+}
+
+/**
+ * A basket built before the learner signed in. `anonymous_id` is a real UUID:
+ * MITx Online's handoff middleware parses it with `uuid.UUID()` and
+ * logs-and-ignores anything that doesn't.
+ */
+const anonymousBasket = (
+  overrides: Partial<BasketWithProduct> = {},
+): BasketWithProduct =>
+  basket({ user: null, anonymous_id: faker.string.uuid(), ...overrides })
+
+export { basket, anonymousBasket }

frontends/api/src/mitxonline/test-utils/factories/index.ts

@@ -8,6 +8,7 @@ import * as user from "./user"
 import * as requirements from "./requirements"
 import * as contracts from "./contracts"
 import * as orders from "./orders"
+import * as baskets from "./baskets"
 
 export {
   mitx as enrollment,
@@ -20,4 +21,5 @@ export {
   requirements,
   contracts,
   orders,
+  baskets,
 }

frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrolledCourseCard.test.tsx

@@ -10,11 +10,7 @@ import {
 } from "@/test-utils"
 import * as mitxonline from "api/mitxonline-test-utils"
 import { mitxonlineLegacyUrl } from "@/common/mitxonline"
-import {
-  makeRequest,
-  urls as learnUrls,
-  factories as learnFactories,
-} from "api/test-utils"
+import { makeRequest } from "api/test-utils"
 import { faker } from "@faker-js/faker/locale/en"
 import moment from "moment"
 import { EnrolledCourseCard } from "./EnrolledCourseCard"
@@ -24,17 +20,6 @@ const EnrollmentMode = {
   Verified: "verified",
 } as const
 
-// useAddToBasket/useClearBasket check Learn's own auth state (separate from
-// mitxonline's) before invalidating the checkout-payload query. File-scoped
-// (not per-describe) since several describe blocks below each have their own
-// tests that don't all route through setupUserApis.
-beforeEach(() => {
-  setMockResponse.get(
-    learnUrls.userMe.get(),
-    learnFactories.user.user({ is_authenticated: true }),
-  )
-})
-
 const setupUserApis = (
   overrides?: Parameters<typeof mitxonline.factories.user.user>[0],
 ) => {
@@ -455,7 +440,7 @@ describe.each([
     const clearUrl = mitxonline.urls.baskets.clear()
     setMockResponse.delete(clearUrl, undefined)
     const basketUrl = mitxonline.urls.baskets.createFromProduct(productId)
-    setMockResponse.post(basketUrl, { id: 1, items: [] })
+    setMockResponse.post(basketUrl, mitxonline.factories.baskets.basket())
 
     renderWithProviders(<EnrolledCourseCard enrollment={enrollment} />)
     await user.click(
@@ -656,7 +641,7 @@ describe.each([
     const clearUrl = mitxonline.urls.baskets.clear()
     setMockResponse.delete(clearUrl, undefined)
     const basketUrl = mitxonline.urls.baskets.createFromProduct(productId)
-    setMockResponse.post(basketUrl, { id: 1, items: [] })
+    setMockResponse.post(basketUrl, mitxonline.factories.baskets.basket())
 
     renderWithProviders(
       <EnrolledCourseCard

frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/HomeEnrollmentsDisplay.test.tsx

@@ -26,7 +26,6 @@ import {
 } from "@/test-utils"
 import { HomeEnrollmentsDisplay } from "./HomeEnrollmentsDisplay"
 import * as mitxonline from "api/mitxonline-test-utils"
-import { urls as learnUrls, factories as learnFactories } from "api/test-utils"
 import { useFeatureFlagEnabled } from "posthog-js/react"
 import { setupEnrollments } from "./test-utils"
 import { faker } from "@faker-js/faker/locale/en"
@@ -39,17 +38,6 @@ const mockedUseFeatureFlagEnabled = jest
 describe("HomeEnrollmentsDisplay", () => {
   setupLocationMock()
 
-  // useAddToBasket/useClearBasket check Learn's own auth state (separate
-  // from mitxonline's) before invalidating the checkout-payload query. Set at
-  // the describe level since many tests here mock the rest of the user/
-  // enrollment APIs inline rather than through setupApis below.
-  beforeEach(() => {
-    setMockResponse.get(
-      learnUrls.userMe.get(),
-      learnFactories.user.user({ is_authenticated: true }),
-    )
-  })
-
   const setupApis = (includeExpired: boolean = true) => {
     const mitxOnlineUser = mitxonline.factories.user.user()
     setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser)

frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/OrganizationCards.test.tsx

@@ -348,7 +348,6 @@ describe("OrganizationCards", () => {
         name: "Test Organization",
         logo: undefined,
         slug: "org-test-org",
-        sso_organization_id: null,
         contracts: [
           mitxOnlineFactories.contracts.contract({
             id: 1,

frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramAsCourseCard.test.tsx

@@ -7,11 +7,7 @@ import {
   user,
   within,
 } from "@/test-utils"
-import {
-  makeRequest,
-  urls as learnUrls,
-  factories as learnFactories,
-} from "api/test-utils"
+import { makeRequest } from "api/test-utils"
 import * as mitxonline from "api/mitxonline-test-utils"
 import { ProgramAsCourseCard } from "./ProgramAsCourseCard"
 import { waitFor } from "@testing-library/react"
@@ -26,15 +22,6 @@ jest.mock("posthog-js/react")
 describe("ProgramAsCourseCard", () => {
   setupLocationMock()
 
-  // useAddToBasket/useClearBasket check Learn's own auth state (separate
-  // from mitxonline's) before invalidating the checkout-payload query.
-  beforeEach(() => {
-    setMockResponse.get(
-      learnUrls.userMe.get(),
-      learnFactories.user.user({ is_authenticated: true }),
-    )
-  })
-
   /**
    * Creates a ProgramAsCourseCard data set with:
    * - A program with two module courses linked via req_tree
@@ -543,7 +530,7 @@ describe("ProgramAsCourseCard", () => {
     const clearUrl = mitxonline.urls.baskets.clear()
     setMockResponse.delete(clearUrl, undefined)
     const basketUrl = mitxonline.urls.baskets.createFromProduct(product.id)
-    setMockResponse.post(basketUrl, { id: 1, items: [] })
+    setMockResponse.post(basketUrl, mitxonline.factories.baskets.basket())
 
     renderWithProviders(
       <ProgramAsCourseCard

frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ProgramEnrollmentDisplay.test.tsx

@@ -27,11 +27,7 @@ import {
 } from "@/test-utils"
 import { ProgramEnrollmentDisplay } from "./ProgramEnrollmentDisplay"
 import * as mitxonline from "api/mitxonline-test-utils"
-import {
-  makeRequest,
-  urls as learnUrls,
-  factories as learnFactories,
-} from "api/test-utils"
+import { makeRequest } from "api/test-utils"
 import { useFeatureFlagEnabled } from "posthog-js/react"
 import { faker } from "@faker-js/faker/locale/en"
 import invariant from "tiny-invariant"
@@ -44,15 +40,6 @@ const mockedUseFeatureFlagEnabled = jest
 describe("ProgramEnrollmentDisplay", () => {
   setupLocationMock()
 
-  // useAddToBasket/useClearBasket check Learn's own auth state (separate
-  // from mitxonline's) before invalidating the checkout-payload query.
-  beforeEach(() => {
-    setMockResponse.get(
-      learnUrls.userMe.get(),
-      learnFactories.user.user({ is_authenticated: true }),
-    )
-  })
-
   test("Filters to single program when programId is provided", async () => {
     const mitxOnlineUser = mitxonline.factories.user.user()
     setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser)

frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/UnenrolledCourseCard.test.tsx

@@ -9,11 +9,7 @@ import {
   within,
 } from "@/test-utils"
 import * as mitxonline from "api/mitxonline-test-utils"
-import {
-  makeRequest,
-  urls as learnUrls,
-  factories as learnFactories,
-} from "api/test-utils"
+import { makeRequest } from "api/test-utils"
 import { faker } from "@faker-js/faker/locale/en"
 import moment from "moment"
 import { cartesianProduct } from "ol-test-utilities"
@@ -29,17 +25,6 @@ const mitxOnlineCourse = mitxonline.factories.courses.course
 
 const mitxUser = mitxonline.factories.user.user
 
-// useAddToBasket/useClearBasket check Learn's own auth state (separate from
-// mitxonline's) before invalidating the checkout-payload query. File-scoped
-// (not per-describe) since several describe blocks below each have their own
-// tests that don't all route through setupUserApis.
-beforeEach(() => {
-  setMockResponse.get(
-    learnUrls.userMe.get(),
-    learnFactories.user.user({ is_authenticated: true }),
-  )
-})
-
 const setupUserApis = (overrides?: Parameters<typeof mitxUser>[0]) => {
   const userData = mitxonline.factories.user.user({
     is_staff: false,
@@ -596,7 +581,7 @@ describe.each([
         const clearUrl = mitxonline.urls.baskets.clear()
         setMockResponse.delete(clearUrl, undefined)
         const basketUrl = mitxonline.urls.baskets.createFromProduct(product.id)
-        setMockResponse.post(basketUrl, { id: 1, items: [] })
+        setMockResponse.post(basketUrl, mitxonline.factories.baskets.basket())
 
         renderWithProviders(<UnenrolledCourseCard course={course} />)
 
@@ -808,7 +793,7 @@ describe.each([
       const clearUrl = mitxonline.urls.baskets.clear()
       setMockResponse.delete(clearUrl, undefined)
       const basketUrl = mitxonline.urls.baskets.createFromProduct(product.id)
-      setMockResponse.post(basketUrl, { id: 1, items: [] })
+      setMockResponse.post(basketUrl, mitxonline.factories.baskets.basket())
 
       renderWithProviders(
         <UnenrolledCourseCard

frontends/main/src/app-pages/ProductPages/CourseEnrollArea.test.tsx

@@ -365,10 +365,10 @@ describe("CourseEnrollArea — click smoke tests", () => {
     const course = makeCourse({ next_run_id: run.id, courseruns: [run] })
 
     setMockResponse.delete(mitxUrls.baskets.clear(), undefined)
-    setMockResponse.post(mitxUrls.baskets.createFromProduct(product.id), {
-      id: 1,
-      items: [],
-    })
+    setMockResponse.post(
+      mitxUrls.baskets.createFromProduct(product.id),
+      mitxFactories.baskets.basket(),
+    )
 
     renderWithProviders(<CourseEnrollArea course={course} selectedRun={run} />)
 

frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx

@@ -485,10 +485,10 @@ describe("CoursePage", () => {
         )
         setMockResponse.get(mitxUrls.enrollment.enrollmentsListV3(), [])
         setMockResponse.delete(mitxUrls.baskets.clear(), undefined)
-        setMockResponse.post(mitxUrls.baskets.createFromProduct(product.id), {
-          id: 1,
-          items: [],
-        })
+        setMockResponse.post(
+          mitxUrls.baskets.createFromProduct(product.id),
+          mitxFactories.baskets.basket(),
+        )
 
         renderWithProviders(<CoursePage readableId={course.readable_id} />)