diff --git a/server/src/mcp/resources.ts b/server/src/mcp/resources.ts index ec84a232c..5b2645e63 100644 --- a/server/src/mcp/resources.ts +++ b/server/src/mcp/resources.ts @@ -5,7 +5,6 @@ import { listDays, listAccommodations } from '../services/dayService'; import { listPlaces } from '../services/placeService'; import { listBudgetItems, getPerPersonSummary, calculateSettlement } from '../services/budgetService'; import { listReservations } from '../services/reservationService'; -import { listNotes as listDayNotes } from '../services/dayNoteService'; import { listNotes as listCollabNotes, listPolls, listMessages } from '../services/collabService'; import { listBucketList, listVisitedCountries, getStats as getAtlasStats, listManuallyVisitedRegions } from '../services/atlasService'; import { getNotifications } from '../services/inAppNotifications'; @@ -132,19 +131,8 @@ export function registerResources(server: McpServer, userId: number, scopes: str } ); - // Day notes - if (canReadTrips(scopes)) server.registerResource( - 'day-notes', - new ResourceTemplate('trek://trips/{tripId}/days/{dayId}/notes', { list: undefined }), - { description: 'Notes for a specific day in a trip', mimeType: 'application/json' }, - async (uri, { tripId, dayId }) => { - const tId = parseId(tripId); - const dId = parseId(dayId); - if (tId === null || dId === null || !canAccessTrip(tId, userId)) return accessDenied(uri.href); - const notes = listDayNotes(dId, tId); - return jsonContent(uri.href, notes); - } - ); + // The day-notes resource moved to the DI-discovered + // src/nest/days/day-notes.mcp.ts (@ResourceTemplate). // Accommodations (hotels, rentals) per trip if (canReadTrips(scopes)) server.registerResource( diff --git a/server/src/mcp/tools/days.ts b/server/src/mcp/tools/days.ts index 4ea491d0f..30fc7962b 100644 --- a/server/src/mcp/tools/days.ts +++ b/server/src/mcp/tools/days.ts @@ -8,10 +8,6 @@ import { createAccommodation, getAccommodation, updateAccommodation, deleteAccommodation, } from '../../services/dayService'; import { createPlace } from '../../services/placeService'; -import { - createNote as createDayNote, getNote as getDayNote, updateNote as updateDayNote, - deleteNote as deleteDayNote, dayExists as dayNoteExists, -} from '../../services/dayNoteService'; import { safeBroadcast, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, @@ -221,78 +217,6 @@ export function registerDayTools(server: McpServer, userId: number, scopes: stri } ); - // --- DAY NOTES --- - - server.registerTool( - 'create_day_note', - { - description: 'Add a note to a specific day in a trip.', - inputSchema: { - tripId: z.number().int().positive(), - dayId: z.number().int().positive(), - text: z.string().min(1).max(500), - time: z.string().max(250).optional().describe('Time label (e.g. "09:00" or "Morning")'), - icon: z.string().optional().describe('Emoji icon for the note'), - }, - annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, - }, - async ({ tripId, dayId, text, time, icon }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); - if (!dayNoteExists(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; - const note = createDayNote(dayId, tripId, text, time, icon); - safeBroadcast(tripId, 'dayNote:created', { dayId, note }); - return ok({ note }); - } - ); - - server.registerTool( - 'update_day_note', - { - description: 'Edit an existing note on a specific day.', - inputSchema: { - tripId: z.number().int().positive(), - dayId: z.number().int().positive(), - noteId: z.number().int().positive(), - text: z.string().min(1).max(500).optional(), - time: z.string().max(250).nullable().optional().describe('Time label (e.g. "09:00" or "Morning"), or null to clear'), - icon: z.string().optional().describe('Emoji icon for the note'), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ tripId, dayId, noteId, text, time, icon }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); - const existing = getDayNote(noteId, dayId, tripId); - if (!existing) return { content: [{ type: 'text' as const, text: 'Note not found.' }], isError: true }; - const note = updateDayNote(noteId, existing, { text, time: time !== undefined ? time : undefined, icon }); - safeBroadcast(tripId, 'dayNote:updated', { dayId, note }); - return ok({ note }); - } - ); - - server.registerTool( - 'delete_day_note', - { - description: 'Delete a note from a specific day.', - inputSchema: { - tripId: z.number().int().positive(), - dayId: z.number().int().positive(), - noteId: z.number().int().positive(), - }, - annotations: TOOL_ANNOTATIONS_DELETE, - }, - async ({ tripId, dayId, noteId }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); - const note = getDayNote(noteId, dayId, tripId); - if (!note) return { content: [{ type: 'text' as const, text: 'Note not found.' }], isError: true }; - deleteDayNote(noteId); - safeBroadcast(tripId, 'dayNote:deleted', { noteId, dayId }); - return ok({ success: true }); - } - ); + // The day-note tools (create/update/delete_day_note) moved to the + // DI-discovered src/nest/days/day-notes.mcp.ts (@McpController). } diff --git a/server/src/nest/README.md b/server/src/nest/README.md index 86af56f9b..a9ffab7c8 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -27,7 +27,7 @@ mount to Nest and leaves the sibling trip routes (days, places, ...) on Express. categories, tags, notifications, atlas. - **Phase 2 (trip sub-domains):** vacay (addon), packing, todo. - **DI-native services (legacy `src/services/*` deleted):** tags, categories, - todo, packing — see the migration recipe below. + todo, packing, day-notes — see the migration recipe below. ## Cross-cutting Foundation pieces @@ -101,8 +101,11 @@ first in-container consumer wiring: `TripsService` injects `TodoService` via `exports: [TodoService]` instead of using the bridge); packing followed (the largest port so far: a 17-tool + 2-resource `packing.mcp.ts` with inline admin gates, and the first `PluginHostDepsFactory` swap done as part of a service -migration — no bridge entry for the plugin host). Repeat these steps per -service (next up: dayNoteService). This is a **pure relocation** — byte-identical +migration — no bridge entry for the plugin host); day-notes followed (the first +migration needing **no bridge at all**: after its three tools + resource moved +to `day-notes.mcp.ts` and the plugin host injection, nothing outside the +container consumed it). Repeat these steps per +service (next up: tripInviteService). This is a **pure relocation** — byte-identical SQL, statuses, bodies, and error strings. The plugin RPC host is **no longer a bridge consumer**: since Option A of `src/nest/plugins/DI-MIGRATION.md` it injects domain services via `PluginHostDepsFactory`, so a migrated domain adds diff --git a/server/src/nest/days/day-notes.mcp.ts b/server/src/nest/days/day-notes.mcp.ts new file mode 100644 index 000000000..49bd41148 --- /dev/null +++ b/server/src/nest/days/day-notes.mcp.ts @@ -0,0 +1,134 @@ +import { + McpController, Tool, ResourceTemplate, type McpContext, + TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, + demoDenied, ok, +} from '@trek/nest-mcp'; +import { z } from 'zod'; +import { isDemoUser } from '../../services/authService'; +import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { DayNotesService } from './day-notes.service'; + +function parseId(value: string | string[]): number | null { + const n = Number(Array.isArray(value) ? value[0] : value); + return Number.isInteger(n) && n > 0 ? n : null; +} + +/** + * Day-note MCP surface — ported 1:1 from the legacy registrars: the three + * tools from the DAY NOTES block of src/mcp/tools/days.ts and the + * trek://trips/{tripId}/days/{dayId}/notes resource from src/mcp/resources.ts + * (identical names, descriptions, schemas, annotations, error/payload shapes + * and broadcasts). The legacy registration-time gates map to the declarative + * trips write/read access markers (registerDayTools' whole-registrar + * `canWrite(scopes, 'trips')` early return and the resource's canReadTrips + * check, resolved by trekMcpAccessPolicy). No addon gate — day notes are core. + */ +@McpController() +export class DayNotesMcp { + constructor(private readonly notes: DayNotesService) {} + + @Tool({ + name: 'create_day_note', + description: 'Add a note to a specific day in a trip.', + inputSchema: { + tripId: z.number().int().positive(), + dayId: z.number().int().positive(), + text: z.string().min(1).max(500), + time: z.string().max(250).optional().describe('Time label (e.g. "09:00" or "Morning")'), + icon: z.string().optional().describe('Emoji icon for the note'), + }, + annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, + access: { group: 'trips', mode: 'write' }, + }) + async createDayNote( + { tripId, dayId, text, time, icon }: { tripId: number; dayId: number; text: string; time?: string; icon?: string }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.notes.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.notes.dayExists(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; + const note = this.notes.create(dayId, tripId, text, time, icon); + safeBroadcast(tripId, 'dayNote:created', { dayId, note }); + return ok({ note }); + } + + @Tool({ + name: 'update_day_note', + description: 'Edit an existing note on a specific day.', + inputSchema: { + tripId: z.number().int().positive(), + dayId: z.number().int().positive(), + noteId: z.number().int().positive(), + text: z.string().min(1).max(500).optional(), + time: z.string().max(250).nullable().optional().describe('Time label (e.g. "09:00" or "Morning"), or null to clear'), + icon: z.string().optional().describe('Emoji icon for the note'), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + access: { group: 'trips', mode: 'write' }, + }) + async updateDayNote( + { tripId, dayId, noteId, text, time, icon }: { tripId: number; dayId: number; noteId: number; text?: string; time?: string | null; icon?: string }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.notes.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + const existing = this.notes.getNote(noteId, dayId, tripId); + if (!existing) return { content: [{ type: 'text' as const, text: 'Note not found.' }], isError: true }; + const note = this.notes.update(noteId, existing, { text, time: time !== undefined ? time : undefined, icon }); + safeBroadcast(tripId, 'dayNote:updated', { dayId, note }); + return ok({ note }); + } + + @Tool({ + name: 'delete_day_note', + description: 'Delete a note from a specific day.', + inputSchema: { + tripId: z.number().int().positive(), + dayId: z.number().int().positive(), + noteId: z.number().int().positive(), + }, + annotations: TOOL_ANNOTATIONS_DELETE, + access: { group: 'trips', mode: 'write' }, + }) + async deleteDayNote({ tripId, dayId, noteId }: { tripId: number; dayId: number; noteId: number }, ctx: McpContext) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.notes.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + const note = this.notes.getNote(noteId, dayId, tripId); + if (!note) return { content: [{ type: 'text' as const, text: 'Note not found.' }], isError: true }; + this.notes.remove(noteId); + safeBroadcast(tripId, 'dayNote:deleted', { noteId, dayId }); + return ok({ success: true }); + } + + @ResourceTemplate({ + name: 'day-notes', + uriTemplate: 'trek://trips/{tripId}/days/{dayId}/notes', + description: 'Notes for a specific day in a trip', + mimeType: 'application/json', + access: { group: 'trips', mode: 'read' }, + }) + async dayNotesResource(uri: URL, { tripId, dayId }: { tripId: string | string[]; dayId: string | string[] }, ctx: McpContext) { + const tId = parseId(tripId); + const dId = parseId(dayId); + if (tId === null || dId === null || !this.notes.verifyTripAccess(tId, ctx.userId)) { + return { + contents: [{ + uri: uri.href, + mimeType: 'application/json', + text: JSON.stringify({ error: 'Trip not found or access denied' }), + }], + }; + } + const notes = this.notes.list(dId, tId); + return { + contents: [{ + uri: uri.href, + mimeType: 'application/json', + text: JSON.stringify(notes, null, 2), + }], + }; + } +} diff --git a/server/src/nest/days/day-notes.service.ts b/server/src/nest/days/day-notes.service.ts index b43cde8b6..a779c0089 100644 --- a/server/src/nest/days/day-notes.service.ts +++ b/server/src/nest/days/day-notes.service.ts @@ -1,22 +1,25 @@ import { Injectable } from '@nestjs/common'; import { broadcast } from '../../websocket'; import { checkPermission } from '../../services/permissions'; -import type { User } from '../../types'; -import * as dayNoteService from '../../services/dayNoteService'; - -type Trip = NonNullable>; +import type { DayNote, User } from '../../types'; +import { DatabaseService, type TripAccess } from '../database/database.service'; /** - * Thin Nest wrapper around the existing day-note service. Trip access + the - * 'day_edit' permission reuse the legacy checks; the SQL is unchanged. + * Day-notes domain service — the legacy dayNoteService SQL folded in over + * the injected DatabaseService (byte-identical statements and + * coercions). Trip access rides DatabaseService.canAccessTrip (absorbing the + * legacy services/tripAccess re-export); the 'day_edit' permission reuses the + * legacy check. */ @Injectable() export class DayNotesService { - verifyTripAccess(tripId: string, userId: number) { - return dayNoteService.verifyTripAccess(tripId, userId); + constructor(private readonly dbs: DatabaseService) {} + + verifyTripAccess(tripId: string | number, userId: number): TripAccess | undefined { + return this.dbs.canAccessTrip(tripId, userId); } - canEdit(trip: Trip, user: User): boolean { + canEdit(trip: TripAccess, user: User): boolean { return checkPermission('day_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); } @@ -24,27 +27,42 @@ export class DayNotesService { broadcast(tripId, event, payload, socketId); } - list(dayId: string, tripId: string) { - return dayNoteService.listNotes(dayId, tripId); + list(dayId: string | number, tripId: string | number) { + return this.dbs.all( + 'SELECT * FROM day_notes WHERE day_id = ? AND trip_id = ? ORDER BY sort_order ASC, created_at ASC', + dayId, tripId, + ); } - dayExists(dayId: string, tripId: string) { - return dayNoteService.dayExists(dayId, tripId); + dayExists(dayId: string | number, tripId: string | number) { + return this.dbs.get('SELECT id FROM days WHERE id = ? AND trip_id = ?', dayId, tripId); } - getNote(id: string, dayId: string, tripId: string) { - return dayNoteService.getNote(id, dayId, tripId); + getNote(id: string | number, dayId: string | number, tripId: string | number) { + return this.dbs.get('SELECT * FROM day_notes WHERE id = ? AND day_id = ? AND trip_id = ?', id, dayId, tripId); } - create(dayId: string, tripId: string, text: string, time?: string, icon?: string, sortOrder?: number) { - return dayNoteService.createNote(dayId, tripId, text, time, icon, sortOrder); + create(dayId: string | number, tripId: string | number, text: string, time?: string | null, icon?: string | null, sortOrder?: number) { + const result = this.dbs.run( + 'INSERT INTO day_notes (day_id, trip_id, text, time, icon, sort_order) VALUES (?, ?, ?, ?, ?, ?)', + dayId, tripId, text.trim(), time || null, icon || '📝', sortOrder ?? 9999, + ); + return this.dbs.get('SELECT * FROM day_notes WHERE id = ?', result.lastInsertRowid); } - update(id: string, current: Parameters[1], fields: { text?: string; time?: string; icon?: string; sort_order?: number }) { - return dayNoteService.updateNote(id, current, fields); + update(id: string | number, current: DayNote, fields: { text?: string; time?: string | null; icon?: string | null; sort_order?: number }) { + this.dbs.run( + 'UPDATE day_notes SET text = ?, time = ?, icon = ?, sort_order = ? WHERE id = ?', + fields.text !== undefined ? fields.text.trim() : current.text, + fields.time !== undefined ? fields.time : current.time, + fields.icon !== undefined ? fields.icon : current.icon, + fields.sort_order !== undefined ? fields.sort_order : current.sort_order, + id, + ); + return this.dbs.get('SELECT * FROM day_notes WHERE id = ?', id); } - remove(id: string): void { - dayNoteService.deleteNote(id); + remove(id: string | number): void { + this.dbs.run('DELETE FROM day_notes WHERE id = ?', id); } } diff --git a/server/src/nest/days/days.module.ts b/server/src/nest/days/days.module.ts index eb19bfe98..84389896c 100644 --- a/server/src/nest/days/days.module.ts +++ b/server/src/nest/days/days.module.ts @@ -3,14 +3,17 @@ import { DaysController } from './days.controller'; import { DaysService } from './days.service'; import { DayNotesController } from './day-notes.controller'; import { DayNotesService } from './day-notes.service'; +import { DayNotesMcp } from './day-notes.mcp'; /** * Days + day-notes domain (S6 — Phase 2 trip sub-domain). The single prefix * /api/trips/:tripId/days covers both the days mount and the nested - * /days/:dayId/notes mount. + * /days/:dayId/notes mount. DayNotesService is exported for the plugin RPC + * host (PluginHostDepsFactory). */ @Module({ controllers: [DaysController, DayNotesController], - providers: [DaysService, DayNotesService], + providers: [DaysService, DayNotesService, DayNotesMcp], + exports: [DayNotesService], }) export class DaysModule {} diff --git a/server/src/nest/plugins/DI-MIGRATION.md b/server/src/nest/plugins/DI-MIGRATION.md index 19ac543b7..07d313045 100644 --- a/server/src/nest/plugins/DI-MIGRATION.md +++ b/server/src/nest/plugins/DI-MIGRATION.md @@ -98,8 +98,8 @@ is `@Global`, so no import needed for it. factory *until their own domain migrates*. Each future migration then swaps one import for one injected service (+ module export/import) — **no more bridge files for the plugin host, ever**. The factory's import list shrinks -monotonically as the recipe in `src/nest/README.md` proceeds (packing swapped -in 2026-07; next up: dayNoteService). +monotonically as the recipe in `src/nest/README.md` proceeds (packing and +day-notes swapped in 2026-07; next up: tripInviteService). ### Test impact (as landed) @@ -215,8 +215,9 @@ none of it is throwaway. 1. ~~**Now / opportunistically — Option A.**~~ **Done (2026-07).** Injectable factory, tags/categories bridges deleted, wiring test rewritten against stubs (see §2). -2. **Per domain, as the service migrations proceed** (`packingService` done - 2026-07; `dayNoteService` next, per `src/nest/README.md`): migrate the +2. **Per domain, as the service migrations proceed** (`packingService` and + `dayNoteService` done 2026-07; `tripInviteService` next, per + `src/nest/README.md`): migrate the service to DI as usual; the factory swaps one legacy import for one injected service. Optionally pilot `tags.rpc.ts` here — tags is small and already fully DI on HTTP + MCP. diff --git a/server/src/nest/plugins/host/plugin-host-deps.factory.ts b/server/src/nest/plugins/host/plugin-host-deps.factory.ts index aef14f59b..4f1da5bda 100644 --- a/server/src/nest/plugins/host/plugin-host-deps.factory.ts +++ b/server/src/nest/plugins/host/plugin-host-deps.factory.ts @@ -28,7 +28,6 @@ import { ADDON_IDS } from '../../../addons'; import { listJourneys, listEntries as listJournalEntriesSvc, createEntry as createJournalEntrySvc, updateEntry as updateJournalEntrySvc, deleteEntry as deleteJournalEntrySvc, createJourney as createJourneySvc, deleteJourney as deleteJourneySvc } from '../../../services/journeyService'; import { listVisitedCountries, listManuallyVisitedRegions, listBucketList, markCountryVisited, unmarkCountryVisited, markRegionVisited, unmarkRegionVisited, createBucketItem as createBucketItemSvc, deleteBucketItem as deleteBucketItemSvc } from '../../../services/atlasService'; import { getPlanData, getActivePlanId, toggleEntry as vacayToggleEntrySvc, toggleCompanyHoliday as vacayToggleCompanyHolidaySvc } from '../../../services/vacayService'; -import { listNotes, createNote, getNote, updateNote, deleteNote, dayExists as dayNoteDayExists } from '../../../services/dayNoteService'; import { listCollections, getCollection, createCollection, updateCollection, savePlace as saveCollectionPlaceSvc, copyToTrip as copyCollectionToTripSvc, deletePlace as deleteCollectionPlaceSvc } from '../../../services/collectionsService'; import { BudgetService } from '../../budget/budget.service'; import { ReservationsService } from '../../reservations/reservations.service'; @@ -36,6 +35,7 @@ import { TagsService } from '../../tags/tags.service'; import { CategoriesService } from '../../categories/categories.service'; import { TodoService } from '../../todo/todo.service'; import { PackingService } from '../../packing/packing.service'; +import { DayNotesService } from '../../days/day-notes.service'; import { notifyBookingChange } from '../../../services/reservationService'; import { PluginRpcHost, ForbiddenResource, BadParams } from './rpc-host'; import { appendAudit } from './plugin-audit'; @@ -150,9 +150,10 @@ export interface PluginCallRouter { * `plugin:{id}:{event}` so a plugin can't forge a core event. * * DI-native domain services (budget, reservations, tags, categories, todo, - * packing, oauth) are constructor-injected; legacy `services/*` domains stay - * plain function imports until their own migration lands (DI-MIGRATION.md), at - * which point each swaps one import for one injected service. + * packing, day-notes, oauth) are constructor-injected; legacy `services/*` + * domains stay plain function imports until their own migration lands + * (DI-MIGRATION.md), at which point each swaps one import for one injected + * service. */ @Injectable() export class PluginHostDepsFactory { @@ -164,6 +165,7 @@ export class PluginHostDepsFactory { private readonly todos: TodoService, private readonly packing: PackingService, private readonly oauth: PluginOAuthService, + private readonly dayNotes: DayNotesService, ) {} create(id: string, granted: ReadonlySet, router: PluginCallRouter): PluginRpcHost { @@ -690,27 +692,27 @@ export class PluginHostDepsFactory { return { deleted: true }; }, // Day notes are core (no addon) and trip-scoped; membership is enforced by the host. - listDayNotes: (tripId, dayId) => listNotes(dayId, tripId), + listDayNotes: (tripId, dayId) => this.dayNotes.list(dayId, tripId), // --- Day notes write (day_edit). The day must belong to the trip; broadcasts the // same dayNote:* events the REST controller emits so open sessions update live. --- createDayNote: (tripId, dayId, input) => { - if (!dayNoteDayExists(dayId, tripId)) throw new ForbiddenResource(`no day ${dayId} on trip ${tripId}`); + if (!this.dayNotes.dayExists(dayId, tripId)) throw new ForbiddenResource(`no day ${dayId} on trip ${tripId}`); const i = input as { text?: string; time?: string; icon?: string; sort_order?: number }; - const note = createNote(dayId, tripId, i.text ?? '', i.time, i.icon, i.sort_order); + const note = this.dayNotes.create(dayId, tripId, i.text ?? '', i.time, i.icon, i.sort_order); broadcast(tripId, 'dayNote:created', { dayId, note }, undefined); return note; }, updateDayNote: (tripId, dayId, noteId, input) => { - const current = getNote(noteId, dayId, tripId); + const current = this.dayNotes.getNote(noteId, dayId, tripId); if (!current) throw new ForbiddenResource(`no note ${noteId} on day ${dayId}`); - const note = updateNote(noteId, current as never, input as { text?: string; time?: string; icon?: string; sort_order?: number }); + const note = this.dayNotes.update(noteId, current as never, input as { text?: string; time?: string; icon?: string; sort_order?: number }); broadcast(tripId, 'dayNote:updated', { dayId, note }, undefined); return note; }, deleteDayNote: (tripId, dayId, noteId) => { - const current = getNote(noteId, dayId, tripId); + const current = this.dayNotes.getNote(noteId, dayId, tripId); if (!current) throw new ForbiddenResource(`no note ${noteId} on day ${dayId}`); - deleteNote(noteId); + this.dayNotes.remove(noteId); broadcast(tripId, 'dayNote:deleted', { noteId, dayId }, undefined); return { deleted: true }; }, diff --git a/server/src/nest/plugins/plugins.module.ts b/server/src/nest/plugins/plugins.module.ts index b8079bbbc..4a6aa7526 100644 --- a/server/src/nest/plugins/plugins.module.ts +++ b/server/src/nest/plugins/plugins.module.ts @@ -30,6 +30,7 @@ import { BudgetModule } from '../budget/budget.module'; import { ReservationsModule } from '../reservations/reservations.module'; import { TodoModule } from '../todo/todo.module'; import { PackingModule } from '../packing/packing.module'; +import { DaysModule } from '../days/days.module'; /** * Plugin system (#plugins). M0 read side + M2 isolated runtime + M3 frontend: @@ -41,7 +42,7 @@ import { PackingModule } from '../packing/packing.module'; @Module({ // The DI-native domain services the plugin host wiring injects // (PluginHostDepsFactory); DatabaseModule is @Global, so not listed. - imports: [TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule], + imports: [TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule, DaysModule], controllers: [PluginsController, PluginsFeedController, PluginsProxyController, PluginFrameController, PlaceDetailsController, TripWarningsController, ViewContributionsController, TripCardContributionsController, PluginPhotosController, PluginCalendarController, MapMarkersController, MapLayersController, PluginRoutesController, DayScheduleController, PdfSectionsController, AtlasLayersController, JournalEntryRowsController, PluginUserSettingsController, PluginOAuthController, PluginActivityController], providers: [PluginsService, PluginRuntimeService, PluginRegistryService, PluginOAuthService, PluginHostDepsFactory], // Exported so the admin addon-toggle handler can cascade-disable plugins whose diff --git a/server/src/services/dayNoteService.ts b/server/src/services/dayNoteService.ts deleted file mode 100644 index 3758678a3..000000000 --- a/server/src/services/dayNoteService.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { db } from '../db/database'; -import { DayNote } from '../types'; - -export { verifyTripAccess } from './tripAccess'; - -export function listNotes(dayId: string | number, tripId: string | number) { - return db.prepare( - 'SELECT * FROM day_notes WHERE day_id = ? AND trip_id = ? ORDER BY sort_order ASC, created_at ASC' - ).all(dayId, tripId); -} - -export function dayExists(dayId: string | number, tripId: string | number) { - return db.prepare('SELECT id FROM days WHERE id = ? AND trip_id = ?').get(dayId, tripId); -} - -export function createNote(dayId: string | number, tripId: string | number, text: string, time?: string, icon?: string, sort_order?: number) { - const result = db.prepare( - 'INSERT INTO day_notes (day_id, trip_id, text, time, icon, sort_order) VALUES (?, ?, ?, ?, ?, ?)' - ).run(dayId, tripId, text.trim(), time || null, icon || '\uD83D\uDCDD', sort_order ?? 9999); - return db.prepare('SELECT * FROM day_notes WHERE id = ?').get(result.lastInsertRowid); -} - -export function getNote(id: string | number, dayId: string | number, tripId: string | number) { - return db.prepare('SELECT * FROM day_notes WHERE id = ? AND day_id = ? AND trip_id = ?').get(id, dayId, tripId) as DayNote | undefined; -} - -export function updateNote(id: string | number, current: DayNote, fields: { text?: string; time?: string; icon?: string; sort_order?: number }) { - db.prepare( - 'UPDATE day_notes SET text = ?, time = ?, icon = ?, sort_order = ? WHERE id = ?' - ).run( - fields.text !== undefined ? fields.text.trim() : current.text, - fields.time !== undefined ? fields.time : current.time, - fields.icon !== undefined ? fields.icon : current.icon, - fields.sort_order !== undefined ? fields.sort_order : current.sort_order, - id - ); - return db.prepare('SELECT * FROM day_notes WHERE id = ?').get(id); -} - -export function deleteNote(id: string | number) { - db.prepare('DELETE FROM day_notes WHERE id = ?').run(id); -} diff --git a/server/tests/e2e/days.e2e.test.ts b/server/tests/e2e/days.e2e.test.ts index e6e869c2e..cc6e83ddd 100644 --- a/server/tests/e2e/days.e2e.test.ts +++ b/server/tests/e2e/days.e2e.test.ts @@ -1,7 +1,8 @@ /** * Days + day-notes module e2e — exercises both migrated mounts through the real - * JwtAuthGuard against a temp SQLite db. The day/day-note services, the - * permission check, canAccessTrip and the WebSocket broadcast are mocked. + * JwtAuthGuard against a temp SQLite db. The day service (still legacy), the + * permission check, canAccessTrip and the WebSocket broadcast are mocked; the + * DI-native DayNotesService runs real SQL against the temp db. */ import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; import request from 'supertest'; @@ -18,6 +19,11 @@ const { db } = vi.hoisted(() => { tmp.exec('PRAGMA journal_mode = WAL'); tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL, email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0);`); + // The tables DayNotesService really queries (real SQL, no service mock). + tmp.exec('CREATE TABLE days (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL);'); + tmp.exec(`CREATE TABLE day_notes (id INTEGER PRIMARY KEY AUTOINCREMENT, day_id INTEGER NOT NULL, + trip_id INTEGER NOT NULL, text TEXT NOT NULL, time TEXT, icon TEXT DEFAULT '📝', + sort_order REAL DEFAULT 0, created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`); return { db: tmp }; }); @@ -30,18 +36,14 @@ vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); vi.mock('../../src/services/permissions', () => ({ checkPermission })); -const { day, note } = vi.hoisted(() => ({ +const { day } = vi.hoisted(() => ({ day: { listDays: vi.fn(), createDay: vi.fn(), getDay: vi.fn(), updateDay: vi.fn(), deleteDay: vi.fn() }, - note: { - verifyTripAccess: vi.fn(), listNotes: vi.fn(), dayExists: vi.fn(), createNote: vi.fn(), - getNote: vi.fn(), updateNote: vi.fn(), deleteNote: vi.fn(), - }, })); vi.mock('../../src/services/dayService', () => day); -vi.mock('../../src/services/dayNoteService', () => note); import { DaysModule } from '../../src/nest/days/days.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; +import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe'; describe('Days + day-notes e2e (real auth guard + temp SQLite)', () => { let server: Server; @@ -51,6 +53,7 @@ describe('Days + day-notes e2e (real auth guard + temp SQLite)', () => { const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, DaysModule] }).compile(); const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); + nest.useGlobalPipes(new ZodValidationPipe()); nest.useGlobalFilters(new TrekExceptionFilter()); await nest.init(); return nest; @@ -58,18 +61,16 @@ describe('Days + day-notes e2e (real auth guard + temp SQLite)', () => { beforeAll(async () => { seedUser(db as never, { id: 1 }); + db.prepare('INSERT INTO days (id, trip_id) VALUES (3, 5)').run(); app = await build(); server = app.getHttpServer(); day.listDays.mockReturnValue({ days: [{ id: 1 }] }); day.createDay.mockReturnValue({ id: 9 }); - note.listNotes.mockReturnValue([{ id: 1 }]); - note.dayExists.mockReturnValue(true); - note.createNote.mockReturnValue({ id: 7 }); }); beforeEach(() => { + db.prepare('DELETE FROM day_notes').run(); canAccessTrip.mockReturnValue({ id: 5, user_id: 1 }); - note.verifyTripAccess.mockReturnValue({ id: 5, user_id: 1 }); checkPermission.mockReturnValue(true); }); @@ -97,13 +98,29 @@ describe('Days + day-notes e2e (real auth guard + temp SQLite)', () => { expect(miss.body).toEqual({ error: 'Trip not found' }); }); - it('201 create note, 400 on over-long text (before access)', async () => { - const ok = await request(server).post('/api/trips/5/days/3/notes').set('Cookie', sessionCookie(1)).send({ text: 'Lunch' }); + it('201 create note (real insert: trim, empty-string coercions), 400 on over-long text (before access)', async () => { + const ok = await request(server).post('/api/trips/5/days/3/notes').set('Cookie', sessionCookie(1)) + .send({ text: ' Lunch ', time: '', icon: '', sort_order: 0 }); expect(ok.status).toBe(201); - expect(ok.body).toEqual({ note: { id: 7 } }); + expect(ok.body.note).toMatchObject({ day_id: 3, trip_id: 5, text: 'Lunch', time: null, icon: '📝', sort_order: 0 }); + const row = db.prepare('SELECT * FROM day_notes WHERE id = ?').get(ok.body.note.id); + expect(row).toMatchObject({ text: 'Lunch', time: null, icon: '📝', sort_order: 0 }); const long = await request(server).post('/api/trips/5/days/3/notes').set('Cookie', sessionCookie(1)).send({ text: 'x'.repeat(501) }); expect(long.status).toBe(400); - expect(long.body).toEqual({ error: 'text must be 500 characters or less' }); + expect(long.body.error).toContain('text'); + }); + + it('201 create accepts null time/icon (moveDayNote re-sends the nullable entity fields)', async () => { + const res = await request(server).post('/api/trips/5/days/3/notes').set('Cookie', sessionCookie(1)) + .send({ text: 'Moved', time: null, icon: null, sort_order: 3 }); + expect(res.status).toBe(201); + expect(res.body.note).toMatchObject({ text: 'Moved', time: null, icon: '📝', sort_order: 3 }); + }); + + it('404 Day not found when the day is not on the trip', async () => { + const res = await request(server).post('/api/trips/5/days/99/notes').set('Cookie', sessionCookie(1)).send({ text: 'Lunch' }); + expect(res.status).toBe(404); + expect(res.body).toEqual({ error: 'Day not found' }); }); it('400 note without text', async () => { @@ -111,4 +128,26 @@ describe('Days + day-notes e2e (real auth guard + temp SQLite)', () => { expect(res.status).toBe(400); expect(res.body).toEqual({ error: 'Text required' }); }); + + it('200 update note merges omitted fields from the current row', async () => { + const created = await request(server).post('/api/trips/5/days/3/notes').set('Cookie', sessionCookie(1)) + .send({ text: 'Lunch', time: '12:00' }); + const id = created.body.note.id; + const res = await request(server).put(`/api/trips/5/days/3/notes/${id}`).set('Cookie', sessionCookie(1)) + .send({ icon: '🍜' }); + expect(res.status).toBe(200); + expect(res.body.note).toMatchObject({ id, text: 'Lunch', time: '12:00', icon: '🍜' }); + const miss = await request(server).put('/api/trips/5/days/3/notes/9999').set('Cookie', sessionCookie(1)).send({ text: 'x' }); + expect(miss.status).toBe(404); + expect(miss.body).toEqual({ error: 'Note not found' }); + }); + + it('200 delete note removes the row', async () => { + const created = await request(server).post('/api/trips/5/days/3/notes').set('Cookie', sessionCookie(1)).send({ text: 'Lunch' }); + const id = created.body.note.id; + const res = await request(server).delete(`/api/trips/5/days/3/notes/${id}`).set('Cookie', sessionCookie(1)); + expect(res.status).toBe(200); + expect(res.body).toEqual({ success: true }); + expect(db.prepare('SELECT * FROM day_notes WHERE id = ?').get(id)).toBeUndefined(); + }); }); diff --git a/server/tests/helpers/factories.ts b/server/tests/helpers/factories.ts index a4bd57af2..7acb2c521 100644 --- a/server/tests/helpers/factories.ts +++ b/server/tests/helpers/factories.ts @@ -279,11 +279,11 @@ export function createDayNote( db: Database.Database, dayId: number, tripId: number, - overrides: Partial<{ text: string; time: string; icon: string }> = {} + overrides: Partial<{ text: string; time: string; icon: string; sort_order: number }> = {} ): TestDayNote { const result = db.prepare( - 'INSERT INTO day_notes (day_id, trip_id, text, time, icon, sort_order) VALUES (?, ?, ?, ?, ?, 9999)' - ).run(dayId, tripId, overrides.text ?? 'Test note', overrides.time ?? null, overrides.icon ?? '📝'); + 'INSERT INTO day_notes (day_id, trip_id, text, time, icon, sort_order) VALUES (?, ?, ?, ?, ?, ?)' + ).run(dayId, tripId, overrides.text ?? 'Test note', overrides.time ?? null, overrides.icon ?? '📝', overrides.sort_order ?? 9999); return db.prepare('SELECT * FROM day_notes WHERE id = ?').get(result.lastInsertRowid) as TestDayNote; } diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index 4bb27f6bf..343b374d8 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -4,6 +4,8 @@ import { trekMcpAccessPolicy } from '../../src/mcp/nest-mcp-policy'; import { CategoriesMcp } from '../../src/nest/categories/categories.mcp'; import { CategoriesService } from '../../src/nest/categories/categories.service'; import { DatabaseService } from '../../src/nest/database/database.service'; +import { DayNotesMcp } from '../../src/nest/days/day-notes.mcp'; +import { DayNotesService } from '../../src/nest/days/day-notes.service'; import { PackingMcp } from '../../src/nest/packing/packing.mcp'; import { PackingService } from '../../src/nest/packing/packing.service'; import { TagsMcp } from '../../src/nest/tags/tags.mcp'; @@ -26,6 +28,7 @@ export function createMcpTestRegistry(): McpRegistry { new CategoriesMcp(new CategoriesService(dbService)), new TodoMcp(new TodoService(dbService)), new PackingMcp(new PackingService(dbService)), + new DayNotesMcp(new DayNotesService(dbService)), ], { accessPolicy: trekMcpAccessPolicy }, ); diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index a5d47ecc2..1a6e6ffc7 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -9,6 +9,7 @@ import { TagsService } from '../../src/nest/tags/tags.service'; import { CategoriesService } from '../../src/nest/categories/categories.service'; import { TodoService } from '../../src/nest/todo/todo.service'; import { PackingService } from '../../src/nest/packing/packing.service'; +import { DayNotesService } from '../../src/nest/days/day-notes.service'; /** * Hand-wired counterpart of the PluginsModule DI graph for no-Nest tests @@ -25,6 +26,7 @@ export function createHostDepsFactory(dbs: DatabaseService): PluginHostDepsFacto new TodoService(dbs), new PackingService(dbs), new PluginOAuthService(dbs), + new DayNotesService(dbs), ); } diff --git a/server/tests/unit/mcp/resources.test.ts b/server/tests/unit/mcp/resources.test.ts index 67a3ed30f..90d14cf57 100644 --- a/server/tests/unit/mcp/resources.test.ts +++ b/server/tests/unit/mcp/resources.test.ts @@ -1,10 +1,12 @@ /** * Unit tests for MCP resources (resources.ts). - * Tests all 10 legacy-registrar resources via InMemoryTransport + Client + * Tests all 9 legacy-registrar resources via InMemoryTransport + Client * (trek://categories moved to the DI-discovered CategoriesMcp — see * tools-categories.test.ts; trek://trips/{tripId}/todos moved to TodoMcp — * see tools-todos.test.ts; trek://trips/{tripId}/packing and .../packing/bags - * moved to PackingMcp — see tools-packing.test.ts). + * moved to PackingMcp — see tools-packing.test.ts; + * trek://trips/{tripId}/days/{dayId}/notes moved to DayNotesMcp — see + * tools-notes.test.ts). */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll, afterEach } from 'vitest'; @@ -43,7 +45,7 @@ vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; -import { createUser, createTrip, createDay, createPlace, addTripMember, createBudgetItem, createReservation, createDayNote, createCollabNote, createBucketListItem, createVisitedCountry, createDayAssignment, createDayAccommodation } from '../../helpers/factories'; +import { createUser, createTrip, createDay, createPlace, addTripMember, createBudgetItem, createReservation, createCollabNote, createBucketListItem, createVisitedCountry, createDayAssignment, createDayAccommodation } from '../../helpers/factories'; import { createMcpHarness, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness'; beforeAll(() => { @@ -268,45 +270,8 @@ describe('Resource: trek://trips/{tripId}/reservations', () => { }); }); -describe('Resource: trek://trips/{tripId}/days/{dayId}/notes', () => { - it('returns notes for a specific day', async () => { - const { user } = createUser(testDb); - const trip = createTrip(testDb, user.id); - const day = createDay(testDb, trip.id); - createDayNote(testDb, day.id, trip.id, { text: 'Check in at noon' }); - - await withHarness(user.id, async (harness) => { - const result = await harness.client.readResource({ uri: `trek://trips/${trip.id}/days/${day.id}/notes` }); - const notes = parseResourceResult(result) as any[]; - expect(notes).toHaveLength(1); - expect(notes[0].text).toBe('Check in at noon'); - }); - }); - - it('returns access denied for unauthorized trip', async () => { - const { user } = createUser(testDb); - const { user: other } = createUser(testDb); - const trip = createTrip(testDb, other.id); - const day = createDay(testDb, trip.id); - - await withHarness(user.id, async (harness) => { - const result = await harness.client.readResource({ uri: `trek://trips/${trip.id}/days/${day.id}/notes` }); - const data = parseResourceResult(result) as any; - expect(data.error).toBeTruthy(); - }); - }); - - it('returns access denied for invalid dayId', async () => { - const { user } = createUser(testDb); - const trip = createTrip(testDb, user.id); - - await withHarness(user.id, async (harness) => { - const result = await harness.client.readResource({ uri: `trek://trips/${trip.id}/days/abc/notes` }); - const data = parseResourceResult(result) as any; - expect(data.error).toBeTruthy(); - }); - }); -}); +// The trek://trips/{tripId}/days/{dayId}/notes resource moved to the +// DI-discovered DayNotesMcp — see tools-notes.test.ts. describe('Resource: trek://trips/{tripId}/accommodations', () => { it('returns accommodations for a trip', async () => { diff --git a/server/tests/unit/mcp/tools-notes.test.ts b/server/tests/unit/mcp/tools-notes.test.ts index a3927b4ea..fca70ae2b 100644 --- a/server/tests/unit/mcp/tools-notes.test.ts +++ b/server/tests/unit/mcp/tools-notes.test.ts @@ -1,6 +1,9 @@ /** - * Unit tests for MCP note tools: create_day_note, update_day_note, delete_day_note, - * create_collab_note, update_collab_note, delete_collab_note. + * Unit tests for MCP note tools: create_day_note, update_day_note, delete_day_note + * (DayNotesMcp, DI-discovered — attached via the nest-mcp registry inside + * registerTools, so every harness here keeps withTools on) and + * create_collab_note, update_collab_note, delete_collab_note (still on the + * legacy registrar). */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; @@ -44,7 +47,7 @@ import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, createDay, createDayNote, createCollabNote } from '../../helpers/factories'; -import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness'; +import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness'; beforeAll(() => { createTables(testDb); @@ -258,6 +261,84 @@ describe('Tool: delete_day_note', () => { }); }); +// --------------------------------------------------------------------------- +// Scope gating (trips write, registration-time — the legacy registerDayTools +// whole-registrar `canWrite(scopes, 'trips')` early return, now the +// declarative access marker on every DayNotesMcp tool) +// --------------------------------------------------------------------------- + +describe('Day-note tools — scope gating', () => { + const WRITE_TOOLS = ['create_day_note', 'update_day_note', 'delete_day_note']; + + async function listToolNames(userId: number, scopes: string[] | null): Promise { + const h = await createMcpHarness({ userId, withResources: false, scopes }); + try { + return (await h.client.listTools()).tools.map((t) => t.name); + } finally { + await h.cleanup(); + } + } + + it('registers all three tools with null scopes (full access)', async () => { + const { user } = createUser(testDb); + const names = await listToolNames(user.id, null); + for (const tool of WRITE_TOOLS) expect(names).toContain(tool); + }); + + it('registers no day-note tools with trips:read only (all three are writes)', async () => { + const { user } = createUser(testDb); + const names = await listToolNames(user.id, ['trips:read']); + for (const tool of WRITE_TOOLS) expect(names).not.toContain(tool); + }); + + it('registers no day-note tools for an unrelated scope', async () => { + const { user } = createUser(testDb); + const names = await listToolNames(user.id, ['budget:read']); + for (const tool of WRITE_TOOLS) expect(names).not.toContain(tool); + }); +}); + +// --------------------------------------------------------------------------- +// trek://trips/{tripId}/days/{dayId}/notes resource (moved from the legacy +// registerResources to DayNotesMcp) +// --------------------------------------------------------------------------- + +describe('Resource: trek://trips/{tripId}/days/{dayId}/notes', () => { + it('returns the day notes ordered by sort_order', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + createDayNote(testDb, day.id, trip.id, { text: 'Second', sort_order: 5 }); + createDayNote(testDb, day.id, trip.id, { text: 'First', sort_order: 1 }); + await withHarness(user.id, async (h) => { + const result = await h.client.readResource({ uri: `trek://trips/${trip.id}/days/${day.id}/notes` }); + const notes = parseResourceResult(result) as { text: string }[]; + expect(notes).toHaveLength(2); + expect(notes[0].text).toBe('First'); + expect(notes[1].text).toBe('Second'); + }); + }); + + it('returns the access-denied payload for a non-member', async () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb); + const trip = createTrip(testDb, other.id); + const day = createDay(testDb, trip.id); + await withHarness(user.id, async (h) => { + const result = await h.client.readResource({ uri: `trek://trips/${trip.id}/days/${day.id}/notes` }); + expect(parseResourceResult(result)).toEqual({ error: 'Trip not found or access denied' }); + }); + }); + + it('returns the access-denied payload for a malformed id', async () => { + const { user } = createUser(testDb); + await withHarness(user.id, async (h) => { + const result = await h.client.readResource({ uri: 'trek://trips/1/days/not-a-number/notes' }); + expect(parseResourceResult(result)).toEqual({ error: 'Trip not found or access denied' }); + }); + }); +}); + // --------------------------------------------------------------------------- // create_collab_note // --------------------------------------------------------------------------- diff --git a/server/tests/unit/nest/day-notes.service.test.ts b/server/tests/unit/nest/day-notes.service.test.ts new file mode 100644 index 000000000..bf9a3f01c --- /dev/null +++ b/server/tests/unit/nest/day-notes.service.test.ts @@ -0,0 +1,223 @@ +/** + * Unit tests for the DI-native DayNotesService — DAYNOTE-SVC-001 through + * DAYNOTE-SVC-016. The legacy dayNoteService had no dedicated suite (it was + * only covered transitively), so these cases are authored fresh with the DI + * move to pin the relocated SQL and its quirks (trim-on-write, `||` empty- + * string coercions vs the `??` sort_order default, JS-side update merge, + * post-write re-selects). Uses a real in-memory SQLite DB so SQL logic is + * exercised faithfully. + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; + +// ── DB setup ────────────────────────────────────────────────────────────────── + +const { testDb, dbMock } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA busy_timeout = 5000'); + const mock = { + db, + closeDb: () => {}, + reinitialize: () => {}, + getPlaceWithTags: () => null, + canAccessTrip: (tripId: number | string, userId: number) => + db.prepare(` + SELECT t.id, t.user_id FROM trips t + LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? + WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL) + `).get(userId, tripId, userId), + isOwner: (tripId: number | string, userId: number) => + !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), + }; + return { testDb: db, dbMock: mock }; +}); + +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../src/config', () => ({ + JWT_SECRET: 'test-secret', + ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', + updateJwtSecret: () => {}, +})); +vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() })); + +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { resetTestDb } from '../../helpers/test-db'; +import { createUser, createTrip, createDay, addTripMember } from '../../helpers/factories'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { DayNotesService } from '../../../src/nest/days/day-notes.service'; +import type { DayNote } from '../../../src/types'; + +const svc = new DayNotesService(new DatabaseService(testDb)); + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + resetTestDb(testDb); +}); + +afterAll(() => { + testDb.close(); +}); + +function seedTripAndDay() { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + return { user, trip, day }; +} + +// ── verifyTripAccess ────────────────────────────────────────────────────────── + +describe('verifyTripAccess', () => { + it('DAYNOTE-SVC-001: returns trip for owner', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const result = svc.verifyTripAccess(trip.id, user.id); + expect(result).toBeDefined(); + expect(result?.id).toBe(trip.id); + }); + + it('DAYNOTE-SVC-002: returns nothing for non-member', () => { + const { user: owner } = createUser(testDb); + const { user: stranger } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + expect(svc.verifyTripAccess(trip.id, stranger.id)).toBeFalsy(); + }); + + it('DAYNOTE-SVC-003: returns trip for member', () => { + const { user: owner } = createUser(testDb); + const { user: member } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + addTripMember(testDb, trip.id, member.id); + expect(svc.verifyTripAccess(trip.id, member.id)).toBeDefined(); + }); +}); + +// ── dayExists ───────────────────────────────────────────────────────────────── + +describe('dayExists', () => { + it('DAYNOTE-SVC-004: truthy only when the day belongs to the trip', () => { + const { trip, day } = seedTripAndDay(); + const { user: other } = createUser(testDb); + const otherTrip = createTrip(testDb, other.id); + expect(svc.dayExists(day.id, trip.id)).toBeTruthy(); + expect(svc.dayExists(day.id, otherTrip.id)).toBeFalsy(); + expect(svc.dayExists(9999, trip.id)).toBeFalsy(); + }); +}); + +// ── create ──────────────────────────────────────────────────────────────────── + +describe('create', () => { + it('DAYNOTE-SVC-005: inserts and returns the re-selected row', () => { + const { trip, day } = seedTripAndDay(); + const note = svc.create(day.id, trip.id, 'Lunch', '12:00', '🍜', 2) as DayNote; + expect(note).toMatchObject({ day_id: day.id, trip_id: trip.id, text: 'Lunch', time: '12:00', icon: '🍜', sort_order: 2 }); + expect(note.id).toBeGreaterThan(0); + const row = testDb.prepare('SELECT * FROM day_notes WHERE id = ?').get(note.id); + expect(row).toEqual(note); + }); + + it('DAYNOTE-SVC-006: trims the text on insert', () => { + const { trip, day } = seedTripAndDay(); + const note = svc.create(day.id, trip.id, ' Lunch ') as DayNote; + expect(note.text).toBe('Lunch'); + }); + + it('DAYNOTE-SVC-007: empty-string time coerces to NULL (`||`, not `??`)', () => { + const { trip, day } = seedTripAndDay(); + expect((svc.create(day.id, trip.id, 'a', '') as DayNote).time).toBeNull(); + expect((svc.create(day.id, trip.id, 'b', undefined) as DayNote).time).toBeNull(); + expect((svc.create(day.id, trip.id, 'c', null) as DayNote).time).toBeNull(); + }); + + it('DAYNOTE-SVC-008: empty/absent icon falls back to 📝 (`||`, not `??`)', () => { + const { trip, day } = seedTripAndDay(); + expect((svc.create(day.id, trip.id, 'a', undefined, '') as DayNote).icon).toBe('📝'); + expect((svc.create(day.id, trip.id, 'b') as DayNote).icon).toBe('📝'); + expect((svc.create(day.id, trip.id, 'c', undefined, null) as DayNote).icon).toBe('📝'); + }); + + it('DAYNOTE-SVC-009: sort_order 0 is preserved, only undefined defaults to 9999 (`??`)', () => { + const { trip, day } = seedTripAndDay(); + expect((svc.create(day.id, trip.id, 'a', undefined, undefined, 0) as DayNote).sort_order).toBe(0); + expect((svc.create(day.id, trip.id, 'b') as DayNote).sort_order).toBe(9999); + }); +}); + +// ── list ────────────────────────────────────────────────────────────────────── + +describe('list', () => { + it('DAYNOTE-SVC-010: returns [] for a day without notes', () => { + const { trip, day } = seedTripAndDay(); + expect(svc.list(day.id, trip.id)).toEqual([]); + }); + + it('DAYNOTE-SVC-011: orders by sort_order then created_at and scopes to day+trip', () => { + const { trip, day } = seedTripAndDay(); + const otherDay = createDay(testDb, trip.id); + const second = svc.create(day.id, trip.id, 'second', undefined, undefined, 5) as DayNote; + const first = svc.create(day.id, trip.id, 'first', undefined, undefined, 1) as DayNote; + svc.create(otherDay.id, trip.id, 'elsewhere'); + const notes = svc.list(day.id, trip.id) as DayNote[]; + expect(notes.map((n) => n.id)).toEqual([first.id, second.id]); + }); +}); + +// ── getNote ─────────────────────────────────────────────────────────────────── + +describe('getNote', () => { + it('DAYNOTE-SVC-012: returns the note only under its own day and trip', () => { + const { trip, day } = seedTripAndDay(); + const otherDay = createDay(testDb, trip.id); + const note = svc.create(day.id, trip.id, 'Lunch') as DayNote; + expect(svc.getNote(note.id, day.id, trip.id)).toEqual(note); + expect(svc.getNote(note.id, otherDay.id, trip.id)).toBeUndefined(); + expect(svc.getNote(note.id, day.id, trip.id + 1)).toBeUndefined(); + }); +}); + +// ── update ──────────────────────────────────────────────────────────────────── + +describe('update', () => { + it('DAYNOTE-SVC-013: merges omitted fields from the current row (JS-side, full-row UPDATE)', () => { + const { trip, day } = seedTripAndDay(); + const note = svc.create(day.id, trip.id, 'Lunch', '12:00', '🍜', 2) as DayNote; + const updated = svc.update(note.id, note, { icon: '🍣' }) as DayNote; + expect(updated).toMatchObject({ id: note.id, text: 'Lunch', time: '12:00', icon: '🍣', sort_order: 2 }); + }); + + it('DAYNOTE-SVC-014: explicit null time clears it, undefined keeps it', () => { + const { trip, day } = seedTripAndDay(); + const note = svc.create(day.id, trip.id, 'Lunch', '12:00') as DayNote; + const kept = svc.update(note.id, note, { text: 'Lunch!' }) as DayNote; + expect(kept.time).toBe('12:00'); + const cleared = svc.update(note.id, kept, { time: null }) as DayNote; + expect(cleared.time).toBeNull(); + }); + + it('DAYNOTE-SVC-015: trims only the new text and preserves sort_order 0 writes', () => { + const { trip, day } = seedTripAndDay(); + const note = svc.create(day.id, trip.id, 'Lunch', undefined, undefined, 5) as DayNote; + const updated = svc.update(note.id, note, { text: ' Dinner ', sort_order: 0 }) as DayNote; + expect(updated.text).toBe('Dinner'); + expect(updated.sort_order).toBe(0); + }); +}); + +// ── remove ──────────────────────────────────────────────────────────────────── + +describe('remove', () => { + it('DAYNOTE-SVC-016: deletes by bare id (trip-scoping is the caller getNote)', () => { + const { trip, day } = seedTripAndDay(); + const note = svc.create(day.id, trip.id, 'Lunch') as DayNote; + svc.remove(note.id); + expect(testDb.prepare('SELECT * FROM day_notes WHERE id = ?').get(note.id)).toBeUndefined(); + }); +}); diff --git a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts index e8c79a975..d05505940 100644 --- a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts +++ b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts @@ -3,9 +3,9 @@ * capability host to the real privileged modules (#plugins, M1). Verifies the * per-plugin data db is cached, a granted db:own call works through the wired * host, and trip broadcasts are force-namespaced to plugin:{id}:{event}. - * DI-native domains (budget/reservations/tags/categories/todo/packing/oauth) - * are constructor-injected stubs; legacy services/* domains stay path-mocked - * until their own DI migration lands. + * DI-native domains (budget/reservations/tags/categories/todo/packing/ + * day-notes/oauth) are constructor-injected stubs; legacy services/* domains + * stay path-mocked until their own DI migration lands. */ import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest'; import fs from 'node:fs'; @@ -273,14 +273,15 @@ vi.mock('../../../src/services/collectionsService', () => { deletePlace: vi.fn((_uid: number, placeId: number) => { if (placeId === 404) httpError(404, 'Collection not found'); }), }; }); -vi.mock('../../../src/services/dayNoteService', () => ({ - listNotes: vi.fn((dayId: number, tripId: number) => [{ id: 1, day_id: dayId, trip_id: tripId }]), - createNote: vi.fn((dayId: number, _tripId: number, text: string) => ({ id: 50, day_id: dayId, text })), +// Day notes are a constructor-injected stub (same behaviors as the old path mock). +const dayNotesStub = { + list: vi.fn((dayId: number, tripId: number) => [{ id: 1, day_id: dayId, trip_id: tripId }]), + create: vi.fn((dayId: number, _tripId: number, text: string) => ({ id: 50, day_id: dayId, text })), getNote: vi.fn((id: number) => (id === 99 ? undefined : { id, text: 'Old' })), - updateNote: vi.fn((id: number, _current: unknown, fields: Record) => ({ id, ...fields })), - deleteNote: vi.fn(), + update: vi.fn((id: number, _current: unknown, fields: Record) => ({ id, ...fields })), + remove: vi.fn(), dayExists: vi.fn((dayId: number) => dayId === 3), -})); +} as unknown as DayNotesService; import { PluginHostDepsFactory, type PluginCallRouter } from '../../../src/nest/plugins/host/plugin-host-deps.factory'; import { getPluginDataDb, closePluginDataDb } from '../../../src/nest/plugins/host/plugin-host-state'; @@ -291,12 +292,13 @@ import type { TagsService } from '../../../src/nest/tags/tags.service'; import type { CategoriesService } from '../../../src/nest/categories/categories.service'; import type { TodoService } from '../../../src/nest/todo/todo.service'; import type { PackingService } from '../../../src/nest/packing/packing.service'; +import type { DayNotesService } from '../../../src/nest/days/day-notes.service'; import type { PluginOAuthService } from '../../../src/nest/plugins/plugin-oauth.service'; // The factory under test, wired exactly like PluginsModule does — but with the // DI-native domain services replaced by the stubs above. The shim keeps the // ~45 historical call sites unchanged and supplies a default no-op router. -const factory = new PluginHostDepsFactory(budgetStub, reservationsStub, tagsStub, categoriesStub, todoStub, packingStub, oauthStub); +const factory = new PluginHostDepsFactory(budgetStub, reservationsStub, tagsStub, categoriesStub, todoStub, packingStub, oauthStub, dayNotesStub); const stubRouter: PluginCallRouter = { callPlugin: async () => undefined, emitPluginEvent: () => {} }; const createRealRpcHost = (id: string, granted: ReadonlySet, router: PluginCallRouter = stubRouter) => factory.create(id, granted, router);