feat(dayplan): a day can be deleted from the reorder dialog, after a warning that says what goes with it

Every row of the reorder dialog, and of the days sheet on a phone, now has a delete button. It first asks, and the question lists what goes with the day: planned places stay in the place list, notes and day texts are deleted, bookings stay without a day, a stay that checks in or out on the day is cancelled with its booking and expense (shown in the danger tone), and on a dated trip the later days and their bookings move one date earlier or the trip ends a day earlier.

The server side moved from a bare DELETE into DayRemovalService, which REST, the delete_day tool and the plugin RPC all use. It closes the numbering, keeps the dates on their positions like a reorder, re-stamps the moved bookings, shifts the road trip boundaries, cancels touching stays through the accommodations domain, refuses the last day of a trip, and fans out day:deleted, day:reordered and trip:updated so collaborators renumber and see the new day count and end date. The trip query moved into a leaf file so the days domain can read it without a module cycle.

On the client the delete is online only, runs optimistically through the days slice with the same reducer collaborators apply, and the ConfirmDialog gained a slot for extra content, an Escape that only closes itself, and the danger and accent tokens.
This commit is contained in:
Maurice
2026-09-23 18:42:02 +02:00
parent 436f0740fb
commit 2f6f921aef
78 changed files with 3086 additions and 170 deletions
+233
View File
@@ -0,0 +1,233 @@
import { Injectable } from '@nestjs/common';
import type { RoadtripDayBoundary } from '@trek/shared';
import { DatabaseService } from '../database/database.service';
import { AccommodationsService, type AccommodationMirror, type MirrorSender } from '../accommodations/accommodations.service';
import { AssignmentsService } from '../assignments/assignments.service';
import { DaysService } from './days.service';
/** A day that has to stay where it is; REST answers 400, MCP a tool error, a plugin BadParams. */
export class DayDeleteError extends Error {}
/** The refusal every surface gives for the last day of a trip, word for word. */
export const LAST_DAY_MESSAGE = 'A trip needs at least one day.';
/** What deleting one day did to the trip, for the surface that announces it. */
export interface DayRemoval {
dayId: number;
/** The days that are left, in their new order. Their day_number is their position now. */
orderedIds: number[];
/** Stays that checked in or out on the day. They are cancelled with it. */
stayIds: number[];
/** The bookings those stays brought, deleted with them. */
reservationIds: number[];
/** The expenses written against those bookings. */
budgetItemIds: number[];
/** What cancelling each stay did to the day plan of the days that are left. */
mirrors: AccommodationMirror[];
/** The road trip day boundaries after they moved up with their days, or null when none changed. */
boundaries: RoadtripDayBoundary[] | null;
/** The trip's new last date when the day took it along, or null when the range stayed. */
endDate: string | null;
/** The trip re-read in list shape: its day count changed, and maybe its end date. */
trip: unknown;
}
/**
* How a surface sends a removal. `all` reaches every socket of the trip, the one
* that asked included; `others` leaves that one out because its own answer
* already carries the change. The MCP tools and the plugin RPC have no socket
* of their own, so they hand the same sender twice.
*/
export interface DayRemovalSenders {
all: MirrorSender;
others: MirrorSender;
socketId?: string;
}
type DayRow = { id: number; day_number: number; date: string | null };
/**
* Deleting a day, for the three surfaces that offer it (REST, MCP, plugin RPC).
*
* The delete used to be a bare DELETE: the day numbers kept a hole, the dates
* stayed where they were, a stay checking in or out on the day vanished by
* cascade while its booking and that booking's expense stayed behind pointing
* nowhere, and the journey was never told. It is the exact reverse of an insert
* now. The later days move up one place, and on a dated trip the dates stay
* pinned to their positions the way a reorder keeps them: every later day, with
* its bookings, takes the date one slot earlier. A day without a date takes the
* last date if the trip has one; otherwise the last date goes, and the trip ends
* one day earlier, which is what insert does the other way round.
*
* Its own class rather than a method on DaysService because it needs the
* accommodations and assignments services, and DaysService is built by hand in
* enough suites that a wider constructor there would ripple through all of them.
*/
@Injectable()
export class DayRemovalService {
constructor(
private readonly db: DatabaseService,
private readonly days: DaysService,
private readonly accommodations: AccommodationsService,
private readonly assignments: AssignmentsService,
) {}
/**
* Delete the day in one transaction. Nothing is written when it throws, and it
* throws a DayDeleteError for the last day of a trip. The journey catches up
* afterwards, outside the transaction, and a failure there does not undo the delete.
*/
remove(tripId: string | number, dayId: string | number, viewer: { userId: number; socketId?: string }): DayRemoval {
const trip = Number(tripId);
const id = Number(dayId);
const removal = this.db.transaction(() => {
const rows = this.db.all<DayRow>('SELECT id, day_number, date FROM days WHERE trip_id = ? ORDER BY day_number', trip);
const target = rows.find(r => r.id === id);
if (!target) throw new DayDeleteError('Day not found');
if (rows.length <= 1) throw new DayDeleteError(LAST_DAY_MESSAGE);
const cancelled = this.cancelStays(trip, id);
const boundariesBefore = this.boundaries(trip);
// The boundary drawn on this day goes with it; the day row takes its
// assignments, notes, stops, roads and booking positions along by cascade.
this.db.run('DELETE FROM roadtrip_day_boundaries WHERE trip_id = ? AND day_number = ?', trip, target.day_number);
this.db.run('DELETE FROM days WHERE id = ?', id);
this.shiftBoundaries(trip, target.day_number);
const remaining = rows.filter(r => r.id !== id);
const endDate = this.renumber(trip, rows, remaining);
const boundariesAfter = this.boundaries(trip);
const boundariesChanged = JSON.stringify(boundariesAfter) !== JSON.stringify(boundariesBefore);
return {
dayId: id,
orderedIds: remaining.map(r => r.id),
...cancelled,
boundaries: boundariesChanged ? boundariesAfter : null,
endDate,
};
});
// After the commit, the way an assignment route does it: the journey mirrors
// the planned stops, and the day's stops are gone. reconcile() swallows its own
// failures, so a journey problem cannot turn a finished delete into an error.
this.assignments.reconcile(trip, viewer.socketId);
return { ...removal, trip: this.days.getTripForViewer(trip, viewer.userId) };
}
/**
* The one place a removal is fanned out. The collaborators drop the day first
* and then renumber off day:reordered, which also makes them pull the re-dated
* days and re-stamped bookings. The cancelled stays follow with what they did to
* the other days, then the rows they took with them.
*/
announce(tripId: string | number, removal: DayRemoval, senders: DayRemovalSenders): void {
const { all, others, socketId } = senders;
others('day:deleted', { dayId: removal.dayId });
others('day:reordered', { orderedIds: removal.orderedIds });
for (const mirror of removal.mirrors) this.accommodations.announceMirror(tripId, mirror, all, socketId);
// Without a socket id, as when a place takes its nights with it: the Bookings
// list and the Costs total of the deleting tab hold these rows too.
for (const reservationId of removal.reservationIds) all('reservation:deleted', { reservationId });
for (const itemId of removal.budgetItemIds) all('budget:deleted', { itemId });
for (const accommodationId of removal.stayIds) others('accommodation:deleted', { accommodationId });
if (removal.boundaries) all('roadtripBoundary:changed', { boundaries: removal.boundaries });
// The day count changed, and the end date when the last date went: the trip
// header of every other open copy of the trip reads both.
if (removal.trip) others('trip:updated', { trip: removal.trip });
}
/**
* Cancel every stay that checks in or out on the day, the way deleting it by
* hand does: the partner booking, its expense and the stop it wrote go too.
* Left to the cascade, the stay row alone would go and the rest stay behind.
* A stay that only runs across the day keeps standing.
*/
private cancelStays(tripId: number, dayId: number) {
const stays = this.db.all<{ id: number }>(
'SELECT id FROM day_accommodations WHERE trip_id = ? AND (start_day_id = ? OR end_day_id = ?) ORDER BY id',
tripId, dayId, dayId,
);
const cancelled = { stayIds: [] as number[], reservationIds: [] as number[], budgetItemIds: [] as number[], mirrors: [] as AccommodationMirror[] };
for (const stay of stays) {
const gone = this.accommodations.deleteAccommodation(stay.id);
cancelled.stayIds.push(stay.id);
cancelled.reservationIds.push(...gone.linkedReservationIds);
cancelled.budgetItemIds.push(...gone.deletedBudgetItemIds);
cancelled.mirrors.push(withoutDay(gone.mirror, dayId));
}
return cancelled;
}
private boundaries(tripId: number): RoadtripDayBoundary[] {
return this.db.all<RoadtripDayBoundary>(
'SELECT day_number, from_assignment_id, to_assignment_id, fraction FROM roadtrip_day_boundaries WHERE trip_id = ? ORDER BY day_number',
tripId,
);
}
/**
* Boundaries are keyed by day number, not by day, so the ones after the deleted
* day move up with their days. One at a time and in ascending order: the key is
* the primary key, and a CHECK keeps it at 1 or above, so the negative two-step
* the days use is not available here.
*/
private shiftBoundaries(tripId: number, deletedNumber: number): void {
const later = this.db.all<{ day_number: number }>(
'SELECT day_number FROM roadtrip_day_boundaries WHERE trip_id = ? AND day_number > ? ORDER BY day_number',
tripId, deletedNumber,
);
const move = this.db.prepare('UPDATE roadtrip_day_boundaries SET day_number = ? WHERE trip_id = ? AND day_number = ?');
for (const row of later) move.run(row.day_number - 1, tripId, row.day_number);
}
/**
* Close the gap and keep the dates on their positions: position i takes the
* i-th date of the trip as it was, the deleted day's date included. Bookings on
* a day that changed date are re-stamped onto it. When there are fewer days left
* than dates, the last date is gone, and a dated trip ends on the new last one.
* Returns that new end date, or null when the range stayed.
*/
private renumber(tripId: number, rows: DayRow[], remaining: DayRow[]): string | null {
// ISO dates sort as plain strings.
const sortedDates = rows.map(r => r.date).filter((d): d is string => !!d).sort();
const setNumber = this.db.prepare('UPDATE days SET day_number = ? WHERE id = ?');
const setNumberAndDate = this.db.prepare('UPDATE days SET day_number = ?, date = ? WHERE id = ?');
// Two phases, to get past UNIQUE(trip_id, day_number) on the way.
remaining.forEach((r, i) => setNumber.run(-(i + 1), r.id));
const oldDateById = new Map(remaining.map(r => [r.id, r.date]));
const newDateById = new Map<number, string | null>();
remaining.forEach((r, i) => {
const date = sortedDates[i] ?? null;
setNumberAndDate.run(i + 1, date, r.id);
newDateById.set(r.id, date);
});
if (sortedDates.length > 0) this.days.restampReservationDates(tripId, oldDateById, newDateById);
if (remaining.length >= sortedDates.length) return null;
const range = this.db.get<{ start_date: string | null; end_date: string | null }>(
'SELECT start_date, end_date FROM trips WHERE id = ?', tripId,
);
if (!range?.start_date || !range.end_date) return null;
const endDate = sortedDates[remaining.length - 1];
this.db.run('UPDATE trips SET end_date = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?', endDate, tripId);
return endDate;
}
}
/**
* A cancelled stay's mirror without the deleted day. Its stop there went with the
* day, and the collaborators drop the whole day on day:deleted, so announcing that
* stop, the day's order or its roads on top would name a day nobody holds anymore.
*/
function withoutDay(mirror: AccommodationMirror, dayId: number): AccommodationMirror {
return {
...mirror,
removed: mirror.removed.filter(stop => stop.dayId !== dayId),
...(mirror.vias ? { vias: mirror.vias.filter(day => day.dayId !== dayId) } : {}),
};
}
+18 -4
View File
@@ -12,6 +12,7 @@ import {
} from '@nestjs/common';
import type { User } from '../../types';
import { DaysService, DayReorderError } from './days.service';
import { DayRemovalService, DayDeleteError, type DayRemoval } from './day-removal.service';
import { DayCreateDto, DayReorderDto, DayTransportDto, DayUpdateDto } from './days.dto';
import { JwtAuthGuard } from '../auth/jwt-auth.guard';
import { CurrentUser } from '../auth/current-user.decorator';
@@ -32,7 +33,7 @@ import { RequirePermission, TripAccessGuard } from '../permissions/trip-access.g
// go through that method, and this keeps the two from drifting.
@UseGuards(JwtAuthGuard, TripAccessGuard)
export class DaysController {
constructor(private readonly days: DaysService) {}
constructor(private readonly days: DaysService, private readonly removal: DayRemovalService) {}
@Get()
list(@CurrentUser() user: User, @Param('tripId') tripId: string) {
@@ -131,8 +132,21 @@ export class DaysController {
if (!this.days.getDay(id, tripId)) {
throw new HttpException({ error: 'Day not found' }, 404);
}
this.days.remove(id);
this.days.broadcast(tripId, 'day:deleted', { dayId: Number(id) }, socketId);
return { success: true };
let removal: DayRemoval;
try {
removal = this.removal.remove(tripId, id, { userId: user.id, socketId });
} catch (err) {
// The last day of a trip stays; nothing was written.
if (err instanceof DayDeleteError) throw new HttpException({ error: err.message }, 400);
throw err;
}
this.removal.announce(tripId, removal, {
all: (event, payload) => this.days.broadcast(tripId, event, payload, undefined),
others: (event, payload) => this.days.broadcast(tripId, event, payload, socketId),
socketId,
});
// `trip` is additive: the socket echo skips this tab, and the trip header
// shows the day count and, when the last date went, the new end date.
return { success: true, trip: removal.trip };
}
}
+16 -5
View File
@@ -12,6 +12,8 @@ import {
} from '@trek/shared';
import type { DayCreateRequest, DayReorderRequest, DayUpdateRequest } from '@trek/shared';
import { DaysService, DayReorderError } from './days.service';
import { DayRemovalService, DayDeleteError, type DayRemoval } from './day-removal.service';
import type { MirrorSender } from '../accommodations/accommodations.service';
function parseId(value: string | string[]): number | null {
const n = Number(Array.isArray(value) ? value[0] : value);
@@ -41,6 +43,7 @@ export class DaysMcp {
private readonly days: DaysService,
private readonly auth: AuthService,
private readonly guards: McpToolGuardsService,
private readonly removal: DayRemovalService,
) {}
@Tool({
@@ -144,7 +147,7 @@ export class DaysMcp {
@Tool({
name: 'delete_day',
description: 'Delete a day from a trip.',
description: 'Delete a day from a trip. The places planned on it stay in the place list of the trip; its notes, title and description are deleted; bookings on it stay on the trip without a day. A stay that checks in or out on the day is cancelled together with its booking and the expense of that booking, while a stay that only runs across the day is kept. The later days move up one place. On a dated trip the dates stay on their positions, so every later day and the bookings on it move one date earlier; a day without a date then takes the last date, and when there is none the trip ends one day earlier. The last day of a trip cannot be deleted.',
inputSchema: {
tripId: z.number().int().positive(),
dayId: z.number().int().positive(),
@@ -157,10 +160,18 @@ export class DaysMcp {
if (!this.days.verifyTripAccess(tripId, ctx.userId)) return noAccess();
if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied();
if (!this.days.getDay(dayId, tripId)) return errorResult('Day not found.');
this.days.remove(dayId);
// REST parity shape ({ dayId }) — the client reads payload.dayId, so the { id }
// variant never removed the day from collaborator screens.
this.guards.safeBroadcast(tripId, 'day:deleted', { dayId });
let removal: DayRemoval;
try {
removal = this.removal.remove(tripId, dayId, { userId: ctx.userId });
} catch (err) {
// The last day stays, and the caller can act on the sentence.
if (err instanceof DayDeleteError) return errorResult(err.message);
throw err;
}
// The same fan-out as REST, day:deleted ({ dayId }, the shape the client reads)
// first. A tool has no socket of its own, so every screen hears all of it.
const send: MirrorSender = (event, payload) => this.guards.safeBroadcast(tripId, event, payload as Record<string, unknown>);
this.removal.announce(tripId, removal, { all: send, others: send });
return ok({ success: true });
}
+13 -2
View File
@@ -3,6 +3,9 @@ import { DaysController } from './days.controller';
import { DaysService } from './days.service';
import { DaysMcp } from './days.mcp';
import { DaysRpc } from './days.rpc';
import { DayRemovalService } from './day-removal.service';
import { AccommodationsDomainModule } from '../accommodations/accommodations-domain.module';
import { AssignmentsDomainModule } from '../assignments/assignments-domain.module';
import { PluginGuardsModule } from '../plugins/host/plugin-guards.module';
import { RealtimeModule } from '../realtime/realtime.module';
import { PermissionsModule } from '../permissions/permissions.module';
@@ -19,11 +22,19 @@ import { McpSharedModule } from '../mcp-shared/mcp-shared.module';
*
* Day notes used to live here too, with their own full file set; they are their
* own domain now (day-notes/).
*
* Deleting a day cancels the stays that check in or out on it and lets the
* journey catch up, so DayRemovalService needs the accommodations and
* assignments services. Both domain modules are leaves (neither reaches days or
* places), and PlacesModule already brings them in, so the edge adds no cycle.
*/
@Module({
imports: [McpSharedModule, PermissionsModule, QueryHelpersModule, PlacesModule, AuthModule, RealtimeModule, PluginGuardsModule],
imports: [
McpSharedModule, PermissionsModule, QueryHelpersModule, PlacesModule, AuthModule, RealtimeModule, PluginGuardsModule,
AccommodationsDomainModule, AssignmentsDomainModule,
],
controllers: [DaysController],
providers: [DaysService, DaysMcp, DaysRpc],
providers: [DaysService, DayRemovalService, DaysMcp, DaysRpc],
exports: [DaysService],
})
export class DaysModule {}
+12 -2
View File
@@ -6,6 +6,8 @@ import { num, schemaMessage } from '../plugins/host/rpc-params';
import type { PluginRpcContext } from '../plugins/host/rpc-kit/types';
import { RealtimeService } from '../realtime/realtime.service';
import { DaysService } from './days.service';
import { DayRemovalService, DayDeleteError, type DayRemoval } from './day-removal.service';
import type { MirrorSender } from '../accommodations/accommodations.service';
const DAY_EDIT_ACTION = 'day_edit';
@@ -21,6 +23,7 @@ export class DaysRpc {
private readonly days: DaysService,
private readonly realtime: RealtimeService,
private readonly guards: PluginGuards,
private readonly removal: DayRemovalService,
) {}
@PluginMethod('days.create', { permission: 'db:write:days' })
@@ -59,8 +62,15 @@ export class DaysRpc {
const actor = this.guards.requireActor(ctx, 'day');
this.guards.requireTripEdit(tripId, actor, DAY_EDIT_ACTION);
if (!this.days.getDay(dayId, tripId)) throw new ForbiddenResource(`no day ${dayId} on trip ${tripId}`);
this.days.remove(dayId);
this.realtime.broadcast(tripId, 'day:deleted', { dayId });
let removal: DayRemoval;
try {
removal = this.removal.remove(tripId, dayId, { userId: actor });
} catch (err) {
if (err instanceof DayDeleteError) throw new BadParams(err.message);
throw err;
}
const send: MirrorSender = (event, payload) => this.realtime.broadcast(tripId, event, payload);
this.removal.announce(tripId, removal, { all: send, others: send });
return { deleted: true };
}
}
+8 -2
View File
@@ -6,6 +6,7 @@ import { PermissionsService } from '../permissions/permissions.service';
import { QueryHelpersService } from '../query-helpers/query-helpers.service';
import { carryVias, locatedStopIds, reseatOwnStop } from '../accommodations/night-seat';
import { formatAssignmentWithPlace } from '../common/rowShape';
import { TRIP_SELECT } from '../trips/trip-select';
import type { AssignmentRow, Day, DayNote, User } from '../../types';
type Trip = TripAccess;
@@ -259,8 +260,13 @@ export class DaysService {
return { ...updatedDay, assignments: this.getAssignmentsForDay(id) };
}
remove(id: string | number): void {
this.db.run('DELETE FROM days WHERE id = ?', id);
/**
* The trip in list shape, as the trip routes answer it, for a day write that
* changed the trip itself (its end date, its day count). Read through the leaf
* query rather than TripsService, which imports this class.
*/
getTripForViewer(tripId: string | number, userId: number): unknown {
return this.db.prepare(`${TRIP_SELECT} WHERE t.id = :tripId`).get({ userId, tripId });
}
// -------------------------------------------------------------------------
+39
View File
@@ -0,0 +1,39 @@
/**
* The trip row in list shape, and the one field that must never leave with it.
*
* A leaf on purpose: it imports nothing. TripsService imports DaysService, so a
* day write that has to hand its collaborators the changed trip (a day deleted
* off the end of a dated trip shortens it) cannot import trips.service.ts
* without closing a module cycle. Reading the same query from here keeps the
* two surfaces from drifting apart instead of copying it into the days domain.
*/
/**
* Strips `feed_token` from a trip row on its way out.
*
* The column is the sole credential for the anonymous /api/feed/trip/:token.ics
* route, and `SELECT t.*` hands it to every reader of the trip. Gating the
* token endpoint on `share_manage` means nothing while any member can read the
* same value out of the trip payload, so the two go together. No TREK client
* reads the field (it is absent from client/ and shared/ entirely).
*/
export function withoutFeedToken<T>(row: T): T {
if (row && typeof row === 'object') delete (row as Record<string, unknown>).feed_token;
return row;
}
// `NULL AS feed_token` after `t.*` rather than an explicit column list: the
// duplicate name wins in the row object, so the credential is blanked once here
// instead of at each of the nine call sites, and the next migration that adds a
// column does not have to remember to extend a hand-maintained list.
export const TRIP_SELECT = `
SELECT t.*,
NULL AS feed_token,
(SELECT COUNT(*) FROM days d WHERE d.trip_id = t.id) as day_count,
(SELECT COUNT(*) FROM places p WHERE p.trip_id = t.id) as place_count,
CASE WHEN t.user_id = :userId THEN 1 ELSE 0 END as is_owner,
u.username as owner_username,
(SELECT COUNT(*) FROM trip_members tm WHERE tm.trip_id = t.id) as shared_count
FROM trips t
JOIN users u ON u.id = t.user_id
`;
+5 -29
View File
@@ -13,6 +13,7 @@ import { UnsplashService } from '../unsplash/unsplash.service';
import { StorageService } from '../storage/storage.service';
import { SettingsService } from '../settings/settings.service';
import { NotFoundError, ValidationError } from '../common/domain-errors';
import { TRIP_SELECT } from './trip-select';
export const MS_PER_DAY = 86400000;
@@ -29,35 +30,10 @@ function assertTripSpan(startDate: string, endDate: string) {
if (span > MAX_TRIP_DAYS) throw new ValidationError(`A trip can span at most ${MAX_TRIP_DAYS} days`);
}
/**
* Strips `feed_token` from a trip row on its way out.
*
* The column is the sole credential for the anonymous /api/feed/trip/:token.ics
* route, and `SELECT t.*` hands it to every reader of the trip. Gating the
* token endpoint on `share_manage` means nothing while any member can read the
* same value out of the trip payload, so the two go together. No TREK client
* reads the field (it is absent from client/ and shared/ entirely).
*/
export function withoutFeedToken<T>(row: T): T {
if (row && typeof row === 'object') delete (row as Record<string, unknown>).feed_token;
return row;
}
// `NULL AS feed_token` after `t.*` rather than an explicit column list: the
// duplicate name wins in the row object, so the credential is blanked once here
// instead of at each of the nine call sites, and the next migration that adds a
// column does not have to remember to extend a hand-maintained list.
export const TRIP_SELECT = `
SELECT t.*,
NULL AS feed_token,
(SELECT COUNT(*) FROM days d WHERE d.trip_id = t.id) as day_count,
(SELECT COUNT(*) FROM places p WHERE p.trip_id = t.id) as place_count,
CASE WHEN t.user_id = :userId THEN 1 ELSE 0 END as is_owner,
u.username as owner_username,
(SELECT COUNT(*) FROM trip_members tm WHERE tm.trip_id = t.id) as shared_count
FROM trips t
JOIN users u ON u.id = t.user_id
`;
// The list-shape query and the feed-token scrub live in a leaf file so the days
// domain can read the same trip row without importing this module (which
// imports DaysService). Re-exported here so every existing importer is unchanged.
export { TRIP_SELECT, withoutFeedToken } from './trip-select';
interface CreateTripData {
title: string;