Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 26 additions & 0 deletions src/components/ui/ProgressBar.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { render, screen } from '@testing-library/react'
import { ProgressBar } from './ProgressBar'

describe('ProgressBar', () => {
it('exposes aria value attributes and fills to value/max when determinate', () => {
render(<ProgressBar value={3} max={8} label="Import progress" />)
const bar = screen.getByRole('progressbar', { name: 'Import progress' })
expect(bar.getAttribute('aria-valuenow')).toBe('3')
expect(bar.getAttribute('aria-valuemax')).toBe('8')
expect(bar.getAttribute('aria-valuemin')).toBe('0')
expect((bar.firstElementChild as HTMLElement).style.width).toBe('37.5%')
})

it('clamps an over-max value to 100%', () => {
render(<ProgressBar value={20} max={8} label="p" />)
const fill = screen.getByRole('progressbar', { name: 'p' }).firstElementChild as HTMLElement
expect(fill.style.width).toBe('100%')
})

it('omits aria value attributes when indeterminate', () => {
render(<ProgressBar label="Working" />)
const bar = screen.getByRole('progressbar', { name: 'Working' })
expect(bar.getAttribute('aria-valuenow')).toBeNull()
expect(bar.getAttribute('aria-valuemax')).toBeNull()
})
})
42 changes: 42 additions & 0 deletions src/components/ui/ProgressBar.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { cn } from '@/lib/cn'

interface ProgressBarProps {
/**
* Completed amount, 0..`max`. Omit for an *indeterminate* bar (a sweeping
* segment) when the total duration is unknown — e.g. a single network fetch.
*/
value?: number
max?: number
label: string
className?: string
}

/**
* A slim monochrome progress track. Determinate when given a `value` (fills to
* `value/max`); indeterminate otherwise (a segment sweeps to signal activity).
* Reduced-motion users get a static bar via the global motion reset.
*/
export function ProgressBar({ value, max = 100, label, className }: ProgressBarProps) {
const indeterminate = value === undefined
const pct = indeterminate ? 0 : Math.max(0, Math.min(100, (value / max) * 100))

return (
<div
role="progressbar"
aria-label={label}
{...(indeterminate
? {}
: { 'aria-valuemin': 0, 'aria-valuemax': max, 'aria-valuenow': value })}
className={cn('relative h-1.5 w-full overflow-hidden rounded-full bg-inset', className)}
>
{indeterminate ? (
<span className="absolute inset-y-0 w-2/5 rounded-full bg-accent [animation:progress-sweep_1.1s_ease-in-out_infinite]" />
) : (
<span
className="block h-full rounded-full bg-accent transition-[width] duration-300 ease-standard"
style={{ width: `${pct}%` }}
/>
)}
</div>
)
}
34 changes: 29 additions & 5 deletions src/features/settings/FetchDataTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ import { useAppState, useSession } from '@/hooks/session'
import { useNow } from '@/hooks/useNow'
import { Button } from '@/components/ui/Button'
import { Field, Input, Select } from '@/components/ui/Field'
import { ProgressBar } from '@/components/ui/ProgressBar'
import { useToast } from '@/components/ui/Toast'
import { reconcileImport } from '@/services/importers/applyImport'
import type { BatchProgress } from '@/services/importers/runImport'
import { SectionTitle } from './SectionTitle'

