From 49197c5370d16678a0b2c8f9856430a09f67f919 Mon Sep 17 00:00:00 2001 From: jubnl Date: Mon, 27 Jul 2026 00:27:11 +0200 Subject: [PATCH] feat(server): adopt zod DTOs for the collab body contracts --- server/src/nest/collab/collab.controller.ts | 51 ++++++++++--------- server/src/nest/collab/collab.dto.ts | 21 ++++++++ .../nest/common/body-contract-allow-list.ts | 6 --- server/tests/integration/collab.test.ts | 4 +- .../tests/unit/nest/collab.controller.test.ts | 25 ++++----- 5 files changed, 60 insertions(+), 47 deletions(-) create mode 100644 server/src/nest/collab/collab.dto.ts diff --git a/server/src/nest/collab/collab.controller.ts b/server/src/nest/collab/collab.controller.ts index 775e1c489..9ce625fe7 100644 --- a/server/src/nest/collab/collab.controller.ts +++ b/server/src/nest/collab/collab.controller.ts @@ -21,6 +21,14 @@ import fs from 'fs'; import { v4 as uuidv4 } from 'uuid'; import type { User } from '../../types'; import { CollabService } from './collab.service'; +import { + CollabNoteCreateDto, + CollabNoteUpdateDto, + CollabPollCreateDto, + CollabPollVoteDto, + CollabMessageCreateDto, + CollabReactionDto, +} from './collab.dto'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; import { BLOCKED_EXTENSIONS } from '../files/files.constants'; @@ -53,7 +61,12 @@ const NOTE_UPLOAD = { * access (404), 'collab_edit' (403) on mutations + 'file_upload' on note files, * create 201 / rest 200 (vote + react POST stay 200), the bespoke 400/403/404 * bodies, the chat/note notifications, and all WebSocket broadcasts with the - * forwarded X-Socket-Id. + * forwarded X-Socket-Id. Bodies validate against the @trek/shared collab + * schemas via the DTO classes in collab.dto.ts + the global ZodValidationPipe + * (400 with the standard `{ error }` envelope on mismatch — this replaced the + * legacy bespoke 'Title is required' / 'Question is required' / '... 2 options + * ...' / 5000-char / 'Emoji is required' checks; the whitespace-only 'Message + * text is required' check stays, since min(1) doesn't trim). */ @Controller('api/trips/:tripId/collab') @UseGuards(JwtAuthGuard) @@ -82,12 +95,9 @@ export class CollabController { } @Post('notes') - createNote(@CurrentUser() user: User, @Param('tripId') tripId: string, @Body() body: { title?: string; content?: string; category?: string; color?: string; website?: string }, @Headers('x-socket-id') socketId?: string) { + createNote(@CurrentUser() user: User, @Param('tripId') tripId: string, @Body() body: CollabNoteCreateDto, @Headers('x-socket-id') socketId?: string) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (!body.title) { - throw new HttpException({ error: 'Title is required' }, 400); - } const note = this.collab.createNote(tripId, user.id, { title: body.title, content: body.content, @@ -101,7 +111,7 @@ export class CollabController { } @Put('notes/:id') - updateNote(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Body() body: { title?: string; content?: string; category?: string; color?: string; pinned?: number | boolean; website?: string }, @Headers('x-socket-id') socketId?: string) { + updateNote(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Body() body: CollabNoteUpdateDto, @Headers('x-socket-id') socketId?: string) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); const note = this.collab.updateNote(tripId, id, { @@ -167,15 +177,9 @@ export class CollabController { } @Post('polls') - createPoll(@CurrentUser() user: User, @Param('tripId') tripId: string, @Body() body: { question?: string; options?: unknown[]; multiple?: boolean; multiple_choice?: boolean; deadline?: string }, @Headers('x-socket-id') socketId?: string) { + createPoll(@CurrentUser() user: User, @Param('tripId') tripId: string, @Body() body: CollabPollCreateDto, @Headers('x-socket-id') socketId?: string) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (!body.question) { - throw new HttpException({ error: 'Question is required' }, 400); - } - if (!Array.isArray(body.options) || body.options.length < 2) { - throw new HttpException({ error: 'At least 2 options are required' }, 400); - } const poll = this.collab.createPoll(tripId, user.id, { question: body.question, options: body.options, @@ -189,10 +193,10 @@ export class CollabController { @Post('polls/:id/vote') @HttpCode(200) - votePoll(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Body('option_index') optionIndex: number, @Headers('x-socket-id') socketId?: string) { + votePoll(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Body() body: CollabPollVoteDto, @Headers('x-socket-id') socketId?: string) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - const result = this.collab.votePoll(tripId, id, user.id, optionIndex); + const result = this.collab.votePoll(tripId, id, user.id, body.option_index); if (result.error === 'not_found') throw new HttpException({ error: 'Poll not found' }, 404); if (result.error === 'closed') throw new HttpException({ error: 'Poll is closed' }, 400); if (result.error === 'invalid_index') throw new HttpException({ error: 'Invalid option index' }, 400); @@ -231,13 +235,13 @@ export class CollabController { } @Post('messages') - createMessage(@CurrentUser() user: User, @Param('tripId') tripId: string, @Body() body: { text?: string; reply_to?: number | null }, @Headers('x-socket-id') socketId?: string) { - if (body.text && body.text.length > 5000) { - throw new HttpException({ error: 'text must be 5000 characters or less' }, 400); - } + createMessage(@CurrentUser() user: User, @Param('tripId') tripId: string, @Body() body: CollabMessageCreateDto, @Headers('x-socket-id') socketId?: string) { + // The pipe's min(1)/max(5000) replaced the bespoke length checks (and still + // rejects before the trip-access check, like the legacy pre-access check + // did); min(1) doesn't trim, so whitespace-only text keeps its bespoke 400. const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (!body.text || !body.text.trim()) { + if (!body.text.trim()) { throw new HttpException({ error: 'Message text is required' }, 400); } const result = this.collab.createMessage(tripId, user.id, body.text, body.reply_to); @@ -252,13 +256,10 @@ export class CollabController { @Post('messages/:id/react') @HttpCode(200) - react(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Body('emoji') emoji: string, @Headers('x-socket-id') socketId?: string) { + react(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Body() body: CollabReactionDto, @Headers('x-socket-id') socketId?: string) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (!emoji) { - throw new HttpException({ error: 'Emoji is required' }, 400); - } - const result = this.collab.reactMessage(id, tripId, user.id, emoji); + const result = this.collab.reactMessage(id, tripId, user.id, body.emoji); if (!result.found) { throw new HttpException({ error: 'Message not found' }, 404); } diff --git a/server/src/nest/collab/collab.dto.ts b/server/src/nest/collab/collab.dto.ts new file mode 100644 index 000000000..bbd8e8eb7 --- /dev/null +++ b/server/src/nest/collab/collab.dto.ts @@ -0,0 +1,21 @@ +import { createZodDto } from 'nestjs-zod'; +import { + collabNoteCreateRequestSchema, + collabNoteUpdateRequestSchema, + collabPollCreateRequestSchema, + collabPollVoteRequestSchema, + collabMessageCreateRequestSchema, + collabReactionRequestSchema, +} from '@trek/shared'; + +/** + * Body DTOs for the collab controller. The global ZodValidationPipe (APP_PIPE + * in app.module.ts) validates any @Body() typed with these by metatype; the + * shared Zod schemas remain the single source of truth for the wire contract. + */ +export class CollabNoteCreateDto extends createZodDto(collabNoteCreateRequestSchema) {} +export class CollabNoteUpdateDto extends createZodDto(collabNoteUpdateRequestSchema) {} +export class CollabPollCreateDto extends createZodDto(collabPollCreateRequestSchema) {} +export class CollabPollVoteDto extends createZodDto(collabPollVoteRequestSchema) {} +export class CollabMessageCreateDto extends createZodDto(collabMessageCreateRequestSchema) {} +export class CollabReactionDto extends createZodDto(collabReactionRequestSchema) {} diff --git a/server/src/nest/common/body-contract-allow-list.ts b/server/src/nest/common/body-contract-allow-list.ts index b972dbe78..ec745e914 100644 --- a/server/src/nest/common/body-contract-allow-list.ts +++ b/server/src/nest/common/body-contract-allow-list.ts @@ -62,12 +62,6 @@ export const BODY_CONTRACT_ALLOW_LIST: string[] = [ 'BudgetController.updateSettlement', 'CategoriesController.create', 'CategoriesController.update', - 'CollabController.createMessage', - 'CollabController.createNote', - 'CollabController.createPoll', - 'CollabController.react', - 'CollabController.updateNote', - 'CollabController.votePoll', 'CollectionsController.deleteMany', 'CollectionsController.reorder', 'DaysController.create', diff --git a/server/tests/integration/collab.test.ts b/server/tests/integration/collab.test.ts index a3ff758aa..512141171 100644 --- a/server/tests/integration/collab.test.ts +++ b/server/tests/integration/collab.test.ts @@ -662,7 +662,9 @@ describe('Collab validation', () => { .send({ question: 'Only one option?', options: ['Option A'] }); expect(res.status).toBe(400); - expect(res.body.error).toMatch(/2 options/i); + // The Zod pipe's standard envelope replaced the bespoke 'At least 2 options + // are required' string when the collab bodies adopted DTOs. + expect(res.body.error).toMatch(/options/i); }); }); diff --git a/server/tests/unit/nest/collab.controller.test.ts b/server/tests/unit/nest/collab.controller.test.ts index ddc0c2cd1..b2cc99e02 100644 --- a/server/tests/unit/nest/collab.controller.test.ts +++ b/server/tests/unit/nest/collab.controller.test.ts @@ -46,9 +46,8 @@ describe('CollabController (parity with the legacy /api/trips/:tripId/collab rou expect(new CollabController(s).listNotes(user, '5')).toEqual({ notes: [{ id: 1 }] }); }); - it('POST 403 without collab_edit, 400 without title, else creates + broadcasts + notifies', () => { + it('POST 403 without collab_edit, else creates + broadcasts + notifies (empty title now 400s in the Zod pipe)', () => { expect(thrown(() => new CollabController(svc({ canEdit: vi.fn().mockReturnValue(false) })).createNote(user, '5', { title: 'T' }))).toEqual({ status: 403, body: { error: 'No permission' } }); - expect(thrown(() => new CollabController(svc()).createNote(user, '5', {}))).toEqual({ status: 400, body: { error: 'Title is required' } }); const createNote = vi.fn().mockReturnValue({ id: 9 }); const broadcast = vi.fn(); const notifyCollab = vi.fn(); @@ -96,20 +95,18 @@ describe('CollabController (parity with the legacy /api/trips/:tripId/collab rou }); describe('polls', () => { - it('POST 400 without question / <2 options, else creates', () => { - expect(thrown(() => new CollabController(svc()).createPoll(user, '5', {}))).toEqual({ status: 400, body: { error: 'Question is required' } }); - expect(thrown(() => new CollabController(svc()).createPoll(user, '5', { question: 'q', options: ['only'] }))).toEqual({ status: 400, body: { error: 'At least 2 options are required' } }); + it('POST creates (missing question / <2 options now 400 in the Zod pipe)', () => { const s = svc({ createPoll: vi.fn().mockReturnValue({ id: 7 }), broadcast: vi.fn() } as Partial); expect(new CollabController(s).createPoll(user, '5', { question: 'q', options: ['a', 'b'] })).toEqual({ poll: { id: 7 } }); }); it('vote maps not_found/closed/invalid_index, else broadcasts the poll', () => { - expect(thrown(() => new CollabController(svc({ votePoll: vi.fn().mockReturnValue({ error: 'not_found' }) } as Partial)).votePoll(user, '5', '7', 0))).toEqual({ status: 404, body: { error: 'Poll not found' } }); - expect(thrown(() => new CollabController(svc({ votePoll: vi.fn().mockReturnValue({ error: 'closed' }) } as Partial)).votePoll(user, '5', '7', 0))).toEqual({ status: 400, body: { error: 'Poll is closed' } }); - expect(thrown(() => new CollabController(svc({ votePoll: vi.fn().mockReturnValue({ error: 'invalid_index' }) } as Partial)).votePoll(user, '5', '7', 9))).toEqual({ status: 400, body: { error: 'Invalid option index' } }); + expect(thrown(() => new CollabController(svc({ votePoll: vi.fn().mockReturnValue({ error: 'not_found' }) } as Partial)).votePoll(user, '5', '7', { option_index: 0 }))).toEqual({ status: 404, body: { error: 'Poll not found' } }); + expect(thrown(() => new CollabController(svc({ votePoll: vi.fn().mockReturnValue({ error: 'closed' }) } as Partial)).votePoll(user, '5', '7', { option_index: 0 }))).toEqual({ status: 400, body: { error: 'Poll is closed' } }); + expect(thrown(() => new CollabController(svc({ votePoll: vi.fn().mockReturnValue({ error: 'invalid_index' }) } as Partial)).votePoll(user, '5', '7', { option_index: 9 }))).toEqual({ status: 400, body: { error: 'Invalid option index' } }); const broadcast = vi.fn(); const s = svc({ votePoll: vi.fn().mockReturnValue({ poll: { id: 7 } }), broadcast } as Partial); - expect(new CollabController(s).votePoll(user, '5', '7', 0, 'sock')).toEqual({ poll: { id: 7 } }); + expect(new CollabController(s).votePoll(user, '5', '7', { option_index: 0 }, 'sock')).toEqual({ poll: { id: 7 } }); expect(broadcast).toHaveBeenCalledWith('5', 'collab:poll:voted', { poll: { id: 7 } }, 'sock'); }); @@ -127,8 +124,7 @@ describe('CollabController (parity with the legacy /api/trips/:tripId/collab rou }); describe('messages', () => { - it('POST 400 over 5000 chars (before access), 400 empty, 400 reply_not_found, else creates + notifies', () => { - expect(thrown(() => new CollabController(svc()).createMessage(user, '5', { text: 'x'.repeat(5001) }))).toEqual({ status: 400, body: { error: 'text must be 5000 characters or less' } }); + it('POST 400 whitespace-only, 400 reply_not_found, else creates + notifies (length checks now in the Zod pipe)', () => { expect(thrown(() => new CollabController(svc()).createMessage(user, '5', { text: ' ' }))).toEqual({ status: 400, body: { error: 'Message text is required' } }); expect(thrown(() => new CollabController(svc({ createMessage: vi.fn().mockReturnValue({ error: 'reply_not_found' }) } as Partial)).createMessage(user, '5', { text: 'hi', reply_to: 99 }))).toEqual({ status: 400, body: { error: 'Reply target message not found' } }); const broadcast = vi.fn(); @@ -139,12 +135,11 @@ describe('CollabController (parity with the legacy /api/trips/:tripId/collab rou expect(notifyCollab).toHaveBeenCalledWith('5', user, 'hello'); }); - it('react 400 without emoji, 404 unknown, else broadcasts reactions', () => { - expect(thrown(() => new CollabController(svc()).react(user, '5', '3', ''))).toEqual({ status: 400, body: { error: 'Emoji is required' } }); - expect(thrown(() => new CollabController(svc({ reactMessage: vi.fn().mockReturnValue({ found: false, reactions: [] }) } as Partial)).react(user, '5', '3', '👍'))).toEqual({ status: 404, body: { error: 'Message not found' } }); + it('react 404 unknown, else broadcasts reactions (empty emoji now 400s in the Zod pipe)', () => { + expect(thrown(() => new CollabController(svc({ reactMessage: vi.fn().mockReturnValue({ found: false, reactions: [] }) } as Partial)).react(user, '5', '3', { emoji: '👍' }))).toEqual({ status: 404, body: { error: 'Message not found' } }); const broadcast = vi.fn(); const s = svc({ reactMessage: vi.fn().mockReturnValue({ found: true, reactions: [{ emoji: '👍', count: 1 }] }), broadcast } as Partial); - expect(new CollabController(s).react(user, '5', '3', '👍', 'sock')).toEqual({ reactions: [{ emoji: '👍', count: 1 }] }); + expect(new CollabController(s).react(user, '5', '3', { emoji: '👍' }, 'sock')).toEqual({ reactions: [{ emoji: '👍', count: 1 }] }); expect(broadcast).toHaveBeenCalledWith('5', 'collab:message:reacted', { messageId: 3, reactions: [{ emoji: '👍', count: 1 }] }, 'sock'); });