From f7f6b83d33c6657ffafb6f92d5f5b3553ae11ee5 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 18 Oct 2026 13:06:31 +0200 Subject: [PATCH] refactor(journey,docsync): keep both services inside their size baseline The time placement of a journey entry moves whole into journey-entry-order, and a moved entry only gets a sort_order when it actually moves, so a patch never carries an undefined one. The Synology login comment shrinks to a line. --- .../providers/synology-drive.client.ts | 7 +-- .../nest/journey/journey-domain.service.ts | 18 ++----- .../src/nest/journey/journey-entry-order.ts | 30 +++++++++++ .../unit/nest/journey-entry-order.test.ts | 51 ++++++++++++++++++- 4 files changed, 86 insertions(+), 20 deletions(-) diff --git a/server/src/nest/doc-sync/providers/synology-drive.client.ts b/server/src/nest/doc-sync/providers/synology-drive.client.ts index 60008dcac..5ae50029a 100644 --- a/server/src/nest/doc-sync/providers/synology-drive.client.ts +++ b/server/src/nest/doc-sync/providers/synology-drive.client.ts @@ -780,11 +780,8 @@ export class SynologyDriveClient { api: 'SYNO.API.Auth', version: '6', method: 'login', - // Plain, unlike the FileStation parameters below. entry.cgi reads a value - // as JSON where it can, auth.cgi takes it as it comes, so a quoted account - // logged in as `"trek"`, quotes included, and every account was refused (#2666). - account: creds.username, - passwd: creds.password, + // Plain: auth.cgi reads no JSON, a quoted account was refused as `"trek"` (#2666). + ...{ account: creds.username, passwd: creds.password }, session: SESSION_NAME, format: 'sid', client: 'browser', diff --git a/server/src/nest/journey/journey-domain.service.ts b/server/src/nest/journey/journey-domain.service.ts index caeab1633..3987d329a 100644 --- a/server/src/nest/journey/journey-domain.service.ts +++ b/server/src/nest/journey/journey-domain.service.ts @@ -10,7 +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 { newEntrySortOrder, placeMovedEntry } 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'; @@ -1210,13 +1210,8 @@ export class JourneyDomainService { ? JSON.stringify(data.pros_cons) : null; - // 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). + // Position and row in one transaction, so two adds cannot read the same MAX (JG74, #2679). const insertedId = await this.uow.transactional(async () => { - 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, @@ -1234,7 +1229,7 @@ export class JourneyDomainService { tags: data.tags?.length ? JSON.stringify(data.tags) : null, pros_cons: prosConsJson, visibility: data.visibility || 'private', - sort_order: sortOrder, + sort_order: await newEntrySortOrder(this.entriesRepo, journeyId, data.entry_date, data.entry_time, now), is_draft: data.is_draft ? 1 : 0, created_at: now, updated_at: now, @@ -1359,13 +1354,8 @@ 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 placeMovedEntry(this.entriesRepo, entry, patch, this.ts()); 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 index 976d201b8..6bd786dc2 100644 --- a/server/src/nest/journey/journey-entry-order.ts +++ b/server/src/nest/journey/journey-entry-order.ts @@ -46,3 +46,33 @@ export async function makeRoomAtTime( await Promise.all(moves.map((m) => entries.updateSortOrder(m.id, m.to, now))); return slot; } + +/** The sort_order a new entry takes: where its time puts it in its day, last when it has none. */ +export async function newEntrySortOrder( + entries: JourneyEntriesRepository, + journeyId: number, + date: string, + time: string | null | undefined, + now: number, +): Promise { + if (time) return await makeRoomAtTime(entries, journeyId, date, time, now); + return ((await entries.maxSortOrderForDate(journeyId, date)) ?? -1) + 1; +} + +/** + * Moves an edited entry to where its new time or day puts it, by giving the patch + * the sort_order it takes. Left alone when neither changed, so a save keeps an + * entry where it was dragged, and when the caller sent a sort_order of its own. + */ +export async function placeMovedEntry( + entries: JourneyEntriesRepository, + entry: { id: number; journey_id: number; entry_date: string; entry_time?: string | null }, + patch: { entry_date?: string; entry_time?: string | null; sort_order?: number }, + now: number, +): Promise { + if (patch.sort_order !== undefined) return; + const date = patch.entry_date ?? entry.entry_date; + const time = patch.entry_time !== undefined ? patch.entry_time : entry.entry_time; + if (!time || (date === entry.entry_date && time === entry.entry_time)) return; + patch.sort_order = await makeRoomAtTime(entries, entry.journey_id, date, time, now, entry.id); +} diff --git a/server/tests/unit/nest/journey-entry-order.test.ts b/server/tests/unit/nest/journey-entry-order.test.ts index fb9a89fe7..0c3c87143 100644 --- a/server/tests/unit/nest/journey-entry-order.test.ts +++ b/server/tests/unit/nest/journey-entry-order.test.ts @@ -1,5 +1,10 @@ import type { JourneyEntriesRepository } from '../../../src/db/repositories/JourneyEntries.repository'; -import { chronologicalSlot, makeRoomAtTime } from '../../../src/nest/journey/journey-entry-order'; +import { + chronologicalSlot, + makeRoomAtTime, + newEntrySortOrder, + placeMovedEntry, +} from '../../../src/nest/journey/journey-entry-order'; import { describe, it, expect, vi } from 'vitest'; @@ -66,3 +71,47 @@ describe('makeRoomAtTime (#2679)', () => { expect(updateSortOrder).not.toHaveBeenCalled(); }); }); + +describe('newEntrySortOrder / placeMovedEntry (#2679)', () => { + const day = [ + { id: 1, entry_time: '08:00', sort_order: 0 }, + { id: 2, entry_time: '19:00', sort_order: 1 }, + ]; + function repo() { + const updateSortOrder = vi.fn(async () => {}); + const entries = { + listDayOrder: vi.fn(async () => day), + updateSortOrder, + maxSortOrderForDate: vi.fn(async () => 4), + } as unknown as JourneyEntriesRepository; + return { entries, updateSortOrder }; + } + const entry = { id: 2, journey_id: 7, entry_date: '2026-04-01', entry_time: '19:00' }; + + it('puts a timed entry by its time and an untimed one last', async () => { + expect(await newEntrySortOrder(repo().entries, 7, '2026-04-01', '12:00', 1)).toBe(1); + expect(await newEntrySortOrder(repo().entries, 7, '2026-04-01', null, 1)).toBe(5); + }); + + it('moves an entry whose time changed', async () => { + const patch: { entry_time?: string; sort_order?: number } = { entry_time: '07:00' }; + await placeMovedEntry(repo().entries, entry, patch, 1); + expect(patch.sort_order).toBe(0); + }); + + it('leaves the patch alone for an unchanged time, no time, or a sort_order of its own', async () => { + const same: { entry_time?: string; sort_order?: number } = { entry_time: '19:00' }; + await placeMovedEntry(repo().entries, entry, same, 1); + expect('sort_order' in same).toBe(false); + + const cleared: { entry_time?: string | null; sort_order?: number } = { entry_time: null }; + await placeMovedEntry(repo().entries, entry, cleared, 1); + expect('sort_order' in cleared).toBe(false); + + const placed = { entry_time: '07:00', sort_order: 3 }; + const { entries, updateSortOrder } = repo(); + await placeMovedEntry(entries, entry, placed, 1); + expect(placed.sort_order).toBe(3); + expect(updateSortOrder).not.toHaveBeenCalled(); + }); +});