diff --git a/app/(dashboard)/administrator/archive-tab.tsx b/app/(dashboard)/administrator/archive-tab.tsx index 3d397c6..aeebe28 100644 --- a/app/(dashboard)/administrator/archive-tab.tsx +++ b/app/(dashboard)/administrator/archive-tab.tsx @@ -1,6 +1,7 @@ 'use client' import { useEffect, useState } from 'react' +import { getJson } from '@/lib/fetch-json' import { Skeleton } from '@/app/_components/skeleton' function ArchiveTabSkeleton() { @@ -266,8 +267,9 @@ export default function ArchiveTab() { const [loading, setLoading] = useState(true) useEffect(() => { - fetch('/api/administrator/archive') - .then(r => r.json()) + // Also fixes a hang: the old chain had no .catch, so a rejected fetch left + // setLoading(false) unreached and the tab stuck on its skeleton forever. + getJson<{ groups?: SemesterGroup[] }>('/api/administrator/archive', {}) .then(data => { setGroups(data.groups || []) setLoading(false) diff --git a/app/(dashboard)/administrator/booking-settings-tab.tsx b/app/(dashboard)/administrator/booking-settings-tab.tsx index 23c75e9..0660242 100644 --- a/app/(dashboard)/administrator/booking-settings-tab.tsx +++ b/app/(dashboard)/administrator/booking-settings-tab.tsx @@ -1,6 +1,7 @@ 'use client' import { useEffect, useState } from 'react' +import { getJson } from '@/lib/fetch-json' import { createBrowserClient } from '@supabase/ssr' import { getAuthedUser } from '@/lib/auth' import { Skeleton } from '@/app/_components/skeleton' @@ -107,9 +108,14 @@ export default function BookingSettingsTab() { const [deletePermissionChecked, setDeletePermissionChecked] = useState(false) useEffect(() => { - fetch('/api/administrator/settings') - .then(r => r.json()) + // The stakes here are higher than a blank screen. Every field below falls + // back to a hardcoded default when the value is missing, so an error body + // would silently populate the form with 0 / 0 / 24 -- and an admin who then + // pressed Save would write those over the real booking limits. Failing the + // load outright keeps the tab on its skeleton instead. + getJson | null>('/api/administrator/settings', null) .then(data => { + if (!data) return setMinDaysRoom(data.min_days_advance_room ?? 0) setMinDaysTabling(data.min_days_advance_tabling ?? 0) setMinHoursSpaces(data.min_hours_advance_spaces ?? 24) diff --git a/app/(dashboard)/administrator/one-time-form.tsx b/app/(dashboard)/administrator/one-time-form.tsx index ba93878..416bc64 100644 --- a/app/(dashboard)/administrator/one-time-form.tsx +++ b/app/(dashboard)/administrator/one-time-form.tsx @@ -1,6 +1,7 @@ 'use client' import { useState, useEffect } from 'react' +import { getJson } from '@/lib/fetch-json' import TimePicker from './time-picker' import DateField from '@/app/_components/date-field' import BookingScopeSelector, { type BookingScopeValue } from '@/app/_components/booking-scope-selector' @@ -100,14 +101,12 @@ export default function OneTimeForm({ bodies, semesters, onClose, onSuccess }: O const [requestTypeFilter, setRequestTypeFilter] = useState<'all' | BookingScope>('all') useEffect(() => { - fetch('/api/administrator/requests') - .then(r => r.json()) + getJson<{ requests?: PendingRequest[] }>('/api/administrator/requests', {}) .then(({ requests }) => { setPendingRequests( (requests ?? []).filter((r: PendingRequest) => r.status === 'Pending' && r.type === 'One-Time Room') ) }) - .catch(() => {}) }, []) const visibleRequests = pendingRequests diff --git a/app/(dashboard)/administrator/sga-spaces-tab.tsx b/app/(dashboard)/administrator/sga-spaces-tab.tsx index 69e17bf..ae9ac9c 100644 --- a/app/(dashboard)/administrator/sga-spaces-tab.tsx +++ b/app/(dashboard)/administrator/sga-spaces-tab.tsx @@ -4,6 +4,7 @@ import { useState, useEffect, useCallback } from 'react' import TimePicker from './time-picker' import { Skeleton } from '@/app/_components/skeleton' import DateField from '@/app/_components/date-field' +import { getJsonArray } from '@/lib/fetch-json' function TableSkeleton({ cols }: { cols: number }) { return ( @@ -155,7 +156,7 @@ export default function SGASpacesTab() { const [spaces, setSpaces] = useState([]) useEffect(() => { - fetch('/api/spaces').then(r => r.json()).then(setSpaces) + getJsonArray('/api/spaces').then(setSpaces) }, []) return ( diff --git a/app/(dashboard)/administrator/tabling-form.tsx b/app/(dashboard)/administrator/tabling-form.tsx index ce97e92..9745489 100644 --- a/app/(dashboard)/administrator/tabling-form.tsx +++ b/app/(dashboard)/administrator/tabling-form.tsx @@ -1,6 +1,7 @@ 'use client' import { useState, useEffect } from 'react' +import { getJson } from '@/lib/fetch-json' import TimePicker from './time-picker' import DateField from '@/app/_components/date-field' import BookingScopeSelector, { type BookingScopeValue } from '@/app/_components/booking-scope-selector' @@ -104,14 +105,12 @@ export default function TablingForm({ bodies, semesters, onClose, onSuccess }: T const [requestTypeFilter, setRequestTypeFilter] = useState<'all' | BookingScope>('all') useEffect(() => { - fetch('/api/administrator/requests') - .then(r => r.json()) + getJson<{ requests?: PendingRequest[] }>('/api/administrator/requests', {}) .then(({ requests }) => { setPendingRequests( (requests ?? []).filter((r: PendingRequest) => r.status === 'Pending' && r.type === 'Tabling') ) }) - .catch(() => {}) }, []) const visibleRequests = pendingRequests diff --git a/app/(dashboard)/administrator/weekly-form.tsx b/app/(dashboard)/administrator/weekly-form.tsx index ba61929..ae8b27b 100644 --- a/app/(dashboard)/administrator/weekly-form.tsx +++ b/app/(dashboard)/administrator/weekly-form.tsx @@ -1,6 +1,7 @@ 'use client' import { useState, useEffect } from 'react' +import { getJson } from '@/lib/fetch-json' import TimePicker from './time-picker' import DateField from '@/app/_components/date-field' import BookingScopeSelector, { type BookingScopeValue } from '@/app/_components/booking-scope-selector' @@ -93,14 +94,12 @@ export default function WeeklyForm({ bodies, semesters, onClose, onSuccess }: We const [requestTypeFilter, setRequestTypeFilter] = useState<'all' | BookingScope>('all') useEffect(() => { - fetch('/api/administrator/requests') - .then(r => r.json()) + getJson<{ requests?: PendingRequest[] }>('/api/administrator/requests', {}) .then(({ requests }) => { setPendingRequests( (requests ?? []).filter((r: PendingRequest) => r.status === 'Pending' && r.type === 'Weekly Room') ) }) - .catch(() => {}) }, []) const visibleRequests = pendingRequests diff --git a/app/(dashboard)/dashboard-shell.tsx b/app/(dashboard)/dashboard-shell.tsx index 661de3f..55c9442 100644 --- a/app/(dashboard)/dashboard-shell.tsx +++ b/app/(dashboard)/dashboard-shell.tsx @@ -210,9 +210,26 @@ export default function DashboardShell({ load() }, [fetchDashboard]) + /** + * Ends the session on this device only. + * + * signOut() defaults to scope 'global', which revokes every refresh token the + * user holds anywhere. Signing out on a laptop therefore also killed the + * session on their phone, in their other tab, and the copy the Edge middleware + * refreshes -- and each of those then failed its next refresh with a 400 and + * started serving 401s. Over 24h, 9 of 28 refresh attempts were failing that + * way, split across the browser and the middleware. + * + * 'local' is what "Sign Out" means on a shared dashboard: this browser, not + * every device I own. The paths that mean "this account may not be used" -- + * deactivation in force-sign-out.tsx and LoginCard, an expired invite -- + * deliberately keep the global scope. + */ + const signOutThisDevice = () => supabase.auth.signOut({ scope: 'local' }) + const handleLogout = async () => { localStorage.removeItem('chambers_last_active') - await supabase.auth.signOut() + await signOutThisDevice() router.push('/') } @@ -227,7 +244,7 @@ export default function DashboardShell({ clearInterval(countdownIntervalRef.current!) countdownIntervalRef.current = null localStorage.removeItem('chambers_last_active') - supabase.auth.signOut().then(() => router.push('/')) + signOutThisDevice().then(() => router.push('/')) } }, 1000) } @@ -251,7 +268,7 @@ export default function DashboardShell({ if (storedLastActive) { const elapsed = Date.now() - parseInt(storedLastActive, 10) if (elapsed >= IDLE_MS) { - supabase.auth.signOut().then(() => router.push('/')) + signOutThisDevice().then(() => router.push('/')) return } } @@ -276,7 +293,7 @@ export default function DashboardShell({ if (elapsed >= IDLE_MS) { if (idleTimerRef.current) clearTimeout(idleTimerRef.current) if (countdownIntervalRef.current) clearInterval(countdownIntervalRef.current) - supabase.auth.signOut().then(() => router.push('/')) + signOutThisDevice().then(() => router.push('/')) } } } diff --git a/app/(dashboard)/force-sign-out.tsx b/app/(dashboard)/force-sign-out.tsx index bd2d6c1..c133bab 100644 --- a/app/(dashboard)/force-sign-out.tsx +++ b/app/(dashboard)/force-sign-out.tsx @@ -19,6 +19,9 @@ export default function ForceSignOut() { useEffect(() => { const signOut = async () => { localStorage.removeItem('chambers_last_active') + // Global scope on purpose, unlike the sign-outs in dashboard-shell. This + // one runs because the account was deactivated, so every session it holds + // anywhere should end -- not just the one in this browser. await createClient().auth.signOut() router.replace('/') } diff --git a/app/(dashboard)/settings-modal.tsx b/app/(dashboard)/settings-modal.tsx index 2e35684..c3d53b5 100644 --- a/app/(dashboard)/settings-modal.tsx +++ b/app/(dashboard)/settings-modal.tsx @@ -1,6 +1,7 @@ 'use client' import { useEffect, useState } from 'react' +import { getJson } from '@/lib/fetch-json' import { Skeleton } from '@/app/_components/skeleton' import { getPrefsForRole, EMAIL_PREF_LABELS, EmailPrefKey } from '@/lib/email-preferences' @@ -57,16 +58,19 @@ export default function SettingsModal({ onClose, cachedSettings, onSettingsLoade const loadSettings = () => { setLoading(true) - fetch('/api/me/settings') - .then(r => r.json()) - .then((data: Settings) => { + // null rather than a blank Settings object on failure. Every consumer below + // is already guarded on `settings &&`, so null renders as empty sections, + // whereas an error body cast to Settings would put undefined into the name + // field and the preference toggles and invite the user to save it back. + getJson('/api/me/settings', null) + .then(data => { + setLoading(false) + if (!data) return setSettings(data) setNameValue(data.full_name ?? '') setSelectedBodyId('') - setLoading(false) onSettingsLoaded?.(data) }) - .catch(() => setLoading(false)) } useEffect(() => { if (cachedSettings == null) loadSettings() }, []) diff --git a/app/(dashboard)/sga-spaces/page.tsx b/app/(dashboard)/sga-spaces/page.tsx index 89fe030..2cfa294 100644 --- a/app/(dashboard)/sga-spaces/page.tsx +++ b/app/(dashboard)/sga-spaces/page.tsx @@ -139,6 +139,7 @@ function CalendarSkeleton() { export default function SGASpacesPage() { const [spaces, setSpaces] = useState([]) const [spacesLoading, setSpacesLoading] = useState(true) + const [spacesError, setSpacesError] = useState(false) const [selectedSpaceId, setSelectedSpaceId] = useState(null) const [bookings, setBookings] = useState([]) const [blackouts, setBlackouts] = useState([]) @@ -184,16 +185,40 @@ export default function SGASpacesPage() { }) }, []) - useEffect(() => { - fetch('/api/spaces') - .then(r => r.json()) - .then((data: Space[]) => { - setSpaces(data) - if (data.length > 0) setSelectedSpaceId(data[0].id) - }) - .finally(() => setSpacesLoading(false)) + // Handled explicitly rather than via getJson: on this page an empty list is a + // meaningful answer ("no rooms configured"), so quietly substituting one for a + // failed request would tell the user something untrue. + // + // Issue #53: /api/spaces returned a transient 401, r.json() resolved happily + // with { error: 'Unauthorized' }, that object went into `spaces`, and the page + // died on the next spaces.find(). The response shape is now checked before it + // is trusted, so a bad response can only ever produce an error state. + const loadSpaces = useCallback(async () => { + setSpacesLoading(true) + setSpacesError(false) + try { + const res = await fetch('/api/spaces') + if (!res.ok) throw new Error(`/api/spaces responded ${res.status}`) + + const data: unknown = await res.json() + if (!Array.isArray(data)) throw new Error('/api/spaces did not return a list') + + const list = data as Space[] + setSpaces(list) + if (list.length > 0) setSelectedSpaceId(prev => prev || list[0].id) + } catch (err) { + console.error('Failed to load spaces:', err) + setSpaces([]) + setSpacesError(true) + } finally { + setSpacesLoading(false) + } }, []) + useEffect(() => { + loadSpaces() + }, [loadSpaces]) + // Remaining hours only change when the user books or cancels -- not when the // calendar week changes. Keying this off `bookings` refetched it on every week // navigation and fired it twice on mount (once with the initial empty array). @@ -279,6 +304,29 @@ export default function SGASpacesPage() { return } + // Distinct from "no rooms configured", which renders the normal empty calendar + // below. The failure that prompted this was transient and a reload cleared it, + // so the useful thing to offer is another attempt. + if (spacesError) { + return ( +
+

SGA Spaces

+
+

Couldn't load the spaces

+

+ Something went wrong reaching the server. This is usually temporary. +

+ +
+
+ ) + } + return ( <>
diff --git a/app/_components/LoginCard.tsx b/app/_components/LoginCard.tsx index 9acbdac..3b030b6 100644 --- a/app/_components/LoginCard.tsx +++ b/app/_components/LoginCard.tsx @@ -53,6 +53,9 @@ export default function LoginCard() { .single() if (!profile?.is_active) { + // Global scope kept deliberately here and below: these mean the account + // may not be used at all, so every session it holds should end. An + // ordinary sign-out (dashboard-shell) is scoped 'local' instead. await supabase.auth.signOut() setError('Your account has been deactivated. Please contact an administrator.') setLoading(false) diff --git a/app/onboarding/page.tsx b/app/onboarding/page.tsx index 0cd5bf7..db7fc0b 100644 --- a/app/onboarding/page.tsx +++ b/app/onboarding/page.tsx @@ -45,6 +45,9 @@ export default function OnboardingPage() { .single() if (!profile?.is_active) { + // Global scope deliberately: the account is deactivated, so every + // session it holds should end, not only this browser's. See + // dashboard-shell for the ordinary 'local' sign-out. await supabase.auth.signOut() router.push('/') return diff --git a/lib/fetch-json.ts b/lib/fetch-json.ts new file mode 100644 index 0000000..50078e4 --- /dev/null +++ b/lib/fetch-json.ts @@ -0,0 +1,48 @@ +/** + * GET a JSON endpoint, falling back to a known-good value when the request does + * not succeed. + * + * Written for a bug that took down the SGA Spaces page (issue #53). The call + * sites all looked like this: + * + * fetch('/api/spaces') + * .then(r => r.json()) + * .then((data: Space[]) => setSpaces(data)) + * + * The annotation is a promise the code cannot keep. An error response is still + * valid JSON, so `r.json()` resolves happily with `{ error: 'Unauthorized' }`, + * that object lands in state typed as an array, and the page dies on the next + * `spaces.find(...)`. TypeScript cannot catch it: the cast is asserted at a + * boundary where the real shape is only known at runtime. + * + * Passing the fallback in makes the failure case impossible to leave out, and + * keeps the type honest -- the return is whatever the caller was already + * prepared to render. + * + * Silent on failure by design: these are background reads whose call sites have + * no error UI. Where "the request failed" is different from "there is nothing + * here" -- as it is on the Spaces page, where an empty list reads as "no rooms + * exist" -- handle the response explicitly instead of reaching for this. + */ +export async function getJson(url: string, fallback: T): Promise { + try { + const res = await fetch(url) + if (!res.ok) return fallback + const data = await res.json() + return (data ?? fallback) as T + } catch { + // Network failure, or a body that is not JSON at all (an HTML error page + // from a proxy, say). Both are "no data", same as a non-2xx. + return fallback + } +} + +/** + * As getJson, but guarantees an array. Use for list endpoints whose result is + * indexed, mapped or searched -- an object arriving where an array is expected + * is precisely what crashed issue #53. + */ +export async function getJsonArray(url: string): Promise { + const data = await getJson(url, []) + return Array.isArray(data) ? (data as T[]) : [] +}