From 0c5843f00e74a172f1ef6ca8a8b667527fe0dd73 Mon Sep 17 00:00:00 2001 From: jubnl Date: Wed, 23 Sep 2026 19:32:26 +0200 Subject: [PATCH] refactor(todo): todo items through repositories --- .../TodoCategoryAssignees.repository.ts | 77 ++++++++- .../db/repositories/TodoItems.repository.ts | 160 +++++++++++++++++- server/src/nest/todo/todo.module.ts | 11 +- server/src/nest/todo/todo.service.ts | 109 +++++------- server/tests/helpers/todo-repos.ts | 28 +++ server/tests/unit/nest/todo.service.test.ts | 54 +++++- 6 files changed, 368 insertions(+), 71 deletions(-) create mode 100644 server/tests/helpers/todo-repos.ts diff --git a/server/src/db/repositories/TodoCategoryAssignees.repository.ts b/server/src/db/repositories/TodoCategoryAssignees.repository.ts index d01c4d8a2..3dd941076 100644 --- a/server/src/db/repositories/TodoCategoryAssignees.repository.ts +++ b/server/src/db/repositories/TodoCategoryAssignees.repository.ts @@ -1,4 +1,79 @@ import type { TodoCategoryAssignees } from '../entities/TodoCategoryAssignees.entity'; import { TrekRepository } from './_shared/trek-repository'; -export class TodoCategoryAssigneesRepository extends TrekRepository {} +/** TD11/TD14's joined projection — the assignee row plus the display fields the picker needs. */ +export interface TodoCategoryAssigneeRow { + user_id: number; + username: string; + avatar: string | null; +} + +interface TodoCategoryAssigneesKyselyDB { + todo_category_assignees: { id: number; trip_id: number; category_name: string; user_id: number }; + users: { id: number; username: string; avatar: string | null }; +} + +interface TodoCategoryAssigneesInsertKyselyDB { + todo_category_assignees: { trip_id: number | string; category_name: string; user_id: number }; +} + +/** + * `todo_category_assignees` — which trip members are pinned to a to-do + * category, driving the picker `TodoService.getCategoryAssignees`/ + * `updateCategoryAssignees` render. Kysely throughout: `trip_id`/`user_id` + * are `persist(false)` relation mirrors (the program-wide trap), and the + * `INSERT OR IGNORE` write has no single `em.upsert`-expressible conflict + * target worth reaching for over Kysely's `ON CONFLICT DO NOTHING` (the + * `BudgetItemMembersRepository.insertIgnore` precedent) — the entity's own + * three-column unique (`trip`, `category_name`, `user`) is the constraint + * SQLite's `INSERT OR IGNORE` already ignores a conflict against. + */ +export class TodoCategoryAssigneesRepository extends TrekRepository { + /** TD11 (`getCategoryAssignees`) — `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 = ?`. */ + async listForTrip(trip_id: number | string): Promise<(TodoCategoryAssigneeRow & { category_name: string })[]> { + return await this.kysely() + .selectFrom('todo_category_assignees as tca') + .innerJoin('users as u', 'u.id', 'tca.user_id') + .select(['tca.category_name', 'tca.user_id', 'u.username', 'u.avatar']) + .where('tca.trip_id', '=', trip_id as number) + .execute(); + } + + /** TD12 (`updateCategoryAssignees`) — `DELETE FROM todo_category_assignees WHERE trip_id = ? AND category_name = ?` (replace-all before a re-insert). */ + async deleteForCategory(trip_id: number | string, category_name: string): Promise { + await this.kysely() + .deleteFrom('todo_category_assignees') + .where('trip_id', '=', trip_id as number) + .where('category_name', '=', category_name) + .execute(); + } + + /** + * TD13 (`updateCategoryAssignees`, looped, roster-filtered) — `INSERT OR + * IGNORE INTO todo_category_assignees (trip_id, category_name, user_id) + * VALUES (?, ?, ?)`. The service filters `userIds` against + * `DatabaseService.rosterUserIds` before calling this once per surviving + * id — dropped rather than rejected: a copied trip carries assignee ids + * across before its members exist, and a 400 would make the picker + * unusable there. Kept exactly (`TodoService.updateCategoryAssignees`'s + * own comment). + */ + async insertIgnore(trip_id: number | string, category_name: string, user_id: number): Promise { + await this.kysely() + .insertInto('todo_category_assignees') + .values({ trip_id: trip_id as number, category_name, user_id }) + .onConflict((oc) => oc.doNothing()) + .execute(); + } + + /** TD14 (`updateCategoryAssignees`'s post-write re-select) — `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 = ?`. */ + async listForCategory(trip_id: number | string, category_name: string): Promise { + return await this.kysely() + .selectFrom('todo_category_assignees as tca') + .innerJoin('users as u', 'u.id', 'tca.user_id') + .select(['tca.user_id', 'u.username', 'u.avatar']) + .where('tca.trip_id', '=', trip_id as number) + .where('tca.category_name', '=', category_name) + .execute(); + } +} diff --git a/server/src/db/repositories/TodoItems.repository.ts b/server/src/db/repositories/TodoItems.repository.ts index 129da71d5..291309b21 100644 --- a/server/src/db/repositories/TodoItems.repository.ts +++ b/server/src/db/repositories/TodoItems.repository.ts @@ -1,4 +1,162 @@ import type { TodoItems } from '../entities/TodoItems.entity'; +import { type AssertRowKeys } from './_shared/rows'; import { TrekRepository } from './_shared/trek-repository'; +import { presenceSet } from './_shared/presence-set'; -export class TodoItemsRepository extends TrekRepository {} +/** A bare `todo_items` row — every scalar column, incl. the two `persist(false)` relation mirrors (`trip_id`, `assigned_user_id`). */ +export interface TodoItemRow { + id: number; + trip_id: number; + name: string; + checked: number | null; + category: string | null; + sort_order: number | null; + due_date: string | null; + description: string | null; + assigned_user_id: number | null; + priority: number | null; + created_at: string | null; + reminded_at: string | null; +} + +const _todoItemRowKeys: AssertRowKeys = true; + +interface TodoItemsKyselyDB { + todo_items: TodoItemRow; +} + +/** The insert-only shape for `insertItem` (TD3) — omits `id`/`created_at`/`reminded_at` (autoincrement / `DEFAULT CURRENT_TIMESTAMP` / never written here). */ +interface TodoItemsInsertKyselyDB { + todo_items: { + trip_id: number | string; name: string; checked: number; category: string | null; sort_order: number; + due_date: string | null; description: string | null; assigned_user_id: number | null; priority: number; + }; +} + +/** The insert-only shape for `insertCopy` (TP73) — a distinct column set from {@link TodoItemsRepository.insertItem}'s: no `assigned_user_id` parameter at all (always written `NULL`, never taken from the caller). */ +interface TodoItemsCopyInsertKyselyDB { + todo_items: { + trip_id: number | string; name: string; checked: number; category: string | null; sort_order: number | null; + due_date: string | null; description: string | null; assigned_user_id: null; priority: number | null; + }; +} + +/** + * `todo_items` — Kysely throughout: `trip_id`/`assigned_user_id` are + * `persist(false)` relation mirrors (the program-wide bare-`select`/`fields` + * drop trap — `TripFilesRepository`'s class docstring, restated at Task 2's + * `BudgetItemsRepository`/Task 6's TC-form), avoided the same way here by + * going through Kysely for every statement rather than the QueryBuilder. + */ +export class TodoItemsRepository extends TrekRepository { + /** TD1 (`listItems`) — `SELECT * FROM todo_items WHERE trip_id = ? ORDER BY sort_order ASC, created_at ASC`. */ + async listForTrip(trip_id: number | string): Promise { + return await this.kysely() + .selectFrom('todo_items') + .selectAll() + .where('trip_id', '=', trip_id as number) + .orderBy('sort_order', 'asc') + .orderBy('created_at', 'asc') + .execute(); + } + + /** TD2 (`createItem`) — `SELECT MAX(sort_order) as max FROM todo_items WHERE trip_id = ?`. */ + async maxSortOrder(trip_id: number | string): Promise { + const row = await this.kysely().selectFrom('todo_items').select((eb) => eb.fn.max('sort_order').as('max')).where('trip_id', '=', trip_id as number).executeTakeFirst(); + return row?.max ?? null; + } + + /** + * TD3 (`createItem`) — `INSERT INTO todo_items (trip_id, name, checked, + * category, sort_order, due_date, description, assigned_user_id, priority) + * VALUES (?, ?, 0, ?, ?, ?, ?, ?, ?)`. Returns the new row's id (TD4's + * re-select key). Named `insertItem`, not `create`/`insert` — both collide + * with `TrekRepository`/`EntityRepository`'s own same-named methods (a + * different signature: an unpersisted-entity builder, not an INSERT). + */ + async insertItem(row: { + trip_id: number | string; name: string; category: string | null; sort_order: number; + due_date: string | null; description: string | null; assigned_user_id: number | null; priority: number; + }): Promise { + const result = await this.kysely() + .insertInto('todo_items') + .values({ ...row, checked: 0 }) + .executeTakeFirstOrThrow(); + return Number(result.insertId); + } + + /** TD4/TD7 (`createItem`/`updateItem`'s post-write re-select) — `SELECT * FROM todo_items WHERE id = ?` (no trip filter — the caller already knows `id` is in-scope, having just written it). */ + async findById(id: number | string): Promise { + return await this.kysely().selectFrom('todo_items').selectAll().where('id', '=', id as number).executeTakeFirst(); + } + + /** TD5 (`updateItem`'s trip-scoping guard) — `SELECT * FROM todo_items WHERE id = ? AND trip_id = ?`. */ + async findInTrip(id: number | string, trip_id: number | string): Promise { + return await this.kysely().selectFrom('todo_items').selectAll().where('id', '=', id as number).where('trip_id', '=', trip_id as number).executeTakeFirst(); + } + + /** TD8 (`deleteItem`'s trip-scoping guard) — `SELECT id FROM todo_items WHERE id = ? AND trip_id = ?`. */ + async existsInTrip(id: number | string, trip_id: number | string): Promise<{ id: number } | undefined> { + return await this.kysely().selectFrom('todo_items').select('id').where('id', '=', id as number).where('trip_id', '=', trip_id as number).executeTakeFirst(); + } + + /** + * TD6 (`updateItem`) — the 7-column presence-sentinel `UPDATE` (R11's + * helper — {@link presenceSet}, copying Task 2's `BudgetItemsRepository + * .update` shape). `name`/`category` keep the legacy `COALESCE(?, col)` + * truthy-wins semantics (the caller passes `present = !!value`, matching + * `data.name || null` / `data.category || null`'s fall-through-on-falsy + * exactly); every other column is a true presence sentinel bound off the + * legacy `bodyKeys` array (`present = bodyKeys.includes('')`). + */ + async update(id: number | string, write: { + name?: readonly [present: boolean, value: string]; + checked?: readonly [present: boolean, value: number]; + category?: readonly [present: boolean, value: string | null]; + due_date?: readonly [present: boolean, value: string | null]; + description?: readonly [present: boolean, value: string | null]; + assigned_user_id?: readonly [present: boolean, value: number | null]; + priority?: readonly [present: boolean, value: number]; + }): Promise { + const data = presenceSet<{ + name: string; checked: number; category: string | null; due_date: string | null; + description: string | null; assigned_user_id: number | null; priority: number; + }>(write); + if (Object.keys(data).length === 0) return; + await this.kysely().updateTable('todo_items').set(data).where('id', '=', id as number).execute(); + } + + /** TD9 (`deleteItem`) — `DELETE FROM todo_items WHERE id = ?` (no trip scoping in the statement itself — relies on the caller's prior {@link existsInTrip} gate, matching legacy). */ + async deleteById(id: number | string): Promise { + await this.kysely().deleteFrom('todo_items').where('id', '=', id as number).execute(); + } + + /** TD10 (`reorderItems`, looped) — `UPDATE todo_items SET sort_order = ? WHERE id = ? AND trip_id = ?`. */ + async setSortOrder(id: number | string, trip_id: number | string, sort_order: number): Promise { + await this.kysely().updateTable('todo_items').set({ sort_order }).where('id', '=', id as number).where('trip_id', '=', trip_id as number).execute(); + } + + /** TP72 (`TripsService.copy`) — `SELECT * FROM todo_items WHERE trip_id = ?`. */ + async listAllForTrip(trip_id: number | string): Promise { + return await this.kysely().selectFrom('todo_items').selectAll().where('trip_id', '=', trip_id as number).execute(); + } + + /** + * TP73 (`TripsService.copy`) — `INSERT INTO todo_items (trip_id, name, + * checked, category, sort_order, due_date, description, assigned_user_id, + * priority) VALUES (?, ?, 0, ?, ?, ?, ?, NULL, ?)`. `assigned_user_id` is + * hard-coded `NULL` here, never taken from the caller — the copy + * deliberately does not carry the old assignee across, since the new trip + * has no roster yet (`TripsService.copy`'s own comment, kept exactly). + */ + async insertCopy(row: { + trip_id: number | string; name: string; category: string | null; sort_order: number | null; + due_date: string | null; description: string | null; priority: number | null; + }): Promise { + const result = await this.kysely() + .insertInto('todo_items') + .values({ ...row, checked: 0, assigned_user_id: null }) + .executeTakeFirstOrThrow(); + return Number(result.insertId); + } +} diff --git a/server/src/nest/todo/todo.module.ts b/server/src/nest/todo/todo.module.ts index 94d17170e..196287368 100644 --- a/server/src/nest/todo/todo.module.ts +++ b/server/src/nest/todo/todo.module.ts @@ -1,4 +1,5 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { TodoController } from './todo.controller'; import { TodoMcp } from './todo.mcp'; import { TodoService } from './todo.service'; @@ -9,11 +10,17 @@ import { PermissionsModule } from '../permissions/permissions.module'; import { AuthModule } from '../auth/auth.module'; import { AddonsModule } from '../addons/addons.module'; import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; +import { TodoItems } from '../../db/entities/TodoItems.entity'; +import { TodoCategoryAssignees } from '../../db/entities/TodoCategoryAssignees.entity'; /** To-do domain (S3 — Phase 2 trip sub-domain). Registered in AppModule. - * Exports TodoService for in-container consumers (TripsService bundle). */ + * Exports TodoService for in-container consumers (TripsService bundle). + * `MikroOrmModule.forFeature([TodoItems, TodoCategoryAssignees])` (Plan 3e + * Task 4) registers `TodoItemsRepository`/`TodoCategoryAssigneesRepository` + * for `TodoService`'s `@InjectRepository` constructor params — the + * `CollabModule`/`FilesModule` precedent. */ @Module({ - imports: [McpSharedModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule], + imports: [MikroOrmModule.forFeature([TodoItems, TodoCategoryAssignees]), McpSharedModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule], controllers: [TodoController], providers: [TodoService, TodoMcp, TodoRpc], exports: [TodoService], diff --git a/server/src/nest/todo/todo.service.ts b/server/src/nest/todo/todo.service.ts index 6ff92eae3..1fcdb51cc 100644 --- a/server/src/nest/todo/todo.service.ts +++ b/server/src/nest/todo/todo.service.ts @@ -1,19 +1,27 @@ import { Injectable } from '@nestjs/common'; +import { InjectRepository } from '@mikro-orm/nestjs'; import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; import type { User } from '../../types'; import { DatabaseService, type TripAccess } from '../database/database.service'; import { UnitOfWork } from '../database/unit-of-work'; +import { TodoItems } from '../../db/entities/TodoItems.entity'; +import type { TodoItemsRepository } from '../../db/repositories/TodoItems.repository'; +import { TodoCategoryAssignees } from '../../db/entities/TodoCategoryAssignees.entity'; +import type { TodoCategoryAssigneesRepository } from '../../db/repositories/TodoCategoryAssignees.repository'; type Trip = TripAccess; /** - * 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. + * Todo domain service — owns the todo SQL, now through + * `TodoItemsRepository`/`TodoCategoryAssigneesRepository` (Plan 3e Task 4, + * moved 1:1 off the legacy raw statements: identical column sets, 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), the roster-filter on category assignees (kept + * unconverted on `DatabaseService.rosterUserIds`, the `BudgetService` + * precedent) 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. */ @@ -24,6 +32,8 @@ export class TodoService { private readonly permissions: PermissionsService, private readonly realtime: RealtimeService, private readonly uow: UnitOfWork, + @InjectRepository(TodoItems) private readonly todoItemsRepo: TodoItemsRepository, + @InjectRepository(TodoCategoryAssignees) private readonly todoCategoryAssigneesRepo: TodoCategoryAssigneesRepository, ) {} async verifyTripAccess(tripId: string | number, userId: number) { @@ -39,25 +49,22 @@ export class TodoService { } async listItems(tripId: string | number) { - return this.db.all( - 'SELECT * FROM todo_items WHERE trip_id = ? ORDER BY sort_order ASC, created_at ASC', - tripId - ); + return this.todoItemsRepo.listForTrip(tripId); } async createItem(tripId: string | number, data: { name: string; category?: string | null; due_date?: string | null; description?: string | null; assigned_user_id?: number | null; 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 maxOrder = await this.todoItemsRepo.maxSortOrder(tripId); + const sortOrder = (maxOrder !== null ? maxOrder : -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 - ); + const id = await this.todoItemsRepo.insertItem({ + trip_id: tripId, name: data.name, category: data.category || null, sort_order: sortOrder, + due_date: data.due_date || null, description: data.description || null, + assigned_user_id: data.assigned_user_id || null, priority: data.priority || 0, + }); - return this.db.get('SELECT * FROM todo_items WHERE id = ?', result.lastInsertRowid); + return this.todoItemsRepo.findById(id); } async updateItem( @@ -66,62 +73,40 @@ export class TodoService { data: { name?: string; checked?: number; category?: string | null; 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); + const item = await this.todoItemsRepo.findInTrip(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 - ); + await this.todoItemsRepo.update(id, { + name: [!!data.name, data.name || ''], + checked: [data.checked !== undefined, data.checked ? 1 : 0], + category: [!!data.category, data.category || ''], + due_date: [bodyKeys.includes('due_date'), data.due_date ?? null], + description: [bodyKeys.includes('description'), data.description ?? null], + assigned_user_id: [bodyKeys.includes('assigned_user_id'), data.assigned_user_id ?? null], + priority: [bodyKeys.includes('priority'), data.priority ?? 0], + }); - return this.db.get('SELECT * FROM todo_items WHERE id = ?', id); + return this.todoItemsRepo.findById(id); } async deleteItem(tripId: string | number, id: string | number): Promise { - const item = this.db.get('SELECT id FROM todo_items WHERE id = ? AND trip_id = ?', id, tripId); + const item = await this.todoItemsRepo.existsInTrip(id, tripId); if (!item) return false; - this.db.run('DELETE FROM todo_items WHERE id = ?', id); + await this.todoItemsRepo.deleteById(id); return true; } async reorderItems(tripId: string | number, orderedIds: number[]): Promise { - const update = this.db.prepare('UPDATE todo_items SET sort_order = ? WHERE id = ? AND trip_id = ?'); await this.uow.transactional(async () => { - orderedIds.forEach((id, index) => { - update.run(index, id, tripId); - }); + for (let index = 0; index < orderedIds.length; index++) { + await this.todoItemsRepo.setSortOrder(orderedIds[index], tripId, index); + } }); } async 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 rows = await this.todoCategoryAssigneesRepo.listForTrip(tripId); const assignees: Record = {}; for (const row of rows) { @@ -134,24 +119,18 @@ export class TodoService { async updateCategoryAssignees(tripId: string | number, categoryName: string, userIds: number[] | undefined) { await this.uow.transactional(async () => { - this.db.run('DELETE FROM todo_category_assignees WHERE trip_id = ? AND category_name = ?', tripId, categoryName); + await this.todoCategoryAssigneesRepo.deleteForCategory(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 (?, ?, ?)'); // Only people on this trip may be assigned, the way packing filters bag // members and reservations filter travellers. Dropped rather than // rejected: a copied trip carries assignee ids across before its members // exist, and a 400 would make the picker unusable there. const roster = await this.db.rosterUserIds(tripId); - for (const uid of userIds) if (roster.has(uid)) insert.run(tripId, categoryName, uid); + for (const uid of userIds) if (roster.has(uid)) await this.todoCategoryAssigneesRepo.insertIgnore(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); + return this.todoCategoryAssigneesRepo.listForCategory(tripId, categoryName); } } diff --git a/server/tests/helpers/todo-repos.ts b/server/tests/helpers/todo-repos.ts new file mode 100644 index 000000000..1c9d46fcc --- /dev/null +++ b/server/tests/helpers/todo-repos.ts @@ -0,0 +1,28 @@ +import type Database from 'better-sqlite3'; +import { sharedTestOrm } from './test-uow'; +import { TodoItems } from '../../src/db/entities/TodoItems.entity'; +import type { TodoItemsRepository } from '../../src/db/repositories/TodoItems.repository'; +import { TodoCategoryAssignees } from '../../src/db/entities/TodoCategoryAssignees.entity'; +import type { TodoCategoryAssigneesRepository } from '../../src/db/repositories/TodoCategoryAssignees.repository'; + +/** + * Plan 3e Task 4 (`TodoService`) test-only repository factories, bound to a + * suite's own better-sqlite3 handle via the SAME memoised `sharedTestOrm` + * `test-uow.ts` exports — required so a repository built here and a + * `UnitOfWork` built from `createTestUnitOfWork` (that file) resolve the + * IDENTICAL context-resolving `EntityManager` (the `files-repos.ts`/ + * `collab-repos.ts` precedent this file otherwise copies verbatim: a + * repository from a SECOND, independent ORM instance would write outside the + * open transaction). + * + * A NEW file rather than an addition to `test-uow.ts`, same reasoning those + * two files give for themselves: avoids colliding with a concurrently + * in-flight task's edits to that shared file. + */ +export function createTestTodoItemsRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(TodoItems)); +} + +export function createTestTodoCategoryAssigneesRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(TodoCategoryAssignees)); +} diff --git a/server/tests/unit/nest/todo.service.test.ts b/server/tests/unit/nest/todo.service.test.ts index 283a4a812..84ad5f8b5 100644 --- a/server/tests/unit/nest/todo.service.test.ts +++ b/server/tests/unit/nest/todo.service.test.ts @@ -49,11 +49,15 @@ import { PermissionsService } from '../../../src/nest/permissions/permissions.se import { TodoService } from '../../../src/nest/todo/todo.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { createTestUnitOfWork, createTestAppSettingsRepo, createTestDatabaseService } from '../../helpers/test-uow'; +import { createTestTodoItemsRepo, createTestTodoCategoryAssigneesRepo } from '../../helpers/todo-repos'; let svc: TodoService; beforeAll(async () => { const uow = await createTestUnitOfWork(testDb); - svc = new TodoService(await createTestDatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), uow), new RealtimeService(), uow); + svc = new TodoService( + await createTestDatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), uow), new RealtimeService(), uow, + await createTestTodoItemsRepo(testDb), await createTestTodoCategoryAssigneesRepo(testDb), + ); }); beforeAll(() => { @@ -320,6 +324,49 @@ describe('getCategoryAssignees / updateCategoryAssignees', () => { }); }); +// ── Plan 3e Task 4 parity: converted read models byte-for-byte against the ── +// legacy statement text, run raw on the same seeded rows (TD1/TD14). + +describe('TodoItemsRepository / TodoCategoryAssigneesRepository — parity', () => { + it('TODO-REPO-001: listItems matches TD1 (`SELECT * FROM todo_items WHERE trip_id = ? ORDER BY sort_order ASC, created_at ASC`) run raw, with every nullable column both set and left null', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + + const full = (await svc.createItem(trip.id, { + name: 'Full item', category: 'Cat', due_date: '2026-06-01', description: 'Desc', assigned_user_id: user.id, priority: 5, + })) as any; + await svc.updateItem(trip.id, full.id, { checked: 1 }, ['checked']); + + // Every nullable column left at its default (null/0). + await svc.createItem(trip.id, { name: 'Bare item' }); + + const converted = await svc.listItems(trip.id); + const legacy = testDb.prepare('SELECT * FROM todo_items WHERE trip_id = ? ORDER BY sort_order ASC, created_at ASC').all(trip.id); + expect(converted).toEqual(legacy); + expect(converted).toHaveLength(2); + }); + + it('TODO-REPO-002: updateCategoryAssignees\' TD14 re-select matches the legacy joined statement run raw, with an off-roster id present to prove the silent drop', async () => { + const { user: owner } = createUser(testDb); + const { user: member } = createUser(testDb); + const { user: stranger } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + addTripMember(testDb, trip.id, member.id); + + const converted = await svc.updateCategoryAssignees(trip.id, 'Packing', [owner.id, stranger.id, member.id]); + + const legacy = testDb.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(trip.id, 'Packing'); + + expect(converted).toEqual(legacy); + expect((converted as { user_id: number }[]).map((r) => r.user_id).sort()).toEqual([owner.id, member.id].sort()); + }); +}); + // TODO-SVC-021..024 (todo.bridge delegation) were deleted with the bridge — // its last consumer, the legacy get_trip_summary registrar in // src/mcp/tools/trips.ts, moved to the DI-discovered trips.mcp.ts. @@ -334,7 +381,10 @@ describe('TodoService.canEdit', () => { it('TODO-SVC-090 asks for packing_edit and flags a non-owner as shared', async () => { const checkPermission = vi.fn(() => true); const permissions = { checkPermission } as unknown as PermissionsService; - const withStub = new TodoService(new DatabaseService(testDb), permissions, new RealtimeService(), await createTestUnitOfWork(testDb)); + const withStub = new TodoService( + new DatabaseService(testDb), permissions, new RealtimeService(), await createTestUnitOfWork(testDb), + await createTestTodoItemsRepo(testDb), await createTestTodoCategoryAssigneesRepo(testDb), + ); const trip = { id: 1, user_id: 1 } as never; expect(await withStub.canEdit(trip, { id: 1, role: 'user' } as never)).toBe(true);