diff --git a/server/src/mcp/resources.ts b/server/src/mcp/resources.ts index 2fa140c0a..ea723be7e 100644 --- a/server/src/mcp/resources.ts +++ b/server/src/mcp/resources.ts @@ -8,7 +8,6 @@ import { listItems as listPackingItems, listBags } from '../services/packingServ import { listReservations } from '../services/reservationService'; import { listNotes as listDayNotes } from '../services/dayNoteService'; import { listNotes as listCollabNotes, listPolls, listMessages } from '../services/collabService'; -import { listItems as listTodoItems } from '../services/todoService'; import { listBucketList, listVisitedCountries, getStats as getAtlasStats, listManuallyVisitedRegions } from '../services/atlasService'; import { getNotifications } from '../services/inAppNotifications'; import { getActivePlanId, getActivePlan, getPlanData, getEntries as getVacayEntries, getHolidays } from '../services/vacayService'; @@ -201,18 +200,9 @@ export function registerResources(server: McpServer, userId: number, scopes: str } ); - // Trip to-do list - if (isAddonEnabled(ADDON_IDS.PACKING) && canRead(scopes, 'todos')) server.registerResource( - 'trip-todos', - new ResourceTemplate('trek://trips/{tripId}/todos', { list: undefined }), - { description: 'To-do items for a trip, ordered by position', mimeType: 'application/json' }, - async (uri, { tripId }) => { - const id = parseId(tripId); - if (id === null || !canAccessTrip(id, userId)) return accessDenied(uri.href); - const items = listTodoItems(id); - return jsonContent(uri.href, items); - } - ); + // The trek://trips/{tripId}/todos resource moved to the DI-discovered + // src/nest/todo/todo.mcp.ts (@ResourceTemplate, attached via the nest-mcp + // registry in registerTools). // The trek://categories resource moved to the DI-discovered // src/nest/categories/categories.mcp.ts (@Resource, attached via the diff --git a/server/src/mcp/tools.ts b/server/src/mcp/tools.ts index a1effa6bc..37cebfab9 100644 --- a/server/src/mcp/tools.ts +++ b/server/src/mcp/tools.ts @@ -1,5 +1,4 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; -import { registerTodoTools } from './tools/todos'; import { registerAssignmentTools } from './tools/assignments'; import { registerJourneyTools } from './tools/journey'; import { registerReservationTools } from './tools/reservations'; @@ -52,7 +51,8 @@ export function registerTools(server: McpServer, userId: number, scopes: string[ registerVacayTools(server, userId, scopes); - registerTodoTools(server, userId, scopes); + // The todo tools moved to the DI-discovered src/nest/todo/todo.mcp.ts + // (@McpController, attached via the nest-mcp registry below). registerMcpPrompts(server, userId, isStaticToken); diff --git a/server/src/mcp/tools/todos.ts b/server/src/mcp/tools/todos.ts deleted file mode 100644 index b7a164e36..000000000 --- a/server/src/mcp/tools/todos.ts +++ /dev/null @@ -1,199 +0,0 @@ -import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; -import { z } from 'zod'; -import { canAccessTrip } from '../../db/database'; -import { isDemoUser } from '../../services/authService'; -import { - listItems as listTodoItems, createItem as createTodoItem, updateItem as updateTodoItem, - deleteItem as deleteTodoItem, reorderItems as reorderTodoItems, - getCategoryAssignees as getTodoCategoryAssignees, updateCategoryAssignees as updateTodoCategoryAssignees, -} from '../../services/todoService'; -import { - safeBroadcast, TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, - TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, - demoDenied, noAccess, ok, hasTripPermission, permissionDenied, -} from './_shared'; -import { canRead, canWrite } from '../scopes'; -import { isAddonEnabled } from '../../services/adminService'; -import { ADDON_IDS } from '../../addons'; - -export function registerTodoTools(server: McpServer, userId: number, scopes: string[] | null): void { - const R = canRead(scopes, 'todos'); - const W = canWrite(scopes, 'todos'); - - if (!isAddonEnabled(ADDON_IDS.PACKING)) return; - - // --- TODOS --- - - if (R) server.registerTool( - 'list_todos', - { - description: 'List all to-do items for a trip, ordered by position.', - inputSchema: { - tripId: z.number().int().positive(), - }, - annotations: TOOL_ANNOTATIONS_READONLY, - }, - async ({ tripId }) => { - if (!canAccessTrip(tripId, userId)) return noAccess(); - const items = listTodoItems(tripId); - return ok({ items }); - } - ); - - if (W) server.registerTool( - 'create_todo', - { - description: 'Create a new to-do item for a trip.', - inputSchema: { - tripId: z.number().int().positive(), - name: z.string().min(1).max(500).describe('To-do item name'), - category: z.string().max(100).optional().describe('Category (e.g. "Logistics", "Booking")'), - due_date: z.string().regex(/^\d{4}-\d{2}-\d{2}$/).optional().describe('Due date (YYYY-MM-DD)'), - description: z.string().max(2000).optional().describe('Additional description'), - assigned_user_id: z.number().int().positive().optional().describe('User ID to assign this task to'), - priority: z.number().int().min(0).max(3).optional().describe('Priority: 0=none, 1=low, 2=medium, 3=high'), - }, - annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, - }, - async ({ tripId, name, category, due_date, description, assigned_user_id, priority }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); - const item = createTodoItem(tripId, { name, category, due_date, description, assigned_user_id, priority }); - safeBroadcast(tripId, 'todo:created', { item }); - return ok({ item }); - } - ); - - if (W) server.registerTool( - 'update_todo', - { - description: 'Update an existing to-do item. Only provided fields are changed; omitted fields stay as-is. Pass null to clear a nullable field.', - inputSchema: { - tripId: z.number().int().positive(), - itemId: z.number().int().positive(), - name: z.string().min(1).max(500).optional(), - category: z.string().max(100).optional(), - due_date: z.string().regex(/^\d{4}-\d{2}-\d{2}$/).nullable().optional().describe('Set to null to clear the due date'), - description: z.string().max(2000).nullable().optional().describe('Set to null to clear'), - assigned_user_id: z.number().int().positive().nullable().optional().describe('Set to null to unassign'), - priority: z.number().int().min(0).max(3).nullable().optional(), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ tripId, itemId, name, category, due_date, description, assigned_user_id, priority }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); - // Build bodyKeys to signal which nullable fields were explicitly provided - const bodyKeys: string[] = []; - if (due_date !== undefined) bodyKeys.push('due_date'); - if (description !== undefined) bodyKeys.push('description'); - if (assigned_user_id !== undefined) bodyKeys.push('assigned_user_id'); - if (priority !== undefined) bodyKeys.push('priority'); - const item = updateTodoItem(tripId, itemId, { name, category, due_date, description, assigned_user_id, priority }, bodyKeys); - if (!item) return { content: [{ type: 'text' as const, text: 'To-do item not found.' }], isError: true }; - safeBroadcast(tripId, 'todo:updated', { item }); - return ok({ item }); - } - ); - - if (W) server.registerTool( - 'toggle_todo', - { - description: 'Mark a to-do item as checked (done) or unchecked.', - inputSchema: { - tripId: z.number().int().positive(), - itemId: z.number().int().positive(), - checked: z.boolean().describe('True to mark done, false to uncheck'), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ tripId, itemId, checked }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); - const item = updateTodoItem(tripId, itemId, { checked: checked ? 1 : 0 }, []); - if (!item) return { content: [{ type: 'text' as const, text: 'To-do item not found.' }], isError: true }; - safeBroadcast(tripId, 'todo:updated', { item }); - return ok({ item }); - } - ); - - if (W) server.registerTool( - 'delete_todo', - { - description: 'Delete a to-do item.', - inputSchema: { - tripId: z.number().int().positive(), - itemId: z.number().int().positive(), - }, - annotations: TOOL_ANNOTATIONS_DELETE, - }, - async ({ tripId, itemId }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); - const deleted = deleteTodoItem(tripId, itemId); - if (!deleted) return { content: [{ type: 'text' as const, text: 'To-do item not found.' }], isError: true }; - safeBroadcast(tripId, 'todo:deleted', { itemId }); - return ok({ success: true }); - } - ); - - if (W) server.registerTool( - 'reorder_todos', - { - description: 'Reorder to-do items within a trip by providing a new ordered list of item IDs.', - inputSchema: { - tripId: z.number().int().positive(), - orderedIds: z.array(z.number().int().positive()).min(1).describe('All item IDs in the desired order'), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ tripId, orderedIds }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); - reorderTodoItems(tripId, orderedIds); - return ok({ success: true }); - } - ); - - if (R) server.registerTool( - 'get_todo_category_assignees', - { - description: 'Get the default assignees configured per to-do category for a trip.', - inputSchema: { - tripId: z.number().int().positive(), - }, - annotations: TOOL_ANNOTATIONS_READONLY, - }, - async ({ tripId }) => { - if (!canAccessTrip(tripId, userId)) return noAccess(); - const assignees = getTodoCategoryAssignees(tripId); - return ok({ assignees }); - } - ); - - if (W) server.registerTool( - 'set_todo_category_assignees', - { - description: 'Set the default assignees for a to-do category on a trip. Pass an empty array to clear.', - inputSchema: { - tripId: z.number().int().positive(), - categoryName: z.string().min(1).max(100).describe('Category name'), - userIds: z.array(z.number().int().positive()).describe('User IDs to assign as defaults for this category'), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ tripId, categoryName, userIds }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('packing_edit', tripId, userId)) return permissionDenied(); - const assignees = updateTodoCategoryAssignees(tripId, categoryName, userIds); - safeBroadcast(tripId, 'todo:assignees', { category: categoryName, assignees }); - return ok({ assignees }); - } - ); -} diff --git a/server/src/mcp/tools/trips.ts b/server/src/mcp/tools/trips.ts index e6037df8e..19f5a6c2b 100644 --- a/server/src/mcp/tools/trips.ts +++ b/server/src/mcp/tools/trips.ts @@ -17,7 +17,7 @@ import { ADDON_IDS } from '../../addons'; import { countMessages, listPolls } from '../../services/collabService'; import { listItems as listTodoItems, -} from '../../services/todoService'; +} from '../../nest/todo/todo.bridge'; import { rebaseTripCurrency } from '../../services/budgetService'; import { safeBroadcast, MAX_MCP_TRIP_DAYS, diff --git a/server/src/nest/README.md b/server/src/nest/README.md index 28288ef44..6881fd5a1 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -26,8 +26,8 @@ mount to Nest and leaves the sibling trip routes (days, places, ...) on Express. - **Phase 1 (leaf):** weather, airports, config (public), system-notices, maps, categories, tags, notifications, atlas. - **Phase 2 (trip sub-domains):** vacay (addon), packing, todo. -- **DI-native services (legacy `src/services/*` deleted):** tags, categories — - see the migration recipe below. +- **DI-native services (legacy `src/services/*` deleted):** tags, categories, + todo — see the migration recipe below. ## Cross-cutting Foundation pieces @@ -95,8 +95,11 @@ at the typed contract (Frontend Track). Pilot: **tags** (`services/tagService.ts` → `nest/tags/tags.service.ts` + `nest/tags/tags.bridge.ts`); categories followed the same shape (and piloted the -first `@Resource` in `categories.mcp.ts`). Repeat these steps per service (next -up: todoService). This is a **pure relocation** — byte-identical +first `@Resource` in `categories.mcp.ts`); todo followed too (and piloted the +first `@ResourceTemplate` plus the `when` addon gate in `todo.mcp.ts`, and the +first in-container consumer wiring: `TripsService` injects `TodoService` via +`exports: [TodoService]` instead of using the bridge). Repeat these steps per +service (next up: packingService). This is a **pure relocation** — byte-identical SQL, statuses, bodies, and error strings. The long-run plan for the last non-Nest consumer (the plugin RPC host, today served via the bridges) is in `src/nest/plugins/DI-MIGRATION.md`. diff --git a/server/src/nest/plugins/host/create-rpc-host.ts b/server/src/nest/plugins/host/create-rpc-host.ts index 4eec6e72a..b772955a3 100644 --- a/server/src/nest/plugins/host/create-rpc-host.ts +++ b/server/src/nest/plugins/host/create-rpc-host.ts @@ -8,7 +8,7 @@ import { isUpdateConflict } from '../../../services/conflictResult'; import { getWeather } from '../../../services/weatherService'; import { listCategories } from '../../categories/categories.bridge'; import { listTags, createTag, updateTag, deleteTag, getTagByIdAndUser } from '../../tags/tags.bridge'; -import { listItems as listTodosSvc, createItem as createTodoSvc, updateItem as updateTodoSvc, deleteItem as deleteTodoSvc } from '../../../services/todoService'; +import { listItems as listTodosSvc, createItem as createTodoSvc, updateItem as updateTodoSvc, deleteItem as deleteTodoSvc } from '../../todo/todo.bridge'; import { listFiles, createFile, createFileLink, getFileById, updateFile, softDeleteFile, findForeignLinkTarget, resolveFilePath, BLOCKED_EXTENSIONS, filesDir } from '../../../services/fileService'; import { createNote as createCollabNoteSvc, createPoll as createCollabPollSvc, votePoll as voteCollabPollSvc, createMessage as createCollabMessageSvc, listNotes as listCollabNotesSvc, listPolls as listCollabPollsSvc, listMessages as listCollabMessagesSvc } from '../../../services/collabService'; import { getRates as getExchangeRates } from '../../../services/exchangeRateService'; diff --git a/server/src/nest/todo/todo.bridge.ts b/server/src/nest/todo/todo.bridge.ts new file mode 100644 index 000000000..c32f31643 --- /dev/null +++ b/server/src/nest/todo/todo.bridge.ts @@ -0,0 +1,38 @@ +import { db } from '../../db/database'; +import { DatabaseService } from '../database/database.service'; +import { TodoService } from './todo.service'; + +/** + * Non-Nest entry point for the todo domain — for code running OUTSIDE the + * Nest container (the plugin RPC host and the legacy get_trip_summary + * registrar in src/mcp/tools/trips.ts; the todo MCP tools and resource moved + * to the DI-discovered todo.mcp.ts). Exports only the legacy + * services/todoService names still consumed outside the container, 1:1, so + * repointing a consumer is an import-path-only diff. Inside the container, + * inject TodoService instead. + * + * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto + * the shared better-sqlite3 singleton (same pattern as categories.bridge.ts). + */ +const todos = new TodoService(new DatabaseService(db)); + +export function listItems(tripId: string | number) { + return todos.listItems(tripId); +} + +export function createItem(tripId: string | number, data: Parameters[1]) { + return todos.createItem(tripId, data); +} + +export function updateItem( + tripId: string | number, + id: string | number, + data: Parameters[2], + bodyKeys: string[] +) { + return todos.updateItem(tripId, id, data, bodyKeys); +} + +export function deleteItem(tripId: string | number, id: string | number): boolean { + return todos.deleteItem(tripId, id); +} diff --git a/server/src/nest/todo/todo.mcp.ts b/server/src/nest/todo/todo.mcp.ts new file mode 100644 index 000000000..27a9f130a --- /dev/null +++ b/server/src/nest/todo/todo.mcp.ts @@ -0,0 +1,246 @@ +import { + McpController, Tool, ResourceTemplate, type McpContext, + TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, + TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, + demoDenied, errorResult, ok, +} from '@trek/nest-mcp'; +import { z } from 'zod'; +import { isDemoUser } from '../../services/authService'; +import { isAddonEnabled } from '../../services/adminService'; +import { ADDON_IDS } from '../../addons'; +import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { TodoService } from './todo.service'; + +/** Legacy registrar gate: the whole todo surface rides the packing addon. */ +const packingAddonOn = () => isAddonEnabled(ADDON_IDS.PACKING); + +function parseId(value: string | string[]): number | null { + const n = Number(Array.isArray(value) ? value[0] : value); + return Number.isInteger(n) && n > 0 ? n : null; +} + +/** + * Todo MCP surface — ported 1:1 from the legacy registrars: the eight tools + * from src/mcp/tools/todos.ts and the trek://trips/{tripId}/todos resource + * from src/mcp/resources.ts (identical names, descriptions, schemas, + * annotations, error/payload shapes and broadcasts). The registration-time + * gates map to `when` (the whole-registrar packing-addon early return) plus + * the declarative todos read/write access markers (the legacy `if (R)` / + * `if (W)` checks, resolved by trekMcpAccessPolicy). + */ +@McpController() +export class TodoMcp { + constructor(private readonly todos: TodoService) {} + + @Tool({ + name: 'list_todos', + description: 'List all to-do items for a trip, ordered by position.', + inputSchema: { + tripId: z.number().int().positive(), + }, + annotations: TOOL_ANNOTATIONS_READONLY, + when: packingAddonOn, + access: { group: 'todos', mode: 'read' }, + }) + async listTodos({ tripId }: { tripId: number }, ctx: McpContext) { + if (!this.todos.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + const items = this.todos.listItems(tripId); + return ok({ items }); + } + + @Tool({ + name: 'create_todo', + description: 'Create a new to-do item for a trip.', + inputSchema: { + tripId: z.number().int().positive(), + name: z.string().min(1).max(500).describe('To-do item name'), + category: z.string().max(100).optional().describe('Category (e.g. "Logistics", "Booking")'), + due_date: z.string().regex(/^\d{4}-\d{2}-\d{2}$/).optional().describe('Due date (YYYY-MM-DD)'), + description: z.string().max(2000).optional().describe('Additional description'), + assigned_user_id: z.number().int().positive().optional().describe('User ID to assign this task to'), + priority: z.number().int().min(0).max(3).optional().describe('Priority: 0=none, 1=low, 2=medium, 3=high'), + }, + annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, + when: packingAddonOn, + access: { group: 'todos', mode: 'write' }, + }) + async createTodo( + { tripId, name, category, due_date, description, assigned_user_id, priority }: { + tripId: number; name: string; category?: string; due_date?: string; description?: string; assigned_user_id?: number; priority?: number; + }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.todos.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + const item = this.todos.createItem(tripId, { name, category, due_date, description, assigned_user_id, priority }); + safeBroadcast(tripId, 'todo:created', { item }); + return ok({ item }); + } + + @Tool({ + name: 'update_todo', + description: 'Update an existing to-do item. Only provided fields are changed; omitted fields stay as-is. Pass null to clear a nullable field.', + inputSchema: { + tripId: z.number().int().positive(), + itemId: z.number().int().positive(), + name: z.string().min(1).max(500).optional(), + category: z.string().max(100).optional(), + due_date: z.string().regex(/^\d{4}-\d{2}-\d{2}$/).nullable().optional().describe('Set to null to clear the due date'), + description: z.string().max(2000).nullable().optional().describe('Set to null to clear'), + assigned_user_id: z.number().int().positive().nullable().optional().describe('Set to null to unassign'), + priority: z.number().int().min(0).max(3).nullable().optional(), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + when: packingAddonOn, + access: { group: 'todos', mode: 'write' }, + }) + async updateTodo( + { tripId, itemId, name, category, due_date, description, assigned_user_id, priority }: { + tripId: number; itemId: number; name?: string; category?: string; due_date?: string | null; description?: string | null; assigned_user_id?: number | null; priority?: number | null; + }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.todos.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + // Build bodyKeys to signal which nullable fields were explicitly provided + const bodyKeys: string[] = []; + if (due_date !== undefined) bodyKeys.push('due_date'); + if (description !== undefined) bodyKeys.push('description'); + if (assigned_user_id !== undefined) bodyKeys.push('assigned_user_id'); + if (priority !== undefined) bodyKeys.push('priority'); + const item = this.todos.updateItem(tripId, itemId, { name, category, due_date, description, assigned_user_id, priority }, bodyKeys); + if (!item) return errorResult('To-do item not found.'); + safeBroadcast(tripId, 'todo:updated', { item }); + return ok({ item }); + } + + @Tool({ + name: 'toggle_todo', + description: 'Mark a to-do item as checked (done) or unchecked.', + inputSchema: { + tripId: z.number().int().positive(), + itemId: z.number().int().positive(), + checked: z.boolean().describe('True to mark done, false to uncheck'), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + when: packingAddonOn, + access: { group: 'todos', mode: 'write' }, + }) + async toggleTodo({ tripId, itemId, checked }: { tripId: number; itemId: number; checked: boolean }, ctx: McpContext) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.todos.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + const item = this.todos.updateItem(tripId, itemId, { checked: checked ? 1 : 0 }, []); + if (!item) return errorResult('To-do item not found.'); + safeBroadcast(tripId, 'todo:updated', { item }); + return ok({ item }); + } + + @Tool({ + name: 'delete_todo', + description: 'Delete a to-do item.', + inputSchema: { + tripId: z.number().int().positive(), + itemId: z.number().int().positive(), + }, + annotations: TOOL_ANNOTATIONS_DELETE, + when: packingAddonOn, + access: { group: 'todos', mode: 'write' }, + }) + async deleteTodo({ tripId, itemId }: { tripId: number; itemId: number }, ctx: McpContext) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.todos.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + const deleted = this.todos.deleteItem(tripId, itemId); + if (!deleted) return errorResult('To-do item not found.'); + safeBroadcast(tripId, 'todo:deleted', { itemId }); + return ok({ success: true }); + } + + @Tool({ + name: 'reorder_todos', + description: 'Reorder to-do items within a trip by providing a new ordered list of item IDs.', + inputSchema: { + tripId: z.number().int().positive(), + orderedIds: z.array(z.number().int().positive()).min(1).describe('All item IDs in the desired order'), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + when: packingAddonOn, + access: { group: 'todos', mode: 'write' }, + }) + async reorderTodos({ tripId, orderedIds }: { tripId: number; orderedIds: number[] }, ctx: McpContext) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.todos.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + this.todos.reorderItems(tripId, orderedIds); + return ok({ success: true }); + } + + @Tool({ + name: 'get_todo_category_assignees', + description: 'Get the default assignees configured per to-do category for a trip.', + inputSchema: { + tripId: z.number().int().positive(), + }, + annotations: TOOL_ANNOTATIONS_READONLY, + when: packingAddonOn, + access: { group: 'todos', mode: 'read' }, + }) + async getTodoCategoryAssignees({ tripId }: { tripId: number }, ctx: McpContext) { + if (!this.todos.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + const assignees = this.todos.getCategoryAssignees(tripId); + return ok({ assignees }); + } + + @Tool({ + name: 'set_todo_category_assignees', + description: 'Set the default assignees for a to-do category on a trip. Pass an empty array to clear.', + inputSchema: { + tripId: z.number().int().positive(), + categoryName: z.string().min(1).max(100).describe('Category name'), + userIds: z.array(z.number().int().positive()).describe('User IDs to assign as defaults for this category'), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + when: packingAddonOn, + access: { group: 'todos', mode: 'write' }, + }) + async setTodoCategoryAssignees({ tripId, categoryName, userIds }: { tripId: number; categoryName: string; userIds: number[] }, ctx: McpContext) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.todos.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('packing_edit', tripId, ctx.userId)) return permissionDenied(); + const assignees = this.todos.updateCategoryAssignees(tripId, categoryName, userIds); + safeBroadcast(tripId, 'todo:assignees', { category: categoryName, assignees }); + return ok({ assignees }); + } + + @ResourceTemplate({ + name: 'trip-todos', + uriTemplate: 'trek://trips/{tripId}/todos', + description: 'To-do items for a trip, ordered by position', + mimeType: 'application/json', + when: packingAddonOn, + access: { group: 'todos', mode: 'read' }, + }) + async tripTodosResource(uri: URL, { tripId }: { tripId: string | string[] }, ctx: McpContext) { + const id = parseId(tripId); + if (id === null || !this.todos.verifyTripAccess(id, ctx.userId)) { + return { + contents: [{ + uri: uri.href, + mimeType: 'application/json', + text: JSON.stringify({ error: 'Trip not found or access denied' }), + }], + }; + } + const items = this.todos.listItems(id); + return { + contents: [{ + uri: uri.href, + mimeType: 'application/json', + text: JSON.stringify(items, null, 2), + }], + }; + } +} diff --git a/server/src/nest/todo/todo.module.ts b/server/src/nest/todo/todo.module.ts index be89ef3a3..21c23353c 100644 --- a/server/src/nest/todo/todo.module.ts +++ b/server/src/nest/todo/todo.module.ts @@ -1,10 +1,13 @@ import { Module } from '@nestjs/common'; import { TodoController } from './todo.controller'; +import { TodoMcp } from './todo.mcp'; import { TodoService } from './todo.service'; -/** To-do domain (S3 — Phase 2 trip sub-domain). Registered in AppModule. */ +/** To-do domain (S3 — Phase 2 trip sub-domain). Registered in AppModule. + * Exports TodoService for in-container consumers (TripsService bundle). */ @Module({ controllers: [TodoController], - providers: [TodoService], + providers: [TodoService, TodoMcp], + exports: [TodoService], }) export class TodoModule {} diff --git a/server/src/nest/todo/todo.service.ts b/server/src/nest/todo/todo.service.ts index 055b35a60..96a96fea1 100644 --- a/server/src/nest/todo/todo.service.ts +++ b/server/src/nest/todo/todo.service.ts @@ -1,20 +1,27 @@ import { Injectable } from '@nestjs/common'; import { broadcast } from '../../websocket'; import { checkPermission } from '../../services/permissions'; +import { verifyTripAccess } from '../../services/tripAccess'; import type { User } from '../../types'; -import * as svc from '../../services/todoService'; +import { DatabaseService } from '../database/database.service'; -type Trip = NonNullable>; +type Trip = NonNullable>; /** - * Thin Nest wrapper around the existing todo service. Trip-access, the - * 'packing_edit' permission (shared with packing), the SQL and the WebSocket - * broadcasts all reuse the legacy code unchanged. + * Todo domain service — owns the todo SQL (moved 1:1 from the legacy + * services/todoService.ts: identical statements, the `||` falsy-coercion + * defaults, the bodyKeys sentinel protocol on update and the post-write + * re-selects). Trip access, the 'packing_edit' permission (shared with + * packing) and the WebSocket broadcast keep their legacy call paths. + * Non-Nest consumers (plugin RPC host, the legacy MCP trips registrar) go + * through todo.bridge.ts instead of importing this class directly. */ @Injectable() export class TodoService { - verifyTripAccess(tripId: string, userId: number) { - return svc.verifyTripAccess(tripId, userId); + constructor(private readonly db: DatabaseService) {} + + verifyTripAccess(tripId: string | number, userId: number) { + return verifyTripAccess(tripId, userId); } canEdit(trip: Trip, user: User): boolean { @@ -25,31 +32,113 @@ export class TodoService { broadcast(tripId, event, payload, socketId); } - listItems(tripId: string) { - return svc.listItems(tripId); + listItems(tripId: string | number) { + return this.db.all( + 'SELECT * FROM todo_items WHERE trip_id = ? ORDER BY sort_order ASC, created_at ASC', + tripId + ); } - createItem(tripId: string, data: Parameters[1]) { - return svc.createItem(tripId, data); + createItem(tripId: string | number, data: { + name: string; category?: string; due_date?: string; description?: string; assigned_user_id?: number; priority?: number; + }) { + const maxOrder = this.db.get<{ max: number | null }>('SELECT MAX(sort_order) as max FROM todo_items WHERE trip_id = ?', tripId)!; + const sortOrder = (maxOrder.max !== null ? maxOrder.max : -1) + 1; + + const result = this.db.run( + 'INSERT INTO todo_items (trip_id, name, checked, category, sort_order, due_date, description, assigned_user_id, priority) VALUES (?, ?, 0, ?, ?, ?, ?, ?, ?)', + tripId, data.name, data.category || null, sortOrder, + data.due_date || null, data.description || null, data.assigned_user_id || null, data.priority || 0 + ); + + return this.db.get('SELECT * FROM todo_items WHERE id = ?', result.lastInsertRowid); } - updateItem(tripId: string, id: string, data: Parameters[2], changedKeys: string[]) { - return svc.updateItem(tripId, id, data, changedKeys); + updateItem( + tripId: string | number, + id: string | number, + data: { name?: string; checked?: number; category?: string; due_date?: string | null; description?: string | null; assigned_user_id?: number | null; priority?: number | null }, + bodyKeys: string[] + ) { + const item = this.db.get('SELECT * FROM todo_items WHERE id = ? AND trip_id = ?', id, tripId); + if (!item) return null; + + this.db.run(` + UPDATE todo_items SET + name = COALESCE(?, name), + checked = CASE WHEN ? IS NOT NULL THEN ? ELSE checked END, + category = COALESCE(?, category), + due_date = CASE WHEN ? THEN ? ELSE due_date END, + description = CASE WHEN ? THEN ? ELSE description END, + assigned_user_id = CASE WHEN ? THEN ? ELSE assigned_user_id END, + priority = CASE WHEN ? THEN ? ELSE priority END + WHERE id = ? + `, + data.name || null, + data.checked !== undefined ? 1 : null, + data.checked ? 1 : 0, + data.category || null, + bodyKeys.includes('due_date') ? 1 : 0, + data.due_date ?? null, + bodyKeys.includes('description') ? 1 : 0, + data.description ?? null, + bodyKeys.includes('assigned_user_id') ? 1 : 0, + data.assigned_user_id ?? null, + bodyKeys.includes('priority') ? 1 : 0, + data.priority ?? 0, + id + ); + + return this.db.get('SELECT * FROM todo_items WHERE id = ?', id); } - deleteItem(tripId: string, id: string): boolean { - return svc.deleteItem(tripId, id); + deleteItem(tripId: string | number, id: string | number): boolean { + const item = this.db.get('SELECT id FROM todo_items WHERE id = ? AND trip_id = ?', id, tripId); + if (!item) return false; + + this.db.run('DELETE FROM todo_items WHERE id = ?', id); + return true; } - reorderItems(tripId: string, orderedIds: Parameters[1]): void { - svc.reorderItems(tripId, orderedIds); + reorderItems(tripId: string | number, orderedIds: number[]): void { + const update = this.db.prepare('UPDATE todo_items SET sort_order = ? WHERE id = ? AND trip_id = ?'); + this.db.transaction(() => { + orderedIds.forEach((id, index) => { + update.run(index, id, tripId); + }); + }); } - getCategoryAssignees(tripId: string) { - return svc.getCategoryAssignees(tripId); + getCategoryAssignees(tripId: string | number) { + const rows = this.db.all<{ category_name: string; user_id: number; username: string; avatar: string | null }>(` + SELECT tca.category_name, tca.user_id, u.username, u.avatar + FROM todo_category_assignees tca + JOIN users u ON tca.user_id = u.id + WHERE tca.trip_id = ? + `, tripId); + + const assignees: Record = {}; + for (const row of rows) { + if (!assignees[row.category_name]) assignees[row.category_name] = []; + assignees[row.category_name].push({ user_id: row.user_id, username: row.username, avatar: row.avatar }); + } + + return assignees; } - updateCategoryAssignees(tripId: string, category: string, userIds: number[]) { - return svc.updateCategoryAssignees(tripId, category, userIds); + updateCategoryAssignees(tripId: string | number, categoryName: string, userIds: number[] | undefined) { + this.db.run('DELETE FROM todo_category_assignees WHERE trip_id = ? AND category_name = ?', tripId, categoryName); + + if (Array.isArray(userIds) && userIds.length > 0) { + const insert = this.db.prepare('INSERT OR IGNORE INTO todo_category_assignees (trip_id, category_name, user_id) VALUES (?, ?, ?)'); + for (const uid of userIds) insert.run(tripId, categoryName, uid); + } + + return this.db.all(` + SELECT tca.user_id, u.username, u.avatar + FROM todo_category_assignees tca + JOIN users u ON tca.user_id = u.id + WHERE tca.trip_id = ? AND tca.category_name = ? + `, tripId, categoryName); } } diff --git a/server/src/nest/trips/trips.module.ts b/server/src/nest/trips/trips.module.ts index e1a97d786..6238f0155 100644 --- a/server/src/nest/trips/trips.module.ts +++ b/server/src/nest/trips/trips.module.ts @@ -1,10 +1,12 @@ import { Module } from '@nestjs/common'; +import { TodoModule } from '../todo/todo.module'; import { TripsController } from './trips.controller'; import { TripsService } from './trips.service'; /** Trips aggregate root (C1 — Phase 3). Uses exact strangler prefixes so it does * not capture the nested sub-domain mounts (collab, files, ...). */ @Module({ + imports: [TodoModule], controllers: [TripsController], providers: [TripsService], }) diff --git a/server/src/nest/trips/trips.service.ts b/server/src/nest/trips/trips.service.ts index 75fceb86f..05ba56731 100644 --- a/server/src/nest/trips/trips.service.ts +++ b/server/src/nest/trips/trips.service.ts @@ -7,7 +7,7 @@ import * as tripSvc from '../../services/tripService'; import { listDays, listAccommodations } from '../../services/dayService'; import { listPlaces } from '../../services/placeService'; import { listItems as listPackingItems } from '../../services/packingService'; -import { listItems as listTodoItems } from '../../services/todoService'; +import { TodoService } from '../todo/todo.service'; import { listBudgetItems, rebaseTripCurrency } from '../../services/budgetService'; import { listReservations } from '../../services/reservationService'; import { listFiles } from '../../services/fileService'; @@ -22,7 +22,10 @@ import { searchUnsplashPhotos, getUnsplashKey } from '../../services/unsplashSer */ @Injectable() export class TripsService { - constructor(private readonly dbs: DatabaseService) {} + constructor( + private readonly dbs: DatabaseService, + private readonly todo: TodoService, + ) {} private get db() { return this.dbs.connection; @@ -134,7 +137,7 @@ export class TripsService { days, places: listPlaces(String(tripId), {}), packingItems: listPackingItems(tripId), - todoItems: listTodoItems(tripId), + todoItems: this.todo.listItems(tripId), budgetItems: listBudgetItems(tripId), reservations: listReservations(tripId), files: listFiles(tripId, false), diff --git a/server/src/services/todoService.ts b/server/src/services/todoService.ts deleted file mode 100644 index d86c541d6..000000000 --- a/server/src/services/todoService.ts +++ /dev/null @@ -1,120 +0,0 @@ -import { db } from '../db/database'; - -export { verifyTripAccess } from './tripAccess'; - -// ── Items ────────────────────────────────────────────────────────────────── - -export function listItems(tripId: string | number) { - return db.prepare( - 'SELECT * FROM todo_items WHERE trip_id = ? ORDER BY sort_order ASC, created_at ASC' - ).all(tripId); -} - -export function createItem(tripId: string | number, data: { - name: string; category?: string; due_date?: string; description?: string; assigned_user_id?: number; priority?: number; -}) { - const maxOrder = db.prepare('SELECT MAX(sort_order) as max FROM todo_items WHERE trip_id = ?').get(tripId) as { max: number | null }; - const sortOrder = (maxOrder.max !== null ? maxOrder.max : -1) + 1; - - const result = db.prepare( - 'INSERT INTO todo_items (trip_id, name, checked, category, sort_order, due_date, description, assigned_user_id, priority) VALUES (?, ?, 0, ?, ?, ?, ?, ?, ?)' - ).run( - tripId, data.name, data.category || null, sortOrder, - data.due_date || null, data.description || null, data.assigned_user_id || null, data.priority || 0 - ); - - return db.prepare('SELECT * FROM todo_items WHERE id = ?').get(result.lastInsertRowid); -} - -export function updateItem( - tripId: string | number, - id: string | number, - data: { name?: string; checked?: number; category?: string; due_date?: string | null; description?: string | null; assigned_user_id?: number | null; priority?: number | null }, - bodyKeys: string[] -) { - const item = db.prepare('SELECT * FROM todo_items WHERE id = ? AND trip_id = ?').get(id, tripId); - if (!item) return null; - - db.prepare(` - UPDATE todo_items SET - name = COALESCE(?, name), - checked = CASE WHEN ? IS NOT NULL THEN ? ELSE checked END, - category = COALESCE(?, category), - due_date = CASE WHEN ? THEN ? ELSE due_date END, - description = CASE WHEN ? THEN ? ELSE description END, - assigned_user_id = CASE WHEN ? THEN ? ELSE assigned_user_id END, - priority = CASE WHEN ? THEN ? ELSE priority END - WHERE id = ? - `).run( - data.name || null, - data.checked !== undefined ? 1 : null, - data.checked ? 1 : 0, - data.category || null, - bodyKeys.includes('due_date') ? 1 : 0, - data.due_date ?? null, - bodyKeys.includes('description') ? 1 : 0, - data.description ?? null, - bodyKeys.includes('assigned_user_id') ? 1 : 0, - data.assigned_user_id ?? null, - bodyKeys.includes('priority') ? 1 : 0, - data.priority ?? 0, - id - ); - - return db.prepare('SELECT * FROM todo_items WHERE id = ?').get(id); -} - -export function deleteItem(tripId: string | number, id: string | number) { - const item = db.prepare('SELECT id FROM todo_items WHERE id = ? AND trip_id = ?').get(id, tripId); - if (!item) return false; - - db.prepare('DELETE FROM todo_items WHERE id = ?').run(id); - return true; -} - -// ── Category Assignees ───────────────────────────────────────────────────── - -export function getCategoryAssignees(tripId: string | number) { - const rows = db.prepare(` - SELECT tca.category_name, tca.user_id, u.username, u.avatar - FROM todo_category_assignees tca - JOIN users u ON tca.user_id = u.id - WHERE tca.trip_id = ? - `).all(tripId); - - const assignees: Record = {}; - for (const row of rows as any[]) { - if (!assignees[row.category_name]) assignees[row.category_name] = []; - assignees[row.category_name].push({ user_id: row.user_id, username: row.username, avatar: row.avatar }); - } - - return assignees; -} - -export function updateCategoryAssignees(tripId: string | number, categoryName: string, userIds: number[] | undefined) { - db.prepare('DELETE FROM todo_category_assignees WHERE trip_id = ? AND category_name = ?').run(tripId, categoryName); - - if (Array.isArray(userIds) && userIds.length > 0) { - const insert = db.prepare('INSERT OR IGNORE INTO todo_category_assignees (trip_id, category_name, user_id) VALUES (?, ?, ?)'); - for (const uid of userIds) insert.run(tripId, categoryName, uid); - } - - return db.prepare(` - SELECT tca.user_id, u.username, u.avatar - FROM todo_category_assignees tca - JOIN users u ON tca.user_id = u.id - WHERE tca.trip_id = ? AND tca.category_name = ? - `).all(tripId, categoryName); -} - -// ── Reorder ──────────────────────────────────────────────────────────────── - -export function reorderItems(tripId: string | number, orderedIds: number[]) { - const update = db.prepare('UPDATE todo_items SET sort_order = ? WHERE id = ? AND trip_id = ?'); - const updateMany = db.transaction((ids: number[]) => { - ids.forEach((id, index) => { - update.run(index, id, tripId); - }); - }); - updateMany(orderedIds); -} diff --git a/server/tests/e2e/todo.e2e.test.ts b/server/tests/e2e/todo.e2e.test.ts index 0174f4722..025927611 100644 --- a/server/tests/e2e/todo.e2e.test.ts +++ b/server/tests/e2e/todo.e2e.test.ts @@ -1,7 +1,10 @@ /** * To-do module e2e — exercises the migrated /api/trips/:tripId/todo endpoints - * through the real JwtAuthGuard against a temp SQLite db. todoService, the - * permission check and the WebSocket broadcast are mocked. + * through the real JwtAuthGuard against a temp SQLite db. TodoService runs its + * real SQL via DatabaseModule (the DATABASE_CONNECTION factory picks up the + * mocked db singleton); trip access resolves through a real-SQL canAccessTrip + * over the temp db. Only the permission check and the WebSocket broadcast stay + * mocked. */ import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; import request from 'supertest'; @@ -16,33 +19,71 @@ const { db } = vi.hoisted(() => { const tmp = new Database(':memory:'); 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);`); + email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0, + avatar TEXT);`); + tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT);'); + tmp.exec('CREATE TABLE trip_members (trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL);'); + tmp.exec(`CREATE TABLE todo_items ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + trip_id INTEGER NOT NULL, + name TEXT NOT NULL, + checked INTEGER NOT NULL DEFAULT 0, + category TEXT, + sort_order INTEGER NOT NULL DEFAULT 0, + due_date TEXT, + description TEXT, + assigned_user_id INTEGER, + priority INTEGER NOT NULL DEFAULT 0, + created_at DATETIME DEFAULT CURRENT_TIMESTAMP + );`); + tmp.exec(`CREATE TABLE todo_category_assignees ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + trip_id INTEGER NOT NULL, + category_name TEXT NOT NULL, + user_id INTEGER NOT NULL, + UNIQUE(trip_id, category_name, user_id) + );`); return { db: tmp }; }); -vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: () => {} })); +vi.mock('../../src/db/database', () => ({ + db, + // Real-SQL trip access over the temp db — TodoService.verifyTripAccess and + // DatabaseModule both read the mocked singleton. + 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: () => false, + getPlaceWithTags: () => null, + closeDb: () => {}, + reinitialize: () => {}, +})); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); vi.mock('../../src/services/permissions', () => ({ checkPermission })); -const { svc } = vi.hoisted(() => ({ - svc: { - verifyTripAccess: vi.fn(), listItems: vi.fn(), createItem: vi.fn(), updateItem: vi.fn(), - deleteItem: vi.fn(), reorderItems: vi.fn(), getCategoryAssignees: vi.fn(), updateCategoryAssignees: vi.fn(), - }, -})); -vi.mock('../../src/services/todoService', () => svc); - import { TodoModule } from '../../src/nest/todo/todo.module'; +import { DatabaseModule } from '../../src/nest/database/database.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; -describe('To-do e2e (real auth guard + temp SQLite)', () => { +function insertItem(tripId: number, name: string, extra: Partial<{ sort_order: number; due_date: string }> = {}): number { + const res = db + .prepare('INSERT INTO todo_items (trip_id, name, sort_order, due_date) VALUES (?, ?, ?, ?)') + .run(tripId, name, extra.sort_order ?? 0, extra.due_date ?? null); + return Number(res.lastInsertRowid); +} + +describe('To-do e2e (real auth guard + real SQL over temp SQLite)', () => { let server: Server; let app: Awaited>; + let tripId: number; async function build() { - const moduleRef = await Test.createTestingModule({ imports: [TodoModule] }).compile(); + const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, TodoModule] }).compile(); const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalFilters(new TrekExceptionFilter()); @@ -52,14 +93,17 @@ describe('To-do e2e (real auth guard + temp SQLite)', () => { beforeAll(async () => { seedUser(db as never, { id: 1 }); + seedUser(db as never, { id: 2, email: 'stranger@example.test' }); app = await build(); server = app.getHttpServer(); - svc.listItems.mockReturnValue([{ id: 1, name: 'Book hotel' }]); - svc.createItem.mockReturnValue({ id: 9, name: 'Book hotel' }); }); beforeEach(() => { - svc.verifyTripAccess.mockReturnValue({ id: 5, user_id: 1 }); + db.exec('DELETE FROM todo_category_assignees'); + db.exec('DELETE FROM todo_items'); + db.exec('DELETE FROM trip_members'); + db.exec('DELETE FROM trips'); + tripId = Number(db.prepare('INSERT INTO trips (user_id, title) VALUES (1, ?)').run('Trip').lastInsertRowid); checkPermission.mockReturnValue(true); }); @@ -68,33 +112,98 @@ describe('To-do e2e (real auth guard + temp SQLite)', () => { }); it('401 without a session cookie', async () => { - const res = await request(server).get('/api/trips/5/todo'); + const res = await request(server).get(`/api/trips/${tripId}/todo`); expect(res.status).toBe(401); }); - it('200 list for an accessible trip', async () => { - const res = await request(server).get('/api/trips/5/todo').set('Cookie', sessionCookie(1)); + it('200 list ordered by sort_order', async () => { + insertItem(tripId, 'Second', { sort_order: 1 }); + insertItem(tripId, 'First', { sort_order: 0 }); + const res = await request(server).get(`/api/trips/${tripId}/todo`).set('Cookie', sessionCookie(1)); expect(res.status).toBe(200); - expect(res.body).toEqual({ items: [{ id: 1, name: 'Book hotel' }] }); + expect(res.body.items.map((i: { name: string }) => i.name)).toEqual(['First', 'Second']); }); it('404 when the trip is not accessible', async () => { - svc.verifyTripAccess.mockReturnValue(undefined); - const res = await request(server).get('/api/trips/5/todo').set('Cookie', sessionCookie(1)); + const res = await request(server).get(`/api/trips/${tripId}/todo`).set('Cookie', sessionCookie(2)); expect(res.status).toBe(404); expect(res.body).toEqual({ error: 'Trip not found' }); }); - it('201 on create with permission', async () => { - const res = await request(server).post('/api/trips/5/todo').set('Cookie', sessionCookie(1)).send({ name: 'Book hotel' }); + it('201 on create, inserting the row with the legacy defaults and incrementing sort_order', async () => { + insertItem(tripId, 'Existing', { sort_order: 4 }); + const res = await request(server).post(`/api/trips/${tripId}/todo`).set('Cookie', sessionCookie(1)).send({ name: 'Book hotel' }); expect(res.status).toBe(201); - expect(res.body).toEqual({ item: { id: 9, name: 'Book hotel' } }); + expect(res.body.item).toMatchObject({ name: 'Book hotel', checked: 0, priority: 0, sort_order: 5 }); + expect(db.prepare('SELECT name FROM todo_items WHERE id = ?').get(res.body.item.id)).toEqual({ name: 'Book hotel' }); }); - it('403 on create without permission', async () => { + it('400 on create without a name', async () => { + const res = await request(server).post(`/api/trips/${tripId}/todo`).set('Cookie', sessionCookie(1)).send({}); + expect(res.status).toBe(400); + expect(res.body).toEqual({ error: 'Item name is required' }); + }); + + it('403 on create without permission, writing nothing', async () => { checkPermission.mockReturnValue(false); - const res = await request(server).post('/api/trips/5/todo').set('Cookie', sessionCookie(1)).send({ name: 'Book hotel' }); + const res = await request(server).post(`/api/trips/${tripId}/todo`).set('Cookie', sessionCookie(1)).send({ name: 'X' }); expect(res.status).toBe(403); expect(res.body).toEqual({ error: 'No permission' }); + expect((db.prepare('SELECT COUNT(*) AS n FROM todo_items').get() as { n: number }).n).toBe(0); + }); + + it('200 on update; a body key with null clears the field, omitted keys stay', async () => { + const id = insertItem(tripId, 'Task', { due_date: '2026-06-01' }); + const renamed = await request(server).put(`/api/trips/${tripId}/todo/${id}`).set('Cookie', sessionCookie(1)).send({ name: 'Renamed' }); + expect(renamed.status).toBe(200); + expect(renamed.body.item).toMatchObject({ id, name: 'Renamed', due_date: '2026-06-01' }); + const cleared = await request(server).put(`/api/trips/${tripId}/todo/${id}`).set('Cookie', sessionCookie(1)).send({ due_date: null }); + expect(cleared.status).toBe(200); + expect(cleared.body.item.due_date).toBeNull(); + }); + + it('404 on update of a missing item', async () => { + const res = await request(server).put(`/api/trips/${tripId}/todo/9999`).set('Cookie', sessionCookie(1)).send({ name: 'X' }); + expect(res.status).toBe(404); + expect(res.body).toEqual({ error: 'Item not found' }); + }); + + it('200 on reorder, persisting the new sort_order', async () => { + const a = insertItem(tripId, 'A', { sort_order: 0 }); + const b = insertItem(tripId, 'B', { sort_order: 1 }); + const res = await request(server).put(`/api/trips/${tripId}/todo/reorder`).set('Cookie', sessionCookie(1)).send({ orderedIds: [b, a] }); + expect(res.status).toBe(200); + expect(res.body).toEqual({ success: true }); + const rows = db.prepare('SELECT id FROM todo_items WHERE trip_id = ? ORDER BY sort_order').all(tripId) as { id: number }[]; + expect(rows.map((r) => r.id)).toEqual([b, a]); + }); + + it('200 on delete, removing the row; 404 when already gone', async () => { + const id = insertItem(tripId, 'Gone'); + const ok = await request(server).delete(`/api/trips/${tripId}/todo/${id}`).set('Cookie', sessionCookie(1)); + expect(ok.status).toBe(200); + expect(ok.body).toEqual({ success: true }); + expect(db.prepare('SELECT id FROM todo_items WHERE id = ?').get(id)).toBeUndefined(); + const missing = await request(server).delete(`/api/trips/${tripId}/todo/${id}`).set('Cookie', sessionCookie(1)); + expect(missing.status).toBe(404); + expect(missing.body).toEqual({ error: 'Item not found' }); + }); + + it('category assignees round-trip: PUT replaces, GET groups by category', async () => { + db.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, ?)').run(tripId, 2); + const put = await request(server) + .put(`/api/trips/${tripId}/todo/category-assignees/Booking`) + .set('Cookie', sessionCookie(1)) + .send({ user_ids: [1, 2] }); + expect(put.status).toBe(200); + expect(put.body.assignees).toHaveLength(2); + const get = await request(server).get(`/api/trips/${tripId}/todo/category-assignees`).set('Cookie', sessionCookie(1)); + expect(get.status).toBe(200); + expect(get.body.assignees.Booking).toHaveLength(2); + const replaced = await request(server) + .put(`/api/trips/${tripId}/todo/category-assignees/Booking`) + .set('Cookie', sessionCookie(1)) + .send({ user_ids: [2] }); + expect(replaced.body.assignees).toEqual([{ user_id: 2, username: 'e2e-user', avatar: null }]); }); }); diff --git a/server/tests/e2e/trips.e2e.test.ts b/server/tests/e2e/trips.e2e.test.ts index 7b59d47a7..fd1051643 100644 --- a/server/tests/e2e/trips.e2e.test.ts +++ b/server/tests/e2e/trips.e2e.test.ts @@ -20,6 +20,11 @@ const { db } = vi.hoisted(() => { 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);`); tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, title TEXT);'); + // bundle()'s todoItems now runs TodoService's real SQL (DI-injected, no mock). + tmp.exec(`CREATE TABLE todo_items (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, + name TEXT NOT NULL, checked INTEGER NOT NULL DEFAULT 0, category TEXT, sort_order INTEGER NOT NULL DEFAULT 0, + due_date TEXT, description TEXT, assigned_user_id INTEGER, priority INTEGER NOT NULL DEFAULT 0, + created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`); return { db: tmp }; }); @@ -47,7 +52,6 @@ vi.mock('../../src/services/tripService', () => tripSvc); vi.mock('../../src/services/dayService', () => ({ listDays: () => ({ days: [] }), listAccommodations: () => [] })); vi.mock('../../src/services/placeService', () => ({ listPlaces: () => [] })); vi.mock('../../src/services/packingService', () => ({ listItems: () => [] })); -vi.mock('../../src/services/todoService', () => ({ listItems: () => [] })); vi.mock('../../src/services/budgetService', () => ({ listBudgetItems: () => [] })); vi.mock('../../src/services/reservationService', () => ({ listReservations: () => [] })); vi.mock('../../src/services/fileService', () => ({ listFiles: () => [] })); diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index 351b376f3..4c8d98f69 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -6,6 +6,8 @@ import { CategoriesService } from '../../src/nest/categories/categories.service' import { DatabaseService } from '../../src/nest/database/database.service'; import { TagsMcp } from '../../src/nest/tags/tags.mcp'; import { TagsService } from '../../src/nest/tags/tags.service'; +import { TodoMcp } from '../../src/nest/todo/todo.mcp'; +import { TodoService } from '../../src/nest/todo/todo.service'; /** * Hand-wired counterpart of the boot-time discovery in McpRegistryService, @@ -20,6 +22,7 @@ export function createMcpTestRegistry(): McpRegistry { [ new TagsMcp(new TagsService(dbService)), new CategoriesMcp(new CategoriesService(dbService)), + new TodoMcp(new TodoService(dbService)), ], { accessPolicy: trekMcpAccessPolicy }, ); diff --git a/server/tests/unit/mcp/resources.test.ts b/server/tests/unit/mcp/resources.test.ts index 4e312d0b1..f90e5b4c9 100644 --- a/server/tests/unit/mcp/resources.test.ts +++ b/server/tests/unit/mcp/resources.test.ts @@ -1,8 +1,9 @@ /** * Unit tests for MCP resources (resources.ts). - * Tests all 13 legacy-registrar resources via InMemoryTransport + Client + * Tests all 12 legacy-registrar resources via InMemoryTransport + Client * (trek://categories moved to the DI-discovered CategoriesMcp — see - * tools-categories.test.ts). + * tools-categories.test.ts; trek://trips/{tripId}/todos moved to TodoMcp — + * see tools-todos.test.ts). */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll, afterEach } from 'vitest'; diff --git a/server/tests/unit/mcp/tools-todos.test.ts b/server/tests/unit/mcp/tools-todos.test.ts index 79bd1ebd9..ec2fbfe34 100644 --- a/server/tests/unit/mcp/tools-todos.test.ts +++ b/server/tests/unit/mcp/tools-todos.test.ts @@ -1,7 +1,12 @@ /** - * Unit tests for MCP todo tools: + * Unit tests for the todo MCP surface (TodoMcp, DI-discovered): * create_todo, update_todo, toggle_todo, delete_todo, reorder_todos, - * list_todos, get_todo_category_assignees, set_todo_category_assignees. + * list_todos, get_todo_category_assignees, set_todo_category_assignees, + * plus the trek://trips/{tripId}/todos resource. + * + * All of it attaches via the nest-mcp registry inside registerTools, so every + * harness here keeps withTools on (the resource is NOT registered by the + * legacy registerResources fan-out anymore). */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; @@ -38,7 +43,8 @@ import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, createTodoItem } from '../../helpers/factories'; -import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness'; +import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness'; +import { ADDON_IDS } from '../../../src/addons'; beforeAll(() => { createTables(testDb); @@ -436,3 +442,101 @@ describe('Tool: set_todo_category_assignees', () => { }); }); }); + +// --------------------------------------------------------------------------- +// Scope gating (todos read/write, registration-time) +// --------------------------------------------------------------------------- + +describe('Todo tools — scope gating', () => { + const READ_TOOLS = ['list_todos', 'get_todo_category_assignees']; + const WRITE_TOOLS = ['create_todo', 'update_todo', 'toggle_todo', 'delete_todo', 'reorder_todos', 'set_todo_category_assignees']; + + 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 eight tools with null scopes (full access)', async () => { + const { user } = createUser(testDb); + const names = await listToolNames(user.id, null); + for (const tool of [...READ_TOOLS, ...WRITE_TOOLS]) expect(names).toContain(tool); + }); + + it('registers only the read tools with todos:read', async () => { + const { user } = createUser(testDb); + const names = await listToolNames(user.id, ['todos:read']); + for (const tool of READ_TOOLS) expect(names).toContain(tool); + for (const tool of WRITE_TOOLS) expect(names).not.toContain(tool); + }); + + it('registers no todo tools for an unrelated scope', async () => { + const { user } = createUser(testDb); + const names = await listToolNames(user.id, ['budget:read']); + for (const tool of [...READ_TOOLS, ...WRITE_TOOLS]) expect(names).not.toContain(tool); + }); +}); + +// --------------------------------------------------------------------------- +// Addon gating (packing addon, the legacy whole-registrar early return — +// now the `when` predicate on every TodoMcp entry) +// --------------------------------------------------------------------------- + +describe('Todo tools — packing addon gating', () => { + it('registers nothing (tools or resource) when the packing addon is disabled', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + testDb.prepare('UPDATE addons SET enabled = 0 WHERE id = ?').run(ADDON_IDS.PACKING); + try { + await withHarness(user.id, async (h) => { + const names = (await h.client.listTools()).tools.map((t) => t.name); + expect(names).not.toContain('list_todos'); + expect(names).not.toContain('create_todo'); + await expect(h.client.readResource({ uri: `trek://trips/${trip.id}/todos` })).rejects.toThrow(); + }); + } finally { + testDb.prepare('UPDATE addons SET enabled = 1 WHERE id = ?').run(ADDON_IDS.PACKING); + } + }); +}); + +// --------------------------------------------------------------------------- +// trek://trips/{tripId}/todos resource (moved from the legacy registerResources) +// --------------------------------------------------------------------------- + +describe('Resource: trek://trips/{tripId}/todos', () => { + it('returns the trip todos ordered by position', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + createTodoItem(testDb, trip.id, { name: 'First' }); + createTodoItem(testDb, trip.id, { name: 'Second' }); + await withHarness(user.id, async (h) => { + const result = await h.client.readResource({ uri: `trek://trips/${trip.id}/todos` }); + const items = parseResourceResult(result) as any[]; + expect(items).toHaveLength(2); + expect(items[0].name).toBe('First'); + expect(items[1].name).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); + await withHarness(user.id, async (h) => { + const result = await h.client.readResource({ uri: `trek://trips/${trip.id}/todos` }); + expect(parseResourceResult(result)).toEqual({ error: 'Trip not found or access denied' }); + }); + }); + + it('returns the access-denied payload for a malformed trip id', async () => { + const { user } = createUser(testDb); + await withHarness(user.id, async (h) => { + const result = await h.client.readResource({ uri: 'trek://trips/not-a-number/todos' }); + expect(parseResourceResult(result)).toEqual({ error: 'Trip not found or access denied' }); + }); + }); +}); diff --git a/server/tests/unit/services/todoService.test.ts b/server/tests/unit/nest/todo.service.test.ts similarity index 65% rename from server/tests/unit/services/todoService.test.ts rename to server/tests/unit/nest/todo.service.test.ts index 49613a371..88d899334 100644 --- a/server/tests/unit/services/todoService.test.ts +++ b/server/tests/unit/nest/todo.service.test.ts @@ -1,6 +1,8 @@ /** - * Unit tests for todoService — TODO-SVC-001 through TODO-SVC-020. - * Uses a real in-memory SQLite DB so SQL logic is exercised faithfully. + * Unit tests for the DI-native TodoService — TODO-SVC-001 through TODO-SVC-024 + * (001–020 moved 1:1 from the legacy tests/unit/services/todoService.test.ts; + * 021–024 pin the todo.bridge delegation). Uses a real in-memory SQLite DB so + * SQL logic is exercised faithfully. */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; @@ -35,21 +37,17 @@ vi.mock('../../../src/config', () => ({ 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, addTripMember } from '../../helpers/factories'; -import { - verifyTripAccess, - listItems, - createItem, - updateItem, - deleteItem, - getCategoryAssignees, - updateCategoryAssignees, - reorderItems, -} from '../../../src/services/todoService'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { TodoService } from '../../../src/nest/todo/todo.service'; +import { listItems, createItem, updateItem, deleteItem } from '../../../src/nest/todo/todo.bridge'; + +const svc = new TodoService(new DatabaseService(testDb)); beforeAll(() => { createTables(testDb); @@ -70,16 +68,16 @@ describe('verifyTripAccess', () => { it('TODO-SVC-001: returns trip for owner', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const result = verifyTripAccess(trip.id, user.id); + const result = svc.verifyTripAccess(trip.id, user.id); expect(result).toBeDefined(); - expect((result as any).id).toBe(trip.id); + expect(result?.id).toBe(trip.id); }); it('TODO-SVC-002: returns null for non-member', () => { const { user: owner } = createUser(testDb); const { user: stranger } = createUser(testDb); const trip = createTrip(testDb, owner.id); - expect(verifyTripAccess(trip.id, stranger.id)).toBeFalsy(); + expect(svc.verifyTripAccess(trip.id, stranger.id)).toBeFalsy(); }); it('TODO-SVC-003: returns trip for member', () => { @@ -87,7 +85,7 @@ describe('verifyTripAccess', () => { const { user: member } = createUser(testDb); const trip = createTrip(testDb, owner.id); addTripMember(testDb, trip.id, member.id); - const result = verifyTripAccess(trip.id, member.id); + const result = svc.verifyTripAccess(trip.id, member.id); expect(result).toBeDefined(); }); }); @@ -98,13 +96,13 @@ describe('listItems and createItem', () => { it('TODO-SVC-004: listItems returns empty array for new trip', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - expect(listItems(trip.id)).toEqual([]); + expect(svc.listItems(trip.id)).toEqual([]); }); it('TODO-SVC-005: createItem inserts a todo with name only', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const item = createItem(trip.id, { name: 'Buy snacks' }) as any; + const item = svc.createItem(trip.id, { name: 'Buy snacks' }) as any; expect(item).toBeDefined(); expect(item.name).toBe('Buy snacks'); expect(item.checked).toBe(0); @@ -115,15 +113,15 @@ describe('listItems and createItem', () => { it('TODO-SVC-006: createItem assigns incrementing sort_order', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const a = createItem(trip.id, { name: 'A' }) as any; - const b = createItem(trip.id, { name: 'B' }) as any; + const a = svc.createItem(trip.id, { name: 'A' }) as any; + const b = svc.createItem(trip.id, { name: 'B' }) as any; expect(b.sort_order).toBe(a.sort_order + 1); }); it('TODO-SVC-007: createItem stores optional fields', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const item = createItem(trip.id, { + const item = svc.createItem(trip.id, { name: 'Pack bag', category: 'Prep', description: 'All the gear', @@ -137,10 +135,10 @@ describe('listItems and createItem', () => { it('TODO-SVC-008: listItems returns items ordered by sort_order', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - createItem(trip.id, { name: 'First' }); - createItem(trip.id, { name: 'Second' }); - createItem(trip.id, { name: 'Third' }); - const items = listItems(trip.id) as any[]; + svc.createItem(trip.id, { name: 'First' }); + svc.createItem(trip.id, { name: 'Second' }); + svc.createItem(trip.id, { name: 'Third' }); + const items = svc.listItems(trip.id) as any[]; expect(items).toHaveLength(3); expect(items[0].sort_order).toBeLessThanOrEqual(items[1].sort_order); expect(items[1].sort_order).toBeLessThanOrEqual(items[2].sort_order); @@ -153,24 +151,24 @@ describe('updateItem', () => { it('TODO-SVC-009: returns null for non-existent item', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - expect(updateItem(trip.id, 99999, { name: 'Ghost' }, ['name'])).toBeNull(); + expect(svc.updateItem(trip.id, 99999, { name: 'Ghost' }, ['name'])).toBeNull(); }); it('TODO-SVC-010: toggles checked status', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const item = createItem(trip.id, { name: 'Visit museum' }) as any; - const updated = updateItem(trip.id, item.id, { checked: 1 }, ['checked']) as any; + const item = svc.createItem(trip.id, { name: 'Visit museum' }) as any; + const updated = svc.updateItem(trip.id, item.id, { checked: 1 }, ['checked']) as any; expect(updated.checked).toBe(1); - const back = updateItem(trip.id, item.id, { checked: 0 }, ['checked']) as any; + const back = svc.updateItem(trip.id, item.id, { checked: 0 }, ['checked']) as any; expect(back.checked).toBe(0); }); it('TODO-SVC-011: updates name and category', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const item = createItem(trip.id, { name: 'Old' }) as any; - const updated = updateItem(trip.id, item.id, { name: 'New', category: 'Misc' }, ['name', 'category']) as any; + const item = svc.createItem(trip.id, { name: 'Old' }) as any; + const updated = svc.updateItem(trip.id, item.id, { name: 'New', category: 'Misc' }, ['name', 'category']) as any; expect(updated.name).toBe('New'); expect(updated.category).toBe('Misc'); }); @@ -178,8 +176,8 @@ describe('updateItem', () => { it('TODO-SVC-012: clears due_date when key is present with null value', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const item = createItem(trip.id, { name: 'Task', due_date: '2026-06-01' }) as any; - const updated = updateItem(trip.id, item.id, { due_date: null }, ['due_date']) as any; + const item = svc.createItem(trip.id, { name: 'Task', due_date: '2026-06-01' }) as any; + const updated = svc.updateItem(trip.id, item.id, { due_date: null }, ['due_date']) as any; expect(updated.due_date).toBeNull(); }); }); @@ -190,15 +188,15 @@ describe('deleteItem', () => { it('TODO-SVC-013: returns false for non-existent item', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - expect(deleteItem(trip.id, 99999)).toBe(false); + expect(svc.deleteItem(trip.id, 99999)).toBe(false); }); it('TODO-SVC-014: deletes item and returns true', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const item = createItem(trip.id, { name: 'Gone' }) as any; - expect(deleteItem(trip.id, item.id)).toBe(true); - expect(listItems(trip.id)).toHaveLength(0); + const item = svc.createItem(trip.id, { name: 'Gone' }) as any; + expect(svc.deleteItem(trip.id, item.id)).toBe(true); + expect(svc.listItems(trip.id)).toHaveLength(0); }); }); @@ -208,11 +206,11 @@ describe('reorderItems', () => { it('TODO-SVC-015: assigns sort_order matching orderedIds array position', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - const a = createItem(trip.id, { name: 'A' }) as any; - const b = createItem(trip.id, { name: 'B' }) as any; - const c = createItem(trip.id, { name: 'C' }) as any; + const a = svc.createItem(trip.id, { name: 'A' }) as any; + const b = svc.createItem(trip.id, { name: 'B' }) as any; + const c = svc.createItem(trip.id, { name: 'C' }) as any; - reorderItems(trip.id, [c.id, a.id, b.id]); + svc.reorderItems(trip.id, [c.id, a.id, b.id]); const rows = testDb.prepare('SELECT id, sort_order FROM todo_items WHERE trip_id = ? ORDER BY sort_order').all(trip.id) as any[]; expect(rows[0].id).toBe(c.id); @@ -227,7 +225,7 @@ describe('getCategoryAssignees / updateCategoryAssignees', () => { it('TODO-SVC-016: returns empty object for new trip', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - expect(getCategoryAssignees(trip.id)).toEqual({}); + expect(svc.getCategoryAssignees(trip.id)).toEqual({}); }); it('TODO-SVC-017: updateCategoryAssignees sets assignees for a category', () => { @@ -236,10 +234,10 @@ describe('getCategoryAssignees / updateCategoryAssignees', () => { const trip = createTrip(testDb, owner.id); addTripMember(testDb, trip.id, member.id); - const rows = updateCategoryAssignees(trip.id, 'Packing', [owner.id, member.id]) as any[]; + const rows = svc.updateCategoryAssignees(trip.id, 'Packing', [owner.id, member.id]); expect(rows).toHaveLength(2); - const assignees = getCategoryAssignees(trip.id) as any; + const assignees = svc.getCategoryAssignees(trip.id); expect(assignees['Packing']).toHaveLength(2); }); @@ -247,11 +245,11 @@ describe('getCategoryAssignees / updateCategoryAssignees', () => { const { user: owner } = createUser(testDb); const trip = createTrip(testDb, owner.id); - updateCategoryAssignees(trip.id, 'Packing', [owner.id]); - const cleared = updateCategoryAssignees(trip.id, 'Packing', []) as any[]; + svc.updateCategoryAssignees(trip.id, 'Packing', [owner.id]); + const cleared = svc.updateCategoryAssignees(trip.id, 'Packing', []); expect(cleared).toHaveLength(0); - const assignees = getCategoryAssignees(trip.id) as any; + const assignees = svc.getCategoryAssignees(trip.id); expect(assignees['Packing']).toBeUndefined(); }); @@ -261,10 +259,10 @@ describe('getCategoryAssignees / updateCategoryAssignees', () => { const trip = createTrip(testDb, owner.id); addTripMember(testDb, trip.id, member.id); - updateCategoryAssignees(trip.id, 'Shopping', [owner.id]); - updateCategoryAssignees(trip.id, 'Logistics', [member.id]); + svc.updateCategoryAssignees(trip.id, 'Shopping', [owner.id]); + svc.updateCategoryAssignees(trip.id, 'Logistics', [member.id]); - const assignees = getCategoryAssignees(trip.id) as any; + const assignees = svc.getCategoryAssignees(trip.id); expect(Object.keys(assignees)).toHaveLength(2); expect(assignees['Shopping']).toHaveLength(1); expect(assignees['Logistics']).toHaveLength(1); @@ -276,12 +274,53 @@ describe('getCategoryAssignees / updateCategoryAssignees', () => { const trip = createTrip(testDb, owner.id); addTripMember(testDb, trip.id, member.id); - updateCategoryAssignees(trip.id, 'Food', [owner.id, member.id]); + svc.updateCategoryAssignees(trip.id, 'Food', [owner.id, member.id]); // Replace with just owner - updateCategoryAssignees(trip.id, 'Food', [owner.id]); + svc.updateCategoryAssignees(trip.id, 'Food', [owner.id]); - const assignees = getCategoryAssignees(trip.id) as any; + const assignees = svc.getCategoryAssignees(trip.id); expect(assignees['Food']).toHaveLength(1); expect(assignees['Food'][0].user_id).toBe(owner.id); }); }); + +// ── todo.bridge (delegation to the service — inside the src/nest coverage gate) + +describe('todo.bridge', () => { + it('TODO-SVC-021 — listItems delegates to the service', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + svc.createItem(trip.id, { name: 'Bridged' }); + const items = listItems(trip.id) as any[]; + expect(items).toHaveLength(1); + expect(items[0].name).toBe('Bridged'); + }); + + it('TODO-SVC-022 — createItem delegates with the legacy defaults', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const item = createItem(trip.id, { name: 'Via bridge' }) as any; + expect(item.checked).toBe(0); + expect(item.priority).toBe(0); + expect(item.sort_order).toBe(0); + }); + + it('TODO-SVC-023 — updateItem delegates and keeps the bodyKeys sentinel protocol', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const item = createItem(trip.id, { name: 'Task', due_date: '2026-06-01' }) as any; + const untouched = updateItem(trip.id, item.id, { name: 'Renamed' }, []) as any; + expect(untouched.due_date).toBe('2026-06-01'); + const cleared = updateItem(trip.id, item.id, { due_date: null }, ['due_date']) as any; + expect(cleared.due_date).toBeNull(); + }); + + it('TODO-SVC-024 — deleteItem delegates to the service', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const item = createItem(trip.id, { name: 'Gone' }) as any; + expect(deleteItem(trip.id, item.id)).toBe(true); + expect(deleteItem(trip.id, item.id)).toBe(false); + expect(svc.listItems(trip.id)).toHaveLength(0); + }); +}); diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index 2396baea7..58e89dcf6 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -28,7 +28,6 @@ vi.mock('../../../src/services/tripService', () => tripSvc); vi.mock('../../../src/services/dayService', () => ({ listDays: () => ({ days: [1] }), listAccommodations: () => [] })); vi.mock('../../../src/services/placeService', () => ({ listPlaces: () => [] })); vi.mock('../../../src/services/packingService', () => ({ listItems: () => [] })); -vi.mock('../../../src/services/todoService', () => ({ listItems: () => [] })); vi.mock('../../../src/services/budgetService', () => ({ listBudgetItems: () => [], rebaseTripCurrency: vi.fn(async () => {}), @@ -37,9 +36,11 @@ vi.mock('../../../src/services/reservationService', () => ({ listReservations: ( vi.mock('../../../src/services/fileService', () => ({ listFiles: () => [] })); import { TripsService } from '../../../src/nest/trips/trips.service'; +import type { TodoService } from '../../../src/nest/todo/todo.service'; import { rebaseTripCurrency } from '../../../src/services/budgetService'; -function svc() { return new TripsService(new DatabaseService(dbConn)); } +const todoStub = { listItems: () => [] } as unknown as TodoService; +function svc() { return new TripsService(new DatabaseService(dbConn), todoStub); } beforeEach(() => vi.clearAllMocks()); describe('TripsService (wrapper delegation + bundle/copy/notify helpers)', () => { diff --git a/server/tests/unit/plugins/create-rpc-host.test.ts b/server/tests/unit/plugins/create-rpc-host.test.ts index cb2dc2a18..05e0d4e4c 100644 --- a/server/tests/unit/plugins/create-rpc-host.test.ts +++ b/server/tests/unit/plugins/create-rpc-host.test.ts @@ -144,7 +144,7 @@ vi.mock('../../../src/nest/tags/tags.bridge', () => ({ updateTag: vi.fn((tagId: number, name?: string) => ({ id: Number(tagId), name })), deleteTag: vi.fn(), })); -vi.mock('../../../src/services/todoService', () => ({ +vi.mock('../../../src/nest/todo/todo.bridge', () => ({ listItems: vi.fn((tid: number) => [{ id: 1, trip_id: Number(tid), name: 'Pack' }]), createItem: vi.fn((tid: number, data: { name: string }) => ({ id: 90, trip_id: Number(tid), name: data.name })), updateItem: vi.fn((_tid: number, id: string) => (Number(id) === 404 ? null : { id: Number(id), name: 'Done' })),