From 2a9eb86505cbed67a35167154b662a99aaae74f5 Mon Sep 17 00:00:00 2001 From: pataniaeli Date: Tue, 15 Sep 2026 20:58:57 -0400 Subject: [PATCH 1/3] feat: add a Meeting Time distinct from the reservation window (#126) A booking's start and end times describe the reservation -- the window the room is held for, which is what CSC is told. They are not when the body meets: a committee that books 6:00-9:00 to allow for setup still tells its members 6:30. Every member-facing surface reported the reservation window, so every one of them answered the wrong question. Meeting Time is a `time` on the tables that own functional times -- the weekly series, its per-week occurrences, and one-time and tabling sessions. Nullable everywhere, with NULL meaning inherit and resolving in the end to start_time, the same convention weekly_room_occurrences already uses for room, time and status. Nothing needs backfilling and no insert path can break: a booking with no meeting time set reads exactly as it did before the column existed. The Slack committee reminder now states the meeting time alone rather than the reservation window, as the issue asks. My Rooms leads its cards, list rows and calendar entries with it, naming the window only when the two differ so the cards keep the height issue #78 settled on; the detail modal shows both, always. Both booking emails carry it, and a week whose only edit was to move when it meets now counts as moved, so its members are told. Applied to the database as migration `meeting_time`; rollback script included. Co-Authored-By: Claude Opus 5 --- app/(dashboard)/bookings/bookings-tab.tsx | 5 + .../bookings/edit-one-time-form.tsx | 18 ++++ .../bookings/edit-tabling-form.tsx | 23 +++- app/(dashboard)/bookings/edit-weekly-form.tsx | 51 ++++++++- app/(dashboard)/bookings/one-time-form.tsx | 16 +++ app/(dashboard)/bookings/tabling-form.tsx | 16 +++ .../bookings/weekly-booking-grid.tsx | 3 + app/(dashboard)/bookings/weekly-form.tsx | 24 +++++ .../my-rooms/booking-detail-modal.tsx | 14 ++- app/(dashboard)/my-rooms/calendar-view.tsx | 12 ++- app/(dashboard)/my-rooms/my-rooms-client.tsx | 30 +++++- app/(dashboard)/my-rooms/shared.ts | 22 ++++ .../administrator/bookings/one-time/route.ts | 19 +++- app/api/administrator/bookings/route.ts | 8 +- .../administrator/bookings/tabling/route.ts | 20 +++- .../administrator/bookings/weekly/route.ts | 46 ++++++-- app/api/cron/slack-reminders/route.ts | 6 +- lib/emails/booking-created.ts | 24 ++++- lib/emails/booking-updated.ts | 20 +++- lib/meeting-reminders.ts | 28 ++++- lib/meeting-time.ts | 102 ++++++++++++++++++ lib/my-rooms-data.ts | 8 +- lib/weekly-occurrences.ts | 10 +- .../20260915000000_meeting_time.sql | 88 +++++++++++++++ .../20260915_meeting_time_rollback.sql | 20 ++++ 25 files changed, 595 insertions(+), 38 deletions(-) create mode 100644 lib/meeting-time.ts create mode 100644 supabase/migrations/20260915000000_meeting_time.sql create mode 100644 supabase/migrations/rollback/20260915_meeting_time_rollback.sql diff --git a/app/(dashboard)/bookings/bookings-tab.tsx b/app/(dashboard)/bookings/bookings-tab.tsx index dcb30fc..02ea7a7 100644 --- a/app/(dashboard)/bookings/bookings-tab.tsx +++ b/app/(dashboard)/bookings/bookings-tab.tsx @@ -53,6 +53,7 @@ interface OneTimeBooking extends BookingBase { booking_date: string start_time: string end_time: string + meeting_time: string | null status: string reservation_code: string | null }[] | null @@ -66,6 +67,7 @@ interface WeeklyBooking extends BookingBase { end_date: string start_time: string end_time: string + meeting_time: string | null status: string reservation_code: string | null weekly_room_occurrences: { @@ -74,6 +76,8 @@ interface WeeklyBooking extends BookingBase { room_name: string | null start_time: string | null end_time: string | null + /** Overrides the series meeting time for this date; null inherits (issue #126). */ + meeting_time: string | null status: string | null reservation_code: string | null senate_type: string | null @@ -97,6 +101,7 @@ interface TablingBooking extends BookingBase { session_date: string start_time: string end_time: string + meeting_time: string | null status: string reservation_code: string | null }[] diff --git a/app/(dashboard)/bookings/edit-one-time-form.tsx b/app/(dashboard)/bookings/edit-one-time-form.tsx index 6ee0083..ea81c8b 100644 --- a/app/(dashboard)/bookings/edit-one-time-form.tsx +++ b/app/(dashboard)/bookings/edit-one-time-form.tsx @@ -38,6 +38,8 @@ interface OneTimeSession { booking_date: string start_time: string end_time: string + /** Blank means the session meets when its reservation starts (issue #126). */ + meeting_time: string status: string reservation_code: string } @@ -47,6 +49,7 @@ const emptySession = (): OneTimeSession => ({ booking_date: '', start_time: '', end_time: '', + meeting_time: '', status: 'Reserved', reservation_code: '', }) @@ -65,6 +68,7 @@ interface EditOneTimeFormProps { booking_date: string start_time: string end_time: string + meeting_time: string | null status: string reservation_code: string | null }[] | null @@ -96,6 +100,7 @@ export default function EditOneTimeForm({ booking, bodies, onClose, onSuccess }: booking_date: d.booking_date ?? '', start_time: d.start_time.slice(0, 5) ?? '', end_time: d.end_time.slice(0, 5) ?? '', + meeting_time: d.meeting_time?.slice(0, 5) ?? '', status: d.status ?? 'Reserved', reservation_code: d.reservation_code ?? '', })) ?? [emptySession()] @@ -221,6 +226,19 @@ export default function EditOneTimeForm({ booking, bodies, onClose, onSuccess }: + {/* When the meeting itself starts, as opposed to when the room is + held. Tracks the start time until touched; the server stores + nothing when the two agree (issue #126). */} +
+ +
+ updateSession(i, 'meeting_time', v)} + /> +
+
+
+ {/* See weekly-form.tsx for why this tracks the start time rather than sitting blank (issue #126). */} +
+ +
+ setForm({ ...form, meeting_time: v })} + /> +
+

+ When the meeting itself starts. Leave it on the start time unless the room is held early for setup. +

