mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(journey): a timed entry takes its place in the day by its time
A new entry always went to the end of its day, so writing up 1 am after 2 am left the night out of order, and changing an entry's time never moved it. A timed entry now lands before the first entry of its day that starts later, on create and when its time or day changes; untimed entries still go last, and the hand-made order of the others stays as it was. A save that leaves time and day untouched keeps an entry where it was dragged. Closes #2679
This commit is contained in:
@@ -241,6 +241,21 @@ export class JourneyEntriesRepository extends TrekRepository<JourneyEntries> {
|
||||
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<JourneyEntriesDateSortKyselyDB>()
|
||||
.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<number | null> {
|
||||
const row = await this.kysely<JourneyEntriesDateSortKyselyDB>()
|
||||
|
||||
@@ -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
|
||||
});
|
||||
|
||||
@@ -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<number> {
|
||||
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;
|
||||
}
|
||||
@@ -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);
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user