diff --git a/client/src/utils/dayMerge.test.ts b/client/src/utils/dayMerge.test.ts index 8608badd6..935bb2c57 100644 --- a/client/src/utils/dayMerge.test.ts +++ b/client/src/utils/dayMerge.test.ts @@ -276,4 +276,30 @@ describe('getMergedItems', () => { const types = result.map(i => i.type) expect(types).toEqual(['place', 'transport', 'place']) }) + + // The same rule the server stores when a start time is saved, so on a day of places + // alone the day it sends back after the save and the day drawn before it agree. + it('keeps untimed places where they were put and sorts only the timed ones', () => { + const place = (id: number, order_index: number, place_time: string | null) => ({ id, order_index, place: { place_time } }) + const ids = (dayAssignments: ReturnType[]) => + getMergedItems({ dayAssignments, dayNotes: [], dayTransports: [], dayId: 5 }).map(i => i.data.id) + + expect(ids([place(1, 0, null), place(2, 1, null), place(3, 2, '14:00')])).toEqual([1, 2, 3]) + expect(ids([place(1, 0, '09:00'), place(2, 1, null), place(3, 2, '14:00')])).toEqual([1, 2, 3]) + expect(ids([place(1, 0, null), place(2, 1, '15:00'), place(3, 2, '10:00')])).toEqual([1, 3, 2]) + }) + + // Where the server's sort and this one part: the server sorts the stops alone, so + // for it place 2 follows the 09:00 place and stays in front of the 10:00 one. Here it + // follows the 12:00 note and is drawn behind both. + it('lets an untimed place take the time of a timed note in front of it', () => { + const dayAssignments = [ + { id: 1, order_index: 0, place: { place_time: '09:00' } }, + { id: 2, order_index: 1, place: { place_time: null } }, + { id: 3, order_index: 2, place: { place_time: '10:00' } }, + ] + const dayNotes = [{ id: 10, sort_order: 0.5, time: '12:00' }] + const result = getMergedItems({ dayAssignments, dayNotes, dayTransports: [], dayId: 5 }) + expect(result.map(i => i.data.id)).toEqual([1, 3, 10, 2]) + }) }) diff --git a/client/src/utils/dayMerge.ts b/client/src/utils/dayMerge.ts index 430ad054b..70395498c 100644 --- a/client/src/utils/dayMerge.ts +++ b/client/src/utils/dayMerge.ts @@ -1,3 +1,4 @@ +import { chronoOrder } from '@trek/shared' // `orderedEndpoints` is the geometry order's single source of truth, in the module // that documents the multi-leg model. Sorting endpoints a second time here is how // the two drift. @@ -270,26 +271,22 @@ export function getAssignmentReservations string | null ): MergedItem[] { - const timeOf = (it: MergedItem): number | null => { + return chronoOrder(items, it => { if (it.type === 'place') return parseTimeToMinutes(it.data?.place?.place_time) if (it.type === 'note') return parseTimeToMinutes(it.data?.time) return parseTimeToMinutes(getDisplayTime(it.data, dayId)) - } - let last = -Infinity - return items - .map((it, i) => { - const t = timeOf(it) - if (t != null) last = t - return { it, i, eff: t != null ? t : last } - }) - .sort((a, b) => a.eff - b.eff || a.i - b.i) - .map(k => k.it) + }) } /** Merge places, notes, and transports into a single ordered day timeline. */ diff --git a/server/src/nest/assignments/assignments.controller.ts b/server/src/nest/assignments/assignments.controller.ts index 3d5908cb9..7d32e03c0 100644 --- a/server/src/nest/assignments/assignments.controller.ts +++ b/server/src/nest/assignments/assignments.controller.ts @@ -167,8 +167,17 @@ export class AssignmentOpsController { if (!this.assignments.getAssignmentForTrip(id, tripId)) { throw new HttpException({ error: 'Assignment not found' }, 404); } - const assignment = this.assignments.updateTime(id, body.place_time, body.end_time); + const { assignment, reordered, vias } = this.assignments.updateTime(id, body.place_time, body.end_time); this.assignments.broadcast(tripId, 'assignment:updated', { assignment }, socketId); + // The whole day when a start moved stops, or collaborators apply the one row + // they were sent to their old order and end up with a third one. + // + // Both to every socket, the writer's included, so the order and the vias pinned + // to it arrive together. The planner holds its vias in memory and would route the + // new anchors on the old order until its reload of the day came back, and a save + // replayed from the offline queue has no reload after it at all. + if (reordered) this.assignments.broadcast(tripId, 'assignment:reordered', reordered, undefined); + if (vias) this.assignments.broadcast(tripId, 'roadtripVia:changed', vias, undefined); this.assignments.reconcile(tripId, socketId); return { assignment }; } diff --git a/server/src/nest/assignments/assignments.mcp.ts b/server/src/nest/assignments/assignments.mcp.ts index 76b5c0c45..1fa212be1 100644 --- a/server/src/nest/assignments/assignments.mcp.ts +++ b/server/src/nest/assignments/assignments.mcp.ts @@ -130,12 +130,15 @@ export class AssignmentsMcp { if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); const existing = this.assignments.getAssignmentForTrip(assignmentId, tripId); if (!existing) return errorResult('Assignment not found.'); - const assignment = this.assignments.updateTime( + const { assignment, reordered, vias } = this.assignments.updateTime( assignmentId, place_time !== undefined ? place_time : existing.assignment_time, end_time !== undefined ? end_time : existing.assignment_end_time ); + // Same three events as PUT /assignments/:id/time. this.guards.safeBroadcast(tripId, 'assignment:updated', { assignment }); + if (reordered) this.guards.safeBroadcast(tripId, 'assignment:reordered', reordered); + if (vias) this.guards.safeBroadcast(tripId, 'roadtripVia:changed', vias); this.assignments.reconcile(tripId); return ok({ assignment }); } diff --git a/server/src/nest/assignments/assignments.service.ts b/server/src/nest/assignments/assignments.service.ts index b0f7d516c..632d5f59a 100644 --- a/server/src/nest/assignments/assignments.service.ts +++ b/server/src/nest/assignments/assignments.service.ts @@ -1,5 +1,6 @@ import { Injectable } from '@nestjs/common'; -import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; +import { chronoOrder, type RoadtripVia, type TrekWsPayload, type TrekWsTripEventName } from '@trek/shared'; +import { isEmptyReanchoring, reanchorByStopOrder, type AnchoredVia } from '@trek/shared/roadtrip'; import { RealtimeService } from '../realtime/realtime.service'; import { DatabaseService, type TripAccess } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; @@ -10,6 +11,39 @@ import { JourneyDomainService } from '../journey/journey-domain.service'; type Trip = TripAccess; +/** One stop of a day as the time sort reads it. */ +interface DayStopRow { + id: number; + order_index: number; + effective_time: string | null; + located: number; +} + +/** + * What saving a time changed besides the stop itself, so each caller can tell the + * trip. `reordered` carries the day's whole order and `vias` the day's re-pinned + * vias; both stay null when the save left every stop where it was. + */ +export interface AssignmentTimeUpdate { + assignment: ReturnType; + reordered: { dayId: number; orderedIds: number[] } | null; + vias: { dayId: number; vias: RoadtripVia[] } | null; +} + +/** + * Where the time sort puts a value it cannot read as a clock time: after every real + * time, which is where the '99:99' sentinel has always put a time without a colon. A + * legacy "morning" keeps sorting where it did. The client reads such a value as no + * time at all; that difference is older than this rule and left alone. + */ +const UNREADABLE_TIME = 99 * 60 + 99; + +function sortMinutes(time: string | null): number | null { + if (!time) return null; + const clock = /(?:^|T)(\d{1,2}):(\d{2})/.exec(time); + return clock ? Number(clock[1]) * 60 + Number(clock[2]) : UNREADABLE_TIME; +} + /** * Assignments domain service — owns the day-assignment SQL (relocated from the * legacy services/assignmentService.ts, then hardened: every multi-statement @@ -206,48 +240,136 @@ export class AssignmentsService { `, assignmentId); } - updateTime(id: string | number, placeTime: unknown, endTime: unknown) { - this.dbs.transaction(() => { + /** + * Saves a visit's own start and end, and puts the day back in time order when the + * start changed. The rule is the planner's (`chronoOrder`): an untimed stop stays + * behind the stop it followed. This used to append every untimed stop after the + * timed ones, so one start time pulled a stop planned last to the top of the day. + * + * The rule is shared, what it reads is not. This sorts the day's stops alone, with a + * booked night timed by its check-in. The planner sorts day notes and bookings in + * between them and never draws the night's row. An untimed stop behind a timed note + * or train takes that item's time there and the previous stop's time here, so on + * such a day the order stored and the order drawn can differ. The old sort did the + * same. + */ + updateTime(id: string | number, placeTime: unknown, endTime: unknown): AssignmentTimeUpdate { + const sorted = this.dbs.transaction(() => { + const stored = this.dbs.get<{ day_id: number; start: string | null }>(` + SELECT da.day_id, COALESCE(da.assignment_time, p.place_time, acc.check_in) AS start + FROM day_assignments da + JOIN places p ON da.place_id = p.id + LEFT JOIN day_accommodations acc ON acc.id = da.accommodation_id + WHERE da.id = ? + `, id); + // Falsy times (null, undefined, '') all clear the override — an empty // string is a clear, not a stored value. this.dbs.run('UPDATE day_assignments SET assignment_time = ?, assignment_end_time = ? WHERE id = ?', placeTime || null, endTime || null, id); - // Auto-sort: reorder timed assignments chronologically within the day - if (placeTime) { - const assignment = this.dbs.get<{ day_id: number }>('SELECT day_id FROM day_assignments WHERE id = ?', id); - if (assignment) { - // A booked night's hour lives on the booking, not on the stop: nobody types a - // time into a hotel row, they type a check-in. Left out of this, the night - // counted as untimed and stayed wherever it had been dropped, so pinning an - // afternoon stop sorted that one and left the hotel sitting in front of or - // behind it by accident. - const dayAssignments = this.dbs.all<{ id: number; effective_time: string | null }>(` - SELECT da.id, COALESCE(da.assignment_time, p.place_time, acc.check_in) as effective_time - FROM day_assignments da - JOIN places p ON da.place_id = p.id - LEFT JOIN day_accommodations acc ON acc.id = da.accommodation_id - WHERE da.day_id = ? - ORDER BY da.order_index ASC - `, assignment.day_id); - - // Separate timed and untimed, sort timed by time - const timed = dayAssignments.filter(a => a.effective_time).sort((a, b) => { - const ta = a.effective_time!.includes(':') ? a.effective_time! : '99:99'; - const tb = b.effective_time!.includes(':') ? b.effective_time! : '99:99'; - return ta.localeCompare(tb); - }); - const untimed = dayAssignments.filter(a => !a.effective_time); - - // Interleave: timed in chronological order, untimed keep relative position - const reordered = [...timed, ...untimed]; - const update = this.dbs.prepare('UPDATE day_assignments SET order_index = ? WHERE id = ?'); - reordered.forEach((a, i) => update.run(i, a.id)); - } - } + // Only a start that moved sorts. An end is a label. A start sent again as it + // stood (the place form saving an End, the stay dialog taking one off, an MCP + // call that names only the end) leaves the day the way the traveller left it, + // which can be out of time order on purpose. Compared the way the sort reads + // it, so a visit given the time its place already had moves nothing either. A + // cleared start leaves the day alone too. + if (!placeTime || !stored) return null; + if (sortMinutes(String(placeTime)) === sortMinutes(stored.start)) return null; + return this.sortDayByTime(stored.day_id); }); - return this.getAssignmentWithPlace(Number(id)); + return { + assignment: this.getAssignmentWithPlace(Number(id)), + reordered: sorted ? { dayId: sorted.dayId, orderedIds: sorted.orderedIds } : null, + vias: sorted?.viasMoved ? { dayId: sorted.dayId, vias: this.listDayVias(sorted.dayId) } : null, + }; + } + + /** + * Puts one day in time order. Writes nothing when it already is, which is the usual + * case: most starts are typed in the order the day is planned. + */ + private sortDayByTime(dayId: number): { dayId: number; orderedIds: number[]; viasMoved: boolean } | null { + // A booked night's hour lives on the booking, not on the stop: nobody types a + // time into a hotel row, they type a check-in. Left out of this, the night + // counted as untimed and stayed wherever it had been dropped, so pinning an + // afternoon stop sorted that one and left the hotel sitting in front of or + // behind it by accident. + const rows = this.dbs.all(` + SELECT da.id, da.order_index, COALESCE(da.assignment_time, p.place_time, acc.check_in) as effective_time, + (p.lat IS NOT NULL AND p.lng IS NOT NULL) as located + FROM day_assignments da + JOIN places p ON da.place_id = p.id + LEFT JOIN day_accommodations acc ON acc.id = da.accommodation_id + WHERE da.day_id = ? + ORDER BY da.order_index ASC, da.created_at ASC, da.id ASC + `, dayId); + + const sorted = chronoOrder(rows, row => sortMinutes(row.effective_time)); + if (sorted.every((row, i) => row === rows[i])) return null; + + // Numbered from 0, the way a drag stores a day (`reorderAssignments`). The order + // goes out as a list of ids and every client numbers it by position, so keys kept + // with their gaps would put the day notes and bookings that sort between stops in + // one place for the writer, who reads the day back, and in another for everyone + // else. Only a stop whose key changes is written. + const update = this.dbs.prepare('UPDATE day_assignments SET order_index = ? WHERE id = ?'); + sorted.forEach((row, i) => { + if (row.order_index !== i) update.run(i, row.id); + }); + + return { dayId, orderedIds: sorted.map(row => row.id), viasMoved: this.reanchorVias(dayId, rows, sorted) }; + } + + /** + * Keeps every drawn road behind the stop it was drawn after, the rule the planner + * applies when stops are dragged (`reanchorByStopOrder`). A via is pinned to a + * POSITION among the day's located stops, so a sort that moves a stop would + * otherwise hand the vias behind it to other legs. Inside the sort's transaction: + * an order without its vias is a road the traveller never drew. + * + * No sequence renumbering, unlike RoadtripService.reanchor: a reorder maps each leg + * onto a different one, so two legs' vias never end up on the same leg. + */ + private reanchorVias(dayId: number, before: DayStopRow[], after: DayStopRow[]): boolean { + const located = (rows: DayStopRow[]) => rows.filter(row => row.located).map(row => row.id); + const previousIds = located(before); + const nextIds = located(after); + // Only stops without coordinates moved. The router never sees those, so every leg + // is still the one it was. + if (previousIds.every((stopId, i) => stopId === nextIds[i])) return false; + + // A via behind the day's last stop bends the drive into the next day, on a trip + // with connected days or a night drive (the planner's `anchorFor` files it there). + // `reanchorByStopOrder` gives a last stop no leg and deletes what follows it, which + // is right for a stop the sort made last and wrong for one that was last already. + const lastAt = previousIds.length - 1; + const seam = previousIds[lastAt] === nextIds[lastAt] ? lastAt : null; + const vias = this.dbs.all('SELECT id, after_order_index, lat, lng FROM roadtrip_vias WHERE day_id = ?', dayId) + .filter(via => via.after_order_index !== seam); + const plan = reanchorByStopOrder(vias, previousIds, nextIds); + for (const viaId of plan.remove) { + this.dbs.run('DELETE FROM roadtrip_vias WHERE id = ? AND day_id = ?', viaId, dayId); + } + for (const via of plan.vias) { + this.dbs.run('UPDATE roadtrip_vias SET after_order_index = ? WHERE id = ? AND day_id = ?', via.after_order_index, via.id, dayId); + } + return !isEmptyReanchoring(plan); + } + + /** + * The day's vias in the shape the road trip routes broadcast them. RoadtripService + * has this query too, but its module imports this one, so it cannot be injected here. + */ + private listDayVias(dayId: number): RoadtripVia[] { + return this.dbs.all( + `SELECT id, day_id, after_order_index, sequence, lat, lng, created_at + FROM roadtrip_vias + WHERE day_id = ? + ORDER BY after_order_index, sequence, id`, + dayId, + ); } setEndDay(id: string | number, endDay: boolean) { diff --git a/server/tests/e2e/assignments.e2e.test.ts b/server/tests/e2e/assignments.e2e.test.ts index 25c089e02..36d53a7a4 100644 --- a/server/tests/e2e/assignments.e2e.test.ts +++ b/server/tests/e2e/assignments.e2e.test.ts @@ -43,6 +43,11 @@ const { db } = vi.hoisted(() => { check_out TEXT, confirmation TEXT, notes TEXT, created_at TEXT DEFAULT (datetime('now')));`); tmp.exec(`CREATE TABLE assignment_participants (assignment_id INTEGER NOT NULL, user_id INTEGER NOT NULL, UNIQUE(assignment_id, user_id));`); + // A start that reorders a day re-pins that day's vias in the same transaction, so + // the sort reads this table whenever it moves a stop. + tmp.exec(`CREATE TABLE roadtrip_vias (id INTEGER PRIMARY KEY AUTOINCREMENT, day_id INTEGER NOT NULL, + after_order_index INTEGER NOT NULL, sequence INTEGER NOT NULL DEFAULT 0, lat REAL NOT NULL, lng REAL NOT NULL, + created_at TEXT DEFAULT CURRENT_TIMESTAMP);`); tmp.exec(`CREATE TABLE tags (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT, color TEXT, created_at TEXT);`); tmp.exec(`CREATE TABLE place_tags (place_id INTEGER NOT NULL, tag_id INTEGER NOT NULL);`); // StorageRegistryService (behind StorageModule, now in this module chain) reads @@ -55,7 +60,8 @@ const { canAccessTrip } = vi.hoisted(() => ({ canAccessTrip: vi.fn() })); vi.mock('../../src/db/database', () => ({ db, canAccessTrip, isOwner: vi.fn(() => true), getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {}, })); -vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); +const { broadcast } = vi.hoisted(() => ({ broadcast: vi.fn() })); +vi.mock('../../src/websocket', () => ({ broadcast })); const { reconcileTripSkeletons } = vi.hoisted(() => ({ reconcileTripSkeletons: vi.fn() })); import { JourneyDomainService } from '../../src/nest/journey/journey-domain.service'; @@ -213,6 +219,77 @@ describe('Assignments e2e (real auth guard + temp SQLite)', () => { expect(reconcileTripSkeletons).toHaveBeenCalledWith(5, undefined); }); + describe('PUT /:id/time and the order of the day', () => { + const seedDay = (times: (string | null)[]) => times.map((time, i) => { + const id = seedAssignment(3, 2, i); + if (time) db.prepare('UPDATE day_assignments SET assignment_time = ? WHERE id = ?').run(time, id); + return id; + }); + const dayOrder = () => + (db.prepare('SELECT id FROM day_assignments WHERE day_id = 3 ORDER BY order_index, id').all() as { id: number }[]).map(r => r.id); + const eventsSent = () => broadcast.mock.calls.map(call => call[1]); + + beforeEach(() => broadcast.mockClear()); + + it('keeps the untimed stops in front of the stop that gets a start', async () => { + const [a, b, c] = seedDay([null, null, null]); + const res = await request(server) + .put(`/api/trips/5/assignments/${c}/time`) + .set('Cookie', sessionCookie(1)) + .set('X-Socket-Id', 'sock-1') + .send({ place_time: '14:00', end_time: null }); + expect(res.status).toBe(200); + expect(res.body.assignment).toMatchObject({ id: c, assignment_time: '14:00', order_index: 2 }); + expect(dayOrder()).toEqual([a, b, c]); + expect(eventsSent()).toEqual(['assignment:updated']); + }); + + it('sorts the timed stops, keeps the untimed head first and sends the whole day', async () => { + const [a, b, c] = seedDay([null, '15:00', null]); + const res = await request(server) + .put(`/api/trips/5/assignments/${c}/time`) + .set('Cookie', sessionCookie(1)) + .set('X-Socket-Id', 'sock-1') + .send({ place_time: '10:00', end_time: null }); + expect(res.status).toBe(200); + expect(dayOrder()).toEqual([a, c, b]); + // No socket left out, so the writer gets the order too. + expect(broadcast).toHaveBeenCalledWith('5', 'assignment:reordered', { dayId: 3, orderedIds: [a, c, b] }, undefined); + // No located stops and no vias on this day, so there is nothing to re-pin. + expect(eventsSent()).not.toContain('roadtripVia:changed'); + }); + + it('stores the order it sends: a day with a gap in its keys is numbered from 0', async () => { + // The gap a deleted stop leaves. Clients number the ids they are sent by position. + const [a, b, c] = [0, 4, 7].map(key => seedAssignment(3, 2, key)); + db.prepare('UPDATE day_assignments SET assignment_time = ? WHERE id = ?').run('15:00', b); + const res = await request(server) + .put(`/api/trips/5/assignments/${c}/time`) + .set('Cookie', sessionCookie(1)) + .send({ place_time: '10:00', end_time: null }); + expect(res.status).toBe(200); + const sent = broadcast.mock.calls.find(call => call[1] === 'assignment:reordered')?.[2] as { orderedIds: number[] }; + expect(sent.orderedIds).toEqual([a, c, b]); + const keyOf = (id: number) => (db.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(id) as { order_index: number }).order_index; + expect(sent.orderedIds.map(keyOf)).toEqual([0, 1, 2]); + expect(res.body.assignment).toMatchObject({ id: c, order_index: 1 }); + }); + + it('leaves a day dragged out of time order alone when only the End changes', async () => { + const [a, b] = seedDay(['14:00', '10:00']); + db.prepare("UPDATE day_assignments SET assignment_end_time = '11:00' WHERE id = ?").run(b); + const res = await request(server) + .put(`/api/trips/5/assignments/${b}/time`) + .set('Cookie', sessionCookie(1)) + .set('X-Socket-Id', 'sock-1') + .send({ place_time: '10:00', end_time: null }); + expect(res.status).toBe(200); + expect(res.body.assignment).toMatchObject({ id: b, assignment_time: '10:00', assignment_end_time: null }); + expect(dayOrder()).toEqual([a, b]); + expect(eventsSent()).toEqual(['assignment:updated']); + }); + }); + it('200 participants (access-only)', async () => { const id = seedAssignment(); db.prepare('INSERT INTO assignment_participants (assignment_id, user_id) VALUES (?, 2)').run(id); diff --git a/server/tests/unit/mcp/tools-assignments.test.ts b/server/tests/unit/mcp/tools-assignments.test.ts index 887b224bd..91b2786bc 100644 --- a/server/tests/unit/mcp/tools-assignments.test.ts +++ b/server/tests/unit/mcp/tools-assignments.test.ts @@ -403,6 +403,75 @@ describe('Tool: update_assignment_time', () => { }); }); + // Same service method as PUT /assignments/:id/time, so the same order and the same + // three events. A: untimed, B: 15:00, C: untimed, D gets 10:00. + it('keeps untimed stops in place, sorts the timed ones and sends the day and its vias like REST', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id); + const [a, b, c, d] = [0, 1, 2, 3].map(i => createDayAssignment(testDb, day.id, place.id, { order_index: i }).id); + testDb.prepare('UPDATE day_assignments SET assignment_time = ? WHERE id = ?').run('15:00', b); + const via = Number(testDb.prepare( + 'INSERT INTO roadtrip_vias (day_id, after_order_index, sequence, lat, lng) VALUES (?, 1, 0, 48.1, 11.5)' + ).run(day.id).lastInsertRowid); + + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ name: 'update_assignment_time', arguments: { tripId: trip.id, assignmentId: d, place_time: '10:00' } }); + expect((parseToolResult(result) as { assignment: { assignment_time: string } }).assignment.assignment_time).toBe('10:00'); + }); + + const order = (testDb.prepare('SELECT id FROM day_assignments WHERE day_id = ? ORDER BY order_index').all(day.id) as { id: number }[]).map(r => r.id); + expect(order).toEqual([a, d, b, c]); + expect(testDb.prepare('SELECT after_order_index FROM roadtrip_vias WHERE id = ?').get(via)).toEqual({ after_order_index: 2 }); + expect(broadcastMock).toHaveBeenCalledWith(trip.id, 'assignment:reordered', expect.objectContaining({ dayId: day.id, orderedIds: [a, d, b, c] })); + expect(broadcastMock).toHaveBeenCalledWith(trip.id, 'roadtripVia:changed', expect.objectContaining({ + dayId: day.id, + vias: [expect.objectContaining({ id: via, after_order_index: 2 })], + })); + }); + + it('sends only the row when the start leaves the day as it was', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id); + const [a, b, c] = [0, 1, 2].map(i => createDayAssignment(testDb, day.id, place.id, { order_index: i }).id); + + await withHarness(user.id, async (h) => { + await h.client.callTool({ name: 'update_assignment_time', arguments: { tripId: trip.id, assignmentId: c, place_time: '14:00' } }); + }); + + const order = (testDb.prepare('SELECT id FROM day_assignments WHERE day_id = ? ORDER BY order_index').all(day.id) as { id: number }[]).map(r => r.id); + expect(order).toEqual([a, b, c]); + expect(broadcastMock.mock.calls.map(call => call[1])).toEqual(['assignment:updated']); + }); + + it('leaves a day out of time order as it is when the call names only the end', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id); + // B starts before A and was put behind it on purpose. + const [a, b] = [0, 1].map(i => createDayAssignment(testDb, day.id, place.id, { order_index: i }).id); + testDb.prepare("UPDATE day_assignments SET assignment_time = '14:00' WHERE id = ?").run(a); + testDb.prepare("UPDATE day_assignments SET assignment_time = '10:00' WHERE id = ?").run(b); + const via = Number(testDb.prepare( + 'INSERT INTO roadtrip_vias (day_id, after_order_index, sequence, lat, lng) VALUES (?, 0, 0, 48.1, 11.5)' + ).run(day.id).lastInsertRowid); + + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ name: 'update_assignment_time', arguments: { tripId: trip.id, assignmentId: b, end_time: '11:30' } }); + expect((parseToolResult(result) as { assignment: { assignment_time: string; assignment_end_time: string } }).assignment) + .toMatchObject({ assignment_time: '10:00', assignment_end_time: '11:30' }); + }); + + const order = (testDb.prepare('SELECT id FROM day_assignments WHERE day_id = ? ORDER BY order_index').all(day.id) as { id: number }[]).map(r => r.id); + expect(order).toEqual([a, b]); + expect(testDb.prepare('SELECT after_order_index FROM roadtrip_vias WHERE id = ?').get(via)).toEqual({ after_order_index: 0 }); + expect(broadcastMock.mock.calls.map(call => call[1])).toEqual(['assignment:updated']); + }); + it('returns error when assignment not found', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); diff --git a/server/tests/unit/nest/assignments.controller.test.ts b/server/tests/unit/nest/assignments.controller.test.ts index 14c4bae28..09d86a6fc 100644 --- a/server/tests/unit/nest/assignments.controller.test.ts +++ b/server/tests/unit/nest/assignments.controller.test.ts @@ -99,13 +99,32 @@ describe('AssignmentOpsController (parity with the per-assignment op routes)', ( it('PUT /:id/time 404 missing, else updates', () => { expect(thrown(() => new AssignmentOpsController(svc({ getAssignmentForTrip: vi.fn().mockReturnValue(undefined) } as Partial)).time(user, '5', '9', {}))).toEqual({ status: 404, body: { error: 'Assignment not found' } }); - const updateTime = vi.fn().mockReturnValue({ id: 9 }); const broadcast = vi.fn(); const reconcile = vi.fn(); + const updateTime = vi.fn().mockReturnValue({ assignment: { id: 9 }, reordered: null, vias: null }); const broadcast = vi.fn(); const reconcile = vi.fn(); const s = svc({ getAssignmentForTrip: vi.fn().mockReturnValue({ id: 9 }), updateTime, broadcast, reconcile } as Partial); expect(new AssignmentOpsController(s).time(user, '5', '9', { place_time: '10:00' }, 'sock')).toEqual({ assignment: { id: 9 } }); expect(updateTime).toHaveBeenCalledWith('9', '10:00', undefined); + // Nothing moved, so the row is all collaborators hear about. + expect(broadcast.mock.calls).toEqual([['5', 'assignment:updated', { assignment: { id: 9 } }, 'sock']]); expect(reconcile).toHaveBeenCalledWith('5', 'sock'); }); + it('PUT /:id/time sends the whole day and the re-pinned vias to every socket when a start moved stops', () => { + const reordered = { dayId: 3, orderedIds: [7, 9, 8] }; + const vias = { dayId: 3, vias: [{ id: 40, day_id: 3, after_order_index: 1, sequence: 0, lat: 1, lng: 2 }] }; + const updateTime = vi.fn().mockReturnValue({ assignment: { id: 9 }, reordered, vias }); const broadcast = vi.fn(); + const s = svc({ getAssignmentForTrip: vi.fn().mockReturnValue({ id: 9 }), updateTime, broadcast } as Partial); + + expect(new AssignmentOpsController(s).time(user, '5', '9', { place_time: '10:00' }, 'sock')).toEqual({ assignment: { id: 9 } }); + + expect(broadcast.mock.calls).toEqual([ + ['5', 'assignment:updated', { assignment: { id: 9 } }, 'sock'], + // The writer's socket too, so the order and the vias pinned to it arrive + // together there as well, a replayed offline save included. + ['5', 'assignment:reordered', reordered, undefined], + ['5', 'roadtripVia:changed', vias, undefined], + ]); + }); + it('PUT /:id/notes 404 missing, else updates + broadcasts (#2163)', () => { expect(thrown(() => new AssignmentOpsController(svc({ getAssignmentForTrip: vi.fn().mockReturnValue(undefined) } as Partial)).notes(user, '5', '9', { notes: 'n' }))).toEqual({ status: 404, body: { error: 'Assignment not found' } }); const updateNotes = vi.fn().mockReturnValue({ id: 9, notes: 'Book the 10:00 entry' }); const broadcast = vi.fn(); const reconcile = vi.fn(); diff --git a/server/tests/unit/nest/assignments.service.test.ts b/server/tests/unit/nest/assignments.service.test.ts index 3c97ffc0e..d7c381081 100644 --- a/server/tests/unit/nest/assignments.service.test.ts +++ b/server/tests/unit/nest/assignments.service.test.ts @@ -348,25 +348,56 @@ describe('getParticipants / setParticipants', () => { // ── updateTime / setLegTransportMode ────────────────────────────────────────── describe('updateTime', () => { + const order = (id: number) => (testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(id) as { order_index: number }).order_index; + /** The day the way every reader lists it: by order_index, then insertion. */ + const dayOrder = (dayId: number) => + (testDb.prepare('SELECT id FROM day_assignments WHERE day_id = ? ORDER BY order_index, created_at, id').all(dayId) as { id: number }[]).map(r => r.id); + + /** + * A day of stops at the given order_index keys, each with its own start (null for + * none). Times go in by SQL so the setup itself never sorts anything. + */ + function dayOf(stops: [string | null, number][]) { + const { trip, day, place } = fixture(); + const ids = stops.map(([time, orderIndex]) => { + const a = createDayAssignment(testDb, day.id, place.id, { order_index: orderIndex }); + if (time) testDb.prepare('UPDATE day_assignments SET assignment_time = ? WHERE id = ?').run(time, a.id); + return a.id; + }); + return { trip, day, ids }; + } + + function addVia(dayId: number, afterOrderIndex: number, sequence = 0): number { + return Number(testDb.prepare( + 'INSERT INTO roadtrip_vias (day_id, after_order_index, sequence, lat, lng) VALUES (?, ?, ?, 48.1, 11.5)' + ).run(dayId, afterOrderIndex, sequence).lastInsertRowid); + } + const viaAnchors = (dayId: number) => + testDb.prepare('SELECT id, after_order_index, sequence FROM roadtrip_vias WHERE day_id = ? ORDER BY id').all(dayId); + it('ASG-SVC-021: persists both times and re-selects the nested shape', () => { const { day, place } = fixture(); const a = createDayAssignment(testDb, day.id, place.id); - const updated = svc.updateTime(a.id, '09:00', '11:30'); - expect(updated).toMatchObject({ id: a.id, assignment_time: '09:00', assignment_end_time: '11:30' }); + const { assignment } = svc.updateTime(a.id, '09:00', '11:30'); + expect(assignment).toMatchObject({ id: a.id, assignment_time: '09:00', assignment_end_time: '11:30' }); }); - it('ASG-SVC-022: a truthy place_time auto-sorts the day chronologically (untimed keep relative position)', () => { - const { day, place } = fixture(); - const a = createDayAssignment(testDb, day.id, place.id, { order_index: 0 }); - const b = createDayAssignment(testDb, day.id, place.id, { order_index: 1 }); - const c = createDayAssignment(testDb, day.id, place.id, { order_index: 2 }); + it('ASG-SVC-022: a start sorts the timed stops and leaves every untimed stop behind the one it followed', () => { + // The tester's day: two stops without a time, then a start on the last one. It + // used to jump to the top, because every untimed stop was appended after it. + const untimedHead = dayOf([[null, 0], [null, 1], [null, 2]]); + svc.updateTime(untimedHead.ids[2], '14:00', null); + expect(dayOrder(untimedHead.day.id)).toEqual(untimedHead.ids); - svc.updateTime(c.id, '08:00', null); - const order = (id: number) => (testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(id) as { order_index: number }).order_index; - expect([order(c.id), order(a.id), order(b.id)]).toEqual([0, 1, 2]); + const between = dayOf([['09:00', 0], [null, 1], [null, 2]]); + svc.updateTime(between.ids[2], '14:00', null); + expect(dayOrder(between.day.id)).toEqual(between.ids); - svc.updateTime(a.id, '07:00', null); - expect([order(a.id), order(c.id), order(b.id)]).toEqual([0, 1, 2]); + // Timed stops still keep time order among themselves. A has nothing timed in + // front of it and stays first. + const [a, b, c] = dayOf([[null, 0], ['15:00', 1], [null, 2]]).ids; + svc.updateTime(c, '10:00', null); + expect([order(a), order(c), order(b)]).toEqual([0, 1, 2]); }); it('ASG-SVC-022b: a booked night sorts by its check-in, which is where its hour lives', () => { @@ -384,34 +415,33 @@ describe('updateTime', () => { svc.updateTime(afternoon.id, '16:00', null); - const order = (id: number) => (testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(id) as { order_index: number }).order_index; expect(order(night.id)).toBeLessThan(order(afternoon.id)); }); it('ASG-SVC-023: clearing with null persists but skips the auto-sort (falsy gate)', () => { - const { day, place } = fixture(); - const a = createDayAssignment(testDb, day.id, place.id, { order_index: 0 }); - const b = createDayAssignment(testDb, day.id, place.id, { order_index: 1 }); - svc.updateTime(b.id, '06:00', null); // sorts b first - const updated = svc.updateTime(b.id, null, null); // clear — no re-sort - expect(updated!.assignment_time).toBeNull(); - const order = (id: number) => (testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(id) as { order_index: number }).order_index; - expect(order(b.id)).toBe(0); - expect(order(a.id)).toBe(1); + const { day, ids: [a, b, c] } = dayOf([['09:00', 0], [null, 1], ['12:00', 2]]); + svc.updateTime(b, '06:00', null); // sorts b first + expect(dayOrder(day.id)).toEqual([b, a, c]); + // Dragged back out of time order on purpose: a sort now would put b first again, + // with c behind it. + svc.reorderAssignments(day.id, [a, b, c]); + + const { assignment } = svc.updateTime(c, null, null); // clear, no re-sort + expect(assignment!.assignment_time).toBeNull(); + expect(dayOrder(day.id)).toEqual([a, b, c]); }); it('ASG-SVC-027: an empty-string time clears the override like null (and skips the auto-sort)', () => { - const { day, place } = fixture(); - const a = createDayAssignment(testDb, day.id, place.id, { order_index: 0 }); - const b = createDayAssignment(testDb, day.id, place.id, { order_index: 1 }); - svc.updateTime(b.id, '06:00', '07:00'); // sorts b first - const updated = svc.updateTime(b.id, '', ''); - expect(updated!.assignment_time).toBeNull(); - expect(updated!.assignment_end_time).toBeNull(); - expect(testDb.prepare('SELECT assignment_time FROM day_assignments WHERE id = ?').get(b.id)).toEqual({ assignment_time: null }); - const order = (id: number) => (testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(id) as { order_index: number }).order_index; - expect(order(b.id)).toBe(0); - expect(order(a.id)).toBe(1); + const { day, ids: [a, b, c] } = dayOf([['09:00', 0], [null, 1], ['12:00', 2]]); + svc.updateTime(b, '06:00', '07:00'); // sorts b first + expect(dayOrder(day.id)).toEqual([b, a, c]); + svc.reorderAssignments(day.id, [a, b, c]); + + const { assignment } = svc.updateTime(c, '', ''); + expect(assignment!.assignment_time).toBeNull(); + expect(assignment!.assignment_end_time).toBeNull(); + expect(testDb.prepare('SELECT assignment_time FROM day_assignments WHERE id = ?').get(c)).toEqual({ assignment_time: null }); + expect(dayOrder(day.id)).toEqual([a, b, c]); }); it('ASG-SVC-024: a time without ":" sorts last among timed via the 99:99 sentinel', () => { @@ -420,10 +450,164 @@ describe('updateTime', () => { const b = createDayAssignment(testDb, day.id, place.id, { order_index: 1 }); svc.updateTime(a.id, 'morning', null); // no colon → '99:99' in the sort svc.updateTime(b.id, '13:00', null); - const order = (id: number) => (testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(id) as { order_index: number }).order_index; expect(order(b.id)).toBe(0); expect(order(a.id)).toBe(1); }); + + it('ASG-SVC-033: writes nothing but the time when the day is already in order', () => { + // Keys with gaps, as a deleted stop leaves them. A day the start leaves in order + // keeps them: nothing about it changed. + const { day, ids: [a, b, c] } = dayOf([['09:00', 0], [null, 5], [null, 9]]); + const via = addVia(day.id, 0); + const changes = () => (testDb.prepare('SELECT total_changes() AS n').get() as { n: number }).n; + const before = changes(); + + const update = svc.updateTime(c, '14:00', null); + + expect(changes() - before).toBe(1); + expect([order(a), order(b), order(c)]).toEqual([0, 5, 9]); + expect(viaAnchors(day.id)).toEqual([{ id: via, after_order_index: 0, sequence: 0 }]); + expect(update.reordered).toBeNull(); + expect(update.vias).toBeNull(); + }); + + it('ASG-SVC-034: numbers a reordered day from 0, the key each client gives a stop of the order it is sent', () => { + // A gap, as a deleted stop leaves it. Kept, the writer would read 0, 4, 7 back and + // everyone else would apply 0, 1, 2, and a note at 3 would sit behind a different + // stop for each of them. + const { day, ids: [a, b, c] } = dayOf([[null, 0], ['15:00', 4], [null, 7]]); + const changes = () => (testDb.prepare('SELECT total_changes() AS n').get() as { n: number }).n; + const before = changes(); + + const update = svc.updateTime(c, '10:00', null); + + expect(update.reordered).toEqual({ dayId: day.id, orderedIds: [a, c, b] }); + expect(update.reordered!.orderedIds.map(order)).toEqual([0, 1, 2]); + // The time, then c and b. A already holds 0 and is not written. + expect(changes() - before).toBe(3); + }); + + it('ASG-SVC-035: numbers a day with colliding keys from 0 again, since a shared key cannot hold an order', () => { + // A move drops a stop in at order_index 0 next to the one already there. + const { day, ids: [a, b, c] } = dayOf([[null, 0], ['15:00', 0], [null, 0]]); + + svc.updateTime(c, '10:00', null); + + expect(dayOrder(day.id)).toEqual([a, c, b]); + expect([order(a), order(c), order(b)]).toEqual([0, 1, 2]); + }); + + it('ASG-SVC-036: keeps each drawn road behind the stop it was drawn after, counting located stops only', () => { + // X has no coordinates, so the router never sees it and the vias do not count it. + const { trip, day, ids: [a, x, b, c, d] } = dayOf([[null, 0], [null, 1], ['15:00', 2], [null, 3], [null, 4]]); + const unmapped = createPlace(testDb, trip.id, { name: 'Somewhere unmapped' }); + testDb.prepare('UPDATE places SET lat = NULL, lng = NULL WHERE id = ?').run(unmapped.id); + testDb.prepare('UPDATE day_assignments SET place_id = ? WHERE id = ?').run(unmapped.id, x); + const afterA = addVia(day.id, 0); + const afterB = addVia(day.id, 1); + const afterBToo = addVia(day.id, 1, 1); + addVia(day.id, 2); // after C + + // D at 10:00 goes in front of B at 15:00, and C stays behind B: A, X, D, B, C. + const update = svc.updateTime(d, '10:00', null); + + expect(dayOrder(day.id)).toEqual([a, x, d, b, c]); + // A keeps its leg, B's two points follow B to 2 in their order, and C, now last, + // has no leg left to bend. + expect(viaAnchors(day.id)).toEqual([ + { id: afterA, after_order_index: 0, sequence: 0 }, + { id: afterB, after_order_index: 2, sequence: 0 }, + { id: afterBToo, after_order_index: 2, sequence: 1 }, + ]); + expect(update.reordered).toEqual({ dayId: day.id, orderedIds: [a, x, d, b, c] }); + expect(update.vias?.dayId).toBe(day.id); + expect(update.vias?.vias.map(v => [v.id, v.after_order_index])).toEqual([[afterA, 0], [afterB, 2], [afterBToo, 2]]); + }); + + it('ASG-SVC-037: reports no vias when the reorder leaves every one on its stop', () => { + const { day, ids: [a, b, c] } = dayOf([[null, 0], ['15:00', 1], [null, 2]]); + const via = addVia(day.id, 0); + + const update = svc.updateTime(c, '10:00', null); + + expect(update.reordered).toEqual({ dayId: day.id, orderedIds: [a, c, b] }); + expect(viaAnchors(day.id)).toEqual([{ id: via, after_order_index: 0, sequence: 0 }]); + expect(update.vias).toBeNull(); + }); + + it('ASG-SVC-038: leaves every via alone when the sort moves only stops without coordinates', () => { + // X has no coordinates. It goes between A and B, and the located stops stay A, B. + const { trip, day, ids: [a, b, x] } = dayOf([['09:00', 0], ['12:00', 1], [null, 2]]); + const unmapped = createPlace(testDb, trip.id, { name: 'Somewhere unmapped' }); + testDb.prepare('UPDATE places SET lat = NULL, lng = NULL WHERE id = ?').run(unmapped.id); + testDb.prepare('UPDATE day_assignments SET place_id = ? WHERE id = ?').run(unmapped.id, x); + const afterA = addVia(day.id, 0); + // Behind the day's last stop: the drive into the next day on a trip with connected + // days. It was deleted here, since a last stop has no leg of its own. + const intoTomorrow = addVia(day.id, 1); + + const update = svc.updateTime(x, '10:00', null); + + expect(dayOrder(day.id)).toEqual([a, x, b]); + expect(update.reordered).toEqual({ dayId: day.id, orderedIds: [a, x, b] }); + expect(viaAnchors(day.id)).toEqual([ + { id: afterA, after_order_index: 0, sequence: 0 }, + { id: intoTomorrow, after_order_index: 1, sequence: 0 }, + ]); + expect(update.vias).toBeNull(); + }); + + it('ASG-SVC-039: keeps the via behind the last stop when that stop is still last', () => { + const { day, ids: [a, b, c, d] } = dayOf([[null, 0], ['15:00', 1], [null, 2], ['18:00', 3]]); + const afterB = addVia(day.id, 1); + const intoTomorrow = addVia(day.id, 3); + + // C at 10:00 goes in front of B. D stays last, and so does the drive out of it. + const update = svc.updateTime(c, '10:00', null); + + expect(dayOrder(day.id)).toEqual([a, c, b, d]); + expect(viaAnchors(day.id)).toEqual([ + { id: afterB, after_order_index: 2, sequence: 0 }, + { id: intoTomorrow, after_order_index: 3, sequence: 0 }, + ]); + expect(update.vias?.vias.map(v => [v.id, v.after_order_index])).toEqual([[afterB, 2], [intoTomorrow, 3]]); + }); + + it('ASG-SVC-040: an End saved with the start as it stood leaves a day dragged out of time order alone', () => { + // B belongs first by time, and the traveller put it second on purpose. + const { day, ids: [a, b, c] } = dayOf([['14:00', 0], ['10:00', 1], ['16:00', 2]]); + testDb.prepare("UPDATE day_assignments SET assignment_end_time = '11:00' WHERE id = ?").run(b); + const afterA = addVia(day.id, 0); + + // The stay dialog taking the End off, then the place form setting a new one. + for (const end of [null, '12:00']) { + const update = svc.updateTime(b, '10:00', end); + expect(update.assignment).toMatchObject({ assignment_time: '10:00', assignment_end_time: end }); + expect(update.reordered).toBeNull(); + expect(update.vias).toBeNull(); + expect(dayOrder(day.id)).toEqual([a, b, c]); + expect(viaAnchors(day.id)).toEqual([{ id: afterA, after_order_index: 0, sequence: 0 }]); + } + + // A start that moves still sorts, and the via follows A. + const update = svc.updateTime(b, '09:00', '12:00'); + expect(dayOrder(day.id)).toEqual([b, a, c]); + expect(update.reordered).toEqual({ dayId: day.id, orderedIds: [b, a, c] }); + expect(viaAnchors(day.id)).toEqual([{ id: afterA, after_order_index: 1, sequence: 0 }]); + }); + + it('ASG-SVC-041: a visit given the start its place already had does not sort either', () => { + // The place form fills Start from the place when the visit has none of its own. + const { trip, day, ids: [a, b] } = dayOf([['14:00', 0], [null, 1]]); + const timedPlace = createPlace(testDb, trip.id, { name: 'Opens at ten' }); + testDb.prepare("UPDATE places SET place_time = '10:00' WHERE id = ?").run(timedPlace.id); + testDb.prepare('UPDATE day_assignments SET place_id = ? WHERE id = ?').run(timedPlace.id, b); + + const update = svc.updateTime(b, '10:00', '11:00'); + + expect(update.reordered).toBeNull(); + expect(dayOrder(day.id)).toEqual([a, b]); + }); }); describe('updateNotes (#2163)', () => { @@ -451,7 +635,9 @@ describe('updateNotes (#2163)', () => { svc.updateTime(a.id, '09:00', '10:00'); const updated = svc.updateNotes(a.id, 'note'); expect(updated).toMatchObject({ assignment_time: '09:00', assignment_end_time: '10:00' }); - expect((testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(a.id) as { order_index: number }).order_index).toBe(0); + // 3, not 0: a start on a day already in order no longer renumbers it, so the key + // this note edit must not touch is still the one the stop was created with. + expect((testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(a.id) as { order_index: number }).order_index).toBe(3); }); }); diff --git a/shared/src/day/chrono-order.spec.ts b/shared/src/day/chrono-order.spec.ts new file mode 100644 index 000000000..dc509d7fe --- /dev/null +++ b/shared/src/day/chrono-order.spec.ts @@ -0,0 +1,52 @@ +import { chronoOrder } from './chrono-order'; + +import { describe, it, expect } from 'vitest'; + +type Stop = { name: string; at: number | null }; + +const stop = (name: string, at: number | null = null): Stop => ({ name, at }); +const order = (stops: Stop[]) => chronoOrder(stops, (s) => s.at).map((s) => s.name); + +describe('chronoOrder', () => { + it('leaves untimed stops in front of the first timed one where they are', () => { + expect(order([stop('A'), stop('B'), stop('C', 14 * 60)])).toEqual(['A', 'B', 'C']); + }); + + it('keeps an untimed stop behind the timed stop it followed', () => { + expect(order([stop('A', 9 * 60), stop('B'), stop('C', 14 * 60)])).toEqual(['A', 'B', 'C']); + }); + + it('sorts timed stops among themselves and leaves the untimed head alone', () => { + expect(order([stop('A'), stop('B', 15 * 60), stop('C', 10 * 60)])).toEqual(['A', 'C', 'B']); + }); + + it('carries an untimed stop along with the timed stop in front of it', () => { + // B inherits 15:00 from A, so both go behind C at 10:00, and B stays behind A. + expect(order([stop('A', 15 * 60), stop('B'), stop('C', 10 * 60)])).toEqual(['C', 'A', 'B']); + }); + + it('keeps the incoming order for equal times', () => { + expect(order([stop('A', 600), stop('B', 600), stop('C', 540), stop('D', 540)])).toEqual(['C', 'D', 'A', 'B']); + }); + + it('treats undefined like null and orders midnight as a real time', () => { + const stops = [ + { name: 'A', at: undefined }, + { name: 'B', at: 60 }, + { name: 'C', at: 0 }, + ]; + expect(chronoOrder(stops, (s) => s.at).map((s) => s.name)).toEqual(['A', 'C', 'B']); + }); + + it('returns a new array and leaves the input as it was', () => { + const stops = [stop('A', 600), stop('B', 540)]; + const sorted = chronoOrder(stops, (s) => s.at); + expect(sorted).not.toBe(stops); + expect(stops.map((s) => s.name)).toEqual(['A', 'B']); + expect(sorted[0]).toBe(stops[1]); + }); + + it('handles an empty day', () => { + expect(chronoOrder([], () => null)).toEqual([]); + }); +}); diff --git a/shared/src/day/chrono-order.ts b/shared/src/day/chrono-order.ts new file mode 100644 index 000000000..7a0c7975e --- /dev/null +++ b/shared/src/day/chrono-order.ts @@ -0,0 +1,30 @@ +/** + * The order of a day plan by time. An item with a time sorts by it. An item without + * one inherits the time of the timed item before it, so it stays where it was put: + * behind the stop it followed, or at the front if nothing timed comes before it. + * Ties keep the incoming order. + * + * Shared so both sides apply one rule. The server persists this order when a stop's + * start changes, and the client applies it again on every render. The server used to + * keep its own copy that put every untimed stop at the end of the day, so one start + * time pulled a stop to the top of a day it had been planned at the bottom of. + * + * The rule is the same, the input is not. The server sorts the day's stops alone; the + * client sorts the day list, with its notes and bookings between the stops. An untimed + * stop behind a timed note inherits the note's time on the client and the previous + * stop's time on the server, so on such a day the two orders can differ. + * + * `minutesOf` is where the two sides differ as well: what counts as a time, and which + * one. It returns null (or undefined) for an item without a time. + */ +export function chronoOrder(items: readonly T[], minutesOf: (item: T) => number | null | undefined): T[] { + let last = -Infinity; + return items + .map((item, index) => { + const minutes = minutesOf(item); + if (minutes !== null && minutes !== undefined) last = minutes; + return { item, index, at: minutes ?? last }; + }) + .sort((a, b) => a.at - b.at || a.index - b.index) + .map((entry) => entry.item); +} diff --git a/shared/src/index.ts b/shared/src/index.ts index c7c005278..06f12de60 100644 --- a/shared/src/index.ts +++ b/shared/src/index.ts @@ -37,6 +37,7 @@ export * from './dawarich/dawarich.schema'; export * from './docsync/docsync.schema'; export * from './day/day.schema'; export * from './day/note-colors'; +export * from './day/chrono-order'; export * from './assignment/assignment.schema'; export * from './place/place.schema'; export * from './place/place-match';