mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-22 15:06:32 +00:00
fix(trips): let a trip run past 365 days and refuse a longer range instead of clipping it
A trip longer than a year stopped at day 365: generateDays capped the day rows at MAX_TRIP_DAYS while the trip kept its full end date, so the last days had a date but no day to plan on, in the day list, the transport and booking dialogs and the phone's date pickers (#2403). The limit is MAX_TRIP_DAYS in @trek/shared now, 999 days, shared by the server, both trip forms and the MCP tools, and a range past it is refused rather than cut short: 400 on REST, an error result on MCP, bad params on the plugin RPC, and a message in the forms before anything is sent. The check runs before the trip row and the budget are touched, also catches a start date moved past the stored end on its own, and holds a day_count rebuild to the same bound. A trip that already carries a longer range can still be renamed. The MCP-only 90-day ceiling on create_trip goes with it; it clipped dated trips the same way. Closes #2403
This commit is contained in:
@@ -17,8 +17,6 @@ export {
|
||||
TOOL_ANNOTATIONS_WRITE,
|
||||
} from '../../nest-mcp';
|
||||
|
||||
export const MAX_MCP_TRIP_DAYS = 90;
|
||||
|
||||
export function noAccess() {
|
||||
return errorResult('Trip not found or access denied.');
|
||||
}
|
||||
|
||||
@@ -8,7 +8,7 @@ import { RealtimeService } from '../realtime/realtime.service';
|
||||
* src/mcp/tools/_shared.ts, reaching the db Proxy, permissions.bridge and the
|
||||
* src/websocket stub as module globals. The @McpController domain classes are
|
||||
* ordinary Nest providers, so they inject this instead. The pure result
|
||||
* helpers (noAccess/permissionDenied/adminRequired/MAX_MCP_TRIP_DAYS and the
|
||||
* helpers (noAccess/permissionDenied/adminRequired and the
|
||||
* src/nest-mcp re-exports) stay in _shared.ts — they carry no dependencies.
|
||||
*/
|
||||
@Injectable()
|
||||
|
||||
@@ -44,7 +44,7 @@ You are connected to TREK, a travel planning application. Below is a compact ref
|
||||
- The authenticated user can only access trips they own or are a member of. Never guess at trip IDs.
|
||||
- Only the trip owner can delete the trip, add members, or remove members.
|
||||
- Deleting a place removes all of its day assignments as well — warn the user before doing this.
|
||||
- Trips created via MCP are capped at 90 days.
|
||||
- A trip can span at most 999 days, by date range or by day_count.
|
||||
|
||||
## Dates and times
|
||||
|
||||
|
||||
@@ -20,8 +20,8 @@ const airtrailAddonOn = addonGate(ADDON_IDS.AIRTRAIL);
|
||||
/**
|
||||
* The picker sends whatever a human ticked; a model can name a hundred ids off
|
||||
* one hallucinated list, and each one becomes a reservation and a broadcast.
|
||||
* The same reasoning as MAX_MCP_TRIP_DAYS: the tool surface caps a bulk write
|
||||
* the REST route leaves open, and says how to continue.
|
||||
* The tool surface caps a bulk write the REST route leaves open, and says how
|
||||
* to continue.
|
||||
*/
|
||||
const MAX_MCP_AIRTRAIL_FLIGHTS = 50;
|
||||
|
||||
|
||||
@@ -23,7 +23,7 @@ import { RuntimeEnvService } from '../app-config/runtime-env.service';
|
||||
import type { Request, Response } from 'express';
|
||||
import type { Options } from 'multer';
|
||||
import path from 'path';
|
||||
import type { ActiveTripResponse } from '@trek/shared';
|
||||
import { MAX_TRIP_DAYS, type ActiveTripResponse } from '@trek/shared';
|
||||
import { StorageService } from '../storage/storage.service';
|
||||
import type { User } from '../../types';
|
||||
import { TripsService } from './trips.service';
|
||||
@@ -116,11 +116,16 @@ export class TripsController {
|
||||
if (start_date && end_date && new Date(end_date) < new Date(start_date)) {
|
||||
throw new HttpException({ error: 'End date must be after start date' }, 400);
|
||||
}
|
||||
const parsedDayCount = day_count ? Math.min(Math.max(Number(day_count) || 7, 1), 365) : undefined;
|
||||
const { trip, tripId, reminderDays } = this.trips.create(user.id, { title, description, start_date, end_date, currency, reminder_days, day_count: parsedDayCount });
|
||||
this.audit.writeAudit({ userId: user.id, action: 'trip.create', ip: getClientIp(req), details: { tripId, title, reminder_days: reminderDays === 0 ? 'none' : `${reminderDays} days` } });
|
||||
if (reminderDays > 0) logInfo(`${user.email} set ${reminderDays}-day reminder for trip "${title}"`);
|
||||
return { trip };
|
||||
const parsedDayCount = day_count ? Math.min(Math.max(Number(day_count) || 7, 1), MAX_TRIP_DAYS) : undefined;
|
||||
try {
|
||||
const { trip, tripId, reminderDays } = this.trips.create(user.id, { title, description, start_date, end_date, currency, reminder_days, day_count: parsedDayCount });
|
||||
this.audit.writeAudit({ userId: user.id, action: 'trip.create', ip: getClientIp(req), details: { tripId, title, reminder_days: reminderDays === 0 ? 'none' : `${reminderDays} days` } });
|
||||
if (reminderDays > 0) logInfo(`${user.email} set ${reminderDays}-day reminder for trip "${title}"`);
|
||||
return { trip };
|
||||
} catch (e: unknown) {
|
||||
if (e instanceof ValidationError) throw new HttpException({ error: e.message }, 400);
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
@Get(':id')
|
||||
|
||||
@@ -12,9 +12,10 @@ import { CalendarService } from '../calendar/calendar.service';
|
||||
import { TripMembersService } from '../trip-members/trip-members.service';
|
||||
import { TripReadModelService } from '../trip-read-model/trip-read-model.service';
|
||||
import { ADDON_IDS } from '../../addons';
|
||||
import { MAX_MCP_TRIP_DAYS, noAccess, permissionDenied } from '../../mcp/tools/_shared';
|
||||
import { noAccess, permissionDenied } from '../../mcp/tools/_shared';
|
||||
import { canRead, canReadTrips, canDeleteTrips } from '../../mcp/scopes';
|
||||
import { TripsService, MAX_TRIP_DAYS, NotFoundError, ValidationError } from './trips.service';
|
||||
import { MAX_TRIP_DAYS } from '@trek/shared';
|
||||
import { TripsService, NotFoundError, ValidationError } from './trips.service';
|
||||
import { TodoService } from '../todo/todo.service';
|
||||
import { CollabService } from '../collab/collab.service';
|
||||
import { AddonsService } from '../addons/addons.service';
|
||||
@@ -81,7 +82,7 @@ export class TripsMcp {
|
||||
|
||||
@Tool({
|
||||
name: 'create_trip',
|
||||
description: 'Create a new trip. Returns the created trip with its generated days.',
|
||||
description: 'Create a new trip. Returns the created trip; its day_count is the number of days generated, one per day of the date range.',
|
||||
inputSchema: {
|
||||
title: z.string().min(1).max(200).describe('Trip title'),
|
||||
description: z.string().max(2000).optional().describe('Trip description'),
|
||||
@@ -117,8 +118,13 @@ export class TripsMcp {
|
||||
if (start_date && end_date && new Date(end_date) < new Date(start_date)) {
|
||||
return { content: [{ type: 'text' as const, text: 'End date must be after start date.' }], isError: true };
|
||||
}
|
||||
const { trip } = this.trips.create(ctx.userId, { title, description, start_date, end_date, currency, day_count, reminder_days }, MAX_MCP_TRIP_DAYS);
|
||||
return ok({ trip });
|
||||
try {
|
||||
const { trip } = this.trips.create(ctx.userId, { title, description, start_date, end_date, currency, day_count, reminder_days });
|
||||
return ok({ trip });
|
||||
} catch (err) {
|
||||
if (err instanceof ValidationError) return errorResult(err.message);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
@Tool({
|
||||
@@ -177,9 +183,14 @@ export class TripsMcp {
|
||||
: { start_date, end_date };
|
||||
// update() re-anchors the budget before the trip row moves off the old
|
||||
// currency (#1543) and then runs the legacy updateTrip core.
|
||||
const { updatedTrip } = await this.trips.update(tripId, ctx.userId, { title, description, ...dates, currency, is_archived, cover_image, day_count, reminder_days, date_shift_mode }, 'user');
|
||||
this.guards.safeBroadcast(tripId, 'trip:updated', { trip: updatedTrip });
|
||||
return ok({ trip: updatedTrip });
|
||||
try {
|
||||
const { updatedTrip } = await this.trips.update(tripId, ctx.userId, { title, description, ...dates, currency, is_archived, cover_image, day_count, reminder_days, date_shift_mode }, 'user');
|
||||
this.guards.safeBroadcast(tripId, 'trip:updated', { trip: updatedTrip });
|
||||
return ok({ trip: updatedTrip });
|
||||
} catch (err) {
|
||||
if (err instanceof ValidationError) return errorResult(err.message);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
@Tool({
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import path from 'path';
|
||||
import { DatabaseService } from '../database/database.service';
|
||||
import type { ActiveTrip, TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
|
||||
import { MAX_TRIP_DAYS, tripSpanDays, type ActiveTrip, type TrekWsPayload, type TrekWsTripEventName } from '@trek/shared';
|
||||
import { RealtimeService } from '../realtime/realtime.service';
|
||||
import { PermissionsService } from '../permissions/permissions.service';
|
||||
import type { Trip, User } from '../../types';
|
||||
@@ -14,7 +14,19 @@ import { StorageService } from '../storage/storage.service';
|
||||
import { NotFoundError, ValidationError } from '../common/domain-errors';
|
||||
|
||||
export const MS_PER_DAY = 86400000;
|
||||
export const MAX_TRIP_DAYS = 365;
|
||||
|
||||
/**
|
||||
* The date range is refused, not cut short: generateDays used to clip the day
|
||||
* rows at the limit while the trip kept its full end date, so everything past
|
||||
* the cut-off had dates but no day to go on (#2403). An inverted range is
|
||||
* refused here as well, because a start date moved past the stored end date
|
||||
* arrives on its own and would otherwise empty the trip.
|
||||
*/
|
||||
function assertTripSpan(startDate: string, endDate: string) {
|
||||
const span = tripSpanDays(startDate, endDate);
|
||||
if (span < 1) throw new ValidationError('End date must be after start date');
|
||||
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.
|
||||
@@ -171,7 +183,7 @@ export class TripsService {
|
||||
|
||||
// ── Day generation ────────────────────────────────────────────────────────
|
||||
|
||||
generateDays(tripId: number | bigint | string, startDate: string | null, endDate: string | null, maxDays?: number, dayCount?: number) {
|
||||
generateDays(tripId: number | bigint | string, startDate: string | null, endDate: string | null, dayCount?: number) {
|
||||
const existing = this.db.prepare('SELECT id, day_number, date FROM days WHERE trip_id = ?').all(tripId) as { id: number; day_number: number; date: string | null }[];
|
||||
const setDayNumber = this.db.prepare('UPDATE days SET day_number = ? WHERE id = ?');
|
||||
|
||||
@@ -215,10 +227,8 @@ export class TripsService {
|
||||
}
|
||||
|
||||
const [sy, sm, sd] = startDate.split('-').map(Number);
|
||||
const [ey, em, ed] = endDate.split('-').map(Number);
|
||||
const startMs = Date.UTC(sy, sm - 1, sd);
|
||||
const endMs = Date.UTC(ey, em - 1, ed);
|
||||
const numDays = Math.min(Math.floor((endMs - startMs) / MS_PER_DAY) + 1, maxDays ?? MAX_TRIP_DAYS);
|
||||
const numDays = tripSpanDays(startDate, endDate);
|
||||
|
||||
const targetDates: string[] = [];
|
||||
for (let i = 0; i < numDays; i++) {
|
||||
@@ -309,7 +319,8 @@ export class TripsService {
|
||||
`).all({ userId, archived });
|
||||
}
|
||||
|
||||
create(userId: number, data: CreateTripData, maxDays?: number) {
|
||||
create(userId: number, data: CreateTripData) {
|
||||
if (data.start_date && data.end_date) assertTripSpan(data.start_date, data.end_date);
|
||||
const rd = data.reminder_days !== undefined
|
||||
? (Number(data.reminder_days) >= 0 && Number(data.reminder_days) <= 30 ? Number(data.reminder_days) : 3)
|
||||
: 3;
|
||||
@@ -320,7 +331,7 @@ export class TripsService {
|
||||
`).run(userId, data.title, data.description || null, data.start_date || null, data.end_date || null, data.currency || 'EUR', rd);
|
||||
|
||||
const tripId = result.lastInsertRowid;
|
||||
this.generateDays(tripId, data.start_date || null, data.end_date || null, maxDays, data.day_count);
|
||||
this.generateDays(tripId, data.start_date || null, data.end_date || null, data.day_count);
|
||||
|
||||
const trip = this.db.prepare(`${TRIP_SELECT} WHERE t.id = :tripId`).get({ userId, tripId });
|
||||
return { trip, tripId: Number(tripId), reminderDays: rd };
|
||||
@@ -384,15 +395,11 @@ export class TripsService {
|
||||
const trip = this.db.prepare('SELECT * FROM trips WHERE id = ?').get(tripId) as Trip & { reminder_days?: number } | undefined;
|
||||
if (!trip) throw new NotFoundError('Trip not found');
|
||||
|
||||
const { title, description, start_date, end_date, currency, is_archived, cover_image, reminder_days } = data;
|
||||
|
||||
if (start_date && end_date && new Date(end_date) < new Date(start_date))
|
||||
throw new ValidationError('End date must be after start date');
|
||||
const { title, description, currency, is_archived, cover_image, reminder_days } = data;
|
||||
const { newStart, newEnd, dayCount, regenerate } = this.resolveRange(trip, data);
|
||||
|
||||
const newTitle = title || trip.title;
|
||||
const newDesc = description !== undefined ? description : trip.description;
|
||||
const newStart = start_date !== undefined ? start_date : trip.start_date;
|
||||
const newEnd = end_date !== undefined ? end_date : trip.end_date;
|
||||
const newCurrency = currency || trip.currency;
|
||||
const newArchived = is_archived !== undefined ? (is_archived ? 1 : 0) : trip.is_archived;
|
||||
const newCover = cover_image !== undefined ? cover_image : trip.cover_image;
|
||||
@@ -410,8 +417,7 @@ export class TripsService {
|
||||
if (trip.start_date && trip.end_date && newStart && newStart !== trip.start_date)
|
||||
this.vacay.shiftOwnerEntriesForTripWindow(trip.user_id, trip.start_date, trip.end_date, newStart);
|
||||
|
||||
const dayCount = data.day_count ? Math.min(Math.max(Number(data.day_count) || 7, 1), MAX_TRIP_DAYS) : undefined;
|
||||
if (newStart !== trip.start_date || newEnd !== trip.end_date || dayCount) {
|
||||
if (regenerate) {
|
||||
this.db.transaction(() => {
|
||||
// Accommodations have no absolute date columns, so their pre-change dates must be
|
||||
// snapshotted before generateDays re-dates the day rows in place.
|
||||
@@ -419,7 +425,7 @@ export class TripsService {
|
||||
(this.db.prepare('SELECT id, date FROM days WHERE trip_id = ?').all(tripId) as { id: number; date: string | null }[])
|
||||
.map(d => [d.id, d.date]),
|
||||
);
|
||||
this.generateDays(tripId, newStart || null, newEnd || null, undefined, dayCount);
|
||||
this.generateDays(tripId, newStart || null, newEnd || null, dayCount);
|
||||
if (data.date_shift_mode === 'shift_all') {
|
||||
// Explicit "shift everything": bookings stay glued to their (re-dated) day rows,
|
||||
// so re-stamp reservation_time to follow — same rules as reorderDays/insertDay.
|
||||
@@ -456,7 +462,31 @@ export class TripsService {
|
||||
return { updatedTrip, changes, isAdminEdit, ownerEmail, newTitle, newReminder, oldReminder };
|
||||
}
|
||||
|
||||
/**
|
||||
* The dates the update leaves the trip with, checked before anything is
|
||||
* written. The day grid is rebuilt whenever a date moves or a day_count
|
||||
* arrives, and a dated rebuild runs over the whole range, so the range is
|
||||
* held to the limit exactly then. A trip stored with a longer range can
|
||||
* still be renamed.
|
||||
*/
|
||||
private resolveRange(trip: Trip, data: UpdateTripData) {
|
||||
const { start_date, end_date } = data;
|
||||
if (start_date && end_date && new Date(end_date) < new Date(start_date))
|
||||
throw new ValidationError('End date must be after start date');
|
||||
const newStart = start_date !== undefined ? start_date : trip.start_date;
|
||||
const newEnd = end_date !== undefined ? end_date : trip.end_date;
|
||||
const dayCount = data.day_count ? Math.min(Math.max(Number(data.day_count) || 7, 1), MAX_TRIP_DAYS) : undefined;
|
||||
const regenerate = newStart !== trip.start_date || newEnd !== trip.end_date || dayCount !== undefined;
|
||||
if (regenerate && newStart && newEnd) assertTripSpan(newStart, newEnd);
|
||||
return { newStart, newEnd, dayCount, regenerate };
|
||||
}
|
||||
|
||||
async update(tripId: string | number, userId: number, body: UpdateTripData, role: string) {
|
||||
// A refused range must not leave the budget rebased onto a currency the trip
|
||||
// never took, so the dates are checked before the first write.
|
||||
const trip = this.getRaw(tripId);
|
||||
if (!trip) throw new NotFoundError('Trip not found');
|
||||
this.resolveRange(trip, body);
|
||||
// Re-anchor the budget while the outgoing currency is still on the trip row,
|
||||
// otherwise the frozen FX rates and the currency-less expenses that inherit the
|
||||
// trip's base are left pointing at a currency that no longer exists (#1543).
|
||||
|
||||
Reference in New Issue
Block a user