diff --git a/server/src/db/repositories/JourneyEntries.repository.ts b/server/src/db/repositories/JourneyEntries.repository.ts index 5c745aa47..02da045cd 100644 --- a/server/src/db/repositories/JourneyEntries.repository.ts +++ b/server/src/db/repositories/JourneyEntries.repository.ts @@ -241,6 +241,21 @@ export class JourneyEntriesRepository extends TrekRepository { return rows as { entry_date: string; m: number }[]; } + /** One day's entries in timeline order, with what placing a timed entry among them reads (#2679). */ + async listDayOrder( + journeyId: number, + entryDate: string, + ): Promise<{ id: number; entry_time: string | null; sort_order: number | null }[]> { + return await this.kysely() + .selectFrom('journey_entries') + .select(['id', 'entry_time', 'sort_order']) + .where('journey_id', '=', journeyId) + .where('entry_date', '=', entryDate) + .orderBy('sort_order', 'asc') + .orderBy('id', 'asc') + .execute(); + } + /** JG42 — `onPlaceCreated`'s next-order lookup: `SELECT MAX(sort_order) AS m FROM journey_entries WHERE journey_id = ? AND entry_date = ?`. The `?? -1` fallback stays the SERVICE's, unchanged (same text as Task 2's own JG74). */ async maxSortOrderForDate(journeyId: number, entryDate: string): Promise { const row = await this.kysely() diff --git a/server/src/nest/journey/journey-domain.service.ts b/server/src/nest/journey/journey-domain.service.ts index d5cacea67..caeab1633 100644 --- a/server/src/nest/journey/journey-domain.service.ts +++ b/server/src/nest/journey/journey-domain.service.ts @@ -10,6 +10,7 @@ import { todayUtc } from '@trek/shared'; import { TrekPhotoRegistrationService } from '../photos/trek-photo-registration.service'; import { getCountryFromCoords } from '../atlas/atlas-geo'; import { computeJourneyStats, type StatsInputPoint } from './journey-stats'; +import { makeRoomAtTime } from './journey-entry-order'; import { Journeys } from '../../db/entities/Journeys.entity'; import type { JourneysRepository } from '../../db/repositories/Journeys.repository'; import { JourneyContributors } from '../../db/entities/JourneyContributors.entity'; @@ -1209,10 +1210,13 @@ export class JourneyDomainService { ? JSON.stringify(data.pros_cons) : null; - // The next position and the row that takes it in one transaction, so two - // entries added at once cannot both read the same MAX (JG74). + // The position and the row that takes it in one transaction, so two entries + // added at once cannot both read the same MAX (JG74). A timed entry goes where + // its time puts it in the day, an untimed one last (#2679). const insertedId = await this.uow.transactional(async () => { - const maxOrder = await this.entriesRepo.maxSortOrderForDate(journeyId, data.entry_date); + const sortOrder = data.entry_time + ? await makeRoomAtTime(this.entriesRepo, journeyId, data.entry_date, data.entry_time, now) + : ((await this.entriesRepo.maxSortOrderForDate(journeyId, data.entry_date)) ?? -1) + 1; return await this.entriesRepo.insertEntry({ journey_id: journeyId, author_id: userId, @@ -1230,7 +1234,7 @@ export class JourneyDomainService { tags: data.tags?.length ? JSON.stringify(data.tags) : null, pros_cons: prosConsJson, visibility: data.visibility || 'private', - sort_order: (maxOrder ?? -1) + 1, + sort_order: sortOrder, is_draft: data.is_draft ? 1 : 0, created_at: now, updated_at: now, @@ -1355,7 +1359,13 @@ export class JourneyDomainService { // was provided — the same condition the legacy field-count check tested. if (Object.keys(patch).length === 1) return decodeEntryRow(entry); + // A new time or day moves the entry to where it belongs in that day, unless the + // caller placed it itself (#2679). + const date = patch.entry_date ?? entry.entry_date; + const time = patch.entry_time !== undefined ? patch.entry_time : entry.entry_time; + const placeAt = patch.sort_order === undefined && (date !== entry.entry_date || time !== entry.entry_time) ? time : null; await this.uow.transactional(async () => { + if (placeAt) patch.sort_order = await makeRoomAtTime(this.entriesRepo, entry.journey_id, date, placeAt, this.ts(), entryId); await this.entriesRepo.updateFields(entryId, patch); await this.journeysRepo.updateFields(entry.journey_id, { updated_at: this.ts() }); // touch the journey }); diff --git a/server/src/nest/journey/journey-entry-order.ts b/server/src/nest/journey/journey-entry-order.ts new file mode 100644 index 000000000..976d201b8 --- /dev/null +++ b/server/src/nest/journey/journey-entry-order.ts @@ -0,0 +1,48 @@ +import type { JourneyEntriesRepository } from '../../db/repositories/JourneyEntries.repository'; + +/** "HH:MM" (seconds ignored) as minutes past midnight, or null for anything else. */ +function minutesOf(time: string | null | undefined): number | null { + const match = /^(\d{1,2}):(\d{2})/.exec(time?.trim() ?? ''); + return match ? Number(match[1]) * 60 + Number(match[2]) : null; +} + +/** + * Where an entry at `time` belongs among its day's entries, given in their + * current order: right before the first one that starts later. Entries + * without a time are no anchor either way, and an unreadable `time` goes last. + */ +export function chronologicalSlot(day: readonly { entry_time: string | null }[], time: string): number { + const at = minutesOf(time); + if (at == null) return day.length; + const later = day.findIndex((e) => { + const other = minutesOf(e.entry_time); + return other != null && other > at; + }); + return later === -1 ? day.length : later; +} + +/** + * Makes room for a timed entry in its day and returns the sort_order it takes. + * + * A new entry used to go to the end of its day whatever its time, so writing + * up the morning after the evening left the morning last (#2679). It now lands + * before the first entry that starts later; the order of everything else, + * dragged by hand or not, stays as it was. Run it inside the write's + * transaction, so the room and the entry that takes it land together. + */ +export async function makeRoomAtTime( + entries: JourneyEntriesRepository, + journeyId: number, + date: string, + time: string, + now: number, + exceptId?: number, +): Promise { + const day = (await entries.listDayOrder(journeyId, date)).filter((e) => e.id !== exceptId); + const slot = chronologicalSlot(day, time); + const moves = day + .map((e, index) => ({ id: e.id, from: e.sort_order, to: index < slot ? index : index + 1 })) + .filter((m) => m.from !== m.to); + await Promise.all(moves.map((m) => entries.updateSortOrder(m.id, m.to, now))); + return slot; +} diff --git a/server/tests/integration/journey.test.ts b/server/tests/integration/journey.test.ts index 416fa1ff2..048cbaebe 100644 --- a/server/tests/integration/journey.test.ts +++ b/server/tests/integration/journey.test.ts @@ -261,6 +261,59 @@ describe('Journey entries', () => { expect(res.body.story).toBe('Now with a story'); }); + it('JOURNEY-INT-010b — a timed entry goes where its time puts it in the day, an untimed one last (#2679)', async () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + const post = (title: string, entry_time?: string) => + request(app) + .post(`/api/journeys/${journey.id}/entries`) + .set('Cookie', authCookie(user.id)) + .send({ title, entry_date: '2026-04-01', ...(entry_time ? { entry_time } : {}) }); + + await post('two', '02:00'); + await post('untimed'); + await post('one', '01:00'); + await post('three', '03:00'); + + const res = await request(app).get(`/api/journeys/${journey.id}`).set('Cookie', authCookie(user.id)); + const titles = res.body.entries.map((e: { title: string }) => e.title); + expect(titles).toEqual(['one', 'two', 'untimed', 'three']); + }); + + it('JOURNEY-INT-011b — a new time moves the entry, an unchanged one keeps a hand-made order (#2679)', async () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + const post = async (title: string, entry_time: string) => + ( + await request(app) + .post(`/api/journeys/${journey.id}/entries`) + .set('Cookie', authCookie(user.id)) + .send({ title, entry_date: '2026-04-01', entry_time }) + ).body.id as number; + const ids = [await post('morning', '08:00'), await post('noon', '12:00'), await post('evening', '19:00')]; + const titles = async () => + (await request(app).get(`/api/journeys/${journey.id}`).set('Cookie', authCookie(user.id))).body.entries.map( + (e: { title: string }) => e.title, + ); + + await request(app) + .patch(`/api/journeys/entries/${ids[2]}`) + .set('Cookie', authCookie(user.id)) + .send({ entry_time: '07:00' }); + expect(await titles()).toEqual(['evening', 'morning', 'noon']); + + // Dragged out of time order, then saved again with the same time: it stays put. + await request(app) + .put(`/api/journeys/${journey.id}/entries/reorder`) + .set('Cookie', authCookie(user.id)) + .send({ orderedIds: [ids[0], ids[1], ids[2]] }); + await request(app) + .patch(`/api/journeys/entries/${ids[2]}`) + .set('Cookie', authCookie(user.id)) + .send({ title: 'early', entry_time: '07:00' }); + expect(await titles()).toEqual(['morning', 'noon', 'early']); + }); + it('JOURNEY-INT-012 — DELETE /api/journeys/entries/:id deletes entry', async () => { const { user } = createUser(testDb); const journey = createJourney(testDb, user.id); diff --git a/server/tests/unit/nest/journey-entry-order.test.ts b/server/tests/unit/nest/journey-entry-order.test.ts new file mode 100644 index 000000000..fb9a89fe7 --- /dev/null +++ b/server/tests/unit/nest/journey-entry-order.test.ts @@ -0,0 +1,68 @@ +import type { JourneyEntriesRepository } from '../../../src/db/repositories/JourneyEntries.repository'; +import { chronologicalSlot, makeRoomAtTime } from '../../../src/nest/journey/journey-entry-order'; + +import { describe, it, expect, vi } from 'vitest'; + +const at = (...times: (string | null)[]) => times.map((entry_time) => ({ entry_time })); + +describe('chronologicalSlot (#2679)', () => { + it('goes before the first entry that starts later', () => { + expect(chronologicalSlot(at('02:00'), '01:00')).toBe(0); + expect(chronologicalSlot(at('01:00', '03:00'), '02:00')).toBe(1); + expect(chronologicalSlot(at('01:00', '02:00'), '05:00')).toBe(2); + }); + + it('keeps an equal time after the one already there', () => { + expect(chronologicalSlot(at('09:00', '10:00'), '09:00')).toBe(1); + }); + + it('passes over entries without a time instead of anchoring on them', () => { + expect(chronologicalSlot(at(null, '08:00', null), '07:00')).toBe(1); + expect(chronologicalSlot(at(null, null), '07:00')).toBe(2); + }); + + it('reads seconds and single-digit hours, and puts an unreadable time last', () => { + expect(chronologicalSlot(at('9:30:00', '10:00:00'), '09:45')).toBe(1); + expect(chronologicalSlot(at('01:00'), 'soon')).toBe(1); + }); +}); + +describe('makeRoomAtTime (#2679)', () => { + function repo(day: { id: number; entry_time: string | null; sort_order: number | null }[]) { + const updateSortOrder = vi.fn(async () => {}); + const listDayOrder = vi.fn(async () => day); + return { entries: { listDayOrder, updateSortOrder } as unknown as JourneyEntriesRepository, updateSortOrder }; + } + + it('moves only the entries after the slot, and returns the slot', async () => { + const { entries, updateSortOrder } = repo([ + { id: 1, entry_time: '08:00', sort_order: 0 }, + { id: 2, entry_time: '12:00', sort_order: 1 }, + { id: 3, entry_time: '19:00', sort_order: 2 }, + ]); + expect(await makeRoomAtTime(entries, 7, '2026-04-01', '10:00', 99)).toBe(1); + expect(updateSortOrder.mock.calls).toEqual([ + [2, 2, 99], + [3, 3, 99], + ]); + }); + + it('leaves the moved entry itself out and closes the gap it leaves', async () => { + const { entries, updateSortOrder } = repo([ + { id: 1, entry_time: '08:00', sort_order: 0 }, + { id: 2, entry_time: '12:00', sort_order: 1 }, + { id: 3, entry_time: '19:00', sort_order: 2 }, + ]); + expect(await makeRoomAtTime(entries, 7, '2026-04-01', '07:00', 99, 3)).toBe(0); + expect(updateSortOrder.mock.calls).toEqual([ + [1, 1, 99], + [2, 2, 99], + ]); + }); + + it('writes nothing when the entry goes last on an evenly numbered day', async () => { + const { entries, updateSortOrder } = repo([{ id: 1, entry_time: '08:00', sort_order: 0 }]); + expect(await makeRoomAtTime(entries, 7, '2026-04-01', '09:00', 99)).toBe(1); + expect(updateSortOrder).not.toHaveBeenCalled(); + }); +});