mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-08-07 21:16:44 +00:00
fix(server): fix the quirks preserved by the reservations DI migration
Two verified defects carried through the parity fold: - The create-path accommodation metadata sync gated on the raw accommodation_id instead of the resolved one, so a hotel whose accommodation was just auto-created never received its metadata check-in/out times or confirmation. Now keyed off resolvedAccommodationId (RESV-SVC-006 re-pins the fixed behavior). - The multi-statement writes ran outside transactions: create (accommodation insert + reservation insert + endpoint save + metadata sync), update (accommodation upsert + update + endpoint replace + sync), remove (the 3-delete cascade), setReservationTravelers (delete + inserts) and resyncReservationDays now run in db.transaction() — a mid-write failure no longer leaves partial state (RESV-FIX-001/002 pin the rollbacks). Left as-is on purpose: the truthy updatePositions dayId check, the empty-string COALESCE keeps on title/status/type, the TEXT accommodation_id normalization, the optional day_plan_position wire tolerance and the swallowed notifyBookingChange catches — all contract or intentional behavior, not defects.
This commit is contained in:
jubnl
@@ -113,8 +113,11 @@ type AccommodationTimesMeta = {
|
||||
/**
|
||||
* Reservations domain service — owns the reservation SQL (moved 1:1 from the
|
||||
* legacy services/reservationService.ts: identical statements, the `||`
|
||||
* falsy-coercion defaults, the COALESCE update semantics, the post-write
|
||||
* re-selects and the un-transactioned multi-statement writes). Trip access,
|
||||
* falsy-coercion defaults, the COALESCE update semantics and the post-write
|
||||
* re-selects; the multi-statement writes gained db.transaction() wrappers in
|
||||
* the post-fold quirk-fix commit, and the accommodation metadata sync now
|
||||
* keys off the resolved accommodation id so auto-created accommodations get
|
||||
* their check-in/out times too). Trip access,
|
||||
* the 'reservation_edit' permission and the WebSocket broadcast keep their
|
||||
* legacy call paths. The legacy route's budget side effects (auto-create /
|
||||
* update / delete a linked budget item) and the booking notification are
|
||||
@@ -240,11 +243,13 @@ export class ReservationsService {
|
||||
setReservationTravelers(reservationId: number | string, tripId: string | number, userIds: number[]): void {
|
||||
const allowed = this.assignableUserIds(tripId);
|
||||
const ids = [...new Set(userIds)].filter(uid => allowed.has(uid));
|
||||
this.db.run('DELETE FROM reservation_travelers WHERE reservation_id = ?', reservationId);
|
||||
if (ids.length > 0) {
|
||||
const insert = this.db.prepare('INSERT OR IGNORE INTO reservation_travelers (reservation_id, user_id) VALUES (?, ?)');
|
||||
for (const uid of ids) insert.run(reservationId, uid);
|
||||
}
|
||||
this.db.transaction(() => {
|
||||
this.db.run('DELETE FROM reservation_travelers WHERE reservation_id = ?', reservationId);
|
||||
if (ids.length > 0) {
|
||||
const insert = this.db.prepare('INSERT OR IGNORE INTO reservation_travelers (reservation_id, user_id) VALUES (?, ?)');
|
||||
for (const uid of ids) insert.run(reservationId, uid);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/** Assign trip members / named guests to a reservation (#1517). Null when off-trip. */
|
||||
@@ -300,16 +305,18 @@ export class ReservationsService {
|
||||
tripId
|
||||
);
|
||||
const update = this.db.prepare('UPDATE reservations SET day_id = ?, end_day_id = ? WHERE id = ?');
|
||||
for (const r of rows) {
|
||||
const newDayId = this.resolveDayIdFromTime(tripId, r.reservation_time, false);
|
||||
if (newDayId == null) continue;
|
||||
const newEndDayId = r.reservation_end_time
|
||||
? (this.resolveDayIdFromTime(tripId, r.reservation_end_time, false) ?? r.end_day_id)
|
||||
: r.end_day_id;
|
||||
if (newDayId !== r.day_id || newEndDayId !== r.end_day_id) {
|
||||
update.run(newDayId, newEndDayId, r.id);
|
||||
this.db.transaction(() => {
|
||||
for (const r of rows) {
|
||||
const newDayId = this.resolveDayIdFromTime(tripId, r.reservation_time, false);
|
||||
if (newDayId == null) continue;
|
||||
const newEndDayId = r.reservation_end_time
|
||||
? (this.resolveDayIdFromTime(tripId, r.reservation_end_time, false) ?? r.end_day_id)
|
||||
: r.end_day_id;
|
||||
if (newDayId !== r.day_id || newEndDayId !== r.end_day_id) {
|
||||
update.run(newDayId, newEndDayId, r.id);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
private saveEndpoints(reservationId: number, endpoints: EndpointInput[]): void {
|
||||
@@ -436,7 +443,13 @@ export class ReservationsService {
|
||||
return row;
|
||||
}
|
||||
|
||||
/** The accommodation insert, the reservation insert, the endpoint save and
|
||||
* the metadata sync are one logical write — all-or-nothing. */
|
||||
create(tripId: string | number, data: CreateReservationData): { reservation: ReservationRow; accommodationCreated: boolean } {
|
||||
return this.db.transaction(() => this.createInTx(tripId, data));
|
||||
}
|
||||
|
||||
private createInTx(tripId: string | number, data: CreateReservationData): { reservation: ReservationRow; accommodationCreated: boolean } {
|
||||
const {
|
||||
title, reservation_time, reservation_end_time, location,
|
||||
confirmation_number, notes, url, day_id, end_day_id, place_id, assignment_id,
|
||||
@@ -500,19 +513,22 @@ export class ReservationsService {
|
||||
this.saveEndpoints(Number(result.lastInsertRowid), endpoints);
|
||||
}
|
||||
|
||||
// Sync check-in/out to accommodation if linked
|
||||
if (accommodation_id && metadata) {
|
||||
// Sync check-in/out to accommodation if linked. Keyed off the RESOLVED id
|
||||
// (quirk fix): the legacy gate read the raw accommodation_id, so a hotel
|
||||
// whose accommodation was just auto-created above never received its
|
||||
// metadata check-in/out times or confirmation.
|
||||
if (resolvedAccommodationId && metadata) {
|
||||
const meta = (typeof metadata === 'string' ? JSON.parse(metadata) : metadata) as AccommodationTimesMeta;
|
||||
if (meta.check_in_time || meta.check_in_end_time || meta.check_out_time) {
|
||||
this.db.run(
|
||||
'UPDATE day_accommodations SET check_in = COALESCE(?, check_in), check_in_end = COALESCE(?, check_in_end), check_out = COALESCE(?, check_out) WHERE id = ?',
|
||||
meta.check_in_time || null, meta.check_in_end_time || null, meta.check_out_time || null, accommodation_id
|
||||
meta.check_in_time || null, meta.check_in_end_time || null, meta.check_out_time || null, resolvedAccommodationId
|
||||
);
|
||||
}
|
||||
if (confirmation_number) {
|
||||
this.db.run(
|
||||
'UPDATE day_accommodations SET confirmation = COALESCE(?, confirmation) WHERE id = ?',
|
||||
confirmation_number, accommodation_id
|
||||
confirmation_number, resolvedAccommodationId
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -546,7 +562,13 @@ export class ReservationsService {
|
||||
return this.db.get<Reservation>('SELECT * FROM reservations WHERE id = ? AND trip_id = ?', id, tripId);
|
||||
}
|
||||
|
||||
/** The accommodation upsert, the reservation update, the endpoint replace
|
||||
* and the metadata sync are one logical write — all-or-nothing. */
|
||||
update(id: string | number, tripId: string | number, data: UpdateReservationData, current: Reservation): { reservation: ReservationRow; accommodationChanged: boolean } {
|
||||
return this.db.transaction(() => this.updateInTx(id, tripId, data, current));
|
||||
}
|
||||
|
||||
private updateInTx(id: string | number, tripId: string | number, data: UpdateReservationData, current: Reservation): { reservation: ReservationRow; accommodationChanged: boolean } {
|
||||
const {
|
||||
title, reservation_time, reservation_end_time, location,
|
||||
confirmation_number, notes, url, day_id, end_day_id, place_id, assignment_id,
|
||||
@@ -685,25 +707,29 @@ export class ReservationsService {
|
||||
return { reservation, accommodationChanged };
|
||||
}
|
||||
|
||||
/** The accommodation + budget-item + reservation deletes are one logical
|
||||
* cascade — all-or-nothing. */
|
||||
remove(id: string | number, tripId: string | number): { deleted: { id: number; title: string; type: string; accommodation_id: number | null } | undefined; accommodationDeleted: boolean; deletedBudgetItemId: number | null } {
|
||||
const reservation = this.db.get<{ id: number; title: string; type: string; accommodation_id: number | null }>(
|
||||
'SELECT id, title, type, accommodation_id FROM reservations WHERE id = ? AND trip_id = ?', id, tripId
|
||||
);
|
||||
if (!reservation) return { deleted: undefined, accommodationDeleted: false, deletedBudgetItemId: null };
|
||||
return this.db.transaction(() => {
|
||||
const reservation = this.db.get<{ id: number; title: string; type: string; accommodation_id: number | null }>(
|
||||
'SELECT id, title, type, accommodation_id FROM reservations WHERE id = ? AND trip_id = ?', id, tripId
|
||||
);
|
||||
if (!reservation) return { deleted: undefined, accommodationDeleted: false, deletedBudgetItemId: null };
|
||||
|
||||
let accommodationDeleted = false;
|
||||
if (reservation.accommodation_id) {
|
||||
this.db.run('DELETE FROM day_accommodations WHERE id = ?', reservation.accommodation_id);
|
||||
accommodationDeleted = true;
|
||||
}
|
||||
let accommodationDeleted = false;
|
||||
if (reservation.accommodation_id) {
|
||||
this.db.run('DELETE FROM day_accommodations WHERE id = ?', reservation.accommodation_id);
|
||||
accommodationDeleted = true;
|
||||
}
|
||||
|
||||
const linkedBudget = this.db.get<{ id: number }>('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?', tripId, id);
|
||||
if (linkedBudget) {
|
||||
this.db.run('DELETE FROM budget_items WHERE id = ?', linkedBudget.id);
|
||||
}
|
||||
const linkedBudget = this.db.get<{ id: number }>('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?', tripId, id);
|
||||
if (linkedBudget) {
|
||||
this.db.run('DELETE FROM budget_items WHERE id = ?', linkedBudget.id);
|
||||
}
|
||||
|
||||
this.db.run('DELETE FROM reservations WHERE id = ?', id);
|
||||
return { deleted: reservation, accommodationDeleted, deletedBudgetItemId: linkedBudget ? linkedBudget.id : null };
|
||||
this.db.run('DELETE FROM reservations WHERE id = ?', id);
|
||||
return { deleted: reservation, accommodationDeleted, deletedBudgetItemId: linkedBudget ? linkedBudget.id : null };
|
||||
});
|
||||
}
|
||||
|
||||
/** POST side effect: auto-create a linked budget item when a price is provided. */
|
||||
|
||||
@@ -134,7 +134,7 @@ describe('ReservationsService (DI-native, real SQL)', () => {
|
||||
expect(rows).toEqual([{ name: 'A', sequence: 0 }, { name: 'B', sequence: 1 }]);
|
||||
});
|
||||
|
||||
it('RESV-SVC-006 (quirk preserved): metadata check-in sync is gated on the raw accommodation_id, so an auto-created accommodation is not updated', () => {
|
||||
it('RESV-SVC-006 (quirk fixed): metadata check-in sync keys off the resolved id, so an auto-created accommodation gets its times too', () => {
|
||||
const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-02' });
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[];
|
||||
@@ -143,8 +143,9 @@ describe('ReservationsService (DI-native, real SQL)', () => {
|
||||
create_accommodation: { place_id: place.id, start_day_id: days[0].id, end_day_id: days[1].id },
|
||||
metadata: { check_in_time: '16:00' },
|
||||
});
|
||||
// The legacy gate read the raw accommodation_id and left this NULL.
|
||||
const acc = testDb.prepare('SELECT check_in FROM day_accommodations WHERE trip_id = ?').get(trip.id) as { check_in: string | null };
|
||||
expect(acc.check_in).toBeNull(); // the L416 gate reads accommodation_id, not resolvedAccommodationId
|
||||
expect(acc.check_in).toBe('16:00');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -734,3 +735,29 @@ describe('ReservationsService — legacy branch parity (coverage of the folded c
|
||||
expect(budget.deleteBudgetItem).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('ReservationsService — quirk fixes (post-fold)', () => {
|
||||
it('RESV-FIX-001: create is atomic — a failing endpoint save rolls back the reservation AND the auto-created accommodation', () => {
|
||||
const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-02' });
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[];
|
||||
// sequence is an unbindable object -> the endpoint INSERT throws mid-write.
|
||||
const badEndpoints = [{ role: 'from', name: 'A', code: null, lat: 1, lng: 2, timezone: null, local_time: null, local_date: null, sequence: {} }];
|
||||
expect(() => svc.create(String(trip.id), {
|
||||
title: 'Hotel', type: 'hotel',
|
||||
create_accommodation: { place_id: place.id, start_day_id: days[0].id, end_day_id: days[1].id },
|
||||
endpoints: badEndpoints,
|
||||
} as never)).toThrow();
|
||||
expect(testDb.prepare('SELECT COUNT(*) as c FROM reservations WHERE trip_id = ?').get(trip.id)).toEqual({ c: 0 });
|
||||
expect(testDb.prepare('SELECT COUNT(*) as c FROM day_accommodations WHERE trip_id = ?').get(trip.id)).toEqual({ c: 0 });
|
||||
});
|
||||
|
||||
it('RESV-FIX-002: update is atomic — a failing endpoint save rolls back the field update', () => {
|
||||
const { trip } = ownerTrip();
|
||||
const res = createReservation(testDb, trip.id, { title: 'Old' });
|
||||
const current = svc.getReservation(String(res.id), String(trip.id))!;
|
||||
const badEndpoints = [{ role: 'from', name: 'A', code: null, lat: 1, lng: 2, timezone: null, local_time: null, local_date: null, sequence: {} }];
|
||||
expect(() => svc.update(String(res.id), String(trip.id), { title: 'New', endpoints: badEndpoints } as never, current)).toThrow();
|
||||
expect(testDb.prepare('SELECT title FROM reservations WHERE id = ?').get(res.id)).toEqual({ title: 'Old' });
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user