diff --git a/.env.example b/.env.example index ac59394..065740a 100644 --- a/.env.example +++ b/.env.example @@ -8,7 +8,12 @@ UPSTASH_REDIS_REST_URL= UPSTASH_REDIS_REST_TOKEN= # Random string used to authenticate kiosk display pages (e.g. display_chambers_2026) DISPLAY_KEY= -# Slack app credentials (from the Slack app's Basic Information page) +# Slack app credentials (from the Slack app's Basic Information page). +# +# The bot token needs chat:write to post committee meeting reminders, and the +# bot has to be invited to each committee's channel -- Slack answers a post to a +# channel it is not in with ok:false, not an HTTP error, so this fails quietly. +# Add chat:write.public if you would rather not invite it to each one. SLACK_BOT_TOKEN= SLACK_SIGNING_SECRET= @@ -20,5 +25,11 @@ SLACK_SIGNING_SECRET= # it will send anything. Spell it exactly -- a misnamed variable reads as unset. CSC_EMAIL= -# Shared secret for the scheduled warm-up route (/api/cron/warm). +# Shared secret for the scheduled routes, /api/cron/warm and +# /api/cron/slack-reminders. Both are driven by GitHub Actions, which send it as +# a bearer token, so it must also be set as a CRON_SECRET repo secret. +# +# /api/cron/warm only does trivial reads and still answers without it. +# /api/cron/slack-reminders writes and posts to Slack, so it refuses to run at +# all unless this is set. CRON_SECRET= diff --git a/.github/workflows/slack-reminders.yml b/.github/workflows/slack-reminders.yml new file mode 100644 index 0000000..34299da --- /dev/null +++ b/.github/workflows/slack-reminders.yml @@ -0,0 +1,36 @@ +name: slack-reminders + +# Hits /api/cron/slack-reminders on production, which posts tomorrow's committee +# meetings to each committee's Slack channel (issue #95). +# +# A GitHub Action rather than a Vercel cron, for the same reason keep-warm.yml is +# one: Hobby plans cap Vercel crons at once per day, and this needs to retry. +# +# The schedule fires five times because GitHub schedules lag and are occasionally +# skipped altogether, and a reminder that silently never arrives is the failure +# this feature is most likely to have. The endpoint is idempotent -- it posts +# nothing before 9am Eastern, and records each reminder it sends so a later run +# the same morning finds the work already done -- so the extra firings cost a +# no-op request each. +# +# 13:00-17:00 UTC is 9am-1pm Eastern in EDT, 8am-noon in EST. The endpoint's own +# 9am-Eastern gate is what makes that offset harmless: under EST the 13:00 run +# declines to post and the 14:00 one does it. +# +# Needs the CRON_SECRET repo secret to match the CRON_SECRET env var on Vercel. +# Unlike /api/cron/warm, this endpoint refuses to run at all without it. + +on: + schedule: + - cron: '0 13,14,15,16,17 * * *' + workflow_dispatch: + +jobs: + post: + runs-on: ubuntu-latest + steps: + - name: Post tomorrow's committee meetings + run: | + curl -fsS -m 60 --retry 2 \ + -H "Authorization: Bearer ${{ secrets.CRON_SECRET }}" \ + https://chambers.northeasternsga.com/api/cron/slack-reminders diff --git a/app/(dashboard)/dashboard-shell.tsx b/app/(dashboard)/dashboard-shell.tsx index 19bb9af..75a99f8 100644 --- a/app/(dashboard)/dashboard-shell.tsx +++ b/app/(dashboard)/dashboard-shell.tsx @@ -435,7 +435,7 @@ export default function DashboardShell({ Chambers

NU Student Gov. Association

-

v1.14.0

+

v1.14.1

