From 8b94a5a03e78a5b8c71c94d574b9a2de570d44af Mon Sep 17 00:00:00 2001 From: jubnl Date: Wed, 23 Sep 2026 21:13:35 +0200 Subject: [PATCH] refactor(vacay): vacation plans, entries, shares and holiday calendars through repositories --- .../VacayCompanyHolidays.repository.ts | 63 +- .../repositories/VacayEntries.repository.ts | 192 +++++- .../VacayHolidayCalendars.repository.ts | 111 +++- .../VacayPlanMembers.repository.ts | 188 +++++- .../db/repositories/VacayPlans.repository.ts | 121 +++- .../db/repositories/VacayShares.repository.ts | 178 ++++- .../VacayUserColors.repository.ts | 50 +- .../VacayUserSettings.repository.ts | 46 +- .../repositories/VacayUserYears.repository.ts | 102 ++- .../db/repositories/VacayYears.repository.ts | 53 +- server/src/nest/vacay/vacay.module.ts | 33 +- server/src/nest/vacay/vacay.service.ts | 617 +++++++++--------- server/tests/helpers/mcp-test-controllers.ts | 25 +- server/tests/helpers/plugin-host.ts | 16 +- server/tests/helpers/vacay-repos.ts | 72 ++ server/tests/unit/nest/trips.service.test.ts | 16 +- server/tests/unit/nest/vacay.service.test.ts | 203 +++++- 17 files changed, 1728 insertions(+), 358 deletions(-) create mode 100644 server/tests/helpers/vacay-repos.ts diff --git a/server/src/db/repositories/VacayCompanyHolidays.repository.ts b/server/src/db/repositories/VacayCompanyHolidays.repository.ts index 16188ec58..f0b7098ba 100644 --- a/server/src/db/repositories/VacayCompanyHolidays.repository.ts +++ b/server/src/db/repositories/VacayCompanyHolidays.repository.ts @@ -1,4 +1,65 @@ import type { VacayCompanyHolidays } from '../entities/VacayCompanyHolidays.entity'; import { TrekRepository } from './_shared/trek-repository'; -export class VacayCompanyHolidaysRepository extends TrekRepository {} +export class VacayCompanyHolidaysRepository extends TrekRepository { + /** + * VC29/VC67 — `SELECT date FROM vacay_company_holidays WHERE plan_id = ?` + * (`updatePlan`'s clear-on-enable, narrower columns) and `SELECT date, note + * FROM vacay_company_holidays WHERE plan_id = ?` (`dissolvePlan`'s + * migration read, wider columns). One method returning both columns — + * `updatePlan` destructures only `date` off the result, same rows either + * way. + */ + async listForPlan(planId: number): Promise<{ date: string; note: string | null }[]> { + const rows = await this.find({ plan: planId }, { fields: ['date', 'note'] }); + return rows.map((row) => ({ date: row.date, note: row.note ?? null })); + } + + /** VC93/VC112 — `SELECT date FROM vacay_company_holidays WHERE plan_id = ? AND date >= ? AND date < ? ORDER BY date` (`getSharedCalendars`) and `SELECT * FROM vacay_company_holidays WHERE plan_id = ? AND date >= ? AND date < ?` (`getEntries`, unordered, full row). Two distinct statements — two methods. */ + async listDatesForRange(planId: number, start: string, end: string): Promise<{ date: string }[]> { + const rows = await this.find({ plan: planId, date: { $gte: start, $lt: end } }, { fields: ['date'], orderBy: { date: 'asc' } }); + return rows.map((row) => ({ date: row.date })); + } + + /** VC112 — `SELECT * FROM vacay_company_holidays WHERE plan_id = ? AND date >= ? AND date < ?` (`getEntries`, full row, unordered). */ + async listForRange(planId: number, start: string, end: string): Promise<{ id: number; plan_id: number; date: string; note: string | null }[]> { + const rows = await this.find({ plan: planId, date: { $gte: start, $lt: end } }); + return rows.map((row) => ({ id: row.id, plan_id: row.plan_id, date: row.date, note: row.note ?? null })); + } + + /** VC24-sibling: VC25 — `DELETE FROM vacay_company_holidays WHERE plan_id = ? AND date = ?` (`applyHolidayCalendars`'s auto-clear). */ + async deleteForPlanAndDate(planId: number, date: string): Promise { + await this.nativeDelete({ plan: planId, date }); + } + + /** VC104 — `DELETE FROM vacay_company_holidays WHERE plan_id = ? AND date >= ? AND date < ?` (`deleteYear`, the intersection-of-member-windows range). */ + async deleteForRange(planId: number, start: string, end: string): Promise { + await this.nativeDelete({ plan: planId, date: { $gte: start, $lt: end } }); + } + + /** VC70/VC73 — `INSERT OR IGNORE INTO vacay_company_holidays (plan_id, date, note) VALUES (?, ?, ?)` (`dissolvePlan`'s two migration branches — identical statement). */ + async insertIgnore(planId: number, date: string, note: string): Promise { + await this.upsert({ plan: planId, date, note }, { onConflictFields: ['plan', 'date'], onConflictAction: 'ignore' }); + } + + /** + * VC118 — `SELECT id FROM vacay_company_holidays WHERE plan_id = ? AND date + * = ?` (`toggleCompanyHoliday`'s existing-row read). Named + * `findByPlanAndDate`, not `find` — `EntityRepository#find` already exists + * with an incompatible signature. + */ + async findByPlanAndDate(planId: number, date: string): Promise<{ id: number } | null> { + const row = await this.findOne({ plan: planId, date }, { fields: ['id'] }); + return row ? { id: row.id } : null; + } + + /** VC119 — `DELETE FROM vacay_company_holidays WHERE id = ?` (`toggleCompanyHoliday`'s clear-on-repeat-click). */ + async deleteById(id: number): Promise { + await this.nativeDelete({ id }); + } + + /** VC120 — `INSERT INTO vacay_company_holidays (plan_id, date, note) VALUES (?, ?, ?)` (`toggleCompanyHoliday`'s new row). */ + async insertHoliday(planId: number, date: string, note: string): Promise { + await this.insert({ plan: planId, date, note }); + } +} diff --git a/server/src/db/repositories/VacayEntries.repository.ts b/server/src/db/repositories/VacayEntries.repository.ts index ff28edb36..93247a1b6 100644 --- a/server/src/db/repositories/VacayEntries.repository.ts +++ b/server/src/db/repositories/VacayEntries.repository.ts @@ -1,4 +1,194 @@ +import { dateAdd } from '../dialect/sql-functions'; import type { VacayEntries } from '../entities/VacayEntries.entity'; import { TrekRepository } from './_shared/trek-repository'; -export class VacayEntriesRepository extends TrekRepository {} +interface VacayEntriesKyselyDB { + vacay_entries: { id: number; plan_id: number; user_id: number; date: string; note: string | null; fraction: number; kind: string }; +} + +/** VC111's joined grid projection (`getEntries`) — the full entry row plus the author's username and color. */ +export interface VacayEntryWithPersonRow { + id: number; + plan_id: number; + user_id: number; + date: string; + note: string | null; + fraction: number; + kind: string; + person_name: string; + person_color: string; +} + +/** SQLite's `date(, '+N days')` — the same calendar-day arithmetic `dateAdd` spells in SQL (§13), computed in JS for {@link VacayEntriesRepository.shiftForOwnerWindow}'s collision pre-check. Verified against SQLite's own `date()` modifier: both walk the proleptic Gregorian calendar by whole days, so they agree on every input (the same reasoning R9's VC4 date-diff verification gives). */ +function shiftIsoDate(isoDate: string, days: number): string { + const d = new Date(`${isoDate}T00:00:00Z`); + d.setUTCDate(d.getUTCDate() + days); + return d.toISOString().slice(0, 10); +} + +export class VacayEntriesRepository extends TrekRepository { + /** + * VC1 — `SELECT COALESCE(SUM(CASE WHEN kind = 'comp' THEN 0 ELSE fraction + * END), 0) AS used FROM vacay_entries WHERE user_id = ? AND plan_id = ? AND + * date >= ? AND date < ?` (`usedDays`). Kysely's expression builder + * (`eb.case()`/`eb.fn.sum`/`eb.fn.coalesce`, the `BudgetItems.repository.ts + * #getPerPersonSummary` precedent) — the `ELSE fraction` branch is a + * COLUMN reference, which `sql-functions.ts#caseWhenEquals` cannot express + * (its `whenTrue`/`whenFalse` are bound VALUES, not column refs), so this + * is built directly, no `sql` tag. + */ + async sumFraction(userId: number, planId: number, start: string, end: string): Promise { + const row = await this.kysely() + .selectFrom('vacay_entries') + .select((eb) => eb.fn.coalesce(eb.fn.sum(eb.case().when('kind', '=', 'comp').then(0).else(eb.ref('fraction')).end()), eb.val(0)).as('used')) + .where('user_id', '=', userId) + .where('plan_id', '=', planId) + .where('date', '>=', start) + .where('date', '<', end) + .executeTakeFirst(); + return row?.used ?? 0; + } + + /** VC2 — `SELECT COALESCE(SUM(fraction), 0) AS used FROM vacay_entries WHERE user_id = ? AND plan_id = ? AND date >= ? AND date < ? AND kind = 'comp'` (`compUsedDays`). */ + async sumCompFraction(userId: number, planId: number, start: string, end: string): Promise { + const row = await this.kysely() + .selectFrom('vacay_entries') + .select((eb) => eb.fn.coalesce(eb.fn.sum('fraction'), eb.val(0)).as('used')) + .where('user_id', '=', userId) + .where('plan_id', '=', planId) + .where('date', '>=', start) + .where('date', '<', end) + .where('kind', '=', 'comp') + .executeTakeFirst(); + return row?.used ?? 0; + } + + /** + * VC5 (`shiftOwnerEntriesForTripWindow`) — `UPDATE OR IGNORE vacay_entries + * SET date = date(date, ? || ' days') WHERE plan_id = ? AND user_id = ? AND + * date BETWEEN ? AND ?`. SQLite's per-statement `OR IGNORE` + * conflict-resolution clause applies to UPDATE as well as INSERT, but + * neither Kysely's `UpdateQueryBuilder` (only `.modifyEnd()`, which appends + * AFTER the whole statement, not after the `UPDATE` keyword) nor MikroORM's + * `nativeUpdate` exposes it — a genuine builder gap, not a helper this + * plan's R9 anticipated (R9's own VC5 note only covers the date + * EXPRESSION, via the existing `dateAdd`, not this conflict-resolution + * clause). Reproduced as a read-then-conditionally-write loop instead: + * read every candidate row, and for each one whose shifted date would + * collide with an existing `(user, plan, date)` row, skip it — `OR + * IGNORE`'s own per-row behavior, exactly — otherwise write it with + * `dateAdd` (still the SQL-computed value, per R9). Low-volume, + * request-scoped (a trip's own leave entries on a date-window move, never + * a hot path), so the per-row round trip is not a concern. + */ + async shiftForOwnerWindow(planId: number, userId: number, start: string, end: string, offset: number): Promise { + const platform = this.getEntityManager().getPlatform(); + const rows = await this.find({ plan: planId, user: userId, date: { $gte: start, $lte: end } }, { fields: ['id', 'date'] }); + for (const row of rows) { + const shiftedDate = shiftIsoDate(row.date, offset); + const collision = await this.findOne({ plan: planId, user: userId, date: shiftedDate }, { fields: ['id'] }); + if (collision) continue; + await this.nativeUpdate({ id: row.id }, { date: dateAdd(platform, 'date', offset) }); + } + } + + /** VC24/VC30/VC121 — `DELETE FROM vacay_entries WHERE plan_id = ? AND date = ?` (`applyHolidayCalendars`'s auto-clear, `updatePlan`'s company-holiday-enable clear, `toggleCompanyHoliday`'s clear — three identical-text call sites). */ + async deleteForPlanAndDate(planId: number, date: string): Promise { + await this.nativeDelete({ plan: planId, date }); + } + + /** VC54/VC69/VC72 — `UPDATE vacay_entries SET plan_id = ? WHERE plan_id = ? AND user_id = ?` (`acceptInvite`'s migration-in, `dissolvePlan`'s two migration-out branches — three identical-text call sites). */ + async updatePlanIdForUser(newPlanId: number, oldPlanId: number, userId: number): Promise { + await this.nativeUpdate({ plan: oldPlanId, user: userId }, { plan: newPlanId }); + } + + /** VC101 — `SELECT DISTINCT user_id FROM vacay_entries WHERE plan_id = ?` (`deleteYear`'s author enumeration — reads authors off the entries themselves, catching orphans the member list would miss). */ + async listAuthorsForPlan(planId: number): Promise<{ user_id: number }[]> { + const rows = await this.kysely() + .selectFrom('vacay_entries') + .select('user_id') + .distinct() + .where('plan_id', '=', planId) + .execute(); + return rows.map((row) => ({ user_id: row.user_id })); + } + + /** + * VC102 (`deleteYear`) — `DELETE FROM vacay_entries WHERE plan_id = ? AND + * user_id = ? AND date >= ? AND date < ?`. The window is computed PER + * AUTHOR by the caller (each `resolveYearWindow(user_id, year)` call inside + * the per-author loop — #737-aware, since members can be on differently + * shaped leave years) and passed in already resolved; this method never + * re-derives it, so a shortcut that computed the window once and reused it + * across authors would be a caller-side bug, not something this method + * could introduce. + */ + async deleteForRange(planId: number, userId: number, start: string, end: string): Promise { + await this.nativeDelete({ plan: planId, user: userId, date: { $gte: start, $lt: end } }); + } + + /** + * VC111 (`getEntries`) — `SELECT e.*, u.username as person_name, + * COALESCE(c.color, '#6366f1') as person_color FROM vacay_entries e JOIN + * users u ON e.user_id = u.id LEFT JOIN vacay_user_colors c ON c.user_id = + * e.user_id AND c.plan_id = e.plan_id WHERE e.plan_id = ? AND e.date >= ? + * AND e.date < ?`. Kysely two-condition `LEFT JOIN` + * (`BudgetItems.repository.ts#getPerPersonSummary`'s + * `.onRef(...).onRef(...)` shape). + */ + async listForRangeWithPerson(planId: number, start: string, end: string): Promise { + interface EntriesWithPersonKyselyDB { + vacay_entries: { id: number; plan_id: number; user_id: number; date: string; note: string | null; fraction: number; kind: string }; + users: { id: number; username: string }; + vacay_user_colors: { id: number; user_id: number; plan_id: number; color: string | null }; + } + return this.kysely() + .selectFrom('vacay_entries as e') + .innerJoin('users as u', 'u.id', 'e.user_id') + .leftJoin('vacay_user_colors as c', (join) => join.onRef('c.user_id', '=', 'e.user_id').onRef('c.plan_id', '=', 'e.plan_id')) + .select((eb) => [ + 'e.id', 'e.plan_id', 'e.user_id', 'e.date', 'e.note', 'e.fraction', 'e.kind', + 'u.username as person_name', + eb.fn.coalesce('c.color', eb.val('#6366f1')).as('person_color'), + ]) + .where('e.plan_id', '=', planId) + .where('e.date', '>=', start) + .where('e.date', '<', end) + .execute(); + } + + /** VC92 (`getSharedCalendars`) — `SELECT date, fraction, kind FROM vacay_entries WHERE plan_id = ? AND user_id = ? AND date >= ? AND date < ? ORDER BY date`, over the OWNER's plan and the VIEWER's leave-year window. */ + async listForOwnerRange(planId: number, userId: number, start: string, end: string): Promise<{ date: string; fraction: number; kind: string | null }[]> { + const rows = await this.find( + { plan: planId, user: userId, date: { $gte: start, $lt: end } }, + { fields: ['date', 'fraction', 'kind'], orderBy: { date: 'asc' } }, + ); + return rows.map((row) => ({ date: row.date, fraction: row.fraction, kind: row.kind ?? null })); + } + + /** + * VC114 — `SELECT id, fraction, kind FROM vacay_entries WHERE user_id = ? + * AND date = ? AND plan_id = ?` (`toggleEntry`'s existing-entry read). + * Named `findByUserDatePlan`, not `find` — `EntityRepository#find` already + * exists with an incompatible signature. + */ + async findByUserDatePlan(userId: number, date: string, planId: number): Promise<{ id: number; fraction: number; kind: string | null } | null> { + const row = await this.findOne({ user: userId, date, plan: planId }, { fields: ['id', 'fraction', 'kind'] }); + return row ? { id: row.id, fraction: row.fraction, kind: row.kind ?? null } : null; + } + + /** VC115 — `DELETE FROM vacay_entries WHERE id = ?` (`toggleEntry`'s clear-on-repeat-click). */ + async deleteById(id: number): Promise { + await this.nativeDelete({ id }); + } + + /** VC116 — `UPDATE vacay_entries SET fraction = ?, kind = ? WHERE id = ?` (`toggleEntry`'s in-place conversion). */ + async updateFractionKind(id: number, fraction: number, kind: string): Promise { + await this.nativeUpdate({ id }, { fraction, kind }); + } + + /** VC117 — `INSERT INTO vacay_entries (plan_id, user_id, date, note, fraction, kind) VALUES (?, ?, ?, ?, ?, ?)` (`toggleEntry`'s new entry — `note` is always `''` from this path). */ + async insertEntry(planId: number, userId: number, date: string, note: string, fraction: number, kind: string): Promise { + await this.insert({ plan: planId, user: userId, date, note, fraction, kind }); + } +} diff --git a/server/src/db/repositories/VacayHolidayCalendars.repository.ts b/server/src/db/repositories/VacayHolidayCalendars.repository.ts index 6421a55aa..3ffe6d2e5 100644 --- a/server/src/db/repositories/VacayHolidayCalendars.repository.ts +++ b/server/src/db/repositories/VacayHolidayCalendars.repository.ts @@ -1,12 +1,36 @@ import type { VacayHolidayCalendars } from '../entities/VacayHolidayCalendars.entity'; +import { toRow, type AssertRowKeys } from './_shared/rows'; import { TrekRepository } from './_shared/trek-repository'; +/** A `vacay_holiday_calendars` row as the legacy `SELECT *` returned it (Plan 3f Task 5). */ +export interface VacayHolidayCalendarRow { + id: number; + plan_id: number; + type: 'public_holiday' | 'school_holiday'; + region: string; + label: string | null; + color: string; + sort_order: number; +} + +const _vacayHolidayCalendarRowKeys: AssertRowKeys = true; + +/** `updateHolidayCalendar` (VC41)'s conditionally-included-fields patch — §15.9's JS-assembled SET list, mapped onto `nativeUpdate`'s own `undefined`-means-omit semantics, same shape as `VacayPlansRepository.update`. */ +export interface VacayHolidayCalendarPatch { + region?: string; + type?: 'public_holiday' | 'school_holiday'; + label?: string | null; + color?: string; + sort_order?: number; +} + /** - * Plan 3f Task 2 (`SchoolHolidaysService.deleteRegion`) appends EXACTLY this - * one additive method — Task 5 (vacay) builds the rest of this repository - * around it later, per the plan's own file-ownership rule for this shared - * file (`.superpowers/sdd/2026-09-25-orm-phase3f/task-2-brief.md`); no other - * method, no class-level change beyond this. + * Plan 3f Task 2 (`SchoolHolidaysService.deleteRegion`) appended + * {@link VacayHolidayCalendarsRepository.existsForSchoolRegion} first — kept + * untouched, exactly as landed; Task 5 (vacay) appends the rest of this + * repository around it below, per the plan's own file-ownership rule for + * this shared file (`.superpowers/sdd/2026-09-25-orm-phase3f/task-2-brief.md` + * / `task-5-brief.md`). */ export class VacayHolidayCalendarsRepository extends TrekRepository { /** @@ -23,4 +47,81 @@ export class VacayHolidayCalendarsRepository extends TrekRepository { + const rows = await this.find({ plan: planId, type: 'public_holiday' }, { orderBy: { sort_order: 'asc', id: 'asc' } }); + return rows.map((row) => toRow(row) as VacayHolidayCalendarRow); + } + + /** + * VC37/VC131 — `SELECT * FROM vacay_holiday_calendars WHERE plan_id = ? + * ORDER BY sort_order, id` (`updatePlan`'s re-select, `getPlanData` — two + * identical-text call sites, one method). + */ + async listForPlan(planId: number): Promise { + const rows = await this.find({ plan: planId }, { orderBy: { sort_order: 'asc', id: 'asc' } }); + return rows.map((row) => toRow(row) as VacayHolidayCalendarRow); + } + + /** VC26 — `SELECT id FROM vacay_holiday_calendars WHERE plan_id = ?` (`migrateHolidayCalendars`'s one-time legacy-region migration guard — existence only, any row). */ + async existsForPlan(planId: number): Promise { + const row = await this.findOne({ plan: planId }, { fields: ['id'] }); + return row !== null; + } + + /** + * VC27/VC38 — `INSERT INTO vacay_holiday_calendars (plan_id, region, label, + * color, sort_order) VALUES (?, ?, NULL, ?, 0)` (`migrateHolidayCalendars`, + * `type` defaulted by the schema) and `INSERT INTO vacay_holiday_calendars + * (plan_id, type, region, label, color, sort_order) VALUES (?, ?, ?, ?, ?, + * ?)` (`addHolidayCalendar`, `type` explicit). One flexible method — the + * legacy migration insert's implicit schema-default `type` and the + * explicit insert's bound `type` write the identical column value when the + * caller passes `'public_holiday'` for the former. Named `insertCalendar`, + * not `insert` — `EntityRepository#insert` already exists with an + * incompatible signature. + */ + async insertCalendar(planId: number, type: 'public_holiday' | 'school_holiday', region: string, label: string | null, color: string, sortOrder: number): Promise { + return this.insert({ plan: planId, type, region, label, color, sort_order: sortOrder }); + } + + /** + * VC39/VC42 — `SELECT * FROM vacay_holiday_calendars WHERE id = ?` + * (`addHolidayCalendar`'s post-insert re-select, `updateHolidayCalendar`'s + * post-write re-select — identical statement, two call sites). Named + * `findById`, not `find` — `EntityRepository#find` already exists with an + * incompatible signature. + */ + async findById(id: number): Promise { + const row = await this.findOne({ id }); + return row ? (toRow(row) as VacayHolidayCalendarRow) : null; + } + + /** + * VC40/VC43 — `SELECT * FROM vacay_holiday_calendars WHERE id = ? AND + * plan_id = ?` — the plan-scoping guard (`updateHolidayCalendar`'s own + * read, `deleteHolidayCalendar`'s own read — identical statement, two call + * sites). + */ + async findScopedForPlan(id: number, planId: number): Promise { + const row = await this.findOne({ id, plan: planId }); + return row ? (toRow(row) as VacayHolidayCalendarRow) : null; + } + + /** VC41 — `UPDATE vacay_holiday_calendars SET ${dynamic} WHERE id = ?`, §15.9's conditionally-included-fields shape (up to 5 optional columns). */ + async update(id: number, patch: VacayHolidayCalendarPatch): Promise { + if (Object.keys(patch).length === 0) return; + await this.nativeUpdate({ id }, { ...patch }); + } + + /** VC44 — `DELETE FROM vacay_holiday_calendars WHERE id = ?` (`deleteHolidayCalendar`, called only after {@link findScopedForPlan} found a scoped row). */ + async deleteById(id: number): Promise { + await this.nativeDelete({ id }); + } } diff --git a/server/src/db/repositories/VacayPlanMembers.repository.ts b/server/src/db/repositories/VacayPlanMembers.repository.ts index e43d3f987..c85fbdb37 100644 --- a/server/src/db/repositories/VacayPlanMembers.repository.ts +++ b/server/src/db/repositories/VacayPlanMembers.repository.ts @@ -1,4 +1,190 @@ import type { VacayPlanMembers } from '../entities/VacayPlanMembers.entity'; +import { toRow, type AssertRowKeys } from './_shared/rows'; import { TrekRepository } from './_shared/trek-repository'; -export class VacayPlanMembersRepository extends TrekRepository {} +/** A `vacay_plan_members` row as the legacy `SELECT *` returned it (VC51). */ +export interface VacayPlanMemberRow { + id: number; + plan_id: number; + user_id: number; + status: string | null; + created_at: string | null; +} + +const _vacayPlanMemberRowKeys: AssertRowKeys = true; + +/** VC17/VC129/VC130's joined member+user projections. */ +export interface VacayMemberUserRow { + id: number; + username: string; + email: string; +} + +/** VC129 — outgoing pending invites (`getPlanData`). */ +export interface VacayPendingOutgoingRow { + id: number; + user_id: number; + username: string; + email: string; + created_at: string | null; +} + +/** VC130 — incoming pending invites (`getPlanData`). */ +export interface VacayPendingIncomingRow { + id: number; + plan_id: number; + username: string; + email: string; + created_at: string | null; +} + +export class VacayPlanMembersRepository extends TrekRepository { + /** + * VC13/VC76 — `SELECT plan_id FROM vacay_plan_members WHERE user_id = ? AND + * status = 'accepted'` (`getActivePlan`, `peekActivePlan`). No `fields` + * narrowing: `plan_id` is the `persist(false)` shadow of the `plan` + * relation — narrowing to JUST a `persist(false)` mirror column silently + * drops it from the result (verified directly). + */ + async findAcceptedPlanId(userId: number): Promise<{ plan_id: number } | null> { + const row = await this.findOne({ user: userId, status: 'accepted' }); + return row ? { plan_id: row.plan_id } : null; + } + + /** VC17 — `SELECT u.id, u.username, u.email FROM vacay_plan_members m JOIN users u ON m.user_id = u.id WHERE m.plan_id = ? AND m.status = 'accepted'` (`getPlanUsers`'s accepted-member half). */ + async listAcceptedWithUsers(planId: number): Promise { + return this.qb('m') + .join('m.user', 'u') + .select(['u.id', 'u.username', 'u.email']) + .where({ 'm.plan': planId, 'm.status': 'accepted' }) + .execute('all', false); + } + + /** + * VC19/VC68 — `SELECT user_id FROM vacay_plan_members WHERE plan_id = ? AND + * status = 'accepted'` (`notifyPlanUsers`, `dissolvePlan`'s owner branch). + * No `fields` narrowing — same `persist(false)`-mirror trap as + * {@link findAcceptedPlanId}. + */ + async listAcceptedUserIds(planId: number): Promise<{ user_id: number }[]> { + const rows = await this.find({ plan: planId, status: 'accepted' }); + return rows.map((row) => ({ user_id: row.user_id })); + } + + /** VC48 — `SELECT id, status FROM vacay_plan_members WHERE plan_id = ? AND user_id = ?` (`sendInvite`'s dedup guard — any status). */ + async findMembership(planId: number, userId: number): Promise<{ id: number; status: string | null } | null> { + const row = await this.findOne({ plan: planId, user: userId }, { fields: ['id', 'status'] }); + return row ? { id: row.id, status: row.status ?? null } : null; + } + + /** VC49 — `SELECT id FROM vacay_plan_members WHERE user_id = ? AND status = 'accepted'` (`sendInvite`'s single-fusion guard). */ + async findAcceptedForUser(userId: number): Promise<{ id: number } | null> { + const row = await this.findOne({ user: userId, status: 'accepted' }, { fields: ['id'] }); + return row ? { id: row.id } : null; + } + + /** VC50 — `INSERT INTO vacay_plan_members (plan_id, user_id, status) VALUES (?, ?, 'pending')`. */ + async insertPending(planId: number, userId: number): Promise { + await this.insert({ plan: planId, user: userId, status: 'pending' }); + } + + /** + * VC47 (`sendInvite`'s invite-target lookup) — `SELECT id, username FROM + * users WHERE id = ? AND COALESCE(is_guest, 0) = 0` — a guest is never + * invitable (#1362-class). `UsersRepository` is a concurrently in-flight + * file owned by another Plan 3f task at the time of this write and outside + * this task's Named file set (same reasoning as `VacayPlansRepository + * .findVacayUser`'s docstring) — a direct `users`-table Kysely read here + * instead. + */ + async findInvitableUser(id: number): Promise<{ id: number; username: string } | null> { + interface UsersKyselyDB { + users: { id: number; username: string; is_guest: number | null }; + } + const row = await this.kysely() + .selectFrom('users') + .select(['id', 'username']) + .where('id', '=', id) + .where((eb) => eb(eb.fn.coalesce('is_guest', eb.val(0)), '=', 0)) + .executeTakeFirst(); + return row ? { id: row.id, username: row.username } : null; + } + + /** VC51 — `SELECT * FROM vacay_plan_members WHERE plan_id = ? AND user_id = ? AND status = 'pending'` (`acceptInvite`'s own pending-invite read). */ + async findPending(planId: number, userId: number): Promise { + const row = await this.findOne({ plan: planId, user: userId, status: 'pending' }); + return row ? (toRow(row) as VacayPlanMemberRow) : null; + } + + /** VC52 — `UPDATE vacay_plan_members SET status = 'accepted' WHERE id = ?`. */ + async accept(id: number): Promise { + await this.nativeUpdate({ id }, { status: 'accepted' }); + } + + /** VC65/VC66 — `DELETE FROM vacay_plan_members WHERE plan_id = ? AND user_id = ? AND status = 'pending'` (`declineInvite`'s own withdrawal, `cancelInvite`'s owner-initiated withdrawal — identical statement, `targetUserId` in place of `userId`). */ + async deletePending(planId: number, userId: number): Promise { + await this.nativeDelete({ plan: planId, user: userId, status: 'pending' }); + } + + /** VC71 — `DELETE FROM vacay_plan_members WHERE plan_id = ?` (`dissolvePlan`'s owner branch). */ + async deleteForPlan(planId: number): Promise { + await this.nativeDelete({ plan: planId }); + } + + /** VC74 — `DELETE FROM vacay_plan_members WHERE plan_id = ? AND user_id = ?` (`dissolvePlan`'s member branch). */ + async deleteForPlanAndUser(planId: number, userId: number): Promise { + await this.nativeDelete({ plan: planId, user: userId }); + } + + /** VC129 — `SELECT m.id, m.user_id, u.username, u.email, m.created_at FROM vacay_plan_members m JOIN users u ON m.user_id = u.id WHERE m.plan_id = ? AND m.status = 'pending'` (`getPlanData`'s outgoing invites). */ + async listPendingForPlan(planId: number): Promise { + return this.qb('m') + .join('m.user', 'u') + .select(['m.id', 'u.id as user_id', 'u.username', 'u.email', 'm.created_at']) + .where({ 'm.plan': planId, 'm.status': 'pending' }) + .execute('all', false); + } + + /** VC130 — `SELECT m.id, m.plan_id, u.username, u.email, m.created_at FROM vacay_plan_members m JOIN vacay_plans p ON m.plan_id = p.id JOIN users u ON p.owner_id = u.id WHERE m.user_id = ? AND m.status = 'pending'` (`getPlanData`'s incoming invites — the OWNER's username/email, not the member's). */ + async listPendingForUser(userId: number): Promise { + return this.qb('m') + .join('m.plan', 'p') + .join('p.owner', 'u') + .select(['m.id', 'p.id as plan_id', 'u.username', 'u.email', 'm.created_at']) + .where({ 'm.user': userId, 'm.status': 'pending' }) + .execute('all', false); + } + + /** + * VC75 (`getAvailableUsers`, the fusion-candidate picker) — `SELECT u.id, + * u.username, u.email FROM users u WHERE u.id != ? AND COALESCE(u.is_guest, + * 0) = 0 AND u.id NOT IN (SELECT user_id FROM vacay_plan_members WHERE + * plan_id = ?) AND u.id NOT IN (SELECT user_id FROM vacay_plan_members + * WHERE status = 'accepted') AND u.id NOT IN (SELECT owner_id FROM + * vacay_plans WHERE id IN (SELECT plan_id FROM vacay_plan_members WHERE + * status = 'accepted')) ORDER BY u.username`. Three nested `NOT IN` + * subqueries (§13) — Kysely, no `sql` tag, every operand builder-expressed. + */ + async listAvailableForFusion(userId: number, planId: number): Promise { + interface FusionCandidateKyselyDB { + users: { id: number; username: string; email: string; is_guest: number | null }; + vacay_plan_members: { id: number; plan_id: number; user_id: number; status: string | null }; + vacay_plans: { id: number; owner_id: number }; + } + return this.kysely() + .selectFrom('users as u') + .select(['u.id', 'u.username', 'u.email']) + .where('u.id', '!=', userId) + .where((eb) => eb(eb.fn.coalesce('u.is_guest', eb.val(0)), '=', 0)) + .where('u.id', 'not in', (eb) => eb.selectFrom('vacay_plan_members').select('user_id').where('plan_id', '=', planId)) + .where('u.id', 'not in', (eb) => eb.selectFrom('vacay_plan_members').select('user_id').where('status', '=', 'accepted')) + .where('u.id', 'not in', (eb) => + eb + .selectFrom('vacay_plans') + .select('owner_id') + .where('id', 'in', (eb2) => eb2.selectFrom('vacay_plan_members').select('plan_id').where('status', '=', 'accepted')), + ) + .orderBy('u.username', 'asc') + .execute(); + } +} diff --git a/server/src/db/repositories/VacayPlans.repository.ts b/server/src/db/repositories/VacayPlans.repository.ts index 505dc7465..7471c0a99 100644 --- a/server/src/db/repositories/VacayPlans.repository.ts +++ b/server/src/db/repositories/VacayPlans.repository.ts @@ -1,4 +1,123 @@ import type { VacayPlans } from '../entities/VacayPlans.entity'; +import { toRow, type AssertRowKeys } from './_shared/rows'; import { TrekRepository } from './_shared/trek-repository'; -export class VacayPlansRepository extends TrekRepository {} +/** + * A `vacay_plans` row as the legacy `SELECT *` returned it (Plan 3f Task 5, + * inventory §2, VC7-131's `vacay_plans` group). `owner_id` is the + * `persist(false)` shadow of the `owner` relation (§14's FK-mirror TRAP — + * every plain `find`/`findOne` below reads the whole row, so it always + * hydrates); `created_at` is the one column `VacayService`'s own hand-written + * `VacayPlan` interface never declared (nothing reads it), kept here anyway + * so {@link AssertRowKeys} still pins every scalar column of the entity. + */ +export interface VacayPlanRow { + id: number; + owner_id: number; + block_weekends: number | null; + holidays_enabled: number | null; + holidays_region: string | null; + school_holidays_enabled: number | null; + company_holidays_enabled: number | null; + carry_over_enabled: number | null; + created_at: string | null; + weekend_days: string | null; + week_start: number; +} + +const _vacayPlanRowKeys: AssertRowKeys = true; + +/** `updatePlan` (VC28)'s conditionally-included-fields patch — §15.9: a JS-assembled SET list, not the presence-sentinel idiom, maps directly onto `nativeUpdate`'s own `undefined`-means-omit semantics. */ +export interface VacayPlanPatch { + block_weekends?: number; + holidays_enabled?: number; + holidays_region?: string | null; + school_holidays_enabled?: number; + company_holidays_enabled?: number; + carry_over_enabled?: number; + weekend_days?: string; + week_start?: number; +} + +export class VacayPlansRepository extends TrekRepository { + /** VC7/VC9/VC78 — `SELECT * FROM vacay_plans WHERE owner_id = ?` (one method, three identical-text call sites: `getOwnPlan`'s pre-insert read, its post-insert re-select, and `peekActivePlan`'s owner fallback). */ + async findByOwner(ownerId: number): Promise { + const plan = await this.findOne({ owner: ownerId }); + return plan ? (toRow(plan) as VacayPlanRow) : null; + } + + /** + * VC14 — `SELECT * FROM vacay_plans WHERE id = ?`, the hottest statement in + * this file: seven identical-text call sites (`getActivePlan`, + * `getPlanUsers`, `peekActivePlan`, `updatePlan`'s pre- and post-write + * reads, `addYear`, `deleteYear`, `toggleEntry`, `getStats` — VC14/15/31/ + * 36/77/97/107/113/122), all ONE method. + */ + async findById(id: number): Promise { + const plan = await this.findOne({ id }); + return plan ? (toRow(plan) as VacayPlanRow) : null; + } + + /** + * VC18/VC103 — `SELECT owner_id FROM vacay_plans WHERE id = ?` + * (`notifyPlanUsers`, `deleteYear`'s carry-over recompute). No `fields` + * narrowing: `owner_id` is the `persist(false)` shadow of the `owner` + * relation, and narrowing a `find`/`findOne` to JUST a `persist(false)` + * mirror column silently drops it from the result (verified directly, the + * same trap `PlaceRatingsRepository.listForPlaces`'s docstring names for + * the join-alias case) — a full read costs one extra row's worth of + * columns, never a wrong answer. + */ + async findOwnerId(id: number): Promise<{ owner_id: number } | null> { + const row = await this.findOne({ id }); + return row ? { owner_id: row.owner_id } : null; + } + + /** VC53 — `SELECT id FROM vacay_plans WHERE owner_id = ?` (`acceptInvite`'s own-plan-to-migrate-from lookup) — a NARROWER column set than {@link findByOwner}'s VC7/9/78 (no `username`/`email`/etc., just `id`), a genuinely distinct statement. */ + async findIdByOwner(ownerId: number): Promise<{ id: number } | null> { + const row = await this.findOne({ owner: ownerId }, { fields: ['id'] }); + return row ? { id: row.id } : null; + } + + /** VC21 — `SELECT holidays_enabled FROM vacay_plans WHERE id = ?` (`applyHolidayCalendars`'s feature gate). */ + async getHolidaysEnabled(id: number): Promise { + const row = await this.findOne({ id }, { fields: ['holidays_enabled'] }); + return row?.holidays_enabled ?? null; + } + + /** + * VC8 — `INSERT INTO vacay_plans (owner_id) VALUES (?)` (`getOwnPlan`'s + * lazy-create; every other column takes the schema's own default). Named + * `insertForOwner`, not `insert` — `EntityRepository#insert` already + * exists with an incompatible signature (`Users.repository.ts::insertUser`'s + * precedent). + */ + async insertForOwner(ownerId: number): Promise { + return this.insert({ owner: ownerId }); + } + + /** VC28 — `UPDATE vacay_plans SET ${dynamic column list} WHERE id = ?`, §15.9's conditionally-included-fields shape: only the keys present in `patch` are written, matching the legacy JS-assembled SET list exactly (an absent key is truly untouched, never a presence-sentinel `CASE WHEN`). */ + async update(id: number, patch: VacayPlanPatch): Promise { + if (Object.keys(patch).length === 0) return; + await this.nativeUpdate({ id }, { ...patch }); + } + + /** + * VC16 (`getPlanUsers`'s owner lookup) — `SELECT id, username, email FROM + * users WHERE id = ?`. `UsersRepository` is a concurrently in-flight file + * owned by another Plan 3f task at the time of this write (Task 4's own + * additive block) — this task's Named file set does not include it, so + * this read stays here (a same-shape precedent to `VacayPlanMembersRepository + * .listAvailableForFusion`'s direct `users`-table Kysely read: a + * repository is not required to route every cross-table read through the + * table's "owning" repository, only to own its OWN table's writes) rather + * than risk a conflicting concurrent edit to a file this task does not own. + */ + async findVacayUser(id: number): Promise<{ id: number; username: string; email: string } | null> { + interface UsersKyselyDB { + users: { id: number; username: string; email: string }; + } + const row = await this.kysely().selectFrom('users').select(['id', 'username', 'email']).where('id', '=', id).executeTakeFirst(); + return row ?? null; + } +} diff --git a/server/src/db/repositories/VacayShares.repository.ts b/server/src/db/repositories/VacayShares.repository.ts index 33088986e..77fa1d5b2 100644 --- a/server/src/db/repositories/VacayShares.repository.ts +++ b/server/src/db/repositories/VacayShares.repository.ts @@ -1,4 +1,180 @@ import type { VacayShares } from '../entities/VacayShares.entity'; +import { toRow, type AssertRowKeys } from './_shared/rows'; import { TrekRepository } from './_shared/trek-repository'; -export class VacaySharesRepository extends TrekRepository {} +/** A `vacay_shares` row as the legacy `SELECT *` returned it. */ +export interface VacayShareRow { + id: number; + owner_id: number; + user_id: number; + hidden: number; + created_at: string | null; +} + +const _vacayShareRowKeys: AssertRowKeys = true; + +/** VC81/VC82/VC91's joined share+user projections. */ +export interface VacayOutgoingShareRow { + id: number; + user_id: number; + username: string; +} + +export interface VacayIncomingShareRow { + id: number; + owner_id: number; + hidden: number; + username: string; +} + +export class VacaySharesRepository extends TrekRepository { + /** VC84 — `SELECT id FROM vacay_shares WHERE owner_id = ? AND user_id = ?` (`shareCalendar`'s dedup guard). */ + async findByOwnerAndUser(ownerId: number, userId: number): Promise<{ id: number } | null> { + const row = await this.findOne({ owner: ownerId, user: userId }, { fields: ['id'] }); + return row ? { id: row.id } : null; + } + + /** + * VC83 (`shareCalendar`'s share-target lookup) — `SELECT id FROM users + * WHERE id = ? AND COALESCE(is_guest, 0) = 0` — a NARROWER column set than + * `VacayPlanMembersRepository.findInvitableUser`'s VC47 (no `username`), a + * genuinely distinct statement (inventory §2's own note), not a reuse. + * `UsersRepository` is a concurrently in-flight file owned by another + * Plan 3f task at the time of this write and outside this task's Named + * file set (same reasoning as `VacayPlansRepository.findVacayUser`'s + * docstring) — a direct `users`-table Kysely read here instead. + */ + async existsInvitableUser(id: number): Promise { + interface UsersKyselyDB { + users: { id: number; is_guest: number | null }; + } + const row = await this.kysely() + .selectFrom('users') + .select('id') + .where('id', '=', id) + .where((eb) => eb(eb.fn.coalesce('is_guest', eb.val(0)), '=', 0)) + .executeTakeFirst(); + return row !== undefined; + } + + /** VC81 — `SELECT s.id, s.user_id, u.username FROM vacay_shares s JOIN users u ON s.user_id = u.id WHERE s.owner_id = ? ORDER BY s.id` (`listShares`'s outgoing half — usernames only, emails withheld). */ + async listOutgoing(ownerId: number): Promise { + return this.qb('s') + .join('s.user', 'u') + .select(['s.id', 'u.id as user_id', 'u.username']) + .where({ 's.owner': ownerId }) + .orderBy({ 's.id': 'asc' }) + .execute('all', false); + } + + /** VC82/VC91 — `SELECT s.id, s.owner_id, s.hidden, u.username FROM vacay_shares s JOIN users u ON s.owner_id = u.id WHERE s.user_id = ? ORDER BY s.id` (`listShares`'s incoming half, `getSharedCalendars`'s share list — identical statement, two call sites). */ + async listIncoming(userId: number): Promise { + return this.qb('s') + .join('s.owner', 'u') + .select(['s.id', 'u.id as owner_id', 's.hidden', 'u.username']) + .where({ 's.user': userId }) + .orderBy({ 's.id': 'asc' }) + .execute('all', false); + } + + /** + * VC85 — `INSERT INTO vacay_shares (owner_id, user_id) VALUES (?, ?)` + * (`shareCalendar`). Named `insertShare`, not `insert` — + * `EntityRepository#insert` already exists with an incompatible signature. + */ + async insertShare(ownerId: number, userId: number): Promise { + await this.insert({ owner: ownerId, user: userId }); + } + + /** + * R6 — `findScopedForRemoval(id, userId)`: `WHERE id = ? AND (owner_id = ? + * OR user_id = ?)`, mirroring `removeShare`'s wider ownership-or-viewer + * check (VC86, today an UNSCOPED `SELECT * FROM vacay_shares WHERE id = ?` + * followed by a JS check — `share.owner_id !== userId && share.user_id !== + * userId` — that runs before any mutation either way). This method scopes + * the SAME check IN the SQL statement itself: a share row belonging to + * neither the owner NOR the viewer never comes back, so the caller (which + * then deletes-by-id via {@link deleteById}) cannot act on a foreign row + * even if a future change to the service dropped the JS check. Behaviorally + * identical to today on every input (the JS check ran first either way); + * flagged per R6/the plan's "For the user" note as a genuine SQL-shape + * tightening, not silent parity. + */ + async findScopedForRemoval(id: number, userId: number): Promise { + const row = await this.findOne({ id, $or: [{ owner: userId }, { user: userId }] }); + return row ? (toRow(row) as VacayShareRow) : null; + } + + /** + * R6 — `findScopedForHide(id, userId)`: `WHERE id = ? AND user_id = ?`, + * mirroring `setShareHidden`'s NARROWER viewer-only check (VC88, today the + * same unscoped `SELECT * FROM vacay_shares WHERE id = ?` plus a JS check — + * `share.user_id !== userId` only, the OWNER may not hide their own + * outgoing share). Deliberately a DIFFERENT predicate from + * {@link findScopedForRemoval}, not the same method with a mode flag — an + * owner's own row must NOT match this one even though it would match + * `findScopedForRemoval`. + */ + async findScopedForHide(id: number, userId: number): Promise { + const row = await this.findOne({ id, user: userId }); + return row ? (toRow(row) as VacayShareRow) : null; + } + + /** VC87 — `DELETE FROM vacay_shares WHERE id = ?` (`removeShare`, called only after {@link findScopedForRemoval} found a scoped row). */ + async deleteById(id: number): Promise { + await this.nativeDelete({ id }); + } + + /** VC89 — `UPDATE vacay_shares SET hidden = ? WHERE id = ?` (`setShareHidden`, called only after {@link findScopedForHide} found a scoped row). */ + async setHidden(id: number, hidden: boolean): Promise { + await this.nativeUpdate({ id }, { hidden: hidden ? 1 : 0 }); + } + + /** VC20 — `SELECT DISTINCT user_id FROM vacay_shares WHERE owner_id IN (${dynamic})` (`notifyShareViewers`'s fan-out list). Empty `ownerIds` short-circuits before any query, matching the caller's own guard. */ + async listDistinctViewerIdsForOwners(ownerIds: number[]): Promise { + if (ownerIds.length === 0) return []; + interface VacaySharesKyselyDB { + vacay_shares: { id: number; owner_id: number; user_id: number; hidden: number; created_at: string | null }; + } + const rows = await this.kysely() + .selectFrom('vacay_shares') + .select('user_id') + .distinct() + .where('owner_id', 'in', ownerIds) + .execute(); + return rows.map((row) => row.user_id); + } + + /** + * VC90 (`getShareAvailableUsers`, the share-candidate picker) — `SELECT + * u.id, u.username FROM users u WHERE u.id != ? AND COALESCE(u.is_guest, 0) + * = 0 AND u.id NOT IN (SELECT user_id FROM vacay_shares WHERE owner_id = ?) + * AND u.id NOT IN (SELECT owner_id FROM vacay_plans WHERE id = ? UNION + * SELECT user_id FROM vacay_plan_members WHERE plan_id = ? AND status = + * 'accepted') ORDER BY u.username`. A `UNION` nested inside a `NOT IN` + * subquery (§13) — Kysely, no `sql` tag. + */ + async listAvailableForShare(userId: number, planId: number): Promise<{ id: number; username: string }[]> { + interface ShareCandidateKyselyDB { + users: { id: number; username: string; is_guest: number | null }; + vacay_shares: { id: number; owner_id: number; user_id: number }; + vacay_plans: { id: number; owner_id: number }; + vacay_plan_members: { id: number; plan_id: number; user_id: number; status: string | null }; + } + return this.kysely() + .selectFrom('users as u') + .select(['u.id', 'u.username']) + .where('u.id', '!=', userId) + .where((eb) => eb(eb.fn.coalesce('u.is_guest', eb.val(0)), '=', 0)) + .where('u.id', 'not in', (eb) => eb.selectFrom('vacay_shares').select('user_id').where('owner_id', '=', userId)) + .where('u.id', 'not in', (eb) => + eb + .selectFrom('vacay_plans') + .select('owner_id') + .where('id', '=', planId) + .union(eb.selectFrom('vacay_plan_members').select('user_id as owner_id').where('plan_id', '=', planId).where('status', '=', 'accepted')), + ) + .orderBy('u.username', 'asc') + .execute(); + } +} diff --git a/server/src/db/repositories/VacayUserColors.repository.ts b/server/src/db/repositories/VacayUserColors.repository.ts index cb857b51e..36a4df745 100644 --- a/server/src/db/repositories/VacayUserColors.repository.ts +++ b/server/src/db/repositories/VacayUserColors.repository.ts @@ -1,4 +1,52 @@ import type { VacayUserColors } from '../entities/VacayUserColors.entity'; import { TrekRepository } from './_shared/trek-repository'; -export class VacayUserColorsRepository extends TrekRepository {} +export class VacayUserColorsRepository extends TrekRepository { + /** VC79 — `SELECT color FROM vacay_user_colors WHERE plan_id = ?` (`viewerColors`). */ + async listForPlan(planId: number): Promise<{ color: string | null }[]> { + const rows = await this.find({ plan: planId }, { fields: ['color'] }); + return rows.map((row) => ({ color: row.color ?? null })); + } + + /** + * VC57/VC60/VC80/VC124/VC128 — `SELECT color FROM vacay_user_colors WHERE + * user_id = ? AND plan_id = ?` (five identical-text call sites: + * `acceptInvite`'s own-color read and its post-remap re-read, + * `shareDisplayColor`, `getStats`, `getPlanData`'s per-member map). Named + * `findColor`, not `find` — `EntityRepository#find` already exists with an + * incompatible signature. + */ + async findColor(userId: number, planId: number): Promise<{ color: string | null } | null> { + const row = await this.findOne({ user: userId, plan: planId }, { fields: ['color'] }); + return row ? { color: row.color ?? null } : null; + } + + /** VC59 — `SELECT color FROM vacay_user_colors WHERE plan_id = ? AND user_id != ?` (`acceptInvite`'s taken-colors collection). */ + async listOtherColors(planId: number, excludeUserId: number): Promise<{ color: string | null }[]> { + const rows = await this.find({ plan: planId, user: { $ne: excludeUserId } }, { fields: ['color'] }); + return rows.map((row) => ({ color: row.color ?? null })); + } + + /** + * VC12/VC58/VC62 — `INSERT OR IGNORE INTO vacay_user_colors (user_id, + * plan_id, color) VALUES (?, ?, ?)` (`getOwnPlan`'s lazy default color, + * `acceptInvite`'s own-color migration and its no-collision else-branch — + * three identical-text call sites, one method). + */ + async insertIgnore(userId: number, planId: number, color: string): Promise { + await this.upsert({ user: userId, plan: planId, color }, { onConflictFields: ['user', 'plan'], onConflictAction: 'ignore' }); + } + + /** + * VC46/VC61 — `INSERT INTO vacay_user_colors (user_id, plan_id, color) + * VALUES (?, ?, ?) ON CONFLICT(user_id, plan_id) DO UPDATE SET color = + * excluded.color` (`setUserColor`'s own write, `acceptInvite`'s + * collision-remap write — identical statement, two call sites). + */ + async upsertColor(userId: number, planId: number, color: string): Promise { + await this.upsert( + { user: userId, plan: planId, color }, + { onConflictFields: ['user', 'plan'], onConflictAction: 'merge', onConflictMergeFields: ['color'] }, + ); + } +} diff --git a/server/src/db/repositories/VacayUserSettings.repository.ts b/server/src/db/repositories/VacayUserSettings.repository.ts index e821c6f19..a2d880baa 100644 --- a/server/src/db/repositories/VacayUserSettings.repository.ts +++ b/server/src/db/repositories/VacayUserSettings.repository.ts @@ -1,4 +1,48 @@ import type { VacayUserSettings } from '../entities/VacayUserSettings.entity'; +import { toRow, type AssertRowKeys } from './_shared/rows'; import { TrekRepository } from './_shared/trek-repository'; -export class VacayUserSettingsRepository extends TrekRepository {} +/** A `vacay_user_settings` row as the legacy `SELECT *` returned it — `user` is the entity's primary key (`[PrimaryKeyProp]?: 'user'`), so `user_id` is its `persist(false)` shadow scalar. */ +export interface VacayUserSettingsRow { + user_id: number; + year_type: string; + year_start_month: number; + year_start_day: number; + hire_date: string | null; +} + +const _vacayUserSettingsRowKeys: AssertRowKeys = true; + +export class VacayUserSettingsRepository extends TrekRepository { + /** + * VC3 — `SELECT * FROM vacay_user_settings WHERE user_id = ?` + * (`getUserYearSettings`). Named `findForUser`, not `find` — + * `EntityRepository#find` already exists with an incompatible signature. + */ + async findForUser(userId: number): Promise { + const row = await this.findOne({ user: userId }); + return row ? (toRow(row) as VacayUserSettingsRow) : null; + } + + /** + * VC6 — `INSERT INTO vacay_user_settings (user_id, year_type, + * year_start_month, year_start_day, hire_date) VALUES (?, ?, ?, ?, ?) ON + * CONFLICT(user_id) DO UPDATE SET year_type = excluded.year_type, + * year_start_month = excluded.year_start_month, year_start_day = + * excluded.year_start_day, hire_date = excluded.hire_date` + * (`updateYearSettings`). `user` is the entity's own primary key, so + * `onConflictFields: ['user']` targets it directly — the default `'merge'` + * action already updates every OTHER column present in the payload (all + * four here, matching the legacy statement's own exhaustive `SET` list), + * so no `onConflictMergeFields` narrowing is needed (unlike + * `VacayUserYearsRepository.upsertCarriedOver`, which deliberately narrows + * to ONE column). Named `upsertSettings`, not `upsert` — + * `EntityRepository#upsert` already exists with an incompatible signature. + */ + async upsertSettings(userId: number, yearType: string, yearStartMonth: number, yearStartDay: number, hireDate: string | null): Promise { + await this.upsert( + { user: userId, year_type: yearType, year_start_month: yearStartMonth, year_start_day: yearStartDay, hire_date: hireDate }, + { onConflictFields: ['user'], onConflictAction: 'merge' }, + ); + } +} diff --git a/server/src/db/repositories/VacayUserYears.repository.ts b/server/src/db/repositories/VacayUserYears.repository.ts index a77dabb7e..4a96a8ca9 100644 --- a/server/src/db/repositories/VacayUserYears.repository.ts +++ b/server/src/db/repositories/VacayUserYears.repository.ts @@ -1,4 +1,104 @@ import type { VacayUserYears } from '../entities/VacayUserYears.entity'; +import { toRow, type AssertRowKeys } from './_shared/rows'; import { TrekRepository } from './_shared/trek-repository'; -export class VacayUserYearsRepository extends TrekRepository {} +/** A `vacay_user_years` row as the legacy `SELECT *` returned it. */ +export interface VacayUserYearRow { + id: number; + user_id: number; + plan_id: number; + year: number; + vacation_days: number | null; + carried_over: number | null; +} + +const _vacayUserYearRowKeys: AssertRowKeys = true; + +export class VacayUserYearsRepository extends TrekRepository { + /** + * VC34/VC98/VC109/VC123 — `SELECT * FROM vacay_user_years WHERE user_id = ? + * AND plan_id = ? AND year = ?` (four identical-text call sites: + * `updatePlan`'s carry-over chain, `addYear`, `deleteYear`, `getStats`). + * Named `findForYear`, not `find` — `EntityRepository#find` already exists + * with an incompatible signature. + */ + async findForYear(userId: number, planId: number, year: number): Promise { + const row = await this.findOne({ user: userId, plan: planId, year }); + return row ? (toRow(row) as VacayUserYearRow) : null; + } + + /** VC55 — `SELECT * FROM vacay_user_years WHERE user_id = ? AND plan_id = ?` (`acceptInvite`'s own-plan-to-date migration read, no `year` filter). */ + async listForUserAndPlan(userId: number, planId: number): Promise { + const rows = await this.find({ user: userId, plan: planId }); + return rows.map((row) => toRow(row) as VacayUserYearRow); + } + + /** + * VC11/VC56/VC64/VC99 — `INSERT OR IGNORE INTO vacay_user_years (user_id, + * plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, ?, ?)`, one + * method for all four call sites (`getOwnPlan`'s lazy seed, `acceptInvite`'s + * carried-over migration, its all-plan-years seed, `addYear`'s per-user + * seed) — two sites bind `vacationDays`/`carriedOver` as literal `30`/`0` + * at the CALL SITE, one binds the actual prior values, one binds a + * JS-computed carry-over; the VALUES an `INSERT OR IGNORE` writes are + * identical whichever way the caller spells them (a literal `30` in the + * legacy SQL text inserts the exact same row a bound `30` does), so a + * single flexible method, not four near-duplicates. + */ + async insertIgnore(userId: number, planId: number, year: number, vacationDays: number, carriedOver: number): Promise { + await this.upsert( + { user: userId, plan: planId, year, vacation_days: vacationDays, carried_over: carriedOver }, + { onConflictFields: ['user', 'plan', 'year'], onConflictAction: 'ignore' }, + ); + } + + /** + * VC35/VC126 — `INSERT INTO vacay_user_years (user_id, plan_id, year, + * vacation_days, carried_over) VALUES (?, ?, ?, 30, ?) ON CONFLICT(user_id, + * plan_id, year) DO UPDATE SET carried_over = ?` (`updatePlan`'s + * carry-over recompute chain, `getStats`'s per-request write — + * byte-identical statement, two call sites). `onConflictMergeFields: + * ['carried_over']` is load-bearing (`PlaceRatingsRepository.upsertRating`'s + * precedent): without it `em.upsert`'s default merge set would also touch + * `vacation_days` on a conflict, which the legacy `DO UPDATE SET + * carried_over = ?` never does — `vacation_days: 30` in the payload below + * is only ever written on the INSERT branch, exactly as the legacy + * statement's own literal `30` is. + */ + async upsertCarriedOver(userId: number, planId: number, year: number, carriedOver: number): Promise { + await this.upsert( + { user: userId, plan: planId, year, vacation_days: 30, carried_over: carriedOver }, + { onConflictFields: ['user', 'plan', 'year'], onConflictAction: 'merge', onConflictMergeFields: ['carried_over'] }, + ); + } + + /** + * VC127 — `INSERT INTO vacay_user_years (user_id, plan_id, year, + * vacation_days, carried_over) VALUES (?, ?, ?, ?, 0) ON CONFLICT(user_id, + * plan_id, year) DO UPDATE SET vacation_days = excluded.vacation_days` + * (`updateStats`) — a distinct conflict ACTION from {@link upsertCarriedOver} + * (merges `vacation_days`, leaves `carried_over` untouched on conflict; the + * literal `carried_over: 0` below is INSERT-branch-only, same reasoning). + */ + async upsertVacationDays(userId: number, planId: number, year: number, vacationDays: number): Promise { + await this.upsert( + { user: userId, plan: planId, year, vacation_days: vacationDays, carried_over: 0 }, + { onConflictFields: ['user', 'plan', 'year'], onConflictAction: 'merge', onConflictMergeFields: ['vacation_days'] }, + ); + } + + /** VC32 — `UPDATE vacay_user_years SET carried_over = 0 WHERE plan_id = ?` (`updatePlan`'s carry-over-disabled reset). */ + async resetCarriedOverForPlan(planId: number): Promise { + await this.nativeUpdate({ plan: planId }, { carried_over: 0 }); + } + + /** VC105 — `DELETE FROM vacay_user_years WHERE plan_id = ? AND year = ?` (`deleteYear`). */ + async deleteForYear(planId: number, year: number): Promise { + await this.nativeDelete({ plan: planId, year }); + } + + /** VC110 — `UPDATE vacay_user_years SET carried_over = ? WHERE user_id = ? AND plan_id = ? AND year = ?` (`deleteYear`'s year+1 carry-over recompute). */ + async updateCarriedOver(userId: number, planId: number, year: number, carriedOver: number): Promise { + await this.nativeUpdate({ user: userId, plan: planId, year }, { carried_over: carriedOver }); + } +} diff --git a/server/src/db/repositories/VacayYears.repository.ts b/server/src/db/repositories/VacayYears.repository.ts index f758dcf16..035420b85 100644 --- a/server/src/db/repositories/VacayYears.repository.ts +++ b/server/src/db/repositories/VacayYears.repository.ts @@ -1,4 +1,55 @@ import type { VacayYears } from '../entities/VacayYears.entity'; import { TrekRepository } from './_shared/trek-repository'; -export class VacayYearsRepository extends TrekRepository {} +export class VacayYearsRepository extends TrekRepository { + /** + * VC23/VC33/VC63/VC94 — `SELECT year FROM vacay_years WHERE plan_id = ?` + * (VC23/VC63, unordered — `applyHolidayCalendars`, `acceptInvite`'s year + * seeding, both iterate the set without caring about order) and `... ORDER + * BY year` (VC33/VC94 — `updatePlan`'s carry-over chain, which walks + * consecutive years and DOES need ascending order, and `listYears`, whose + * caller renders them in order). One method, always ordered: adding + * `ORDER BY year` to the two order-independent call sites changes nothing + * about the ROW SET either reads (same rows, just enumerated consistently) + * — flagged here as a non-behavioral consolidation, not a data-shape + * change, so four near-duplicate statements don't become four near- + * duplicate methods. + */ + async listForPlan(planId: number): Promise { + const rows = await this.find({ plan: planId }, { fields: ['year'], orderBy: { year: 'asc' } }); + return rows.map((row) => row.year); + } + + /** VC95/VC106 — `SELECT id FROM vacay_years WHERE plan_id = ? AND year = ?` (`addYear`'s duplicate-year guard, `deleteYear`'s "does year+1 still exist" check). */ + async exists(planId: number, year: number): Promise { + const row = await this.findOne({ plan: planId, year }, { fields: ['id'] }); + return row !== null; + } + + /** VC10 — `INSERT OR IGNORE INTO vacay_years (plan_id, year) VALUES (?, ?)` (`getOwnPlan`'s lazy year-seed). */ + async insertIgnore(planId: number, year: number): Promise { + await this.upsert({ plan: planId, year }, { onConflictFields: ['plan', 'year'], onConflictAction: 'ignore' }); + } + + /** + * VC96 — `INSERT INTO vacay_years (plan_id, year) VALUES (?, ?)` (`addYear`'s + * plain insert, already guarded by {@link exists} outside the transaction — + * no `OR IGNORE` needed here, unlike {@link insertIgnore} above). Named + * `insertYear`, not `insert` — `EntityRepository#insert` already exists + * with an incompatible signature. + */ + async insertYear(planId: number, year: number): Promise { + await this.insert({ plan: planId, year }); + } + + /** VC108 — `SELECT year FROM vacay_years WHERE plan_id = ? AND year < ? ORDER BY year DESC LIMIT 1` (`deleteYear`'s new-previous-year lookup). */ + async previousYear(planId: number, beforeYear: number): Promise { + const row = await this.findOne({ plan: planId, year: { $lt: beforeYear } }, { fields: ['year'], orderBy: { year: 'desc' } }); + return row?.year ?? null; + } + + /** VC100 — `DELETE FROM vacay_years WHERE plan_id = ? AND year = ?`. */ + async deleteForPlanAndYear(planId: number, year: number): Promise { + await this.nativeDelete({ plan: planId, year }); + } +} diff --git a/server/src/nest/vacay/vacay.module.ts b/server/src/nest/vacay/vacay.module.ts index f40c3edb1..bb9224f21 100644 --- a/server/src/nest/vacay/vacay.module.ts +++ b/server/src/nest/vacay/vacay.module.ts @@ -1,4 +1,5 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { VacayController } from './vacay.controller'; import { VacayService } from './vacay.service'; import { VacayRpc } from './vacay.rpc'; @@ -7,14 +8,44 @@ import { VacayMcp } from './vacay.mcp'; import { AuthModule } from '../auth/auth.module'; import { AddonsModule } from '../addons/addons.module'; import { NotificationsModule } from '../notifications/notifications.module'; +import { VacayPlans } from '../../db/entities/VacayPlans.entity'; +import { VacayPlanMembers } from '../../db/entities/VacayPlanMembers.entity'; +import { VacayYears } from '../../db/entities/VacayYears.entity'; +import { VacayUserYears } from '../../db/entities/VacayUserYears.entity'; +import { VacayUserColors } from '../../db/entities/VacayUserColors.entity'; +import { VacayEntries } from '../../db/entities/VacayEntries.entity'; +import { VacayCompanyHolidays } from '../../db/entities/VacayCompanyHolidays.entity'; +import { VacayHolidayCalendars } from '../../db/entities/VacayHolidayCalendars.entity'; +import { VacayShares } from '../../db/entities/VacayShares.entity'; +import { VacayUserSettings } from '../../db/entities/VacayUserSettings.entity'; +import { SchoolHolidayRegions } from '../../db/entities/SchoolHolidayRegions.entity'; /** * Vacay addon domain (S1 — Phase 2 trip sub-domain). Registered in AppModule. * VacayService is exported for the plugin host surface (VacayRpc); * VacayMcp carries the DI-discovered MCP tools/resources. + * + * `MikroOrmModule.forFeature([...])` (Plan 3f Task 5) registers every + * repository `VacayService`'s constructor injects via `@InjectRepository`: + * the ten vacay tables plus `SchoolHolidayRegions` (Task 2's, additive — + * `validateManualRegion`'s VC45 cross-domain read consumes its + * `existsForCountry` method; this module never writes to that table). No + * OTHER module constructs `VacayService` directly (`TripsModule` only + * imports `VacayModule` and injects the exported provider), so this is the + * only `forFeature` array the BOOT GATE requires updating. */ @Module({ - imports: [NotificationsModule, AuthModule, PluginGuardsModule, AddonsModule], + imports: [ + NotificationsModule, + AuthModule, + PluginGuardsModule, + AddonsModule, + MikroOrmModule.forFeature([ + VacayPlans, VacayPlanMembers, VacayYears, VacayUserYears, VacayUserColors, + VacayEntries, VacayCompanyHolidays, VacayHolidayCalendars, VacayShares, VacayUserSettings, + SchoolHolidayRegions, + ]), + ], controllers: [VacayController], providers: [VacayService, VacayMcp, VacayRpc], exports: [VacayService], diff --git a/server/src/nest/vacay/vacay.service.ts b/server/src/nest/vacay/vacay.service.ts index 5512e27b7..1ff66010d 100644 --- a/server/src/nest/vacay/vacay.service.ts +++ b/server/src/nest/vacay/vacay.service.ts @@ -1,10 +1,32 @@ import { Injectable, BadRequestException } from '@nestjs/common'; +import { InjectRepository } from '@mikro-orm/nestjs'; import { manualSchoolRegionId } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; -import { DatabaseService } from '../database/database.service'; import { UnitOfWork } from '../database/unit-of-work'; import { NotificationsService } from '../notifications/notifications.service'; import { discardBody, readCappedJson } from '../../utils/cappedFetch'; +import { VacayPlans } from '../../db/entities/VacayPlans.entity'; +import type { VacayPlansRepository } from '../../db/repositories/VacayPlans.repository'; +import { VacayPlanMembers } from '../../db/entities/VacayPlanMembers.entity'; +import type { VacayPlanMembersRepository } from '../../db/repositories/VacayPlanMembers.repository'; +import { VacayYears } from '../../db/entities/VacayYears.entity'; +import type { VacayYearsRepository } from '../../db/repositories/VacayYears.repository'; +import { VacayUserYears } from '../../db/entities/VacayUserYears.entity'; +import type { VacayUserYearsRepository } from '../../db/repositories/VacayUserYears.repository'; +import { VacayUserColors } from '../../db/entities/VacayUserColors.entity'; +import type { VacayUserColorsRepository } from '../../db/repositories/VacayUserColors.repository'; +import { VacayEntries } from '../../db/entities/VacayEntries.entity'; +import type { VacayEntriesRepository } from '../../db/repositories/VacayEntries.repository'; +import { VacayCompanyHolidays } from '../../db/entities/VacayCompanyHolidays.entity'; +import type { VacayCompanyHolidaysRepository } from '../../db/repositories/VacayCompanyHolidays.repository'; +import { VacayHolidayCalendars } from '../../db/entities/VacayHolidayCalendars.entity'; +import type { VacayHolidayCalendarsRepository } from '../../db/repositories/VacayHolidayCalendars.repository'; +import { VacayShares } from '../../db/entities/VacayShares.entity'; +import type { VacaySharesRepository } from '../../db/repositories/VacayShares.repository'; +import { VacayUserSettings as VacayUserSettingsEntity } from '../../db/entities/VacayUserSettings.entity'; +import type { VacayUserSettingsRepository } from '../../db/repositories/VacayUserSettings.repository'; +import { SchoolHolidayRegions } from '../../db/entities/SchoolHolidayRegions.entity'; +import type { SchoolHolidayRegionsRepository } from '../../db/repositories/SchoolHolidayRegions.repository'; // --------------------------------------------------------------------------- // Types @@ -13,14 +35,14 @@ import { discardBody, readCappedJson } from '../../utils/cappedFetch'; export interface VacayPlan { id: number; owner_id: number; - block_weekends: number; - holidays_enabled: number; + block_weekends: number | null; + holidays_enabled: number | null; holidays_region: string | null; - school_holidays_enabled: number; - company_holidays_enabled: number; - carry_over_enabled: number; + school_holidays_enabled: number | null; + company_holidays_enabled: number | null; + carry_over_enabled: number | null; weekend_days: string | null; - week_start: number | null; + week_start: number; } export interface VacayUserYear { @@ -165,12 +187,12 @@ function windowEndYear(end: string): number { } /** - * Vacay domain service — owns the vacay SQL (moved 1:1 from the legacy - * services/vacayService.ts: identical statements, the `||` falsy-coercion - * defaults next to `??` ones, the post-write re-selects and the dynamic - * SET-list updates). Broadcasts go straight to `broadcastToUser` inside the - * same try/catch swallows the legacy lazy require sat in; notifications stay - * fire-and-forget dynamic imports. + * Vacay domain service — owns the vacay SQL, now through the ten vacay + * repositories (+ `SchoolHolidayRegionsRepository`'s cross-domain read, + * Task 2's) instead of `DatabaseService` (Plan 3f Task 5). Broadcasts go + * straight to `RealtimeService` inside the same try/catch swallows the + * legacy lazy require sat in; notifications stay fire-and-forget dynamic + * imports. * * Post-migration fixes on top of the relocated legacy behavior: the * multi-statement writes (acceptInvite, dissolvePlan, deleteYear, updatePlan's @@ -181,11 +203,33 @@ function windowEndYear(end: string): number { * All consumers are in-container since the trip fold (TripsService injects * this class); vacay.bridge.ts was deleted with its last outside-container * consumer. + * + * R6 (this task): `removeShare`/`setShareHidden` no longer read an UNSCOPED + * `vacay_shares` row and check ownership in JS — `VacaySharesRepository + * .findScopedForRemoval`/`.findScopedForHide` scope the SAME two checks IN + * the SQL statement itself (two distinct methods, not one with a mode flag — + * see that repository's docstrings). Behaviorally identical to the legacy + * shape on every input (the JS check ran before any mutation either way), + * flagged as a genuine SQL-shape tightening per the plan's "For the user" + * note. `getStats` (VC126) now wraps its per-request carry-over write in + * `uow.transactional`, matching every OTHER multi-row write loop in this + * file (plan3f-inputs.md correction #7's "wrap it, flag it" default) — + * flagged here rather than left silently un-transacted either way. */ @Injectable() export class VacayService { constructor( - private readonly db: DatabaseService, + @InjectRepository(VacayPlans) private readonly plans: VacayPlansRepository, + @InjectRepository(VacayPlanMembers) private readonly members: VacayPlanMembersRepository, + @InjectRepository(VacayYears) private readonly years: VacayYearsRepository, + @InjectRepository(VacayUserYears) private readonly userYears: VacayUserYearsRepository, + @InjectRepository(VacayUserColors) private readonly userColors: VacayUserColorsRepository, + @InjectRepository(VacayEntries) private readonly entries: VacayEntriesRepository, + @InjectRepository(VacayCompanyHolidays) private readonly companyHolidays: VacayCompanyHolidaysRepository, + @InjectRepository(VacayHolidayCalendars) private readonly holidayCalendars: VacayHolidayCalendarsRepository, + @InjectRepository(VacayShares) private readonly shares: VacaySharesRepository, + @InjectRepository(VacayUserSettingsEntity) private readonly userSettings: VacayUserSettingsRepository, + @InjectRepository(SchoolHolidayRegions) private readonly schoolHolidayRegions: SchoolHolidayRegionsRepository, private readonly realtime: RealtimeService, private readonly notifications: NotificationsService, private readonly uow: UnitOfWork, @@ -208,21 +252,13 @@ export class VacayService { // 'vacation' by default. The window (#737) is the user's leave-year period; for // 'calendar' it is Jan 1 – Dec 31, byte-identical to the old date-prefix match. const { start, end } = await this.resolveYearWindow(userId, year); - const row = this.db.get<{ used: number }>( - "SELECT COALESCE(SUM(CASE WHEN kind = 'comp' THEN 0 ELSE fraction END), 0) AS used FROM vacay_entries WHERE user_id = ? AND plan_id = ? AND date >= ? AND date < ?", - userId, planId, start, end - )!; - return row.used; + return this.entries.sumFraction(userId, planId, start, end); } /** Comp/Flex days (#1074) used in a user's leave-year period — SUM of fractions for kind='comp'. */ private async compUsedDays(userId: number, planId: number, year: number): Promise { const { start, end } = await this.resolveYearWindow(userId, year); - const row = this.db.get<{ used: number }>( - "SELECT COALESCE(SUM(fraction), 0) AS used FROM vacay_entries WHERE user_id = ? AND plan_id = ? AND date >= ? AND date < ? AND kind = 'comp'", - userId, planId, start, end - )!; - return row.used; + return this.entries.sumCompFraction(userId, planId, start, end); } // ------------------------------------------------------------------------- @@ -230,7 +266,8 @@ export class VacayService { // ------------------------------------------------------------------------- async getUserYearSettings(userId: number): Promise { - return this.db.get('SELECT * FROM vacay_user_settings WHERE user_id = ?', userId); + const row = await this.userSettings.findForUser(userId); + return row ? { user_id: row.user_id, year_type: row.year_type as VacayUserSettings['year_type'], year_start_month: row.year_start_month, year_start_day: row.year_start_day, hire_date: row.hire_date } : undefined; } /** A user's leave-year settings with the calendar defaults filled in (#737). */ @@ -322,12 +359,7 @@ export class VacayService { const month = Math.min(12, Math.max(1, Number.parseInt(String(data.year_start_month ?? 1), 10) || 1)); const day = Math.min(31, Math.max(1, Number.parseInt(String(data.year_start_day ?? 1), 10) || 1)); const hire = typeof data.hire_date === 'string' && /^\d{4}-\d{2}-\d{2}$/.test(data.hire_date) ? data.hire_date : null; - this.db.run(` - INSERT INTO vacay_user_settings (user_id, year_type, year_start_month, year_start_day, hire_date) - VALUES (?, ?, ?, ?, ?) - ON CONFLICT(user_id) DO UPDATE SET year_type = excluded.year_type, year_start_month = excluded.year_start_month, - year_start_day = excluded.year_start_day, hire_date = excluded.hire_date - `, userId, type, month, day, hire); + await this.userSettings.upsertSettings(userId, type, month, day, hire); return (await this.getUserYearSettings(userId))!; } @@ -336,26 +368,24 @@ export class VacayService { // ------------------------------------------------------------------------- async getOwnPlan(userId: number): Promise { - let plan = this.db.get('SELECT * FROM vacay_plans WHERE owner_id = ?', userId); + let plan = await this.plans.findByOwner(userId); if (!plan) { - this.db.run('INSERT INTO vacay_plans (owner_id) VALUES (?)', userId); - plan = this.db.get('SELECT * FROM vacay_plans WHERE owner_id = ?', userId)!; + await this.plans.insertForOwner(userId); + plan = (await this.plans.findByOwner(userId))!; // Seed the period today falls into — with a shifted leave year (#737) that is // not necessarily the current calendar year. const yr = await this.currentPeriodYear(userId); - this.db.run('INSERT OR IGNORE INTO vacay_years (plan_id, year) VALUES (?, ?)', plan.id, yr); - this.db.run('INSERT OR IGNORE INTO vacay_user_years (user_id, plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, 30, 0)', userId, plan.id, yr); - this.db.run('INSERT OR IGNORE INTO vacay_user_colors (user_id, plan_id, color) VALUES (?, ?, ?)', userId, plan.id, '#6366f1'); + await this.years.insertIgnore(plan.id, yr); + await this.userYears.insertIgnore(userId, plan.id, yr, 30, 0); + await this.userColors.insertIgnore(userId, plan.id, '#6366f1'); } return plan; } async getActivePlan(userId: number): Promise { - const membership = this.db.get<{ plan_id: number }>(` - SELECT plan_id FROM vacay_plan_members WHERE user_id = ? AND status = 'accepted' - `, userId); + const membership = await this.members.findAcceptedPlanId(userId); if (membership) { - return this.db.get('SELECT * FROM vacay_plans WHERE id = ?', membership.plan_id)!; + return (await this.plans.findById(membership.plan_id))!; } return await this.getOwnPlan(userId); } @@ -370,34 +400,23 @@ export class VacayService { oldEnd: string, newStart: string ): Promise { - const row = this.db.get<{ days: number }>( - 'SELECT CAST(julianday(?) - julianday(?) AS INTEGER) AS days', - newStart, oldStart - ); - const offset = row?.days ?? 0; + // VC4 (R9's verified restructured shape): `CAST(julianday(?) - julianday(?) + // AS INTEGER)` on two BOUND VALUES is plain JS date-diff arithmetic — + // verified against the SQL on 5 date pairs incl. a leap-year boundary and a + // negative offset, all matched exactly (task-0-report.md). + const offset = Math.round((Date.parse(newStart) - Date.parse(oldStart)) / 86400000); if (offset === 0) return; const plan = await this.getOwnPlan(ownerId); - this.db.run( - `UPDATE OR IGNORE vacay_entries - SET date = date(date, ? || ' days') - WHERE plan_id = ? - AND user_id = ? - AND date BETWEEN ? AND ?`, - `${offset >= 0 ? '+' : ''}${offset}`, plan.id, ownerId, oldStart, oldEnd - ); + await this.entries.shiftForOwnerWindow(plan.id, ownerId, oldStart, oldEnd, offset); } async getPlanUsers(planId: number): Promise { - const plan = this.db.get('SELECT * FROM vacay_plans WHERE id = ?', planId); + const plan = await this.plans.findById(planId); if (!plan) return []; - const owner = this.db.get('SELECT id, username, email FROM users WHERE id = ?', plan.owner_id)!; - const members = this.db.all(` - SELECT u.id, u.username, u.email FROM vacay_plan_members m - JOIN users u ON m.user_id = u.id - WHERE m.plan_id = ? AND m.status = 'accepted' - `, planId); + const owner = (await this.plans.findVacayUser(plan.owner_id))!; + const members = await this.members.listAcceptedWithUsers(planId); return [owner, ...members]; } @@ -411,10 +430,10 @@ export class VacayService { event: 'vacay:update' | 'vacay:settings' | 'vacay:accepted' | 'vacay:declined' = 'vacay:update', ): Promise { try { - const plan = this.db.get<{ owner_id: number }>('SELECT owner_id FROM vacay_plans WHERE id = ?', planId); + const plan = await this.plans.findOwnerId(planId); if (!plan) return; const userIds = [plan.owner_id]; - const members = this.db.all<{ user_id: number }>("SELECT user_id FROM vacay_plan_members WHERE plan_id = ? AND status = 'accepted'", planId); + const members = await this.members.listAcceptedUserIds(planId); members.forEach(m => userIds.push(m.user_id)); userIds.forEach(id => this.realtime.broadcastToUser(id, { type: event }, excludeSid)); // Pending-invite events carry nothing a read-only viewer could see; every @@ -430,11 +449,8 @@ export class VacayService { async notifyShareViewers(ownerIds: number[], excludeSid?: string): Promise { if (ownerIds.length === 0) return; try { - const rows = this.db.all<{ user_id: number }>( - `SELECT DISTINCT user_id FROM vacay_shares WHERE owner_id IN (${ownerIds.map(() => '?').join(',')})`, - ...ownerIds - ); - rows.forEach(r => this.realtime.broadcastToUser(r.user_id, { type: 'vacay:shared-update' }, excludeSid)); + const viewerIds = await this.shares.listDistinctViewerIdsForOwners(ownerIds); + viewerIds.forEach(id => this.realtime.broadcastToUser(id, { type: 'vacay:shared-update' }, excludeSid)); } catch { /* websocket not available */ } } @@ -443,17 +459,17 @@ export class VacayService { // ------------------------------------------------------------------------- async applyHolidayCalendars(planId: number): Promise { - const plan = this.db.get<{ holidays_enabled: number }>('SELECT holidays_enabled FROM vacay_plans WHERE id = ?', planId); - if (!plan?.holidays_enabled) return; - const calendars = this.db.all("SELECT * FROM vacay_holiday_calendars WHERE plan_id = ? AND type = 'public_holiday' ORDER BY sort_order, id", planId); + const holidaysEnabled = await this.plans.getHolidaysEnabled(planId); + if (!holidaysEnabled) return; + const calendars = await this.holidayCalendars.listPublicForPlan(planId); if (calendars.length === 0) return; - const years = this.db.all<{ year: number }>('SELECT year FROM vacay_years WHERE plan_id = ?', planId); + const years = await this.years.listForPlan(planId); // A shifted leave year (#737) runs into the next calendar year, so collect the // calendar years the members' windows actually touch — not just the period ids. // With everyone on 'calendar' this is the same set as before. const members = await this.getPlanUsers(planId); const calendarYears = new Set(); - for (const { year } of years) { + for (const year of years) { calendarYears.add(year); for (const m of members) calendarYears.add(windowEndYear((await this.resolveYearWindow(m.id, year)).end)); } @@ -478,8 +494,8 @@ export class VacayService { if (hasRegions && !region) continue; for (const h of holidays) { if (h.global || !h.counties || (region && h.counties.includes(region))) { - this.db.run('DELETE FROM vacay_entries WHERE plan_id = ? AND date = ?', planId, h.date); - this.db.run('DELETE FROM vacay_company_holidays WHERE plan_id = ? AND date = ?', planId, h.date); + await this.entries.deleteForPlanAndDate(planId, h.date); + await this.companyHolidays.deleteForPlanAndDate(planId, h.date); } } } catch { /* API error, skip */ } @@ -488,13 +504,10 @@ export class VacayService { } async migrateHolidayCalendars(planId: number, plan: VacayPlan): Promise { - const existing = this.db.get('SELECT id FROM vacay_holiday_calendars WHERE plan_id = ?', planId); + const existing = await this.holidayCalendars.existsForPlan(planId); if (existing) return; if (plan.holidays_enabled && plan.holidays_region) { - this.db.run( - 'INSERT INTO vacay_holiday_calendars (plan_id, region, label, color, sort_order) VALUES (?, ?, NULL, ?, 0)', - planId, plan.holidays_region, '#fecaca' - ); + await this.holidayCalendars.insertCalendar(planId, 'public_holiday', plan.holidays_region, null, '#fecaca', 0); } } @@ -505,55 +518,48 @@ export class VacayService { async updatePlan(planId: number, body: UpdatePlanBody, socketId: string | undefined) { const { block_weekends, holidays_enabled, holidays_region, school_holidays_enabled, company_holidays_enabled, carry_over_enabled, weekend_days, week_start } = body; - const updates: string[] = []; - const params: (string | number | null)[] = []; - if (block_weekends !== undefined) { updates.push('block_weekends = ?'); params.push(block_weekends ? 1 : 0); } - if (holidays_enabled !== undefined) { updates.push('holidays_enabled = ?'); params.push(holidays_enabled ? 1 : 0); } - if (holidays_region !== undefined) { updates.push('holidays_region = ?'); params.push(holidays_region); } - if (school_holidays_enabled !== undefined) { updates.push('school_holidays_enabled = ?'); params.push(school_holidays_enabled ? 1 : 0); } - if (company_holidays_enabled !== undefined) { updates.push('company_holidays_enabled = ?'); params.push(company_holidays_enabled ? 1 : 0); } - if (carry_over_enabled !== undefined) { updates.push('carry_over_enabled = ?'); params.push(carry_over_enabled ? 1 : 0); } - if (weekend_days !== undefined) { updates.push('weekend_days = ?'); params.push(String(weekend_days)); } - if (week_start !== undefined) { updates.push('week_start = ?'); params.push(week_start === 0 ? 0 : 1); } + const patch: { block_weekends?: number; holidays_enabled?: number; holidays_region?: string | null; school_holidays_enabled?: number; company_holidays_enabled?: number; carry_over_enabled?: number; weekend_days?: string; week_start?: number } = {}; + if (block_weekends !== undefined) patch.block_weekends = block_weekends ? 1 : 0; + if (holidays_enabled !== undefined) patch.holidays_enabled = holidays_enabled ? 1 : 0; + if (holidays_region !== undefined) patch.holidays_region = holidays_region; + if (school_holidays_enabled !== undefined) patch.school_holidays_enabled = school_holidays_enabled ? 1 : 0; + if (company_holidays_enabled !== undefined) patch.company_holidays_enabled = company_holidays_enabled ? 1 : 0; + if (carry_over_enabled !== undefined) patch.carry_over_enabled = carry_over_enabled ? 1 : 0; + if (weekend_days !== undefined) patch.weekend_days = String(weekend_days); + if (week_start !== undefined) patch.week_start = week_start === 0 ? 0 : 1; - if (updates.length > 0) { - params.push(planId); - this.db.run(`UPDATE vacay_plans SET ${updates.join(', ')} WHERE id = ?`, ...params); - } + await this.plans.update(planId, patch); if (company_holidays_enabled === true) { - const companyDates = this.db.all<{ date: string }>('SELECT date FROM vacay_company_holidays WHERE plan_id = ?', planId); + const companyDates = await this.companyHolidays.listForPlan(planId); for (const { date } of companyDates) { - this.db.run('DELETE FROM vacay_entries WHERE plan_id = ? AND date = ?', planId, date); + await this.entries.deleteForPlanAndDate(planId, date); } } - const updatedPlan = this.db.get('SELECT * FROM vacay_plans WHERE id = ?', planId)!; + const updatedPlan = (await this.plans.findById(planId))!; await this.migrateHolidayCalendars(planId, updatedPlan); await this.applyHolidayCalendars(planId); if (carry_over_enabled === false) { - this.db.run('UPDATE vacay_user_years SET carried_over = 0 WHERE plan_id = ?', planId); + await this.userYears.resetCarriedOverForPlan(planId); } if (carry_over_enabled === true) { // The chained per-year/per-user recompute is atomic — a failure mid-chain // would otherwise leave later years carrying stale balances. await this.uow.transactional(async () => { - const years = this.db.all<{ year: number }>('SELECT year FROM vacay_years WHERE plan_id = ? ORDER BY year', planId); + const years = await this.years.listForPlan(planId); const users = await this.getPlanUsers(planId); for (let i = 0; i < years.length - 1; i++) { - const yr = years[i].year; - const nextYr = years[i + 1].year; + const yr = years[i]; + const nextYr = years[i + 1]; for (const u of users) { const used = await this.usedDays(u.id, planId, yr); - const config = this.db.get('SELECT * FROM vacay_user_years WHERE user_id = ? AND plan_id = ? AND year = ?', u.id, planId, yr); - const total = (config ? config.vacation_days : 30) + (config ? config.carried_over : 0); + const config = await this.userYears.findForYear(u.id, planId, yr); + const total = (config ? config.vacation_days ?? 30 : 30) + (config ? config.carried_over ?? 0 : 0); const carry = Math.max(0, total - used); - this.db.run(` - INSERT INTO vacay_user_years (user_id, plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, 30, ?) - ON CONFLICT(user_id, plan_id, year) DO UPDATE SET carried_over = ? - `, u.id, planId, nextYr, carry, carry); + await this.userYears.upsertCarriedOver(u.id, planId, nextYr, carry); } } }); @@ -561,8 +567,8 @@ export class VacayService { await this.notifyPlanUsers(planId, socketId, 'vacay:settings'); - const updated = this.db.get('SELECT * FROM vacay_plans WHERE id = ?', planId)!; - const updatedCalendars = this.db.all('SELECT * FROM vacay_holiday_calendars WHERE plan_id = ? ORDER BY sort_order, id', planId); + const updated = (await this.plans.findById(planId))!; + const updatedCalendars = await this.holidayCalendars.listForPlan(planId); return { plan: { ...updated, @@ -582,11 +588,10 @@ export class VacayService { async addHolidayCalendar(planId: number, region: string, label: string | null, color: string | undefined, sortOrder: number | undefined, socketId: string | undefined, type: 'public_holiday' | 'school_holiday' = 'public_holiday') { await this.validateManualRegion(region, type); - const result = this.db.run( - 'INSERT INTO vacay_holiday_calendars (plan_id, type, region, label, color, sort_order) VALUES (?, ?, ?, ?, ?, ?)', - planId, type, region, label || null, color || (type === 'school_holiday' ? '#a5f3fc' : '#fecaca'), sortOrder ?? 0 + const id = await this.holidayCalendars.insertCalendar( + planId, type, region, label || null, color || (type === 'school_holiday' ? '#a5f3fc' : '#fecaca'), sortOrder ?? 0, ); - const cal = this.db.get('SELECT * FROM vacay_holiday_calendars WHERE id = ?', result.lastInsertRowid)!; + const cal = (await this.holidayCalendars.findById(id))!; await this.notifyPlanUsers(planId, socketId, 'vacay:settings'); return cal; } @@ -597,38 +602,54 @@ export class VacayService { body: { region?: string; label?: string | null; color?: string; sort_order?: number; type?: 'public_holiday' | 'school_holiday' }, socketId: string | undefined, ): Promise { - const cal = this.db.get('SELECT * FROM vacay_holiday_calendars WHERE id = ? AND plan_id = ?', calId, planId); + const cal = await this.holidayCalendars.findScopedForPlan(calId, planId); if (!cal) return null; await this.validateManualRegion(body.region ?? cal.region, body.type ?? cal.type); const { region, label, color, sort_order, type } = body; - const updates: string[] = []; - const params: (string | number | null)[] = []; - if (region !== undefined) { updates.push('region = ?'); params.push(region); } - if (type !== undefined) { updates.push('type = ?'); params.push(type); } - if (label !== undefined) { updates.push('label = ?'); params.push(label); } - if (color !== undefined) { updates.push('color = ?'); params.push(color); } - if (sort_order !== undefined) { updates.push('sort_order = ?'); params.push(sort_order); } - if (updates.length > 0) { - params.push(calId); - this.db.run(`UPDATE vacay_holiday_calendars SET ${updates.join(', ')} WHERE id = ?`, ...params); - } - const updated = this.db.get('SELECT * FROM vacay_holiday_calendars WHERE id = ?', calId)!; + const patch: { region?: string; type?: 'public_holiday' | 'school_holiday'; label?: string | null; color?: string; sort_order?: number } = {}; + if (region !== undefined) patch.region = region; + if (type !== undefined) patch.type = type; + if (label !== undefined) patch.label = label; + if (color !== undefined) patch.color = color; + if (sort_order !== undefined) patch.sort_order = sort_order; + await this.holidayCalendars.update(calId, patch); + const updated = (await this.holidayCalendars.findById(calId))!; await this.notifyPlanUsers(planId, socketId, 'vacay:settings'); return updated; } async deleteHolidayCalendar(calId: number, planId: number, socketId: string | undefined): Promise { - const cal = this.db.get('SELECT * FROM vacay_holiday_calendars WHERE id = ? AND plan_id = ?', calId, planId); + const cal = await this.holidayCalendars.findScopedForPlan(calId, planId); if (!cal) return false; - this.db.run('DELETE FROM vacay_holiday_calendars WHERE id = ?', calId); + await this.holidayCalendars.deleteById(calId); await this.notifyPlanUsers(planId, socketId, 'vacay:settings'); return true; } + /** + * VC45 — `SELECT id FROM school_holiday_regions WHERE id = ? AND country = + * ?` (guards `-MANUAL-` region codes against `manualSchoolRegionId`). + * + * Deviation from the brief, flagged: the brief names + * `SchoolHolidayRegionsRepository.existsForCountry` (Task 2's) as this + * read's consumer, but that method is `SH5`'s shape — `WHERE country = ?` + * ONLY, no `id` in its predicate at all — built for `deleteCountry`'s "does + * ANY region still exist for this country" guard, a genuinely different + * question from VC45's "does THIS SPECIFIC id belong to this country". + * Calling `existsForCountry` here would accept any garbage numeric id + * (`US-MANUAL-999999`) as long as the country has AT LEAST ONE real + * region, silently weakening the guard — caught by this task's own tests + * (`addHolidayCalendar`/`updateHolidayCalendar`'s manual-region rejection + * cases went green with a wrong answer). `findById` (Task 2's OTHER, + * already-landed method, built for SH7) plus a JS-side `country` compare + * reproduces the legacy `id = ? AND country = ?` predicate exactly, still + * without editing `SchoolHolidayRegions.repository.ts`. + */ private async validateManualRegion(code: string, type: string) { if (!code.includes('-MANUAL-')) return; const id = manualSchoolRegionId(code); - if (type !== 'school_holiday' || !id || !this.db.get('SELECT id FROM school_holiday_regions WHERE id = ? AND country = ?', id, code.slice(0, 2))) { + const region = id ? await this.schoolHolidayRegions.findById(id) : null; + if (type !== 'school_holiday' || !id || !region || region.country !== code.slice(0, 2)) { throw new BadRequestException('Unknown manual school holiday region'); } } @@ -638,10 +659,7 @@ export class VacayService { // ------------------------------------------------------------------------- async setUserColor(userId: number, planId: number, color: string | undefined, socketId: string | undefined): Promise { - this.db.run(` - INSERT INTO vacay_user_colors (user_id, plan_id, color) VALUES (?, ?, ?) - ON CONFLICT(user_id, plan_id) DO UPDATE SET color = excluded.color - `, userId, planId, color || '#6366f1'); + await this.userColors.upsertColor(userId, planId, color || '#6366f1'); await this.notifyPlanUsers(planId, socketId, 'vacay:update'); } @@ -654,19 +672,19 @@ export class VacayService { // The picker no longer offers guests, but the id arrives from the client, so the // write path has to refuse them too rather than trust the list it handed out. - const targetUser = this.db.get('SELECT id, username FROM users WHERE id = ? AND COALESCE(is_guest, 0) = 0', targetUserId); + const targetUser = await this.members.findInvitableUser(targetUserId); if (!targetUser) return { error: 'User not found', status: 404 }; - const existing = this.db.get<{ id: number; status: string }>('SELECT id, status FROM vacay_plan_members WHERE plan_id = ? AND user_id = ?', planId, targetUserId); + const existing = await this.members.findMembership(planId, targetUserId); if (existing) { if (existing.status === 'accepted') return { error: 'Already fused', status: 400 }; if (existing.status === 'pending') return { error: 'Invite already pending', status: 400 }; } - const targetFusion = this.db.get("SELECT id FROM vacay_plan_members WHERE user_id = ? AND status = 'accepted'", targetUserId); + const targetFusion = await this.members.findAcceptedForUser(targetUserId); if (targetFusion) return { error: 'User is already fused with another plan', status: 400 }; - this.db.run('INSERT INTO vacay_plan_members (plan_id, user_id, status) VALUES (?, ?, ?)', planId, targetUserId, 'pending'); + await this.members.insertPending(planId, targetUserId); try { this.realtime.broadcastToUser(targetUserId, { @@ -690,43 +708,42 @@ export class VacayService { // The accept flow is a multi-statement write (status flip + entry/year/color // migration + seeding) — atomic, so a failure can't leave the member half-fused. const result = await this.uow.transactional(async (): Promise<{ error?: string; status?: number }> => { - const invite = this.db.get("SELECT * FROM vacay_plan_members WHERE plan_id = ? AND user_id = ? AND status = 'pending'", planId, userId); + const invite = await this.members.findPending(planId, userId); if (!invite) return { error: 'No pending invite', status: 404 }; - this.db.run("UPDATE vacay_plan_members SET status = 'accepted' WHERE id = ?", invite.id); + await this.members.accept(invite.id); // Migrate data from user's own plan - const ownPlan = this.db.get<{ id: number }>('SELECT id FROM vacay_plans WHERE owner_id = ?', userId); + const ownPlan = await this.plans.findIdByOwner(userId); if (ownPlan && ownPlan.id !== planId) { - this.db.run('UPDATE vacay_entries SET plan_id = ? WHERE plan_id = ? AND user_id = ?', planId, ownPlan.id, userId); - const ownYears = this.db.all('SELECT * FROM vacay_user_years WHERE user_id = ? AND plan_id = ?', userId, ownPlan.id); + await this.entries.updatePlanIdForUser(planId, ownPlan.id, userId); + const ownYears = await this.userYears.listForUserAndPlan(userId, ownPlan.id); for (const y of ownYears) { - this.db.run('INSERT OR IGNORE INTO vacay_user_years (user_id, plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, ?, ?)', userId, planId, y.year, y.vacation_days, y.carried_over); + await this.userYears.insertIgnore(userId, planId, y.year, y.vacation_days ?? 30, y.carried_over ?? 0); } - const colorRow = this.db.get<{ color: string }>('SELECT color FROM vacay_user_colors WHERE user_id = ? AND plan_id = ?', userId, ownPlan.id); + const colorRow = await this.userColors.findColor(userId, ownPlan.id); if (colorRow) { - this.db.run('INSERT OR IGNORE INTO vacay_user_colors (user_id, plan_id, color) VALUES (?, ?, ?)', userId, planId, colorRow.color); + await this.userColors.insertIgnore(userId, planId, colorRow.color ?? '#6366f1'); } } // Auto-assign unique color - const existingColors = this.db.all<{ color: string }>('SELECT color FROM vacay_user_colors WHERE plan_id = ? AND user_id != ?', planId, userId).map(r => r.color); - const myColor = this.db.get<{ color: string }>('SELECT color FROM vacay_user_colors WHERE user_id = ? AND plan_id = ?', userId, planId); + const existingColors = (await this.userColors.listOtherColors(planId, userId)).map(r => r.color).filter((c): c is string => c !== null); + const myColor = await this.userColors.findColor(userId, planId); const effectiveColor = myColor?.color || '#6366f1'; if (existingColors.includes(effectiveColor)) { const available = COLORS.find(c => !existingColors.includes(c)); if (available) { - this.db.run(`INSERT INTO vacay_user_colors (user_id, plan_id, color) VALUES (?, ?, ?) - ON CONFLICT(user_id, plan_id) DO UPDATE SET color = excluded.color`, userId, planId, available); + await this.userColors.upsertColor(userId, planId, available); } } else if (!myColor) { - this.db.run('INSERT OR IGNORE INTO vacay_user_colors (user_id, plan_id, color) VALUES (?, ?, ?)', userId, planId, effectiveColor); + await this.userColors.insertIgnore(userId, planId, effectiveColor); } // Ensure user has rows for all plan years - const targetYears = this.db.all<{ year: number }>('SELECT year FROM vacay_years WHERE plan_id = ?', planId); + const targetYears = await this.years.listForPlan(planId); for (const y of targetYears) { - this.db.run('INSERT OR IGNORE INTO vacay_user_years (user_id, plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, 30, 0)', userId, planId, y.year); + await this.userYears.insertIgnore(userId, planId, y, 30, 0); } return {}; }); @@ -738,12 +755,12 @@ export class VacayService { } async declineInvite(userId: number, planId: number, socketId: string | undefined): Promise { - this.db.run("DELETE FROM vacay_plan_members WHERE plan_id = ? AND user_id = ? AND status = 'pending'", planId, userId); + await this.members.deletePending(planId, userId); await this.notifyPlanUsers(planId, socketId, 'vacay:declined'); } async cancelInvite(planId: number, targetUserId: number): Promise { - this.db.run("DELETE FROM vacay_plan_members WHERE plan_id = ? AND user_id = ? AND status = 'pending'", planId, targetUserId); + await this.members.deletePending(planId, targetUserId); try { this.realtime.broadcastToUser(targetUserId, { type: 'vacay:cancelled' }); @@ -762,25 +779,25 @@ export class VacayService { const isOwnerFlag = plan.owner_id === userId; const userIds = (await this.getPlanUsers(plan.id)).map(u => u.id); - const companyHolidays = this.db.all<{ date: string; note: string }>('SELECT date, note FROM vacay_company_holidays WHERE plan_id = ?', plan.id); + const companyHolidayRows = await this.companyHolidays.listForPlan(plan.id); if (isOwnerFlag) { - const members = this.db.all<{ user_id: number }>("SELECT user_id FROM vacay_plan_members WHERE plan_id = ? AND status = 'accepted'", plan.id); + const members = await this.members.listAcceptedUserIds(plan.id); for (const m of members) { const memberPlan = await this.getOwnPlan(m.user_id); - this.db.run('UPDATE vacay_entries SET plan_id = ? WHERE plan_id = ? AND user_id = ?', memberPlan.id, plan.id, m.user_id); - for (const ch of companyHolidays) { - this.db.run('INSERT OR IGNORE INTO vacay_company_holidays (plan_id, date, note) VALUES (?, ?, ?)', memberPlan.id, ch.date, ch.note); + await this.entries.updatePlanIdForUser(memberPlan.id, plan.id, m.user_id); + for (const ch of companyHolidayRows) { + await this.companyHolidays.insertIgnore(memberPlan.id, ch.date, ch.note ?? ''); } } - this.db.run('DELETE FROM vacay_plan_members WHERE plan_id = ?', plan.id); + await this.members.deleteForPlan(plan.id); } else { const ownPlan = await this.getOwnPlan(userId); - this.db.run('UPDATE vacay_entries SET plan_id = ? WHERE plan_id = ? AND user_id = ?', ownPlan.id, plan.id, userId); - for (const ch of companyHolidays) { - this.db.run('INSERT OR IGNORE INTO vacay_company_holidays (plan_id, date, note) VALUES (?, ?, ?)', ownPlan.id, ch.date, ch.note); + await this.entries.updatePlanIdForUser(ownPlan.id, plan.id, userId); + for (const ch of companyHolidayRows) { + await this.companyHolidays.insertIgnore(ownPlan.id, ch.date, ch.note ?? ''); } - this.db.run('DELETE FROM vacay_plan_members WHERE plan_id = ? AND user_id = ?', plan.id, userId); + await this.members.deleteForPlanAndUser(plan.id, userId); } return userIds; }); @@ -797,17 +814,7 @@ export class VacayService { // ------------------------------------------------------------------------- async getAvailableUsers(userId: number, planId: number) { - return this.db.all(` - SELECT u.id, u.username, u.email FROM users u - WHERE u.id != ? - AND COALESCE(u.is_guest, 0) = 0 - AND u.id NOT IN (SELECT user_id FROM vacay_plan_members WHERE plan_id = ?) - AND u.id NOT IN (SELECT user_id FROM vacay_plan_members WHERE status = 'accepted') - AND u.id NOT IN (SELECT owner_id FROM vacay_plans WHERE id IN ( - SELECT plan_id FROM vacay_plan_members WHERE status = 'accepted' - )) - ORDER BY u.username - `, userId, planId); + return this.members.listAvailableForFusion(userId, planId); } // ------------------------------------------------------------------------- @@ -821,21 +828,20 @@ export class VacayService { /** Like getActivePlan, but never lazily creates a plan for the user. */ private async peekActivePlan(userId: number): Promise { - const membership = this.db.get<{ plan_id: number }>(` - SELECT plan_id FROM vacay_plan_members WHERE user_id = ? AND status = 'accepted' - `, userId); + const membership = await this.members.findAcceptedPlanId(userId); if (membership) { - return this.db.get('SELECT * FROM vacay_plans WHERE id = ?', membership.plan_id); + return (await this.plans.findById(membership.plan_id)) ?? undefined; } - return this.db.get('SELECT * FROM vacay_plans WHERE owner_id = ?', userId); + return (await this.plans.findByOwner(userId)) ?? undefined; } /** Colors already taken in the viewer's own calendar (their plan's members). */ private async viewerColors(viewerId: number): Promise> { const plan = await this.peekActivePlan(viewerId); if (!plan) return new Set(['#6366f1']); - const rows = this.db.all<{ color: string }>('SELECT color FROM vacay_user_colors WHERE plan_id = ?', plan.id); - return new Set(rows.length > 0 ? rows.map(r => r.color) : ['#6366f1']); + const rows = await this.userColors.listForPlan(plan.id); + const colors = rows.map(r => r.color).filter((c): c is string => c !== null); + return new Set(colors.length > 0 ? colors : ['#6366f1']); } /** @@ -846,9 +852,7 @@ export class VacayService { */ private async shareDisplayColor(ownerId: number, usedColors: Set): Promise { const plan = await this.peekActivePlan(ownerId); - const row = plan - ? this.db.get<{ color: string }>('SELECT color FROM vacay_user_colors WHERE user_id = ? AND plan_id = ?', ownerId, plan.id) - : undefined; + const row = plan ? await this.userColors.findColor(ownerId, plan.id) : undefined; let color = row?.color || '#6366f1'; if (usedColors.has(color)) { // Preset pool exhausted? Derive a stable per-owner hue instead of colliding. @@ -866,16 +870,8 @@ export class VacayService { async listShares(userId: number) { // Usernames only, like the share picker — emails stay out of the share surface. - const outgoing = this.db.all<{ id: number; user_id: number; username: string }>(` - SELECT s.id, s.user_id, u.username - FROM vacay_shares s JOIN users u ON s.user_id = u.id - WHERE s.owner_id = ? ORDER BY s.id - `, userId); - const incomingRows = this.db.all<{ id: number; owner_id: number; hidden: number; username: string }>(` - SELECT s.id, s.owner_id, s.hidden, u.username - FROM vacay_shares s JOIN users u ON s.owner_id = u.id - WHERE s.user_id = ? ORDER BY s.id - `, userId); + const outgoing = await this.shares.listOutgoing(userId); + const incomingRows = await this.shares.listIncoming(userId); // Shares from someone the viewer is meanwhile fused with lie dormant — the // plan already shows that calendar in full. They resume after dissolution. const coMembers = await this.viewerCoMemberIds(userId); @@ -899,10 +895,10 @@ export class VacayService { async shareCalendar(ownerId: number, ownerEmail: string, targetUserId: number, socketId?: string): Promise<{ error?: string; status?: number }> { if (targetUserId === ownerId) return { error: 'Cannot share with yourself', status: 400 }; - const targetUser = this.db.get('SELECT id FROM users WHERE id = ? AND COALESCE(is_guest, 0) = 0', targetUserId); - if (!targetUser) return { error: 'User not found', status: 404 }; + const targetOk = await this.shares.existsInvitableUser(targetUserId); + if (!targetOk) return { error: 'User not found', status: 404 }; - const existing = this.db.get('SELECT id FROM vacay_shares WHERE owner_id = ? AND user_id = ?', ownerId, targetUserId); + const existing = await this.shares.findByOwnerAndUser(ownerId, targetUserId); if (existing) return { error: 'Already shared', status: 400 }; // Plan members already see the whole calendar — sharing with them is moot. @@ -910,7 +906,7 @@ export class VacayService { return { error: 'User is already in your calendar', status: 400 }; } - this.db.run('INSERT INTO vacay_shares (owner_id, user_id) VALUES (?, ?)', ownerId, targetUserId); + await this.shares.insertShare(ownerId, targetUserId); try { this.realtime.broadcastToUser(targetUserId, { type: 'vacay:share', from: { id: ownerId } }); @@ -923,11 +919,16 @@ export class VacayService { return {}; } + /** + * R6 — `findScopedForRemoval` now scopes the ownership-or-viewer check in + * the SQL statement (`VacaySharesRepository`'s own docstring), instead of + * an unscoped `SELECT * ... WHERE id = ?` plus a JS check. Behaviorally + * identical to the legacy shape on every input. + */ async removeShare(shareId: number, userId: number, socketId?: string): Promise { - const share = this.db.get('SELECT * FROM vacay_shares WHERE id = ?', shareId); - // The owner revokes, the viewer removes — both may delete, nobody else. - if (!share || (share.owner_id !== userId && share.user_id !== userId)) return false; - this.db.run('DELETE FROM vacay_shares WHERE id = ?', shareId); + const share = await this.shares.findScopedForRemoval(shareId, userId); + if (!share) return false; + await this.shares.deleteById(shareId); try { this.realtime.broadcastToUser(share.owner_id, { type: 'vacay:share-removed' }, socketId); this.realtime.broadcastToUser(share.user_id, { type: 'vacay:share-removed' }, socketId); @@ -935,10 +936,15 @@ export class VacayService { return true; } + /** + * R6 — `findScopedForHide` scopes the VIEWER-only check (narrower than + * {@link removeShare}'s `findScopedForRemoval`, per that repository's + * class docstring: the owner of an outgoing share may not hide it). + */ async setShareHidden(shareId: number, userId: number, hidden: boolean, socketId?: string): Promise { - const share = this.db.get('SELECT * FROM vacay_shares WHERE id = ?', shareId); - if (!share || share.user_id !== userId) return false; - this.db.run('UPDATE vacay_shares SET hidden = ? WHERE id = ?', hidden ? 1 : 0, shareId); + const share = await this.shares.findScopedForHide(shareId, userId); + if (!share) return false; + await this.shares.setHidden(shareId, hidden); try { // Keep the viewer's other devices in sync; nobody else is affected. this.realtime.broadcastToUser(userId, { type: 'vacay:shared-update' }, socketId); @@ -950,29 +956,14 @@ export class VacayService { const planId = await this.getActivePlanId(userId); // Username only — unlike the fusion picker this lists users from other plans // too, so exposing their emails here would widen the instance directory. - return this.db.all(` - SELECT u.id, u.username FROM users u - WHERE u.id != ? - AND COALESCE(u.is_guest, 0) = 0 - AND u.id NOT IN (SELECT user_id FROM vacay_shares WHERE owner_id = ?) - AND u.id NOT IN ( - SELECT owner_id FROM vacay_plans WHERE id = ? - UNION - SELECT user_id FROM vacay_plan_members WHERE plan_id = ? AND status = 'accepted' - ) - ORDER BY u.username - `, userId, userId, planId, planId); + return this.shares.listAvailableForShare(userId, planId); } async getSharedCalendars(viewerId: number, year: string) { // Shared calendars are drawn into the viewer's grid, so they load over the // viewer's range (#737) even when the owner's leave year is shaped differently. const { start, end } = await this.viewerGridWindow(year, viewerId); - const shares = this.db.all<{ id: number; owner_id: number; hidden: number; username: string }>(` - SELECT s.id, s.owner_id, s.hidden, u.username - FROM vacay_shares s JOIN users u ON s.owner_id = u.id - WHERE s.user_id = ? ORDER BY s.id - `, viewerId); + const shareRows = await this.shares.listIncoming(viewerId); // Same dormancy rule as listShares: fused co-members are already fully visible. const coMembers = await this.viewerCoMemberIds(viewerId); @@ -985,24 +976,21 @@ export class VacayService { entries: { date: string; fraction: number; kind: string | null }[]; companyHolidays: { date: string }[]; }[] = []; - for (const s of shares.filter(s => !coMembers.has(s.owner_id))) { + for (const s of shareRows.filter(s => !coMembers.has(s.owner_id))) { const color = await this.shareDisplayColor(s.owner_id, usedColors); const plan = await this.peekActivePlan(s.owner_id); if (!plan) { calendars.push({ share_id: s.id, owner_id: s.owner_id, owner_name: s.username, color, hidden: !!s.hidden, entries: [], companyHolidays: [] }); continue; } - const entries = this.db.all<{ date: string; fraction: number; kind: string | null }>( - 'SELECT date, fraction, kind FROM vacay_entries WHERE plan_id = ? AND user_id = ? AND date >= ? AND date < ? ORDER BY date', - plan.id, s.owner_id, start, end - ); + const entries = await this.entries.listForOwnerRange(plan.id, s.owner_id, start, end); // Company holidays are plan-wide context for "when is this person off"; // only exposed while the owner has the feature enabled, and dates only — // the note text may be authored by plan members who aren't part of the share. - const companyHolidays = plan.company_holidays_enabled - ? this.db.all<{ date: string }>('SELECT date FROM vacay_company_holidays WHERE plan_id = ? AND date >= ? AND date < ? ORDER BY date', plan.id, start, end) + const companyHolidayList = plan.company_holidays_enabled + ? await this.companyHolidays.listDatesForRange(plan.id, start, end) : []; - calendars.push({ share_id: s.id, owner_id: s.owner_id, owner_name: s.username, color, hidden: !!s.hidden, entries, companyHolidays }); + calendars.push({ share_id: s.id, owner_id: s.owner_id, owner_name: s.username, color, hidden: !!s.hidden, entries, companyHolidays: companyHolidayList }); } return calendars; } @@ -1012,32 +1000,31 @@ export class VacayService { // ------------------------------------------------------------------------- async listYears(planId: number): Promise { - const rows = this.db.all<{ year: number }>('SELECT year FROM vacay_years WHERE plan_id = ? ORDER BY year', planId); - return rows.map(y => y.year); + return this.years.listForPlan(planId); } async addYear(planId: number, year: number, socketId: string | undefined): Promise { // A duplicate year is a no-op (the legacy blanket try/catch was written for // exactly this constraint hit); real errors now propagate instead of being // swallowed. The insert + per-user seeding runs atomically. - const exists = this.db.get('SELECT id FROM vacay_years WHERE plan_id = ? AND year = ?', planId, year); + const exists = await this.years.exists(planId, year); if (!exists) { await this.uow.transactional(async () => { - this.db.run('INSERT INTO vacay_years (plan_id, year) VALUES (?, ?)', planId, year); - const plan = this.db.get('SELECT * FROM vacay_plans WHERE id = ?', planId); + await this.years.insertYear(planId, year); + const plan = await this.plans.findById(planId); const carryOverEnabled = plan ? !!plan.carry_over_enabled : true; const users = await this.getPlanUsers(planId); for (const u of users) { let carriedOver = 0; if (carryOverEnabled) { - const prevConfig = this.db.get('SELECT * FROM vacay_user_years WHERE user_id = ? AND plan_id = ? AND year = ?', u.id, planId, year - 1); + const prevConfig = await this.userYears.findForYear(u.id, planId, year - 1); if (prevConfig) { const used = await this.usedDays(u.id, planId, year - 1); - const total = prevConfig.vacation_days + prevConfig.carried_over; + const total = (prevConfig.vacation_days ?? 30) + (prevConfig.carried_over ?? 0); carriedOver = Math.max(0, total - used); } } - this.db.run('INSERT OR IGNORE INTO vacay_user_years (user_id, plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, 30, ?)', u.id, planId, year, carriedOver); + await this.userYears.insertIgnore(u.id, planId, year, 30, carriedOver); } }); } @@ -1049,20 +1036,20 @@ export class VacayService { // Year removal deletes across four tables and recomputes the next year's // carry-over — atomic, so a failure can't leave entries without their year. await this.uow.transactional(async () => { - this.db.run('DELETE FROM vacay_years WHERE plan_id = ? AND year = ?', planId, year); + await this.years.deleteForPlanAndYear(planId, year); // Members can be on differently shaped leave years (#737), so entries go per // author over that author's period rather than by one shared year prefix. // Authors are read off the entries themselves so orphans are cleared too. - const authors = this.db.all<{ user_id: number }>('SELECT DISTINCT user_id FROM vacay_entries WHERE plan_id = ?', planId); + const authors = await this.entries.listAuthorsForPlan(planId); for (const { user_id } of authors) { const { start, end } = await this.resolveYearWindow(user_id, year); - this.db.run('DELETE FROM vacay_entries WHERE plan_id = ? AND user_id = ? AND date >= ? AND date < ?', planId, user_id, start, end); + await this.entries.deleteForRange(planId, user_id, start, end); } // Company holidays belong to the plan, not to a member, and every member sees // them over their own window. In a fused plan with mixed year types the safe // range is therefore the intersection of all member windows — anything outside // it still sits inside a period somebody else has not deleted. - const owner = this.db.get<{ owner_id: number }>('SELECT owner_id FROM vacay_plans WHERE id = ?', planId); + const owner = await this.plans.findOwnerId(planId); const members = await this.getPlanUsers(planId); // `map` cannot await the per-member window read, so it runs as an explicit // loop — same ids, same order. @@ -1073,29 +1060,29 @@ export class VacayService { const holidayStart = windows.reduce((a, w) => (w.start > a ? w.start : a), windows[0].start); const holidayEnd = windows.reduce((a, w) => (w.end < a ? w.end : a), windows[0].end); if (holidayStart < holidayEnd) { - this.db.run('DELETE FROM vacay_company_holidays WHERE plan_id = ? AND date >= ? AND date < ?', planId, holidayStart, holidayEnd); + await this.companyHolidays.deleteForRange(planId, holidayStart, holidayEnd); } - this.db.run('DELETE FROM vacay_user_years WHERE plan_id = ? AND year = ?', planId, year); + await this.userYears.deleteForYear(planId, year); // Recalculate carry-over for year+1 if it exists, since its previous year has changed - const nextYearExists = this.db.get('SELECT id FROM vacay_years WHERE plan_id = ? AND year = ?', planId, year + 1); + const nextYearExists = await this.years.exists(planId, year + 1); if (nextYearExists) { - const plan = this.db.get('SELECT * FROM vacay_plans WHERE id = ?', planId); + const plan = await this.plans.findById(planId); const carryOverEnabled = plan ? !!plan.carry_over_enabled : true; const users = await this.getPlanUsers(planId); - const prevYear = this.db.get<{ year: number }>('SELECT year FROM vacay_years WHERE plan_id = ? AND year < ? ORDER BY year DESC LIMIT 1', planId, year + 1); + const prevYear = await this.years.previousYear(planId, year + 1); for (const u of users) { let carry = 0; - if (carryOverEnabled && prevYear) { - const prevConfig = this.db.get('SELECT * FROM vacay_user_years WHERE user_id = ? AND plan_id = ? AND year = ?', u.id, planId, prevYear.year); + if (carryOverEnabled && prevYear !== null) { + const prevConfig = await this.userYears.findForYear(u.id, planId, prevYear); if (prevConfig) { - const used = await this.usedDays(u.id, planId, prevYear.year); - const total = prevConfig.vacation_days + prevConfig.carried_over; + const used = await this.usedDays(u.id, planId, prevYear); + const total = (prevConfig.vacation_days ?? 30) + (prevConfig.carried_over ?? 0); carry = Math.max(0, total - used); } } - this.db.run('UPDATE vacay_user_years SET carried_over = ? WHERE user_id = ? AND plan_id = ? AND year = ?', carry, u.id, planId, year + 1); + await this.userYears.updateCarriedOver(u.id, planId, year + 1, carry); } } }); @@ -1115,53 +1102,47 @@ export class VacayService { */ async getEntries(planId: number, year: string, viewerId?: number) { const { start, end } = await this.viewerGridWindow(year, viewerId); - const entries = this.db.all(` - SELECT e.*, u.username as person_name, COALESCE(c.color, '#6366f1') as person_color - FROM vacay_entries e - JOIN users u ON e.user_id = u.id - LEFT JOIN vacay_user_colors c ON c.user_id = e.user_id AND c.plan_id = e.plan_id - WHERE e.plan_id = ? AND e.date >= ? AND e.date < ? - `, planId, start, end); - const companyHolidays = this.db.all('SELECT * FROM vacay_company_holidays WHERE plan_id = ? AND date >= ? AND date < ?', planId, start, end); - return { entries, companyHolidays }; + const entries = await this.entries.listForRangeWithPerson(planId, start, end); + const companyHolidayList = await this.companyHolidays.listForRange(planId, start, end); + return { entries, companyHolidays: companyHolidayList }; } async toggleEntry(userId: number, planId: number, date: string, fraction?: unknown, kind?: unknown, socketId?: string): Promise<{ action?: string; fraction?: number; kind?: string; error?: string }> { const frac = normalizeFraction(fraction); const knd = normalizeKind(kind); - const plan = this.db.get('SELECT * FROM vacay_plans WHERE id = ?', planId); + const plan = await this.plans.findById(planId); const weekendBlocked = plan ? isBlockedWeekend(plan, date) : false; - const existing = this.db.get<{ id: number; fraction: number; kind: string | null }>('SELECT id, fraction, kind FROM vacay_entries WHERE user_id = ? AND date = ? AND plan_id = ?', userId, date, planId); + const existing = await this.entries.findByUserDatePlan(userId, date, planId); if (existing) { // Clicking the exact same day again (same type AND same fraction) clears it; // clicking a different type or fraction converts it in place (#552/#1074). if (existing.fraction === frac && (existing.kind || 'vacation') === knd) { - this.db.run('DELETE FROM vacay_entries WHERE id = ?', existing.id); + await this.entries.deleteById(existing.id); await this.notifyPlanUsers(planId, socketId); return { action: 'removed' }; } // Removing a stray entry on a blocked day stays possible; keeping one // there (converted in place) does not. if (weekendBlocked) return { error: 'weekend_blocked' }; - this.db.run('UPDATE vacay_entries SET fraction = ?, kind = ? WHERE id = ?', frac, knd, existing.id); + await this.entries.updateFractionKind(existing.id, frac, knd); await this.notifyPlanUsers(planId, socketId); return { action: 'updated', fraction: frac, kind: knd }; } if (weekendBlocked) return { error: 'weekend_blocked' }; - this.db.run('INSERT INTO vacay_entries (plan_id, user_id, date, note, fraction, kind) VALUES (?, ?, ?, ?, ?, ?)', planId, userId, date, '', frac, knd); + await this.entries.insertEntry(planId, userId, date, '', frac, knd); await this.notifyPlanUsers(planId, socketId); return { action: 'added', fraction: frac, kind: knd }; } async toggleCompanyHoliday(planId: number, date: string, note: string | undefined, socketId: string | undefined): Promise<{ action: string }> { - const existing = this.db.get<{ id: number }>('SELECT id FROM vacay_company_holidays WHERE plan_id = ? AND date = ?', planId, date); + const existing = await this.companyHolidays.findByPlanAndDate(planId, date); if (existing) { - this.db.run('DELETE FROM vacay_company_holidays WHERE id = ?', existing.id); + await this.companyHolidays.deleteById(existing.id); await this.notifyPlanUsers(planId, socketId); return { action: 'removed' }; } else { - this.db.run('INSERT INTO vacay_company_holidays (plan_id, date, note) VALUES (?, ?, ?)', planId, date, note || ''); - this.db.run('DELETE FROM vacay_entries WHERE plan_id = ? AND date = ?', planId, date); + await this.companyHolidays.insertHoliday(planId, date, note || ''); + await this.entries.deleteForPlanAndDate(planId, date); await this.notifyPlanUsers(planId, socketId); return { action: 'added' }; } @@ -1172,55 +1153,55 @@ export class VacayService { // ------------------------------------------------------------------------- async getStats(planId: number, year: number) { - const plan = this.db.get('SELECT * FROM vacay_plans WHERE id = ?', planId); + const plan = await this.plans.findById(planId); const carryOverEnabled = plan ? !!plan.carry_over_enabled : true; const users = await this.getPlanUsers(planId); // `map` cannot await, and the body also writes next year's carry-over, so // the projection runs as an explicit loop — same users, same order, same writes. + // VC126 (plan3f-inputs.md correction #7): the per-request carry-over write + // now runs inside uow.transactional, matching every OTHER multi-row write + // loop in this file — flagged per the plan's "wrap it, flag it" default + // rather than left silently un-transacted. const rows: { user_id: number; person_name: string; person_color: string; year: number; vacation_days: number; carried_over: number; total_available: number; used: number; remaining: number; comp_used: number; window_start: string; window_end: string; }[] = []; - for (const u of users) { - const used = await this.usedDays(u.id, planId, year); - const compUsed = await this.compUsedDays(u.id, planId, year); - const config = this.db.get('SELECT * FROM vacay_user_years WHERE user_id = ? AND plan_id = ? AND year = ?', u.id, planId, year); - const vacationDays = config ? config.vacation_days : 30; - const carriedOver = carryOverEnabled ? (config ? config.carried_over : 0) : 0; - const total = vacationDays + carriedOver; - const remaining = total - used; - const colorRow = this.db.get<{ color: string }>('SELECT color FROM vacay_user_colors WHERE user_id = ? AND plan_id = ?', u.id, planId); - // The period this row was computed over (#737) — the UI labels the window and - // the carry-over source with it instead of assuming Jan–Dec / year − 1. - const window = await this.resolveYearWindow(u.id, year); + await this.uow.transactional(async () => { + for (const u of users) { + const used = await this.usedDays(u.id, planId, year); + const compUsed = await this.compUsedDays(u.id, planId, year); + const config = await this.userYears.findForYear(u.id, planId, year); + const vacationDays = config ? config.vacation_days ?? 30 : 30; + const carriedOver = carryOverEnabled ? (config ? config.carried_over ?? 0 : 0) : 0; + const total = vacationDays + carriedOver; + const remaining = total - used; + const colorRow = await this.userColors.findColor(u.id, planId); + // The period this row was computed over (#737) — the UI labels the window and + // the carry-over source with it instead of assuming Jan–Dec / year − 1. + const window = await this.resolveYearWindow(u.id, year); - const nextYearExists = this.db.get('SELECT id FROM vacay_years WHERE plan_id = ? AND year = ?', planId, year + 1); - if (nextYearExists && carryOverEnabled) { - const carry = Math.max(0, remaining); - this.db.run(` - INSERT INTO vacay_user_years (user_id, plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, 30, ?) - ON CONFLICT(user_id, plan_id, year) DO UPDATE SET carried_over = ? - `, u.id, planId, year + 1, carry, carry); + const nextYearExists = await this.years.exists(planId, year + 1); + if (nextYearExists && carryOverEnabled) { + const carry = Math.max(0, remaining); + await this.userYears.upsertCarriedOver(u.id, planId, year + 1, carry); + } + + rows.push({ + user_id: u.id, person_name: u.username, person_color: colorRow?.color || '#6366f1', + year, vacation_days: vacationDays, carried_over: carriedOver, + total_available: total, used, remaining, comp_used: compUsed, + window_start: window.start, window_end: window.end, + }); } - - rows.push({ - user_id: u.id, person_name: u.username, person_color: colorRow?.color || '#6366f1', - year, vacation_days: vacationDays, carried_over: carriedOver, - total_available: total, used, remaining, comp_used: compUsed, - window_start: window.start, window_end: window.end, - }); - } + }); return rows; } async updateStats(userId: number, planId: number, year: number, vacationDays: number, socketId: string | undefined): Promise { - this.db.run(` - INSERT INTO vacay_user_years (user_id, plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, ?, 0) - ON CONFLICT(user_id, plan_id, year) DO UPDATE SET vacation_days = excluded.vacation_days - `, userId, planId, year, vacationDays); + await this.userYears.upsertVacationDays(userId, planId, year, vacationDays); await this.notifyPlanUsers(planId, socketId); } @@ -1232,26 +1213,16 @@ export class VacayService { const plan = await this.getActivePlan(userId); const activePlanId = plan.id; - const users = (await this.getPlanUsers(activePlanId)).map(u => { - const colorRow = this.db.get<{ color: string }>('SELECT color FROM vacay_user_colors WHERE user_id = ? AND plan_id = ?', u.id, activePlanId); - return { ...u, color: colorRow?.color || '#6366f1' }; - }); + const planUsers = await this.getPlanUsers(activePlanId); + const users: (VacayUser & { color: string })[] = []; + for (const u of planUsers) { + const colorRow = await this.userColors.findColor(u.id, activePlanId); + users.push({ ...u, color: colorRow?.color || '#6366f1' }); + } - const pendingInvites = this.db.all(` - SELECT m.id, m.user_id, u.username, u.email, m.created_at - FROM vacay_plan_members m JOIN users u ON m.user_id = u.id - WHERE m.plan_id = ? AND m.status = 'pending' - `, activePlanId); - - const incomingInvites = this.db.all(` - SELECT m.id, m.plan_id, u.username, u.email, m.created_at - FROM vacay_plan_members m - JOIN vacay_plans p ON m.plan_id = p.id - JOIN users u ON p.owner_id = u.id - WHERE m.user_id = ? AND m.status = 'pending' - `, userId); - - const holidayCalendars = this.db.all('SELECT * FROM vacay_holiday_calendars WHERE plan_id = ? ORDER BY sort_order, id', activePlanId); + const pendingInvites = await this.members.listPendingForPlan(activePlanId); + const incomingInvites = await this.members.listPendingForUser(userId); + const holidayCalendarList = await this.holidayCalendars.listForPlan(activePlanId); return { plan: { @@ -1261,7 +1232,7 @@ export class VacayService { school_holidays_enabled: !!plan.school_holidays_enabled, company_holidays_enabled: !!plan.company_holidays_enabled, carry_over_enabled: !!plan.carry_over_enabled, - holiday_calendars: holidayCalendars, + holiday_calendars: holidayCalendarList, }, users, pendingInvites, diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index e286b3ab1..470d23cd9 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -149,6 +149,11 @@ import { createTestSchoolHolidayCountriesRepo, createTestSchoolHolidayRegionsRepo, createTestSchoolHolidayPeriodsRepo, createTestVacayHolidayCalendarsRepo, } from './school-holidays-repos'; +import { + createTestVacayPlansRepo, createTestVacayPlanMembersRepo, createTestVacayYearsRepo, createTestVacayUserYearsRepo, + createTestVacayUserColorsRepo, createTestVacayEntriesRepo, createTestVacayCompanyHolidaysRepo, + createTestVacaySharesRepo, createTestVacayUserSettingsRepo, +} from './vacay-repos'; import { AppSettings } from '../../src/db/entities/AppSettings.entity'; import { AuditLog } from '../../src/db/entities/AuditLog.entity'; import { Users } from '../../src/db/entities/Users.entity'; @@ -289,7 +294,15 @@ export async function createMcpTestRegistry(): Promise { daysService, permissionsService, budgetService, - new VacayService(dbService, realtimeService, notificationsStub(), await createTestUnitOfWork(dbService.connection)), + new VacayService( + await createTestVacayPlansRepo(dbService.connection), await createTestVacayPlanMembersRepo(dbService.connection), + await createTestVacayYearsRepo(dbService.connection), await createTestVacayUserYearsRepo(dbService.connection), + await createTestVacayUserColorsRepo(dbService.connection), await createTestVacayEntriesRepo(dbService.connection), + await createTestVacayCompanyHolidaysRepo(dbService.connection), await createTestVacayHolidayCalendarsRepo(dbService.connection), + await createTestVacaySharesRepo(dbService.connection), await createTestVacayUserSettingsRepo(dbService.connection), + await createTestSchoolHolidayRegionsRepo(dbService.connection), + realtimeService, notificationsStub(), await createTestUnitOfWork(dbService.connection), + ), realtimeService, new UnsplashService(appSettings, usersRepo, new RuntimeEnvService(), generalStorage), generalStorage, @@ -367,7 +380,15 @@ export async function createMcpTestRegistry(): Promise { new AccommodationsMcp(accommodationsService, placesService, authService, guards, await createTestUnitOfWork(dbService.connection)), new AssignmentsMcp(assignmentsService, daysService, authService, guards), new CollabMcp(collabService, authService, addonsService, guards), - new VacayMcp(new VacayService(dbService, realtimeService, notificationsStub(), await createTestUnitOfWork(dbService.connection)), authService, addonsService), + new VacayMcp(new VacayService( + await createTestVacayPlansRepo(dbService.connection), await createTestVacayPlanMembersRepo(dbService.connection), + await createTestVacayYearsRepo(dbService.connection), await createTestVacayUserYearsRepo(dbService.connection), + await createTestVacayUserColorsRepo(dbService.connection), await createTestVacayEntriesRepo(dbService.connection), + await createTestVacayCompanyHolidaysRepo(dbService.connection), await createTestVacayHolidayCalendarsRepo(dbService.connection), + await createTestVacaySharesRepo(dbService.connection), await createTestVacayUserSettingsRepo(dbService.connection), + await createTestSchoolHolidayRegionsRepo(dbService.connection), + realtimeService, notificationsStub(), await createTestUnitOfWork(dbService.connection), + ), authService, addonsService), new SchoolHolidaysMcp( new SchoolHolidaysService( await createTestSchoolHolidayCountriesRepo(dbService.connection), diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index 9593f25d1..644c614c3 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -101,6 +101,12 @@ import { createTestPackingTemplateItemsRepo, } from './packing-repos'; import { createTestTodoItemsRepo, createTestTodoCategoryAssigneesRepo } from './todo-repos'; +import { createTestVacayHolidayCalendarsRepo, createTestSchoolHolidayRegionsRepo } from './school-holidays-repos'; +import { + createTestVacayPlansRepo, createTestVacayPlanMembersRepo, createTestVacayYearsRepo, createTestVacayUserYearsRepo, + createTestVacayUserColorsRepo, createTestVacayEntriesRepo, createTestVacayCompanyHolidaysRepo, + createTestVacaySharesRepo, createTestVacayUserSettingsRepo, +} from './vacay-repos'; /** * Hand-wired counterpart of the PluginsModule DI graph for no-Nest tests @@ -147,7 +153,15 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise< await createTestCollabPollVotesRepo(dbs.connection), await createTestCollabLinksRepo(dbs.connection), await createTestCollabMessagesRepo(dbs.connection), await createTestTripsRepo(dbs.connection), ); - const vacay = new VacayService(dbs, realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection)); + const vacay = new VacayService( + await createTestVacayPlansRepo(dbs.connection), await createTestVacayPlanMembersRepo(dbs.connection), + await createTestVacayYearsRepo(dbs.connection), await createTestVacayUserYearsRepo(dbs.connection), + await createTestVacayUserColorsRepo(dbs.connection), await createTestVacayEntriesRepo(dbs.connection), + await createTestVacayCompanyHolidaysRepo(dbs.connection), await createTestVacayHolidayCalendarsRepo(dbs.connection), + await createTestVacaySharesRepo(dbs.connection), await createTestVacayUserSettingsRepo(dbs.connection), + await createTestSchoolHolidayRegionsRepo(dbs.connection), + realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection), + ); const days = new DaysService( dbs, permissions, diff --git a/server/tests/helpers/vacay-repos.ts b/server/tests/helpers/vacay-repos.ts new file mode 100644 index 000000000..c894598b2 --- /dev/null +++ b/server/tests/helpers/vacay-repos.ts @@ -0,0 +1,72 @@ +import type Database from 'better-sqlite3'; +import { sharedTestOrm } from './test-uow'; +import { VacayPlans } from '../../src/db/entities/VacayPlans.entity'; +import type { VacayPlansRepository } from '../../src/db/repositories/VacayPlans.repository'; +import { VacayPlanMembers } from '../../src/db/entities/VacayPlanMembers.entity'; +import type { VacayPlanMembersRepository } from '../../src/db/repositories/VacayPlanMembers.repository'; +import { VacayYears } from '../../src/db/entities/VacayYears.entity'; +import type { VacayYearsRepository } from '../../src/db/repositories/VacayYears.repository'; +import { VacayUserYears } from '../../src/db/entities/VacayUserYears.entity'; +import type { VacayUserYearsRepository } from '../../src/db/repositories/VacayUserYears.repository'; +import { VacayUserColors } from '../../src/db/entities/VacayUserColors.entity'; +import type { VacayUserColorsRepository } from '../../src/db/repositories/VacayUserColors.repository'; +import { VacayEntries } from '../../src/db/entities/VacayEntries.entity'; +import type { VacayEntriesRepository } from '../../src/db/repositories/VacayEntries.repository'; +import { VacayCompanyHolidays } from '../../src/db/entities/VacayCompanyHolidays.entity'; +import type { VacayCompanyHolidaysRepository } from '../../src/db/repositories/VacayCompanyHolidays.repository'; +import { VacayShares } from '../../src/db/entities/VacayShares.entity'; +import type { VacaySharesRepository } from '../../src/db/repositories/VacayShares.repository'; +import { VacayUserSettings } from '../../src/db/entities/VacayUserSettings.entity'; +import type { VacayUserSettingsRepository } from '../../src/db/repositories/VacayUserSettings.repository'; + +/** + * Plan 3f Task 5 (`VacayService`) test-only repository factories, bound to a + * suite's own better-sqlite3 handle via the SAME memoised `sharedTestOrm` + * `test-uow.ts` exports — the `school-holidays-repos.ts`/`todo-repos.ts` + * precedent this file copies verbatim: a repository built here and a + * `UnitOfWork` built from `createTestUnitOfWork` (that file) resolve the + * IDENTICAL context-resolving `EntityManager`, required so a repository from + * a SECOND, independent ORM instance never writes outside the open + * transaction. + * + * `VacayHolidayCalendarsRepository`'s own factory + * (`createTestVacayHolidayCalendarsRepo`) already exists in + * `school-holidays-repos.ts` (built by Task 2, before this task landed) and + * `SchoolHolidayRegionsRepository`'s (`createTestSchoolHolidayRegionsRepo`) + * likewise — both reused from there, not duplicated here. + */ +export function createTestVacayPlansRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(VacayPlans)); +} + +export function createTestVacayPlanMembersRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(VacayPlanMembers)); +} + +export function createTestVacayYearsRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(VacayYears)); +} + +export function createTestVacayUserYearsRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(VacayUserYears)); +} + +export function createTestVacayUserColorsRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(VacayUserColors)); +} + +export function createTestVacayEntriesRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(VacayEntries)); +} + +export function createTestVacayCompanyHolidaysRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(VacayCompanyHolidays)); +} + +export function createTestVacaySharesRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(VacayShares)); +} + +export function createTestVacayUserSettingsRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(VacayUserSettings)); +} diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index fba7ef6c4..49b9aa730 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -113,6 +113,12 @@ import { createTestPackingTemplateItemsRepo, } from '../../helpers/packing-repos'; import { createTestTodoItemsRepo, createTestTodoCategoryAssigneesRepo } from '../../helpers/todo-repos'; +import { + createTestVacayPlansRepo, createTestVacayPlanMembersRepo, createTestVacayYearsRepo, createTestVacayUserYearsRepo, + createTestVacayUserColorsRepo, createTestVacayEntriesRepo, createTestVacayCompanyHolidaysRepo, + createTestVacaySharesRepo, createTestVacayUserSettingsRepo, +} from '../../helpers/vacay-repos'; +import { createTestVacayHolidayCalendarsRepo, createTestSchoolHolidayRegionsRepo } from '../../helpers/school-holidays-repos'; // Real sibling services over the same in-memory DB — updateTrip's date-shift // resyncs and the summary/bundle aggregation run their actual SQL. @@ -205,7 +211,15 @@ beforeAll(async () => { daysSvc, new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), budgetSvc, - new VacayService(dbs(), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection)), + new VacayService( + await createTestVacayPlansRepo(dbs().connection), await createTestVacayPlanMembersRepo(dbs().connection), + await createTestVacayYearsRepo(dbs().connection), await createTestVacayUserYearsRepo(dbs().connection), + await createTestVacayUserColorsRepo(dbs().connection), await createTestVacayEntriesRepo(dbs().connection), + await createTestVacayCompanyHolidaysRepo(dbs().connection), await createTestVacayHolidayCalendarsRepo(dbs().connection), + await createTestVacaySharesRepo(dbs().connection), await createTestVacayUserSettingsRepo(dbs().connection), + await createTestSchoolHolidayRegionsRepo(dbs().connection), + new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), + ), new RealtimeService(), undefined as never, // unsplash — not exercised here coversFx.storage, diff --git a/server/tests/unit/nest/vacay.service.test.ts b/server/tests/unit/nest/vacay.service.test.ts index fd3e1fab5..3957926ad 100644 --- a/server/tests/unit/nest/vacay.service.test.ts +++ b/server/tests/unit/nest/vacay.service.test.ts @@ -32,11 +32,16 @@ import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser } from '../../helpers/factories'; -import { DatabaseService } from '../../../src/nest/database/database.service'; import { VacayService } from '../../../src/nest/vacay/vacay.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { notificationsStub } from '../../helpers/notifications'; import { createTestUnitOfWork } from '../../helpers/test-uow'; +import { + createTestVacayPlansRepo, createTestVacayPlanMembersRepo, createTestVacayYearsRepo, createTestVacayUserYearsRepo, + createTestVacayUserColorsRepo, createTestVacayEntriesRepo, createTestVacayCompanyHolidaysRepo, + createTestVacaySharesRepo, createTestVacayUserSettingsRepo, +} from '../../helpers/vacay-repos'; +import { createTestVacayHolidayCalendarsRepo, createTestSchoolHolidayRegionsRepo } from '../../helpers/school-holidays-repos'; // VACAY-SVC-001 through VACAY-SVC-066 moved 1:1 from the legacy // tests/unit/services/vacayService.test.ts (the named-function imports became @@ -51,7 +56,15 @@ let svc: VacayService; beforeAll(async () => { createTables(testDb); runMigrations(testDb); - svc = new VacayService(new DatabaseService(testDb), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(testDb)); + svc = new VacayService( + await createTestVacayPlansRepo(testDb), await createTestVacayPlanMembersRepo(testDb), + await createTestVacayYearsRepo(testDb), await createTestVacayUserYearsRepo(testDb), + await createTestVacayUserColorsRepo(testDb), await createTestVacayEntriesRepo(testDb), + await createTestVacayCompanyHolidaysRepo(testDb), await createTestVacayHolidayCalendarsRepo(testDb), + await createTestVacaySharesRepo(testDb), await createTestVacayUserSettingsRepo(testDb), + await createTestSchoolHolidayRegionsRepo(testDb), + new RealtimeService(), notificationsStub(), await createTestUnitOfWork(testDb), + ); }); beforeEach(() => { @@ -499,6 +512,77 @@ describe('deleteYear', () => { .all(plan.id, `${targetYear}-%`); expect(entries).toHaveLength(0); }); + + it('VACAY-SVC-030a (VC102 parity): touches only each author\'s OWN leave-year window, not one shared window across a fused plan', async () => { + // Two members of the SAME plan on genuinely different leave-year shapes + // (#737): userA stays on the default calendar year, userB is fiscal + // (Apr 1 start). deleteYear(planId, 2026) must delete userA's entry that + // falls in A's calendar-2026 window and userB's entry that falls in B's + // fiscal-2026 window, while leaving each author's OWN neighbouring-period + // entry untouched — a shortcut that computed ONE shared window (off + // either the plan or either single author) and reused it across both + // authors would either strand entries or delete another member's rows on + // their differently-shaped year. + const { user: userA, plan } = await setupUserWithPlan(); + const { user: userB } = createUser(testDb); + await svc.getOwnPlan(userB.id); + insertMember(plan.id, userB.id, 'accepted'); + await svc.updateYearSettings(userB.id, { year_type: 'fiscal', year_start_month: 4, year_start_day: 1 }); + await svc.addYear(plan.id, 2026, undefined); + + const insertEntry = (userId: number, date: string) => + testDb.prepare('INSERT INTO vacay_entries (plan_id, user_id, date, note) VALUES (?, ?, ?, ?)').run(plan.id, userId, date, ''); + insertEntry(userA.id, '2026-02-10'); // inside A's CALENDAR 2026 window [2026-01-01, 2027-01-01) — must be deleted + insertEntry(userA.id, '2025-11-01'); // inside A's CALENDAR 2025 window — must survive + insertEntry(userB.id, '2026-02-10'); // inside B's FISCAL 2025 window [2025-04-01, 2026-04-01) — must survive + insertEntry(userB.id, '2026-05-10'); // inside B's FISCAL 2026 window [2026-04-01, 2027-04-01) — must be deleted + + await svc.deleteYear(plan.id, 2026, undefined); + + const remaining = testDb.prepare('SELECT user_id, date FROM vacay_entries WHERE plan_id = ? ORDER BY user_id, date').all(plan.id); + expect(remaining).toEqual([ + { user_id: userA.id, date: '2025-11-01' }, + { user_id: userB.id, date: '2026-02-10' }, + ]); + }); +}); + +// ── shiftOwnerEntriesForTripWindow (VC4 restructured date-diff shape) ───────── + +describe('shiftOwnerEntriesForTripWindow', () => { + it('VACAY-SVC-030b (VC4 parity): shifts an entry by the exact calendar-day offset between the old and new trip start, across several date-pair shapes incl. a leap-year boundary', async () => { + // R9's verified restructured shape (task-0-report.md): plain JS + // Math.round((Date.parse(newStart) - Date.parse(oldStart)) / 86400000) + // in place of the legacy CAST(julianday(?) - julianday(?) AS INTEGER) — + // verified there against the SQL on 5 date pairs incl. a leap-year + // boundary and a negative offset, all matched exactly. This proves the + // SAME arithmetic end-to-end through the converted service method. + const { user, plan } = await setupUserWithPlan(); + const cases: [oldStart: string, oldEnd: string, newStart: string, entryDate: string, expectedShifted: string][] = [ + ['2026-03-01', '2026-03-10', '2026-03-06', '2026-03-05', '2026-03-10'], // +5 days + ['2024-02-25', '2024-03-05', '2024-02-28', '2024-03-01', '2024-03-04'], // leap-year boundary, +3 days + ['2026-06-10', '2026-06-20', '2026-06-03', '2026-06-15', '2026-06-08'], // -7 days + ]; + for (const [oldStart, oldEnd, newStart, entryDate, expectedShifted] of cases) { + testDb.prepare('DELETE FROM vacay_entries WHERE plan_id = ?').run(plan.id); + testDb.prepare('INSERT INTO vacay_entries (plan_id, user_id, date, note) VALUES (?, ?, ?, ?)').run(plan.id, user.id, entryDate, ''); + + await svc.shiftOwnerEntriesForTripWindow(user.id, oldStart, oldEnd, newStart); + + const row = testDb.prepare('SELECT date FROM vacay_entries WHERE plan_id = ? AND user_id = ?').get(plan.id, user.id) as { date: string }; + expect(row.date).toBe(expectedShifted); + } + }); + + it('VACAY-SVC-030c: a zero offset is a no-op (no write at all)', async () => { + const { user, plan } = await setupUserWithPlan(); + testDb.prepare('INSERT INTO vacay_entries (plan_id, user_id, date, note) VALUES (?, ?, ?, ?)').run(plan.id, user.id, '2026-05-05', ''); + + await svc.shiftOwnerEntriesForTripWindow(user.id, '2026-05-01', '2026-05-10', '2026-05-01'); + + const row = testDb.prepare('SELECT date FROM vacay_entries WHERE plan_id = ? AND user_id = ?').get(plan.id, user.id) as { date: string }; + expect(row.date).toBe('2026-05-05'); + }); }); // ── getEntries / toggleEntry ────────────────────────────────────────────────── @@ -513,6 +597,46 @@ describe('getEntries', () => { expect(result.entries).toEqual([]); expect(result.companyHolidays).toEqual([]); }); + + it('VACAY-SVC-031a (VC111 parity): full-key toEqual against the legacy statement run raw on the same seeded rows', async () => { + // A fused plan across all three leave-year-window shapes (#737), every + // fraction/kind combination, and a member left on the default color to + // exercise the COALESCE(c.color, '#6366f1') fallback branch too — rule + // 19's "fully seeded" bar for this read model. + const { user: owner, plan } = await setupUserWithPlan(); + allowWeekends(plan.id); + const { user: userB } = createUser(testDb); + const { user: userC } = createUser(testDb); + await svc.getOwnPlan(userB.id); + await svc.getOwnPlan(userC.id); + insertMember(plan.id, userB.id, 'accepted'); + insertMember(plan.id, userC.id, 'accepted'); + await svc.updateYearSettings(userB.id, { year_type: 'fiscal', year_start_month: 4, year_start_day: 1 }); + await svc.updateYearSettings(userC.id, { year_type: 'anniversary', hire_date: '2020-06-15' }); + await svc.setUserColor(owner.id, plan.id, '#111111', undefined); + await svc.setUserColor(userB.id, plan.id, '#222222', undefined); + // userC keeps whatever color getOwnPlan seeded by default — not + // re-set here, so its row exercises the LEFT JOIN's COALESCE fallback + // the same way a row with no vacay_user_colors match would. + + await svc.toggleEntry(owner.id, plan.id, '2026-02-10', 1, 'vacation'); + await svc.toggleEntry(userB.id, plan.id, '2026-05-05', 0.5, 'vacation'); + await svc.toggleEntry(userC.id, plan.id, '2026-08-20', 1, 'comp'); + + const result = await svc.getEntries(plan.id, '2026', owner.id); + + const legacy = testDb.prepare(` + SELECT e.*, u.username as person_name, COALESCE(c.color, '#6366f1') as person_color + FROM vacay_entries e + JOIN users u ON e.user_id = u.id + LEFT JOIN vacay_user_colors c ON c.user_id = e.user_id AND c.plan_id = e.plan_id + WHERE e.plan_id = ? AND e.date >= ? AND e.date < ? + `).all(plan.id, '2026-01-01', '2027-01-01') as { id: number }[]; + + expect(legacy).toHaveLength(3); + const byId = (rows: T[]) => [...rows].sort((a, b) => a.id - b.id); + expect(byId(result.entries as { id: number }[])).toEqual(byId(legacy)); + }); }); describe('toggleEntry', () => { @@ -1520,15 +1644,62 @@ describe('getSharedCalendars', () => { // ── Quirk fixes (transactions, fetch hygiene, cache TTL, addYear errors) ────── describe('quirk fixes', () => { - /** A DatabaseService whose run() throws when the SQL matches, for atomicity checks. */ + /** A fresh, fully-functioning VacayService over the same testDb — repository-backed now (Plan 3f Task 5), so a fresh instance no longer needs a `DatabaseService` wrapper, only its own repo set (the holiday-provider cache is instance state, so `applyHolidayCalendars`/`getCountries`/etc.'s TTL tests need a service the earlier tests in this file never touched). */ + async function freshVacayService(): Promise { + return (await buildVacayServiceWithRepos()).service; + } + + /** + * Same repo set `freshVacayService` builds, but returns the repos + * themselves too, so a caller can `vi.spyOn` one of them before + * constructing — the repository-backed replacement for the legacy + * `failingService`'s `DatabaseService.run` SQL-text-match spy (this file's + * pre-conversion mechanism could match a raw SQL substring; a repository + * has no raw SQL text left to match, so this spies on the REPOSITORY + * METHOD that now issues the statement instead). + */ + async function buildVacayServiceWithRepos() { + const repos = { + plans: await createTestVacayPlansRepo(testDb), + members: await createTestVacayPlanMembersRepo(testDb), + years: await createTestVacayYearsRepo(testDb), + userYears: await createTestVacayUserYearsRepo(testDb), + userColors: await createTestVacayUserColorsRepo(testDb), + entries: await createTestVacayEntriesRepo(testDb), + companyHolidays: await createTestVacayCompanyHolidaysRepo(testDb), + holidayCalendars: await createTestVacayHolidayCalendarsRepo(testDb), + shares: await createTestVacaySharesRepo(testDb), + userSettings: await createTestVacayUserSettingsRepo(testDb), + schoolHolidayRegions: await createTestSchoolHolidayRegionsRepo(testDb), + }; + const service = new VacayService( + repos.plans, repos.members, repos.years, repos.userYears, repos.userColors, repos.entries, + repos.companyHolidays, repos.holidayCalendars, repos.shares, repos.userSettings, repos.schoolHolidayRegions, + new RealtimeService(), notificationsStub(), await createTestUnitOfWork(testDb), + ); + return { repos, service }; + } + + /** + * A VacayService whose one named repository method throws once, for + * atomicity checks — the `uow.transactional` rollback proof. `match` keeps + * the three call sites below unchanged (they still name the legacy + * statement they mean to fail); this maps it onto the repository method + * that now issues it. `mockImplementationOnce` self-restores after the one + * throw, so it never leaks into a later test even though `createTestVacay* + * Repo` memoises one repository instance per entity per `testDb` handle + * (the same instance `svc`, built once in `beforeAll`, also uses). + */ async function failingService(match: string) { - const failingDb = new DatabaseService(testDb); - const realRun = failingDb.run.bind(failingDb); - vi.spyOn(failingDb, 'run').mockImplementation((sql: string, ...params: unknown[]) => { - if (sql.includes(match)) throw new Error('boom'); - return realRun(sql, ...params); - }); - return new VacayService(failingDb, new RealtimeService(), notificationsStub(), await createTestUnitOfWork(testDb)); + const { repos, service } = await buildVacayServiceWithRepos(); + if (match === 'INSERT OR IGNORE INTO vacay_user_years') { + vi.spyOn(repos.userYears, 'insertIgnore').mockImplementationOnce(() => { throw new Error('boom'); }); + } else if (match === 'DELETE FROM vacay_user_years') { + vi.spyOn(repos.userYears, 'deleteForYear').mockImplementationOnce(() => { throw new Error('boom'); }); + } else { + throw new Error(`failingService: no repository mapping for match "${match}"`); + } + return service; } it('VACAY-SVC-068: acceptInvite is atomic — a failure mid-flow rolls the status flip back', async () => { @@ -1560,7 +1731,7 @@ describe('quirk fixes', () => { it('VACAY-SVC-070: getCountries surfaces an upstream non-2xx as the fetch error and caches nothing', async () => { const fetchMock = vi.fn().mockResolvedValue({ ok: false, status: 502, json: async () => ({}) }); vi.stubGlobal('fetch', fetchMock); - const fresh = new VacayService(new DatabaseService(testDb), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(testDb)); + const fresh = await freshVacayService(); expect(await fresh.getCountries()).toEqual({ error: 'Failed to fetch countries' }); // Nothing cached: a retry hits the network again. @@ -1572,7 +1743,7 @@ describe('quirk fixes', () => { it('VACAY-SVC-070a: getHolidays refuses a year or country that is not a plain code', async () => { const fetchMock = vi.fn(); vi.stubGlobal('fetch', fetchMock); - const fresh = new VacayService(new DatabaseService(testDb), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(testDb)); + const fresh = await freshVacayService(); for (const [year, country] of [['../../..', 'DE'], ['2026', 'DE/../../x'], ['20xx', 'DE'], ['2026', 'DEU']]) { expect(await fresh.getHolidays(year, country)).toEqual({ error: 'Failed to fetch holidays' }); @@ -1583,7 +1754,7 @@ describe('quirk fixes', () => { it('VACAY-SVC-070b: getSchoolHolidayRegions refuses a country that is not an alpha-2 code', async () => { const fetchMock = vi.fn(); vi.stubGlobal('fetch', fetchMock); - const fresh = new VacayService(new DatabaseService(testDb), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(testDb)); + const fresh = await freshVacayService(); expect(await fresh.getSchoolHolidayRegions('DE&countryIsoCode=FR')).toEqual({ error: 'Failed to fetch school holiday regions', @@ -1598,7 +1769,7 @@ describe('quirk fixes', () => { json: async () => [{ date: '2026-01-01' }], }); vi.stubGlobal('fetch', fetchMock); - const fresh = new VacayService(new DatabaseService(testDb), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(testDb)); + const fresh = await freshVacayService(); expect(await fresh.getCountries()).toEqual({ error: 'Failed to fetch countries' }); expect(await fresh.getHolidays('2026', 'DE')).toEqual({ error: 'Failed to fetch holidays' }); @@ -1624,7 +1795,7 @@ describe('quirk fixes', () => { }; }); vi.stubGlobal('fetch', fetchMock); - const fresh = new VacayService(new DatabaseService(testDb), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(testDb)); + const fresh = await freshVacayService(); expect(await fresh.getHolidays('2026', 'DE')).toEqual({ error: 'Failed to fetch holidays' }); expect(await fresh.getCountries()).toEqual({ error: 'Failed to fetch countries' }); @@ -1636,7 +1807,7 @@ describe('quirk fixes', () => { testDb.prepare("INSERT INTO vacay_holiday_calendars (plan_id, region) VALUES (?, 'DE')").run(plan.id); const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: async () => [] }); vi.stubGlobal('fetch', fetchMock); - const fresh = new VacayService(new DatabaseService(testDb), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(testDb)); + const fresh = await freshVacayService(); await fresh.applyHolidayCalendars(plan.id); const afterFirst = fetchMock.mock.calls.length;