Skip to content

fix: marking a cancellation Done applies the outcome it asked for (#96) #101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pataniaeli merged 1 commit into from
Sep 10, 2026
Merged

fix: marking a cancellation Done applies the outcome it asked for (#96) #101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 13 additions & 40 deletions app/api/administrator/cancellations/auto-cancel/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@ import { createClient as createAdminClient } from '@supabase/supabase-js'
import { NextResponse } from 'next/server'
import { checkRateLimit } from '@/lib/check-rate-limit'
import { getAuthedUserWithLiveRoles } from '@/lib/authorization'
import { sendCscCancellationRequest, type CancellationLine } from '@/lib/emails/csc-cancellation-request'
import { collectPending, lineKey, requestsFullyCovered } from '@/lib/pending-cancellations'
import { sendCscCancellationRequest } from '@/lib/emails/csc-cancellation-request'
import {
applyCancellationOutcomes,
cancellationAuditRows,
collectPending,
lineKey,
requestsFullyCovered,
} from '@/lib/pending-cancellations'

const adminSupabase = createAdminClient(
process.env.NEXT_PUBLIC_SUPABASE_URL!,
Expand Down Expand Up @@ -182,36 +188,10 @@ export async function POST(request: Request) {
return NextResponse.json({ error: 'The email could not be sent. No bookings were changed.' }, { status: 502 })
}

// Grouped by table *and* by the status each row is due, 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 sent stop
// being pending, and the rest of the series is untouched.
const TABLE_OF: Record<CancellationLine['source'], string> = {
one_time: 'one_time_room_bookings',
occurrence: 'weekly_room_occurrences',
tabling_session: 'tabling_sessions',
}

const batches = new Map<string, { table: string; status: string; ids: string[] }>()
for (const l of selected) {
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(`Auto-Cancel could not mark ${table} as ${status}:`, error)
failures.push(`${table} (${status})`)
}
}
// Shared with marking a request Done by hand, which has to reach the same
// result: the same request resolved either way should leave the database in
// the same state.
const failures = await applyCancellationOutcomes(selected)

// Close the cancellation requests this send acted on, so nobody has to go and
// press "Mark as Done" for work Auto-Cancel already did.
Expand Down Expand Up @@ -241,14 +221,7 @@ export async function POST(request: Request) {
// beside every other status change rather than appearing to have happened by
// itself. Best effort: the email is out and the statuses are moved, and
// failing the request over a missing log would invite a resend.
// Keyed on booking *and* status: one booking can contribute both a cancelled
// week and a virtual one in the same batch, and a single row saying 'Cancelled'
// would misreport the other.
const auditRows = [...new Map(
selected
.filter(l => l.bookingId)
.map(l => [`${l.bookingId}:${l.resultingStatus}`, { booking_id: l.bookingId, admin_id: user.id, new_status: l.resultingStatus }])
).values()]
const auditRows = cancellationAuditRows(selected, user.id)
if (auditRows.length) {
const { error } = await adminSupabase.from('audit_logs').insert(auditRows)
if (error) console.error('Auto-Cancel audit log failed:', error)
Expand Down
72 changes: 64 additions & 8 deletions app/api/administrator/cancellations/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ import { createClient as createAdminClient } from '@supabase/supabase-js'
import { NextResponse } from 'next/server'
import { checkRateLimit } from '@/lib/check-rate-limit'
import { getAuthedUserWithLiveRoles } from '@/lib/authorization'
import {
applyCancellationOutcomes,
cancellationAuditRows,
collectPending,
} from '@/lib/pending-cancellations'

const adminSupabase = createAdminClient(
process.env.NEXT_PUBLIC_SUPABASE_URL!,
Expand All @@ -23,7 +28,7 @@ export async function GET() {
const { data: cancellations } = await supabase
.from('cancellation_requests')
.select(`
id, scope, status, created_at, cancellation_type, occurrence_id, booking_id,
id, scope, status, created_at, cancellation_type, occurrence_id, occurrence_date, booking_id,
bookings(id, type, purpose, bodies(name)),
users(full_name)
`)
Expand All @@ -39,12 +44,20 @@ export async function GET() {

if (c.scope === 'occurrence' && c.occurrence_id) {
if (bookingType === 'Weekly Room') {
const { data: occ } = await adminSupabase
// Matched on the stored date first, and only then on the id. The id
// does not survive an edit to the booking -- the PATCH handler
// regenerates every occurrence -- which is why these rows showed no
// date at all (issue #96).
const q = adminSupabase
.from('weekly_room_occurrences')
.select('occurrence_date, reservation_code')
.eq('id', c.occurrence_id)
.single()
occurrence_date = occ?.occurrence_date ?? null
.select('occurrence_date, reservation_code, weekly_room_bookings!inner(booking_id)')
const { data: occ } = c.occurrence_date
? await q
.eq('occurrence_date', c.occurrence_date)
.eq('weekly_room_bookings.booking_id', c.booking_id)
.maybeSingle()
: await q.eq('id', c.occurrence_id).maybeSingle()
occurrence_date = occ?.occurrence_date ?? c.occurrence_date ?? null
reservation_code = occ?.reservation_code ?? null
} else if (bookingType === 'One-Time Room') {
const { data: session } = await adminSupabase
Expand Down Expand Up @@ -88,7 +101,10 @@ export async function GET() {
}
}

return { ...c, occurrence_date, reservation_code }
// The stored date is the fallback for every type: even where the code
// lookup fails because the row was regenerated, the request still knows
// which date it was about.
return { ...c, occurrence_date: occurrence_date ?? c.occurrence_date ?? null, reservation_code }
})
)

Expand All @@ -107,6 +123,38 @@ export async function PATCH(request: Request) {
if (rateLimitRes) return rateLimitRes

const { id } = await request.json()
if (!id) return NextResponse.json({ error: 'A cancellation request id is required.' }, { status: 400 })

const { data: req } = await adminSupabase
.from('cancellation_requests')
.select('id')
.eq('id', id)
.maybeSingle()

if (!req) return NextResponse.json({ error: 'Cancellation request not found.' }, { status: 404 })

// Which dated reservations this request covers, and what each is due. Read
// from the same collector Auto-Cancel uses, so the two agree about whose row a
// request owns -- including the inheritance that makes a series-level
// cancellation produce pending occurrences that do not say so themselves.
//
// `skipped` is included here where Auto-Cancel excludes it. A missing
// reservation code means CSC cannot be asked, which is why Auto-Cancel will
// not touch those rows; it says nothing about whether an administrator has
// dealt with it. Marking Done by hand is that administrator saying they have.
const { lines, skipped } = await collectPending()
const covered = [...lines, ...skipped].filter(l => l.cancellationRequestId === id)

const failures = await applyCancellationOutcomes(covered)

// One entry per booking touched, so the change shows up in the Audit tab
// beside every other status change rather than appearing to have happened by
// itself. Best effort, as in Auto-Cancel.
const auditRows = cancellationAuditRows(covered, user.id)
if (auditRows.length) {
const { error: auditError } = await adminSupabase.from('audit_logs').insert(auditRows)
if (auditError) console.error('Cancellation done audit log failed:', auditError)
}

const { error } = await adminSupabase
.from('cancellation_requests')
Expand All @@ -115,5 +163,13 @@ export async function PATCH(request: Request) {

if (error) return NextResponse.json({ error: error.message }, { status: 500 })

return NextResponse.json({ success: true })
return NextResponse.json({
success: true,
applied: covered.length,
cancelled: covered.filter(l => l.resultingStatus === 'Cancelled').length,
virtual: covered.filter(l => l.resultingStatus === 'Virtual').length,
// The request is closed either way -- the admin has said it is handled -- but
// they need to know if a status did not move with it.
...(failures.length ? { statusUpdateFailed: failures } : {}),
})
}
27 changes: 27 additions & 0 deletions app/api/cancellation-requests/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,39 @@ export async function POST(request: Request) {

const bookingType = guard.row.type

// The date this request is about, resolved now while occurrence_id still
// names a live row (issue #96).
//
// It will not always. The weekly PATCH handler deletes and reinserts every
// occurrence on each save, so an edit to the booking leaves occurrence_id
// pointing at nothing while the values live on under new ids, carried across
// on the date. Storing the date is what lets the request still be matched to
// its reservation afterwards -- without it, marking the request Done finds
// nothing to do, and Auto-Cancel cannot tell which request asked for what.
let occurrenceDate: string | null = null
if (scope === 'occurrence' && occurrence_id) {
if (bookingType === 'One-Time Room') {
const { data } = await adminSupabase
.from('one_time_room_bookings').select('booking_date').eq('id', occurrence_id).maybeSingle()
occurrenceDate = data?.booking_date ?? null
} else if (bookingType === 'Tabling') {
const { data } = await adminSupabase
.from('tabling_sessions').select('session_date').eq('id', occurrence_id).maybeSingle()
occurrenceDate = data?.session_date ?? null
} else {
const { data } = await adminSupabase
.from('weekly_room_occurrences').select('occurrence_date').eq('id', occurrence_id).maybeSingle()
occurrenceDate = data?.occurrence_date ?? null
}
}

// Create cancellation request
const { error: requestError } = await adminSupabase
.from('cancellation_requests')
.insert({
booking_id,
occurrence_id: occurrence_id || null,
occurrence_date: occurrenceDate,
requested_by: user.id,
scope,
status: 'Pending',
Expand Down
Loading
Loading