{userName && (

{getGreeting()},
{userName}

diff --git a/app/(dashboard)/management/bodies-tab.tsx b/app/(dashboard)/management/bodies-tab.tsx index 21620d4..3418c3f 100644 --- a/app/(dashboard)/management/bodies-tab.tsx +++ b/app/(dashboard)/management/bodies-tab.tsx @@ -2,6 +2,7 @@ import { useEffect, useState } from 'react' import { Skeleton } from '@/app/_components/skeleton' +import { BODY_TYPES, bodyTypeGetsSlackReminders, type BodyType } from '@/lib/body-types' function BodiesTabSkeleton() { return ( @@ -43,16 +44,28 @@ interface Body { division: string is_active: boolean body_open: boolean + body_type: BodyType + slack_channel_id: string | null + slack_reminders_enabled: boolean } export default function BodiesTab() { const [bodies, setBodies] = useState([]) const [loading, setLoading] = useState(true) const [showCreateForm, setShowCreateForm] = useState(false) - const [newBody, setNewBody] = useState({ name: '', division: '' }) + const [newBody, setNewBody] = useState<{ name: string; division: string; body_type: BodyType }>( + { name: '', division: '', body_type: 'Other' } + ) const [creating, setCreating] = useState(false) const [editingId, setEditingId] = useState(null) - const [editValues, setEditValues] = useState({ name: '', division: '', is_active: true, body_open: false }) + const [editValues, setEditValues] = useState<{ + name: string; division: string; is_active: boolean; body_open: boolean + body_type: BodyType; slack_channel_id: string; slack_reminders_enabled: boolean + }>({ + name: '', division: '', is_active: true, body_open: false, + body_type: 'Other', slack_channel_id: '', slack_reminders_enabled: true, + }) + const [saveError, setSaveError] = useState(null) const fetchBodies = async () => { const res = await fetch('/api/administrator/bodies') @@ -72,25 +85,42 @@ export default function BodiesTab() { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(newBody), }) - setNewBody({ name: '', division: '' }) + setNewBody({ name: '', division: '', body_type: 'Other' }) setShowCreateForm(false) await fetchBodies() setCreating(false) } const saveEdit = async (id: string) => { - await fetch('/api/administrator/bodies', { + setSaveError(null) + const res = await fetch('/api/administrator/bodies', { method: 'PATCH', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ id, ...editValues }), }) + // The channel ID is the one field here that can be rejected, and a silent + // failure would leave the row looking saved and reminders never arriving. + if (!res.ok) { + const data = await res.json().catch(() => ({})) + setSaveError(data.error ?? 'Could not save that body.') + return + } setEditingId(null) await fetchBodies() } const startEdit = (body: Body) => { setEditingId(body.id) - setEditValues({ name: body.name, division: body.division, is_active: body.is_active, body_open: body.body_open ?? false }) + setSaveError(null) + setEditValues({ + name: body.name, + division: body.division, + is_active: body.is_active, + body_open: body.body_open ?? false, + body_type: body.body_type ?? 'Other', + slack_channel_id: body.slack_channel_id ?? '', + slack_reminders_enabled: body.slack_reminders_enabled ?? true, + }) } if (loading) return @@ -130,6 +160,15 @@ export default function BodiesTab() { ))} +
+
+
+

v1.14.1 — released

+

+ Update emails about a weekly booking now describe the week that actually changed. They had been pointing at whichever week carried the oldest override, which was usually not the week anyone had touched — so the email named a date months off and listed no changes at all. If one save moves several weeks, the email now covers each of them rather than only the first. +

+

+ Senate session types you deselect in Settings now stop the emails and the alerts too, not just the rows in My Rooms. If you follow Full Body but not Office Hours, you will still hear about a change that moved both. +

+

+ An SGA Space booking inside the advance notice window can be edited again. You can shorten it, start it later, rename it or cancel it outright at any point — only adding time to a booking still needs notice, and extending one that ends outside the window is fine. Previously such a booking could not be opened at all. +

+
+
+

v1.14.0 — released

diff --git a/lib/app-zone.ts b/lib/app-zone.ts new file mode 100644 index 0000000..3ffd683 --- /dev/null +++ b/lib/app-zone.ts @@ -0,0 +1,25 @@ +/** + * The timezone every booking date is expressed in. + * + * `booking_date`, `occurrence_date` and `session_date` are all DATE columns -- + * no time, no offset. They mean a calendar day in Boston, because that is where + * the rooms are. "Today" therefore has to mean Boston's today, not the server's + * and not the viewer's: a student on co-op in California at 10pm PT is still + * looking at Northeastern's schedule, and should see the same day their peers on + * campus see. + * + * Pinning it also makes the value reproducible, which is what lets the My Rooms + * page be server-rendered at all -- see todayInAppZone(). + * + * Lives in lib/ rather than beside todayInAppZone() because the Slack reminder + * job needs it too (issue #95), and a route reaching into app/(dashboard)/ for a + * constant is the wrong direction of travel. + * + * Not to be confused with lib/boston-time.ts, which names the same zone for a + * different job. This one is about DATE columns -- which calendar day a booking + * falls on. That one is about SGA Spaces timestamps, which store Boston + * wall-clock digits with a Z on the end and therefore need a "now" in the same + * shape to compare against (issue #87). A booking date has no time of day to get + * wrong; a space booking is nothing but one. + */ +export const APP_TIME_ZONE = 'America/New_York' diff --git a/lib/body-types.ts b/lib/body-types.ts new file mode 100644 index 0000000..6c882dd --- /dev/null +++ b/lib/body-types.ts @@ -0,0 +1,38 @@ +/** + * What kind of body a body is (issue #95). + * + * Committees were distinguished from boards, teams and working groups by their + * name and nothing else, so no query could ask for "the committees" without + * pattern-matching a string. + * + * Mirrors bodies_body_type_check in + * supabase/migrations/20260909000000_body_types_and_slack_reminders.sql. The two + * must agree; if you change one, change the other. + */ +export const BODY_TYPES = [ + 'Committee', + 'Board', + 'Advisory Board', + 'Working Group', + 'Team', + 'Other', +] as const + +export type BodyType = (typeof BODY_TYPES)[number] + +export function isBodyType(v: unknown): v is BodyType { + return typeof v === 'string' && (BODY_TYPES as readonly string[]).includes(v) +} + +/** + * The types the Slack bot posts weekly meeting reminders for. + * + * Only committees today, which is what the issue asks for. Kept as a set rather + * than an equality check so widening it later -- to working groups, say -- is a + * one-line change here and not a hunt through the reminder job. + */ +export const SLACK_REMINDER_BODY_TYPES: readonly BodyType[] = ['Committee'] + +export function bodyTypeGetsSlackReminders(bodyType: string | null | undefined): boolean { + return !!bodyType && (SLACK_REMINDER_BODY_TYPES as readonly string[]).includes(bodyType) +} diff --git a/lib/booking-scope.ts b/lib/booking-scope.ts index c891734..a409f95 100644 --- a/lib/booking-scope.ts +++ b/lib/booking-scope.ts @@ -1,6 +1,7 @@ import type { SupabaseClient } from '@supabase/supabase-js' import { NextResponse } from 'next/server' import { hasLiveAdmin, type AuthedUser } from './auth' +import { wantsAnySenateSession } from './senate-types' /** * Multi-body bookings (issue #19). @@ -339,11 +340,18 @@ export interface Recipient { fullName: string } +interface RecipientUser { + email: string + full_name: string + is_active: boolean + senate_type_preferences: Record | null +} + interface RecipientRow { user_id: string body_id: string role: string - users: { email: string; full_name: string; is_active: boolean } | { email: string; full_name: string; is_active: boolean }[] | null + users: RecipientUser | RecipientUser[] | null } /** @@ -367,11 +375,17 @@ interface RecipientRow { * * `leadershipOnly` narrows to Leadership across the whole audience regardless of scope -- used for * the missed-reservation email, which only ever went to leadership. + * + * `senateTypes` are the session types this notification is about. Pass them and members of the + * Senate who have deselected every one of those types in Settings drop out of the audience -- + * which is what that preference was always supposed to mean, rather than only hiding the rows on + * the My Rooms page (issues #92, #93). Omit it for a notification that is not about particular + * sessions; nobody is filtered then. */ export async function resolveBookingRecipients( adminSupabase: SupabaseClient, row: ScopedRow, - opts: { leadershipOnly?: boolean } = {} + opts: { leadershipOnly?: boolean; senateTypes?: (string | null | undefined)[] } = {} ): Promise { const bodyIds = await resolveBookingBodyIds(adminSupabase, row) if (bodyIds.length === 0) return [] @@ -379,11 +393,16 @@ export async function resolveBookingRecipients( const [{ data }, { data: bookingRow }] = await Promise.all([ adminSupabase .from('board_memberships') - .select('user_id, body_id, role, users(email, full_name, is_active)') + .select('user_id, body_id, role, users(email, full_name, is_active, senate_type_preferences)') .in('body_id', bodyIds), - adminSupabase.from('bookings').select('hidden').eq('id', row.id).maybeSingle(), + // bodies(name) is read for the Senate session-type filter below, which keys on the owning + // body being the one literally named "Senate" -- the same thing the My Rooms filter keys on. + adminSupabase.from('bookings').select('hidden, bodies(name)').eq('id', row.id).maybeSingle(), ]) + const ownerBody = Array.isArray(bookingRow?.bodies) ? bookingRow?.bodies[0] : bookingRow?.bodies + const ownerBodyName = (ownerBody as { name: string } | null | undefined)?.name ?? null + // A hidden booking notifies only the people who can manage it, which is exactly the set // canManageScoped() admits: Leadership anywhere in the booking's audience. const leadershipOnly = !!opts.leadershipOnly || !!bookingRow?.hidden @@ -397,6 +416,17 @@ export async function resolveBookingRecipients( if (leadershipOnly && m.role !== 'Leadership') continue + // Applies to Leadership too. Someone who has said they do not follow Office Hours does not + // start wanting those emails because they lead the body -- the preference is about what they + // read, not about what they are responsible for, and Leadership can still see every session + // on the booking itself. + if ( + opts.senateTypes && + !wantsAnySenateSession(user.senate_type_preferences, ownerBodyName, opts.senateTypes) + ) { + continue + } + // Divisional: peer bodies contribute only their leadership. if ( !leadershipOnly && diff --git a/lib/emails/booking-updated.ts b/lib/emails/booking-updated.ts index fd269f4..dd48e85 100644 --- a/lib/emails/booking-updated.ts +++ b/lib/emails/booking-updated.ts @@ -2,6 +2,26 @@ import { resend } from '@/lib/resend' import { sanitize, buildEmailHtml } from './utils' import { formatDate, formatTime, renderChanges, type BookingChange } from './changes' +/** + * One session of a repeating booking that an edit actually moved. + * + * The values are the session's own -- effective ones, with any per-week override + * already resolved against the series -- so the email can describe the week that + * changed rather than the series it belongs to. + */ +export interface UpdatedSession { + date: string + startTime: string + endTime: string + roomOrTable: string + status: string + purpose?: string | null + /** Where this session sits in the run, e.g. "week 3 of 12". Omitted if unknown. */ + position?: string | null + /** What moved in this session specifically. */ + changes?: BookingChange[] +} + interface BookingUpdatedEmailParams { bodyName: string roomOrTable: string @@ -11,59 +31,86 @@ interface BookingUpdatedEmailParams { status: string recipients: string[] /** - * What an administrator actually altered. Optional so a caller that cannot - * work it out still sends the email it sent before (issue #79). + * What an administrator actually altered at the series level. Optional so a + * caller that cannot work it out still sends the email it sent before (issue + * #79). */ changes?: BookingChange[] /** - * Set when the edit was to one session of a repeating booking rather than to - * the series. The email then describes that session -- its date, its time, its - * room -- instead of restating the series' start date, which is what it used - * to do no matter which week had been touched. + * The sessions of a repeating booking this edit moved, when the series itself + * did not move. The email then describes those sessions -- their dates, times + * and rooms -- instead of restating the series' start date. + * + * This is a list rather than a single session because the weekly editor saves + * every week at once, so one save can move several of them. Naming only one + * meant the other weeks changed silently (issue #91). */ - occurrence?: { - /** Where this session sits in the run, e.g. "week 3 of 12". Omitted if unknown. */ - position?: string | null - } | null + sessions?: UpdatedSession[] | null /** Shown above the details when set, e.g. the booking's purpose. */ purpose?: string | null } +/** The "Body / Room / Date / Time / Status" block, in both bodies of the email. */ +function renderDetails( + fields: { label: string; value: string }[] +): { text: string; html: string } { + return { + text: fields.map(f => `${f.label}: ${f.value}`).join('\n'), + html: fields.map(f => `${f.label}: ${f.value}
`).join(''), + } +} + +/** One session's heading, change rows and current details, for the multi-session email. */ +function renderSession(session: UpdatedSession): { text: string; html: string } { + const heading = session.position + ? `${formatDate(session.date)} (${sanitize(session.position)})` + : formatDate(session.date) + + // The session's date is already the heading here, so the change rows go in bare. + const rendered = renderChanges(session.changes ?? [], { heading: false }) + + const details = renderDetails([ + ...(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: 'Status', value: sanitize(session.status) }, + ]) + + return { + // Indented under the date, and held apart from the change rows above them: + // run together, "- Purpose: Weekly Meeting -> Exec Sync" followed + // immediately by "Purpose: Exec Sync" reads as a contradiction. + text: `${heading} +${rendered ? `${rendered.text}\n` : ''} +${details.text.split('\n').map(l => ` ${l}`).join('\n')}`, + html: ` +
+

${heading}

+ ${rendered ? rendered.html : ''} +

${details.html}

+
`, + } +} + export async function sendBookingUpdatedEmail(params: BookingUpdatedEmailParams) { const { bodyName, roomOrTable, date, startTime, endTime, status, recipients, - changes = [], occurrence = null, purpose = null, + changes = [], sessions = null, purpose = null, } = params if (!recipients.length) return const sBodyName = sanitize(bodyName) - const sRoomOrTable = sanitize(roomOrTable) - const sStatus = sanitize(status) const sPurpose = purpose ? sanitize(purpose) : null - const isOccurrence = !!occurrence - const rendered = renderChanges(changes) - - // A recipient scanning a phone should be able to tell from the subject whether - // this is their whole booking or one week of it. - const subject = isOccurrence - ? `Chambers — A Session of Your ${sBodyName} Booking Has Changed` - : 'Chambers — Your Booking Has Been Updated' - - const lead = isOccurrence - ? `One session of your ${sBodyName} weekly booking has been updated by a Chambers administrator. The rest of the series is unchanged.` - : `Your ${sBodyName} booking has been updated by a Chambers administrator.` + const moved = sessions ?? [] - const dateLabel = isOccurrence ? 'Session date' : 'Date' - const positionText = occurrence?.position ? `\nSession: ${sanitize(occurrence.position)}` : '' - const positionHtml = occurrence?.position - ? `Session: ${sanitize(occurrence.position)}
` - : '' - - const purposeText = sPurpose ? `Purpose: ${sPurpose}\n` : '' - const purposeHtml = sPurpose ? `Purpose: ${sPurpose}
` : '' - - const changesText = rendered ? `\nWhat changed:\n${rendered.text}\n` : '' + // Three shapes, one email: the whole series moved, one week of it moved, or + // several weeks of it moved. + const { subject, lead, text, html } = moved.length > 1 + ? buildMultiSession(sBodyName, moved) + : moved.length === 1 + ? buildSingleSession(sBodyName, moved[0]) + : buildSeries(sBodyName, sPurpose, { roomOrTable, date, startTime, endTime, status }, changes) await resend.emails.send({ from: process.env.RESEND_FROM_EMAIL!, @@ -71,28 +118,84 @@ export async function sendBookingUpdatedEmail(params: BookingUpdatedEmailParams) bcc: recipients, subject, text: `${lead} -${changesText} -${isOccurrence ? 'This session now reads:' : 'The booking now reads:'} -${purposeText}Body: ${sBodyName} -Room/Table: ${sRoomOrTable} -${dateLabel}: ${formatDate(date)} -Time: ${formatTime(startTime)} to ${formatTime(endTime)} -Status: ${sStatus}${positionText} +${text} If you have questions, please reach out to sgaOperations@northeastern.edu.`, html: buildEmailHtml(`

${lead}

- ${rendered ? rendered.html : ''} -

${isOccurrence ? 'This session now reads' : 'The booking now reads'}

-

- ${purposeHtml}Body: ${sBodyName}
- Room/Table: ${sRoomOrTable}
- ${dateLabel}: ${formatDate(date)}
- Time: ${formatTime(startTime)} to ${formatTime(endTime)}
- Status: ${sStatus}
- ${positionHtml} -

+ ${html} `), }) } + +function buildSeries( + sBodyName: string, + sPurpose: string | null, + now: { roomOrTable: string; date: string; startTime: string; endTime: string; status: string }, + changes: BookingChange[] +) { + const rendered = renderChanges(changes) + const details = renderDetails([ + ...(sPurpose ? [{ label: 'Purpose', value: sPurpose }] : []), + { label: 'Body', value: sBodyName }, + { label: 'Room/Table', value: sanitize(now.roomOrTable) }, + { label: 'Date', value: formatDate(now.date) }, + { label: 'Time', value: `${formatTime(now.startTime)} to ${formatTime(now.endTime)}` }, + { label: 'Status', value: sanitize(now.status) }, + ]) + + return { + subject: 'Chambers — Your Booking Has Been Updated', + lead: `Your ${sBodyName} booking has been updated by a Chambers administrator.`, + text: `${rendered ? `What changed:\n${rendered.text}\n\n` : ''}The booking now reads: + +${details.text}`, + html: ` + ${rendered ? rendered.html : ''} +

The booking now reads

+

${details.html}

`, + } +} + +function buildSingleSession(sBodyName: string, session: UpdatedSession) { + const rendered = renderChanges(session.changes ?? []) + const details = renderDetails([ + ...(session.purpose ? [{ label: 'Purpose', value: sanitize(session.purpose) }] : []), + { label: 'Body', value: sBodyName }, + { 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: 'Status', value: sanitize(session.status) }, + ...(session.position ? [{ label: 'Session', value: sanitize(session.position) }] : []), + ]) + + return { + // A recipient scanning a phone should be able to tell from the subject + // whether this is their whole booking or one week of it. + subject: `Chambers — A Session of Your ${sBodyName} Booking Has Changed`, + lead: `One session of your ${sBodyName} weekly booking has been updated by a Chambers administrator. The rest of the series is unchanged.`, + text: `${rendered ? `What changed:\n${rendered.text}\n\n` : ''}This session now reads: + +${details.text}`, + html: ` + ${rendered ? rendered.html : ''} +

This session now reads

+

${details.html}

`, + } +} + +function buildMultiSession(sBodyName: string, sessions: UpdatedSession[]) { + const blocks = sessions.map(renderSession) + + return { + subject: `Chambers — ${sessions.length} Sessions of Your ${sBodyName} Booking Have Changed`, + lead: `${sessions.length} sessions of your ${sBodyName} weekly booking have been updated by a Chambers administrator. The rest of the series is unchanged.`, + text: `These sessions now read: + +${blocks.map(b => b.text).join('\n\n')}`, + html: ` +

These sessions now read

+ ${blocks.map(b => b.html).join('')}`, + } +} diff --git a/lib/emails/changes.ts b/lib/emails/changes.ts index 3ff6c1c..23972c8 100644 --- a/lib/emails/changes.ts +++ b/lib/emails/changes.ts @@ -76,8 +76,15 @@ export function collectChanges(...items: (BookingChange | null)[]): BookingChang * is filed under), and inventing a "What changed" section that lists nothing * would be worse than leaving it out. The email then reads as it did before, as * a statement of where the booking now stands. + * + * `heading: false` drops the "What changed" label, for a caller that already + * has a heading of its own above the rows -- the per-session blocks of a + * multi-session update email, where repeating it under every date would shout. */ -export function renderChanges(changes: BookingChange[]): { text: string; html: string } | null { +export function renderChanges( + changes: BookingChange[], + { heading = true }: { heading?: boolean } = {} +): { text: string; html: string } | null { if (!changes.length) return null const text = changes @@ -85,7 +92,7 @@ export function renderChanges(changes: BookingChange[]): { text: string; html: s .join('\n') const html = ` -

What changed

+ ${heading ? '

What changed

' : ''} ${changes.map(c => ` diff --git a/lib/emails/missed-reservation.ts b/lib/emails/missed-reservation.ts index e545eb0..995f061 100644 --- a/lib/emails/missed-reservation.ts +++ b/lib/emails/missed-reservation.ts @@ -1,38 +1,55 @@ import { resend } from '@/lib/resend' import { sanitize, buildEmailHtml } from './utils' +import { formatDate, formatTime } from './changes' +/** + * The alert Operational Affairs gets when a reservation is marked Missed. + * + * This template did not move when the booking emails were rewritten in #79, and + * drifted (issue #99). It rendered its times straight out of the payload, so an + * alert read "Time of Reservation: 18:30:00 to 20:00:00" where every other + * Chambers email said "6:30 PM to 8:00 PM"; it carried a second copy of the date + * formatter under a different name; and it took its date already formatted, + * which is why the raw times were easy to miss -- the one value the caller + * prepared looked right, and the two it passed through did not. + * + * It now takes raw values and formats them here, like every other template. + */ interface MissedReservationEmailParams { bodyName: string + /** 'YYYY-MM-DD'. Formatted here, not by the caller. */ date: string + /** 'HH:MM' or 'HH:MM:SS' -- both are formatted the same way. */ startTime: string endTime: string contacts: string[] -} - -export function formatDateLong(dateStr: string): string { - return new Date(dateStr + 'T00:00:00').toLocaleDateString('en-US', { - weekday: 'long', - month: 'long', - day: 'numeric', - year: 'numeric', - }) + /** The room or table that went unused. Omitted when the caller has none. */ + roomOrTable?: string | null } export async function sendMissedReservationEmail(params: MissedReservationEmailParams) { - const { bodyName, date, startTime, endTime, contacts } = params + const { bodyName, date, startTime, endTime, contacts, roomOrTable = null } = params const sBodyName = sanitize(bodyName) - const sContacts = contacts.map(sanitize).join(', ') + // An em dash, so a list of names cannot read as one hyphenated name, and a + // placeholder rather than an empty line when a body has no leadership on file. + const sContacts = contacts.length ? contacts.map(sanitize).join(', ') : 'None on file' + const sRoomOrTable = roomOrTable ? sanitize(roomOrTable) : null + + const roomText = sRoomOrTable ? `Room/Table: ${sRoomOrTable}\n` : '' + const roomHtml = sRoomOrTable ? `Room/Table: ${sRoomOrTable}
` : '' await resend.emails.send({ from: process.env.RESEND_FROM_EMAIL!, + // Straight to Operational Affairs rather than bcc'd to a membership: this is + // an internal alert about a body, not a notification to it. to: process.env.OPS_EMAIL!, - subject: 'Chambers Alert - Reservation Missed', + subject: 'Chambers — Reservation Missed', text: `This is an automatic alert that a SGA reservation was marked as missed by a Chambers administrator. Responsible Body: ${sBodyName} -Date of Reservation: ${date} -Time of Reservation: ${startTime} to ${endTime} +${roomText}Date of Reservation: ${formatDate(date)} +Time of Reservation: ${formatTime(startTime)} to ${formatTime(endTime)} Contacts: ${sContacts} @@ -41,8 +58,8 @@ For further information, please reach out to the Comptroller.`,

This is an automatic alert that a SGA reservation was marked as missed by a Chambers administrator.

Responsible Body: ${sBodyName}
- Date of Reservation: ${date}
- Time of Reservation: ${startTime} to ${endTime}
+ ${roomHtml}Date of Reservation: ${formatDate(date)}
+ Time of Reservation: ${formatTime(startTime)} to ${formatTime(endTime)}
Contacts: ${sContacts}

`, 'For further information, please reach out to the Comptroller.'), diff --git a/lib/meeting-reminders.ts b/lib/meeting-reminders.ts new file mode 100644 index 0000000..1a58c8e --- /dev/null +++ b/lib/meeting-reminders.ts @@ -0,0 +1,174 @@ +import { APP_TIME_ZONE } from '@/lib/app-zone' + +/** + * Working out which committee meetings the Slack bot should remind a channel + * about, and what to say (issue #95). + * + * Kept apart from the route so the selection rules -- which weeks count, which + * are suppressed, how an override resolves against its series -- can be read and + * exercised without a database or a Slack workspace. + */ + +/** The local hour, in APP_TIME_ZONE, at or after which the day-before reminder posts. */ +export const REMINDER_HOUR = 9 + +/** + * Statuses that mean the meeting is not happening, so a reminder would be wrong. + * + * Deliberately short. 'Pending Cancellation' is *not* here: that week may still + * go ahead, and the people in the channel are exactly the ones who need to know + * it is in doubt -- so it is reported, with its status shown. + */ +const NOT_HAPPENING = new Set(['Cancelled', 'Repurposed', 'Missed']) + +/** Statuses ordinary enough that naming them in the reminder would be noise. */ +const UNREMARKABLE = new Set(['Reserved', 'Confirmed']) + +/** 'YYYY-MM-DD' and the hour, in APP_TIME_ZONE, for an instant. */ +export function appZoneParts(now: Date = new Date()): { date: string; hour: number } { + const parts = new Intl.DateTimeFormat('en-CA', { + timeZone: APP_TIME_ZONE, + year: 'numeric', + month: '2-digit', + day: '2-digit', + hour: '2-digit', + hour12: false, + }).formatToParts(now) + + const get = (type: string) => parts.find(p => p.type === type)?.value ?? '' + // Intl renders midnight as '24' in some ICU versions under hour12: false. + const hour = Number(get('hour')) % 24 + + return { date: `${get('year')}-${get('month')}-${get('day')}`, hour } +} + +/** The day after `date` ('YYYY-MM-DD'), computed on the calendar rather than by adding hours. */ +export function nextDay(date: string): string { + const [y, m, d] = date.split('-').map(Number) + // UTC noon, so a DST boundary cannot push the arithmetic onto the wrong day. + const at = new Date(Date.UTC(y, m - 1, d, 12)) + at.setUTCDate(at.getUTCDate() + 1) + return at.toISOString().slice(0, 10) +} + +/** + * One occurrence as the reminder query returns it, with its series and body + * attached. Only the fields the rules below touch are declared. + */ +export interface ReminderCandidate { + occurrence_date: string + room_name: string | null + start_time: string | null + end_time: string | null + status: string | null + purpose: string | null + hidden: boolean | null + weekly_booking_id: string + series: { + room_name: string | null + start_time: string | null + end_time: string | null + status: string | null + } + booking: { + purpose: string | null + hidden: boolean | null + } + body: { + name: string + slack_channel_id: string | null + } +} + +/** A candidate resolved to the values that actually apply to that week. */ +export interface ResolvedMeeting { + weeklyBookingId: string + date: string + channelId: string + bodyName: string + roomName: string | null + startTime: string | null + endTime: string | null + status: string | null + purpose: string | null +} + +/** + * Resolves a candidate against its series, or returns null when no reminder + * should be posted for it. + * + * An occurrence field that is null inherits -- from the series for room, times + * and status, and from the booking above it for purpose and visibility. That + * precedence is the same one My Rooms and the update emails apply. + */ +export function resolveMeeting(c: ReminderCandidate): ResolvedMeeting | null { + if (!c.body.slack_channel_id) return null + + // A hidden booking is visible only to the people who can manage it, so + // announcing it to a channel would disclose it to everyone in that channel. + // `?? booking.hidden` is the inheritance: a visible series can hide one week. + if (c.hidden ?? c.booking.hidden) return null + + const status = c.status ?? c.series.status + if (status && NOT_HAPPENING.has(status)) return null + + 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, + endTime: c.end_time ?? c.series.end_time, + status, + purpose: c.purpose ?? c.booking.purpose, + } +} + +function formatTime(time: string | null): string | null { + if (!time) return null + const [h, m] = time.split(':').map(Number) + if (Number.isNaN(h) || Number.isNaN(m)) return null + const ampm = h >= 12 ? 'PM' : 'AM' + return `${h % 12 || 12}:${String(m).padStart(2, '0')} ${ampm}` +} + +function formatDate(date: string): string { + const [y, m, d] = date.split('-').map(Number) + return new Date(Date.UTC(y, m - 1, d, 12)).toLocaleDateString('en-US', { + weekday: 'long', + month: 'long', + day: 'numeric', + timeZone: 'UTC', + }) +} + +/** Slack mrkdwn escaping: only these three characters carry meaning in message text. */ +function esc(s: string): string { + return s.replace(/&/g, '&').replace(//g, '>') +} + +/** + * The reminder text. + * + * States what is known and stays quiet about what is not: a week with no room + * secured says so rather than printing a dash, and an ordinary status is left + * off entirely so that a status line always means something is unusual. + */ +export function formatReminder(m: ResolvedMeeting): string { + const start = formatTime(m.startTime) + const end = formatTime(m.endTime) + const when = start && end ? `${start}–${end}` : start ?? 'time to be confirmed' + + const lines = [ + `:calendar: *${esc(m.bodyName)}* meets tomorrow — ${formatDate(m.date)}, ${when}`, + ] + + lines.push(m.roomName ? `*Room:* ${esc(m.roomName)}` : '*Room:* not yet confirmed') + + if (m.purpose?.trim()) lines.push(`*Purpose:* ${esc(m.purpose.trim())}`) + + if (m.status && !UNREMARKABLE.has(m.status)) lines.push(`*Status:* ${esc(m.status)}`) + + return lines.join('\n') +} diff --git a/lib/pending-cancellations.ts b/lib/pending-cancellations.ts index 3dfd252..dd6c974 100644 --- a/lib/pending-cancellations.ts +++ b/lib/pending-cancellations.ts @@ -95,11 +95,19 @@ export function hasUsableCode(code: string | null | undefined): boolean { return typeof code === 'string' && code.trim().length > 0 } -export interface SkippedReservation { - date: string - roomOrTable: string - bodyName: string - bookingType: 'One-Time Room' | 'Weekly Room' | 'Tabling' +/** + * Everything a line carries except a code CSC could act on. + * + * It used to carry only the four display fields, because the only thing anyone + * did with a skipped row was print it in the Auto-Cancel preview. Marking a + * cancellation request Done has to *act* on these rows (issue #96): the admin + * saying they have handled it is not conditional on CSC having had a code to + * work from, so the row still needs its status applied and therefore still needs + * its id, its table and its outcome. + */ +export interface SkippedReservation extends Omit { + /** Always null. Its absence is what makes the row skipped. */ + reservationCode: null } /** @@ -191,20 +199,31 @@ export async function collectPending(): Promise { // request beats a resolved one, and the most recent beats an older one. const { data: requests } = await adminSupabase .from('cancellation_requests') - .select('id, booking_id, occurrence_id, scope, status, cancellation_type, created_at') + .select('id, booking_id, occurrence_id, occurrence_date, scope, status, cancellation_type, created_at') .order('created_at', { ascending: false }) // The id travels with the type: sending closes the request it acted on, so // knowing *which* row said 'Virtual' matters as much as the value. const byOccurrence = new Map() + // Keyed on (booking, date), which is what survives an edit. occurrence_id does + // not: the weekly PATCH handler regenerates every occurrence row on each save, + // so a request made before an edit points at nothing afterwards -- and every + // request in production was in exactly that state (issue #96). Falling through + // to bySeriesBooking would have been wrong, and falling through to nothing lost + // the cancellation_type, so a request that asked to go Virtual came out + // Cancelled. + const byBookingDate = new Map() const bySeriesBooking = new Map() const byBooking = new Map() + const dateKey = (bookingId: string, date: string) => `${bookingId}|${date}` + for (const pass of ['Pending', 'other'] as const) { for (const r of (requests ?? []) as { id: string booking_id: string | null occurrence_id: string | null + occurrence_date: string | null scope: string status: string | null cancellation_type: string @@ -217,6 +236,10 @@ export async function collectPending(): Promise { if (r.occurrence_id && !byOccurrence.has(r.occurrence_id)) { byOccurrence.set(r.occurrence_id, ref) } + if (r.booking_id && r.occurrence_date) { + const k = dateKey(r.booking_id, r.occurrence_date) + if (!byBookingDate.has(k)) byBookingDate.set(k, ref) + } if (r.booking_id) { if (r.scope === 'series' && !bySeriesBooking.has(r.booking_id)) { bySeriesBooking.set(r.booking_id, ref) @@ -233,19 +256,17 @@ export async function collectPending(): Promise { request: RequestRef | undefined, ) => { const outcome = outcomeOf(request?.type) - if (hasUsableCode(code)) lines.push({ + const resolved = { ...line, - reservationCode: code!.trim(), resultingStatus: outcome.status, outcomeFromRequest: outcome.fromRequest, cancellationRequestId: request?.id ?? null, - }) - else skipped.push({ - date: line.date, - roomOrTable: line.roomOrTable, - bodyName: line.bodyName, - bookingType: line.bookingType, - }) + } + // Built once and routed, rather than assembled differently on each branch. + // The two used to diverge, and a skipped row lost the id and table that + // marking a request Done now needs (issue #96). + if (hasUsableCode(code)) lines.push({ ...resolved, reservationCode: code!.trim() }) + else skipped.push({ ...resolved, reservationCode: null }) } { @@ -265,7 +286,7 @@ export async function collectPending(): Promise { roomOrTable: r.room_name || 'Not recorded', bodyName: bodyNameOf(r.bookings), bookingType: 'One-Time Room', - }, byBooking.get(r.booking_id)) + }, byBookingDate.get(dateKey(r.booking_id, r.booking_date)) ?? byBooking.get(r.booking_id)) } } @@ -291,7 +312,9 @@ export async function collectPending(): Promise { roomOrTable: r.location || 'Not recorded', bodyName: bodyNameOf(parent?.bookings ?? null), bookingType: 'Tabling', - }, parent?.booking_id ? byBooking.get(parent.booking_id) : undefined) + }, parent?.booking_id + ? byBookingDate.get(dateKey(parent.booking_id, r.session_date)) ?? byBooking.get(parent.booking_id) + : undefined) } } @@ -339,8 +362,14 @@ export async function collectPending(): Promise { bookingType: 'Weekly Room', }, // A request naming this exact week wins over one covering the series. + // The id is tried first because it is exact when it resolves; the + // (booking, date) key is what still works once the row has been + // regenerated, which is the usual case rather than the exception. byOccurrence.get(r.id) - ?? (series?.booking_id ? bySeriesBooking.get(series.booking_id) : undefined) + ?? (series?.booking_id + ? byBookingDate.get(dateKey(series.booking_id, r.occurrence_date)) + ?? bySeriesBooking.get(series.booking_id) + : undefined) ) } } @@ -353,3 +382,68 @@ export async function collectPending(): Promise { } } + +/** Which table each source's `id` belongs to. */ +const TABLE_OF: Record = { + one_time: 'one_time_room_bookings', + occurrence: 'weekly_room_occurrences', + tabling_session: 'tabling_sessions', +} + +/** The rows a cancellation outcome can be written to. */ +export type OutcomeTarget = Pick + +/** + * Writes each reservation the status it is due, and returns the batches that + * failed. + * + * Grouped by table *and* by status, so a batch containing both kinds writes each + * its own value. Marking everything 'Cancelled' would be wrong for a booking + * whose request said it was going virtual: that meeting still happens, it just + * does not need the room. + * + * An occurrence whose status was inherited gets its value written onto the + * occurrence itself, which is correct -- only the dates actually acted on stop + * being pending, and the rest of the series is untouched. + * + * Shared by Auto-Cancel and by marking a request Done by hand, which have to + * agree about this: the same request resolved either way should leave the + * database in the same state. + */ +export async function applyCancellationOutcomes(rows: OutcomeTarget[]): Promise { + const batches = new Map() + for (const l of rows) { + const table = TABLE_OF[l.source] + const bucket = `${table}:${l.resultingStatus}` + if (!batches.has(bucket)) batches.set(bucket, { table, status: l.resultingStatus, ids: [] }) + batches.get(bucket)!.ids.push(l.id) + } + + const failures: string[] = [] + for (const { table, status, ids } of batches.values()) { + const { error } = await adminSupabase.from(table).update({ status }).in('id', ids) + if (error) { + console.error(`Could not mark ${table} as ${status}:`, error) + failures.push(`${table} (${status})`) + } + } + return failures +} + +/** + * Audit rows for a set of reservations, one per booking and status. + * + * Keyed on booking *and* status: one booking can contribute both a cancelled + * week and a virtual one in the same action, and a single row saying 'Cancelled' + * would misreport the other. + */ +export function cancellationAuditRows(rows: OutcomeTarget[], adminId: string) { + return [...new Map( + rows + .filter(l => l.bookingId) + .map(l => [ + `${l.bookingId}:${l.resultingStatus}`, + { booking_id: l.bookingId, admin_id: adminId, new_status: l.resultingStatus }, + ]) + ).values()] +} diff --git a/lib/senate-types.ts b/lib/senate-types.ts new file mode 100644 index 0000000..20af897 --- /dev/null +++ b/lib/senate-types.ts @@ -0,0 +1,64 @@ +/** + * Senate session types, and the one rule that decides whether a member wants to + * hear about a session of a given type. + * + * The list was declared twice -- once in the Settings modal that writes the + * preference, once in the My Rooms code that read it -- and the rule itself + * existed only on the client, inside the My Rooms list filter. So deselecting + * Office Hours hid those sessions from the page and did nothing else: update + * emails and dashboard alerts still went to everyone on the Senate, including + * the people who had said they did not want them (issues #92, #93). + * + * Both now live here, so the page, the emails and the alerts cannot drift apart. + */ + +/** + * Session types only exist on bookings owned by the body literally named + * "Senate" -- that is what gates the Session Type field in the weekly editor, + * and what the My Rooms filter has always keyed on. + */ +export const SENATE_BODY_NAME = 'Senate' + +export const SENATE_TYPES = ['Full Body', 'Weekly', 'Office Hours'] as const + +export type SenateType = (typeof SENATE_TYPES)[number] + +/** + * Whether this member wants to hear about one session. + * + * Defaults to yes at every step: a non-Senate booking, a Senate session with no + * type set, or a type the member has never expressed an opinion about. A + * preference has to be deselected explicitly to suppress anything, so a member + * who has never opened Settings sees and hears about everything. + */ +export function wantsSenateSession( + prefs: Record | null | undefined, + bodyName: string | null | undefined, + senateType: string | null | undefined +): boolean { + if (bodyName !== SENATE_BODY_NAME) return true + if (!senateType) return true + return prefs?.[senateType] ?? true +} + +/** + * Whether this member wants to hear about an edit that touched several sessions + * at once. + * + * One wanted session is enough. Someone who reads Full Body but not Office Hours + * still needs the email about a save that moved both, because the Full Body week + * moved -- suppressing it would lose a session they asked to know about, which is + * a worse failure than one line of Office Hours detail they did not. + * + * An empty list means the edit was not session-specific (the series moved), so + * there is nothing to filter on and everyone hears about it. + */ +export function wantsAnySenateSession( + prefs: Record | null | undefined, + bodyName: string | null | undefined, + senateTypes: (string | null | undefined)[] +): boolean { + if (bodyName !== SENATE_BODY_NAME) return true + if (!senateTypes.length) return true + return senateTypes.some(t => wantsSenateSession(prefs, bodyName, t)) +} diff --git a/lib/slack.ts b/lib/slack.ts new file mode 100644 index 0000000..33b77c4 --- /dev/null +++ b/lib/slack.ts @@ -0,0 +1,62 @@ +/** + * Posting to Slack. + * + * chat.postMessage was called inline in the slash-command route with its own + * fetch and its own header block. Now that the reminder job posts too (issue + * #95), the call lives here -- mainly so the failure path does, because Slack + * answers a refused post with HTTP 200 and `ok: false`, and a bare fetch reads + * that as success. A reminder that silently never arrived is the failure this + * feature is most likely to have, so it has to be visible in the logs. + */ + +const SLACK_POST_MESSAGE_URL = 'https://slack.com/api/chat.postMessage' + +export interface SlackPostResult { + ok: boolean + /** Slack's machine-readable reason, e.g. 'not_in_channel', 'channel_not_found'. */ + error?: string +} + +/** + * Sends one message. `channel` is a channel id for a channel post, or a user id + * for a DM. + * + * `text` is used as the notification fallback and as the body; pass `blocks` as + * well for a richer layout, and Slack renders those instead while still using + * `text` for the push notification and the accessibility label. + */ +export async function postSlackMessage( + channel: string, + text: string, + blocks?: unknown[] +): Promise { + try { + const res = await fetch(SLACK_POST_MESSAGE_URL, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${process.env.SLACK_BOT_TOKEN}`, + }, + body: JSON.stringify(blocks ? { channel, text, blocks } : { channel, text }), + }) + + // A 200 from Slack is not a delivered message. The common refusals here are + // not_in_channel (the bot was never invited) and channel_not_found (the id + // is wrong, or the channel is private and the bot cannot see it) -- both of + // which look like a working integration that just never posts. + const data = (await res.json()) as { ok?: boolean; error?: string } + if (!data.ok) { + console.error(`Slack chat.postMessage refused for ${channel}:`, data.error) + return { ok: false, error: data.error ?? 'unknown_error' } + } + return { ok: true } + } catch (e) { + console.error(`Slack chat.postMessage failed for ${channel}:`, e) + return { ok: false, error: 'request_failed' } + } +} + +/** An ephemeral slash-command reply: visible only to whoever ran the command. */ +export function ephemeral(text: string): Response { + return Response.json({ response_type: 'ephemeral', text }) +} diff --git a/lib/spaces-advance-notice.ts b/lib/spaces-advance-notice.ts new file mode 100644 index 0000000..89171c9 --- /dev/null +++ b/lib/spaces-advance-notice.ts @@ -0,0 +1,89 @@ +import { bostonWallClockNow } from './boston-time' + +/** + * The advance-notice rule for SGA Spaces, in one place (issue #94). + * + * The rule exists so that time is *claimed* a set number of hours before it is + * used. It follows that the thing to check is the time an edit newly claims, not + * whether the start moved: shortening a booking, pushing its start later, + * renaming it or cancelling it outright all release time or leave it alone, and + * none of them needs notice. + * + * The PATCH route used to reject any change to start_time that landed inside the + * window, which meant a booking that had entered the window could not be + * shortened, moved later, or given a different name -- and the calendar would + * not even open it, so in practice it could not be touched at all. + */ + +export interface BookingInterval { + /** ISO instant. */ + start: string + /** ISO instant. */ + end: string +} + +/** + * The earliest instant `next` claims that `prev` did not, or null when it claims + * nothing new. + * + * `prev` is null for a booking being created, where everything is new. + * + * The three shapes that claim time: + * + * start moved earlier the new block runs from the new start + * end moved later the new block runs from the *old* end -- which is + * why extending a booking whose end is still outside + * the window is fine + * moved clear of the old the whole interval is new, so it runs from the new + * interval start; relocating to next week is not an extension + */ +export function earliestNewlyClaimed( + next: BookingInterval, + prev: BookingInterval | null +): string | null { + if (!prev) return next.start + + const nextStart = Date.parse(next.start) + const nextEnd = Date.parse(next.end) + const prevStart = Date.parse(prev.start) + const prevEnd = Date.parse(prev.end) + + if (nextStart < prevStart) return next.start + if (nextEnd > prevEnd) return nextStart > prevEnd ? next.start : prev.end + return null +} + +/** + * The error to reject this booking with, or null when it is allowed. + * + * Returning the message rather than a boolean keeps the wording next to the rule + * it explains -- an edit and a creation fail for the same reason but need to be + * told different things about what to do next. + * + * `now` defaults to Boston wall-clock now and NOT to Date.now(), which would be + * wrong here in a way that is easy to miss: a space booking's start_time holds + * Boston wall-clock digits with a Z on the end, so measuring it against a real + * instant makes every booking look an offset earlier than it is. That is issue + * #87, fixed in 1e3d894, and putting the default here rather than at each call + * site is what keeps it fixed -- the browser is a caller too, and its clock is + * in whatever zone the viewer is sitting in. + */ +export function advanceNoticeError( + next: BookingInterval, + prev: BookingInterval | null, + minHours: number, + now: number = bostonWallClockNow().getTime() +): string | null { + if (minHours <= 0) return null + + const earliest = earliestNewlyClaimed(next, prev) + if (earliest === null) return null + + if (Date.parse(earliest) >= now + minHours * 60 * 60 * 1000) return null + + const hours = `${minHours} hour${minHours === 1 ? '' : 's'}` + + return prev + ? `Adding time to a booking needs at least ${hours} of notice. You can still shorten this booking, start it later, rename it, or cancel it.` + : `Bookings must be made at least ${hours} in advance.` +} diff --git a/lib/weekly-occurrences.ts b/lib/weekly-occurrences.ts new file mode 100644 index 0000000..0b64a64 --- /dev/null +++ b/lib/weekly-occurrences.ts @@ -0,0 +1,68 @@ +/** + * Telling which weeks of a repeating booking an edit actually moved. + * + * The weekly PATCH deletes and reinserts every occurrence on every save, so + * "what changed" cannot be read off the write -- it has to be a comparison + * against the rows that were there before. + * + * This used to be approximated as "the first week carrying any override", which + * is a different question and usually a different week: an override set on week + * 1 months ago is still an override today, so every later edit to the series was + * reported against week 1, with an empty change list, because week 1 had not in + * fact moved (issue #91). + */ + +/** + * The occurrence columns a PATCH can move. Everything but is_event is an + * override, so null is a real value here and means "inherit". + * + * 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. + */ +export const OCCURRENCE_FIELDS = [ + 'room_name', 'start_time', 'end_time', 'status', + 'reservation_code', 'purpose', 'senate_type', 'hidden', 'is_event', +] as const + +export type OccurrenceField = (typeof OCCURRENCE_FIELDS)[number] + +/** Any row carrying the override columns -- a stored one or a freshly built one. */ +export type OccurrenceRow = Partial> + +/** + * Normalises one override for comparison, so a value that only changed shape is + * not read as a change: Postgres returns a time as '18:30:00' where the editor + * submits '18:30', and a cleared text field arrives as '' where the stored row + * holds null. + */ +export function normalizeOccurrenceValue(field: OccurrenceField, value: unknown): string | null { + // is_event is the one column here that is not an override: an absent value + // means "not an event" rather than "inherit", so null and false are the same + // state and must not compare as a change. Without this, a week the series had + // not previously covered looked edited the moment it was generated. + if (field === 'is_event') return value === true ? 'true' : 'false' + + if (value === null || value === undefined) return null + 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 +} + +/** + * Whether this week's overrides differ from what was stored for it. + * + * `prev` is undefined for a date the series did not previously cover, which + * compares as all-null -- so extending a series does not, by itself, mark the + * new weeks as edited. That edit shows up as a change to the series' end date + * instead, which is where a recipient would look for it. + */ +export function occurrenceMoved( + prev: OccurrenceRow | undefined, + next: OccurrenceRow +): boolean { + return OCCURRENCE_FIELDS.some( + f => normalizeOccurrenceValue(f, prev?.[f]) !== normalizeOccurrenceValue(f, next[f]) + ) +} diff --git a/package-lock.json b/package-lock.json index 7cc8c12..36b0822 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "chambers", - "version": "1.14.0", + "version": "1.14.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "chambers", - "version": "1.14.0", + "version": "1.14.1", "dependencies": { "@supabase/ssr": "^0.9.0", "@supabase/supabase-js": "^2.99.1", diff --git a/package.json b/package.json index 0defb8c..54158d9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "chambers", - "version": "1.14.0", + "version": "1.14.1", "private": true, "scripts": { "dev": "next dev", diff --git a/supabase/migrations/20260909000000_body_types_and_slack_reminders.sql b/supabase/migrations/20260909000000_body_types_and_slack_reminders.sql new file mode 100644 index 0000000..6824be1 --- /dev/null +++ b/supabase/migrations/20260909000000_body_types_and_slack_reminders.sql @@ -0,0 +1,109 @@ +-- Body types, and the Chambers Slack bot's weekly meeting reminders (issue #95). +-- +-- Committees were distinguished from boards, teams and working groups by their +-- name and nothing else, so no query could ask for "the committees" without +-- pattern-matching a string. This gives bodies a type, and gives the one type +-- that needs it -- Committee -- a Slack channel to post meeting reminders to. + +-- --------------------------------------------------------------------------- +-- bodies.body_type +-- --------------------------------------------------------------------------- + +alter table public.bodies + add column if not exists body_type text not null default 'Other'; + +alter table public.bodies + drop constraint if exists bodies_body_type_check; + +alter table public.bodies + add constraint bodies_body_type_check + check (body_type in ('Committee', 'Board', 'Advisory Board', 'Working Group', 'Team', 'Other')); + +comment on column public.bodies.body_type is + 'What kind of body this is. Committee is the only type the Slack bot posts meeting reminders for.'; + +-- Backfill from the name, which is what has been carrying this distinction all +-- along. Order matters: 'Advisory Board' has to be tested before 'Board', or the +-- three advisory boards would come out as plain boards. +-- +-- Checked against the 31 bodies live at the time of writing: 9 Committee, +-- 9 Board, 3 Advisory Board, 5 Team, 3 Working Group, and 2 that are genuinely +-- none of these -- Senate and SGA General -- which land on Other correctly +-- rather than as a failure to match. Management can change any of them. +update public.bodies set body_type = case + when name ilike '%advisory board%' then 'Advisory Board' + when name ilike '%committee%' then 'Committee' + when name ilike '%board%' then 'Board' + when name ilike '%working group%' then 'Working Group' + when name ilike '%team%' then 'Team' + else 'Other' +end; + +-- --------------------------------------------------------------------------- +-- bodies: where the bot posts, and whether it may +-- --------------------------------------------------------------------------- + +alter table public.bodies + add column if not exists slack_channel_id text; + +-- A channel id (C0123ABCDEF), not a name. Channels get renamed; ids do not, and +-- a reminder that silently stops posting because someone tidied up a channel +-- name is worse than no reminder at all. +comment on column public.bodies.slack_channel_id is + 'Slack channel id the bot posts this committee''s meeting reminders to. NULL means no channel is linked, and nothing is posted.'; + +alter table public.bodies + add column if not exists slack_reminders_enabled boolean not null default true; + +-- Leadership of the committee turns this off from inside its own channel, with +-- /chambers-reminders off. Defaults to true so that linking a channel is the +-- only step needed to start; there is nothing to post to until then anyway. +comment on column public.bodies.slack_reminders_enabled is + 'Whether the bot may post meeting reminders for this body. Toggled by Leadership of the body via /chambers-reminders in the linked channel, or by Management.'; + +-- --------------------------------------------------------------------------- +-- slack_meeting_reminders: what has already been posted +-- --------------------------------------------------------------------------- +-- The reminder job is scheduled from a GitHub Action, which may run several +-- times in the posting window (schedules there lag and are occasionally skipped, +-- so retries are the point). This table is what makes a repeat run a no-op +-- rather than a second ping. +-- +-- Keyed on (weekly_booking_id, occurrence_date) rather than on an occurrence id. +-- The weekly PATCH handler regenerates its occurrences on every save -- it +-- deletes them all and reinserts, so their ids change -- and a foreign key to +-- weekly_room_occurrences(id) would forget every reminder the next time anyone +-- edited the booking, re-posting the lot. weekly_booking_id survives that +-- regeneration, and the date is the stable identifier within a series. This is +-- the same reasoning event_tracking already follows in +-- 20260829001000_occurrence_events.sql. +-- +-- The pair, rather than the date alone, so a body running two weekly series that +-- both meet on a Tuesday gets a reminder for each. + +create table if not exists public.slack_meeting_reminders ( + id uuid primary key default gen_random_uuid(), + weekly_booking_id uuid not null references public.weekly_room_bookings(id) on delete cascade, + occurrence_date date not null, + channel_id text not null, + posted_at timestamptz not null default now(), + unique (weekly_booking_id, occurrence_date) +); + +comment on table public.slack_meeting_reminders is + 'One row per meeting reminder the Slack bot has posted. Makes a repeated run of the reminder job a no-op.'; + +create index if not exists slack_meeting_reminders_date_idx + on public.slack_meeting_reminders (occurrence_date); + +alter table public.slack_meeting_reminders enable row level security; + +-- Written only by the reminder job, which uses the service-role client and +-- bypasses RLS. Admins can read it to see what the bot has done; nobody else has +-- any reason to, and there is deliberately no INSERT or UPDATE policy. +drop policy if exists "slack_meeting_reminders_select_admin" on public.slack_meeting_reminders; +create policy "slack_meeting_reminders_select_admin" + on public.slack_meeting_reminders + for select + to authenticated + using (is_admin()); diff --git a/supabase/migrations/20260909010000_cancellation_requests_occurrence_date.sql b/supabase/migrations/20260909010000_cancellation_requests_occurrence_date.sql new file mode 100644 index 0000000..b4baa7a --- /dev/null +++ b/supabase/migrations/20260909010000_cancellation_requests_occurrence_date.sql @@ -0,0 +1,60 @@ +-- Cancellation requests remember the date, not just the row id (issue #96). +-- +-- cancellation_requests.occurrence_id names the dated row a request is about. +-- For a weekly booking that row does not survive an edit: the weekly PATCH +-- handler deletes every occurrence and reinserts it on each save, so the ids +-- change while the values are carried across on the date. The request is left +-- pointing at nothing. +-- +-- Every cancellation_requests row in production is currently in that state -- +-- all four of them, including the one still Pending. The consequences are worse +-- than they look: +-- +-- * Marking a request Done could not find the reservation it was about, so it +-- closed the request and changed no booking status. That is the reported bug. +-- * Auto-Cancel finds those reservations anyway, because it scans by status +-- rather than by request -- but it cannot attribute them back to a request, +-- so it loses the cancellation_type and falls back to 'Cancelled'. A request +-- that asked to go Virtual would have cancelled the meeting outright. +-- +-- The date is the stable identifier the write model actually preserves, which is +-- the same conclusion 20260829001000_occurrence_events.sql reached for +-- event_tracking, and the same one issue #69 reached for calendar UIDs. This +-- follows it: occurrence_id stays for the rows that still resolve, and +-- occurrence_date becomes the key that survives a regeneration. + +alter table public.cancellation_requests + add column if not exists occurrence_date date; + +comment on column public.cancellation_requests.occurrence_date is + 'The date of the reservation this request is about, for occurrence-scoped requests. The stable key: occurrence_id does not survive an edit to a weekly booking, because the PATCH handler regenerates every occurrence row. NULL for series-scoped requests, and for occurrence-scoped ones created before this column existed whose row had already been regenerated.'; + +-- Backfill from occurrence_id wherever it still resolves. This sets nothing in +-- production, where every row has already been orphaned -- there is no record +-- anywhere of which date those requests named, so they cannot be recovered and +-- are deliberately left NULL rather than guessed at. It is written for the +-- environments where the rows are still intact, and so that applying this +-- migration to a restored backup does the right thing. + +update public.cancellation_requests r +set occurrence_date = o.occurrence_date +from public.weekly_room_occurrences o +where r.occurrence_id = o.id + and r.occurrence_date is null; + +update public.cancellation_requests r +set occurrence_date = s.booking_date +from public.one_time_room_bookings s +where r.occurrence_id = s.id + and r.occurrence_date is null; + +update public.cancellation_requests r +set occurrence_date = t.session_date +from public.tabling_sessions t +where r.occurrence_id = t.id + and r.occurrence_date is null; + +-- Pending requests are looked up by (booking_id, occurrence_date) on every +-- Auto-Cancel preview and every Cancellations tab load. +create index if not exists cancellation_requests_booking_date_idx + on public.cancellation_requests (booking_id, occurrence_date); diff --git a/supabase/migrations/rollback/20260909_body_types_and_slack_reminders_rollback.sql b/supabase/migrations/rollback/20260909_body_types_and_slack_reminders_rollback.sql new file mode 100644 index 0000000..61f3160 --- /dev/null +++ b/supabase/migrations/rollback/20260909_body_types_and_slack_reminders_rollback.sql @@ -0,0 +1,21 @@ +-- Rollback for 20260909000000_body_types_and_slack_reminders.sql. +-- +-- Drops the reminder log and the three columns on bodies. Any types Management +-- had corrected by hand after the backfill are lost, as is every channel link +-- and every "Leadership turned this off" decision -- so re-applying the +-- migration will re-infer types from names and re-enable reminders for every +-- committee whose channel is linked again. + +drop table if exists public.slack_meeting_reminders; + +alter table public.bodies + drop column if exists slack_reminders_enabled; + +alter table public.bodies + drop column if exists slack_channel_id; + +alter table public.bodies + drop constraint if exists bodies_body_type_check; + +alter table public.bodies + drop column if exists body_type; diff --git a/supabase/migrations/rollback/20260909_cancellation_requests_occurrence_date_rollback.sql b/supabase/migrations/rollback/20260909_cancellation_requests_occurrence_date_rollback.sql new file mode 100644 index 0000000..db1c7e1 --- /dev/null +++ b/supabase/migrations/rollback/20260909_cancellation_requests_occurrence_date_rollback.sql @@ -0,0 +1,11 @@ +-- Rollback for 20260909010000_cancellation_requests_occurrence_date.sql. +-- +-- Drops the column, which returns cancellation requests to being identified only +-- by an occurrence_id that does not survive an edit to a weekly booking. Any +-- dates recorded since the migration are lost, so requests created in between +-- become orphaned in exactly the way the migration was written to stop. + +drop index if exists public.cancellation_requests_booking_date_idx; + +alter table public.cancellation_requests + drop column if exists occurrence_date;