+
+
setForm({ ...form, reservation_code: e.target.value })} className={inputCls} /> @@ -267,7 +289,8 @@ export default function EditWeeklyForm({ booking, bodies, initialExpandedOcc, on {getWeeklyDates(form.start_date, form.end_date).map(date => { const occ = occurrences.find(o => o.occurrence_date === date) // `hidden != null` because false is an override, not an absence. - const hasOverride = occ && (occ.room_name || occ.start_time || occ.end_time || occ.status + const hasOverride = occ && (occ.room_name || occ.start_time || occ.end_time + || occ.meeting_time || occ.status || occ.reservation_code || occ.purpose || occ.hidden != null) const isExpanded = expandedOcc === date @@ -320,6 +343,32 @@ export default function EditWeeklyForm({ booking, bodies, initialExpandedOcc, on
+ {/* + Shows what this week currently resolves to -- its own meeting + time, else the series', else whichever start time applies to + it -- and writes an override the moment it is touched, exactly + as the room and time overrides above do. + + Not run through meetingTimeForStorage on save: null here means + "inherit the series", not "meet at the start time", so a week + that genuinely meets at its own start time has to say so + explicitly (issue #126). + */} +
+ +
+ updateOccurrence(occ.id, 'meeting_time', v)} + /> +
+
+
({ session_date: '', start_time: '09:00', end_time: '10:00', + meeting_time: '', status: 'Reserved', }) @@ -276,6 +279,19 @@ export default function TablingForm({ bodies, semesters, onClose, onSuccess }: T
+ {/* When the meeting itself starts, as opposed to when the room is + held. Tracks the start time until touched; the server stores + nothing when the two agree (issue #126). */} +
+ +
+ updateSession(i, 'meeting_time', v)} + /> +
+
+
Date {formatDate(booking.date)}
+ {/* + Both rows, always, even when they carry the same time. This is the + detail view -- the one place someone comes to find out exactly what + was booked -- so it is worth a line to say that the meeting starts at + one time and the room is held from another, rather than collapsing + them the way the cards do and leaving the distinction unexplained + (issue #126). + */}
- Time + Meeting Time + {formatTime(booking.meetingTime)} +
+
+ Reserved {formatTime(booking.startTime)} – {formatTime(booking.endTime)}
{booking.reservationCode && ( diff --git a/app/(dashboard)/my-rooms/calendar-view.tsx b/app/(dashboard)/my-rooms/calendar-view.tsx index 276d8e4..1cf47a5 100644 --- a/app/(dashboard)/my-rooms/calendar-view.tsx +++ b/app/(dashboard)/my-rooms/calendar-view.tsx @@ -8,6 +8,7 @@ import { statusTextColors, formatTime, } from './shared' +import { meetingTimeMatchesStart } from '@/lib/meeting-time' interface CalendarViewProps { bookings: FlatBooking[] @@ -168,7 +169,16 @@ export default function CalendarView({ bookings, onSelect, today }: CalendarView {b.scopeLabel} )} -

{b.location} · {formatTime(b.startTime)} – {formatTime(b.endTime)}

+ {/* Meeting time first, reservation window only when it differs (issue #126). */} +

+ {b.location} ·{' '} + {formatTime(b.meetingTime)} + {meetingTimeMatchesStart(b.meetingTime, b.startTime) ? ( + <> – {formatTime(b.endTime)} + ) : ( + <> (reserved {formatTime(b.startTime)} – {formatTime(b.endTime)}) + )} +

{b.status} diff --git a/app/(dashboard)/my-rooms/my-rooms-client.tsx b/app/(dashboard)/my-rooms/my-rooms-client.tsx index cbbb85c..e23622f 100644 --- a/app/(dashboard)/my-rooms/my-rooms-client.tsx +++ b/app/(dashboard)/my-rooms/my-rooms-client.tsx @@ -22,6 +22,7 @@ import { formatTime, formatDate, } from './shared' +import { meetingTimeMatchesStart } from '@/lib/meeting-time' function MyRoomsSkeleton() { return ( @@ -267,8 +268,25 @@ export default function MyRoomsClient({ {b.scopeLabel} · {b.location}

{formatDate(b.date)}

+ {/* + The meeting time leads, because it is the one a member acts + on -- the reservation window is when the room is held, which + is Chambers' concern rather than theirs (issue #126). When no + distinct meeting time is set the two are the same, so the + line reads exactly as it always did with the start bolded; + only a booking that genuinely meets later spends the extra + words saying what the room is held for. Either way it is one + line, so the card keeps the height issue #78 settled on. + */}
-

{formatTime(b.startTime)} – {formatTime(b.endTime)}

+

+ {formatTime(b.meetingTime)} + {meetingTimeMatchesStart(b.meetingTime, b.startTime) ? ( + <> – {formatTime(b.endTime)} + ) : ( + <> · reserved {formatTime(b.startTime)} – {formatTime(b.endTime)} + )} +

{b.senateType && ( {b.senateType} )} @@ -376,7 +394,15 @@ export default function MyRoomsClient({ {b.senateType} )}
-

{b.location} · {formatDate(b.date)} · {formatTime(b.startTime)} – {formatTime(b.endTime)}

+

+ {b.location} · {formatDate(b.date)} ·{' '} + {formatTime(b.meetingTime)} + {meetingTimeMatchesStart(b.meetingTime, b.startTime) ? ( + <> – {formatTime(b.endTime)} + ) : ( + <> (reserved {formatTime(b.startTime)} – {formatTime(b.endTime)}) + )} +

{b.type === 'One-Time Room' ? 'One-Time/Multiple Room' : b.type} {b.status} diff --git a/app/(dashboard)/my-rooms/shared.ts b/app/(dashboard)/my-rooms/shared.ts index ff3fbda..ee00e7b 100644 --- a/app/(dashboard)/my-rooms/shared.ts +++ b/app/(dashboard)/my-rooms/shared.ts @@ -1,5 +1,6 @@ import { formatScopeLabel, type BookingScope, type Division } from '@/lib/booking-scope' import { APP_TIME_ZONE } from '@/lib/app-zone' +import { resolveMeetingTime } from '@/lib/meeting-time' export interface FlatBooking { id: string @@ -12,6 +13,12 @@ export interface FlatBooking { date: string startTime: string endTime: string + /** + * When the meeting itself starts, already resolved through its inheritance + * chain, so a row always has one (issue #126). Falls back to startTime, which + * is what every surface here reported before the field existed. + */ + meetingTime: string status: string reservationCode: string | null senateType: string | null @@ -238,6 +245,9 @@ export function flattenMyRooms(data: MyRoomsResponse, today: string): FlatBookin date: d.booking_date, startTime: d.start_time, endTime: d.end_time, + // A one-time session has no series above it, so the chain is two + // levels: its own meeting time, or its own start time (issue #126). + meetingTime: resolveMeetingTime(d.meeting_time, d.start_time), status: d.status, reservationCode: d.reservation_code, senateType: null, @@ -268,6 +278,15 @@ export function flattenMyRooms(data: MyRoomsResponse, today: string): FlatBookin date: occ.occurrence_date, startTime: occ.start_time || w.start_time, endTime: occ.end_time || w.end_time, + // Three levels, most specific first: this week's override, the series + // value, then the start time this week actually resolved to -- so a + // week that moved its start time and set no meeting time reports the + // moved time rather than the series' (issue #126). + meetingTime: resolveMeetingTime( + occ.meeting_time, + w.meeting_time, + occ.start_time || w.start_time + ), status: occ.status || w.status, reservationCode: occ.reservation_code || w.reservation_code, senateType: occ.senate_type ?? null, @@ -294,6 +313,9 @@ export function flattenMyRooms(data: MyRoomsResponse, today: string): FlatBookin date: s.session_date, startTime: s.start_time, endTime: s.end_time, + // As for one-time: each tabling session carries its own date and times, + // so there is no parent value to inherit (issue #126). + meetingTime: resolveMeetingTime(s.meeting_time, s.start_time), status: s.status, reservationCode: s.reservation_code || t.reservation_code, senateType: null, diff --git a/app/api/administrator/bookings/one-time/route.ts b/app/api/administrator/bookings/one-time/route.ts index 113f984..ee1480a 100644 --- a/app/api/administrator/bookings/one-time/route.ts +++ b/app/api/administrator/bookings/one-time/route.ts @@ -4,6 +4,7 @@ import { NextResponse } from 'next/server' import { sendMissedReservationEmail } from '@/lib/emails/missed-reservation' import { sendBookingUpdatedEmail } from '@/lib/emails/booking-updated' import { sendBookingCreatedEmail } from '@/lib/emails/booking-created' +import { meetingTimeForStorage, resolveMeetingTime } from '@/lib/meeting-time' import { changed, collectChanges, formatDate, formatTime } from '@/lib/emails/changes' import { checkRateLimit } from '@/lib/check-rate-limit' import { planInvites } from '@/lib/room-calendar' @@ -30,6 +31,8 @@ interface OneTimeSession { booking_date: string start_time: string end_time: string + /** Blank means the session meets when its reservation starts (issue #126). */ + meeting_time: string status: string reservation_code: string } @@ -95,6 +98,7 @@ export async function POST(request: Request) { booking_date: s.booking_date, start_time: s.start_time, end_time: s.end_time, + meeting_time: meetingTimeForStorage(s.meeting_time, s.start_time), reservation_code: s.reservation_code || null, status: s.status, })) @@ -141,10 +145,11 @@ export async function POST(request: Request) { purpose, roomOrTable: sessionRows[0]?.room_name || 'N/A', status: sessionRows[0]?.status ?? 'Reserved', - sessions: sessionRows.map((r: { booking_date: string; start_time: string; end_time: string; room_name: string | null }) => ({ + sessions: sessionRows.map((r: { booking_date: string; start_time: string; end_time: string; meeting_time: string | null; room_name: string | null }) => ({ date: r.booking_date, startTime: r.start_time, endTime: r.end_time, + meetingTime: resolveMeetingTime(r.meeting_time, r.start_time), roomOrTable: r.room_name, })), recipients: audience.recipients, @@ -184,7 +189,7 @@ export async function PATCH(request: Request) { adminSupabase.from('bookings').select('purpose').eq('id', booking_id).single(), adminSupabase .from('one_time_room_bookings') - .select('id, room_name, booking_date, start_time, end_time, status, reservation_code') + .select('id, room_name, booking_date, start_time, end_time, meeting_time, status, reservation_code') .eq('booking_id', booking_id) .order('booking_date', { ascending: true }), ]) @@ -213,6 +218,7 @@ export async function PATCH(request: Request) { booking_date: s.booking_date, start_time: s.start_time, end_time: s.end_time, + meeting_time: meetingTimeForStorage(s.meeting_time, s.start_time), reservation_code: s.reservation_code || null, status: s.status, })) @@ -318,6 +324,14 @@ export async function PATCH(request: Request) { changed('Date', prevFirst?.booking_date, firstSession.booking_date, formatDate), changed('Start time', prevFirst?.start_time, firstSession.start_time, formatTime), changed('End time', prevFirst?.end_time, firstSession.end_time, formatTime), + // Effective values on both sides, so a session that has never set a + // meeting time does not report one when its start time moves (#126). + changed( + 'Meeting time', + resolveMeetingTime(prevFirst?.meeting_time, prevFirst?.start_time), + resolveMeetingTime(firstSession.meeting_time, firstSession.start_time), + formatTime + ), changed('Status', prevFirst?.status, firstSession.status), changed('Reservation code', prevFirst?.reservation_code, firstSession.reservation_code), ) @@ -348,6 +362,7 @@ export async function PATCH(request: Request) { date: firstSession.booking_date, startTime: firstSession.start_time, endTime: firstSession.end_time, + meetingTime: resolveMeetingTime(firstSession.meeting_time, firstSession.start_time), status: firstSession.status, changes, recipients: audience.recipients, diff --git a/app/api/administrator/bookings/route.ts b/app/api/administrator/bookings/route.ts index ceca428..7614241 100644 --- a/app/api/administrator/bookings/route.ts +++ b/app/api/administrator/bookings/route.ts @@ -42,7 +42,7 @@ export async function GET(request: Request) { bodies(name), booking_bodies(body_id, bodies(name)), creator_role, - one_time_room_bookings(id, room_name, booking_date, start_time, end_time, status, reservation_code) + one_time_room_bookings(id, room_name, booking_date, start_time, end_time, meeting_time, status, reservation_code) `) .eq('type', 'One-Time Room') .order('created_at', { ascending: false }) @@ -55,8 +55,8 @@ export async function GET(request: Request) { bodies(name), booking_bodies(body_id, bodies(name)), creator_role, - weekly_room_bookings(id, room_name, start_date, end_date, start_time, end_time, status, reservation_code, - weekly_room_occurrences(id, occurrence_date, room_name, start_time, end_time, status, reservation_code, senate_type, purpose, hidden, is_event) + weekly_room_bookings(id, room_name, start_date, end_date, start_time, end_time, meeting_time, status, reservation_code, + weekly_room_occurrences(id, occurrence_date, room_name, start_time, end_time, meeting_time, status, reservation_code, senate_type, purpose, hidden, is_event) ) `) .eq('type', 'Weekly Room') @@ -71,7 +71,7 @@ export async function GET(request: Request) { booking_bodies(body_id, bodies(name)), creator_role, tabling_bookings(id, reservation_code, - tabling_sessions(id, location, session_date, start_time, end_time, status, reservation_code) + tabling_sessions(id, location, session_date, start_time, end_time, meeting_time, status, reservation_code) ) `) .eq('type', 'Tabling') diff --git a/app/api/administrator/bookings/tabling/route.ts b/app/api/administrator/bookings/tabling/route.ts index 6b18f36..7128435 100644 --- a/app/api/administrator/bookings/tabling/route.ts +++ b/app/api/administrator/bookings/tabling/route.ts @@ -4,6 +4,7 @@ import { NextResponse } from 'next/server' import { sendMissedReservationEmail } from '@/lib/emails/missed-reservation' import { sendBookingUpdatedEmail } from '@/lib/emails/booking-updated' import { sendBookingCreatedEmail } from '@/lib/emails/booking-created' +import { meetingTimeForStorage, resolveMeetingTime } from '@/lib/meeting-time' import { changed, collectChanges, formatDate, formatTime } from '@/lib/emails/changes' import { checkRateLimit } from '@/lib/check-rate-limit' import { getAuthedUserWithLiveRoles } from '@/lib/authorization' @@ -90,6 +91,7 @@ export async function POST(request: Request) { session_date: string start_time: string end_time: string + meeting_time: string | null reservation_code: string status: string }) => ({ @@ -98,6 +100,7 @@ export async function POST(request: Request) { session_date: s.session_date, start_time: s.start_time, end_time: s.end_time, + meeting_time: meetingTimeForStorage(s.meeting_time, s.start_time), reservation_code: s.reservation_code || null, status: s.status, })) @@ -133,10 +136,11 @@ export async function POST(request: Request) { purpose, roomOrTable: sessionRows[0]?.location || 'N/A', status: sessionRows[0]?.status ?? 'Reserved', - sessions: sessionRows.map((r: { session_date: string; start_time: string; end_time: string; location: string }) => ({ + sessions: sessionRows.map((r: { session_date: string; start_time: string; end_time: string; meeting_time: string | null; location: string }) => ({ date: r.session_date, startTime: r.start_time, endTime: r.end_time, + meetingTime: resolveMeetingTime(r.meeting_time, r.start_time), roomOrTable: r.location, })), recipients: recipients.map(r => r.email), @@ -155,6 +159,8 @@ interface Session { session_date: string start_time: string end_time: string + /** Blank means the session meets when its reservation starts (issue #126). */ + meeting_time: string | null status: string reservation_code: string | null } @@ -182,7 +188,7 @@ export async function PATCH(request: Request) { adminSupabase.from('tabling_bookings').select('reservation_code').eq('id', tabling_id).single(), adminSupabase .from('tabling_sessions') - .select('location, session_date, start_time, end_time, status') + .select('location, session_date, start_time, end_time, meeting_time, status') .eq('tabling_booking_id', tabling_id) .order('session_date', { ascending: true }), ]) @@ -226,6 +232,7 @@ export async function PATCH(request: Request) { session_date: s.session_date, start_time: s.start_time, end_time: s.end_time, + meeting_time: meetingTimeForStorage(s.meeting_time, s.start_time), status: s.status, reservation_code: s.reservation_code || null, })) @@ -288,6 +295,14 @@ export async function PATCH(request: Request) { changed('Date', prevFirst?.session_date, sessions[0]?.session_date, formatDate), changed('Start time', prevFirst?.start_time, sessions[0]?.start_time, formatTime), changed('End time', prevFirst?.end_time, sessions[0]?.end_time, formatTime), + // Effective values on both sides, so a session that has never set a + // meeting time does not report one when its start time moves (#126). + changed( + 'Meeting time', + resolveMeetingTime(prevFirst?.meeting_time, prevFirst?.start_time), + resolveMeetingTime(sessions[0]?.meeting_time, sessions[0]?.start_time), + formatTime + ), ) await sendBookingUpdatedEmail({ @@ -297,6 +312,7 @@ export async function PATCH(request: Request) { date: sessions[0]?.session_date ?? '', startTime: sessions[0]?.start_time ?? '', endTime: sessions[0]?.end_time ?? '', + meetingTime: resolveMeetingTime(sessions[0]?.meeting_time, sessions[0]?.start_time ?? ''), status: statusSummary, changes, recipients: emails, diff --git a/app/api/administrator/bookings/weekly/route.ts b/app/api/administrator/bookings/weekly/route.ts index d599813..511ed7b 100644 --- a/app/api/administrator/bookings/weekly/route.ts +++ b/app/api/administrator/bookings/weekly/route.ts @@ -6,6 +6,7 @@ import { sendBookingUpdatedEmail } from '@/lib/emails/booking-updated' import { sendBookingCreatedEmail } from '@/lib/emails/booking-created' import { changed, collectChanges, formatDate, formatTime } from '@/lib/emails/changes' import { occurrenceMoved } from '@/lib/weekly-occurrences' +import { meetingTimeForStorage, resolveMeetingTime } from '@/lib/meeting-time' import { checkRateLimit } from '@/lib/check-rate-limit' import { planInvites } from '@/lib/room-calendar' import { appToday, sendPerAudience, weeklyRoomSessions, type OccurrenceRow } from '@/lib/room-invites' @@ -33,6 +34,7 @@ interface OccurrenceInput { room_name: string | null start_time: string | null end_time: string | null + meeting_time: string | null status: string | null reservation_code: string | null senate_type: string | null @@ -54,6 +56,7 @@ interface PrevOccurrenceRow { room_name: string | null start_time: string | null end_time: string | null + meeting_time: string | null status: string | null reservation_code: string | null purpose: string | null @@ -86,7 +89,7 @@ export async function POST(request: Request) { const rateLimitRes = await checkRateLimit(user.id) if (rateLimitRes) return rateLimitRes - const { body_id, purpose, room_name, start_date, end_date, start_time, end_time, reservation_code, status, semester_id, scope, division, body_ids } = await request.json() + const { body_id, purpose, room_name, start_date, end_date, start_time, end_time, meeting_time, reservation_code, status, semester_id, scope, division, body_ids } = await request.json() const { data: semester } = await adminSupabase .from('semesters') @@ -132,7 +135,10 @@ export async function POST(request: Request) { // Create weekly room booking const { data: weekly, error: weeklyError } = await adminSupabase .from('weekly_room_bookings') - .insert({ booking_id: booking.id, room_name, start_date, end_date, start_time, end_time, reservation_code: reservation_code || null, status }) + // meeting_time collapses to null when it matches the start time, so the two + // stay tied together until an administrator genuinely separates them + // (issue #126). + .insert({ booking_id: booking.id, room_name, start_date, end_date, start_time, end_time, meeting_time: meetingTimeForStorage(meeting_time, start_time), reservation_code: reservation_code || null, status }) .select() .single() @@ -200,7 +206,12 @@ export async function POST(request: Request) { dateRange: { start: start_date, end: end_date }, // Freshly generated, so every occurrence carries the series' room and // time -- there are no per-week overrides to report yet. - sessions: dates.map(d => ({ date: d, startTime: start_time, endTime: end_time })), + sessions: dates.map(d => ({ + date: d, + startTime: start_time, + endTime: end_time, + meetingTime: resolveMeetingTime(meeting_time, start_time), + })), recipients: audience.recipients, invite: audience.plan, }) @@ -225,7 +236,7 @@ export async function PATCH(request: Request) { const rateLimitRes = await checkRateLimit(user.id) if (rateLimitRes) return rateLimitRes - const { booking_id, weekly_id, body_id, purpose, room_name, start_date, end_date, start_time, end_time, reservation_code, status, occurrences, scope, division, body_ids } = await request.json() + const { booking_id, weekly_id, body_id, purpose, room_name, start_date, end_date, start_time, end_time, meeting_time, reservation_code, status, occurrences, scope, division, body_ids } = await request.json() const ctx = await loadScopeContext(supabase, user) const selection = validateScopeSelection(ctx, { scope, body_id, division, body_ids }) @@ -239,13 +250,13 @@ export async function PATCH(request: Request) { adminSupabase.from('bookings').select('purpose').eq('id', booking_id).single(), adminSupabase .from('weekly_room_bookings') - .select('room_name, start_date, end_date, start_time, end_time, status, reservation_code') + .select('room_name, start_date, end_date, start_time, end_time, meeting_time, status, reservation_code') .eq('id', weekly_id) .single(), adminSupabase .from('weekly_room_occurrences') .select( - 'id, occurrence_date, room_name, start_time, end_time, status, reservation_code, purpose, senate_type, hidden, is_event' + 'id, occurrence_date, room_name, start_time, end_time, meeting_time, status, reservation_code, purpose, senate_type, hidden, is_event' ) .eq('weekly_booking_id', weekly_id), ]) @@ -272,7 +283,7 @@ export async function PATCH(request: Request) { // Update weekly booking base fields const { error: weeklyError } = await adminSupabase .from('weekly_room_bookings') - .update({ room_name, start_date, end_date, start_time, end_time, reservation_code: reservation_code || null, status }) + .update({ room_name, start_date, end_date, start_time, end_time, meeting_time: meetingTimeForStorage(meeting_time, start_time), reservation_code: reservation_code || null, status }) .eq('id', weekly_id) if (weeklyError) return NextResponse.json({ error: weeklyError.message }, { status: 500 }) @@ -286,6 +297,10 @@ export async function PATCH(request: Request) { room_name: existing?.room_name || null, start_time: existing?.start_time || null, end_time: existing?.end_time || null, + // Null inherits the series meeting time, which itself falls back to the + // start time -- so clearing this week's override restores whatever the + // series says rather than blanking the meeting (issue #126). + meeting_time: existing?.meeting_time || null, status: existing?.status || null, reservation_code: existing?.reservation_code || null, senate_type: existing?.senate_type ?? null, @@ -387,6 +402,15 @@ export async function PATCH(request: Request) { changed('End date', prevWeekly?.end_date, end_date, formatDate), changed('Start time', prevWeekly?.start_time, start_time, formatTime), changed('End time', prevWeekly?.end_time, end_time, formatTime), + // Compared as effective values, so a series that has never set a meeting + // time does not report one the first time a start time moves: both sides + // resolve through the same fallback (issue #126). + changed( + 'Meeting time', + resolveMeetingTime(prevWeekly?.meeting_time, prevWeekly?.start_time), + resolveMeetingTime(meeting_time, start_time), + formatTime + ), changed('Status', prevWeekly?.status, status), changed('Reservation code', prevWeekly?.reservation_code, reservation_code || null), ) @@ -439,6 +463,12 @@ export async function PATCH(request: Request) { changed('Room', prev?.room_name ?? prevWeekly?.room_name, occ.room_name ?? room_name), changed('Start time', prev?.start_time ?? prevWeekly?.start_time, occ.start_time ?? start_time, formatTime), changed('End time', prev?.end_time ?? prevWeekly?.end_time, occ.end_time ?? end_time, formatTime), + changed( + 'Meeting time', + resolveMeetingTime(prev?.meeting_time, prevWeekly?.meeting_time, prev?.start_time ?? prevWeekly?.start_time), + resolveMeetingTime(occ.meeting_time, meeting_time, occ.start_time ?? start_time), + formatTime + ), changed('Status', prev?.status ?? prevWeekly?.status, occ.status ?? status), changed('Purpose', prev?.purpose ?? prevBooking?.purpose, occ.purpose ?? purpose), changed('Reservation code', prev?.reservation_code ?? prevWeekly?.reservation_code, occ.reservation_code ?? (reservation_code || null)), @@ -450,6 +480,7 @@ export async function PATCH(request: Request) { date: occ.occurrence_date, startTime: occ.start_time || start_time, endTime: occ.end_time || end_time, + meetingTime: resolveMeetingTime(occ.meeting_time, meeting_time, occ.start_time || start_time), roomOrTable: occ.room_name || room_name || 'N/A', status: occ.status || status, purpose: occ.purpose ?? purpose, @@ -498,6 +529,7 @@ export async function PATCH(request: Request) { date: start_date, startTime: start_time, endTime: end_time, + meetingTime: resolveMeetingTime(meeting_time, start_time), status, changes: seriesChanges, sessions, diff --git a/app/api/cron/slack-reminders/route.ts b/app/api/cron/slack-reminders/route.ts index 89e916c..98b5ab9 100644 --- a/app/api/cron/slack-reminders/route.ts +++ b/app/api/cron/slack-reminders/route.ts @@ -41,9 +41,9 @@ const adminSupabase = createAdminClient( * which is what lets the body-level filters below narrow the occurrence rows. */ const SELECT = ` - occurrence_date, room_name, start_time, end_time, status, hidden, weekly_booking_id, + occurrence_date, room_name, start_time, end_time, meeting_time, status, hidden, weekly_booking_id, weekly_room_bookings!inner( - room_name, start_time, end_time, status, + room_name, start_time, end_time, meeting_time, status, bookings!inner( hidden, bodies!inner(name, body_type, slack_channel_id, slack_reminders_enabled) @@ -102,6 +102,7 @@ export async function GET(request: Request) { room_name: row.room_name, start_time: row.start_time, end_time: row.end_time, + meeting_time: row.meeting_time, status: row.status, hidden: row.hidden, weekly_booking_id: row.weekly_booking_id, @@ -109,6 +110,7 @@ export async function GET(request: Request) { room_name: series.room_name, start_time: series.start_time, end_time: series.end_time, + meeting_time: series.meeting_time, status: series.status, }, booking: { hidden: booking.hidden }, diff --git a/lib/emails/booking-created.ts b/lib/emails/booking-created.ts index f91258c..1b8f24c 100644 --- a/lib/emails/booking-created.ts +++ b/lib/emails/booking-created.ts @@ -4,6 +4,7 @@ import { formatDate, formatTime } from './changes' import { icsSequenceNow } from './ics-core' import { roomIcsAttachments } from './room-ics' import type { InvitePlan } from '@/lib/room-calendar' +import { meetingTimeMatchesStart } from '@/lib/meeting-time' /** * One dated slot on the booking. A one-time booking can carry several, a weekly @@ -13,9 +14,28 @@ export interface BookingSession { date: string startTime: string endTime: string + /** + * When the meeting itself starts, already resolved (issue #126). Required so + * a caller cannot omit it; it is only *printed* when it differs from + * startTime, since otherwise the reservation window on the same line already + * says it and a semester of identical "meets at" suffixes is noise. + */ + meetingTime: string roomOrTable?: string | null } +/** + * "meets 6:30 PM", preceded by `sep`, or nothing at all when the meeting starts + * with the reservation. The separator is a parameter because the two bodies of + * this email spell it differently -- a literal middot in the text part, the HTML + * entity in the markup. + */ +function meetsSuffix(s: BookingSession, sep: string): string { + return meetingTimeMatchesStart(s.meetingTime, s.startTime) + ? '' + : `${sep}meets ${formatTime(s.meetingTime)}` +} + interface BookingCreatedEmailParams { bodyName: string bookingType: 'One-Time Room' | 'Weekly Room' | 'Tabling' @@ -60,13 +80,13 @@ export async function sendBookingCreatedEmail(params: BookingCreatedEmailParams) const remaining = sessions.length - listed.length const sessionLinesText = listed - .map(s => ` ${formatDate(s.date)} · ${formatTime(s.startTime)} to ${formatTime(s.endTime)}${ + .map(s => ` ${formatDate(s.date)} · ${formatTime(s.startTime)} to ${formatTime(s.endTime)}${meetsSuffix(s, ' · ')}${ s.roomOrTable ? ` · ${sanitize(s.roomOrTable)}` : '' }`) .join('\n') const sessionLinesHtml = listed - .map(s => `${formatDate(s.date)} · ${formatTime(s.startTime)} to ${formatTime(s.endTime)}${ + .map(s => `${formatDate(s.date)} · ${formatTime(s.startTime)} to ${formatTime(s.endTime)}${meetsSuffix(s, ' · ')}${ s.roomOrTable ? ` · ${sanitize(s.roomOrTable)}` : '' }`) .join('') diff --git a/lib/emails/booking-updated.ts b/lib/emails/booking-updated.ts index 063471e..f3faf6d 100644 --- a/lib/emails/booking-updated.ts +++ b/lib/emails/booking-updated.ts @@ -16,6 +16,12 @@ export interface UpdatedSession { date: string startTime: string endTime: string + /** + * When the meeting itself starts, already resolved against the series + * (issue #126). Required rather than optional so a caller cannot quietly omit + * it and send an email that describes only the reservation window. + */ + meetingTime: string roomOrTable: string status: string purpose?: string | null @@ -31,6 +37,8 @@ interface BookingUpdatedEmailParams { date: string startTime: string endTime: string + /** The series' resolved meeting time (issue #126). */ + meetingTime: string status: string recipients: string[] /** @@ -83,6 +91,7 @@ function renderSession(session: UpdatedSession): { text: string; html: string } ...(session.purpose ? [{ label: 'Purpose', value: sanitize(session.purpose) }] : []), { label: 'Room/Table', value: sanitize(session.roomOrTable) }, { label: 'Time', value: `${formatTime(session.startTime)} to ${formatTime(session.endTime)}` }, + { label: 'Meeting time', value: formatTime(session.meetingTime) }, { label: 'Status', value: sanitize(session.status) }, ]) @@ -104,7 +113,7 @@ ${details.text.split('\n').map(l => ` ${l}`).join('\n')}`, export async function sendBookingUpdatedEmail(params: BookingUpdatedEmailParams) { const { - bodyName, roomOrTable, date, startTime, endTime, status, recipients, + bodyName, roomOrTable, date, startTime, endTime, meetingTime, status, recipients, changes = [], sessions = null, purpose = null, invite = null, } = params if (!recipients.length) return @@ -120,7 +129,7 @@ export async function sendBookingUpdatedEmail(params: BookingUpdatedEmailParams) ? buildMultiSession(sBodyName, moved) : moved.length === 1 ? buildSingleSession(sBodyName, moved[0]) - : buildSeries(sBodyName, sPurpose, { roomOrTable, date, startTime, endTime, status }, changes) + : buildSeries(sBodyName, sPurpose, { roomOrTable, date, startTime, endTime, meetingTime, status }, changes) await resend.emails.send({ from: emailFrom(), @@ -145,7 +154,7 @@ If you have questions, please reach out to sgaOperations@northeastern.edu.`, function buildSeries( sBodyName: string, sPurpose: string | null, - now: { roomOrTable: string; date: string; startTime: string; endTime: string; status: string }, + now: { roomOrTable: string; date: string; startTime: string; endTime: string; meetingTime: string; status: string }, changes: BookingChange[] ) { const rendered = renderChanges(changes) @@ -154,7 +163,11 @@ function buildSeries( { label: 'Body', value: sBodyName }, { label: 'Room/Table', value: sanitize(now.roomOrTable) }, { label: 'Date', value: formatDate(now.date) }, + // Kept next to the reservation window rather than replacing it: the window + // is what Chambers holds and what a body needs if it is setting up early, + // and the meeting time is the one its members act on (issue #126). { label: 'Time', value: `${formatTime(now.startTime)} to ${formatTime(now.endTime)}` }, + { label: 'Meeting time', value: formatTime(now.meetingTime) }, { label: 'Status', value: sanitize(now.status) }, ]) @@ -179,6 +192,7 @@ function buildSingleSession(sBodyName: string, session: UpdatedSession) { { label: 'Room/Table', value: sanitize(session.roomOrTable) }, { label: 'Session date', value: formatDate(session.date) }, { label: 'Time', value: `${formatTime(session.startTime)} to ${formatTime(session.endTime)}` }, + { label: 'Meeting time', value: formatTime(session.meetingTime) }, { label: 'Status', value: sanitize(session.status) }, ...(session.position ? [{ label: 'Session', value: sanitize(session.position) }] : []), ]) diff --git a/lib/meeting-reminders.ts b/lib/meeting-reminders.ts index dbcf421..b8cf658 100644 --- a/lib/meeting-reminders.ts +++ b/lib/meeting-reminders.ts @@ -1,4 +1,5 @@ import { APP_TIME_ZONE } from '@/lib/app-zone' +import { resolveMeetingTime } from '@/lib/meeting-time' /** * Working out which committee meetings the Slack bot should remind a channel @@ -69,6 +70,7 @@ export interface ReminderCandidate { room_name: string | null start_time: string | null end_time: string | null + meeting_time: string | null status: string | null hidden: boolean | null weekly_booking_id: string @@ -76,6 +78,7 @@ export interface ReminderCandidate { room_name: string | null start_time: string | null end_time: string | null + meeting_time: string | null status: string | null } booking: { @@ -96,6 +99,13 @@ export interface ResolvedMeeting { roomName: string | null startTime: string | null endTime: string | null + /** + * The time the meeting itself starts, already resolved (issue #126). This is + * the only time the reminder prints; startTime and endTime stay on the shape + * because the resolution below falls back to startTime and because a future + * reminder may want to say what the room is held for. + */ + meetingTime: string | null status: string } @@ -118,14 +128,21 @@ export function resolveMeeting(c: ReminderCandidate): ResolvedMeeting | null { const status = c.status ?? c.series.status if (!status || !REMINDED_STATUSES.has(status)) return null + const startTime = c.start_time ?? c.series.start_time + return { weeklyBookingId: c.weekly_booking_id, date: c.occurrence_date, channelId: c.body.slack_channel_id, bodyName: c.body.name, roomName: c.room_name ?? c.series.room_name, - startTime: c.start_time ?? c.series.start_time, + startTime, endTime: c.end_time ?? c.series.end_time, + // Same precedence as everything above it, with one extra level on the end: + // a series that has never had a meeting time set falls back to the start + // time this week resolved to, so the reminder reads exactly as it did + // before the field existed (issue #126). + meetingTime: resolveMeetingTime(c.meeting_time, c.series.meeting_time, startTime), status, } } @@ -174,9 +191,12 @@ export function formatReminder(m: ResolvedMeeting): string { return [opening, 'Check with your Chair/Director for virtual meeting information.'].join('\n') } - const start = formatTime(m.startTime) - const end = formatTime(m.endTime) - const when = start && end ? `${start}–${end}` : start ?? 'to be confirmed' + // The meeting time alone, not the reservation window (issue #126). The window + // is what Chambers holds the room for -- it usually opens before the meeting + // does and runs past the end of it -- and printing it here told a channel to + // turn up at a time nobody meant. Still labelled "Time", the colloquial + // reading the issue asks for. + const when = formatTime(m.meetingTime) ?? 'to be confirmed' const room = m.roomName ? esc(m.roomName) : 'not yet confirmed' const roomLabel = ALTERNATE_ROOM.has(m.status) ? '*Alternate* Room' : 'Room' diff --git a/lib/meeting-time.ts b/lib/meeting-time.ts new file mode 100644 index 0000000..5cb8bdb --- /dev/null +++ b/lib/meeting-time.ts @@ -0,0 +1,102 @@ +/** + * Resolving a booking's Meeting Time (issue #126). + * + * A booking has always been described by the window its room is reserved for -- + * start_time to end_time. That window is a fact about the reservation, not about + * the meeting: a body that books 6:00-9:00 to allow for setup still tells its + * members to turn up at 6:30. Meeting Time is that second number, stored + * alongside the window rather than replacing it, so the reservation keeps saying + * what CSC was told while every member-facing surface can say when to arrive. + * + * Kept in its own module because the precedence below has five callers across + * My Rooms, the admin editors, both booking emails and the Slack reminder, and + * an inheritance rule that is applied inconsistently is worse than one that is + * wrong everywhere. + */ + +/** + * The value that actually applies, given each level from most specific to least. + * + * NULL at any level means inherit, exactly as it does for the room, status and + * time overrides that weekly_room_occurrences already carries, so the first + * level that is genuinely set wins. Callers pass their own chain: + * + * weekly resolveMeetingTime(occ.meeting_time, series.meeting_time, startTime) + * one-time resolveMeetingTime(session.meeting_time, session.start_time) + * tabling resolveMeetingTime(session.meeting_time, session.start_time) + * + * Ending the chain with the resolved start_time is what makes this safe to roll + * out against a table full of rows that predate the column: a booking with no + * meeting time set reads as meeting when its reservation begins, which is the + * assumption every one of these surfaces made before Meeting Time existed. + * + * Variadic rather than a fixed (override, base, fallback) signature because the + * chains are genuinely different lengths -- a one-time session has no series + * above it to inherit from -- and padding the short ones with nulls would read + * as though a level had been forgotten. + * + * The first overload says that a chain ending in a plain string cannot resolve + * to null, so the callers that end theirs with a NOT NULL start_time get + * `string` back rather than having to assert away a null that cannot happen. + */ +export function resolveMeetingTime( + ...levels: [...(string | null | undefined)[], string] +): string +export function resolveMeetingTime( + ...levels: (string | null | undefined)[] +): string | null +export function resolveMeetingTime( + ...levels: (string | null | undefined)[] +): string | null { + for (const level of levels) { + // Empty string is treated as unset, not as a value. The editors normalise a + // cleared field to null before it is written, but a blank that slips through + // should inherit rather than render as an empty time. + if (level != null && level !== '') return level + } + return null +} + +/** + * True when the meeting starts exactly when the reservation does. + * + * Postgres hands back `time` as 'HH:MM:SS' while the editors submit 'HH:MM', so + * the two are compared at minute precision rather than as raw strings -- without + * this, '18:30:00' and '18:30' read as a meeting time that differs from the + * start time, and every surface that flags an unusual one would flag all of them. + */ +export function meetingTimeMatchesStart( + meetingTime: string | null | undefined, + startTime: string | null | undefined +): boolean { + if (!meetingTime || !startTime) return true + return meetingTime.slice(0, 5) === startTime.slice(0, 5) +} + +/** + * What to store for a meeting time whose inheritance chain falls straight + * through to `startTime` -- a weekly series, or a one-time or tabling session. + * + * The editors show this field pre-filled with the start time rather than blank, + * because "meets at 6, room held from 6" is the truth for most bookings and an + * empty box invites someone to retype it. That means an untouched field submits + * a value identical to the start time, and storing it would pin the meeting time + * to a number the start time no longer has to agree with: move the reservation + * to 7:00 later and the booking would still claim to meet at 6:00. + * + * Collapsing the two back to NULL keeps them tied together until somebody + * genuinely separates them, and loses nothing -- NULL resolves to the start time, + * so both forms render identically. + * + * Deliberately NOT for a weekly occurrence's override, where NULL means "inherit + * the series" rather than "meet at the start time". Collapsing there would turn + * a week that really does meet at its start time into a week that follows a + * series meeting at some other one. + */ +export function meetingTimeForStorage( + meetingTime: string | null | undefined, + startTime: string | null | undefined +): string | null { + if (!meetingTime) return null + return meetingTimeMatchesStart(meetingTime, startTime) ? null : meetingTime +} diff --git a/lib/my-rooms-data.ts b/lib/my-rooms-data.ts index 7ff1432..9b5c6e7 100644 --- a/lib/my-rooms-data.ts +++ b/lib/my-rooms-data.ts @@ -123,7 +123,7 @@ export async function fetchMyRooms( .from('bookings') .select(` ${SELECT_BASE}, - one_time_room_bookings(id, room_name, booking_date, start_time, end_time, status, reservation_code) + one_time_room_bookings(id, room_name, booking_date, start_time, end_time, meeting_time, status, reservation_code) `) .eq('type', 'One-Time Room') .eq('semester_id', activeSemesterId) @@ -134,8 +134,8 @@ export async function fetchMyRooms( .from('bookings') .select(` ${SELECT_BASE}, - weekly_room_bookings(id, room_name, start_time, end_time, status, reservation_code, - weekly_room_occurrences(id, occurrence_date, room_name, start_time, end_time, status, reservation_code, senate_type, purpose, hidden, is_event) + weekly_room_bookings(id, room_name, start_time, end_time, meeting_time, status, reservation_code, + weekly_room_occurrences(id, occurrence_date, room_name, start_time, end_time, meeting_time, status, reservation_code, senate_type, purpose, hidden, is_event) ) `) .eq('type', 'Weekly Room') @@ -148,7 +148,7 @@ export async function fetchMyRooms( .select(` ${SELECT_BASE}, tabling_bookings(id, reservation_code, - tabling_sessions(id, location, session_date, start_time, end_time, status, reservation_code) + tabling_sessions(id, location, session_date, start_time, end_time, meeting_time, status, reservation_code) ) `) .eq('type', 'Tabling') diff --git a/lib/weekly-occurrences.ts b/lib/weekly-occurrences.ts index 8887f1c..a60306a 100644 --- a/lib/weekly-occurrences.ts +++ b/lib/weekly-occurrences.ts @@ -20,9 +20,13 @@ * senate_type, hidden and is_event are compared even though the update email has * no row for them: they still say *which* week an administrator touched, which * is the question this module exists to answer. + * + * meeting_time is in the list for the plainer reason that the email does report + * it (issue #126), and a week whose only edit was to move when it meets has to + * count as moved or nobody is told. */ export const OCCURRENCE_FIELDS = [ - 'room_name', 'start_time', 'end_time', 'status', + 'room_name', 'start_time', 'end_time', 'meeting_time', 'status', 'reservation_code', 'purpose', 'senate_type', 'hidden', 'is_event', ] as const @@ -48,7 +52,9 @@ export function normalizeOccurrenceValue(field: OccurrenceField, value: unknown) if (typeof value === 'boolean') return value ? 'true' : 'false' const s = String(value).trim() if (!s) return null - return field === 'start_time' || field === 'end_time' ? s.slice(0, 5) : s + return field === 'start_time' || field === 'end_time' || field === 'meeting_time' + ? s.slice(0, 5) + : s } /** diff --git a/supabase/migrations/20260915000000_meeting_time.sql b/supabase/migrations/20260915000000_meeting_time.sql new file mode 100644 index 0000000..9083be7 --- /dev/null +++ b/supabase/migrations/20260915000000_meeting_time.sql @@ -0,0 +1,88 @@ +-- Meeting Time, distinct from the reserved start and end times (issue #126). +-- +-- start_time and end_time describe the *reservation*: the window the room is +-- held for, which is what CSC is told and what the calendar draws. They are not +-- the same thing as when the meeting actually begins, and a body that reserves +-- 6:00–9:00 so it can set up may well tell its members to arrive at 6:30. +-- Until now the only time anyone could be shown was the reservation window, so +-- every member-facing surface reported the wrong answer to "when do we meet?". +-- +-- Meeting Time is a `time`, not free text, so it sorts, formats through the same +-- helpers as the other two, and cannot drift into "6ish". +-- +-- --------------------------------------------------------------------------- +-- Why every column is nullable, and what NULL means +-- --------------------------------------------------------------------------- +-- NULL means inherit, resolving in the end to start_time. That is exactly the +-- convention weekly_room_occurrences already uses for room_name, start_time, +-- end_time and status, and that 20260829000000_occurrence_purpose_and_hidden.sql +-- extended to purpose and hidden. +-- +-- Two things follow from it, both deliberate: +-- +-- * No backfill is needed. Every existing row keeps behaving exactly as it +-- does today -- a booking with no meeting time set reads as meeting at its +-- start time, which is the assumption the whole app made before this column +-- existed. Nothing has to be migrated, and nothing changes for a body that +-- never sets one. +-- +-- * No insert path can break. A NOT NULL column would have had to be supplied +-- by six write paths across three booking types, and any one of them missed +-- is a 500 in production rather than a field left blank. +-- +-- The cost is that a weekly occurrence cannot say "this week, meet at the start +-- time" while its series says otherwise -- NULL there means inherit, so the +-- series value wins. Setting the occurrence's meeting time explicitly to the +-- start time expresses the same thing, which is why this is not worth the third +-- state that `hidden` needed. (`hidden` had no such escape: a boolean has only +-- two values and both were already spoken for.) +-- +-- --------------------------------------------------------------------------- +-- Why these four tables and not `bookings` +-- --------------------------------------------------------------------------- +-- The issue asks for the field on "every booking, session, and occurrence". +-- Those three map onto the tables that own functional times, and `bookings` is +-- not one of them -- it carries purpose, scope and visibility, and has never +-- held a time of any kind: +-- +-- booking -> weekly_room_bookings (the series; the weekly base value) +-- session -> one_time_room_bookings (one row per date of a one-time booking) +-- -> tabling_sessions (one row per date of a tabling booking) +-- occurrence -> weekly_room_occurrences (per-week override of the series) +-- +-- One-time and tabling sessions each carry their own date and their own start +-- and end times, so each is independently overridable by definition and has no +-- parent time to inherit from. Only the weekly series/occurrence pair needs two +-- levels, and it gets them. + +-- --------------------------------------------------------------------------- +-- Weekly: the series value, and the per-week override of it +-- --------------------------------------------------------------------------- + +alter table public.weekly_room_bookings + add column if not exists meeting_time time; + +comment on column public.weekly_room_bookings.meeting_time is + 'When the meeting itself starts, as opposed to when the room reservation does. NULL inherits start_time.'; + +alter table public.weekly_room_occurrences + add column if not exists meeting_time time; + +comment on column public.weekly_room_occurrences.meeting_time is + 'Overrides weekly_room_bookings.meeting_time for this week. NULL inherits, resolving to the series meeting_time and then to start_time.'; + +-- --------------------------------------------------------------------------- +-- One-time and tabling: per-session, with nothing above them to inherit from +-- --------------------------------------------------------------------------- + +alter table public.one_time_room_bookings + add column if not exists meeting_time time; + +comment on column public.one_time_room_bookings.meeting_time is + 'When the meeting itself starts, as opposed to when the room reservation does. NULL inherits this session''s start_time.'; + +alter table public.tabling_sessions + add column if not exists meeting_time time; + +comment on column public.tabling_sessions.meeting_time is + 'When the session itself starts, as opposed to when the table reservation does. NULL inherits this session''s start_time.'; diff --git a/supabase/migrations/rollback/20260915_meeting_time_rollback.sql b/supabase/migrations/rollback/20260915_meeting_time_rollback.sql new file mode 100644 index 0000000..66ca806 --- /dev/null +++ b/supabase/migrations/rollback/20260915_meeting_time_rollback.sql @@ -0,0 +1,20 @@ +-- Rollback for 20260915000000_meeting_time.sql. +-- +-- Drops all four columns. Any meeting times that had been set are lost, and +-- every booking falls back to reporting its start_time as the time it meets -- +-- which is precisely what the app did before the migration, so nothing breaks. +-- The only visible effect is that a body which had set a meeting time distinct +-- from its reservation window goes back to being announced at the reservation +-- window. + +alter table public.weekly_room_bookings + drop column if exists meeting_time; + +alter table public.weekly_room_occurrences + drop column if exists meeting_time; + +alter table public.one_time_room_bookings + drop column if exists meeting_time; + +alter table public.tabling_sessions + drop column if exists meeting_time; From 93ba6ca4d0cb9ad769b16cb09ea421d863d254ee Mon Sep 17 00:00:00 2001 From: pataniaeli Date: Fri, 18 Sep 2026 10:52:41 -0400 Subject: [PATCH 2/3] chore: rename the #126 migration to the version production recorded Its file carried 20260915000000, the version dev's space_booking_series migration already uses. Production recorded meeting_time as 20260916005401, so the file is named after that, as #119 and #140 did for theirs. Contents unchanged. Co-Authored-By: Claude Opus 5 --- ...5000000_meeting_time.sql => 20260916005401_meeting_time.sql} | 0 ...ing_time_rollback.sql => 20260916_meeting_time_rollback.sql} | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename supabase/migrations/{20260915000000_meeting_time.sql => 20260916005401_meeting_time.sql} (100%) rename supabase/migrations/rollback/{20260915_meeting_time_rollback.sql => 20260916_meeting_time_rollback.sql} (93%) diff --git a/supabase/migrations/20260915000000_meeting_time.sql b/supabase/migrations/20260916005401_meeting_time.sql similarity index 100% rename from supabase/migrations/20260915000000_meeting_time.sql rename to supabase/migrations/20260916005401_meeting_time.sql diff --git a/supabase/migrations/rollback/20260915_meeting_time_rollback.sql b/supabase/migrations/rollback/20260916_meeting_time_rollback.sql similarity index 93% rename from supabase/migrations/rollback/20260915_meeting_time_rollback.sql rename to supabase/migrations/rollback/20260916_meeting_time_rollback.sql index 66ca806..a0cfea1 100644 --- a/supabase/migrations/rollback/20260915_meeting_time_rollback.sql +++ b/supabase/migrations/rollback/20260916_meeting_time_rollback.sql @@ -1,4 +1,4 @@ --- Rollback for 20260915000000_meeting_time.sql. +-- Rollback for 20260916005401_meeting_time.sql. -- -- Drops all four columns. Any meeting times that had been set are lost, and -- every booking falls back to reporting its start_time as the time it meets -- From 19a35c0aa0a89b42460b9aca03cdf703ee00c7bb Mon Sep 17 00:00:00 2001 From: pataniaeli Date: Fri, 18 Sep 2026 13:12:04 -0400 Subject: [PATCH 3/3] fix: give Start Time its own line on My Rooms cards instead of bolding it (#126) Cards led the time line with the meeting time in bold, run together with the reservation window. That was hard to read. The start time and the reservation now sit on separate labelled lines, "Start Time" and "Reserved", with no bold, and both always show so every card has the same shape. The Senate session badge moves beside the date to make room. The list and calendar rows lose their emphasis too. Where the two times differ they are labelled, since two unmarked times in a row do not say which is which; where they match the row reads as it always did. The detail modal calls the field Start Time, unbolded, as the cards do. Co-Authored-By: Claude Opus 5 --- .../my-rooms/booking-detail-modal.tsx | 4 +- app/(dashboard)/my-rooms/calendar-view.tsx | 8 ++-- app/(dashboard)/my-rooms/my-rooms-client.tsx | 45 +++++++++---------- 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/app/(dashboard)/my-rooms/booking-detail-modal.tsx b/app/(dashboard)/my-rooms/booking-detail-modal.tsx index aba297c..285cc30 100644 --- a/app/(dashboard)/my-rooms/booking-detail-modal.tsx +++ b/app/(dashboard)/my-rooms/booking-detail-modal.tsx @@ -103,8 +103,8 @@ export default function BookingDetailModal({ booking, isLeadership, onClose, onC (issue #126). */}
- Meeting Time - {formatTime(booking.meetingTime)} + Start Time + {formatTime(booking.meetingTime)}
Reserved diff --git a/app/(dashboard)/my-rooms/calendar-view.tsx b/app/(dashboard)/my-rooms/calendar-view.tsx index 1cf47a5..e600a6a 100644 --- a/app/(dashboard)/my-rooms/calendar-view.tsx +++ b/app/(dashboard)/my-rooms/calendar-view.tsx @@ -169,14 +169,14 @@ export default function CalendarView({ bookings, onSelect, today }: CalendarView {b.scopeLabel} )}
- {/* Meeting time first, reservation window only when it differs (issue #126). */} + {/* The reservation window as always; the start time is named only when it differs from it (issue #126). */}

{b.location} ·{' '} - {formatTime(b.meetingTime)} {meetingTimeMatchesStart(b.meetingTime, b.startTime) ? ( - <> – {formatTime(b.endTime)} + <>{formatTime(b.startTime)} – {formatTime(b.endTime)} ) : ( - <> (reserved {formatTime(b.startTime)} – {formatTime(b.endTime)}) + // Labelled once they differ: two unmarked times in a row would not say which is which. + <>Start Time {formatTime(b.meetingTime)} · Reserved {formatTime(b.startTime)} – {formatTime(b.endTime)} )}

diff --git a/app/(dashboard)/my-rooms/my-rooms-client.tsx b/app/(dashboard)/my-rooms/my-rooms-client.tsx index e23622f..4b1f42e 100644 --- a/app/(dashboard)/my-rooms/my-rooms-client.tsx +++ b/app/(dashboard)/my-rooms/my-rooms-client.tsx @@ -267,30 +267,29 @@ export default function MyRoomsClient({

{b.scopeLabel} · {b.location}

-

{formatDate(b.date)}

- {/* - The meeting time leads, because it is the one a member acts - on -- the reservation window is when the room is held, which - is Chambers' concern rather than theirs (issue #126). When no - distinct meeting time is set the two are the same, so the - line reads exactly as it always did with the start bolded; - only a booking that genuinely meets later spends the extra - words saying what the room is held for. Either way it is one - line, so the card keeps the height issue #78 settled on. - */} -
-

- {formatTime(b.meetingTime)} - {meetingTimeMatchesStart(b.meetingTime, b.startTime) ? ( - <> – {formatTime(b.endTime)} - ) : ( - <> · reserved {formatTime(b.startTime)} – {formatTime(b.endTime)} - )} -

+
+

{formatDate(b.date)}

{b.senateType && ( {b.senateType} )}
+ {/* + Start Time and the reservation window on lines of their own, + each labelled, rather than run together with the start in + bold (issue #126). The start time is when members should + arrive; the reservation is when the room is held, often + earlier for setup. "Start Time" is the name members already + use for it, as the issue allows. + + Both lines always show, even when the two start together, so + every card in the grid has the same shape. + */} +
+
Start Time
+
{formatTime(b.meetingTime)}
+
Reserved
+
{formatTime(b.startTime)} – {formatTime(b.endTime)}
+
) })} @@ -396,11 +395,11 @@ export default function MyRoomsClient({

{b.location} · {formatDate(b.date)} ·{' '} - {formatTime(b.meetingTime)} {meetingTimeMatchesStart(b.meetingTime, b.startTime) ? ( - <> – {formatTime(b.endTime)} + <>{formatTime(b.startTime)} – {formatTime(b.endTime)} ) : ( - <> (reserved {formatTime(b.startTime)} – {formatTime(b.endTime)}) + // Labelled once they differ: two unmarked times in a row would not say which is which. + <>Start Time {formatTime(b.meetingTime)} · Reserved {formatTime(b.startTime)} – {formatTime(b.endTime)} )}