From bafe24b778dc5719e092c99884abf2b361268e65 Mon Sep 17 00:00:00 2001 From: Jennie Robinson Faber Date: Mon, 27 Apr 2026 11:13:35 +0100 Subject: [PATCH] chore(tests): replace source-grep tests with handler tests --- .../server/api/event-save-validators.test.js | 176 +++++++++--- .../api/events/payment-deletion.test.js | 31 --- .../api/series-tickets-purchase.test.js | 253 ++++++++++++------ 3 files changed, 299 insertions(+), 161 deletions(-) delete mode 100644 tests/server/api/events/payment-deletion.test.js diff --git a/tests/server/api/event-save-validators.test.js b/tests/server/api/event-save-validators.test.js index 049f694..4645d13 100644 --- a/tests/server/api/event-save-validators.test.js +++ b/tests/server/api/event-save-validators.test.js @@ -1,49 +1,139 @@ -import { describe, it, expect } from 'vitest' -import { readFileSync, existsSync } from 'node:fs' -import { resolve } from 'node:path' +import { describe, it, expect, vi, beforeEach } from 'vitest' -const eventsDir = resolve(import.meta.dirname, '../../../server/api/events/[id]') +import Event from '../../../server/models/event.js' +import Member from '../../../server/models/member.js' +import { waitlistSchema, waitlistDeleteSchema } from '../../../server/utils/schemas.js' +import waitlistPostHandler from '../../../server/api/events/[id]/waitlist.post.js' +import waitlistDeleteHandler from '../../../server/api/events/[id]/waitlist.delete.js' +import { createMockEvent } from '../helpers/createMockEvent.js' -describe('waitlist.post.js bypasses validators on event.save()', () => { - const source = readFileSync(resolve(eventsDir, 'waitlist.post.js'), 'utf-8') +vi.mock('../../../server/models/event.js', () => ({ + default: { findOne: vi.fn() } +})) +vi.mock('../../../server/models/member.js', () => ({ + default: { findOne: vi.fn() } +})) +vi.mock('../../../server/utils/mongoose.js', () => ({ connectDB: vi.fn() })) - it('calls eventData.save with validateBeforeSave: false', () => { - expect(source).toContain('eventData.save({ validateBeforeSave: false })') - }) +vi.stubGlobal('waitlistSchema', waitlistSchema) +vi.stubGlobal('waitlistDeleteSchema', waitlistDeleteSchema) - it('does not contain a bare eventData.save() call', () => { - expect(source).not.toMatch(/eventData\.save\(\s*\)/) - }) -}) +// Override the global validateBody stub so the route actually parses against +// the schema it passed in. +vi.stubGlobal('validateBody', vi.fn(async (event, schema) => { + const body = await readBody(event) + return schema.parse(body) +})) -describe('waitlist.delete.js bypasses validators on event.save()', () => { - const source = readFileSync(resolve(eventsDir, 'waitlist.delete.js'), 'utf-8') - - it('calls eventData.save with validateBeforeSave: false', () => { - expect(source).toContain('eventData.save({ validateBeforeSave: false })') - }) - - it('does not contain a bare eventData.save() call', () => { - expect(source).not.toMatch(/eventData\.save\(\s*\)/) - }) -}) - -// payment.post.js cases are handled by Fix #3 (file deletion). -// If the file still exists, it should also pass the validators bypass. -describe.skipIf(!existsSync(resolve(eventsDir, 'payment.post.js')))( - 'payment.post.js bypasses validators on event.save()', - () => { - const source = existsSync(resolve(eventsDir, 'payment.post.js')) - ? readFileSync(resolve(eventsDir, 'payment.post.js'), 'utf-8') - : '' - - it('has exactly two eventData.save({ validateBeforeSave: false }) calls', () => { - const matches = source.match(/eventData\.save\(\{\s*validateBeforeSave:\s*false\s*\}\)/g) || [] - expect(matches.length).toBe(2) - }) - - it('does not contain a bare eventData.save() call', () => { - expect(source).not.toMatch(/eventData\.save\(\s*\)/) - }) +/** + * Build a mock Event document whose `save()` simulates the legacy validator + * problem we're protecting against: when called WITHOUT `validateBeforeSave: + * false` it throws (mimicking a stale `location` validator failing on + * unrelated writes). When called WITH `validateBeforeSave: false` it resolves + * normally. The route is correct iff it bypasses validators. + */ +function makeMockEvent(overrides = {}) { + const doc = { + _id: 'event-1', + slug: 'event-slug', + tickets: { + waitlist: { + enabled: true, + maxSize: 10, + entries: [], + }, + }, + registrations: [], + ...overrides, } -) + doc.save = vi.fn(async (options) => { + if (!options || options.validateBeforeSave !== false) { + const err = new Error('Validation failed: location: legacy field invalid') + err.name = 'ValidationError' + throw err + } + return doc + }) + return doc +} + +function buildPostEvent(body) { + const ev = createMockEvent({ + method: 'POST', + path: '/api/events/event-slug/waitlist', + body, + }) + ev.context = { params: { id: 'event-slug' } } + return ev +} + +function buildDeleteEvent(body) { + const ev = createMockEvent({ + method: 'DELETE', + path: '/api/events/event-slug/waitlist', + body, + }) + ev.context = { params: { id: 'event-slug' } } + return ev +} + +describe('POST /api/events/[id]/waitlist — bypasses save validators', () => { + beforeEach(() => { + vi.clearAllMocks() + Member.findOne.mockResolvedValue(null) + }) + + it('save() succeeds because the route passes { validateBeforeSave: false }', async () => { + const mockEvent = makeMockEvent() + Event.findOne.mockResolvedValue(mockEvent) + + const result = await waitlistPostHandler(buildPostEvent({ + name: 'Waiter', + email: 'wait@example.com', + })) + + expect(result.success).toBe(true) + expect(mockEvent.save).toHaveBeenCalledTimes(1) + expect(mockEvent.save).toHaveBeenCalledWith({ validateBeforeSave: false }) + // Entry was actually appended. + expect(mockEvent.tickets.waitlist.entries).toHaveLength(1) + expect(mockEvent.tickets.waitlist.entries[0].email).toBe('wait@example.com') + }) +}) + +describe('DELETE /api/events/[id]/waitlist — bypasses save validators', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('save() succeeds because the route passes { validateBeforeSave: false }', async () => { + const mockEvent = makeMockEvent({ + tickets: { + waitlist: { + enabled: true, + maxSize: 10, + entries: [ + { + name: 'Waiter', + email: 'wait@example.com', + membershipLevel: 'non-member', + addedAt: new Date(), + notified: false, + }, + ], + }, + }, + }) + Event.findOne.mockResolvedValue(mockEvent) + + const result = await waitlistDeleteHandler(buildDeleteEvent({ + email: 'wait@example.com', + })) + + expect(result.success).toBe(true) + expect(mockEvent.save).toHaveBeenCalledTimes(1) + expect(mockEvent.save).toHaveBeenCalledWith({ validateBeforeSave: false }) + // Entry was actually removed. + expect(mockEvent.tickets.waitlist.entries).toHaveLength(0) + }) +}) diff --git a/tests/server/api/events/payment-deletion.test.js b/tests/server/api/events/payment-deletion.test.js deleted file mode 100644 index 01e7634..0000000 --- a/tests/server/api/events/payment-deletion.test.js +++ /dev/null @@ -1,31 +0,0 @@ -import { describe, it, expect } from 'vitest' -import { existsSync } from 'node:fs' -import { resolve } from 'node:path' - -/** - * Regression: `events/[id]/payment.post.js` was deleted because its - * unauthenticated POST allowed any caller to spam-register an existing - * member to any paid event by supplying their email. See - * docs/superpowers/specs/2026-04-25-fix-3.md. - * - * With the route file gone, Nitro's filesystem router will not register - * a handler at `/api/events/{id}/payment`, so a POST returns 404 — the - * spam-register attack surface no longer exists at the network layer. - */ -describe('events/[id]/payment route deletion', () => { - it('the payment.post.js route file no longer exists', () => { - const routePath = resolve( - import.meta.dirname, - '../../../../server/api/events/[id]/payment.post.js' - ) - expect(existsSync(routePath)).toBe(false) - }) - - it('the secure replacement at tickets/purchase.post.js still exists', () => { - const replacementPath = resolve( - import.meta.dirname, - '../../../../server/api/events/[id]/tickets/purchase.post.js' - ) - expect(existsSync(replacementPath)).toBe(true) - }) -}) diff --git a/tests/server/api/series-tickets-purchase.test.js b/tests/server/api/series-tickets-purchase.test.js index 16483e1..c8f03d5 100644 --- a/tests/server/api/series-tickets-purchase.test.js +++ b/tests/server/api/series-tickets-purchase.test.js @@ -1,98 +1,177 @@ -import { describe, it, expect } from 'vitest' -import { readFileSync } from 'node:fs' -import { resolve } from 'node:path' +import { describe, it, expect, vi, beforeEach } from 'vitest' -const seriesDir = resolve(import.meta.dirname, '../../../server/api/series/[id]') +import Member from '../../../server/models/member.js' +import Series from '../../../server/models/series.js' +import Event from '../../../server/models/event.js' +import { + validateSeriesTicketPurchase, + completeSeriesTicketPurchase, + registerForAllSeriesEvents, + hasMemberAccess, +} from '../../../server/utils/tickets.js' +import { sendSeriesPassConfirmation } from '../../../server/utils/resend.js' +import { seriesTicketPurchaseSchema } from '../../../server/utils/schemas.js' +import handler from '../../../server/api/series/[id]/tickets/purchase.post.js' +import { createMockEvent } from '../helpers/createMockEvent.js' -describe('series tickets/purchase.post.js — guest account upsert (Fix #8)', () => { - const source = readFileSync(resolve(seriesDir, 'tickets/purchase.post.js'), 'utf-8') +vi.mock('../../../server/models/member.js', () => ({ + default: { findOne: vi.fn(), findOneAndUpdate: vi.fn() } +})) +vi.mock('../../../server/models/series.js', () => ({ + default: { findOne: vi.fn() } +})) +vi.mock('../../../server/models/event.js', () => ({ + default: { + find: vi.fn(() => ({ sort: vi.fn().mockResolvedValue([]) })) + } +})) +vi.mock('../../../server/utils/mongoose.js', () => ({ connectDB: vi.fn() })) +vi.mock('../../../server/utils/tickets.js', () => ({ + validateSeriesTicketPurchase: vi.fn(), + calculateSeriesTicketPrice: vi.fn(), + reserveSeriesTicket: vi.fn(), + releaseSeriesTicket: vi.fn(), + completeSeriesTicketPurchase: vi.fn().mockResolvedValue(undefined), + registerForAllSeriesEvents: vi.fn().mockResolvedValue([]), + hasMemberAccess: vi.fn(() => false), +})) +vi.mock('../../../server/utils/resend.js', () => ({ + sendSeriesPassConfirmation: vi.fn().mockResolvedValue(undefined), +})) - it('uses validateBody with seriesTicketPurchaseSchema', () => { - expect(source).toContain('validateBody(event, seriesTicketPurchaseSchema)') +// Auto-imports the handler relies on but the global setup doesn't stub. +const setAuthCookieMock = vi.fn() +vi.stubGlobal('setAuthCookie', setAuthCookieMock) +vi.stubGlobal('seriesTicketPurchaseSchema', seriesTicketPurchaseSchema) + +// Capture schema passed to validateBody so we can prove the route validates +// against seriesTicketPurchaseSchema specifically. +const validateBodyCalls = [] +const validateBodyMock = vi.fn(async (event, schema) => { + validateBodyCalls.push(schema) + // Mirror real behavior: parse body via the schema so invalid bodies still throw. + const body = await readBody(event) + return schema.parse(body) +}) +vi.stubGlobal('validateBody', validateBodyMock) + +function buildEvent(body) { + const ev = createMockEvent({ + method: 'POST', + path: '/api/series/series-1/tickets/purchase', + body, }) + ev.context = { params: { id: 'series-1' } } + return ev +} - it('Case 1 (free) + Case 2 (paid): upserts a guest Member when unauthenticated buyer provides name+email', () => { - // Mirror event endpoint upsert pattern; ALWAYS-CREATE-GUEST (no opt-in - // checkbox), so guard is `if (!member)` rather than `if (!member && body.createAccount)`. - expect(source).toContain('findOneAndUpdate') - expect(source).toContain('$setOnInsert') - expect(source).toContain('status: "guest"') - expect(source).toContain('upsert: true') - expect(source).toContain('circle: "community"') - expect(source).toContain('contributionAmount: 0') - // ALWAYS-CREATE — must NOT gate on a createAccount flag - expect(source).not.toContain('body.createAccount') - }) - - it('Case 3 (idempotency): upsert pattern handles concurrent same-email registrations atomically', () => { - // findOneAndUpdate with $setOnInsert + upsert:true is the idempotent pattern; - // email has a unique index. No duplicate Member doc created on retry. - expect(source).toMatch(/findOneAndUpdate\(\s*\{\s*email:/) - expect(source).toContain('upsert: true') - expect(source).toContain('new: true') - expect(source).toContain('setDefaultsOnInsert: true') - }) - - it('Case 4 (existing real member): does not auto-login real members entered via public form', () => { - // Auto-login only for newly-created accounts and existing guests. - // Real members (active/pending_payment) get requiresSignIn: true instead. - expect(source).toContain('accountCreated || member.status === "guest"') - expect(source).toContain('requiresSignIn = true') - }) - - it('Case 5 (authenticated guest): sets auth cookie on signedIn:true response', () => { - // setAuthCookie fires for both new accounts and returning guests. - expect(source).toContain('setAuthCookie(event, member)') - expect(source).toContain('signedIn = true') - }) - - it('Case 6 (missing fields): relies on schema validation to reject missing name/email', () => { - // No new validation logic added — existing seriesTicketPurchaseSchema - // already requires name+email; validateBody throws 400 if missing. - expect(source).toContain('validateBody(event, seriesTicketPurchaseSchema)') - }) - - it('includes accountCreated, signedIn, and requiresSignIn in response (parity with event endpoint)', () => { - expect(source).toContain('accountCreated,') - expect(source).toContain('signedIn,') - expect(source).toContain('requiresSignIn,') - }) - - it('still uses hasMemberAccess to gate member pricing (guest/suspended/cancelled treated as non-members)', () => { - expect(source).toContain('hasMemberAccess(member)') - }) - - it('preserves try/catch around requireAuth so unauthenticated callers fall through', () => { - // Required for unauth guest-purchase flow to work at all. - expect(source).toMatch(/try\s*\{[^}]*requireAuth\(event\)[^}]*\}\s*catch/s) - }) - - it('does not block purchase when confirmation email fails', () => { - const emailCallIndex = source.indexOf('await sendSeriesPassConfirmation') - expect(emailCallIndex).toBeGreaterThan(-1) - const afterEmail = source.slice(emailCallIndex) - const catchBlock = afterEmail.match(/catch\s*\(\w+\)\s*\{[^}]*\}/s) - expect(catchBlock).not.toBeNull() - expect(catchBlock[0]).toContain('console.error') - }) +const baseSeries = () => ({ + _id: 'series-1', + id: 'series-1', + slug: 'series-slug', + title: 'Test Series', + description: 'desc', + type: 'workshop', + registrations: [], }) -describe('SeriesPassPurchase.vue — client auth refresh (Fix #8)', () => { - const source = readFileSync( - resolve(import.meta.dirname, '../../../app/components/SeriesPassPurchase.vue'), - 'utf-8' - ) - - it('refreshes client auth state via useAuth().checkMemberStatus() when server reports signedIn', () => { - expect(source).toContain('useAuth().checkMemberStatus()') - expect(source).toMatch(/purchaseResponse\?\.signedIn/) +describe('POST /api/series/[id]/tickets/purchase — guest upsert + auth cookie', () => { + beforeEach(() => { + vi.clearAllMocks() + validateBodyCalls.length = 0 + // Default: unauthenticated buyer + globalThis.requireAuth = vi.fn().mockRejectedValue( + Object.assign(new Error('Unauthorized'), { statusCode: 401 }) + ) + Series.findOne.mockResolvedValue(baseSeries()) + Member.findOne.mockResolvedValue(null) + validateSeriesTicketPurchase.mockReturnValue({ + valid: true, + ticketInfo: { + ticketType: 'public', + price: 0, + currency: 'CAD', + isFree: true, + }, + }) }) - it('shows a one-line guest-account hint under the form (no checkbox)', () => { - // Per ALWAYS-CREATE-GUEST decision: hint only, no UI control. - expect(source).toMatch(/free guest account/i) - // Make sure no checkbox was added by mistake. - expect(source).not.toMatch(/createAccount/) - expect(source).not.toMatch(/]*type="checkbox"/i) + it('upserts a guest Member with $setOnInsert + upsert:true when buyer has no account', async () => { + Member.findOneAndUpdate.mockResolvedValue({ + _id: 'new-member-1', + email: 'guest@example.com', + status: 'guest', + }) + + await handler(buildEvent({ + name: 'Guest Buyer', + email: 'guest@example.com', + ticketType: 'public', + })) + + expect(Member.findOneAndUpdate).toHaveBeenCalledTimes(1) + const [filter, update, options] = Member.findOneAndUpdate.mock.calls[0] + expect(filter).toEqual({ email: 'guest@example.com' }) + expect(update).toEqual({ + $setOnInsert: { + email: 'guest@example.com', + name: 'Guest Buyer', + circle: 'community', + contributionAmount: 0, + status: 'guest', + }, + }) + expect(options).toEqual({ + upsert: true, + new: true, + setDefaultsOnInsert: true, + }) + }) + + it('sets the auth cookie for newly-created guest accounts', async () => { + const newMember = { + _id: 'new-member-2', + email: 'newbie@example.com', + status: 'guest', + } + Member.findOneAndUpdate.mockResolvedValue(newMember) + + const result = await handler(buildEvent({ + name: 'Newbie', + email: 'newbie@example.com', + ticketType: 'public', + })) + + expect(setAuthCookieMock).toHaveBeenCalledTimes(1) + expect(setAuthCookieMock.mock.calls[0][1]).toBe(newMember) + expect(result.signedIn).toBe(true) + expect(result.accountCreated).toBe(true) + expect(result.requiresSignIn).toBe(false) + }) + + it('validates input via seriesTicketPurchaseSchema', async () => { + Member.findOneAndUpdate.mockResolvedValue({ + _id: 'm', + email: 'a@b.com', + status: 'guest', + }) + + await handler(buildEvent({ + name: 'A', + email: 'a@b.com', + ticketType: 'public', + })) + + expect(validateBodyMock).toHaveBeenCalled() + expect(validateBodyCalls[0]).toBe(seriesTicketPurchaseSchema) + }) + + it('rejects invalid body (no name) via schema validation', async () => { + await expect( + handler(buildEvent({ email: 'a@b.com', ticketType: 'public' })) + ).rejects.toBeDefined() + + expect(Member.findOneAndUpdate).not.toHaveBeenCalled() + expect(setAuthCookieMock).not.toHaveBeenCalled() }) })