const BATCH_SEASONS = ['Winter', 'Spring', 'Summer'] as const
Expand All @@ -28,6 +30,7 @@ export function FetchDataTab() {
const [icsUrl, setIcsUrl] = useState('')
const [status, setStatus] = useState<string | null>(null)
const [busy, setBusy] = useState(false)
const [progress, setProgress] = useState<BatchProgress | null>(null)
const [batch, setBatch] = useState(false)
const years = useMemo(() => batchYearOptions(now), [now])
const [startSeason, setStartSeason] = useState<BatchSeason>('Winter')
Expand All @@ -50,6 +53,7 @@ export function FetchDataTab() {
return
}
setBusy(true)
setProgress(null)
setStatus('Fetching semesters and course details…')
const { runBatchIcsImport, BatchIcsError } = await import('@/services/importers/runImport')
const snapshot = session.appStore.getState().data
Expand All @@ -59,7 +63,7 @@ export function FetchDataTab() {
icsUrl.trim(),
{ season: startSeason, year: startYear },
{ season: endSeason, year: endYear },
{ semesterName: '', nowIso: now.toISOString() },
{ semesterName: '', nowIso: now.toISOString(), onProgress: setProgress },
)
applyResult(snapshot, result.data)
if (result.imported.length === 0) {
Expand All @@ -85,6 +89,7 @@ export function FetchDataTab() {
toast.error('Batch import failed')
} finally {
setBusy(false)
setProgress(null)
}
}

Expand Down Expand Up @@ -277,10 +282,29 @@ export function FetchDataTab() {
</div>
) : null}

