diff --git a/client/src/store/journeyStore.test.ts b/client/src/store/journeyStore.test.ts index 564969ec..d5c7a2a3 100644 --- a/client/src/store/journeyStore.test.ts +++ b/client/src/store/journeyStore.test.ts @@ -355,6 +355,37 @@ describe('journeyStore', () => { expect(useJourneyStore.getState().loading).toBe(false); }); + // ── reorderEntries ─────────────────────────────────────────────────────── + + it('FE-STORE-JOURNEY-018: reorderEntries reorders by sort_order not entry_time', async () => { + const a = buildEntry({ id: 201, entry_date: '2026-04-01', entry_time: '09:00', sort_order: 0 }); + const b = buildEntry({ id: 202, entry_date: '2026-04-01', entry_time: '11:00', sort_order: 1 }); + const c = buildEntry({ id: 203, entry_date: '2026-04-01', entry_time: '14:00', sort_order: 2 }); + const detail = buildJourneyDetail({ id: 55, entries: [a, b, c] }); + useJourneyStore.setState({ current: detail }); + + server.use( + http.put('/api/journeys/55/entries/reorder', () => HttpResponse.json({ success: true })) + ); + await useJourneyStore.getState().reorderEntries(55, [202, 201, 203]); + const ids = useJourneyStore.getState().current?.entries.map(e => e.id); + expect(ids).toEqual([202, 201, 203]); + }); + + it('FE-STORE-JOURNEY-019: reorderEntries rolls back on API failure', async () => { + const a = buildEntry({ id: 211, entry_date: '2026-04-01', sort_order: 0 }); + const b = buildEntry({ id: 212, entry_date: '2026-04-01', sort_order: 1 }); + const detail = buildJourneyDetail({ id: 56, entries: [a, b] }); + useJourneyStore.setState({ current: detail }); + + server.use( + http.put('/api/journeys/56/entries/reorder', () => HttpResponse.json({}, { status: 403 })) + ); + await expect(useJourneyStore.getState().reorderEntries(56, [212, 211])).rejects.toBeTruthy(); + const ids = useJourneyStore.getState().current?.entries.map(e => e.id); + expect(ids).toEqual([211, 212]); + }); + // ── clear ──────────────────────────────────────────────────────────────── it('FE-STORE-JOURNEY-015: clear resets state', () => { diff --git a/client/src/store/journeyStore.ts b/client/src/store/journeyStore.ts index 47b75971..c2edfa69 100644 --- a/client/src/store/journeyStore.ts +++ b/client/src/store/journeyStore.ts @@ -223,10 +223,8 @@ export const useJourneyStore = create((set, get) => ({ ) entries.sort((a, b) => { if (a.entry_date !== b.entry_date) return a.entry_date.localeCompare(b.entry_date) - const atime = a.entry_time || '' - const btime = b.entry_time || '' - if (atime !== btime) return atime.localeCompare(btime) - return (a.sort_order || 0) - (b.sort_order || 0) + if (a.sort_order !== b.sort_order) return (a.sort_order || 0) - (b.sort_order || 0) + return a.id - b.id }) return { current: { ...s.current, entries } } }) diff --git a/server/src/db/migrations.ts b/server/src/db/migrations.ts index 46f48e8c..29640339 100644 --- a/server/src/db/migrations.ts +++ b/server/src/db/migrations.ts @@ -2107,6 +2107,29 @@ function runMigrations(db: Database.Database): void { != substr(reservations.reservation_time, 1, 10) `); }, + // #846: make sort_order authoritative within a day. Previous ORDER BY put + // entry_time before sort_order, silently ignoring reorder clicks when two + // same-date entries had different times. Backfill renumbers using the old + // effective key (entry_time ASC, id ASC) so existing journeys retain their + // current visual order. + () => { + db.exec(` + WITH ranked AS ( + SELECT id, + ROW_NUMBER() OVER ( + PARTITION BY journey_id, entry_date + ORDER BY entry_time ASC, id ASC + ) - 1 AS rn + FROM journey_entries + ) + UPDATE journey_entries + SET sort_order = (SELECT rn FROM ranked WHERE ranked.id = journey_entries.id) + `); + db.exec( + 'CREATE INDEX IF NOT EXISTS idx_journey_entries_order ' + + 'ON journey_entries(journey_id, entry_date, sort_order)' + ); + }, ]; if (currentVersion < migrations.length) { diff --git a/server/src/services/journeyService.ts b/server/src/services/journeyService.ts index a03d1fea..afa6c7b3 100644 --- a/server/src/services/journeyService.ts +++ b/server/src/services/journeyService.ts @@ -120,7 +120,7 @@ export function getJourneyFull(journeyId: number, userId: number) { if (!journey) return null; const entries = db.prepare( - 'SELECT * FROM journey_entries WHERE journey_id = ? ORDER BY entry_date ASC, entry_time ASC, sort_order ASC' + 'SELECT * FROM journey_entries WHERE journey_id = ? ORDER BY entry_date ASC, sort_order ASC, id ASC' ).all(journeyId) as JourneyEntry[]; const photos = db.prepare( @@ -306,12 +306,21 @@ export function syncTripPlaces(journeyId: number, tripId: number, authorId: numb ).all(journeyId, tripId) as { source_place_id: number }[]; const existingPlaceIds = new Set(existing.map(e => e.source_place_id)); + // Track next sort_order per date so synced skeletons get unique, sequential positions. + const dateMaxOrder = new Map(); + const maxRows = db.prepare( + 'SELECT entry_date, COALESCE(MAX(sort_order), -1) AS m FROM journey_entries WHERE journey_id = ? GROUP BY entry_date' + ).all(journeyId) as { entry_date: string; m: number }[]; + for (const row of maxRows) dateMaxOrder.set(row.entry_date, row.m); + for (const place of places) { if (existingPlaceIds.has(place.id)) continue; existingPlaceIds.add(place.id); const entryDate = place.day_date || new Date().toISOString().split('T')[0]; const entryTime = place.assignment_time || place.place_time || null; + const nextOrder = (dateMaxOrder.get(entryDate) ?? -1) + 1; + dateMaxOrder.set(entryDate, nextOrder); db.prepare(` INSERT INTO journey_entries (journey_id, source_trip_id, source_place_id, author_id, type, title, entry_date, entry_time, location_name, location_lat, location_lng, sort_order, created_at, updated_at) @@ -320,7 +329,7 @@ export function syncTripPlaces(journeyId: number, tripId: number, authorId: numb journeyId, tripId, place.id, authorId, place.name, entryDate, entryTime, place.address || place.name, place.lat || null, place.lng || null, - place.day_number || 0, now, now + nextOrder, now, now ); } } @@ -367,15 +376,19 @@ export function onPlaceCreated(tripId: number, placeId: number) { const journey = db.prepare('SELECT user_id FROM journeys WHERE id = ?').get(link.journey_id) as { user_id: number }; const entryDate = place.day_date; + const maxOrder = db.prepare( + 'SELECT MAX(sort_order) AS m FROM journey_entries WHERE journey_id = ? AND entry_date = ?' + ).get(link.journey_id, entryDate) as { m: number | null }; + const nextOrder = (maxOrder?.m ?? -1) + 1; db.prepare(` INSERT INTO journey_entries (journey_id, source_trip_id, source_place_id, author_id, type, title, entry_date, entry_time, location_name, location_lat, location_lng, sort_order, created_at, updated_at) - VALUES (?, ?, ?, ?, 'skeleton', ?, ?, ?, ?, ?, ?, 0, ?, ?) + VALUES (?, ?, ?, ?, 'skeleton', ?, ?, ?, ?, ?, ?, ?, ?, ?) `).run( link.journey_id, tripId, placeId, journey.user_id, place.name, entryDate, place.assignment_time || place.place_time || null, place.address || place.name, place.lat || null, place.lng || null, - now, now + nextOrder, now, now ); } } @@ -451,7 +464,7 @@ export function listEntries(journeyId: number, userId: number) { if (!canAccessJourney(journeyId, userId)) return null; const entries = db.prepare( - 'SELECT * FROM journey_entries WHERE journey_id = ? ORDER BY entry_date ASC, entry_time ASC, sort_order ASC' + 'SELECT * FROM journey_entries WHERE journey_id = ? ORDER BY entry_date ASC, sort_order ASC, id ASC' ).all(journeyId) as JourneyEntry[]; const photos = db.prepare( diff --git a/server/tests/unit/services/journeyService.test.ts b/server/tests/unit/services/journeyService.test.ts index 950eff04..82b14d55 100644 --- a/server/tests/unit/services/journeyService.test.ts +++ b/server/tests/unit/services/journeyService.test.ts @@ -68,6 +68,7 @@ import { removeContributor, getSuggestions, syncTripPlaces, + reorderEntries, onPlaceCreated, onPlaceUpdated, onPlaceDeleted, @@ -1465,3 +1466,108 @@ describe('addProviderPhoto — passphrase', () => { expect(row?.passphrase).not.toBe('secret-pp'); }); }); + +// -- reorderEntries (#846) ---------------------------------------------------- + +function insertEntry(journeyId: number, authorId: number, opts: { entry_date: string; entry_time?: string | null; sort_order?: number }): { id: number } { + const now = Date.now(); + const res = testDb.prepare(` + INSERT INTO journey_entries (journey_id, author_id, type, entry_date, entry_time, sort_order, visibility, created_at, updated_at) + VALUES (?, ?, 'entry', ?, ?, ?, 'private', ?, ?) + `).run(journeyId, authorId, opts.entry_date, opts.entry_time ?? null, opts.sort_order ?? 0, now, now); + return { id: Number(res.lastInsertRowid) }; +} + +describe('reorderEntries', () => { + it('JOURNEY-SVC-089: reorder persists and listEntries returns requested order regardless of entry_time', () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + const e1 = insertEntry(journey.id, user.id, { entry_date: '2026-08-01', entry_time: '09:00', sort_order: 0 }); + const e2 = insertEntry(journey.id, user.id, { entry_date: '2026-08-01', entry_time: '14:00', sort_order: 1 }); + + const ok = reorderEntries(journey.id, user.id, [e2.id, e1.id]); + expect(ok).toBe(true); + + const entries = listEntries(journey.id, user.id)!; + const dayEntries = entries.filter(e => e.entry_date === '2026-08-01'); + expect(dayEntries.map(e => e.id)).toEqual([e2.id, e1.id]); + }); + + it('JOURNEY-SVC-090: reorderEntries rejects ids from another journey', () => { + const { user } = createUser(testDb); + const j1 = createJourney(testDb, user.id); + const j2 = createJourney(testDb, user.id); + const entry = createJourneyEntry(testDb, j2.id, user.id, { entry_date: '2026-08-02' }); + + const ok = reorderEntries(j1.id, user.id, [entry.id]); + expect(ok).toBe(false); + }); + + it('JOURNEY-SVC-091: reorderEntries does not affect entries on other days', () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + const day1a = insertEntry(journey.id, user.id, { entry_date: '2026-08-01', sort_order: 0 }); + const day1b = insertEntry(journey.id, user.id, { entry_date: '2026-08-01', sort_order: 1 }); + const day2 = insertEntry(journey.id, user.id, { entry_date: '2026-08-02', sort_order: 0 }); + + reorderEntries(journey.id, user.id, [day1b.id, day1a.id]); + + const entries = listEntries(journey.id, user.id)!; + const day2Entry = entries.find(e => e.id === day2.id)!; + expect(day2Entry.sort_order).toBe(0); + }); +}); + +describe('syncTripPlaces sort_order', () => { + it('JOURNEY-SVC-092: assigns unique sequential sort_order per date for same-day places', () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + const trip = createTrip(testDb, user.id, { + title: 'Order Trip', + start_date: '2026-09-01', + end_date: '2026-09-02', + }); + const day = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY date ASC LIMIT 1').get(trip.id) as { id: number }; + const p1 = createPlace(testDb, trip.id, { name: 'Place A' }); + const p2 = createPlace(testDb, trip.id, { name: 'Place B' }); + const p3 = createPlace(testDb, trip.id, { name: 'Place C' }); + createDayAssignment(testDb, day.id, p1.id); + createDayAssignment(testDb, day.id, p2.id); + createDayAssignment(testDb, day.id, p3.id); + + syncTripPlaces(journey.id, trip.id, user.id); + + const rows = testDb.prepare( + 'SELECT sort_order FROM journey_entries WHERE journey_id = ? ORDER BY sort_order ASC' + ).all(journey.id) as { sort_order: number }[]; + const orders = rows.map(r => r.sort_order); + expect(new Set(orders).size).toBe(orders.length); + expect(orders).toEqual([0, 1, 2]); + }); +}); + +describe('onPlaceCreated sort_order', () => { + it('JOURNEY-SVC-093: assigns MAX+1 sort_order when entries already exist on the target date', () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + const trip = createTrip(testDb, user.id, { + title: 'Append Trip', + start_date: '2026-10-01', + end_date: '2026-10-02', + }); + addTripToJourney(journey.id, trip.id, user.id); + + const day = testDb.prepare('SELECT id, date FROM days WHERE trip_id = ? ORDER BY date ASC LIMIT 1').get(trip.id) as { id: number; date: string }; + insertEntry(journey.id, user.id, { entry_date: day.date, sort_order: 5 }); + + const place = createPlace(testDb, trip.id, { name: 'Late Addition' }); + createDayAssignment(testDb, day.id, place.id); + onPlaceCreated(trip.id, place.id); + + const newEntry = testDb.prepare( + 'SELECT sort_order FROM journey_entries WHERE journey_id = ? AND source_place_id = ?' + ).get(journey.id, place.id) as { sort_order: number } | undefined; + expect(newEntry).toBeDefined(); + expect(newEntry!.sort_order).toBe(6); + }); +});