diff --git a/__tests__/enterprise/consent-gates.test.ts b/__tests__/enterprise/consent-gates.test.ts index bcc804147..2d263a918 100644 --- a/__tests__/enterprise/consent-gates.test.ts +++ b/__tests__/enterprise/consent-gates.test.ts @@ -9,16 +9,35 @@ */ const mockFindFirst = jest.fn(); +const mockUpdateMany = jest.fn(); jest.mock("../../lib/prisma", () => ({ __esModule: true, - default: { consentArtifact: { findFirst: (...a: unknown[]) => mockFindFirst(...a) } }, + default: { + consentArtifact: { + findFirst: (...a: unknown[]) => mockFindFirst(...a), + updateMany: (...a: unknown[]) => mockUpdateMany(...a), + }, + }, })); -import { checkConsent } from "@/lib/compliance/dpdp"; -import { PURPOSE_CODES } from "@/lib/compliance/purpose-codes"; +// withdrawConsent fires a SESSION_BOOKING cascade event through a dynamic +// import; the gate under test does not depend on it. +jest.mock("../../lib/enterprise/system-events", () => ({ + __esModule: true, + recordSystemEvent: jest.fn(), +})); + +import { checkConsent, withdrawConsent } from "@/lib/compliance/dpdp"; +import { + PURPOSE_CODES, + purposeCodeAliases, +} from "@/lib/compliance/purpose-codes"; -beforeEach(() => mockFindFirst.mockReset()); +beforeEach(() => { + mockFindFirst.mockReset(); + mockUpdateMany.mockReset(); +}); describe("checkConsent — fail-closed (#701)", () => { it("is true when a live artifact for the purpose exists", async () => { @@ -31,7 +50,8 @@ describe("checkConsent — fail-closed (#701)", () => { // The query must filter withdrawn + expired out and match the purpose code. const where = mockFindFirst.mock.calls[0][0].where; expect(where.userId).toBe("u1"); - expect(where.purposeCodes).toEqual({ has: PURPOSE_CODES.SESSION_BOOKING }); + // #1472 — the canonical code plus its legacy aliases, not an exact match. + expect(where.purposeCodes.hasSome).toContain(PURPOSE_CODES.SESSION_BOOKING); expect(where.withdrawnAt).toBeNull(); expect(where.auditRetainedUntil.gt).toBeInstanceOf(Date); }); @@ -50,3 +70,53 @@ describe("checkConsent — fail-closed (#701)", () => { expect(PURPOSE_CODES.PRIMARY_PROCESSING).toBe("PRIMARY_PROCESSING"); }); }); + +/** + * #1472 — the pre-taxonomy kebab-case codes are still on disk (no backfill: + * pre-MVP reset). A consent record is a legal artifact, so the gate that + * decides whether a consultant can be booked must recognise every code the + * platform ever wrote for that purpose, and a narrow withdrawal must reach the + * same rows the gate reads. + */ +describe("#1472 legacy purpose codes are recognised by the runtime gates", () => { + /** Match an artifact's stored codes against a `hasSome` clause, as PG would. */ + const matches = ( + where: { purposeCodes: { hasSome: string[] } }, + stored: string[], + ) => stored.some((code) => where.purposeCodes.hasSome.includes(code)); + + it("lets a `session-booking` artifact satisfy a SESSION_BOOKING check", async () => { + mockFindFirst.mockResolvedValue({ id: "legacy-artifact" }); + await checkConsent({ + userId: "u1", + purposeCode: PURPOSE_CODES.SESSION_BOOKING, + }); + + const where = mockFindFirst.mock.calls[0][0].where; + expect(matches(where, ["session-booking"])).toBe(true); + // An unrelated legacy code is NOT swept in by the same alias set. + expect(matches(where, ["marketing"])).toBe(false); + }); + + it("withdraws a `session-booking` artifact on a SESSION_BOOKING withdrawal", async () => { + mockUpdateMany.mockResolvedValue({ count: 1 }); + await withdrawConsent({ + userId: "u1", + purposeCode: PURPOSE_CODES.SESSION_BOOKING, + }); + + const where = mockUpdateMany.mock.calls[0][0].where; + expect(matches(where, ["session-booking"])).toBe(true); + expect(matches(where, ["third-party-sharing-with-stream"])).toBe(false); + }); + + it("resolves aliases per purpose, never across purposes", () => { + expect(purposeCodeAliases(PURPOSE_CODES.SESSION_BOOKING)).toEqual([ + "SESSION_BOOKING", + "session-booking", + ]); + expect(purposeCodeAliases(PURPOSE_CODES.MARKETING_COMMS)).not.toContain( + "session-booking", + ); + }); +}); diff --git a/__tests__/payments/checkout-pool-1-nesting.test.ts b/__tests__/payments/checkout-pool-1-nesting.test.ts index a1928d05b..0602a7072 100644 --- a/__tests__/payments/checkout-pool-1-nesting.test.ts +++ b/__tests__/payments/checkout-pool-1-nesting.test.ts @@ -35,6 +35,11 @@ const mockTxClient = { slotOfAppointment: { findFirst: jest.fn(async () => null), }, + // #1463 — the self-hold lookup is a fourth read on this helper's path, and + // it must ride the transaction client like every other one. + appointment: { + findMany: jest.fn(async (): Promise => []), + }, }; jest.mock("../../lib/prisma", () => { @@ -124,7 +129,9 @@ function slotInput(): CheckoutInput { } /** The shape every real caller uses: the helper runs inside an open tx. */ -function validateInsideTransaction(): Promise { +function validateInsideTransaction(): Promise<{ + selfHoldAppointmentIds: string[]; +}> { return prisma.$transaction(async (tx) => validateSlotAvailability( tx as unknown as Tx, @@ -145,7 +152,9 @@ describe("#1421 checkout does not starve the single-connection pool", () => { }); it("runs the consent gate on the transaction client, not the global one", async () => { - await expect(validateInsideTransaction()).resolves.toBeUndefined(); + await expect(validateInsideTransaction()).resolves.toEqual({ + selfHoldAppointmentIds: [], + }); expect(mockTxClient.consentArtifact.findFirst).toHaveBeenCalledTimes(1); expect(mockGlobalTouches).toEqual([]); diff --git a/__tests__/payments/checkout-self-hold-resume.test.ts b/__tests__/payments/checkout-self-hold-resume.test.ts new file mode 100644 index 000000000..d121024a8 --- /dev/null +++ b/__tests__/payments/checkout-self-hold-resume.test.ts @@ -0,0 +1,247 @@ +/** + * @jest-environment node + */ + +/** + * #1463 — a buyer who closes the gateway modal and clicks Pay again used to be + * blocked by their OWN tentative hold. `validateSlotAvailability` ran before + * the open-order resume (`findReusablePendingOrderPayment`, "Rec C") and threw + * "Time slot is already booked", so the documented same-order resume was + * unreachable and the buyer waited for the hold to expire. + * + * The exclusion has to be exactly as narrow as the resume gate it feeds, which + * is what this pin holds in place: the same buyer, the same plan, the same + * gateway and the exact same window passes; a different buyer on the same slot + * still blocks; the same buyer on an overlapping-but-different window still + * blocks; and (#1465-triage) so does a hold minted on a different gateway, + * which `findReusablePendingOrderPayment` could neither resume nor supersede. + * + * The transaction client below evaluates the two blocking predicates against an + * in-memory hold rather than asserting on query shape, so the self-hold + * exclusion has to actually work for these to pass. Exactness in particular is + * real: a booked window is stored as N contiguous 30-minute atoms, and the + * helper decides coverage from the run's first start and last end. + */ + +// Boundary mocks. `lib/payments/operations/checkout` transitively imports the +// auth stack through the payouts barrel, which is ESM-only under this Jest +// transform; none of it is reached by the availability helper, which is handed +// its own transaction client. +jest.mock("../../lib/prisma", () => ({ __esModule: true, default: {} })); + +jest.mock("../../lib/payments/payouts", () => ({ + __esModule: true, + createEarningsFromPayment: jest.fn(), +})); + +jest.mock("../../lib/payments/index", () => ({ + __esModule: true, + createPaymentIntent: jest.fn(), + cancelPaymentIntent: jest.fn(), +})); + +jest.mock("../../utils/appointmentlock", () => ({ + __esModule: true, + CHECKOUT_WAIT_RETRY_CONFIG: { retryCount: 5 }, + CHECKOUT_LOCK_TTL_MS: {}, + EventFullError: class extends Error {}, + lockSlotBooking: jest.fn(), + unlockSlotBooking: jest.fn(), + lockEventCheckout: jest.fn(), + unlockEventCheckout: jest.fn(), + lockConsulteeBooking: jest.fn(), + unlockConsulteeBooking: jest.fn(), + extendLock: jest.fn(), + extendSlotInterval: jest.fn(), +})); + +jest.mock("../../lib/compliance/dpdp", () => ({ + __esModule: true, + checkConsent: jest.fn(async () => true), + PURPOSE_CODES: { SESSION_BOOKING: "SESSION_BOOKING" }, +})); + +import type { Tx } from "../../lib/prisma"; +import type { CheckoutInput } from "../../schemas/checkout"; +import { validateSlotAvailability } from "../../lib/payments/operations/checkout"; + +const MINUTE_MS = 60 * 1000; +const HOUR_MS = 60 * MINUTE_MS; + +const BUYER = "buyer-user-1"; +const CONSULTANT = "consultant-user-1"; +const PLAN = "plan-1"; + +/** The held window: 48 h out, one hour long, stored as two 30-minute atoms. */ +const WINDOW_START = new Date(Date.now() + 48 * HOUR_MS); +const WINDOW_END = new Date(WINDOW_START.getTime() + HOUR_MS); + +interface HeldSlot { + appointmentId: string; + startsAt: Date; + endsAt: Date; + isTentative: boolean; +} + +/** One live tentative hold, minted by a previous checkout attempt. */ +const HOLD_APPOINTMENT_ID = "appt-hold"; +const heldSlots: HeldSlot[] = [ + { + appointmentId: HOLD_APPOINTMENT_ID, + startsAt: WINDOW_START, + endsAt: new Date(WINDOW_START.getTime() + 30 * MINUTE_MS), + isTentative: true, + }, + { + appointmentId: HOLD_APPOINTMENT_ID, + startsAt: new Date(WINDOW_START.getTime() + 30 * MINUTE_MS), + endsAt: WINDOW_END, + isTentative: true, + }, +]; + +/** The hold's live PENDING payment belongs to the buyer, and to nobody else. */ +const HOLD_OWNER = BUYER; + +/** ...and it was minted on the gateway the resume gate would look for. */ +const HOLD_GATEWAY = "RAZORPAY"; + +/** The `AND` terms of the two blocking slot queries this suite discriminates. */ +interface SlotWhereTerm { + NOT?: SlotWhereTerm; + appointmentId?: { in: string[] }; + startsAt?: { lt: Date }; + endsAt?: { gt: Date }; + isTentative?: boolean; +} + +/** The self-hold lookup's `where`, as far as this suite reads it. */ +interface SelfHoldWhere { + payment?: { + some?: { + userId?: string; + paymentGateway?: string; + organizationId?: string | null; + }; + }; + consultation?: { consultationPlanId?: string }; + slotsOfAppointment?: { some?: { startsAt?: Date } }; +} + +/** + * Evaluate one term of the blocking queries' `AND` array against a held slot. + * Only the terms this suite can discriminate are modelled; the relation terms + * (consultant membership, occupancy, the live-payment join) are true for the + * single fixture hold by construction. + */ +function termMatches(slot: HeldSlot, term: SlotWhereTerm): boolean { + if (term.NOT) return !termMatches(slot, term.NOT); + if (term.appointmentId?.in) { + return term.appointmentId.in.includes(slot.appointmentId); + } + if (term.startsAt?.lt) return slot.startsAt < term.startsAt.lt; + if (term.endsAt?.gt) return slot.endsAt > term.endsAt.gt; + if (term.isTentative !== undefined) { + return slot.isTentative === term.isTentative; + } + return true; +} + +const tx = { + // The self-hold lookup: same buyer, same plan, an atom starting at the + // requested window's start. Exact coverage is decided by the helper itself. + appointment: { + findMany: async ({ where }: { where: SelfHoldWhere }) => { + const wantedStart = where.slotsOfAppointment?.some?.startsAt; + if (where.payment?.some?.userId !== HOLD_OWNER) return []; + // #1465-triage — the resume gate's own scope, and therefore this + // exclusion's: a hold on another gateway or another org is not adoptable. + if (where.payment?.some?.paymentGateway !== HOLD_GATEWAY) return []; + if ((where.payment?.some?.organizationId ?? null) !== null) return []; + if (where.consultation?.consultationPlanId !== PLAN) return []; + if ( + !wantedStart || + !heldSlots.some((s) => s.startsAt.getTime() === wantedStart.getTime()) + ) { + return []; + } + return [ + { + id: HOLD_APPOINTMENT_ID, + slotsOfAppointment: heldSlots.map((s) => ({ + startsAt: s.startsAt, + endsAt: s.endsAt, + })), + }, + ]; + }, + }, + slotOfAppointment: { + findFirst: async ({ where }: { where: { AND: SlotWhereTerm[] } }) => + heldSlots.find((slot) => + where.AND.every((term) => termMatches(slot, term)), + ) ?? null, + }, +} as unknown as Tx; + +function checkoutInput( + startsAt: Date, + endsAt: Date, + paymentGateway: string = HOLD_GATEWAY, +): CheckoutInput { + return { + appointmentType: "CONSULTATION", + planId: PLAN, + paymentGateway, + startsAt: startsAt.toISOString(), + endsAt: endsAt.toISOString(), + } as unknown as CheckoutInput; +} + +describe("#1463 the buyer's own live hold does not block their resume", () => { + it("lets the same buyer, same plan and exact window through to the open-order resume", async () => { + const result = await validateSlotAvailability( + tx, + checkoutInput(WINDOW_START, WINDOW_END), + BUYER, + CONSULTANT, + ); + + expect(result.selfHoldAppointmentIds).toEqual([HOLD_APPOINTMENT_ID]); + }); + + it("still blocks a different buyer on the same slot", async () => { + await expect( + validateSlotAvailability( + tx, + checkoutInput(WINDOW_START, WINDOW_END), + "other-buyer", + CONSULTANT, + ), + ).rejects.toThrow("Time slot is already booked"); + }); + + it("still blocks a hold this request could not resume (other gateway)", async () => { + await expect( + validateSlotAvailability( + tx, + checkoutInput(WINDOW_START, WINDOW_END, "STRIPE"), + BUYER, + CONSULTANT, + ), + ).rejects.toThrow("Time slot is already booked"); + }); + + it("still blocks the same buyer on an overlapping but different window", async () => { + const shifted = new Date(WINDOW_START.getTime() + 30 * MINUTE_MS); + + await expect( + validateSlotAvailability( + tx, + checkoutInput(shifted, new Date(shifted.getTime() + HOUR_MS)), + BUYER, + CONSULTANT, + ), + ).rejects.toThrow("Time slot is already booked"); + }); +}); diff --git a/__tests__/payments/gateway-note-limits.test.ts b/__tests__/payments/gateway-note-limits.test.ts index 707593e4c..b20307f68 100644 --- a/__tests__/payments/gateway-note-limits.test.ts +++ b/__tests__/payments/gateway-note-limits.test.ts @@ -96,4 +96,33 @@ describe("#1437 gateway note limits", () => { // sat at exactly Razorpay's 15-key ceiling before `discountCode` was cut. expect(Object.keys(metadata).length).toBeLessThanOrEqual(14); }); + + /** + * #1462 — the same payload, seen from the webhook's side. A scheduling-period + * subscription has no slot times, and sending them as `""` failed + * `z.string().datetime().optional()` on every capture, stranding the sale as + * REQUIRES_MANUAL_RECOVERY with the buyer already charged. + */ + it("omits every empty optional field instead of sending it as an empty string", () => { + const metadata = buildPaymentMetadata( + { + appointmentType: "SUBSCRIPTION", + planId: "plan-1", + paymentGateway: "RAZORPAY", + schedulingPeriodStartsAt: "2026-09-01T00:00:00.000Z", + schedulingPeriodEndsAt: "2026-12-01T00:00:00.000Z", + } as unknown as CheckoutInput, + "user-1", + ); + + expect(metadata).not.toHaveProperty("startsAt"); + expect(metadata).not.toHaveProperty("endsAt"); + expect(metadata).not.toHaveProperty("slotOfAvailabilityWeeklyId"); + expect(metadata).not.toHaveProperty("slotOfAvailabilityCustomId"); + expect(metadata).not.toHaveProperty("notes"); + expect(Object.values(metadata)).not.toContain(""); + // What the sale actually needs still travels. + expect(metadata.schedulingPeriodStartsAt).toBe("2026-09-01T00:00:00.000Z"); + expect(metadata.schedulingPeriodEndsAt).toBe("2026-12-01T00:00:00.000Z"); + }); }); diff --git a/__tests__/schemas/webhook-metadata.test.ts b/__tests__/schemas/webhook-metadata.test.ts index 5f70bf5d7..aea9ce9ec 100644 --- a/__tests__/schemas/webhook-metadata.test.ts +++ b/__tests__/schemas/webhook-metadata.test.ts @@ -65,3 +65,57 @@ describe("validateWebhookMetadata — slot-key rename dual-read", () => { expect(() => validateWebhookMetadata({ ...BASE })).toThrow(); }); }); + +/** + * #1462 — a scheduling-period subscription carries no direct slots, and the + * builder used to send `startsAt`/`endsAt` to the gateway as empty strings. + * `z.string().datetime().optional()` admits an ABSENT key and rejects `""`, so + * every capture for such a sale failed validation and was stamped + * REQUIRES_MANUAL_RECOVERY with the money already taken. The builder no longer + * emits those keys, but Razorpay orders never expire, so orders already minted + * with empty strings keep replaying and the validator has to absorb them. + */ +describe("validateWebhookMetadata — empty-string notes are absent fields", () => { + const SUBSCRIPTION_BASE = { + ...BASE, + appointmentType: "SUBSCRIPTION", + schedulingPeriodStartsAt: "2026-09-01T00:00:00.000Z", + schedulingPeriodEndsAt: "2026-12-01T00:00:00.000Z", + }; + + it("validates an in-flight scheduling-period order carrying startsAt/endsAt as empty strings", () => { + const parsed = validateWebhookMetadata({ + ...SUBSCRIPTION_BASE, + startsAt: "", + endsAt: "", + slotOfAvailabilityWeeklyId: "", + notes: "", + }); + + if (parsed.appointmentType !== "SUBSCRIPTION") { + throw new Error("expected subscription metadata"); + } + expect(parsed.startsAt).toBeUndefined(); + expect(parsed.endsAt).toBeUndefined(); + expect(parsed.notes).toBeUndefined(); + expect(parsed.schedulingPeriodStartsAt).toBe( + SUBSCRIPTION_BASE.schedulingPeriodStartsAt, + ); + }); + + it("does not let an empty legacy key shadow a real slot time", () => { + const parsed = validateWebhookMetadata({ + ...BASE, + slotStartTimeInUTC: NEW_KEYS.startsAt, + slotEndTimeInUTC: NEW_KEYS.endsAt, + startsAt: "", + endsAt: "", + }); + + if (parsed.appointmentType !== "CONSULTATION") { + throw new Error("expected consultation metadata"); + } + expect(parsed.startsAt).toBe(NEW_KEYS.startsAt); + expect(parsed.endsAt).toBe(NEW_KEYS.endsAt); + }); +}); diff --git a/docs/payments/checkout-flow/01-overview-and-consultation.md b/docs/payments/checkout-flow/01-overview-and-consultation.md index 645ee8dba..8775090a3 100644 --- a/docs/payments/checkout-flow/01-overview-and-consultation.md +++ b/docs/payments/checkout-flow/01-overview-and-consultation.md @@ -107,6 +107,8 @@ All checkout data is stored in payment intent metadata: > **Webhook backward-compat note:** Razorpay order `notes` objects embedded with the old keys (`startsAt` / `endsAt`) are still accepted by the webhook handler for in-flight orders created before the rename. `normalizeLegacySlotKeys()` in `schemas/webhooks/metadata.ts` maps old → new on ingest; new orders always use `startsAt` / `endsAt`. ``` +> **Empty optional fields are omitted, never sent as `""` (#1462).** `buildPaymentMetadata()` in `lib/payments/operations/checkout.ts` includes an optional key only when it has a value, because the webhook schemas type those fields with `.optional()`, which accepts an absent key and rejects an empty string. A subscription bought for a scheduling period carries no direct slot times, so it used to reach the gateway with `startsAt: ""` and `endsAt: ""`, and every capture webhook for such a sale then failed validation and stamped the payment `REQUIRES_MANUAL_RECOVERY` with the buyer already charged. Because a Razorpay order never expires, orders minted before the fix keep replaying with those empty strings, so `validateWebhookMetadata()` also strips empty-string entries before it normalizes legacy keys and parses. Omitting empty keys has the useful side effect of giving the fifteen-key gateway ceiling more headroom. + **Benefits:** - Webhook can recreate appointment even if frontend crashes @@ -133,27 +135,38 @@ await prisma.$transaction(async (tx) => { // - Webhook will be retried by gateway ``` -### 5. Three-Layer Race Condition Protection +### 5. Slot Occupancy Checks and the Buyer's Own Hold -For consultation and subscription slot bookings: +For consultation and subscription slot bookings, `validateSlotAvailability()` runs two blocking checks. The first rejects the request when any live appointment overlaps the requested window for this consultant, which includes another buyer's tentative hold. The second rejects it when the requesting buyer already holds an overlapping window with a live pending payment. ```typescript -// Layer 1: Check confirmed bookings -if (overlappingConfirmedBooking exists) { +// Check 1: any live overlapping appointment for this consultant +if (overlappingLiveAppointment exists) { throw Error("Time slot is already booked"); } -// Layer 2: Check same user duplicates -if (userHasPendingBookingForThisSlot) { - throw Error("You already have a pending booking"); -} - -// Layer 3: Rate limiting -if (pendingAttempts >= 3) { - throw Error("Time slot temporarily unavailable due to high demand"); +// Check 2: this buyer's own overlapping live hold +if (buyerHasOverlappingPendingHold) { + throw Error("You already have a pending booking for this time slot..."); } ``` +Both checks subtract the buyer's **self-hold** (#1463). A self-hold is an appointment that belongs to the requesting buyer, is for the same plan, has a payment that is still `PENDING` and still inside its expiry window, and covers exactly the window being requested. Such an appointment is not an occupant of the slot; it is the buyer's own open gateway order, and the open-order resume described below is the path that finishes or replaces it. Anything else keeps blocking, including a different buyer's hold on the same slot, a hold on a different plan, and this buyer's own hold on a window that merely overlaps the requested one. When the plan identity cannot be resolved at all, as with webinars and classes whose slot rows are shared between attendees, nothing is excluded. + +Exact coverage is compared against the appointment's whole slot run rather than a single row, because a booked window is stored as a series of contiguous thirty-minute atoms: the run's first start and last end are what must equal the request. + +The consultee-side conflict check inside the checkout lock applies the same exclusion, so the buyer's own hold does not resurface there as "You already have a session booked during this time." + +There is deliberately no per-slot attempt cap. Since check 1 blocks on any live hold, a count of pending attempts for one slot can never exceed one, so the hold itself is the cap. + +### 6. Open-Order Reuse + +A checkout that is remounted, reopened in a new tab, or retried after the buyer dismissed the gateway modal mints a fresh client idempotency key, so the same-key replay cannot recognise it. Before any gateway call, `findReusablePendingOrderPayment()` therefore looks for an open order the buyer can simply finish paying: a payment of theirs that is still `PENDING`, still inside its minted expiry window, on the same gateway, under the same organization, and joined to an appointment for the same plan. + +A candidate is adopted only when it is for the same booking as the current request. A consultation candidate must cover exactly the requested slot window, a subscription candidate must carry exactly the requested scheduling period, and every candidate's frozen amount must equal the total this request computed, so a changed coupon or credit balance can never be charged at a stale price. When a candidate is adopted, checkout returns the existing order id, amount and currency with `reused: true` and creates no second appointment and no second payment. + +Candidates that fail those gates are **superseded** rather than left open. Superseding runs in one transaction: the payment moves `PENDING` → `EXPIRED` through a compare-and-set that carries the old status in its `WHERE` clause, the appointment's tentative slots are cancelled through `transitionSlotCompletion()`, and the parent consultation or subscription is cancelled through its own guarded transition. Releasing the hold is not optional bookkeeping. If the payment were expired while its appointment kept occupying the calendar, the buyer's very next attempt would be rejected by the occupancy check above, which is the wall #1463 describes. Group events are excluded from the release because their slot rows are shared between attendees, so giving back a seat is a disconnect rather than a status move and belongs to the cancel-pending front door. + --- ## Consultation Checkout Flow diff --git a/lib/compliance/dpdp.ts b/lib/compliance/dpdp.ts index 25ec299cf..48c412501 100644 --- a/lib/compliance/dpdp.ts +++ b/lib/compliance/dpdp.ts @@ -85,7 +85,7 @@ import { createHash } from "node:crypto"; import prisma, { type Tx } from "@/lib/prisma"; -import type { PurposeCode } from "./purpose-codes"; +import { purposeCodeAliases, type PurposeCode } from "./purpose-codes"; /** * Thrown when a purpose-scoped action is blocked because the user has not @@ -203,7 +203,11 @@ export async function checkConsent( const artifact = await db.consentArtifact.findFirst({ where: { userId, - purposeCodes: { has: purposeCode }, + // #1472 — `hasSome` over the canonical code AND its legacy aliases. An + // exact `has` made every pre-taxonomy artifact (`session-booking`) + // invisible here, so the fail-closed gate denied a consultant who had in + // fact granted consent. See purposeCodeAliases. + purposeCodes: { hasSome: purposeCodeAliases(purposeCode) }, withdrawnAt: null, auditRetainedUntil: { gt: now }, }, @@ -239,7 +243,9 @@ export async function checkConsentBatch(params: { const artifacts = await prisma.consentArtifact.findMany({ where: { userId: { in: userIds.slice(i, i + CHUNK) }, - purposeCodes: { has: purposeCode }, + // #1472 — same alias set as checkConsent; the batch form must not + // answer differently from the per-user gate it stands in for. + purposeCodes: { hasSome: purposeCodeAliases(purposeCode) }, withdrawnAt: null, auditRetainedUntil: { gt: now }, }, @@ -271,7 +277,10 @@ export async function withdrawConsent(params: { ? { userId, withdrawnAt: null, - purposeCodes: { has: purposeCode }, + // #1472 — a narrow withdrawal has to reach the artifact under whatever + // code it was written with, or the user withdraws and the gate keeps + // reading the legacy row as live consent. + purposeCodes: { hasSome: purposeCodeAliases(purposeCode) }, } : { userId, withdrawnAt: null }; diff --git a/lib/compliance/purpose-codes.ts b/lib/compliance/purpose-codes.ts index 5f19d9b60..f1151141e 100644 --- a/lib/compliance/purpose-codes.ts +++ b/lib/compliance/purpose-codes.ts @@ -102,3 +102,24 @@ export function normalizePurposeCode(code: string): PurposeCode | undefined { ? (code as PurposeCode) : undefined; } + +/** + * Reverse of `normalizePurposeCode`: every string an artifact may legitimately + * be STORED under for a given canonical code — the canonical code itself plus + * each legacy alias that normalises to it. + * + * #1472 — the runtime gates matched the canonical code exactly, so an artifact + * written under the pre-taxonomy kebab-case code (`session-booking`) was + * invisible to them and every booking against that consultant answered 403 + * although `withdrawnAt` was null. A consent record is a legal artifact: the + * gate has to recognise every code the platform ever wrote, so reads go through + * `hasSome: purposeCodeAliases(code)` rather than `has: code`. Writes still + * normalise to the canonical form, and the DB is deliberately NOT backfilled + * (pre-MVP reset). + */ +export function purposeCodeAliases(code: PurposeCode): string[] { + const legacy = Object.keys(LEGACY_PURPOSE_CODE_MAP).filter( + (alias) => LEGACY_PURPOSE_CODE_MAP[alias] === code, + ); + return [code, ...legacy]; +} diff --git a/lib/payments/billing/consumer-invoice.ts b/lib/payments/billing/consumer-invoice.ts index a0cc6e287..02658b91a 100644 --- a/lib/payments/billing/consumer-invoice.ts +++ b/lib/payments/billing/consumer-invoice.ts @@ -23,7 +23,10 @@ import prisma, { type Tx } from "@/lib/prisma"; import { reportSentryError } from "@/lib/observability/report"; -import { recordSystemError } from "@/lib/enterprise/system-events"; +import { + recordSystemError, + recordSystemEvent, +} from "@/lib/enterprise/system-events"; import { numericStateCode } from "@/lib/compliance/state-codes"; import { sumPaise } from "@/lib/payments/utils/money"; import { deriveGstBreakdown } from "@/lib/compliance/gst"; @@ -550,20 +553,41 @@ export async function mintConsumerCreditNote( // Not a silent no-op: the caller believes it reversed money, and an // over-credit attempt means a refund and a chargeback both landed on one // payment. That needs an operator, not a swallowed return. - void recordSystemError({ + // + // It does not need a pager, though. The cumulative cap from #1393 refusing + // a second reversal is the cap WORKING — no money moved, the invoice is + // intact, and the operator's job is reconciliation rather than incident + // response. `recordSystemError` escalates to Sentry at the default error + // level, so this outcome opened an error-level issue every time it fired. + // The durable SystemEvent row is kept exactly as it was; only the Sentry + // report is downgraded to the modelled-refusal shape used elsewhere. + const refusal = new Error( + `ConsumerInvoice ${invoice.id} is credited in full; ${params.amountPaise}p could not be reversed.`, + ); + void recordSystemEvent({ organizationId: null, category: "PAYMENT", - summary: - "Consumer credit note not minted: invoice already fully credited", - err: new Error( - `ConsumerInvoice ${invoice.id} is credited in full; ${params.amountPaise}p could not be reversed.`, - ), + severity: "ERROR", + message: `Consumer credit note not minted: invoice already fully credited: ${refusal.message}`, context: { paymentId: params.paymentId, refundId: params.refundId ?? null, disputeId: params.disputeId ?? null, + errorMessage: refusal.message, }, }).catch(() => {}); + reportSentryError(refusal, { + subsystem: "payments", + op: "mintConsumerCreditNote", + level: "warning", + expected: true, + extra: { + consumerInvoiceId: invoice.id, + paymentId: params.paymentId, + refundId: params.refundId ?? null, + disputeId: params.disputeId ?? null, + }, + }); return { consumerCreditNoteId: null }; } if (derived.outcome === "NOTHING_TO_CREDIT") { diff --git a/lib/payments/operations/checkout.ts b/lib/payments/operations/checkout.ts index 4d70c5a30..31e2e5e9f 100644 --- a/lib/payments/operations/checkout.ts +++ b/lib/payments/operations/checkout.ts @@ -16,8 +16,12 @@ import { } from "@/lib/booking/participants"; import { appendCreationHistory, + transitionConsultationRequest, + transitionSlotCompletion, + transitionSubscriptionRequest, transitionTrialSession, } from "@/lib/booking/transitions"; +import { IllegalTransitionError } from "@/lib/enterprise/transitions"; import { PaymentError } from "@/lib/payments/core/types"; import prisma, { type Tx } from "@/lib/prisma"; import { CheckoutInput, checkoutSchema } from "@/schemas/checkout"; @@ -161,9 +165,19 @@ type SubscriptionCheckoutResult = { * 15 keys, leaving no headroom for the next one, and the discount is already * reflected in the order amount and recorded on the Payment row, while nothing * in the webhook path ever reads it back out of the gateway. + * + * #1462 — an optional field with no value is omitted rather than emitted as an + * empty string, because the webhook schemas type those fields as `.optional()`, + * which admits an absent key but rejects `""`; sending the empty key made every + * scheduling-period subscription fail validation at capture, and it also spent + * key budget the 15-key ceiling above has no room for. */ const GATEWAY_NOTE_MAX_CHARS = 256; +/** Why a superseded open order's booking was cancelled (#1463). */ +const SUPERSEDED_HOLD_NOTE = + "Superseded by a newer checkout attempt for the same booking"; + /** * Build payment metadata for both payment intents and webhook handlers * Ensures consistency between payment creation and mock payment flows. @@ -182,18 +196,31 @@ export function buildPaymentMetadata( fundingSource: "PERSONAL" | "WALLET" | "INVOICE" | "LICENSE" | null; }, ): { appointmentId: string; appointmentType: string; [key: string]: string } { + const notes = (data.notes || "").slice(0, GATEWAY_NOTE_MAX_CHARS); return { appointmentId: "pending", appointmentType: data.appointmentType, userId: userId, planId: data.planId, - startsAt: data.startsAt || "", - endsAt: data.endsAt || "", - slotOfAvailabilityWeeklyId: data.slotOfAvailabilityWeeklyId || "", - slotOfAvailabilityCustomId: data.slotOfAvailabilityCustomId || "", - schedulingPeriodStartsAt: data.schedulingPeriodStartsAt || "", - schedulingPeriodEndsAt: data.schedulingPeriodEndsAt || "", - notes: (data.notes || "").slice(0, GATEWAY_NOTE_MAX_CHARS), + // #1462 — every key below is conditional. A scheduling-period subscription + // carries no direct slots, so `startsAt`/`endsAt` used to reach the gateway + // as `""` and then failed `z.string().datetime().optional()` on the way + // back in, stranding a captured sale as REQUIRES_MANUAL_RECOVERY. + ...(data.startsAt && { startsAt: data.startsAt }), + ...(data.endsAt && { endsAt: data.endsAt }), + ...(data.slotOfAvailabilityWeeklyId && { + slotOfAvailabilityWeeklyId: data.slotOfAvailabilityWeeklyId, + }), + ...(data.slotOfAvailabilityCustomId && { + slotOfAvailabilityCustomId: data.slotOfAvailabilityCustomId, + }), + ...(data.schedulingPeriodStartsAt && { + schedulingPeriodStartsAt: data.schedulingPeriodStartsAt, + }), + ...(data.schedulingPeriodEndsAt && { + schedulingPeriodEndsAt: data.schedulingPeriodEndsAt, + }), + ...(notes && { notes }), ...(data.eventId && { eventId: data.eventId }), ...(orgContext?.organizationId && { organizationId: orgContext.organizationId, @@ -240,13 +267,32 @@ interface ReusableOrder { amount: number; currency: string; isMockPayment: boolean; - /** First slot of the booked window (consultation/class shape); empty for + /** The booked window's slot rows (consultation/class shape); empty for * subscription placeholders whose period lives on the slot rows too. */ appointment?: { slotsOfAppointment: Array<{ startsAt: Date; endsAt: Date }>; } | null; } +/** + * The [start, end) a set of slot rows actually covers. + * + * #1463 — a booked window is stored as N contiguous 30-minute atoms (#1319), so + * the window gate below cannot read the first row's endpoints: for anything + * longer than half an hour the first atom ends 30 minutes into the booking and + * every resume was rejected as a slot-window mismatch. The run's first start and + * last end are the window. + */ +function slotRunWindow( + slots: Array<{ startsAt: Date; endsAt: Date }> | undefined, +): { startsAt: Date; endsAt: Date } | null { + if (!slots || slots.length === 0) return null; + return { + startsAt: new Date(Math.min(...slots.map((s) => s.startsAt.getTime()))), + endsAt: new Date(Math.max(...slots.map((s) => s.endsAt.getTime()))), + }; +} + export async function findReusablePendingOrderPayment( db: Pick, params: { @@ -324,7 +370,9 @@ export async function findReusablePendingOrderPayment( slotsOfAppointment: { select: { startsAt: true, endsAt: true }, orderBy: { startsAt: "asc" as const }, - take: 1, + // #1463 — the whole run, not its first atom. Bounded well above any + // single bookable window so a pathological row cannot widen the read. + take: 48, }, }, }, @@ -342,14 +390,14 @@ export async function findReusablePendingOrderPayment( // Gate 1 — slot window (#1220-triage Critical): a second checkout for a // DIFFERENT appointment time must never resume the first attempt's order. if (params.appointmentType === "CONSULTATION") { - const slot = appt?.slotsOfAppointment?.[0]; - if (!params.slotWindow || !slot) { + const run = slotRunWindow(appt?.slotsOfAppointment); + if (!params.slotWindow || !run) { supersede.push({ id: candidate.id, reason: "window-unmatchable" }); continue; } if ( - slot.startsAt.getTime() !== params.slotWindow.startsAt.getTime() || - slot.endsAt.getTime() !== params.slotWindow.endsAt.getTime() + run.startsAt.getTime() !== params.slotWindow.startsAt.getTime() || + run.endsAt.getTime() !== params.slotWindow.endsAt.getTime() ) { supersede.push({ id: candidate.id, reason: "slot-window-mismatch" }); continue; @@ -359,10 +407,7 @@ export async function findReusablePendingOrderPayment( const reqPeriod = params.schedulingPeriod ?? null; // Subscription windows ride the SAME slot rows as consultations — the // minted placeholder's slot carries the scheduling-period bounds. - const subSlot = appt?.slotsOfAppointment?.[0]; - const rowPeriod = subSlot - ? { startsAt: subSlot.startsAt, endsAt: subSlot.endsAt } - : null; + const rowPeriod = slotRunWindow(appt?.slotsOfAppointment); if (!!reqPeriod !== !!rowPeriod) { supersede.push({ id: candidate.id, reason: "period-mismatch" }); continue; @@ -393,6 +438,122 @@ export async function findReusablePendingOrderPayment( return { reusable: reusable[0] ?? null, supersede }; } +/** + * #1463 — superseding an open order is a RELEASE, not just a status flip. + * + * Expiring the Payment row alone left the superseded attempt's tentative + * appointment and slots occupying the calendar, so the very next attempt for + * the same window hit "Time slot is already booked" again and the buyer was + * walled in until the cleanup sweep ran. The hold has to go back at the same + * moment its payment stops being payable, which is why the payment CAS, the + * slot release and the parent request's cancellation all sit in one + * transaction: a partial release is exactly the state that reopens the wall. + * + * Every write is CAS-in-WHERE per ADR 21 — the payment claim carries + * `paymentStatus: PENDING` so a capture that landed a millisecond earlier wins + * and its booking is left completely alone, and the appointment and slot moves + * go through the guarded helpers in `lib/booking/transitions.ts` rather than a + * bare update. A parent that has already moved on (its own payment succeeded) + * throws `IllegalTransitionError`, which is caught per appointment so the rest + * of the release still commits. + * + * Group events are deliberately untouched: their slots are shared between + * attendees, so releasing a seat is a disconnect rather than a status move and + * belongs to `cancelPendingCheckout`, which owns that shape. No event checkout + * is blocked by a per-buyer hold, so nothing here depends on it. + */ +async function releaseSupersededHolds(params: { + paymentIds: string[]; + userId: string; +}): Promise { + await prisma.$transaction(async (tx: Tx) => { + const claimed = await tx.payment.updateManyAndReturn({ + where: { + id: { in: params.paymentIds }, + userId: params.userId, + paymentStatus: PaymentStatus.PENDING, + }, + data: { paymentStatus: PaymentStatus.EXPIRED, expiresAt: new Date() }, + select: { id: true, appointmentId: true }, + }); + + const appointmentIds = claimed + .map((row) => row.appointmentId) + .filter((id): id is string => id !== null); + if (appointmentIds.length === 0) return; + + const appointments = await tx.appointment.findMany({ + where: { id: { in: appointmentIds }, deletedAt: null }, + select: { + id: true, + webinarId: true, + classId: true, + consultation: { select: { id: true } }, + subscription: { select: { id: true } }, + }, + }); + + for (const appointment of appointments) { + if (appointment.webinarId || appointment.classId) continue; + + // Doctrine rule 2: a slot is freed by status, never by DELETE — the + // buyer keeps the record of the attempt they abandoned. + await transitionSlotCompletion(tx, { + where: { + appointmentId: appointment.id, + isTentative: true, + deletedAt: null, + }, + to: "CANCELLED", + data: { deletedAt: new Date() }, + reason: SUPERSEDED_HOLD_NOTE, + actorUserId: params.userId, + allowZero: true, + }); + + try { + if (appointment.consultation) { + await transitionConsultationRequest(tx, { + where: { id: appointment.consultation.id }, + to: "CANCELLED", + fromIn: ["PENDING", "APPROVED_PENDING_PAYMENT"], + actorUserId: params.userId, + reason: SUPERSEDED_HOLD_NOTE, + data: { + cancellationNotes: SUPERSEDED_HOLD_NOTE, + cancelledAt: new Date(), + }, + }); + } + if (appointment.subscription) { + await transitionSubscriptionRequest(tx, { + where: { id: appointment.subscription.id }, + to: "CANCELLED", + fromIn: ["PENDING", "APPROVED_PENDING_PAYMENT"], + actorUserId: params.userId, + reason: SUPERSEDED_HOLD_NOTE, + data: { + cancellationNotes: SUPERSEDED_HOLD_NOTE, + cancelledAt: new Date(), + }, + }); + } + } catch (error) { + // The parent moved past the payment stage under us, which means some + // other payment already carried it — that booking is not ours to + // cancel. Modelled, so it is reported for visibility only. + if (!(error instanceof IllegalTransitionError)) throw error; + reportSentryError(error, { + subsystem: "payments", + level: "warning", + expected: true, + extra: { appointmentId: appointment.id }, + }); + } + } + }); +} + // ============================================================================ // Payment Intent Manager // ============================================================================ @@ -517,6 +678,13 @@ export async function calculateAmountAndValidate( validatedData: CheckoutInput, userId: string, buyerCountry: string = "IN", + /** + * #1465-triage — the org scope `handleCheckout` already resolved (membership + * verified) before it calls this. Only reaches the slot-availability gate, + * where it scopes the self-hold exclusion to holds this request could + * actually resume. Null default keeps every non-org caller personal. + */ + organizationId: string | null = null, ) { return await prisma.$transaction(async (tx) => { let amount = 0; @@ -599,11 +767,14 @@ export async function calculateAmountAndValidate( assertPlanPurchasable(plan, "This consultation"); + // #1463 — the buyer's User id, not their ConsulteeProfile id: the + // duplicate-hold step compares it to `Payment.userId`. await validateSlotAvailability( tx, validatedData, - user.consulteeProfile.id, + userId, plan.consultantProfile.user.id, // FIX: Pass consultant user ID to filter by consultant + organizationId, ); amount = plan.price; priceCurrency = plan.priceCurrency; @@ -630,11 +801,13 @@ export async function calculateAmountAndValidate( assertPlanPurchasable(plan, "This subscription"); + // #1463 — the buyer's User id; see the consultation arm above. await validateSlotAvailability( tx, validatedData, - user.consulteeProfile.id, + userId, plan.consultantProfile.user.id, // FIX: Pass consultant user ID to filter by consultant + organizationId, ); amount = plan.price; priceCurrency = plan.priceCurrency; @@ -860,20 +1033,183 @@ export async function calculateAmountAndValidate( // Slot Availability Validation // ============================================================================ +/** + * The one definition of "this buyer's hold is still live". + * + * #1463 — step 2 below and the self-hold exclusion must agree exactly on what + * a live hold is, or a hold could be excluded from one and not the other. The + * shape is the one step 2 has always used: still PENDING, and either inside its + * minted expiry window or young enough that the window has not been stamped yet. + * `deletedAt: null` is the single addition, matching what + * `findReusablePendingOrderPayment` will adopt — a soft-deleted payment is not + * a hold anybody can resume. + * + * Liveness only. The self-hold exclusion narrows this further with the resume + * gate's own scope (gateway + org) — see `findSelfHoldAppointmentIds`. Step 2's + * duplicate-attempt guard must NOT carry that scope: it asks "does this buyer + * already hold this window at all", and scoping it would let a second attempt + * on another gateway slip past the guard entirely. + */ +function buildLiveHoldPaymentFilter( + buyerUserId: string, + now: Date, +): Prisma.PaymentWhereInput { + return { + userId: buyerUserId, + paymentStatus: PaymentStatus.PENDING, + deletedAt: null, + OR: [ + { expiresAt: { gt: now } }, + { + AND: [ + { expiresAt: null }, + { createdAt: { gte: new Date(now.getTime() - 5 * 60 * 1000) } }, + ], + }, + ], + }; +} + +/** + * #1463 — the appointments that are this buyer's OWN open order for exactly + * this booking, and therefore are not occupants of the slot they hold. + * + * A buyer who closes the gateway modal and clicks Pay again used to be told + * "Time slot is already booked" by their own hold, which made the documented + * open-order resume (`findReusablePendingOrderPayment`, "Rec C") unreachable: + * the availability gate ran first and threw. Excluding these appointments lets + * the request reach that gate, which then either resumes the same gateway order + * or supersedes it and releases the hold. + * + * The exclusion is deliberately as narrow as the resume gate itself. It takes + * the same buyer, the same plan, the same gateway and the same org scope, a + * payment that is still PENDING and still live, and a window that matches + * EXACTLY — a different buyer, a different plan, or any + * overlapping-but-different window keeps blocking, and a shape whose plan + * identity cannot be resolved (webinars and classes, whose slots are shared + * between attendees) is never excluded at all. + * + * #1465-triage — gateway and org are part of that narrowness, not decoration. + * `findReusablePendingOrderPayment` requires both to match before it will + * resume or supersede a candidate, so a hold minted on a different gateway (or + * under a different org scope) is one this request can neither adopt nor + * expire. Excluding it from availability without those two terms let the same + * buyer mint a SECOND tentative appointment and a second payable order over the + * same window, and both orders could capture. A hold that cannot be resumed + * must keep blocking; the buyer waits out its `expiresAt` instead of + * double-paying. + * + * Exactness is decided in code rather than in the WHERE clause because a booked + * window is stored as N contiguous 30-minute atoms (#1319), so no single row + * carries both endpoints: the run's first start and last end are what must + * equal the request. + */ +export async function findSelfHoldAppointmentIds( + tx: Tx, + params: { + buyerUserId: string; + appointmentType: CheckoutInput["appointmentType"]; + planId: string; + /** The gateway this request will mint on — the resume gate's own scope. */ + paymentGateway: PaymentGateway; + /** Server-resolved org scope; null for personal/marketplace checkouts. */ + organizationId: string | null; + slotStart: Date; + slotEnd: Date; + now: Date; + }, +): Promise { + // A switch rather than a ternary chain: sonar S3358 flags the nested form, + // and the exhaustive shape is what keeps a new appointment type from silently + // inheriting an exclusion it was never reasoned about. + let planScope: Prisma.AppointmentWhereInput | null; + switch (params.appointmentType) { + case "CONSULTATION": + planScope = { consultation: { consultationPlanId: params.planId } }; + break; + case "SUBSCRIPTION": + planScope = { subscription: { subscriptionPlanId: params.planId } }; + break; + default: + planScope = null; + } + if (!planScope) return []; + + const candidates = await tx.appointment.findMany({ + where: { + ...planScope, + deletedAt: null, + payment: { + some: { + ...buildLiveHoldPaymentFilter(params.buyerUserId, params.now), + // The two terms `findReusablePendingOrderPayment` also requires. + // Null-safe org equality: personal stays personal. + paymentGateway: params.paymentGateway, + organizationId: params.organizationId, + }, + }, + // Cheap index-served pre-filter on the run's first atom; the run's full + // extent is checked below. + slotsOfAppointment: { + some: { + startsAt: params.slotStart, + isTentative: true, + deletedAt: null, + }, + }, + }, + select: { + id: true, + slotsOfAppointment: { + where: { deletedAt: null }, + select: { startsAt: true, endsAt: true }, + }, + }, + // Bounded: one buyer can hold one window on one plan; anything beyond a + // handful is a state this exclusion should not be widening for anyway. + take: 5, + }); + + return candidates + .filter((appointment) => { + const slots = appointment.slotsOfAppointment; + if (slots.length === 0) return false; + const runStart = Math.min(...slots.map((s) => s.startsAt.getTime())); + const runEnd = Math.max(...slots.map((s) => s.endsAt.getTime())); + return ( + runStart === params.slotStart.getTime() && + runEnd === params.slotEnd.getTime() + ); + }) + .map((appointment) => appointment.id); +} + /** * Validate slot availability with protection against race conditions * Checks for: * 1. Confirmed overlapping bookings * 2. Duplicate tentative bookings by same user * 3. Excessive tentative bookings (rate limiting) + * + * #1463 — returns the buyer's own self-held appointment ids so the caller's + * own conflict checks can exclude the same rows this function did; re-deriving + * them there would be a second query answering an identical question. */ export async function validateSlotAvailability( tx: Tx, data: CheckoutInput, - userId?: string, + buyerUserId?: string, consultantUserId?: string, // NEW: Filter by consultant to prevent blocking across different consultants -) { - if (!data.startsAt || !data.endsAt) return; + /** + * #1465-triage — the SERVER-resolved org scope for this request, which is + * what `findReusablePendingOrderPayment` matches on. Defaults to null + * (personal) so a caller that cannot resolve it fails closed: an org-scoped + * hold then keeps blocking rather than being excluded from availability by a + * request that could never resume it. + */ + organizationId: string | null = null, +): Promise<{ selfHoldAppointmentIds: string[] }> { + if (!data.startsAt || !data.endsAt) return { selfHoldAppointmentIds: [] }; // LCY-2 consent cascade (#701/#1230) — a consultant who withdrew // SESSION_BOOKING consent must not receive new bookings. Fail-closed: @@ -906,6 +1242,7 @@ export async function validateSlotAvailability( const slotStart = new Date(data.startsAt); const slotEnd = new Date(data.endsAt); + const now = new Date(); // 0. Validate slot is not in the past or too soon (minimum lead time check) const timingError = validateSlotTiming(slotStart); @@ -1008,6 +1345,26 @@ export async function validateSlotAvailability( } } + // #1463 — the buyer's own open order for exactly this booking. Resolved once + // and subtracted from both blocking steps below; see + // findSelfHoldAppointmentIds for why the exclusion is this narrow. + const selfHoldAppointmentIds = buyerUserId + ? await findSelfHoldAppointmentIds(tx, { + buyerUserId, + appointmentType: data.appointmentType, + planId: data.planId, + paymentGateway: data.paymentGateway, + organizationId, + slotStart, + slotEnd, + now, + }) + : []; + const notSelfHeld: Prisma.SlotOfAppointmentWhereInput[] = + selfHoldAppointmentIds.length > 0 + ? [{ NOT: { appointmentId: { in: selfHoldAppointmentIds } } }] + : []; + // 1. Check for confirmed overlapping appointments FOR THIS CONSULTANT ONLY // FIX Bug #05: Use canonical overlap predicate that catches all 4 overlap shapes // (partial start, partial end, full containment, and exact match) @@ -1046,10 +1403,15 @@ export async function validateSlotAvailability( appointment: { AND: [ { OR: buildOccupiedAppointmentFilter() }, - { NOT: buildDeadHoldFilter(new Date()) }, + { NOT: buildDeadHoldFilter(now) }, ], }, }, + // #1463 — the buyer's own live hold on exactly this window and plan is + // their open order, not another occupant, and the Rec C block below + // (findReusablePendingOrderPayment) is the path that resumes or + // supersedes it. Everything else still blocks. + ...notSelfHeld, ], }, }); @@ -1060,7 +1422,12 @@ export async function validateSlotAvailability( // 2. Check for duplicate tentative bookings by the same user FOR THIS CONSULTANT // FIX Bug #05: Use canonical overlap predicate - if (userId) { + // + // #1463 — this step took the caller's ConsulteeProfile id and compared it to + // `Payment.userId`, which is a User id, so it could never match and the step + // never fired. The parameter is the buyer's User id now, which is also the + // identity the self-hold exclusion needs. + if (buyerUserId) { const recentAttempt = await tx.slotOfAppointment.findFirst({ where: { AND: [ @@ -1082,30 +1449,14 @@ export async function validateSlotAvailability( { appointment: { payment: { - some: { - AND: [ - { userId: userId }, - { paymentStatus: "PENDING" }, - { - OR: [ - { expiresAt: { gt: new Date() } }, // Not yet expired - { - AND: [ - { expiresAt: null }, // No expiration set - { - createdAt: { - gte: new Date(Date.now() - 5 * 60 * 1000), - }, - }, // Within 5 min - ], - }, - ], - }, - ], - }, + some: buildLiveHoldPaymentFilter(buyerUserId, now), }, }, }, + // #1463 — same exclusion as step 1: telling the buyer to "complete + // your current payment" while giving them no way to do so is the + // dead end this issue is about. + ...notSelfHeld, ], }, }); @@ -1121,6 +1472,8 @@ export async function validateSlotAvailability( // #1169 PR 2 step 1 blocks on ANY live hold, this count could never reach // one, let alone three. Do not re-add a per-slot attempt cap here; the hold // itself is the cap. + + return { selfHoldAppointmentIds }; } // ============================================================================ @@ -1641,11 +1994,14 @@ async function revalidateInsideLock( }); if (!consultationPlan) throw new Error("Consultation plan not found"); - await validateSlotAvailability( + // #1463 — the buyer's User id, and the self-held appointments it + // resolves are excluded from the consultee-side check below too. + const { selfHoldAppointmentIds } = await validateSlotAvailability( tx, data, - user.consulteeProfile.id, + userId, consultationPlan.consultantProfile.user.id, + orgContext?.organizationId ?? null, ); // Consultee-side conflict check. @@ -1674,6 +2030,14 @@ async function revalidateInsideLock( { OR: buildOccupiedAppointmentFilter() }, // #1319 — parity with step 1 of validateSlotAvailability. { NOT: buildDeadHoldFilter(new Date()) }, + // #1463 — and parity with its self-hold exclusion: the buyer's + // own open order for this exact window is not a competing + // session on their calendar, it is the thing they are trying to + // finish paying for. Without this the availability fix above + // would only move the wall one query to the right. + ...(selfHoldAppointmentIds.length > 0 + ? [{ NOT: { id: { in: selfHoldAppointmentIds } } }] + : []), { slotsOfAppointment: { some: { @@ -1712,11 +2076,13 @@ async function revalidateInsideLock( }); if (!subscriptionPlan) throw new Error("Subscription plan not found"); - await validateSlotAvailability( + // #1463 — the buyer's User id; see the consultation arm above. + const { selfHoldAppointmentIds } = await validateSlotAvailability( tx, data, - user.consulteeProfile.id, + userId, subscriptionPlan.consultantProfile.user.id, + orgContext?.organizationId ?? null, ); // Consultee-side conflict check for direct-slot subscriptions. @@ -1730,6 +2096,10 @@ async function revalidateInsideLock( { OR: buildOccupiedAppointmentFilter() }, // #1319 — parity with step 1 of validateSlotAvailability. { NOT: buildDeadHoldFilter(new Date()) }, + // #1463 — same self-hold exclusion as the consultation arm. + ...(selfHoldAppointmentIds.length > 0 + ? [{ NOT: { id: { in: selfHoldAppointmentIds } } }] + : []), { slotsOfAppointment: { some: { @@ -1877,11 +2247,13 @@ export async function handleConsultationCheckout( // Validate slot availability // FIX: Pass consultant user ID to filter by consultant + // #1463 — the buyer's User id, which is what `Payment.userId` holds. await validateSlotAvailability( tx, data, - consulteeProfileId, + consulteeUserId, consultantUserId, + organizationId, ); // Create consultation @@ -2718,7 +3090,14 @@ export async function handleCheckout( creditsApplied, buyerCountry: detectedBuyerCountry, isInternational, - } = await calculateAmountAndValidate(validatedData, userId, buyerCountry); + } = await calculateAmountAndValidate( + validatedData, + userId, + buyerCountry, + // #1465-triage — resolved and membership-verified above; the slot gate + // needs it to scope the self-hold exclusion to a resumable hold. + organizationId, + ); const displayCurrencyAtCheckout = validatedData.displayCurrency?.toUpperCase() || currency; @@ -2863,12 +3242,12 @@ export async function handleCheckout( : {}), }); if (supersededOrders.length > 0) { - await prisma.payment.updateMany({ - where: { id: { in: supersededOrders.map((s) => s.id) } }, - data: { - paymentStatus: PaymentStatus.EXPIRED, - expiresAt: new Date(), - }, + // #1463 — expiring the payment is only half of it; the hold it minted has + // to come off the calendar in the same transaction or this buyer's next + // attempt walls itself out again. See releaseSupersededHolds. + await releaseSupersededHolds({ + paymentIds: supersededOrders.map((s) => s.id), + userId, }); console.log( JSON.stringify({ diff --git a/schemas/webhooks/metadata.ts b/schemas/webhooks/metadata.ts index 36d0c00d9..b51b8066d 100644 --- a/schemas/webhooks/metadata.ts +++ b/schemas/webhooks/metadata.ts @@ -131,7 +131,21 @@ export function normalizeLegacySlotKeys( * @throws ZodError if validation fails */ export function validateWebhookMetadata(rawMetadata: Record) { - const metadata = normalizeLegacySlotKeys(rawMetadata); + // #1462 — an empty-string note is an ABSENT field, not a present one. The + // optional datetime fields above accept a missing key and reject `""`, so a + // scheduling-period subscription whose order was minted with + // `startsAt: ""` failed validation on every capture and stranded the sale as + // REQUIRES_MANUAL_RECOVERY. The builder no longer emits those keys, but + // gateway notes are persisted external data and a Razorpay order never + // expires, so orders already minted with empty strings keep replaying for as + // long as they are payable; stripping here is what makes those replays land. + // It runs before the legacy-key normalization so an empty legacy key cannot + // shadow a real new-key value either. + const present: Record = {}; + for (const [key, value] of Object.entries(rawMetadata)) { + if (value !== "") present[key] = value; + } + const metadata = normalizeLegacySlotKeys(present); // First parse appointmentType to determine which schema to use const { appointmentType } = baseMetadataSchema.parse(metadata);