diff --git a/server/src/nest/common/body-contract-allow-list.ts b/server/src/nest/common/body-contract-allow-list.ts index ad2679ae3..5874c60df 100644 --- a/server/src/nest/common/body-contract-allow-list.ts +++ b/server/src/nest/common/body-contract-allow-list.ts @@ -115,17 +115,6 @@ export const BODY_CONTRACT_ALLOW_LIST: string[] = [ 'NotificationsController.testWebhook', 'OauthApiController.authorize', 'OauthApiController.createClient', - 'PackingController.applyTemplate', - 'PackingController.create', - 'PackingController.createBag', - 'PackingController.importItems', - 'PackingController.reorder', - 'PackingController.saveAsTemplate', - 'PackingController.setBagMembers', - 'PackingController.setSharing', - 'PackingController.update', - 'PackingController.updateBag', - 'PackingController.updateCategoryAssignees', 'PasskeyController.loginVerify', 'PasskeyController.registerOptions', 'PasskeyController.registerVerify', diff --git a/server/src/nest/packing/packing.controller.ts b/server/src/nest/packing/packing.controller.ts index 2e16ba2f1..f6cef116f 100644 --- a/server/src/nest/packing/packing.controller.ts +++ b/server/src/nest/packing/packing.controller.ts @@ -16,6 +16,19 @@ import { PackingService } from './packing.service'; import { isUpdateConflict } from '../../services/conflictResult'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; +import { + PackingApplyTemplateDto, + PackingBagMembersDto, + PackingCategoryAssigneesDto, + PackingCreateBagDto, + PackingCreateItemDto, + PackingImportDto, + PackingReorderDto, + PackingSaveTemplateDto, + PackingSetSharingDto, + PackingUpdateBagDto, + PackingUpdateItemDto, +} from './packing.dto'; /** A packing item row carrying the privacy fields (#858) used to scope broadcasts. */ type PackingItemRow = { is_private?: number; owner_id?: number | null; recipients?: { user_id: number }[]; [key: string]: unknown }; @@ -30,6 +43,12 @@ type PackingItemRow = { is_private?: number; owner_id?: number | null; recipient * creates, 200 elsewhere — note POST /apply-template stays 200); and the bespoke * 400/404 bodies are reproduced. Mutations broadcast over WebSocket with the * forwarded X-Socket-Id. /reorder is declared before /:id so it wins over the param. + * + * Bodies validate via the @trek/shared Zod contracts (packing.dto.ts + the + * global ZodValidationPipe). The pipe's 400 envelope replaced the legacy + * bespoke name checks that the schemas now enforce (missing item name, invalid + * visibility); checks the schemas cannot express (whitespace-only names, empty + * import arrays, the admin template gate) keep their exact legacy strings. */ @Controller('api/trips/:tripId/packing') @UseGuards(JwtAuthGuard) @@ -62,15 +81,16 @@ export class PackingController { importItems( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body('items') items: unknown, + @Body() body: PackingImportDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (!Array.isArray(items) || items.length === 0) { + // The schema guarantees an array; the empty-array rejection stays a bespoke 400. + if (body.items.length === 0) { throw new HttpException({ error: 'items must be a non-empty array' }, 400); } - const created = this.packing.bulkImport(tripId, items, user.id); + const created = this.packing.bulkImport(tripId, body.items, user.id); for (const item of created) { this.packing.broadcastItem(tripId, 'packing:created', { item }, item, socketId); } @@ -81,15 +101,13 @@ export class PackingController { create( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body() body: { name?: string; category?: string; checked?: boolean; is_private?: boolean; visibility?: 'common' | 'personal' | 'shared'; recipient_ids?: number[] }, + @Body() body: PackingCreateItemDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (!body.name) { - throw new HttpException({ error: 'Item name is required' }, 400); - } - const item = this.packing.createItem(tripId, { name: body.name, category: body.category, checked: body.checked, is_private: body.is_private, visibility: body.visibility, recipient_ids: body.recipient_ids }, user.id); + // checked arrives as boolean or legacy 0/1 — the service coerces by truthiness. + const item = this.packing.createItem(tripId, { name: body.name, category: body.category, checked: body.checked === undefined ? undefined : !!body.checked, is_private: body.is_private, visibility: body.visibility, recipient_ids: body.recipient_ids }, user.id); this.emitToViewers(tripId, 'packing:created', { item }, item, socketId); return { item }; } @@ -109,12 +127,12 @@ export class PackingController { reorder( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body('orderedIds') orderedIds: number[], + @Body() body: PackingReorderDto, @Headers('x-socket-id') _socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - this.packing.reorderItems(tripId, orderedIds); + this.packing.reorderItems(tripId, body.orderedIds); return { success: true }; } @@ -123,7 +141,7 @@ export class PackingController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, - @Body() body: Record, + @Body() body: PackingUpdateItemDto, @Headers('x-socket-id') socketId?: string, @Headers('x-base-updated-at') ifMatch?: string, ) { @@ -132,8 +150,11 @@ export class PackingController { // Privacy state before the change, so a public↔private toggle (#858) can route // the broadcast correctly instead of leaking a freshly-privatized item. const before = this.packing.getItemPrivacy(tripId, id); - const { name, checked, category, weight_grams, bag_id, quantity, is_private } = body as Record; - const updated = this.packing.updateItem(tripId, id, { name, checked, category, weight_grams, bag_id, quantity, is_private }, Object.keys(body), ifMatch, user.id); + const { name, checked, category, weight_grams, bag_id, quantity, is_private } = body; + // bodyKeys carries which keys the request actually provided (the presence- + // sentinel protocol); the parsed body only ever holds known schema keys. + // checked arrives as boolean or legacy 0/1 — normalize to the 0/1 the SQL binds. + const updated = this.packing.updateItem(tripId, id, { name, checked: checked === undefined ? undefined : checked ? 1 : 0, category, weight_grams, bag_id, quantity, is_private }, Object.keys(body), ifMatch, user.id); if (!updated) { throw new HttpException({ error: 'Item not found' }, 404); } @@ -199,14 +220,11 @@ export class PackingController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, - @Body() body: { visibility?: 'common' | 'personal' | 'shared'; recipient_ids?: number[] }, + @Body() body: PackingSetSharingDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (body.visibility !== 'common' && body.visibility !== 'personal' && body.visibility !== 'shared') { - throw new HttpException({ error: 'Invalid visibility' }, 400); - } const updated = this.packing.setItemSharing(tripId, id, user.id, body.visibility, Array.isArray(body.recipient_ids) ? body.recipient_ids : []); if (!updated) { throw new HttpException({ error: 'Item not found' }, 404); @@ -288,12 +306,13 @@ export class PackingController { createBag( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body() body: { name?: string; color?: string }, + @Body() body: PackingCreateBagDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (!body.name?.trim()) { + // The schema requires a non-empty name; whitespace-only still 400s here. + if (!body.name.trim()) { throw new HttpException({ error: 'Name is required' }, 400); } const bag = this.packing.createBag(tripId, { name: body.name, color: body.color }); @@ -306,12 +325,14 @@ export class PackingController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('bagId') bagId: string, - @Body() body: Record, + @Body() body: PackingUpdateBagDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - const { name, color, weight_limit_grams, user_id } = body as Record; + const { name, color, weight_limit_grams, user_id } = body; + // bodyKeys carries which keys the request actually provided (the presence- + // sentinel protocol); the parsed body only ever holds known schema keys. const updated = this.packing.updateBag(tripId, bagId, { name, color, weight_limit_grams, user_id }, Object.keys(body)); if (!updated) { throw new HttpException({ error: 'Bag not found' }, 404); @@ -348,7 +369,7 @@ export class PackingController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('templateId') templateId: string, - @Body() body: { visibility?: 'common' | 'personal' }, + @Body() body: PackingApplyTemplateDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); @@ -367,12 +388,12 @@ export class PackingController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('bagId') bagId: string, - @Body('user_ids') userIds: unknown, + @Body() body: PackingBagMembersDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - const members = this.packing.setBagMembers(tripId, bagId, Array.isArray(userIds) ? userIds : []); + const members = this.packing.setBagMembers(tripId, bagId, body.user_ids); if (!members) { throw new HttpException({ error: 'Bag not found' }, 404); } @@ -384,16 +405,17 @@ export class PackingController { saveAsTemplate( @CurrentUser() user: User, @Param('tripId') tripId: string, - @Body('name') name?: string, + @Body() body: PackingSaveTemplateDto, ) { this.requireTrip(tripId, user); if (user.role !== 'admin') { throw new HttpException({ error: 'Admin access required' }, 403); } - if (!name?.trim()) { + // The schema requires a non-empty name; whitespace-only still 400s here. + if (!body.name.trim()) { throw new HttpException({ error: 'Template name is required' }, 400); } - const template = this.packing.saveAsTemplate(tripId, user.id, name.trim()); + const template = this.packing.saveAsTemplate(tripId, user.id, body.name.trim()); if (!template) { throw new HttpException({ error: 'No items to save' }, 400); } @@ -411,15 +433,15 @@ export class PackingController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('categoryName') categoryName: string, - @Body('user_ids') userIds: number[], + @Body() body: PackingCategoryAssigneesDto, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); const category = decodeURIComponent(categoryName); - const rows = this.packing.updateCategoryAssignees(tripId, category, userIds); + const rows = this.packing.updateCategoryAssignees(tripId, category, body.user_ids); this.packing.broadcast(tripId, 'packing:assignees', { category, assignees: rows }, socketId); - this.packing.notifyTagged(tripId, user, category, userIds); + this.packing.notifyTagged(tripId, user, category, body.user_ids); return { assignees: rows }; } } diff --git a/server/src/nest/packing/packing.dto.ts b/server/src/nest/packing/packing.dto.ts new file mode 100644 index 000000000..1757451a2 --- /dev/null +++ b/server/src/nest/packing/packing.dto.ts @@ -0,0 +1,32 @@ +import { createZodDto } from 'nestjs-zod'; +import { + packingCreateItemRequestSchema, + packingUpdateItemRequestSchema, + packingSetSharingRequestSchema, + packingImportRequestSchema, + packingReorderRequestSchema, + packingCreateBagRequestSchema, + packingUpdateBagRequestSchema, + packingBagMembersRequestSchema, + packingSaveTemplateRequestSchema, + packingApplyTemplateRequestSchema, + packingCategoryAssigneesRequestSchema, +} from '@trek/shared'; + +/** + * Server-side createZodDto wrappers over the @trek/shared packing contracts. + * The global ZodValidationPipe (APP_PIPE in app.module.ts) validates any + * @Body() parameter typed with one of these classes by metatype — the Zod + * schemas in shared/ remain the single source of truth for the wire contract. + */ +export class PackingCreateItemDto extends createZodDto(packingCreateItemRequestSchema) {} +export class PackingUpdateItemDto extends createZodDto(packingUpdateItemRequestSchema) {} +export class PackingSetSharingDto extends createZodDto(packingSetSharingRequestSchema) {} +export class PackingImportDto extends createZodDto(packingImportRequestSchema) {} +export class PackingReorderDto extends createZodDto(packingReorderRequestSchema) {} +export class PackingCreateBagDto extends createZodDto(packingCreateBagRequestSchema) {} +export class PackingUpdateBagDto extends createZodDto(packingUpdateBagRequestSchema) {} +export class PackingBagMembersDto extends createZodDto(packingBagMembersRequestSchema) {} +export class PackingSaveTemplateDto extends createZodDto(packingSaveTemplateRequestSchema) {} +export class PackingApplyTemplateDto extends createZodDto(packingApplyTemplateRequestSchema) {} +export class PackingCategoryAssigneesDto extends createZodDto(packingCategoryAssigneesRequestSchema) {} diff --git a/server/tests/e2e/packing.e2e.test.ts b/server/tests/e2e/packing.e2e.test.ts index 909bcaf96..686cb2488 100644 --- a/server/tests/e2e/packing.e2e.test.ts +++ b/server/tests/e2e/packing.e2e.test.ts @@ -118,6 +118,7 @@ vi.mock('../../src/services/permissions', () => ({ checkPermission })); import { PackingModule } from '../../src/nest/packing/packing.module'; import { DatabaseModule } from '../../src/nest/database/database.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; +import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe'; function insertItem(tripId: number, name: string, extra: Partial<{ sort_order: number; category: string; is_private: number; owner_id: number }> = {}): number { const res = db @@ -136,6 +137,9 @@ describe('Packing e2e (real auth guard + real SQL over temp SQLite)', () => { const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalFilters(new TrekExceptionFilter()); + // Mirror the production APP_PIPE (app.module.ts): the DTO-typed bodies + // validate by metatype, exactly as they do under buildApp(). + nest.useGlobalPipes(new ZodValidationPipe()); await nest.init(); return nest; } @@ -275,6 +279,38 @@ describe('Packing e2e (real auth guard + real SQL over temp SQLite)', () => { expect(res.body).toEqual({ error: 'items must be a non-empty array' }); }); + it('400 from the Zod pipe on create without a name', async () => { + const res = await request(server).post(`/api/trips/${tripId}/packing`).set('Cookie', sessionCookie(1)).send({}); + expect(res.status).toBe(400); + expect(res.body.error).toContain('name'); + }); + + it('400 from the Zod pipe on reorder without orderedIds', async () => { + const res = await request(server).put(`/api/trips/${tripId}/packing/reorder`).set('Cookie', sessionCookie(1)).send({}); + expect(res.status).toBe(400); + expect(res.body.error).toContain('orderedIds'); + }); + + it('400 from the Zod pipe on sharing with an invalid visibility', async () => { + const id = insertItem(tripId, 'Tent'); + const res = await request(server).put(`/api/trips/${tripId}/packing/${id}/sharing`).set('Cookie', sessionCookie(1)).send({ visibility: 'secret' }); + expect(res.status).toBe(400); + expect(res.body.error).toContain('visibility'); + }); + + it('accepts the legacy numeric checked form through the pipe', async () => { + const id = insertItem(tripId, 'Toggle me'); + const res = await request(server).put(`/api/trips/${tripId}/packing/${id}`).set('Cookie', sessionCookie(1)).send({ checked: 1 }); + expect(res.status).toBe(200); + expect(res.body.item.checked).toBe(1); + }); + + it('whitespace-only bag name still gets the bespoke 400', async () => { + const res = await request(server).post(`/api/trips/${tripId}/packing/bags`).set('Cookie', sessionCookie(1)).send({ name: ' ' }); + expect(res.status).toBe(400); + expect(res.body).toEqual({ error: 'Name is required' }); + }); + it('bags round-trip: 201 create (default color), 200 update (COALESCE), 200 delete, then 404', async () => { const created = await request(server).post(`/api/trips/${tripId}/packing/bags`).set('Cookie', sessionCookie(1)).send({ name: 'Duffel' }); expect(created.status).toBe(201); diff --git a/server/tests/unit/nest/packing.controller.test.ts b/server/tests/unit/nest/packing.controller.test.ts index 6877c3486..0b4f32716 100644 --- a/server/tests/unit/nest/packing.controller.test.ts +++ b/server/tests/unit/nest/packing.controller.test.ts @@ -57,12 +57,10 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r }); }); - it('400 when name missing', () => { - const svc = makeService(); - expect(thrown(() => new PackingController(svc).create(user, '5', {}))).toEqual({ - status: 400, body: { error: 'Item name is required' }, - }); - }); + // The missing-name 400 moved from a bespoke controller check into the + // ZodValidationPipe (packingCreateItemRequestSchema) — direct method calls + // bypass parameter pipes, so that path is covered by the e2e suite and + // the schema spec in @trek/shared. it('creates an item (owned by the creator) and broadcasts a Common item to the room', () => { // Common item (is_private falsy) → viewersOf null → whole-room broadcast. @@ -93,23 +91,20 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r }); describe('POST /import', () => { - it('400 when items is not a non-empty array (empty array)', () => { + it('400 when items is an empty array (bespoke check, schema permits [])', () => { const svc = makeService(); - expect(thrown(() => new PackingController(svc).importItems(user, '5', []))).toEqual({ + expect(thrown(() => new PackingController(svc).importItems(user, '5', { items: [] }))).toEqual({ status: 400, body: { error: 'items must be a non-empty array' }, }); }); - it('400 when items is not an array at all (non-array branch)', () => { - const svc = makeService(); - expect(thrown(() => new PackingController(svc).importItems(user, '5', 'nope'))).toEqual({ - status: 400, body: { error: 'items must be a non-empty array' }, - }); - }); + // The non-array 400 moved into the ZodValidationPipe (packingImportRequestSchema + // requires an array) — direct method calls bypass parameter pipes, so that + // path is covered by the e2e suite and the schema spec in @trek/shared. it('403 without packing_edit permission', () => { const svc = makeService({ canEdit: vi.fn().mockReturnValue(false) }); - expect(thrown(() => new PackingController(svc).importItems(user, '5', [{ name: 'a' }]))).toEqual({ + expect(thrown(() => new PackingController(svc).importItems(user, '5', { items: [{ name: 'a' }] }))).toEqual({ status: 403, body: { error: 'No permission' }, }); }); @@ -118,7 +113,7 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r const bulkImport = vi.fn().mockReturnValue([{ id: 1 }, { id: 2 }]); const broadcastItem = vi.fn(); const svc = makeService({ bulkImport, broadcastItem } as Partial); - const res = new PackingController(svc).importItems(user, '5', [{ name: 'a' }, { name: 'b' }], 'sock'); + const res = new PackingController(svc).importItems(user, '5', { items: [{ name: 'a' }, { name: 'b' }] }, 'sock'); expect(res).toEqual({ items: [{ id: 1 }, { id: 2 }], count: 2 }); expect(bulkImport).toHaveBeenCalledWith('5', [{ name: 'a' }, { name: 'b' }], user.id); expect(broadcastItem).toHaveBeenCalledTimes(2); @@ -138,8 +133,9 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r const broadcast = vi.fn(); const svc = makeService({ updateItem, broadcast } as Partial); new PackingController(svc).update(user, '5', '9', { name: 'X', checked: true }, 'sock'); - // acting user id is forwarded so privatizing an unowned item can stamp the owner (#858) - expect(updateItem).toHaveBeenCalledWith('5', '9', expect.objectContaining({ name: 'X', checked: true }), ['name', 'checked'], undefined, user.id); + // acting user id is forwarded so privatizing an unowned item can stamp the + // owner (#858); checked is normalized to the 0/1 the SQL binds. + expect(updateItem).toHaveBeenCalledWith('5', '9', expect.objectContaining({ name: 'X', checked: 1 }), ['name', 'checked'], undefined, user.id); // A public item (is_private undefined, was public) broadcasts to the whole room. expect(broadcast).toHaveBeenCalledWith('5', 'packing:updated', { item: { id: 9, name: 'X' } }, 'sock'); }); @@ -196,13 +192,13 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r it('reorders the items and reports success', () => { const reorderItems = vi.fn(); const svc = makeService({ reorderItems } as Partial); - expect(new PackingController(svc).reorder(user, '5', [3, 1, 2])).toEqual({ success: true }); + expect(new PackingController(svc).reorder(user, '5', { orderedIds: [3, 1, 2] })).toEqual({ success: true }); expect(reorderItems).toHaveBeenCalledWith('5', [3, 1, 2]); }); it('403 without packing_edit permission', () => { const svc = makeService({ canEdit: vi.fn().mockReturnValue(false) }); - expect(thrown(() => new PackingController(svc).reorder(user, '5', [1]))).toEqual({ + expect(thrown(() => new PackingController(svc).reorder(user, '5', { orderedIds: [1] }))).toEqual({ status: 403, body: { error: 'No permission' }, }); }); @@ -236,8 +232,11 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r }); describe('sharing, contributors, clone (#858 three-tier)', () => { - it('PUT /:id/sharing 400 invalid, 404 missing, 403 non-owner, else drops + re-emits', () => { - expect(thrown(() => new PackingController(makeService()).setSharing(user, '5', '9', { visibility: 'nope' as never }))).toEqual({ status: 400, body: { error: 'Invalid visibility' } }); + // The invalid-visibility 400 moved into the ZodValidationPipe + // (packingSetSharingRequestSchema requires the enum) — direct method calls + // bypass parameter pipes, so that path is covered by the e2e suite and the + // schema spec in @trek/shared. + it('PUT /:id/sharing 404 missing, 403 non-owner, else drops + re-emits', () => { expect(thrown(() => new PackingController(makeService({ setItemSharing: vi.fn().mockReturnValue(null) } as Partial)).setSharing(user, '5', '9', { visibility: 'personal' }))).toEqual({ status: 404, body: { error: 'Item not found' } }); expect(thrown(() => new PackingController(makeService({ setItemSharing: vi.fn().mockReturnValue({ forbidden: true }) } as Partial)).setSharing(user, '5', '9', { visibility: 'personal' }))).toEqual({ status: 403, body: { error: 'Only the owner can change sharing' } }); @@ -292,19 +291,16 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r expect(new PackingController(svc).listBags(user, '5')).toEqual({ bags: [{ id: 3, name: 'Carry-on' }] }); }); - it('400 on bag create with blank name', () => { + it('400 on bag create with blank name (bespoke check — the schema cannot see whitespace)', () => { const svc = makeService(); expect(thrown(() => new PackingController(svc).createBag(user, '5', { name: ' ' }))).toEqual({ status: 400, body: { error: 'Name is required' }, }); }); - it('400 on bag create with no name at all (optional-chain short-circuit)', () => { - const svc = makeService(); - expect(thrown(() => new PackingController(svc).createBag(user, '5', {}))).toEqual({ - status: 400, body: { error: 'Name is required' }, - }); - }); + // The missing-name 400 moved into the ZodValidationPipe + // (packingCreateBagRequestSchema requires a non-empty name) — direct method + // calls bypass parameter pipes, so that path is covered by the e2e suite. it('creates a bag and broadcasts', () => { const createBag = vi.fn().mockReturnValue({ id: 3, name: 'Carry-on' }); @@ -349,27 +345,24 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r it('404 on set-members when the bag is missing', () => { const svc = makeService({ setBagMembers: vi.fn().mockReturnValue(null) } as Partial); - expect(thrown(() => new PackingController(svc).setBagMembers(user, '5', '3', [1, 2]))).toEqual({ + expect(thrown(() => new PackingController(svc).setBagMembers(user, '5', '3', { user_ids: [1, 2] }))).toEqual({ status: 404, body: { error: 'Bag not found' }, }); }); - it('sets bag members and broadcasts (array branch)', () => { + it('sets bag members and broadcasts', () => { const setBagMembers = vi.fn().mockReturnValue([{ user_id: 1 }, { user_id: 2 }]); const broadcast = vi.fn(); const svc = makeService({ setBagMembers, broadcast } as Partial); - const res = new PackingController(svc).setBagMembers(user, '5', '3', [1, 2], 'sock'); + const res = new PackingController(svc).setBagMembers(user, '5', '3', { user_ids: [1, 2] }, 'sock'); expect(res).toEqual({ members: [{ user_id: 1 }, { user_id: 2 }] }); expect(setBagMembers).toHaveBeenCalledWith('5', '3', [1, 2]); expect(broadcast).toHaveBeenCalledWith('5', 'packing:bag-members-updated', { bagId: 3, members: [{ user_id: 1 }, { user_id: 2 }] }, 'sock'); }); - it('coerces non-array members to an empty list (ternary else branch)', () => { - const setBagMembers = vi.fn().mockReturnValue([]); - const svc = makeService({ setBagMembers } as Partial); - new PackingController(svc).setBagMembers(user, '5', '3', 'not-an-array'); - expect(setBagMembers).toHaveBeenCalledWith('5', '3', []); - }); + // The non-array coercion is gone: packingBagMembersRequestSchema requires + // user_ids to be an array, so the pipe 400s a malformed body before the + // handler runs (covered by the schema spec in @trek/shared). }); describe('templates', () => { @@ -418,28 +411,24 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r expect(applyTemplate).toHaveBeenCalledWith('5', 't1', 'common', user.id); }); - it('400 when an admin saves a template with no name (whitespace)', () => { + it('400 when an admin saves a template with no name (whitespace — the schema cannot see it)', () => { const saveAsTemplate = vi.fn(); const svc = makeService({ saveAsTemplate } as Partial); - expect(thrown(() => new PackingController(svc).saveAsTemplate(admin, '5', ' '))).toEqual({ + expect(thrown(() => new PackingController(svc).saveAsTemplate(admin, '5', { name: ' ' }))).toEqual({ status: 400, body: { error: 'Template name is required' }, }); expect(saveAsTemplate).not.toHaveBeenCalled(); }); - it('400 when an admin saves a template with no name at all (optional-chain)', () => { - const saveAsTemplate = vi.fn(); - const svc = makeService({ saveAsTemplate } as Partial); - expect(thrown(() => new PackingController(svc).saveAsTemplate(admin, '5'))).toEqual({ - status: 400, body: { error: 'Template name is required' }, - }); - expect(saveAsTemplate).not.toHaveBeenCalled(); - }); + // The missing-name 400 moved into the ZodValidationPipe + // (packingSaveTemplateRequestSchema requires a non-empty name) — direct + // method calls bypass parameter pipes, so that path is covered by the e2e + // suite and the schema spec in @trek/shared. it('403 when a non-admin tries to save a template', () => { const saveAsTemplate = vi.fn(); const svc = makeService({ saveAsTemplate } as Partial); - expect(thrown(() => new PackingController(svc).saveAsTemplate(user, '5', 'My template'))).toEqual({ + expect(thrown(() => new PackingController(svc).saveAsTemplate(user, '5', { name: 'My template' }))).toEqual({ status: 403, body: { error: 'Admin access required' }, }); expect(saveAsTemplate).not.toHaveBeenCalled(); @@ -447,7 +436,7 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r it('400 when an admin saves a template with no items', () => { const svc = makeService({ saveAsTemplate: vi.fn().mockReturnValue(null) } as Partial); - expect(thrown(() => new PackingController(svc).saveAsTemplate(admin, '5', 'My template'))).toEqual({ + expect(thrown(() => new PackingController(svc).saveAsTemplate(admin, '5', { name: 'My template' }))).toEqual({ status: 400, body: { error: 'No items to save' }, }); }); @@ -455,7 +444,7 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r it('saves a template for an admin', () => { const saveAsTemplate = vi.fn().mockReturnValue({ id: 7, name: 'My template' }); const svc = makeService({ saveAsTemplate } as Partial); - expect(new PackingController(svc).saveAsTemplate(admin, '5', 'My template')).toEqual({ + expect(new PackingController(svc).saveAsTemplate(admin, '5', { name: 'My template' })).toEqual({ template: { id: 7, name: 'My template' }, }); expect(saveAsTemplate).toHaveBeenCalledWith('5', admin.id, 'My template'); @@ -477,7 +466,7 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r const broadcast = vi.fn(); const notifyTagged = vi.fn(); const svc = makeService({ updateCategoryAssignees, broadcast, notifyTagged } as Partial); - new PackingController(svc).updateCategoryAssignees(user, '5', 'Toys%20%26%20Games', [2]); + new PackingController(svc).updateCategoryAssignees(user, '5', 'Toys%20%26%20Games', { user_ids: [2] }); expect(updateCategoryAssignees).toHaveBeenCalledWith('5', 'Toys & Games', [2]); }); @@ -486,7 +475,7 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r const broadcast = vi.fn(); const notifyTagged = vi.fn(); const svc = makeService({ updateCategoryAssignees, broadcast, notifyTagged } as Partial); - const res = new PackingController(svc).updateCategoryAssignees(user, '5', 'Clothes', [2], 'sock'); + const res = new PackingController(svc).updateCategoryAssignees(user, '5', 'Clothes', { user_ids: [2] }, 'sock'); expect(res).toEqual({ assignees: [{ user_id: 2 }] }); expect(broadcast).toHaveBeenCalledWith('5', 'packing:assignees', { category: 'Clothes', assignees: [{ user_id: 2 }] }, 'sock'); expect(notifyTagged).toHaveBeenCalledWith('5', user, 'Clothes', [2]); diff --git a/shared/src/packing/packing.schema.ts b/shared/src/packing/packing.schema.ts index 25656fbf4..919fe97da 100644 --- a/shared/src/packing/packing.schema.ts +++ b/shared/src/packing/packing.schema.ts @@ -84,7 +84,8 @@ export type PackingVisibility = z.infer; export const packingCreateItemRequestSchema = z.object({ name: z.string().min(1), category: z.string().optional(), - checked: z.boolean().optional(), + // The legacy route accepted both boolean and 0/1 for checked — both stay valid. + checked: z.union([z.boolean(), z.number().int().min(0).max(1)]).optional(), // Mark the new item private to its creator (#858, legacy flag). is_private: z.boolean().optional(), // Three-tier sharing (#858): which list the item belongs to, and — for 'shared' — @@ -103,7 +104,8 @@ export type PackingSetSharingRequest = z.infer; -export const packingApplyTemplateRequestSchema = z.object({ - visibility: z.enum(['common', 'personal']).optional(), -}); +// The whole body is optional: the legacy route accepted a body-less POST +// (visibility then defaults to 'common' in the controller). +export const packingApplyTemplateRequestSchema = z + .object({ + visibility: z.enum(['common', 'personal']).optional(), + }) + .optional(); export type PackingApplyTemplateRequest = z.infer; export const packingTemplateSummarySchema = z.object({