From 11afd3648e470c2fb180d0316e12e3e890e460e8 Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 10 Aug 2026 12:19:21 +0200 Subject: [PATCH] Put the coverage back where the code went Every split on this branch dropped the giving domain's ratchet without a line being tested any worse: coverage follows the source file, so when well-covered code leaves, the weaker branches that stay behind become the whole denominator. days fell from 82 to 77.5 percent branches that way, feeds to 76.7, trip-read-model to 62.5. The rule in vitest.config.ts is that the only way past a number is to raise it, so these are tests, not edits to the thresholds. days is back at 92.1, calendar at 96.2, trip-read-model at 100. feeds and trip-members and trip-read-model had no unit test at all - feeds' whole threshold rested on one e2e, which is exactly the file whose contract this branch rewrote. They have one now, and the read-model test pins the thing in that file that would leak data if it regressed: getTripSummary and bundle both thread the viewer id into packing.listItems so another member's private items stay hidden, and nothing asserted that before. The five thresholds this branch introduced move up to what they now measure, with a few points of slack. The pre-existing entries are left alone on purpose: they were generated from a Linux run, CI measures there, and a handful of branches are platform-specific - raising them from a Windows measurement is how you get a failure that reproduces nowhere local. --- .../mcp/tools-days-accommodations.test.ts | 21 + .../unit/nest/accommodations.service.test.ts | 266 +++++++++++- .../tests/unit/nest/calendar.service.test.ts | 311 +++++++++++++- server/tests/unit/nest/days.service.test.ts | 397 +++++++++++++++++- server/tests/unit/nest/feeds.service.test.ts | 393 +++++++++++++++++ .../unit/nest/trip-members.service.test.ts | 363 ++++++++++++++++ .../unit/nest/trip-read-model.service.test.ts | 261 ++++++++++++ .../tests/unit/nest/trips.controller.test.ts | 116 +++++ server/vitest.config.ts | 10 +- 9 files changed, 2128 insertions(+), 10 deletions(-) create mode 100644 server/tests/unit/nest/feeds.service.test.ts create mode 100644 server/tests/unit/nest/trip-members.service.test.ts create mode 100644 server/tests/unit/nest/trip-read-model.service.test.ts diff --git a/server/tests/unit/mcp/tools-days-accommodations.test.ts b/server/tests/unit/mcp/tools-days-accommodations.test.ts index 7700c353e..6139928c1 100644 --- a/server/tests/unit/mcp/tools-days-accommodations.test.ts +++ b/server/tests/unit/mcp/tools-days-accommodations.test.ts @@ -108,6 +108,27 @@ describe('Tool: create_day', () => { }); }); + it('reports every bad reference in one message rather than the first', async () => { + // The validator returns a list and the tool joins it. Without a case that + // produces more than zero errors, that join is never executed, and a tool + // that silently created a stay against a foreign place would look fine. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id, { date: '2025-06-15' }); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'create_accommodation', + arguments: { tripId: trip.id, place_id: 999999, start_day_id: 999998, end_day_id: day.id }, + }); + expect(result.isError).toBe(true); + const text = (result.content as { text: string }[])[0].text; + expect(text).toContain('Place not found'); + expect(text).toContain('Start day not found'); + expect(text).toContain(', '); + }); + expect(testDb.prepare('SELECT COUNT(*) AS n FROM day_accommodations').get()).toEqual({ n: 0 }); + }); + it('returns access denied for non-member', async () => { const { user } = createUser(testDb); const { user: other } = createUser(testDb); diff --git a/server/tests/unit/nest/accommodations.service.test.ts b/server/tests/unit/nest/accommodations.service.test.ts index 44ae29ff2..0112e1dbc 100644 --- a/server/tests/unit/nest/accommodations.service.test.ts +++ b/server/tests/unit/nest/accommodations.service.test.ts @@ -37,7 +37,7 @@ vi.mock('../../../src/websocket', () => ({ broadcast })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; -import { createUser, createTrip, createDay, createPlace, createDayAccommodation } from '../../helpers/factories'; +import { createUser, createTrip, createDay, createPlace, createDayAccommodation, addTripMember } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; @@ -92,6 +92,24 @@ describe('validateAccommodationRefs', () => { const errors = svc.validateAccommodationRefs(trip.id, place.id, 99999, day.id); expect(errors.some((e: any) => e.field === 'start_day_id')).toBe(true); }); + + it('ACC-003 — reports every bad ref, end_day_id included', () => { + // The controller only surfaces errors[0], so an unchecked end_day_id would let a + // stay point at a day from another trip and never be reported. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const errors = svc.validateAccommodationRefs(trip.id, 99999, 99998, 99997); + expect(errors.map((e: any) => e.field)).toEqual(['place_id', 'start_day_id', 'end_day_id']); + }); + + it('ACC-004 — skips the refs the caller left undefined', () => { + // The plugin update path (accommodations.rpc) passes only the fields it changes. + // If an absent ref were validated, every partial update would 403 "Place not found". + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + expect(svc.validateAccommodationRefs(trip.id)).toHaveLength(0); + expect(svc.validateAccommodationRefs(trip.id, undefined, undefined, undefined)).toHaveLength(0); + }); }); describe('createAccommodation', () => { @@ -128,6 +146,23 @@ describe('createAccommodation', () => { expect(reservation.type).toBe('hotel'); expect(reservation.status).toBe('confirmed'); }); + + it('ACC-005 — titles the partner reservation "Hotel" when the stay has no place', () => { + // day_accommodations.place_id is nullable (ON DELETE SET NULL) while + // reservations.title is NOT NULL: without the fallback the reservation insert + // fails and takes the whole transaction — and the stay — down with it. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id) as any; + + const accom = svc.createAccommodation(trip.id, { + place_id: null as unknown as number, start_day_id: day.id, end_day_id: day.id, + }) as any; + + expect(accom.place_name).toBeNull(); + const reservation = testDb.prepare('SELECT title FROM reservations WHERE accommodation_id = ?').get(accom.id) as any; + expect(reservation.title).toBe('Hotel'); + }); }); describe('getAccommodation', () => { @@ -187,6 +222,63 @@ describe('updateAccommodation', () => { const row = svc.getAccommodation(accom.id, trip.id) as any; expect(row.confirmation).toBe('ABC123'); }); + + it('ACC-006 — updates a stay that has no linked reservation', () => { + // Stays imported before the auto-reservation existed have no partner row. The sync + // block must be skipped for them, not throw on a missing reservation. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id) as any; + const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; + // The factory writes the stay row directly, without the partner reservation. + const accom = createDayAccommodation(testDb, trip.id, place.id, day.id, day.id) as any; + + const existing = svc.getAccommodation(accom.id, trip.id)!; + const updated = svc.updateAccommodation(accom.id, existing as any, { check_in: '15:00' }) as any; + + expect(updated.check_in).toBe('15:00'); + expect(testDb.prepare('SELECT COUNT(*) as n FROM reservations WHERE accommodation_id = ?').get(accom.id)).toMatchObject({ n: 0 }); + }); + + it('ACC-007 — merges into the reservation metadata instead of replacing it', () => { + // The stay is created with a check-in, so the reservation already carries metadata. + // An update that only moves the check-out must parse and extend that object — + // writing a fresh one would silently drop the check-in window. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id) as any; + const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; + const accom = svc.createAccommodation(trip.id, { + place_id: place.id, start_day_id: day.id, end_day_id: day.id, check_in: '15:00', + }) as any; + + const existing = svc.getAccommodation(accom.id, trip.id)!; + svc.updateAccommodation(accom.id, existing as any, { check_in_end: '20:00', check_out: '11:00' }); + + const reservation = testDb.prepare('SELECT metadata FROM reservations WHERE accommodation_id = ?').get(accom.id) as any; + expect(JSON.parse(reservation.metadata)).toEqual({ + check_in_time: '15:00', check_in_end_time: '20:00', check_out_time: '11:00', + }); + }); + + it('ACC-008 — leaves the reservation confirmation number alone when the stay has none', () => { + // COALESCE(?, confirmation_number): a stay without a confirmation must not null out + // a number that was typed on the reservation itself. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id) as any; + const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; + const accom = svc.createAccommodation(trip.id, { + place_id: place.id, start_day_id: day.id, end_day_id: day.id, + }) as any; + testDb.prepare('UPDATE reservations SET confirmation_number = ? WHERE accommodation_id = ?').run('RES-9', accom.id); + + const existing = svc.getAccommodation(accom.id, trip.id)!; + svc.updateAccommodation(accom.id, existing as any, { check_in: '15:00' }); + + const reservation = testDb.prepare('SELECT confirmation_number FROM reservations WHERE accommodation_id = ?').get(accom.id) as any; + expect(reservation.confirmation_number).toBe('RES-9'); + }); }); describe('deleteAccommodation', () => { @@ -225,6 +317,29 @@ describe('deleteAccommodation', () => { const result = svc.deleteAccommodation(accom.id); expect(result.linkedReservationId).toBeNull(); }); + + it('ACC-009 — also deletes the budget item that hangs off the linked reservation', () => { + // The cost row references the reservation, not the stay, so the delete has to + // cascade twice. Skipping the second hop strands a budget line whose reservation + // is gone, and the costs view can no longer reach it to remove it. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id) as any; + const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; + const accom = svc.createAccommodation(trip.id, { + place_id: place.id, start_day_id: day.id, end_day_id: day.id, + }) as any; + + const reservation = testDb.prepare('SELECT id FROM reservations WHERE accommodation_id = ?').get(accom.id) as any; + const budgetItemId = testDb.prepare( + 'INSERT INTO budget_items (trip_id, name, category, total_price, reservation_id) VALUES (?, ?, ?, ?, ?)' + ).run(trip.id, 'Hotel stay', 'Accommodation', 240, reservation.id).lastInsertRowid as number; + + const result = svc.deleteAccommodation(accom.id); + + expect(result).toEqual({ linkedReservationId: reservation.id, deletedBudgetItemId: budgetItemId }); + expect(testDb.prepare('SELECT id FROM budget_items WHERE id = ?').get(budgetItemId)).toBeUndefined(); + }); }); describe('listAccommodations', () => { @@ -260,6 +375,155 @@ describe('quirk fixes', () => { }); }); +describe('route-facing delegators', () => { + // list/validateRefs/get/create/update/remove are what the controller, the plugin RPC + // and trip-read-model call; the MCP tools call the long names. Both sets have to land + // on the same SQL, and the short ones get raw route params — strings where the long + // ones get numbers. + + it('ACC-010 — list() returns what listAccommodations() returns, for a string and a number trip id', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id) as any; + const place = createPlace(testDb, trip.id, { name: 'Pension Alpen' }) as any; + createDayAccommodation(testDb, trip.id, place.id, day.id, day.id); + + expect(svc.list(String(trip.id))).toEqual(svc.listAccommodations(trip.id)); + expect((svc.list(trip.id) as any[])[0].place_name).toBe('Pension Alpen'); + }); + + it('ACC-011 — validateRefs() returns the same errors validateAccommodationRefs() does', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id) as any; + const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; + + expect(svc.validateRefs(trip.id, place.id, day.id, day.id)).toHaveLength(0); + // The controller 404s with errors[0].message, so the shape matters, not just the count. + expect(svc.validateRefs(trip.id, 99999, day.id, day.id)).toEqual([{ field: 'place_id', message: 'Place not found' }]); + }); + + it('ACC-012 — create() writes the stay, get() reads it back trip-scoped', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const otherTrip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id) as any; + const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; + + // Both ids arrive from the route as strings. + const created = svc.create(String(trip.id), { + place_id: place.id, start_day_id: day.id, end_day_id: day.id, confirmation: 'XY-1', + }) as any; + + expect(svc.get(String(created.id), String(trip.id))).toMatchObject({ id: created.id, confirmation: 'XY-1' }); + // Trip-scoped: a stay must not be readable through another trip's URL. + expect(svc.get(created.id, otherTrip.id)).toBeUndefined(); + }); + + it('ACC-013 — update() syncs the partner reservation even when the id is a string', () => { + // updateAccommodation looks the reservation up with Number(id); if that coercion + // regresses, the REST path silently stops syncing check-in times. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id) as any; + const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; + const accom = svc.create(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id }) as any; + + const existing = svc.get(accom.id, trip.id)!; + const updated = svc.update(String(accom.id), existing as any, { check_in: '16:00', notes: 'late arrival' }) as any; + + expect(updated).toMatchObject({ check_in: '16:00', notes: 'late arrival' }); + const reservation = testDb.prepare('SELECT metadata FROM reservations WHERE accommodation_id = ?').get(accom.id) as any; + expect(JSON.parse(reservation.metadata).check_in_time).toBe('16:00'); + }); + + it('ACC-014 — remove() reports the partner reservation it took with it', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id) as any; + const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; + const accom = svc.create(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id }) as any; + const reservation = testDb.prepare('SELECT id FROM reservations WHERE accommodation_id = ?').get(accom.id) as any; + + // The controller broadcasts reservation:deleted off this return value. + expect(svc.remove(String(accom.id))).toEqual({ linkedReservationId: reservation.id, deletedBudgetItemId: null }); + expect(svc.get(accom.id, trip.id)).toBeUndefined(); + }); +}); + +describe('trip access and edit permission', () => { + it('ACC-015 — verifyTripAccess resolves the trip from a string id and from a number', () => { + // The REST path hands over the raw :tripId param, the MCP tools the parsed number. + // Without the Number() coercion one of the two callers stops finding its trip. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + + expect(svc.verifyTripAccess(String(trip.id), user.id)).toMatchObject({ id: trip.id, user_id: user.id }); + expect(svc.verifyTripAccess(trip.id, user.id)).toMatchObject({ id: trip.id, user_id: user.id }); + }); + + it('ACC-016 — verifyTripAccess returns nothing for a user who is neither owner nor member', () => { + const { user: owner } = createUser(testDb); + const { user: stranger } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + + expect(svc.verifyTripAccess(trip.id, stranger.id)).toBeUndefined(); + }); + + it('ACC-017 — canEdit lets the owner through on the default day_edit level', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const access = svc.verifyTripAccess(trip.id, user.id)!; + + expect(svc.canEdit(access, user as any)).toBe(true); + }); + + it('ACC-018 — canEdit lets a member through: it derives isMember from the trip owner id', () => { + // day_edit defaults to trip_member. canEdit passes `trip.user_id !== user.id` as the + // isMember flag, so inverting that comparison would lock every invited traveller out + // of the day plan while the owner still edits fine. + const { user: owner } = createUser(testDb); + const { user: member } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + addTripMember(testDb, trip.id, member.id); + const access = svc.verifyTripAccess(trip.id, member.id)!; + + expect(svc.canEdit(access, member as any)).toBe(true); + }); + + it('ACC-019 — canEdit denies a member once day_edit is raised to trip_owner', () => { + const { user: owner } = createUser(testDb); + const { user: member } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + addTripMember(testDb, trip.id, member.id); + const permissions = new PermissionsService(new DatabaseService(testDb)); + + try { + permissions.savePermissions({ day_edit: 'trip_owner' }); + expect(svc.canEdit(svc.verifyTripAccess(trip.id, member.id)!, member as any)).toBe(false); + // The owner keeps the right, otherwise the setting would lock out the whole trip. + expect(svc.canEdit(svc.verifyTripAccess(trip.id, owner.id)!, owner as any)).toBe(true); + } finally { + // The permission cache is module-scoped and outlives resetTestDb — drop the row and + // the cache together, or every later case in this file inherits the raised level. + testDb.prepare("DELETE FROM app_settings WHERE key = 'perm_day_edit'").run(); + permissions.invalidatePermissionsCache(); + } + }); + + it('ACC-020 — broadcast forwards the socket id so the originating client is not echoed', () => { + // The X-Socket-Id contract: the facade must pass the id straight through as the + // fourth argument. Dropping it makes the sender apply its own change twice. + broadcast.mockClear(); + + svc.broadcast('5', 'accommodation:deleted', { accommodationId: 9 }, 'sock-1'); + expect(broadcast).toHaveBeenCalledWith('5', 'accommodation:deleted', { accommodationId: 9 }, 'sock-1'); + + svc.broadcast('5', 'accommodation:updated', { accommodation: { id: 9 } }, undefined); + expect(broadcast).toHaveBeenLastCalledWith('5', 'accommodation:updated', { accommodation: { id: 9 } }, undefined); + }); +}); + describe('AccommodationsService wiring', () => { it('ACC-001: the module carries the controller, the service and the RPC surface', () => { expectRegisteredController(AccommodationsModule, AccommodationsController); diff --git a/server/tests/unit/nest/calendar.service.test.ts b/server/tests/unit/nest/calendar.service.test.ts index 07486b4ad..db20dbc73 100644 --- a/server/tests/unit/nest/calendar.service.test.ts +++ b/server/tests/unit/nest/calendar.service.test.ts @@ -51,7 +51,7 @@ import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { ReservationsService } from '../../../src/nest/reservations/reservations.service'; import { BudgetService } from '../../../src/nest/budget/budget.service'; import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; -import { CalendarService } from '../../../src/nest/calendar/calendar.service'; +import { CalendarService, foldICS } from '../../../src/nest/calendar/calendar.service'; import { CalendarModule } from '../../../src/nest/calendar/calendar.module'; import { expectRegisteredProvider } from '../../helpers/module-providers'; @@ -331,6 +331,256 @@ describe('exportICS', () => { expect(ics).toContain('BEGIN:VTIMEZONE\r\nTZID:Asia/Tokyo'); expect(ics).not.toContain('DTSTART:20250602T090000'); }); + + it('CAL-011: a reservation_time with seconds is kept as is and a date-only end time emits no DTEND', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Second Precision' }); + const reservation = createReservation(testDb, trip.id, { title: 'Guided Walk', type: 'activity' }); + // Importers write both shapes: "…T14:00" (padded to seconds) and "…T14:00:00" + // (already 15 chars). Padding the second one again would produce a 17-char + // value that no client parses. + testDb + .prepare('UPDATE reservations SET reservation_time=?, reservation_end_time=? WHERE id=?') + .run('2025-06-02T14:00:00', '2025-06-03', reservation.id); + + const { ics } = svc.exportICS(trip.id); + + expect(ics).toContain('DTSTART:20250602T140000'); + // The end value carries no clock time, so it cannot be a DTEND for a timed + // event; emitting the bare "20250603" would make the event span a nonsense + // range instead of being left open. + expect(ics).not.toContain('DTEND'); + }); + + it('CAL-012: reservations with no placeable time are dropped instead of emitting a broken VEVENT', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Timeless' }); + // Relative "Day N" trips store a clock time without a date; there is nothing + // to anchor it to, so the reservation must not reach the calendar at all. + const timeOnly = createReservation(testDb, trip.id, { title: 'Floating Dinner', type: 'restaurant' }); + testDb.prepare('UPDATE reservations SET reservation_time=? WHERE id=?').run('19:30', timeOnly.id); + // A transport row whose endpoints were never imported has no fallback time either. + const noEndpoints = createReservation(testDb, trip.id, { title: 'Endpointless Train', type: 'transport' }); + testDb.prepare('UPDATE reservations SET reservation_time=NULL WHERE id=?').run(noEndpoints.id); + + const { ics } = svc.exportICS(trip.id); + + expect(ics).not.toContain('SUMMARY:Floating Dinner'); + expect(ics).not.toContain('SUMMARY:Endpointless Train'); + // The trip itself has no dates and no days, so dropping both reservations + // leaves an event-free calendar rather than a partially filled one. + expect(ics).not.toContain('BEGIN:VEVENT'); + }); + + it('CAL-013: endpoints without a stored timezone fall back to their coordinates', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'No Stored Zone' }); + const reservation = createReservation(testDb, trip.id, { title: 'CDG to JFK', type: 'flight' }); + testDb.prepare('UPDATE reservations SET reservation_time=NULL WHERE id=?').run(reservation.id); + const insertEp = testDb.prepare( + 'INSERT INTO reservation_endpoints (reservation_id, role, sequence, name, code, lat, lng, timezone, local_time, local_date) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)' + ); + // Endpoints created before the importer learned to store IANA zones have only + // coordinates. Without the lookup fallback both ends would go floating and the + // subscriber would see the flight in their own zone (#1453). + insertEp.run(reservation.id, 'from', 0, 'Paris CDG', 'CDG', 49.0, 2.5, null, '09:00', '2025-06-02'); + insertEp.run(reservation.id, 'to', 1, 'New York JFK', 'JFK', 40.6, -73.8, null, '12:00', '2025-06-02'); + + const { ics } = svc.exportICS(trip.id); + + expect(ics).toContain('DTSTART;TZID=Europe/Paris:20250602T090000'); + expect(ics).toContain('DTEND;TZID=America/New_York:20250602T120000'); + }); + + it('CAL-014: a single-endpoint transport emits DTSTART only', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'One Way' }); + const reservation = createReservation(testDb, trip.id, { title: 'Airport Transfer', type: 'transport' }); + testDb.prepare('UPDATE reservations SET reservation_time=NULL WHERE id=?').run(reservation.id); + // Only the departure was imported. Using it for DTEND as well would emit a + // zero-length event; leaving DTEND out lets clients apply their default duration. + testDb.prepare( + 'INSERT INTO reservation_endpoints (reservation_id, role, sequence, name, code, lat, lng, timezone, local_time, local_date) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)' + ).run(reservation.id, 'from', 0, 'Paris CDG', 'CDG', 49.0, 2.5, 'Europe/Paris', '09:00', '2025-06-02'); + + const { ics } = svc.exportICS(trip.id); + + expect(ics).toContain('DTSTART;TZID=Europe/Paris:20250602T090000'); + expect(ics).not.toContain('DTEND'); + }); + + it('CAL-015: a timed assignment carries its notes and address into DESCRIPTION and LOCATION', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Guided Day' }); + const day = createDay(testDb, trip.id, { date: '2025-06-02' }); + const place = createPlace(testDb, trip.id, { name: 'Louvre', lat: 48.8606, lng: 2.3376 }); + testDb.prepare("UPDATE places SET address = 'Rue de Rivoli' WHERE id = ?").run(place.id); + const withNotes = createDayAssignment(testDb, day.id, place.id, { notes: 'meet the guide' }); + testDb.prepare('UPDATE day_assignments SET assignment_time=? WHERE id=?').run('09:00', withNotes.id); + const withoutNotes = createDayAssignment(testDb, day.id, place.id); + testDb.prepare('UPDATE day_assignments SET assignment_time=? WHERE id=?').run('11:00', withoutNotes.id); + + const ics = svc.exportICS(trip.id).ics.replace(/\r\n /g, ''); + + // Notes come first and the address is appended on its own line; dropping the + // separator would glue them into one unreadable run. + expect(ics).toContain('DESCRIPTION:meet the guide\\nRue de Rivoli'); + // Without notes the address must start the description rather than a stray newline. + expect(ics).toContain('DESCRIPTION:Rue de Rivoli\r\n'); + expect(ics).toContain('LOCATION:Rue de Rivoli'); + }); + + it('CAL-016: a day without a date is skipped and a titled day uses its own title', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Mixed Days' }); + // Days of a relative "Day N" trip have no date; they cannot be placed on a + // calendar, and emitting them produced a VEVENT with an empty DTSTART. + const ghost = createDay(testDb, trip.id, { title: 'Ghost Day' }); + createDayNote(testDb, ghost.id, trip.id, { text: 'never exported' }); + const named = createDay(testDb, trip.id, { date: '2025-06-02', title: 'Museum Day' }); + const place = createPlace(testDb, trip.id, { name: 'Bare Spot' }); + createDayAssignment(testDb, named.id, place.id); + + const ics = svc.exportICS(trip.id).ics.replace(/\r\n /g, ''); + + expect(ics).not.toContain('Ghost Day'); + expect(ics).not.toContain('never exported'); + // A day with a title must not be renamed to "Day ". + expect(ics).toContain('SUMMARY:Museum Day'); + // No address and no notes → the bullet stays bare, with no empty "()" or dash. + expect(ics).toContain('DESCRIPTION:• Bare Spot\r\n'); + }); + + it('CAL-017: a reservation with no type, metadata or notes emits no empty DESCRIPTION', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Bare' }); + const reservation = createReservation(testDb, trip.id, { title: 'Bare Booking', type: 'hotel' }); + // Rows imported before `type` became mandatory still exist in prod. An empty + // DESCRIPTION line is invalid enough for some clients to reject the file. + testDb + .prepare('UPDATE reservations SET type=NULL, reservation_time=? WHERE id=?') + .run('2025-06-02', reservation.id); + + const { ics } = svc.exportICS(trip.id); + + expect(ics).toContain('SUMMARY:Bare Booking'); + expect(ics).not.toContain('DESCRIPTION'); + }); + + it('CAL-018: flight metadata with only one airport emits only that side of the route', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Half Routes' }); + const inbound = createReservation(testDb, trip.id, { title: 'Inbound', type: 'flight' }); + const outbound = createReservation(testDb, trip.id, { title: 'Outbound', type: 'flight' }); + const setMeta = testDb.prepare('UPDATE reservations SET reservation_time=?, metadata=? WHERE id=?'); + // A hand-entered flight often has one airport only; the missing side must be + // left out instead of printing "To: undefined". + setMeta.run('2025-06-02T09:00', JSON.stringify({ arrival_airport: 'JFK' }), inbound.id); + setMeta.run('2025-06-09T09:00', JSON.stringify({ departure_airport: 'CDG' }), outbound.id); + + const ics = svc.exportICS(trip.id).ics.replace(/\r\n /g, ''); + + expect(ics).toContain('DESCRIPTION:Type: flight\\nTo: JFK\r\n'); + expect(ics).toContain('DESCRIPTION:Type: flight\\nFrom: CDG\r\n'); + }); + + it('CAL-019: legs and endpoints without names produce no Route line', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Nameless' }); + const legs = createReservation(testDb, trip.id, { title: 'Codeless Legs', type: 'flight' }); + // Multi-leg metadata whose legs carry no airports would otherwise render + // "Route: " with nothing after it. + testDb.prepare('UPDATE reservations SET reservation_time=?, metadata=? WHERE id=?') + .run('2025-06-02T09:00', JSON.stringify({ legs: [{}, {}] }), legs.id); + const ferry = createReservation(testDb, trip.id, { title: 'Nameless Ferry', type: 'transport' }); + testDb.prepare('UPDATE reservations SET reservation_time=NULL WHERE id=?').run(ferry.id); + const insertEp = testDb.prepare( + 'INSERT INTO reservation_endpoints (reservation_id, role, sequence, name, code, lat, lng, timezone, local_time, local_date) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)' + ); + // Same for endpoints with neither a code nor a name: the derived route would + // collapse to a single arrow between two blanks. + insertEp.run(ferry.id, 'from', 0, '', null, 1.0, 1.0, null, null, '2025-06-03'); + insertEp.run(ferry.id, 'to', 1, '', null, 1.1, 1.1, null, null, '2025-06-03'); + + const ics = svc.exportICS(trip.id).ics.replace(/\r\n /g, ''); + + expect(ics).toContain('SUMMARY:Codeless Legs'); + expect(ics).toContain('SUMMARY:Nameless Ferry'); + expect(ics).not.toContain('Route:'); + }); + + it('CAL-020: an empty trip title falls back for SUMMARY, X-WR-CALNAME and the filename', () => { + const { user } = createUser(testDb); + // The title is only NOT NULL, not non-empty; an empty one used to produce + // "SUMMARY:" and a filename of ".ics". + const trip = createTrip(testDb, user.id, { title: '', start_date: '2025-06-01', end_date: '2025-06-02' }); + + const { ics, filename } = svc.exportICS(trip.id); + + expect(ics).toContain('X-WR-CALNAME:TREK Trip'); + expect(ics).toContain('SUMMARY:Trip'); + expect(filename).toBe('trek-trip.ics'); + }); + + it('CAL-021: a corrupt day date degrades the VTIMEZONE offset instead of throwing', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Corrupt Date' }); + // days.date is free-form TEXT; a legacy/imported row can hold a date that no + // parser accepts. The representative date reaches Intl through the VTIMEZONE + // fallback, and Intl throws a RangeError on an invalid Date — which would take + // the whole export (and the trip's slot in the all-trips feed) down with it. + const day = createDay(testDb, trip.id, { date: '2025-13-45' }); + const place = createPlace(testDb, trip.id, { name: 'Senso-ji', lat: 35.7148, lng: 139.7967 }); + const assignment = createDayAssignment(testDb, day.id, place.id); + testDb.prepare('UPDATE day_assignments SET assignment_time=? WHERE id=?').run('09:00', assignment.id); + + let ics = ''; + expect(() => { ics = svc.exportICS(trip.id).ics; }).not.toThrow(); + + expect(ics).toContain('DTSTART;TZID=Asia/Tokyo:20251345T090000'); + expect(ics).toContain('BEGIN:VTIMEZONE\r\nTZID:Asia/Tokyo'); + expect(ics).toContain('TZOFFSETFROM:+0000'); + expect(ics).toContain('TZOFFSETTO:+0000'); + }); + + it('CAL-022: the timezone cache stays correct after it hits its 1000-entry bound', () => { + const { user } = createUser(testDb); + const bulkTrip = createTrip(testDb, user.id, { title: 'Cache Filler' }); + const realTrip = createTrip(testDb, user.id, { title: 'After The Bound' }); + + const insertRes = testDb.prepare('INSERT INTO reservations (trip_id, title, type, reservation_time) VALUES (?, ?, ?, NULL)'); + const insertEp = testDb.prepare( + 'INSERT INTO reservation_endpoints (reservation_id, role, sequence, name, code, lat, lng, timezone, local_time, local_date) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)' + ); + // Endpoint timezones are free-form strings written by importers and plugins, so + // the validity cache is keyed by attacker-ish input and has to be bounded. Push + // it past the bound in one export. + testDb.transaction(() => { + for (let i = 0; i < 1010; i++) { + const resId = insertRes.run(bulkTrip.id, `Bulk ${i}`, 'transport').lastInsertRowid as number; + insertEp.run(resId, 'from', 0, `Stop ${i}`, null, 49.0, 2.5, `Bogus/Zone-${i}`, '09:00', '2025-06-02'); + } + })(); + + const bulk = svc.exportICS(bulkTrip.id).ics; + // Every one of them has to reach the zone check — otherwise the bound is never + // approached and this case would pass without exercising anything. + expect(bulk).toContain('SUMMARY:Bulk 0'); + expect(bulk).toContain('SUMMARY:Bulk 1009'); + expect(bulk).toContain('DTSTART:20250602T090000'); + expect(bulk).not.toContain('TZID=Bogus/Zone-0:'); + + const reservation = createReservation(testDb, realTrip.id, { title: 'CDG to JFK', type: 'flight' }); + testDb.prepare('UPDATE reservations SET reservation_time=NULL WHERE id=?').run(reservation.id); + insertEp.run(reservation.id, 'from', 0, 'Paris CDG', 'CDG', 49.0, 2.5, 'Europe/Paris', '09:00', '2025-06-02'); + + // Clearing the cache must make later zones be re-checked. A regression that + // evicted by writing `false` instead would silently strip the TZID from every + // export after the thousandth distinct zone string. + const { ics } = svc.exportICS(realTrip.id); + expect(ics).toContain('DTSTART;TZID=Europe/Paris:20250602T090000'); + expect(ics).toContain('BEGIN:VTIMEZONE\r\nTZID:Europe/Paris'); + }); }); describe('folded quirk branches', () => { @@ -379,6 +629,65 @@ describe('folded quirk branches', () => { }); +// foldICS is the last thing that touches the bytes of both the download and the +// feed, and it is exported, so it is pinned directly instead of through a trip. +describe('foldICS', () => { + // Physical lines are measured in octets, so the split points below are byte + // offsets, not character offsets. + const octets = (s: string) => Buffer.from(s, 'utf8').length; + const unfold = (s: string) => s.replace(/\r\n /g, ''); + + it('CAL-023: lines up to 75 octets are untouched and longer ones fold at 75 then 74', () => { + const short = 'SUMMARY:' + 'a'.repeat(67); // exactly 75 octets + expect(foldICS(short)).toBe(short); + + const long = 'SUMMARY:' + 'a'.repeat(200); + const physical = foldICS(long).split('\r\n'); + expect(physical[0]).toHaveLength(75); + // Continuation lines spend one octet on the leading space, so they carry 74 + // payload octets — budgeting 75 there produces 76-octet lines that strict + // validators reject. + expect(physical[1].startsWith(' ')).toBe(true); + expect(physical[1]).toHaveLength(75); + expect(unfold(foldICS(long))).toBe(long); + }); + + it('CAL-024: a two-byte codepoint straddling the 75th octet is not split', () => { + // The fold boundary lands inside "é": without the backoff the two halves are + // decoded separately and the title arrives as U+FFFD in every client. + const line = 'DESCRIPTION:' + 'é'.repeat(60); + expect(octets(line)).toBeGreaterThan(75); + + const folded = foldICS(line); + + expect(folded).not.toContain('�'); + expect(unfold(folded)).toBe(line); + for (const part of folded.split('\r\n')) expect(octets(part)).toBeLessThanOrEqual(75); + }); + + it('CAL-025: a four-byte codepoint backs the split off by more than one octet', () => { + // An emoji needs the backoff to run several times before it reaches a lead + // byte; stopping after a single step still cuts the sequence. + const line = 'SUMMARY:' + '🎌'.repeat(40); + + const folded = foldICS(line); + + expect(folded).not.toContain('�'); + expect(unfold(folded)).toBe(line); + for (const part of folded.split('\r\n')) expect(octets(part)).toBeLessThanOrEqual(75); + }); + + it('CAL-026: folding is applied per content line, so short lines around a long one survive', () => { + const ics = 'BEGIN:VEVENT\r\nSUMMARY:' + 'ü'.repeat(80) + '\r\nEND:VEVENT'; + + const folded = foldICS(ics); + + expect(folded.startsWith('BEGIN:VEVENT\r\n')).toBe(true); + expect(folded.endsWith('\r\nEND:VEVENT')).toBe(true); + expect(unfold(folded)).toBe(ics); + }); +}); + // The snapshot below was taken from the implementation as it stood when the code // moved out of TripsService, before the calendar grew a structured model. It is // the parity net for any change to how the calendar is assembled: the 20 cases diff --git a/server/tests/unit/nest/days.service.test.ts b/server/tests/unit/nest/days.service.test.ts index 0e88a8d2d..e1034e748 100644 --- a/server/tests/unit/nest/days.service.test.ts +++ b/server/tests/unit/nest/days.service.test.ts @@ -3,7 +3,11 @@ * moved 1:1 from the legacy tests/unit/services/dayService.test.ts; * DAY-SVC-027 through DAY-SVC-032 pin the days.bridge delegation; * DAY-SVC-033 through DAY-SVC-036 pin the post-port defect fixes (update - * presence sentinels, accommodation write atomicity, batched tag load). + * presence sentinels, accommodation write atomicity, batched tag load); + * DAY-SVC-037 through DAY-SVC-054 cover the reorder/re-date machinery that + * stayed behind when the accommodation CRUD moved to accommodations/ — mostly + * the SKIP paths (booking without a day, snapshot without a date, stay that + * must not be re-anchored), which the happy-path integration test never walks. * Uses a real in-memory SQLite DB so SQL logic is exercised faithfully. */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; @@ -48,10 +52,10 @@ vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; -import { createUser, createTrip, createDay, createPlace, createDayAssignment, createDayAccommodation } from '../../helpers/factories'; +import { createUser, createTrip, createDay, createPlace, createDayAssignment, createDayAccommodation, createDayNote, createTag } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; -import { DaysService, addDays } from '../../../src/nest/days/days.service'; +import { DaysService, DayReorderError, addDays } from '../../../src/nest/days/days.service'; import { getDay as bridgeGetDay, listDays as bridgeListDays } from '../../../src/nest/days/days.bridge'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; @@ -359,6 +363,393 @@ describe('quirk fixes', () => { }); }); +// ── reorder / re-date machinery (#589) ──────────────────────────────────────── +// The integration test walks the happy paths (permute, re-pin, extend). What is +// only reachable from here are the paths where something must NOT move: a +// booking the maps say nothing about, a stay whose old dates no longer resolve, +// a trip with no dates at all. Those are the branches a regression would take +// silently — the day list still looks right while a booking or a stay quietly +// jumps to the wrong day. + +describe('restampReservationDates', () => { + it('DAY-SVC-037 — leaves a booking alone when it has no day, no time, or its day did not move', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const stayer = createDay(testDb, trip.id, { date: '2026-01-01' }); + const unmapped = createDay(testDb, trip.id, { date: '2026-01-02' }); + const halfMapped = createDay(testDb, trip.id, { date: '2026-01-03' }); + + const insert = testDb.prepare('INSERT INTO reservations (trip_id, day_id, title, reservation_time) VALUES (?, ?, ?, ?)'); + const unplaced = Number(insert.run(trip.id, null, 'Unplaced', '2026-01-01T09:00').lastInsertRowid); + const untimed = Number(insert.run(trip.id, stayer.id, 'Untimed', null).lastInsertRowid); + const parked = Number(insert.run(trip.id, stayer.id, 'Parked', '2026-01-01T19:00').lastInsertRowid); + const offMap = Number(insert.run(trip.id, unmapped.id, 'Off map', '2026-01-02T12:00').lastInsertRowid); + const halfOff = Number(insert.run(trip.id, halfMapped.id, 'Half off', '2026-01-03T12:00').lastInsertRowid); + + svc.restampReservationDates( + trip.id, + new Map([[stayer.id, '2026-01-01'], [halfMapped.id, '2026-01-03']]), + // stayer keeps its date, unmapped is in neither map, halfMapped only in the old one. + new Map([[stayer.id, '2026-01-01']]), + ); + + const times = Object.fromEntries( + (testDb.prepare('SELECT id, reservation_time FROM reservations WHERE trip_id = ?').all(trip.id) as { id: number; reservation_time: string | null }[]) + .map(r => [r.id, r.reservation_time]) + ); + expect(times).toEqual({ + [unplaced]: '2026-01-01T09:00', + [untimed]: null, + [parked]: '2026-01-01T19:00', + [offMap]: '2026-01-02T12:00', + [halfOff]: '2026-01-03T12:00', + }); + }); + + it('DAY-SVC-038 — shifts every transport leg by the booking\'s day delta and skips a leg with no date', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id, { date: '2026-01-01' }); + const flight = Number(testDb.prepare( + 'INSERT INTO reservations (trip_id, day_id, title, type, reservation_time) VALUES (?, ?, ?, ?, ?)' + ).run(trip.id, day.id, 'HEL-NRT', 'flight', '2026-01-01T08:00').lastInsertRowid); + + const endpoint = testDb.prepare( + 'INSERT INTO reservation_endpoints (reservation_id, role, sequence, name, lat, lng, local_date) VALUES (?, ?, ?, ?, ?, ?, ?)' + ); + const departure = Number(endpoint.run(flight, 'from', 0, 'HEL', 60.31, 24.96, '2026-01-01').lastInsertRowid); + // The arrival sits a day after departure; the legs must keep that distance, + // otherwise a moved overnight flight lands before it took off. + const arrival = Number(endpoint.run(flight, 'to', 1, 'NRT', 35.76, 140.38, '2026-01-02').lastInsertRowid); + const undated = Number(endpoint.run(flight, 'to', 2, 'Unknown', 0, 0, null).lastInsertRowid); + + svc.restampReservationDates(trip.id, new Map([[day.id, '2026-01-01']]), new Map([[day.id, '2026-01-04']])); + + const legs = Object.fromEntries( + (testDb.prepare('SELECT id, local_date FROM reservation_endpoints WHERE reservation_id = ?').all(flight) as { id: number; local_date: string | null }[]) + .map(l => [l.id, l.local_date]) + ); + expect(legs).toEqual({ [departure]: '2026-01-04', [arrival]: '2026-01-05', [undated]: null }); + }); + + it('DAY-SVC-039 — follows end_day_id and keeps a date-only end time date-only', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const start = createDay(testDb, trip.id, { date: '2026-01-01' }); + const end = createDay(testDb, trip.id, { date: '2026-01-03' }); + + const insert = testDb.prepare( + 'INSERT INTO reservations (trip_id, day_id, end_day_id, title, reservation_time, reservation_end_time) VALUES (?, ?, ?, ?, ?, ?)' + ); + const spanning = Number(insert.run(trip.id, start.id, end.id, 'Sleeper train', '2026-01-01T15:00', '2026-01-03').lastInsertRowid); + const openEnded = Number(insert.run(trip.id, null, end.id, 'Open ended', null, null).lastInsertRowid); + + svc.restampReservationDates( + trip.id, + new Map([[start.id, '2026-01-01'], [end.id, '2026-01-03']]), + new Map([[start.id, '2026-01-02'], [end.id, '2026-01-05']]), + ); + + const moved = testDb.prepare('SELECT reservation_time, reservation_end_time FROM reservations WHERE id = ?').get(spanning) as + { reservation_time: string; reservation_end_time: string }; + expect(moved.reservation_time).toBe('2026-01-02T15:00'); + // A date-only end time must not grow a time suffix from the new date string. + expect(moved.reservation_end_time).toBe('2026-01-05'); + expect(testDb.prepare('SELECT reservation_end_time FROM reservations WHERE id = ?').get(openEnded)) + .toMatchObject({ reservation_end_time: null }); + }); +}); + +describe('resyncAccommodationDays', () => { + /** The hotel reservation accommodations/ auto-creates next to a stay. */ + const linkHotel = (tripId: number, accId: number, dayId: number | null, time: string | null) => + Number(testDb.prepare( + "INSERT INTO reservations (trip_id, day_id, title, type, accommodation_id, reservation_time) VALUES (?, ?, 'Hotel', 'hotel', ?, ?)" + ).run(tripId, dayId, accId, time).lastInsertRowid); + + it('DAY-SVC-040 — returns before touching a hotel booking when the trip has no stays', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id, { date: '2026-05-01' }); + const res = linkHotel(trip.id, 4242, null, null); + + svc.resyncAccommodationDays(trip.id, new Map([[day.id, '2026-04-01']])); + + // No stay owns this booking any more, so the restamp must not adopt it. + expect(testDb.prepare('SELECT day_id FROM reservations WHERE id = ?').get(res)).toMatchObject({ day_id: null }); + }); + + it('DAY-SVC-041 — leaves a stay glued to its rows when the snapshot has no date for them', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const d1 = createDay(testDb, trip.id, { date: '2026-05-01' }); + const d2 = createDay(testDb, trip.id, { date: '2026-05-02' }); + const place = createPlace(testDb, trip.id, { name: 'Hotel' }); + const stay = createDayAccommodation(testDb, trip.id, place.id, d1.id, d2.id); + const res = linkHotel(trip.id, stay.id, d2.id, '2026-04-30T14:00'); + + svc.resyncAccommodationDays(trip.id, new Map()); + + expect(testDb.prepare('SELECT start_day_id, end_day_id FROM day_accommodations WHERE id = ?').get(stay.id)) + .toMatchObject({ start_day_id: d1.id, end_day_id: d2.id }); + // The linked booking still gets pulled back onto the stay's start day: its + // reservation_time is a stale snapshot of that day's date, check-in time kept. + expect(testDb.prepare('SELECT day_id, reservation_time FROM reservations WHERE id = ?').get(res)) + .toMatchObject({ day_id: d1.id, reservation_time: '2026-05-01T14:00' }); + }); + + it('DAY-SVC-042 — does not rewrite a stay whose days already hold their old dates', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const d1 = createDay(testDb, trip.id, { date: '2026-05-01' }); + const d2 = createDay(testDb, trip.id, { date: '2026-05-02' }); + const place = createPlace(testDb, trip.id, { name: 'Hotel' }); + const stay = createDayAccommodation(testDb, trip.id, place.id, d1.id, d2.id); + const res = linkHotel(trip.id, stay.id, d1.id, null); + + svc.resyncAccommodationDays(trip.id, new Map([[d1.id, '2026-05-01'], [d2.id, '2026-05-02']])); + + expect(testDb.prepare('SELECT start_day_id, end_day_id FROM day_accommodations WHERE id = ?').get(stay.id)) + .toMatchObject({ start_day_id: d1.id, end_day_id: d2.id }); + // A booking with no time at all takes the bare date, not date + empty suffix. + expect(testDb.prepare('SELECT reservation_time FROM reservations WHERE id = ?').get(res)) + .toMatchObject({ reservation_time: '2026-05-01' }); + }); + + it('DAY-SVC-043 — refuses to re-anchor a stay onto an inverted day pair', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const d1 = createDay(testDb, trip.id, { date: '2026-05-01' }); + const d2 = createDay(testDb, trip.id, { date: '2026-05-02' }); + const place = createPlace(testDb, trip.id, { name: 'Hotel' }); + const stay = createDayAccommodation(testDb, trip.id, place.id, d1.id, d2.id); + + // The range change swapped the two dates across the rows, so the day now + // holding the old start date sits AFTER the one holding the old end date. + testDb.prepare('UPDATE days SET date = ? WHERE id = ?').run('2026-05-02', d1.id); + testDb.prepare('UPDATE days SET date = ? WHERE id = ?').run('2026-05-01', d2.id); + + svc.resyncAccommodationDays(trip.id, new Map([[d1.id, '2026-05-01'], [d2.id, '2026-05-02']])); + + expect(testDb.prepare('SELECT start_day_id, end_day_id FROM day_accommodations WHERE id = ?').get(stay.id)) + .toMatchObject({ start_day_id: d1.id, end_day_id: d2.id }); + }); + + it('DAY-SVC-044 — leaves a stay alone when one of its old dates is outside the new range', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const d1 = createDay(testDb, trip.id, { date: '2026-05-01' }); + const d2 = createDay(testDb, trip.id, { date: '2026-05-02' }); + const d3 = createDay(testDb, trip.id, { date: '2026-05-03' }); + const place = createPlace(testDb, trip.id, { name: 'Hotel' }); + // d2's old date (2026-04-29) dropped out of the range: the first stay finds + // no day for its end, the second finds none for its start. + const endGone = createDayAccommodation(testDb, trip.id, place.id, d1.id, d2.id); + const startGone = createDayAccommodation(testDb, trip.id, place.id, d2.id, d3.id); + + svc.resyncAccommodationDays( + trip.id, + new Map([[d1.id, '2026-05-01'], [d2.id, '2026-04-29'], [d3.id, '2026-05-03']]), + ); + + expect(testDb.prepare('SELECT start_day_id, end_day_id FROM day_accommodations WHERE id = ?').get(endGone.id)) + .toMatchObject({ start_day_id: d1.id, end_day_id: d2.id }); + expect(testDb.prepare('SELECT start_day_id, end_day_id FROM day_accommodations WHERE id = ?').get(startGone.id)) + .toMatchObject({ start_day_id: d2.id, end_day_id: d3.id }); + }); + + it('DAY-SVC-045 — skips the linked-booking restamp when the start day has no date', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id, { name: 'Hostel' }); + const stay = createDayAccommodation(testDb, trip.id, place.id, day.id, day.id); + const res = linkHotel(trip.id, stay.id, null, null); + + svc.resyncAccommodationDays(trip.id, new Map([[day.id, null]])); + + // On a dateless trip there is no date to stamp; writing one would invent a + // calendar the trip does not have. + expect(testDb.prepare('SELECT day_id, reservation_time FROM reservations WHERE id = ?').get(res)) + .toMatchObject({ day_id: null, reservation_time: null }); + }); +}); + +describe('reorder', () => { + const orderedDays = (tripId: number) => + testDb.prepare('SELECT id, day_number, date FROM days WHERE trip_id = ? ORDER BY day_number').all(tripId) as + { id: number; day_number: number; date: string | null }[]; + + it('DAY-SVC-046 — rejects a same-length list that contains a foreign day id', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const d1 = createDay(testDb, trip.id); + createDay(testDb, trip.id); + + // Same length as the trip's day list, so only the membership check can catch + // it — without it the renumber would leave one day orphaned at a negative + // day_number and drop another out of the itinerary. + expect(() => svc.reorder(trip.id, [d1.id, 999999])).toThrow(DayReorderError); + expect(orderedDays(trip.id).map(d => d.day_number)).toEqual([1, 2]); + }); + + it('DAY-SVC-047 — renumbers a dateless trip without inventing dates or touching bookings', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const d1 = createDay(testDb, trip.id); + const d2 = createDay(testDb, trip.id); + const d3 = createDay(testDb, trip.id); + const res = Number(testDb.prepare( + 'INSERT INTO reservations (trip_id, day_id, title, reservation_time) VALUES (?, ?, ?, ?)' + ).run(trip.id, d2.id, 'Dinner', '2026-02-02T19:00').lastInsertRowid); + + const result = svc.reorder(trip.id, [d3.id, d1.id, d2.id]); + + expect(result.days.map(d => d.id)).toEqual([d3.id, d1.id, d2.id]); + expect(orderedDays(trip.id).map(d => d.date)).toEqual([null, null, null]); + // A trip without dates has no slots to re-pin, so the re-stamp pass is + // skipped entirely — a booking keeps whatever time it was given by hand. + expect(testDb.prepare('SELECT reservation_time FROM reservations WHERE id = ?').get(res)) + .toMatchObject({ reservation_time: '2026-02-02T19:00' }); + }); + + it('DAY-SVC-048 — pins the known dates to the leading slots and nulls the slots beyond them', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const dated = createDay(testDb, trip.id, { date: '2026-03-01' }); + const dateless = createDay(testDb, trip.id); + + svc.reorder(trip.id, [dateless.id, dated.id]); + + // The slot beyond the known dates must resolve to null: better-sqlite3 + // refuses to bind the `undefined` a bare index lookup would hand it. + expect(orderedDays(trip.id)).toEqual([ + { id: dateless.id, day_number: 1, date: '2026-03-01' }, + { id: dated.id, day_number: 2, date: null }, + ]); + }); + + it('DAY-SVC-049 — allows a move that keeps a stay ordered and rolls back one that inverts it', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const d1 = createDay(testDb, trip.id, { date: '2026-03-01' }); + const d2 = createDay(testDb, trip.id, { date: '2026-03-02' }); + const d3 = createDay(testDb, trip.id, { date: '2026-03-03' }); + const place = createPlace(testDb, trip.id, { name: 'Hotel' }); + createDayAccommodation(testDb, trip.id, place.id, d1.id, d2.id); + + // Stretching the stay over a third slot is legal — the guard only rejects + // an end that lands before its start. + svc.reorder(trip.id, [d1.id, d3.id, d2.id]); + expect(orderedDays(trip.id).map(d => d.id)).toEqual([d1.id, d3.id, d2.id]); + + expect(() => svc.reorder(trip.id, [d2.id, d3.id, d1.id])).toThrow(DayReorderError); + // The guard runs inside the transaction, so the rejected move leaves the + // day numbers AND the re-pinned dates exactly as the legal move left them. + expect(orderedDays(trip.id)).toEqual([ + { id: d1.id, day_number: 1, date: '2026-03-01' }, + { id: d3.id, day_number: 2, date: '2026-03-02' }, + { id: d2.id, day_number: 3, date: '2026-03-03' }, + ]); + }); +}); + +describe('insert', () => { + it('DAY-SVC-050 — appends at the end when no position is given and shifts nothing', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const d1 = createDay(testDb, trip.id); + const d2 = createDay(testDb, trip.id); + + const created = svc.insert(trip.id); + + expect(created).toMatchObject({ day_number: 3, date: null, assignments: [], notes_items: [] }); + const rows = testDb.prepare('SELECT id, day_number FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as + { id: number; day_number: number }[]; + expect(rows).toEqual([ + { id: d1.id, day_number: 1 }, + { id: d2.id, day_number: 2 }, + { id: created.id, day_number: 3 }, + ]); + }); +}); + +// ── day shaping ─────────────────────────────────────────────────────────────── + +describe('setDefaultTransportMode', () => { + it('DAY-SVC-051 — sets and clears the whole-day mode without disturbing notes or title', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id, { title: 'Arrival' }); + svc.update(day.id, day as never, { notes: 'Keep me' }); + + const set = svc.setDefaultTransportMode(day.id, 'walk'); + expect(set).toMatchObject({ default_transport_mode: 'walk', title: 'Arrival', notes: 'Keep me' }); + + // Its own endpoint exists precisely so clearing the mode cannot wipe the + // day's text the way a general update would. + const cleared = svc.setDefaultTransportMode(day.id, null); + expect(cleared).toMatchObject({ default_transport_mode: null, title: 'Arrival', notes: 'Keep me' }); + }); +}); + +describe('day shaping', () => { + it('DAY-SVC-052 — getAssignmentsForDay reports a null category for an uncategorised place', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const placeId = Number(testDb.prepare('INSERT INTO places (trip_id, name, category_id) VALUES (?, ?, NULL)') + .run(trip.id, 'Uncategorised').lastInsertRowid); + createDayAssignment(testDb, day.id, placeId); + + // The LEFT JOIN yields category_name/color/icon as NULL, which must collapse + // to a null category instead of an object of nulls the client would render. + expect(svc.getAssignmentsForDay(day.id)[0].place.category).toBeNull(); + }); + + it('DAY-SVC-053 — list groups assignments, tags, participants and notes onto their own day', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const busy = createDay(testDb, trip.id); + const empty = createDay(testDb, trip.id); + const tagged = createPlace(testDb, trip.id, { name: 'Louvre' }); + const plain = createPlace(testDb, trip.id, { name: 'Pont Neuf' }); + const shared = createDayAssignment(testDb, busy.id, tagged.id, { order_index: 0 }); + createDayAssignment(testDb, busy.id, plain.id, { order_index: 1 }); + const tag = createTag(testDb, user.id, { name: 'Museum' }); + testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(tagged.id, tag.id); + testDb.prepare('INSERT INTO assignment_participants (assignment_id, user_id) VALUES (?, ?)').run(shared.id, user.id); + createDayNote(testDb, busy.id, trip.id, { text: 'Breakfast', sort_order: 1 }); + createDayNote(testDb, busy.id, trip.id, { text: 'Dinner', sort_order: 2 }); + + const { days } = svc.list(trip.id); + + expect(days[0].assignments.map(a => a.place.name)).toEqual(['Louvre', 'Pont Neuf']); + expect(days[0].assignments[0].place.tags.map(t => t.name)).toEqual(['Museum']); + expect(days[0].assignments[0].participants.map(p => p.user_id)).toEqual([user.id]); + // The batch loaders return nothing for the untagged place and the assignment + // nobody joined; both have to fall back to [] rather than undefined. + expect(days[0].assignments[1].place.tags).toEqual([]); + expect(days[0].assignments[1].participants).toEqual([]); + expect(days[0].notes_items.map(n => n.text)).toEqual(['Breakfast', 'Dinner']); + // A day nobody planned anything on still ships both collections. + expect(days[1]).toMatchObject({ id: empty.id, assignments: [], notes_items: [] }); + }); + + it('DAY-SVC-054 — create keeps an explicit date and notes, update clears a title sent as null', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + + const day = svc.create(trip.id, '2026-09-14', 'Ferry to the island'); + expect(day).toMatchObject({ date: '2026-09-14', notes: 'Ferry to the island' }); + + // The MCP update_day tool clears a title by sending null, which must reach + // the column instead of being treated as "key absent, keep the old title". + const titled = svc.update(day.id, day as never, { title: 'Crossing' }); + expect(svc.update(day.id, titled as never, { title: null })).toMatchObject({ title: null, notes: 'Ferry to the island' }); + }); +}); + /** * canEdit is MCP-only now: the HTTP path checks the same right through * TripAccessGuard's @RequirePermission('day_edit'), so nothing in a controller test diff --git a/server/tests/unit/nest/feeds.service.test.ts b/server/tests/unit/nest/feeds.service.test.ts new file mode 100644 index 000000000..64d6c9a07 --- /dev/null +++ b/server/tests/unit/nest/feeds.service.test.ts @@ -0,0 +1,393 @@ +/** + * Unit tests for FeedsService — FEED-SVC-001 through FEED-SVC-021. The domain had + * no unit suite at all: everything rested on tests/e2e/feeds.e2e.test.ts, which + * drives the HTTP surface and therefore only ever merges a single trip's calendar. + * These cases pin what the e2e cannot reach — the merge rules buildUserIcs applies + * across several trips (TZID dedupe, a failing trip being skipped, header vs body + * folding) and the second-call branches of the token lifecycle. + * + * buildUserIcs/buildTripIcs consume CalendarService.buildTripCalendar's PARTS now + * instead of scanning a finished ICS document back apart, so the calendar is a stub + * here and the parts are the test's own. The token lookups are real SQL against an + * in-memory SQLite DB (same harness as calendar.service.test.ts). + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; + +// ── DB setup ────────────────────────────────────────────────────────────────── + +const { testDb, dbMock } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA busy_timeout = 5000'); + const mock = { + db, + closeDb: () => {}, + reinitialize: () => {}, + getPlaceWithTags: () => null, + canAccessTrip: (tripId: number | string, userId: number) => + db.prepare(` + SELECT t.id, t.user_id FROM trips t + LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? + WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL) + `).get(userId, tripId, userId), + isOwner: (tripId: number | string, userId: number) => + !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), + }; + return { testDb: db, dbMock: mock }; +}); + +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../src/config', () => ({ + JWT_SECRET: 'test-secret', + ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', + updateJwtSecret: () => {}, +})); +vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() })); + +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { resetTestDb } from '../../helpers/test-db'; +import { createUser, createTrip, addTripMember } from '../../helpers/factories'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { FeedsService } from '../../../src/nest/feeds/feeds.service'; +import { FeedsModule } from '../../../src/nest/feeds/feeds.module'; +import { + FeedsPublicController, + TripFeedTokenController, + UserFeedTokenController, +} from '../../../src/nest/feeds/feeds.controller'; +import type { CalendarService, TripCalendar } from '../../../src/nest/calendar/calendar.service'; +import { expectRegisteredProvider, expectRegisteredController } from '../../helpers/module-providers'; + +const BASE = 'https://trek.example.test'; + +const buildTripCalendar = vi.fn(); +const svc = new FeedsService( + new DatabaseService(testDb), + { buildTripCalendar } as unknown as CalendarService, +); + +// ── Calendar parts the stub hands back ──────────────────────────────────────── + +const vevent = (summary: string) => + `BEGIN:VEVENT\r\nUID:trek-trip-${summary}@trek\r\nDTSTAMP:20260101T000000Z\r\n` + + `DTSTART;VALUE=DATE:20260101\r\nDTEND;VALUE=DATE:20260102\r\nSUMMARY:${summary}\r\nEND:VEVENT\r\n`; + +const vtimezone = (tzid: string, tzname = tzid) => + `BEGIN:VTIMEZONE\r\nTZID:${tzid}\r\nBEGIN:STANDARD\r\nDTSTART:19700101T000000\r\n` + + `TZOFFSETFROM:+0900\r\nTZOFFSETTO:+0900\r\nTZNAME:${tzname}\r\nEND:STANDARD\r\nEND:VTIMEZONE\r\n`; + +const calendarParts = (overrides: Partial = {}): TripCalendar => ({ + calName: 'Sample', + filename: 'sample.ics', + timezones: new Map(), + events: [vevent('Sample')], + ...overrides, +}); + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + resetTestDb(testDb); + buildTripCalendar.mockReset(); + buildTripCalendar.mockImplementation(() => calendarParts()); +}); + +afterAll(() => { + testDb.close(); +}); + +function seedTrip(token?: string) { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + if (token) testDb.prepare('UPDATE trips SET feed_token = ? WHERE id = ?').run(token, trip.id); + return { user, trip, tripId: String(trip.id) }; +} + +function seedUserWithToken(token: string, overrides: Partial<{ username: string }> = {}) { + const { user } = createUser(testDb, overrides); + testDb.prepare('UPDATE users SET feed_token = ? WHERE id = ?').run(token, user.id); + return user; +} + +// ── Trip feed token ─────────────────────────────────────────────────────────── + +describe('trip feed token lifecycle', () => { + it('FEED-SVC-001: reports no URL while the trip has no token', () => { + const { user, tripId } = seedTrip(); + + expect(svc.getTripToken(tripId, user.id, BASE)).toEqual({ feed_url: null }); + }); + + it('FEED-SVC-002: reports the absolute feed URL once a token exists', () => { + const { user, tripId } = seedTrip('tok-trip'); + + expect(svc.getTripToken(tripId, user.id, BASE)).toEqual({ + feed_url: `${BASE}/api/feed/trip/tok-trip.ics`, + }); + }); + + it('FEED-SVC-003: a trailing slash on the base is stripped, never doubled into //api', () => { + // APP_URL is user-supplied config; pasted with a trailing slash it would + // otherwise produce https://host//api/feed/... which some clients reject. + const { user, tripId } = seedTrip('tok-trip'); + + expect(svc.getTripToken(tripId, user.id, `${BASE}/`).feed_url).toBe( + `${BASE}/api/feed/trip/tok-trip.ics`, + ); + }); + + it('FEED-SVC-004: a user without access gets null, not the token of a foreign trip', () => { + // The token is the credential for the public feed, so leaking it through the + // authenticated GET would hand a stranger the whole trip. + const { tripId } = seedTrip('tok-trip'); + const { user: outsider } = createUser(testDb); + + expect(svc.getTripToken(tripId, outsider.id, BASE)).toEqual({ feed_url: null }); + }); + + it('FEED-SVC-005: a trip shared with the user as a member resolves too', () => { + const { trip, tripId } = seedTrip('tok-trip'); + const { user: member } = createUser(testDb); + addTripMember(testDb, trip.id, member.id); + + expect(svc.getTripToken(tripId, member.id, BASE).feed_url).toBe( + `${BASE}/api/feed/trip/tok-trip.ics`, + ); + }); + + it('FEED-SVC-006: generate mints a token once and stays idempotent', () => { + // Enabling twice must not invalidate a URL the user already handed to their + // calendar client — that is what rotate is for. + const { user, tripId } = seedTrip(); + + const first = svc.generateTripToken(tripId, user.id, BASE); + const second = svc.generateTripToken(tripId, user.id, BASE); + + expect(first.feed_url).toMatch(new RegExp(`^${BASE}/api/feed/trip/[0-9a-f-]+\\.ics$`)); + expect(second.feed_url).toBe(first.feed_url); + }); + + it('FEED-SVC-007: rotate issues a fresh token and the previous URL stops resolving', () => { + const { user, tripId } = seedTrip(); + const before = svc.generateTripToken(tripId, user.id, BASE).feed_url; + const oldToken = before.match(/trip\/([0-9a-f-]+)\.ics$/)![1]; + + const after = svc.rotateTripToken(tripId, BASE).feed_url; + + expect(after).not.toBe(before); + expect(svc.buildTripIcs(oldToken)).toBeNull(); + }); + + it('FEED-SVC-008: disable clears the column so the public URL dies', () => { + const { user, tripId } = seedTrip(); + const url = svc.generateTripToken(tripId, user.id, BASE).feed_url; + const token = url.match(/trip\/([0-9a-f-]+)\.ics$/)![1]; + + svc.disableTripToken(tripId); + + expect(svc.getTripToken(tripId, user.id, BASE)).toEqual({ feed_url: null }); + expect(svc.buildTripIcs(token)).toBeNull(); + }); +}); + +// ── User (all-trips) feed token ─────────────────────────────────────────────── + +describe('user feed token lifecycle', () => { + it('FEED-SVC-009: reports null before and the absolute URL after generation', () => { + const { user } = createUser(testDb); + + expect(svc.getUserToken(user.id, BASE)).toEqual({ feed_url: null }); + + const generated = svc.generateUserToken(user.id, BASE).feed_url; + + expect(generated).toMatch(new RegExp(`^${BASE}/api/feed/user/[0-9a-f-]+\\.ics$`)); + expect(svc.getUserToken(user.id, BASE).feed_url).toBe(generated); + }); + + it('FEED-SVC-010: generate is idempotent — the existing URL is returned unchanged', () => { + const { user } = createUser(testDb); + + const first = svc.generateUserToken(user.id, BASE); + const second = svc.generateUserToken(user.id, BASE); + + expect(second.feed_url).toBe(first.feed_url); + }); + + it('FEED-SVC-011: rotate issues a fresh token and the previous URL stops resolving', () => { + const { user } = createUser(testDb); + const before = svc.generateUserToken(user.id, BASE).feed_url; + const oldToken = before.match(/user\/([0-9a-f-]+)\.ics$/)![1]; + + const after = svc.rotateUserToken(user.id, BASE).feed_url; + + expect(after).not.toBe(before); + expect(svc.buildUserIcs(oldToken)).toBeNull(); + }); + + it('FEED-SVC-012: disable clears the column so the public URL dies', () => { + const { user } = createUser(testDb); + const url = svc.generateUserToken(user.id, BASE).feed_url; + const token = url.match(/user\/([0-9a-f-]+)\.ics$/)![1]; + + svc.disableUserToken(user.id); + + expect(svc.getUserToken(user.id, BASE)).toEqual({ feed_url: null }); + expect(svc.buildUserIcs(token)).toBeNull(); + }); +}); + +// ── buildTripIcs ────────────────────────────────────────────────────────────── + +describe('buildTripIcs', () => { + it('FEED-SVC-013: an unknown token yields null without asking the calendar', () => { + seedTrip('tok-trip'); + + expect(svc.buildTripIcs('00000000-0000-0000-0000-000000000000')).toBeNull(); + expect(buildTripCalendar).not.toHaveBeenCalled(); + }); + + it('FEED-SVC-014: a calendar that throws yields null instead of propagating', () => { + // The public feed is unauthenticated: a trip the calendar cannot render (a row + // deleted mid-request, unparseable data) has to come back as a 404, not a 500 + // that a subscribing client retries hourly forever. + seedTrip('tok-trip'); + buildTripCalendar.mockImplementation(() => { + throw new Error('calendar exploded'); + }); + + expect(svc.buildTripIcs('tok-trip')).toBeNull(); + }); + + it('FEED-SVC-015: the refresh hints sit in the preamble, ahead of X-WR-CALNAME and every component', () => { + // REFRESH-INTERVAL/X-PUBLISHED-TTL are calendar properties: RFC 5545 puts them + // before the first component, and clients that scan only the preamble stop + // re-fetching if they slip behind a VTIMEZONE. The document is concatenated from + // the calendar's parts now, so the order is an assembly decision, not a given. + const { trip } = seedTrip('tok-trip'); + buildTripCalendar.mockImplementation(() => + calendarParts({ + calName: 'Golden Trip', + filename: 'golden-trip.ics', + timezones: new Map([['Asia/Tokyo', vtimezone('Asia/Tokyo')]]), + }), + ); + + const result = svc.buildTripIcs('tok-trip'); + + expect(result).not.toBeNull(); + expect(result!.filename).toBe('golden-trip.ics'); + expect(result!.ics).toContain( + 'METHOD:PUBLISH\r\nREFRESH-INTERVAL;VALUE=DURATION:PT1H\r\nX-PUBLISHED-TTL:PT1H\r\n' + + 'X-WR-CALNAME:Golden Trip\r\nBEGIN:VTIMEZONE\r\n', + ); + expect(result!.ics.startsWith('BEGIN:VCALENDAR\r\nVERSION:2.0\r\n')).toBe(true); + expect(result!.ics.endsWith('END:VEVENT\r\nEND:VCALENDAR\r\n')).toBe(true); + expect(buildTripCalendar).toHaveBeenCalledWith(trip.id); + }); +}); + +// ── buildUserIcs ────────────────────────────────────────────────────────────── + +describe('buildUserIcs', () => { + it('FEED-SVC-016: an unknown token yields null without asking the calendar', () => { + seedUserWithToken('tok-user'); + + expect(svc.buildUserIcs('00000000-0000-0000-0000-000000000000')).toBeNull(); + expect(buildTripCalendar).not.toHaveBeenCalled(); + }); + + it('FEED-SVC-017: a trip whose calendar throws is skipped, the rest are still emitted', () => { + // One unrenderable trip must not take the whole all-trips subscription down — + // the user would silently lose every calendar entry because of a single bad row. + const user = seedUserWithToken('tok-user'); + const good = createTrip(testDb, user.id, { start_date: '2026-01-01' }); + const broken = createTrip(testDb, user.id, { start_date: '2026-02-01' }); + const alsoGood = createTrip(testDb, user.id, { start_date: '2026-03-01' }); + buildTripCalendar.mockImplementation((id: number) => { + if (id === broken.id) throw new Error('calendar exploded'); + return calendarParts({ events: [vevent(`Trip${id}`)] }); + }); + + const result = svc.buildUserIcs('tok-user'); + + expect(buildTripCalendar).toHaveBeenCalledTimes(3); + expect(result!.ics).toContain(`SUMMARY:Trip${good.id}\r\n`); + expect(result!.ics).toContain(`SUMMARY:Trip${alsoGood.id}\r\n`); + expect(result!.ics).not.toContain(`SUMMARY:Trip${broken.id}\r\n`); + expect(result!.ics.endsWith('END:VCALENDAR\r\n')).toBe(true); + }); + + it('FEED-SVC-018: a TZID shared by two trips is defined exactly once, ahead of every VEVENT', () => { + // Two VTIMEZONE blocks with the same TZID make the document invalid and clients + // drop the events referencing it; a block emitted after the VEVENT that uses it + // does not resolve either (#1453). First definition wins. + const user = seedUserWithToken('tok-user'); + const first = createTrip(testDb, user.id, { start_date: '2026-01-01' }); + createTrip(testDb, user.id, { start_date: '2026-02-01' }); + buildTripCalendar.mockImplementation((id: number) => ({ + calName: `Trip ${id}`, + filename: `trip-${id}.ics`, + timezones: new Map([ + ['Asia/Tokyo', vtimezone('Asia/Tokyo', id === first.id ? 'Asia/Tokyo' : 'Second/Definition')], + ]), + events: [vevent(`Trip${id}`)], + })); + + const { ics } = svc.buildUserIcs('tok-user')!; + + expect(ics.split('BEGIN:VTIMEZONE').length - 1).toBe(1); + expect(ics).toContain('TZNAME:Asia/Tokyo\r\n'); + expect(ics).not.toContain('Second/Definition'); + expect(ics.indexOf('BEGIN:VTIMEZONE')).toBeLessThan(ics.indexOf('BEGIN:VEVENT')); + }); + + it('FEED-SVC-019: the header is never folded, the body always is', () => { + // Folding is applied to the body only, on purpose: a long display name would + // otherwise wrap X-WR-CALNAME across two physical lines, which several clients + // render as a truncated calendar title. The body still has to fold — RFC 5545 + // caps a content line at 75 octets. + const username = 'Ferdinand-Bartholomew-'.repeat(5); + const user = seedUserWithToken('tok-user', { username }); + createTrip(testDb, user.id, { start_date: '2026-01-01' }); + const longSummary = 'A'.repeat(120); + buildTripCalendar.mockImplementation(() => calendarParts({ events: [vevent(longSummary)] })); + + const { ics, calName } = svc.buildUserIcs('tok-user')!; + + expect(calName).toBe(`${username} – All Trips`); + const preamble = ics.slice(0, ics.indexOf('BEGIN:VEVENT')); + expect(preamble).toContain(`X-WR-CALNAME:${calName}\r\n`); + expect(preamble.split('\r\n').filter((line) => line.startsWith(' '))).toEqual([]); + // The body is folded and unfolds back to the original content line. + expect(ics).toContain('\r\n '); + expect(ics.replace(/\r\n /g, '')).toContain(`SUMMARY:${longSummary}\r\n`); + }); + + it('FEED-SVC-020: the display name is escaped for the header but returned raw', () => { + // An unescaped ; or , ends the property value early, so the calendar shows up + // under a truncated name. The returned calName feeds the HTTP layer, not ICS, + // and must stay verbatim. + const user = seedUserWithToken('tok-user', { username: 'Alice; Bob, Co\\Ltd' }); + createTrip(testDb, user.id, { start_date: '2026-01-01' }); + + const { ics, calName } = svc.buildUserIcs('tok-user')!; + + expect(calName).toBe('Alice; Bob, Co\\Ltd – All Trips'); + expect(ics).toContain('X-WR-CALNAME:Alice\\; Bob\\, Co\\\\Ltd – All Trips\r\n'); + }); +}); + +describe('FeedsService wiring', () => { + it('FEED-SVC-021: the module registers the service and all three controllers', () => { + expectRegisteredProvider(FeedsModule, FeedsService); + expectRegisteredController(FeedsModule, FeedsPublicController); + expectRegisteredController(FeedsModule, TripFeedTokenController); + expectRegisteredController(FeedsModule, UserFeedTokenController); + }); +}); diff --git a/server/tests/unit/nest/trip-members.service.test.ts b/server/tests/unit/nest/trip-members.service.test.ts new file mode 100644 index 000000000..f0fbee744 --- /dev/null +++ b/server/tests/unit/nest/trip-members.service.test.ts @@ -0,0 +1,363 @@ +/** + * Unit tests for TripMembersService — MEMBERS-SVC-001 through MEMBERS-SVC-015. + * + * The roster cases that came over with the split still run in + * tests/unit/nest/trips.service.test.ts (TRIP-SVC-020…023, 030…034, 049, 052, + * 053) against this same service; they were left there so the diff shows the + * move rather than a rewrite. This file adds only what that set never reached: + * the five helpers the controller talks through, and the fallbacks each + * mutation takes when a row is missing or a name is unusable. Same in-memory + * SQLite harness, so the SQL is exercised for real. + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; + +// ── DB setup ────────────────────────────────────────────────────────────────── + +const { testDb, dbMock, broadcast, notifySend } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA busy_timeout = 5000'); + const mock = { + db, + closeDb: () => {}, + reinitialize: () => {}, + getPlaceWithTags: () => null, + canAccessTrip: (tripId: any, userId: number) => + db.prepare(` + SELECT t.id, t.user_id FROM trips t + LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? + WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL) + `).get(userId, tripId, userId), + isOwner: (tripId: any, userId: number) => + !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), + }; + return { + testDb: db, + dbMock: mock, + broadcast: vi.fn(), + notifySend: vi.fn().mockResolvedValue(undefined), + }; +}); + +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../src/config', () => ({ + JWT_SECRET: 'test-secret', + ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', + updateJwtSecret: () => {}, +})); +vi.mock('../../../src/websocket', () => ({ broadcast })); +// notifyInvite reaches the bridge through a dynamic import — keep the send in scope +// but out of the transports. +vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send: notifySend })); + +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { resetTestDb } from '../../helpers/test-db'; +import { createUser, createTrip, addTripMember } from '../../helpers/factories'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; +import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; +import { BudgetService } from '../../../src/nest/budget/budget.service'; +import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; +import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; +import { TripMembersService } from '../../../src/nest/trip-members/trip-members.service'; +import { NotFoundError, ValidationError } from '../../../src/nest/common/domain-errors'; +import type { User } from '../../../src/types'; + +const dbs = () => new DatabaseService(testDb); +const budgetSvc = new BudgetService(dbs(), new PermissionsService(dbs()), new ExchangeRatesService(), new RealtimeService()); +const roster = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs()), new PermissionsService(dbs()), new RealtimeService()); + +/** + * A roster whose connection reports "no such row" for the first result of every + * statement containing `match`. Both fallbacks it serves sit behind a FOREIGN + * KEY, so the missing row cannot be produced through the real schema — freezing + * the read is the only way to keep those branches honest. Same Proxy shape as + * the failingConnection helper in trips.service.test.ts. + */ +function rosterWithMissingRow(match: string) { + const conn = new Proxy(testDb, { + get(target, prop) { + if (prop === 'prepare') { + return (sql: string) => { + const stmt = target.prepare(sql); + if (!sql.includes(match)) return stmt; + return new Proxy(stmt, { + get(s, p) { + if (p === 'get') return () => undefined; + const v = (s as any)[p]; + return typeof v === 'function' ? v.bind(s) : v; + }, + }); + }; + } + const v = (target as any)[prop]; + return typeof v === 'function' ? v.bind(target) : v; + }, + }); + const frozen = { connection: conn, canAccessTrip: dbMock.canAccessTrip, isOwner: dbMock.isOwner } as unknown as DatabaseService; + return new TripMembersService(frozen, budgetSvc, new UserCleanupService(dbs()), new PermissionsService(dbs()), new RealtimeService()); +} + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + resetTestDb(testDb); + broadcast.mockClear(); + notifySend.mockClear(); +}); + +afterAll(() => { + testDb.close(); +}); + +// ── Delegating helpers (what the controller calls around the mutations) ─────── + +describe('TripMembersService delegation', () => { + it('MEMBERS-SVC-001: canAccessTrip forwards the db helper — owner and member get the access row, a stranger gets nothing', () => { + const { user: owner } = createUser(testDb); + const { user: member } = createUser(testDb); + const { user: stranger } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + addTripMember(testDb, trip.id, member.id); + + // Every member route answers 404 on a falsy result and reads access.user_id as + // the trip owner, so a delegation that stopped forwarding would lock the whole + // roster out rather than fail loudly. + expect(roster.canAccessTrip(String(trip.id), owner.id)).toMatchObject({ user_id: owner.id }); + expect(roster.canAccessTrip(trip.id, member.id)).toMatchObject({ user_id: owner.id }); + expect(roster.canAccessTrip(trip.id, stranger.id)).toBeUndefined(); + }); + + it('MEMBERS-SVC-002: can() resolves member_manage at its trip_owner default and lets admins through', () => { + const { user: owner } = createUser(testDb); + const { user: member } = createUser(testDb); + + // Inviting and evicting both hang off this single call, so a member must not + // pass it while the action sits at its default level. + expect(roster.can('member_manage', 'user', owner.id, owner.id, false)).toBe(true); + expect(roster.can('member_manage', 'user', owner.id, member.id, true)).toBe(false); + expect(roster.can('member_manage', 'admin', owner.id, member.id, true)).toBe(true); + }); + + it('MEMBERS-SVC-003: broadcast forwards the socket id so the originating client is not echoed', () => { + roster.broadcast('9', 'trip:updated', { trip: { id: 9 } } as never, 'sock-1'); + expect(broadcast).toHaveBeenCalledWith('9', 'trip:updated', { trip: { id: 9 } }, 'sock-1'); + + // A request without X-Socket-Id must still reach everybody — dropping the + // argument here would exclude an arbitrary socket instead of none. + roster.broadcast('9', 'trip:updated', { trip: { id: 9 } } as never, undefined); + expect(broadcast).toHaveBeenLastCalledWith('9', 'trip:updated', { trip: { id: 9 } }, undefined); + }); + + it('MEMBERS-SVC-004: getTripForViewer re-reads the trip in list shape, with is_owner per viewer', () => { + const { user: owner } = createUser(testDb); + const { user: member } = createUser(testDb); + const trip = createTrip(testDb, owner.id, { title: 'Handover' }); + addTripMember(testDb, trip.id, member.id); + + // The handover broadcast hands the raw :id route param straight in, so the + // named-parameter query has to keep matching a string id against the INTEGER + // column — and the payload the clients re-read must carry their own is_owner. + const asOwner = roster.getTripForViewer(String(trip.id), owner.id) as Record; + expect(asOwner).toMatchObject({ id: trip.id, title: 'Handover', is_owner: 1, owner_username: owner.username, shared_count: 1 }); + const asMember = roster.getTripForViewer(trip.id, member.id) as Record; + expect(asMember.is_owner).toBe(0); + expect(roster.getTripForViewer(999999, owner.id)).toBeUndefined(); + }); + + it('MEMBERS-SVC-005: notifyInvite sends a user-scoped trip_invite carrying the actor, invitee and trip', async () => { + const actor = { id: 4, email: 'actor@example.test' } as User; + roster.notifyInvite('9', actor, 12, 'Roadtrip', 'invitee@example.test'); + + // The bridge is imported lazily inside the call, so the send lands a microtask later. + await vi.waitFor(() => expect(notifySend).toHaveBeenCalledTimes(1)); + expect(notifySend).toHaveBeenCalledWith({ + event: 'trip_invite', + actorId: 4, + scope: 'user', + targetId: 12, + params: { trip: 'Roadtrip', actor: 'actor@example.test', invitee: 'invitee@example.test', tripId: '9' }, + }); + }); + + it('MEMBERS-SVC-006: a rejected notification never reaches the invite request', async () => { + notifySend.mockRejectedValueOnce(new Error('bridge down')); + const actor = { id: 4, email: 'actor@example.test' } as User; + + // The membership row is already committed when this fires — an unhandled + // rejection would turn a successful invite into a 500 (and crash the process + // on an unhandled promise rejection). + expect(() => roster.notifyInvite('9', actor, 12, 'Roadtrip', 'invitee@example.test')).not.toThrow(); + await vi.waitFor(() => expect(notifySend).toHaveBeenCalledTimes(1)); + }); +}); + +// ── addMember fallbacks ────────────────────────────────────────────────────── + +describe('addMember fallbacks', () => { + it("MEMBERS-SVC-007: addMember still reports a title when the trip row cannot be read ('Untitled')", () => { + const { user: owner } = createUser(testDb); + const { user: invitee } = createUser(testDb); + const trip = createTrip(testDb, owner.id, { title: 'Readable' }); + + // The title only feeds the invite notification, and it is read after the + // membership is inserted — losing that row must not cost the invitee their + // access or throw on an undefined title. + const broken = rosterWithMissingRow('SELECT title FROM trips WHERE id = ?'); + const result = broken.addMember(trip.id, invitee.email, owner.id, owner.id); + expect(result.tripTitle).toBe('Untitled'); + expect(testDb.prepare('SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, invitee.id)).toBeDefined(); + }); + + it('MEMBERS-SVC-008: addMember resolves a padded identifier and matches on username as well as email', () => { + const { user: owner } = createUser(testDb); + const { user: invitee } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + + // Pasted invites arrive with surrounding whitespace; without the trim the + // lookup misses and the box answers 'User not found' for a real account. + expect(roster.addMember(trip.id, ` ${invitee.username} `, owner.id, owner.id).member.id).toBe(invitee.id); + }); +}); + +// ── transferOwnership guard rails (#973) ───────────────────────────────────── + +describe('transferOwnership guard rails', () => { + it('MEMBERS-SVC-009: rejects a trip that no longer exists, before any other check', () => { + const { user: owner } = createUser(testDb); + + // NotFoundError, not ValidationError: the controller maps the two to 404 and + // 400. Passing the owner as the new owner too proves the trip lookup runs + // first — otherwise this would surface as 'You already own this trip'. + expect(() => roster.transferOwnership(999999, owner.id, owner.id)).toThrow(NotFoundError); + expect(() => roster.transferOwnership(999999, owner.id, owner.id)).toThrow('Trip not found'); + }); + + it('MEMBERS-SVC-010: rejects an id with no user row before it checks membership', () => { + const { user: owner } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + + // A stale id from the client must read as a 404 'User not found' rather than + // the 400 the non-member branch below it would produce, and the trip must keep + // its owner either way. + expect(() => roster.transferOwnership(trip.id, 999999, owner.id)).toThrow(NotFoundError); + expect(() => roster.transferOwnership(trip.id, 999999, owner.id)).toThrow('User not found'); + expect((testDb.prepare('SELECT user_id FROM trips WHERE id = ?').get(trip.id) as { user_id: number }).user_id).toBe(owner.id); + }); + + it('MEMBERS-SVC-011: completes with an empty fromEmail when the former owner cannot be read', () => { + const { user: owner } = createUser(testDb); + const { user: member } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + addTripMember(testDb, trip.id, member.id); + + // fromEmail is audit detail only; an unreadable row must not abort the + // handover halfway, which would leave the owner pointer and the member rows + // disagreeing about who owns the trip. + const broken = rosterWithMissingRow('SELECT email FROM users WHERE id = ?'); + const result = broken.transferOwnership(trip.id, member.id, owner.id); + expect(result.fromEmail).toBe(''); + expect(result.toEmail).toBe(member.email); + expect((testDb.prepare('SELECT user_id FROM trips WHERE id = ?').get(trip.id) as { user_id: number }).user_id).toBe(member.id); + }); +}); + +// ── Guest name validation (#1362) ──────────────────────────────────────────── + +describe('guest name validation', () => { + it('MEMBERS-SVC-012: createGuest rejects an absent, blank or over-long name and writes nothing', () => { + const { user: owner } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + + expect(() => roster.createGuest(trip.id, undefined as never, owner.id)).toThrow(ValidationError); + expect(() => roster.createGuest(trip.id, ' ', owner.id)).toThrow('Guest name is required'); + expect(() => roster.createGuest(trip.id, 'x'.repeat(51), owner.id)).toThrow('Guest name must be 50 characters or fewer'); + + // The guards run ahead of the transaction, so a rejected name can never leave + // a credential-less users row behind with no trip to belong to. + expect((testDb.prepare('SELECT COUNT(*) AS n FROM users WHERE is_guest = 1').get() as { n: number }).n).toBe(0); + + // 50 is the accepted boundary the DTO shares — off by one here and the API + // starts refusing names the client believes are valid. + expect(roster.createGuest(trip.id, 'x'.repeat(50), owner.id).member.username).toHaveLength(50); + }); + + it('MEMBERS-SVC-013: renameGuest rejects an absent, blank or over-long name — before the trip-scope check', () => { + const { user: owner } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + const { member: guest } = roster.createGuest(trip.id, 'Ida', owner.id); + + expect(() => roster.renameGuest(trip.id, guest.id, undefined as never)).toThrow(ValidationError); + expect(() => roster.renameGuest(trip.id, guest.id, ' ')).toThrow('Guest name is required'); + expect(() => roster.renameGuest(trip.id, guest.id, 'x'.repeat(51))).toThrow('Guest name must be 50 characters or fewer'); + + // Order matters for the status code: an unusable name throws (400) even for an + // id that is not a guest of this trip, where the scope check returns false (404). + expect(() => roster.renameGuest(trip.id, owner.id, '')).toThrow('Guest name is required'); + expect((testDb.prepare('SELECT display_name FROM users WHERE id = ?').get(guest.id) as { display_name: string }).display_name).toBe('Ida'); + + // A padded name is stored trimmed, so the roster does not render the spaces. + expect(roster.renameGuest(trip.id, guest.id, ' Ida M. ')).toBe(true); + expect((testDb.prepare('SELECT display_name FROM users WHERE id = ?').get(guest.id) as { display_name: string }).display_name).toBe('Ida M.'); + }); + + it("MEMBERS-SVC-014: deleteGuest is trip-scoped — another trip's owner cannot erase this trip's guest", () => { + const { user: owner } = createUser(testDb); + const { user: other } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + const otherTrip = createTrip(testDb, other.id); + const { member: guest } = roster.createGuest(trip.id, 'Jo', owner.id); + + // The route only proves ownership of the trip in the URL, so this check is the + // only thing between it and a foreign guest's users row — and the delete + // cascades every assignment that guest is on. + expect(roster.deleteGuest(otherTrip.id, guest.id)).toBe(false); + expect(testDb.prepare('SELECT id FROM users WHERE id = ?').get(guest.id)).toBeDefined(); + }); +}); + +// ── listMembers shaping ────────────────────────────────────────────────────── + +describe('listMembers shaping', () => { + it('MEMBERS-SVC-015: avatar_url follows the storage form and the inviter is named, not just its id', () => { + const { user: owner } = createUser(testDb); + const { user: uploaded } = createUser(testDb); + const { user: sso } = createUser(testDb); + const { user: bare } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + testDb.prepare('UPDATE users SET avatar = ?, display_name = ? WHERE id = ?').run('me.png', 'Owner Displayed', owner.id); + testDb.prepare('UPDATE users SET avatar = ? WHERE id = ?').run('a.png', uploaded.id); + testDb.prepare('UPDATE users SET avatar = ? WHERE id = ?').run('https://idp.example.test/p.jpg', sso.id); + testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(trip.id, uploaded.id, owner.id); + testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(trip.id, sso.id, owner.id); + addTripMember(testDb, trip.id, bare.id); + + const { owner: ownerRow, members } = roster.listMembers(trip.id, owner.id); + // added_at has second resolution, so rows created in one test are tied — index + // by id rather than asserting the ORDER BY. + const byId = new Map(members.map(m => [m.id, m])); + + // An uploaded file name becomes a /uploads/avatars path; an OIDC picture claim + // (#1399) is an absolute URL and must pass through untouched; a member without + // one must stay null instead of rendering '/uploads/avatars/null'. + expect(ownerRow.avatar_url).toBe('/uploads/avatars/me.png'); + expect(byId.get(uploaded.id)!.avatar_url).toBe('/uploads/avatars/a.png'); + expect(byId.get(sso.id)!.avatar_url).toBe('https://idp.example.test/p.jpg'); + expect(byId.get(bare.id)!.avatar_url).toBeNull(); + + // The inviter is resolved through a LEFT JOIN that prefers display_name; a row + // with no inviter reports null rather than dropping out of the list. + expect(byId.get(uploaded.id)!.invited_by_username).toBe('Owner Displayed'); + expect(byId.get(bare.id)!.invited_by_username).toBeNull(); + + // is_guest is a SQLite integer on the way out and a boolean on the wire. + expect(byId.get(bare.id)!.is_guest).toBe(false); + expect(ownerRow.is_guest).toBe(false); + }); +}); diff --git a/server/tests/unit/nest/trip-read-model.service.test.ts b/server/tests/unit/nest/trip-read-model.service.test.ts new file mode 100644 index 000000000..b9af5a8e5 --- /dev/null +++ b/server/tests/unit/nest/trip-read-model.service.test.ts @@ -0,0 +1,261 @@ +/** + * Unit tests for the DI-native TripReadModelService — TRIP-READ-001..006. + * The two read aggregates (MCP summary + offline bundle) moved here when the + * trips aggregate root was split; the happy-path aggregation cases stayed in + * tests/unit/nest/trips.service.test.ts so that diff reads as a move. This file + * pins the guards around them: the early returns, the falsy-price fold, the + * checked tally and the viewer scoping that keeps other members' private + * packing items (#858) out of both aggregates. + * Uses a real in-memory SQLite DB so SQL logic is exercised faithfully. + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; + +// ── DB setup ────────────────────────────────────────────────────────────────── + +const { testDb, dbMock } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA busy_timeout = 5000'); + const mock = { + db, + closeDb: () => {}, + reinitialize: () => {}, + getPlaceWithTags: () => null, + canAccessTrip: (tripId: any, userId: number) => + db.prepare(` + SELECT t.id, t.user_id FROM trips t + LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? + WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL) + `).get(userId, tripId, userId), + isOwner: (tripId: any, userId: number) => + !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), + }; + return { testDb: db, dbMock: mock }; +}); + +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../src/config', () => ({ + JWT_SECRET: 'test-secret', + ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', + updateJwtSecret: () => {}, +})); +const { broadcast } = vi.hoisted(() => ({ broadcast: vi.fn() })); +vi.mock('../../../src/websocket', () => ({ broadcast })); + +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { resetTestDb } from '../../helpers/test-db'; +import { createUser, createTrip, addTripMember } from '../../helpers/factories'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { DaysService } from '../../../src/nest/days/days.service'; +import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; +import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; +import { TodoService } from '../../../src/nest/todo/todo.service'; +import { PackingService } from '../../../src/nest/packing/packing.service'; +import { FilesService } from '../../../src/nest/files/files.service'; +import { ReservationsService } from '../../../src/nest/reservations/reservations.service'; +import { BudgetService } from '../../../src/nest/budget/budget.service'; +import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; +import { CollabService } from '../../../src/nest/collab/collab.service'; +import { PlacesService } from '../../../src/nest/places/places.service'; +import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; +import { TripMembersService } from '../../../src/nest/trip-members/trip-members.service'; +import { TripReadModelService } from '../../../src/nest/trip-read-model/trip-read-model.service'; +import { AccommodationsService } from '../../../src/nest/accommodations/accommodations.service'; +import { MapsService } from '../../../src/nest/maps/maps.service'; +import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; + +// Real sibling services over the same in-memory DB — the aggregation runs the +// actual SQL of every domain it fans out to, so a shape change downstream shows +// up here instead of being papered over by a stub. +const dbs = () => new DatabaseService(testDb); +const budgetSvc = new BudgetService(dbs(), new PermissionsService(dbs()), new ExchangeRatesService(), new RealtimeService()); +const daysSvc = new DaysService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new QueryHelpersService(dbs())); +const placesSvc = new PlacesService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new MapsService(dbs()), new QueryHelpersService(dbs())); +const accommodationsSvc = new AccommodationsService(dbs(), new PermissionsService(dbs()), new RealtimeService()); +const membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs()), new PermissionsService(dbs()), new RealtimeService()); + +const buildReadModel = (database: DatabaseService, roster: TripMembersService = membersSvc) => + new TripReadModelService( + database, roster, daysSvc, accommodationsSvc, budgetSvc, + new PackingService(dbs(), new PermissionsService(dbs()), new RealtimeService()), + new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService()), + new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService()), + placesSvc, + new TodoService(dbs(), new PermissionsService(dbs()), new RealtimeService()), + new FilesService(dbs(), new PermissionsService(dbs()), new RealtimeService()), + ); + +const svc = buildReadModel(dbs()); + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + resetTestDb(testDb); +}); + +afterAll(() => { + testDb.close(); +}); + +// ── Helpers ─────────────────────────────────────────────────────────────────── + +const addBudgetItem = (tripId: number, name: string, totalPrice: number) => + testDb.prepare("INSERT INTO budget_items (trip_id, category, name, total_price) VALUES (?, 'food', ?, ?)") + .run(tripId, name, totalPrice); + +const addPackingItem = (tripId: number, name: string, checked: number) => + testDb.prepare('INSERT INTO packing_items (trip_id, name, checked) VALUES (?, ?, ?)') + .run(tripId, name, checked); + +/** + * A DatabaseService whose connection cannot resolve the owner lookup, standing in + * for the trip row disappearing between the two SELECTs getTripSummary issues. + * Only the read model's own two statements go through this connection; the + * sibling services keep the real one. + */ +function ownerlessDbs(): DatabaseService { + const conn = new Proxy(testDb, { + get(target, prop) { + if (prop === 'prepare') { + return (sql: string) => + (sql.includes('SELECT user_id FROM trips') ? { get: () => undefined } : target.prepare(sql)); + } + const v = (target as any)[prop]; + return typeof v === 'function' ? v.bind(target) : v; + }, + }); + return { connection: conn } as unknown as DatabaseService; +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +describe('getTripSummary guards', () => { + it('TRIP-READ-001: returns null for a missing trip instead of throwing', () => { + // The MCP get_trip_summary tool hands over whatever id the model produced, so + // an unknown id has to come back as an empty answer; a throw there surfaces as + // a tool error rather than "no such trip". + expect(svc.getTripSummary(99999)).toBeNull(); + expect(svc.getTripSummary(99999, 1)).toBeNull(); + }); + + it('TRIP-READ-002: returns null when the owner row cannot be read', () => { + const { user: owner } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + + // Trip row and owner id are two separate SELECTs; if the second one comes back + // empty (trip deleted in between) the guard has to stop. Without it listMembers + // runs with an undefined owner id and the summary reports an ownerless trip. + expect(buildReadModel(ownerlessDbs()).getTripSummary(trip.id, owner.id)).toBeNull(); + + // Same trip through the real connection still aggregates — the null above is + // the missing owner row, not a broken fixture. + expect(svc.getTripSummary(trip.id, owner.id)).not.toBeNull(); + }); +}); + +describe('getTripSummary shaping', () => { + it('TRIP-READ-003: folds a falsy total_price into the budget total instead of poisoning it', () => { + const { user: owner } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + addBudgetItem(trip.id, 'Dinner', 40); + // total_price is NOT NULL DEFAULT 0, so 0 is the reachable falsy price: a free + // entry someone logged to keep it on the list. The `|| 0` in the reduce is what + // keeps any such value out of the sum — drop it and a price that arrives + // non-numeric turns the whole trip total into NaN, which the MCP summary and + // the offline clients both render as an empty budget. + addBudgetItem(trip.id, 'Free walking tour', 0); + + const summary = svc.getTripSummary(trip.id, owner.id)!; + expect(summary.budget.item_count).toBe(2); + expect(summary.budget.total).toBe(40); + expect(summary.budget.currency).toBe('EUR'); + }); + + it('TRIP-READ-004: counts only checked packing items, not the whole list', () => { + const { user: owner } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + addPackingItem(trip.id, 'Socks', 1); + addPackingItem(trip.id, 'Charger', 1); + addPackingItem(trip.id, 'Passport', 0); + + // total and checked come from the same array; if the filter is ever widened the + // packing progress the summary reports jumps to 100% while items are still open. + const summary = svc.getTripSummary(trip.id, owner.id)!; + expect(summary.packing.total).toBe(3); + expect(summary.packing.checked).toBe(2); + }); +}); + +describe('bundle shaping', () => { + it('TRIP-READ-005: keeps the member list a flat array when the roster has no members', () => { + const { user: owner } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + + // The bundle spreads the roster's members into the array it ships offline. The + // `|| []` is the guard for a roster shape without that array — without it the + // spread throws and the whole offline bundle fails, taking days, places and + // reservations down with it over a trip that simply has no collaborators. + const roster = vi.spyOn(membersSvc, 'listMembers').mockReturnValue({ + owner: { id: owner.id, username: 'solo' }, members: undefined, + } as never); + try { + const result = svc.bundle(String(trip.id), { user_id: owner.id }, owner.id) as any; + expect(result.members).toEqual([{ id: owner.id, username: 'solo' }]); + } finally { + roster.mockRestore(); + } + }); + + it('TRIP-READ-006: drops a falsy owner rather than shipping a hole in the member list', () => { + const { user: owner } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + + // filter(Boolean) exists because the owner lookup can come back empty (a user + // row deleted while the trip lingers). Clients index the member list to render + // avatars, so an undefined slot in it crashes the offline view. + const roster = vi.spyOn(membersSvc, 'listMembers').mockReturnValue({ + owner: undefined, members: [{ id: 42, username: 'left-behind' }], + } as never); + try { + const result = svc.bundle(String(trip.id), { user_id: owner.id }, owner.id) as any; + expect(result.members).toEqual([{ id: 42, username: 'left-behind' }]); + } finally { + roster.mockRestore(); + } + }); +}); + +describe('private packing items stay viewer-scoped (#858)', () => { + it("TRIP-READ-007: neither summary nor bundle leaks another member's private item", () => { + const { user: owner } = createUser(testDb); + const { user: viewer } = createUser(testDb); + const trip = createTrip(testDb, owner.id, { start_date: '2025-06-01', end_date: '2025-06-02' }); + addTripMember(testDb, trip.id, viewer.id); + testDb.prepare("INSERT INTO packing_items (trip_id, name, is_private, owner_id) VALUES (?, 'Ring', 1, ?)") + .run(trip.id, owner.id); + testDb.prepare("INSERT INTO packing_items (trip_id, name) VALUES (?, 'Tent')").run(trip.id); + + // Both read paths pass the viewer into packing.listItems; that argument is the + // ONLY thing filtering the list. If either call site loses it, listItems falls + // back to the unfiltered query and the surprise the owner is carrying shows up + // in the other member's MCP summary and in their offline cache. + const asViewer = svc.getTripSummary(trip.id, viewer.id)!; + expect(asViewer.packing.items.map((i: any) => i.name)).toEqual(['Tent']); + expect(asViewer.packing.total).toBe(1); + + const bundled = svc.bundle(String(trip.id), { user_id: owner.id }, viewer.id) as any; + expect(bundled.packingItems.map((i: any) => i.name)).toEqual(['Tent']); + + // The owner still sees their own private item through both paths, so the + // assertions above are the filter working, not an empty fixture. + expect(svc.getTripSummary(trip.id, owner.id)!.packing.items.map((i: any) => i.name)).toEqual(['Ring', 'Tent']); + expect((svc.bundle(String(trip.id), { user_id: owner.id }, owner.id) as any) + .packingItems.map((i: any) => i.name)).toEqual(['Ring', 'Tent']); + }); +}); diff --git a/server/tests/unit/nest/trips.controller.test.ts b/server/tests/unit/nest/trips.controller.test.ts index 338c0d3a3..08dc9dd41 100644 --- a/server/tests/unit/nest/trips.controller.test.ts +++ b/server/tests/unit/nest/trips.controller.test.ts @@ -2,6 +2,9 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import type { UnsplashService } from '../../../src/nest/unsplash/unsplash.service'; import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { HttpException } from '@nestjs/common'; +import { INTERCEPTORS_METADATA } from '@nestjs/common/constants'; +import fs from 'fs'; +import path from 'path'; import type { Request } from 'express'; vi.mock('../../../src/nest/audit/client-ip', () => ({ getClientIp: vi.fn(() => '1.2.3.4') })); @@ -102,6 +105,41 @@ describe('TripsController (parity with the legacy /api/trips route)', () => { }); }); + describe('GET /cover-images/search', () => { + it('passes the caller through to the service and unwraps the photo list', async () => { + const searchCoverImages = vi.fn().mockResolvedValue({ photos: [{ id: 'p1' }] }); + const s = svc({ searchCoverImages } as Partial); + expect(await tc(s).coverImages(user, 'kyoto')).toEqual({ photos: [{ id: 'p1' }] }); + // The Unsplash key is resolved per user behind the service; dropping the id + // here would silently search every request with an admin's key. + expect(searchCoverImages).toHaveBeenCalledWith('kyoto', 1); + }); + + it('sends an absent query as "" so the service keeps owning the empty-query 400', async () => { + const searchCoverImages = vi.fn().mockResolvedValue({ error: 'Search query is required', status: 400 }); + const s = svc({ searchCoverImages } as Partial); + expect(await thrownAsync(() => tc(s).coverImages(user, undefined))).toEqual({ status: 400, body: { error: 'Search query is required' } }); + expect(searchCoverImages).toHaveBeenCalledWith('', 1); + }); + + it('keeps the upstream status instead of flattening it into the catch-all 500', async () => { + const s = svc({ searchCoverImages: vi.fn().mockResolvedValue({ error: 'Rate limit reached', status: 429 }) } as Partial); + // A 429 that arrives as a 500 makes the client retry immediately and burn + // the rest of the hourly Unsplash quota. + expect(await thrownAsync(() => tc(s).coverImages(user, 'kyoto'))).toEqual({ status: 429, body: { error: 'Rate limit reached' } }); + }); + + it('maps an unexpected failure to a 500 that does not leak the cause', async () => { + const err = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + const s = svc({ searchCoverImages: vi.fn().mockRejectedValue(new Error('ECONNREFUSED api.unsplash.com')) } as Partial); + expect(await thrownAsync(() => tc(s).coverImages(user, 'kyoto'))).toEqual({ status: 500, body: { error: 'Error searching for cover images' } }); + } finally { + err.mockRestore(); + } + }); + }); + describe('POST / (create)', () => { it('403 without trip_create; a missing title 400s in the ZodValidationPipe', () => { expect(thrown(() => tc(svc({ can: vi.fn().mockReturnValue(false) })).create(user, { title: 'T' }, req))).toEqual({ status: 403, body: { error: 'No permission to create trips' } }); @@ -285,6 +323,84 @@ describe('TripsController (parity with the legacy /api/trips route)', () => { else process.env.DEMO_MODE = prev; } }); + + /** + * The multer options are a module-private const wired into the route through + * @UseInterceptors, so the handler tests above never reach them: a broken + * destination/filename/filter would still let every case in this file pass. + * Pulling the interceptor back off the route and reading the multer instance + * it built (multer keeps storage, limits and fileFilter verbatim) exercises + * the real callbacks instead of asserting an object literal. + */ + type StorageCb = (err: Error | null, value: string) => void; + type CoverUpload = { + storage: { + getDestination: (req: unknown, file: Express.Multer.File, cb: StorageCb) => void; + getFilename: (req: unknown, file: Express.Multer.File, cb: StorageCb) => void; + }; + limits: { fileSize: number }; + fileFilter: (req: unknown, file: Express.Multer.File, cb: (err: Error | null, accept: boolean) => void) => void; + }; + function coverUpload(): CoverUpload { + const [Interceptor] = Reflect.getMetadata(INTERCEPTORS_METADATA, TripsController.prototype.cover) as Array { multer: CoverUpload }>; + return new Interceptor().multer; + } + const coversDir = path.join('uploads', 'covers'); + + it('creates uploads/covers on the first upload and reuses it afterwards', () => { + // A fresh container has no uploads/covers, and multer does not create the + // destination itself, so without the mkdir the very first cover upload on + // a new instance fails with ENOENT. + const exists = vi.spyOn(fs, 'existsSync').mockReturnValue(false); + const mkdir = vi.spyOn(fs, 'mkdirSync').mockImplementation(() => undefined); + try { + const { getDestination } = coverUpload().storage; + const cb = vi.fn(); + getDestination(req, file, cb); + expect(mkdir).toHaveBeenCalledWith(expect.stringContaining(coversDir), { recursive: true }); + expect(cb).toHaveBeenCalledWith(null, expect.stringContaining(coversDir)); + exists.mockReturnValue(true); + mkdir.mockClear(); + getDestination(req, file, cb); + expect(mkdir).not.toHaveBeenCalled(); + expect(cb).toHaveBeenLastCalledWith(null, expect.stringContaining(coversDir)); + } finally { + exists.mockRestore(); + mkdir.mockRestore(); + } + }); + + it('gives every stored cover a fresh name and keeps the original extension', () => { + const { getFilename } = coverUpload().storage; + const cb = vi.fn(); + const upload = { originalname: 'sunset.JPG' } as Express.Multer.File; + getFilename(req, upload, cb); + getFilename(req, upload, cb); + const [first, second] = cb.mock.calls.map((c) => c[1] as string); + // Keeping the client's name would let one trip's upload overwrite another's; + // dropping the extension leaves the static handler with no mime to serve. + expect(first).toMatch(/^[0-9a-f-]{36}\.JPG$/); + expect(second).not.toBe(first); + }); + + it('accepts the four allowed image types and rejects svg, a faked extension and a non-image mime', () => { + const { fileFilter, limits } = coverUpload(); + const verdict = (originalname: string, mimetype: string) => { + const cb = vi.fn(); + fileFilter(req, { originalname, mimetype } as Express.Multer.File, cb); + return cb.mock.calls[0]; + }; + expect(verdict('a.jpg', 'image/jpeg')).toEqual([null, true]); + // The extension check is case-insensitive; phones hand up .JPG/.PNG. + expect(verdict('a.WEBP', 'image/webp')).toEqual([null, true]); + // svg passes the image/* prefix but runs script when the browser opens the + // stored cover, so it is rejected even under an allowed extension. + expect(verdict('logo.png', 'image/svg+xml')).toEqual([expect.any(Error), false]); + // A binary renamed to an image mime must still fail on the extension. + expect(verdict('payload.exe', 'image/png')).toEqual([expect.any(Error), false]); + expect(verdict('a.png', 'application/octet-stream')).toEqual([expect.any(Error), false]); + expect(limits.fileSize).toBe(20 * 1024 * 1024); + }); }); describe('GET /:id/export.ics', () => { diff --git a/server/vitest.config.ts b/server/vitest.config.ts index f01618522..6131fc524 100644 --- a/server/vitest.config.ts +++ b/server/vitest.config.ts @@ -61,7 +61,7 @@ export default defineConfig({ // A file matched by a glob below is EXCLUDED from the catch-all, so the last entry // only covers what sits directly under src/nest (app.module.ts and the like). thresholds: { - 'src/nest/accommodations/**/*.ts': { statements: 92, branches: 82, functions: 98, lines: 97 }, + 'src/nest/accommodations/**/*.ts': { statements: 92, branches: 87, functions: 99, lines: 97 }, 'src/nest/addons/**/*.ts': { statements: 99, branches: 87, functions: 99, lines: 99 }, 'src/nest/admin/**/*.ts': { statements: 86, branches: 70, functions: 89, lines: 88 }, 'src/nest/airports/**/*.ts': { statements: 68, branches: 60, functions: 93, lines: 73 }, @@ -73,14 +73,14 @@ export default defineConfig({ 'src/nest/backup/**/*.ts': { statements: 96, branches: 92, functions: 99, lines: 98 }, 'src/nest/booking-import/**/*.ts': { statements: 48, branches: 28, functions: 56, lines: 52 }, 'src/nest/budget/**/*.ts': { statements: 91, branches: 77, functions: 97, lines: 95 }, - 'src/nest/calendar/**/*.ts': { statements: 92, branches: 82, functions: 91, lines: 94 }, + 'src/nest/calendar/**/*.ts': { statements: 96, branches: 93, functions: 99, lines: 99 }, 'src/nest/categories/**/*.ts': { statements: 99, branches: 80, functions: 99, lines: 99 }, 'src/nest/collab/**/*.ts': { statements: 92, branches: 85, functions: 94, lines: 97 }, 'src/nest/collections/**/*.ts': { statements: 83, branches: 70, functions: 91, lines: 92 }, 'src/nest/common/**/*.ts': { statements: 95, branches: 89, functions: 99, lines: 97 }, 'src/nest/config/**/*.ts': { statements: 99, branches: 100, functions: 99, lines: 99 }, 'src/nest/database/**/*.ts': { statements: 99, branches: 100, functions: 99, lines: 99 }, - 'src/nest/day-notes/**/*.ts': { statements: 92, branches: 82, functions: 98, lines: 97 }, + 'src/nest/day-notes/**/*.ts': { statements: 92, branches: 83, functions: 99, lines: 99 }, 'src/nest/days/**/*.ts': { statements: 92, branches: 82, functions: 98, lines: 97 }, 'src/nest/feeds/**/*.ts': { statements: 91, branches: 83, functions: 83, lines: 91 }, 'src/nest/files/**/*.ts': { statements: 97, branches: 95, functions: 99, lines: 98 }, @@ -112,8 +112,8 @@ export default defineConfig({ 'src/nest/transit/**/*.ts': { statements: 92, branches: 83, functions: 97, lines: 94 }, 'src/nest/trip-invite/**/*.ts': { statements: 91, branches: 93, functions: 93, lines: 89 }, 'src/nest/trip-membership/**/*.ts': { statements: 99, branches: 86, functions: 99, lines: 99 }, - 'src/nest/trip-members/**/*.ts': { statements: 92, branches: 82, functions: 91, lines: 94 }, - 'src/nest/trip-read-model/**/*.ts': { statements: 92, branches: 82, functions: 91, lines: 94 }, + 'src/nest/trip-members/**/*.ts': { statements: 93, branches: 89, functions: 99, lines: 94 }, + 'src/nest/trip-read-model/**/*.ts': { statements: 97, branches: 96, functions: 99, lines: 97 }, 'src/nest/trips/**/*.ts': { statements: 92, branches: 82, functions: 91, lines: 94 }, 'src/nest/unsplash/**/*.ts': { statements: 97, branches: 82, functions: 99, lines: 96 }, 'src/nest/vacay/**/*.ts': { statements: 82, branches: 66, functions: 89, lines: 86 },