From 4e369d57285233d1fa623c6c86444d8fdfca30a8 Mon Sep 17 00:00:00 2001 From: eleith Date: Sat, 24 Oct 2026 18:40:38 -0700 Subject: [PATCH] fix(web): show historical details for removed meeting types --- .changeset/removed-meeting-detail.md | 7 + apps/web/e2e/removed-meeting.e2e.ts | 125 ++++++++++++++++++ .../lib/server/appointment/snapshot.test.ts | 80 +++++++++++ .../src/lib/server/appointment/snapshot.ts | 20 +++ .../(app)/appointment/[id]/+page.server.ts | 33 +++-- .../(app)/appointment/[id]/+page.svelte | 26 +++- .../(app)/appointment/[id]/server.test.ts | 107 +++++++++------ 7 files changed, 331 insertions(+), 67 deletions(-) create mode 100644 .changeset/removed-meeting-detail.md create mode 100644 apps/web/e2e/removed-meeting.e2e.ts create mode 100644 apps/web/src/lib/server/appointment/snapshot.test.ts create mode 100644 apps/web/src/lib/server/appointment/snapshot.ts diff --git a/.changeset/removed-meeting-detail.md b/.changeset/removed-meeting-detail.md new file mode 100644 index 00000000..9d56d8cd --- /dev/null +++ b/.changeset/removed-meeting-detail.md @@ -0,0 +1,7 @@ +--- +'@when/web': patch +'@when/worker': patch +'@when/cli': patch +--- + +Show hosts saved appointment details with a clear warning when a meeting type is no longer configured. Read partial snapshots as display information rather than configuration, keep unavailable controls and calendar links hidden, and return a deliberate not-found response for unusable historical details without changing guest access. diff --git a/apps/web/e2e/removed-meeting.e2e.ts b/apps/web/e2e/removed-meeting.e2e.ts new file mode 100644 index 00000000..eaaeb6c1 --- /dev/null +++ b/apps/web/e2e/removed-meeting.e2e.ts @@ -0,0 +1,125 @@ +import { expect, test } from '@playwright/test'; +import { openDb } from '@when/db'; +import { signInAsAdmin } from './support/auth.ts'; +import { APP_DB_PATH, readAppointment } from './support/database.ts'; +import { seedAppointment } from './support/seed.ts'; + +const postOptions = { + headers: { origin: 'http://localhost:4183', accept: 'application/json' }, + maxRedirects: 0 +}; + +function futureWeekday(retry: number): string { + const date = new Date(); + date.setUTCDate(date.getUTCDate() + 52); + for (let index = 0; index <= retry; index++) { + if (index > 0) date.setUTCDate(date.getUTCDate() + 1); + while (date.getUTCDay() === 0 || date.getUTCDay() === 6) { + date.setUTCDate(date.getUTCDate() + 1); + } + } + return date.toISOString().slice(0, 10); +} + +test('host sees a marked historical fallback from a real booking snapshot, while guests remain denied', async ({ + page, + request +}, testInfo) => { + await signInAsAdmin(page); + const day = futureWeekday(testInfo.retry); + const created = await page.request.post('/admin/schedule/chat', { + ...postOptions, + form: { + slot: `${day}T13:30:00Z`, + duration: '30', + timezone: 'UTC', + name: 'Historical Guest', + email: 'historical-guest@example.test' + } + }); + const redirect = await created.json(); + expect(redirect).toMatchObject({ type: 'redirect', status: 303 }); + const id = new URL(redirect.location, 'http://localhost:4183').pathname.split('/').at(-1)!; + const booked = await readAppointment(id); + const snapshot = JSON.parse(booked.meeting_snapshot!); + expect(snapshot).toEqual({ + title: '30-minute chat', + duration_minutes: 30, + description: 'A quick introductory call.', + slug: 'chat' + }); + expect(snapshot).not.toHaveProperty('additional_duration_minutes'); + + // Exercise the missing-config branch without editing running configuration. + // The historical snapshot is exactly what the production booking service wrote. + const db = openDb(APP_DB_PATH); + try { + await db + .updateTable('appointments') + .set({ event_type_id: 'removed-meeting' }) + .where('id', '=', id) + .executeTakeFirstOrThrow(); + } finally { + await db.destroy(); + } + const beforeView = await readAppointment(id); + const guest = await request.get(`/appointment/${id}?token=${booked.cancel_token}`); + expect(guest.status()).toBe(404); + const response = await page.goto(`/appointment/${id}`); + expect(response?.status()).toBe(200); + await expect(page.getByRole('heading', { name: snapshot.title, exact: true })).toBeVisible(); + await expect(page.getByText(snapshot.description, { exact: true })).toBeVisible(); + await expect(page.getByRole('alert')).toContainText('This meeting type is no longer configured.'); + await expect(page.getByRole('alert')).toContainText('Showing saved appointment details'); + await expect(page.getByText('Historical Guest', { exact: false })).toBeVisible(); + await expect(page.getByRole('button', { name: 'Appointment actions', exact: true })).toHaveCount( + 0 + ); + await expect(page.getByRole('button', { name: 'Add to calendar', exact: true })).toHaveCount(0); + await expect(page.getByRole('dialog', { name: 'Change duration', exact: true })).toHaveCount(0); + await expect(page.getByRole('link', { name: 'Admin dashboard', exact: true })).toBeVisible(); + + const duration = await page.request.post(`/admin/appointment/${id}?/changeDuration`, { + ...postOptions, + form: { duration: '60' } + }); + const durationResult = await duration.json(); + expect(durationResult).toMatchObject({ type: 'failure', status: 409 }); + const move = await page.request.post(`/admin/appointment/${id}/reschedule`, { + ...postOptions, + form: { slot: `${day}T15:00:00Z`, duration: '30', reschedule_reason: 'Not available' } + }); + const moveResult = await move.json(); + expect(moveResult).toMatchObject({ type: 'failure', status: 409 }); + const afterView = await readAppointment(id); + expect(afterView).toEqual(beforeView); +}); + +test('missing or corrupt historical details return a deliberate 404 rather than a server crash', async ({ + page, + request +}) => { + await signInAsAdmin(page); + for (const snapshot of [null, '{broken']) { + const row = await seedAppointment({ eventTypeId: 'removed-unusable', status: 'confirmed' }); + if (snapshot !== null) { + const db = openDb(APP_DB_PATH); + try { + await db + .updateTable('appointments') + .set({ meeting_snapshot: snapshot }) + .where('id', '=', row.id) + .executeTakeFirstOrThrow(); + } finally { + await db.destroy(); + } + } + const response = await page.goto(`/appointment/${row.id}`); + expect(response?.status()).toBe(404); + await expect( + page.getByText('Saved meeting details are unavailable.', { exact: true }) + ).toBeVisible(); + const guest = await request.get(`/appointment/${row.id}?token=${row.cancel_token}`); + expect(guest.status()).toBe(404); + } +}); diff --git a/apps/web/src/lib/server/appointment/snapshot.test.ts b/apps/web/src/lib/server/appointment/snapshot.test.ts new file mode 100644 index 00000000..3bfd8b51 --- /dev/null +++ b/apps/web/src/lib/server/appointment/snapshot.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, test } from 'vitest'; +import { readMeetingSnapshot } from './snapshot'; + +describe('readMeetingSnapshot', () => { + test('reads only display details from the partial snapshot written by booking', () => { + const snapshot = JSON.stringify({ + title: '30-minute chat', + duration_minutes: 30, + description: 'A quick introduction.', + slug: 'chat' + }); + expect(readMeetingSnapshot(snapshot)).toEqual({ + title: '30-minute chat', + description: 'A quick introduction.' + }); + }); + + test('accepts a title without a description or scheduling settings', () => { + expect(readMeetingSnapshot(JSON.stringify({ title: 'Historical meeting' }))).toEqual({ + title: 'Historical meeting', + description: undefined + }); + }); + + test.each([null, 42, ['text'], { text: 'not a description' }])( + 'ignores non-text description %j', + (description) => { + expect( + readMeetingSnapshot(JSON.stringify({ title: 'Historical meeting', description })) + ).toEqual({ + title: 'Historical meeting', + description: undefined + }); + } + ); + + test('does not invent configuration from a full or forged snapshot', () => { + const snapshot = JSON.stringify({ + title: 'Historical meeting', + description: 'Saved description', + additional_duration_minutes: [999], + booking_calendar: 'missing', + schedule: 'missing', + require_approval: false, + video_chat: { provider: 'fake' } + }); + expect(readMeetingSnapshot(snapshot)).toEqual({ + title: 'Historical meeting', + description: 'Saved description' + }); + }); + + test('keeps saved text as data for normal escaped rendering', () => { + const title = 'Historical meeting'; + const description = ''; + expect(readMeetingSnapshot(JSON.stringify({ title, description }))).toEqual({ + title, + description + }); + }); + + test.each([ + null, + '', + '{broken', + 'null', + 'true', + '42', + '"meeting"', + '[]', + '{}', + '{"title":null}', + '{"title":42}', + '{"title":[]}', + '{"title":""}', + '{"title":" "}' + ])('returns no display details for unusable snapshot %j', (snapshot) => { + expect(readMeetingSnapshot(snapshot)).toBeNull(); + }); +}); diff --git a/apps/web/src/lib/server/appointment/snapshot.ts b/apps/web/src/lib/server/appointment/snapshot.ts new file mode 100644 index 00000000..8b1ae624 --- /dev/null +++ b/apps/web/src/lib/server/appointment/snapshot.ts @@ -0,0 +1,20 @@ +export interface MeetingSummary { + title: string; + description: string | undefined; +} + +export function readMeetingSnapshot(snapshot: string | null): MeetingSummary | null { + if (!snapshot) return null; + let value: unknown; + try { + value = JSON.parse(snapshot); + } catch { + return null; + } + if (value === null || typeof value !== 'object' || Array.isArray(value)) return null; + if (!('title' in value) || typeof value.title !== 'string') return null; + if (!value.title.trim()) return null; + const description = + 'description' in value && typeof value.description === 'string' ? value.description : undefined; + return { title: value.title, description }; +} diff --git a/apps/web/src/routes/(app)/appointment/[id]/+page.server.ts b/apps/web/src/routes/(app)/appointment/[id]/+page.server.ts index 9c77de84..8b42ab01 100644 --- a/apps/web/src/routes/(app)/appointment/[id]/+page.server.ts +++ b/apps/web/src/routes/(app)/appointment/[id]/+page.server.ts @@ -17,7 +17,8 @@ import { parseActionLog, type Appointment } from '@when/db'; -import { toPublicAppointment, toPublicEventType } from '$lib/server/appointment/sanitize'; +import { toPublicAppointment } from '$lib/server/appointment/sanitize'; +import { readMeetingSnapshot } from '$lib/server/appointment/snapshot'; import { resolveBookingDurations } from '$lib/server/appointment/duration'; import type { Actions, PageServerLoad } from './$types'; import { cancelAppointment } from '$lib/server/appointment/cancel'; @@ -60,20 +61,13 @@ export const load: PageServerLoad = async ({ params, url, locals, cookies }) => const cfg = getConfig(); const eventType = cfg.meetings[row.event_type_id]; - let resolvedEventType = eventType; - if (!resolvedEventType && isHost && row.meeting_snapshot) { - try { - resolvedEventType = JSON.parse(row.meeting_snapshot); - } catch { - // fallback - } - } + if (!eventType && !isHost) error(404); + const meeting = eventType + ? { title: eventType.title, description: eventType.description } + : readMeetingSnapshot(row.meeting_snapshot); + if (!meeting) error(404, 'Saved meeting details are unavailable.'); - if (!resolvedEventType) { - error(404); - } - - let actions = resolveAppointmentActions({ row, role, now, eventType: resolvedEventType }); + let actions = resolveAppointmentActions({ row, role, now, eventType }); if (!eventType) { actions = { cancel: { allowed: false, reason: 'terminal_status' }, @@ -90,9 +84,9 @@ export const load: PageServerLoad = async ({ params, url, locals, cookies }) => const bookedUrl = `${url.origin}/appointment/${row.id}${tokenEnc ? `?token=${tokenEnc}` : ''}`; const icsUrl = `${url.origin}/appointment/${row.id}/ics${tokenEnc ? `?token=${tokenEnc}` : ''}`; - const eventName = resolvedEventType?.title ?? row.event_type_id; + const eventName = meeting.title; const calendarLinks = - row.status === 'confirmed' && clockStatus !== 'concluded' + eventType && row.status === 'confirmed' && clockStatus !== 'concluded' ? buildAddToCalendarLinks( { start: row.start_time, @@ -129,17 +123,20 @@ export const load: PageServerLoad = async ({ params, url, locals, cookies }) => : false; const videoChatRequested = isHost && + Boolean(eventType) && isActiveStatus(row.status) && !row.video_chat && row.video_chat_request !== 'suppress' && (row.video_chat_request === 'generate' || automaticVideoChat); - const bookingDurations = resolveBookingDurations(resolvedEventType, role, row); + const bookingDurations = eventType ? resolveBookingDurations(eventType, role, row) : null; return { appointment, videoChatRequested, - eventType: toPublicEventType(row.event_type_id, resolvedEventType, role, bookingDurations), + meeting, + meetingUnavailable: !eventType, + bookingDurations, canEditDuration: isHost && Boolean(eventType) && isEditAllowed(row, now), calendarLinks, actions, diff --git a/apps/web/src/routes/(app)/appointment/[id]/+page.svelte b/apps/web/src/routes/(app)/appointment/[id]/+page.svelte index ffa59fa0..8059ed18 100644 --- a/apps/web/src/routes/(app)/appointment/[id]/+page.svelte +++ b/apps/web/src/routes/(app)/appointment/[id]/+page.svelte @@ -134,7 +134,10 @@ return status; }); let isEditable = $derived( - displayStatus === 'pending' || displayStatus === 'confirmed' || displayStatus === 'in_progress' + !data.meetingUnavailable && + (displayStatus === 'pending' || + displayStatus === 'confirmed' || + displayStatus === 'in_progress') ); let stateTone = $derived.by(() => { if ( @@ -199,6 +202,15 @@
+ {#if data.meetingUnavailable} + + {/if} {#if form?.error}
-{#if data.canEditDuration} +{#if data.canEditDuration && data.bookingDurations} {/if} diff --git a/apps/web/src/routes/(app)/appointment/[id]/server.test.ts b/apps/web/src/routes/(app)/appointment/[id]/server.test.ts index 59e913b0..16edb040 100644 --- a/apps/web/src/routes/(app)/appointment/[id]/server.test.ts +++ b/apps/web/src/routes/(app)/appointment/[id]/server.test.ts @@ -4,7 +4,6 @@ import { validConfig } from '$lib/server/__fixtures__/valid-config'; import type { Appointment } from '@when/db'; import type { AppointmentRole } from '$lib/server/appointment/actions'; import { resolveBookingDurations } from '$lib/server/appointment/duration'; -import { toPublicEventType } from '$lib/server/appointment/sanitize'; const h = vi.hoisted(() => ({ cancelAppointment: vi.fn() })); @@ -13,10 +12,6 @@ vi.mock('$lib/server/appointment/duration', async (importOriginal) => { const original = await importOriginal(); return { ...original, resolveBookingDurations: vi.fn(original.resolveBookingDurations) }; }); -vi.mock('$lib/server/appointment/sanitize', async (importOriginal) => { - const original = await importOriginal(); - return { ...original, toPublicEventType: vi.fn(original.toPublicEventType) }; -}); beforeEach(() => vi.clearAllMocks()); @@ -48,11 +43,23 @@ const mockAppt: Appointment = { calendar_synced_revision: null, has_possible_conflict: 0, ics_sequence: 0, - meeting_snapshot: JSON.stringify(validConfig.meetings['30-min-chat']), + meeting_snapshot: JSON.stringify({ + title: validConfig.meetings['30-min-chat'].title, + duration_minutes: 30, + description: validConfig.meetings['30-min-chat'].description, + slug: '30-min-chat' + }), created_at: '', updated_at: '' }; +const unusableSnapshots: Record = { + 'appt-deleted-no-snapshot': null, + 'appt-deleted-invalid-json': '{broken', + 'appt-deleted-invalid-title': JSON.stringify({ title: 42 }), + 'appt-deleted-primitive': 'null' +}; + const bookedAppointment: Appointment = { ...mockAppt, id: 'appt-long', @@ -181,7 +188,21 @@ vi.mock('@when/db', async (importOriginal) => { start_time: '2020-01-01T00:00:00Z', end_time: '2099-01-01T00:00:00Z' }; - if (id === 'appt-deleted') return { ...mockAppt, id: 'appt-deleted', event_type_id: 'gone' }; + if (id === 'appt-deleted') + return { + ...mockAppt, + id, + event_type_id: 'gone', + video_chat_request: 'generate', + end_time: bookedAppointment.end_time + }; + if (id in unusableSnapshots) + return { + ...mockAppt, + id, + event_type_id: 'gone', + meeting_snapshot: unusableSnapshots[id] + }; return chain[id] ?? null; }, findChainTip: async (_db: unknown, origin: string) => @@ -226,16 +247,11 @@ describe('/appointment/[id] server load', () => { expect(resolveBookingDurations).toHaveBeenCalledExactlyOnceWith(meeting, role, row); expect(vi.mocked(resolveBookingDurations).mock.calls[0][2]).toBe(row); const bookingDurations = vi.mocked(resolveBookingDurations).mock.results[0].value; - expect(toPublicEventType).toHaveBeenCalledExactlyOnceWith( - row.event_type_id, - meeting, - role, - bookingDurations - ); - expect(vi.mocked(toPublicEventType).mock.calls[0][3]).toBe(bookingDurations); - expect(result.eventType.duration_minutes).toBe(90); - expect(result.eventType.durations).toBe(bookingDurations.durations); - expect(result.eventType.durations).toContain(90); + expect(result.bookingDurations).toBe(bookingDurations); + expect(result.bookingDurations?.defaultDuration).toBe(90); + expect(result.bookingDurations?.durations).toContain(90); + expect(result.meeting).toEqual({ title: meeting.title, description: meeting.description }); + expect(result.meetingUnavailable).toBe(false); } ); @@ -284,7 +300,7 @@ describe('/appointment/[id] server load', () => { } as unknown as Parameters[0])) as Exclude>, void>; expect(result.appointment.id).toBe('appt-1'); - expect(result.eventType.slug).toBe('30-min-chat'); + expect(result.meeting.title).toBe(validConfig.meetings['30-min-chat'].title); expect(result.actions.cancel.allowed).toBe(true); }); @@ -331,34 +347,41 @@ describe('/appointment/[id] server load', () => { ).rejects.toThrow(); }); - test('renders for admin when event type is deleted from config (loads via snapshot)', async () => { - const mockLocals = { - auth: vi.fn().mockResolvedValue({ user: { name: 'admin' } }) - }; - - const url = new URL('http://localhost/appointment/appt-deleted'); - const mockCookies = { - get: vi.fn().mockReturnValue(undefined), - delete: vi.fn() - }; - - const result = (await load({ - url, - locals: mockLocals, - params: { id: 'appt-deleted' }, - cookies: mockCookies - } as unknown as Parameters[0])) as Exclude>, void>; - + test('shows the host real partial snapshot details without treating them as configuration', async () => { + const result = await loadAppointment('appt-deleted', null, 'host'); expect(result.appointment.id).toBe('appt-deleted'); - // Should parse and return details from mockAppt.meeting_snapshot - expect(result.eventType.title).toBe('30 minute chat'); - // Actions must be locked down to disabled - expect(result.actions.cancel.allowed).toBe(false); - expect(result.actions.reschedule.allowed).toBe(false); - // Delete must be allowed (constraints lifted) + expect(result.appointment.end_time).toBe(bookedAppointment.end_time); + expect(result.meeting).toEqual({ + title: validConfig.meetings['30-min-chat'].title, + description: validConfig.meetings['30-min-chat'].description + }); + expect(result.meetingUnavailable).toBe(true); + expect(result.bookingDurations).toBeNull(); + expect(resolveBookingDurations).not.toHaveBeenCalled(); + for (const action of [ + 'cancel', + 'reschedule', + 'accept', + 'decline', + 'generateVideoChat' + ] as const) { + expect(result.actions[action].allowed).toBe(false); + } + expect(result.canEditDuration).toBe(false); + expect(result.calendarLinks).toBeNull(); + expect(result.videoChatRequested).toBe(false); expect(result.deleteCheck?.terminal).toBe(true); }); + test.for(Object.keys(unusableSnapshots))( + 'returns a deliberate 404 for unusable snapshot %s', + async (id) => { + const response = loadAppointment(id, null, 'host'); + await expect(response).rejects.toMatchObject({ status: 404 }); + expect(resolveBookingDurations).not.toHaveBeenCalled(); + } + ); + test('computes clockStatus upcoming and builds calendarLinks for upcoming appointments', async () => { const result = await loadAppointment('appt-1', 'tok-abc'); expect(result.clockStatus).toBe('upcoming'); -- 2.54.0