{status ? (
<p role="status" aria-live="polite" className="text-xs text-ink-muted">
{status}
</p>
{busy || status ? (
<div className="flex flex-col gap-1.5">
{busy ? (
progress ? (
<ProgressBar
value={progress.completed}
max={progress.total}
label="Import progress"
/>
) : (
<ProgressBar label="Working" />
)
) : null}
{busy && progress ? (
<p role="status" aria-live="polite" className="text-xs text-ink-muted">
{progress.current} · {progress.completed}/{progress.total}
</p>
) : status ? (
<p role="status" aria-live="polite" className="text-xs text-ink-muted">
{status}
</p>
) : null}
</div>
) : null}
</div>
)
Expand Down
31 changes: 30 additions & 1 deletion src/services/importers/corsProxy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,35 @@ describe('fetchViaProxies', () => {
expect(delays).toEqual([])
})

it('short-circuits on a 404 (not found) without trying the other proxies', async () => {
const { impl, calls } = makeFetchStub([new Response('nope', { status: 404 })])
const { delayFn } = makeDelaySpy()

const error = await expectProxyFetchError(
fetchViaProxies(TARGET, { fetchImpl: impl, delayFn, proxies: TEST_PROXIES }),
)

expect(error.notFound).toBe(true)
expect(error.attempts).toBe(1)
// A 404 is authoritative — it must not fall through to proxies 2 and 3.
expect(calls).toHaveLength(1)
expect(calls[0]?.url).toBe(TEST_PROXIES[0]?.(TARGET))
})

it('treats a 200 carrying X-Proxy-Status:404 as not found (the worker signal)', async () => {
const { impl, calls } = makeFetchStub([
new Response('', { status: 200, headers: { 'X-Proxy-Status': '404' } }),
])
const { delayFn } = makeDelaySpy()

const error = await expectProxyFetchError(
fetchViaProxies(TARGET, { fetchImpl: impl, delayFn, proxies: TEST_PROXIES }),
)

expect(error.notFound).toBe(true)
expect(calls).toHaveLength(1)
})

it('retries the same proxy after a 429 with a rate-limit delay, then succeeds', async () => {
const { impl, calls } = makeFetchStub([
new Response('slow down', { status: 429 }),
Expand Down Expand Up @@ -291,7 +320,7 @@ describe('fetchViaProxies', () => {

it('counts attempts correctly across mixed failure modes', async () => {
const { impl } = makeFetchStub([
new Response('', { status: 404 }), // proxy 1: skip after one attempt
new Response('', { status: 403 }), // proxy 1: skip after one attempt
new TypeError('fetch failed'), // proxy 2, attempt 1
new TypeError('fetch failed'), // proxy 2, attempt 2
new Response('', { status: 502 }), // proxy 3, attempt 1
Expand Down
22 changes: 21 additions & 1 deletion src/services/importers/corsProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,18 @@ const BACKOFF_MULTIPLIER = 2

export class ProxyFetchError extends Error {
readonly attempts: number
/**
* True when a proxy authoritatively reported the target as *not found* (a 404,
* or the worker's `X-Proxy-Status: 404` signal). Callers use this to skip a
* missing resource quietly — a batch import expects semesters that don't exist.
*/
readonly notFound: boolean

constructor(message: string, attempts: number) {
constructor(message: string, attempts: number, notFound = false) {
super(message)
this.name = 'ProxyFetchError'
this.attempts = attempts
this.notFound = notFound
}
}

Expand Down Expand Up @@ -150,6 +157,17 @@ export async function fetchViaProxies(
headers: { Accept: 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8' },
})

// Authoritative "not found": the target was reached and has no such
// resource (a missing semester in a batch, a mistyped link). Every proxy
// would return the same, so stop the whole chain and flag it notFound so
// the caller can skip it quietly — otherwise the fallback proxies get
// tried and spray the console with CORS errors for a file that isn't
// there. Our worker reports this as a 200 + `X-Proxy-Status: 404` (a bare
// 404 status makes the browser log its own error); public proxies 404.
if (response.status === 404 || response.headers.get('X-Proxy-Status') === '404') {
throw new ProxyFetchError(`Target not found (404): ${url}`, attempts, true)
}

if (response.ok) {
const body = await response.text()
if (validate && !validate(body)) {
Expand Down Expand Up @@ -177,6 +195,8 @@ export async function fetchViaProxies(
errors.push(`Proxy ${proxyIndex + 1}: HTTP ${response.status}`)
break
} catch (error) {
// A terminal "not found" short-circuits the whole chain (thrown above).
if (error instanceof ProxyFetchError && error.notFound) throw error
const message = error instanceof Error ? error.message : String(error)
errors.push(`Proxy ${proxyIndex + 1}, attempt ${retry + 1}: ${message}`)
} finally {
Expand Down
31 changes: 31 additions & 0 deletions src/services/importers/runImport.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
deriveIcsBaseUrl,
icsFileName,
BatchIcsError,
type BatchProgress,
} from './runImport'
import { appDataSchema, type AppData } from '@/domain/model'
import { createCourse, type CourseInput } from '@/domain/course'
Expand Down Expand Up @@ -269,6 +270,36 @@ describe('runBatchIcsImport', () => {
])
})

it('reports progress for the range (Preparing → each semester → Done)', async () => {
const fetchImpl = vi.fn(async (input: RequestInfo | URL) => {
const url = String(input)
if (url.includes('winter-2024-2025'))
return new Response(ICS_FOR('קורס חורף'), { status: 200 })
if (url.includes('summer-2024')) return new Response(ICS_FOR('קורס קיץ'), { status: 200 })
return new Response('', { status: 404 }) // spring-2024 + catalog
})
const seen: BatchProgress[] = []

await runBatchIcsImport(
baseData(),
'https://cheesefork.cf/ical/winter-2024-2025.ics',
{ season: 'Winter', year: 2024 },
{ season: 'Summer', year: 2024 },
{
semesterName: '',
nowIso: NOW,
fetchImpl,
delayFn: async () => {},
onProgress: (p) => seen.push(p),
},
)

expect(seen[0]).toEqual({ completed: 0, total: 3, current: 'Preparing…' })
expect(seen.at(-1)).toEqual({ completed: 3, total: 3, current: 'Done' })
expect(seen.map((p) => p.current)).toContain('Winter 2024-2025')
expect(seen.every((p) => p.total === 3)).toBe(true)
})

it('throws when the sample url is not an .ics link', async () => {
await expect(
runBatchIcsImport(
Expand Down
18 changes: 18 additions & 0 deletions src/services/importers/runImport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ export interface IcsImportOptions {
enrich?: boolean
/** A pre-fetched catalog to enrich from, so a batch downloads it only once. */
catalog?: Map<string, CatalogEntry>
/** Batch only: called as each semester is processed, so the UI can show real progress. */
onProgress?: (progress: BatchProgress) => void
}

export interface BatchProgress {
/** Semesters finished (imported or skipped) so far. */
completed: number
/** Total semesters in the requested range. */
total: number
/** The semester currently being fetched, or a `Preparing…`/`Done` label. */
current: string
}

export interface IcsImportResult {
Expand Down Expand Up @@ -102,6 +113,7 @@ export async function runBatchIcsImport(
// Download the (large) Technion catalog once and reuse it for every semester in
// the range, rather than re-fetching it per import. Best-effort: a failure
// yields an empty catalog and the schedules still import un-enriched.
options.onProgress?.({ completed: 0, total: range.length, current: 'Preparing…' })
const catalog =
options.enrich === false
? new Map<string, CatalogEntry>()
Expand All @@ -113,6 +125,11 @@ export async function runBatchIcsImport(

for (const ref of range) {
const name = semesterName(ref.season, ref.year)
options.onProgress?.({
completed: imported.length + skipped.length,
total: range.length,
current: name,
})
try {
const result = await runIcsImport(current, base + icsFileName(ref), {
semesterName: name,
Expand Down Expand Up @@ -142,6 +159,7 @@ export async function runBatchIcsImport(
}
}

options.onProgress?.({ completed: range.length, total: range.length, current: 'Done' })
return { data: current, imported, skipped }
}

Expand Down
11 changes: 11 additions & 0 deletions src/styles/theme.css
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,17 @@
animation: highlight-pulse 1.4s ease-out;
}

/* Indeterminate progress: a segment sweeps across the track while work of an
unknown duration is in flight (e.g. a single schedule fetch). */
@keyframes progress-sweep {
0% {
left: -40%;
}
100% {
left: 100%;
}
}

@media (prefers-reduced-motion: reduce) {
*,
*::before,
Expand Down
11 changes: 10 additions & 1 deletion vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,18 @@ function corsDevProxy(): Plugin {
upstreamHeaders.Cookie = 'SOCS=CAI; CONSENT=YES+cb.20210328-17-p0.en+FX+678'
}
const upstream = await fetch(target, { redirect: 'follow', headers: upstreamHeaders })
res.setHeader('Access-Control-Allow-Origin', '*')
res.setHeader('Access-Control-Expose-Headers', 'X-Proxy-Status')
// Report not-found as a 200 + header, not a 4xx, so the browser doesn't
// log a console error for an expected miss (see workers/cors-proxy).
if (upstream.status === 404) {
res.statusCode = 200
res.setHeader('X-Proxy-Status', '404')
res.end()
return
}
const body = Buffer.from(await upstream.arrayBuffer())
res.statusCode = upstream.status
res.setHeader('Access-Control-Allow-Origin', '*')
res.setHeader(
'Content-Type',
upstream.headers.get('content-type') ?? 'text/plain; charset=utf-8',
Expand Down
12 changes: 12 additions & 0 deletions workers/cors-proxy/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ function corsHeaders(origin) {
'Access-Control-Allow-Origin': allowOrigin,
'Access-Control-Allow-Methods': 'GET, OPTIONS',
'Access-Control-Allow-Headers': '*',
// So the client can read the not-found signal (see the 404 handling below).
'Access-Control-Expose-Headers': 'X-Proxy-Status',
Vary: 'Origin',
}
}
Expand Down Expand Up @@ -157,6 +159,16 @@ export default {
return new Response('Upstream returned an unparseable url', { status: 502, headers: cors })
}

// Report an upstream "not found" as a 200 carrying `X-Proxy-Status: 404`,
// not a bare 404: a 4xx makes the browser log a console error, even though a
// missing file (e.g. a semester that doesn't exist in a batch import) is
// expected. The client reads this header and skips the resource quietly.
if (upstream.status === 404) {
const notFound = new Headers(cors)
notFound.set('X-Proxy-Status', '404')
return new Response(null, { status: 200, headers: notFound })
}

const headers = new Headers(cors)
const contentType = upstream.headers.get('content-type')
if (contentType) headers.set('Content-Type', contentType)
Expand Down
Loading