diff --git a/server/src/nest/common/body-contract-allow-list.ts b/server/src/nest/common/body-contract-allow-list.ts index 5874c60df..3886ce203 100644 --- a/server/src/nest/common/body-contract-allow-list.ts +++ b/server/src/nest/common/body-contract-allow-list.ts @@ -76,8 +76,6 @@ export const BODY_CONTRACT_ALLOW_LIST: string[] = [ 'CollectionsController.reorder', 'DayAssignmentsController.create', 'DayAssignmentsController.reorder', - 'DayNotesController.create', - 'DayNotesController.update', 'DaysController.create', 'DaysController.reorder', 'DaysController.transport', diff --git a/server/src/nest/days/day-notes.controller.ts b/server/src/nest/days/day-notes.controller.ts index fbbe45fbd..02b2fbbf0 100644 --- a/server/src/nest/days/day-notes.controller.ts +++ b/server/src/nest/days/day-notes.controller.ts @@ -12,33 +12,19 @@ import { } from '@nestjs/common'; import type { User } from '../../types'; import { DayNotesService } from './day-notes.service'; +import { DayNoteCreateDto, DayNoteUpdateDto } from './day-notes.dto'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; -type DayNoteBody = { text?: string; time?: string; icon?: string; sort_order?: number }; - -// Runs BEFORE the trip-access check, so an over-long field 400s first. The `time` -// cap matches the shared dayNote schema (max 250) and the note dialog's counter; -// it was 150 here, which rejected valid 151–250 char notes with a confusing error. -const MAX_LENGTHS: Record = { text: 500, time: 250 }; - -function validateLengths(body: Record): void { - for (const [field, max] of Object.entries(MAX_LENGTHS)) { - const value = body[field]; - if (value && typeof value === 'string' && value.length > max) { - throw new HttpException({ error: `${field} must be ${max} characters or less` }, 400); - } - } -} - /** * /api/trips/:tripId/days/:dayId/notes — free-text annotations on a day. * - * Byte-identical to the legacy Express route (server/src/routes/dayNotes.ts): - * the string-length guard runs first (400), then trip access (404), then the - * 'day_edit' permission (403); create 201 / rest 200; the bespoke "Day not - * found" / "Note not found" / "Text required" bodies; WebSocket broadcasts with - * the forwarded X-Socket-Id. + * Bodies validate against the @trek/shared day-note contracts via the global + * ZodValidationPipe (which, like the legacy string-length guard, 400s before + * the trip-access check), then trip access (404), then the 'day_edit' + * permission (403); create 201 / rest 200; the bespoke "Day not found" / + * "Note not found" / "Text required" bodies; WebSocket broadcasts with the + * forwarded X-Socket-Id. */ @Controller('api/trips/:tripId/days/:dayId/notes') @UseGuards(JwtAuthGuard) @@ -70,10 +56,9 @@ export class DayNotesController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('dayId') dayId: string, - @Body() body: DayNoteBody, + @Body() body: DayNoteCreateDto, @Headers('x-socket-id') socketId?: string, ) { - validateLengths(body); const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); if (!this.notes.dayExists(dayId, tripId)) { @@ -93,10 +78,9 @@ export class DayNotesController { @Param('tripId') tripId: string, @Param('dayId') dayId: string, @Param('id') id: string, - @Body() body: DayNoteBody, + @Body() body: DayNoteUpdateDto, @Headers('x-socket-id') socketId?: string, ) { - validateLengths(body); const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); const current = this.notes.getNote(id, dayId, tripId); diff --git a/server/src/nest/days/day-notes.dto.ts b/server/src/nest/days/day-notes.dto.ts new file mode 100644 index 000000000..561fc45b8 --- /dev/null +++ b/server/src/nest/days/day-notes.dto.ts @@ -0,0 +1,11 @@ +import { createZodDto } from 'nestjs-zod'; +import { dayNoteCreateRequestSchema, dayNoteUpdateRequestSchema } from '@trek/shared'; + +/** + * Server-side createZodDto wrappers over the @trek/shared day-note 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 DayNoteCreateDto extends createZodDto(dayNoteCreateRequestSchema) {} +export class DayNoteUpdateDto extends createZodDto(dayNoteUpdateRequestSchema) {} diff --git a/server/tests/unit/nest/days.controller.test.ts b/server/tests/unit/nest/days.controller.test.ts index 9df693148..b57c92cf3 100644 --- a/server/tests/unit/nest/days.controller.test.ts +++ b/server/tests/unit/nest/days.controller.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi } from 'vitest'; import { HttpException } from '@nestjs/common'; import { DaysController } from '../../../src/nest/days/days.controller'; import { DayNotesController } from '../../../src/nest/days/day-notes.controller'; +import { DayNoteCreateDto, DayNoteUpdateDto } from '../../../src/nest/days/day-notes.dto'; import { DayReorderError } from '../../../src/services/dayService'; import type { DaysService } from '../../../src/nest/days/days.service'; import type { DayNotesService } from '../../../src/nest/days/day-notes.service'; @@ -117,19 +118,16 @@ describe('DaysController (parity with the legacy /api/trips/:tripId/days route)' }); describe('DayNotesController (parity with the legacy /api/.../days/:dayId/notes route)', () => { - it('400 on an over-long text BEFORE the trip-access check (middleware order)', () => { - const verifyTripAccess = vi.fn().mockReturnValue(undefined); // would 404 if reached - const svc = notesSvc({ verifyTripAccess }); - expect(thrown(() => new DayNotesController(svc).create(user, '5', '3', { text: 'x'.repeat(501) }))).toEqual({ - status: 400, body: { error: 'text must be 500 characters or less' }, - }); - expect(verifyTripAccess).not.toHaveBeenCalled(); - }); - - it('400 on an over-long time', () => { - expect(thrown(() => new DayNotesController(notesSvc()).create(user, '5', '3', { text: 'ok', time: 'y'.repeat(251) }))).toEqual({ - status: 400, body: { error: 'time must be 250 characters or less' }, - }); + // The legacy in-controller length guard moved to the global ZodValidationPipe + // (day-notes.dto.ts over the @trek/shared schemas) — like the old guard it + // rejects before the trip-access check. The pipe path is exercised end-to-end + // in tests/e2e/days.e2e.test.ts; here we pin the caps on the DTO schemas. + it('DTO schemas carry the legacy length caps and moveDayNote nulls', () => { + expect(DayNoteCreateDto.schema.safeParse({ text: 'x'.repeat(501) }).success).toBe(false); + expect(DayNoteCreateDto.schema.safeParse({ text: 'ok', time: 'y'.repeat(251) }).success).toBe(false); + expect(DayNoteCreateDto.schema.safeParse({ text: 'ok', time: null, icon: null }).success).toBe(true); + expect(DayNoteUpdateDto.schema.safeParse({ time: 'y'.repeat(251) }).success).toBe(false); + expect(DayNoteUpdateDto.schema.safeParse({ time: null }).success).toBe(true); }); it('404 trip, 403 permission, 404 day, 400 empty text, then creates', () => { diff --git a/shared/src/day/day.schema.spec.ts b/shared/src/day/day.schema.spec.ts index 45596e377..1e765f5bf 100644 --- a/shared/src/day/day.schema.spec.ts +++ b/shared/src/day/day.schema.spec.ts @@ -21,6 +21,10 @@ describe('dayNoteCreateRequestSchema', () => { }).success, ).toBe(false); }); + + it('accepts null time/icon (moveDayNote re-sends the nullable entity fields)', () => { + expect(dayNoteCreateRequestSchema.safeParse({ text: 'ok', time: null, icon: null }).success).toBe(true); + }); }); describe('dayNoteUpdateRequestSchema', () => { @@ -29,4 +33,8 @@ describe('dayNoteUpdateRequestSchema', () => { expect(dayNoteUpdateRequestSchema.safeParse({ icon: '🍽️' }).success).toBe(true); expect(dayNoteUpdateRequestSchema.safeParse({ text: 'x'.repeat(501) }).success).toBe(false); }); + + it('accepts an explicit null time (clears the label)', () => { + expect(dayNoteUpdateRequestSchema.safeParse({ time: null }).success).toBe(true); + }); }); diff --git a/shared/src/day/day.schema.ts b/shared/src/day/day.schema.ts index f245220bf..e1dc166df 100644 --- a/shared/src/day/day.schema.ts +++ b/shared/src/day/day.schema.ts @@ -68,18 +68,21 @@ export const dayUpdateRequestSchema = z.object({ }); export type DayUpdateRequest = z.infer; +// `time`/`icon` accept null on the wire: the client's moveDayNote re-sends the +// entity fields (both nullable on dayNoteSchema) through create, and an explicit +// null time on update means "clear" (the legacy raw-body route accepted both). export const dayNoteCreateRequestSchema = z.object({ text: z.string().min(1).max(500), - time: z.string().max(250).optional(), - icon: z.string().optional(), + time: z.string().max(250).nullable().optional(), + icon: z.string().nullable().optional(), sort_order: z.number().optional(), }); export type DayNoteCreateRequest = z.infer; export const dayNoteUpdateRequestSchema = z.object({ text: z.string().max(500).optional(), - time: z.string().max(250).optional(), - icon: z.string().optional(), + time: z.string().max(250).nullable().optional(), + icon: z.string().nullable().optional(), sort_order: z.number().optional(), }); export type DayNoteUpdateRequest = z.infer;