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', }) @@ -277,6 +280,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 + Start 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..e600a6a 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)}

+ {/* The reservation window as always; the start time is named only when it differs from it (issue #126). */} +

+ {b.location} ·{' '} + {meetingTimeMatchesStart(b.meetingTime, b.startTime) ? ( + <>{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)} + )} +

{b.status} diff --git a/app/(dashboard)/my-rooms/my-rooms-client.tsx b/app/(dashboard)/my-rooms/my-rooms-client.tsx index cbbb85c..4b1f42e 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 ( @@ -266,13 +267,29 @@ export default function MyRoomsClient({

{b.scopeLabel} · {b.location}

-

{formatDate(b.date)}

-
-

{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)}
+
) })} @@ -376,7 +393,15 @@ export default function MyRoomsClient({ {b.senateType} )} -

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

+

+ {b.location} · {formatDate(b.date)} ·{' '} + {meetingTimeMatchesStart(b.meetingTime, b.startTime) ? ( + <>{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)} + )} +

{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 4e8348c..6d84af1 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' @@ -32,6 +33,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 } @@ -97,6 +100,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, })) @@ -150,10 +154,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, @@ -193,7 +198,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 }), ]) @@ -222,6 +227,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, })) @@ -274,13 +280,15 @@ export async function PATCH(request: Request) { // on id, which they now keep across saves (issue #69), so a session is // compared with itself rather than with whichever one happened to sort first. type SessionValues = { - room: string | null; date: string; start: string; end: string; status: string; code: string | null + room: string | null; date: string; start: string; end: string; meeting: string; status: string; code: string | null } - const sessionValues = (r: { room_name: string | null; booking_date: string; start_time: string; end_time: string; status: string; reservation_code: string | null }): SessionValues => ({ + const sessionValues = (r: { room_name: string | null; booking_date: string; start_time: string; end_time: string; meeting_time: string | null; status: string; reservation_code: string | null }): SessionValues => ({ room: r.room_name || null, date: r.booking_date, start: r.start_time, end: r.end_time, + // Resolved, so a blank meeting time reads as the start time it means (#126). + meeting: resolveMeetingTime(r.meeting_time, r.start_time), status: r.status, code: r.reservation_code || null, }) @@ -289,11 +297,12 @@ export async function PATCH(request: Request) { { label: 'Date', get: v => v.date, format: formatDate }, { label: 'Start time', get: v => v.start, format: formatTime }, { label: 'End time', get: v => v.end, format: formatTime }, + { label: 'Meeting time', get: v => v.meeting, format: formatTime }, { label: 'Status', get: v => v.status }, { label: 'Reservation code', get: v => v.code }, ] - type PrevSession = { id: string; room_name: string | null; booking_date: string; start_time: string; end_time: string; status: string; reservation_code: string | null } + type PrevSession = { id: string; room_name: string | null; booking_date: string; start_time: string; end_time: string; meeting_time: string | null; status: string; reservation_code: string | null } const prevById = new Map(((prevSessions ?? []) as PrevSession[]).map(p => [p.id, p])) const auditRows: AuditRow[] = [] @@ -395,6 +404,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), ) @@ -425,6 +442,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 0ad95a1..ab95b95 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' @@ -93,6 +94,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 }) => ({ @@ -101,6 +103,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, })) @@ -143,10 +146,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), @@ -165,6 +169,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 } @@ -192,7 +198,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, reservation_code') + .select('location, session_date, start_time, end_time, meeting_time, status, reservation_code') .eq('tabling_booking_id', tabling_id) .order('session_date', { ascending: true }), ]) @@ -236,6 +242,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, })) @@ -253,13 +260,15 @@ export async function PATCH(request: Request) { // moved. Tabling still replaces every session on save, so ids cannot match // them up; pairByDate does it by day instead. type TablingValues = { - location: string; date: string; start: string; end: string; status: string; code: string | null + location: string; date: string; start: string; end: string; meeting: string; status: string; code: string | null } - const tablingValues = (r: { location: string; session_date: string; start_time: string; end_time: string; status: string; reservation_code: string | null }): TablingValues => ({ + const tablingValues = (r: { location: string; session_date: string; start_time: string; end_time: string; meeting_time: string | null; status: string; reservation_code: string | null }): TablingValues => ({ location: r.location, date: r.session_date, start: r.start_time, end: r.end_time, + // Resolved, so a blank meeting time reads as the start time it means (#126). + meeting: resolveMeetingTime(r.meeting_time, r.start_time), status: r.status, code: r.reservation_code || null, }) @@ -267,6 +276,7 @@ export async function PATCH(request: Request) { { label: 'Table', get: v => v.location }, { label: 'Start time', get: v => v.start, format: formatTime }, { label: 'End time', get: v => v.end, format: formatTime }, + { label: 'Meeting time', get: v => v.meeting, format: formatTime }, { label: 'Status', get: v => v.status }, { label: 'Reservation code', get: v => v.code }, ] @@ -360,6 +370,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({ @@ -369,6 +387,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 c709758..1e959b2 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 { diffFields, formatEvent, formatVisibility, insertAuditRows, type AuditField, type AuditRow } from '@/lib/audit' import { checkRateLimit } from '@/lib/check-rate-limit' import { planInvites } from '@/lib/room-calendar' @@ -35,6 +36,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 @@ -56,6 +58,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 @@ -88,7 +91,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') @@ -134,7 +137,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() @@ -209,7 +215,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, }) @@ -234,7 +245,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 }) @@ -248,13 +259,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), ]) @@ -281,7 +292,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 }) @@ -295,6 +306,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, @@ -390,6 +405,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), ) @@ -402,7 +426,7 @@ export async function PATCH(request: Request) { // not repeat what the series entry already says, while a cleared override // still reads as "old override -> series value", which is what the admin did. type WeekValues = { - room: string | null; start: string | null; end: string | null; status: string | null + room: string | null; start: string | null; end: string | null; meeting: string | null; status: string | null purpose: string | null; code: string | null; senate: string | null hidden: boolean | null; event: boolean } @@ -410,6 +434,9 @@ export async function PATCH(request: Request) { room: o?.room_name ?? room_name, start: o?.start_time ?? start_time, end: o?.end_time ?? end_time, + // Resolved, so a week with no meeting time of its own reads as whatever it + // actually meets at -- the series' meeting time, else its start (#126). + meeting: resolveMeetingTime(o?.meeting_time, meeting_time, o?.start_time ?? start_time), status: o?.status ?? status, purpose: o?.purpose ?? purpose, code: o?.reservation_code ?? (reservation_code || null), @@ -421,6 +448,7 @@ export async function PATCH(request: Request) { { label: 'Room', get: v => v.room }, { label: 'Start time', get: v => v.start, format: formatTime }, { label: 'End time', get: v => v.end, format: formatTime }, + { label: 'Meeting time', get: v => v.meeting, format: formatTime }, { label: 'Status', get: v => v.status }, { label: 'Purpose', get: v => v.purpose }, { label: 'Reservation code', get: v => v.code }, @@ -502,6 +530,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)), @@ -513,6 +547,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, @@ -561,6 +596,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/20260916005401_meeting_time.sql b/supabase/migrations/20260916005401_meeting_time.sql new file mode 100644 index 0000000..9083be7 --- /dev/null +++ b/supabase/migrations/20260916005401_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/20260916_meeting_time_rollback.sql b/supabase/migrations/rollback/20260916_meeting_time_rollback.sql new file mode 100644 index 0000000..a0cfea1 --- /dev/null +++ b/supabase/migrations/rollback/20260916_meeting_time_rollback.sql @@ -0,0 +1,20 @@ +-- 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 -- +-- 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;