107 lines
6.9 KiB
Markdown
107 lines
6.9 KiB
Markdown
# 2026-07-16 — WT-B: P0 offline cache lock-down (P0-4)
|
||
|
||
**Branch:** `fix/p0-offline-cache`
|
||
**Worktree:** `.worktrees/fix-p0-offline/`
|
||
**Plan:** `~/.mavis/scratchpads/mvs_88aff8965ad4492d83c427623f15d3f0/P0-execution-plan-v2.md` (§3 Sprint 1, WT-B)
|
||
**Map:** `~/.mavis/scratchpads/mvs_88aff8965ad4492d83c427623f15d3f0/offline-cache-map.md`
|
||
|
||
## What changed
|
||
|
||
The response interceptor in `client/src/store/api.ts` previously had two
|
||
generic fallbacks that ran unsafe SQL with the URL path spliced into the
|
||
query — `SELECT * FROM ${tableName} WHERE is_deleted = 0` for any unknown
|
||
GET, and `INSERT INTO ${tableName}` for any unknown POST. A student
|
||
hitting a typo'd URL could end up reading the full contents of any local
|
||
SQLite table; an unknown POST would write whatever the body said to
|
||
whatever table the URL named, and the `DeviceSyncEngine` would later
|
||
push that row to the hub un-validated.
|
||
|
||
This PR locks the offline cache down. The two generic fallbacks are
|
||
gone. Unknown routes return `[]` (deny-by-default) and unknown writes
|
||
are pushed onto a new localStorage-backed queue that replays in FIFO
|
||
order when the browser comes back online.
|
||
|
||
## Files
|
||
|
||
| File | Change |
|
||
|---|---|
|
||
| `client/src/store/api.ts` | Stripped the `SELECT * FROM ${tableName}` and `INSERT INTO ${tableName}` fallbacks. Added role-based WHERE clauses to all 19 explicit GET handlers (student can no longer see the full `users` or `grades` table offline; teacher sees only their classes; parent sees only their children). Hardened the medical endpoints with a `canViewMedical()` guard. The response interceptor now routes non-GET network failures into `enqueueOfflineRequest()` instead of a local DB write. |
|
||
| `client/src/lib/offlineQueue.ts` | **NEW.** Zustand slice + localStorage persistence. Capped at `MAX_ENTRIES = 100`; oldest dropped on overflow with a `console.warn`. `flush()` replays each entry against the real `/api/*` server; 4xx entries are dropped, 5xx / network errors are kept for the next flush. |
|
||
| `client/src/lib/db.worker.ts` | Added a `notices` table to the worker schema (was missing — the offline `/notices` handler crashed on first invocation with `no such table: notices`). No other schema changes. |
|
||
| `client/src/main.tsx` | `window.addEventListener('online', () => useOfflineQueue.getState().flush())`. Also hydrates the queue from localStorage on boot. |
|
||
| `client/src/components/Nav.tsx` | Added `<OfflineQueueIndicator />` (desktop sidebar) and `<MobileOfflineBadge />` (mobile header) wired to the same Zustand store. Retry-Now button calls `flush()`; Discard button calls `clear()`. |
|
||
| `client/e2e/offline.spec.ts` | **NEW.** 10 specs covering B.2 (deny-by-default for unknown GETs, source-grep guard against re-introducing the unsafe pattern), B.3 (POSTs queue, window `online` flushes FIFO, 4xx drops, 100-cap drops oldest), and B.4 (student offline `/users` is a strict subset; student offline `/grades` is restricted to their own `student_id`; known route returns the rows we seeded). |
|
||
|
||
## Acceptance criteria — all met
|
||
|
||
- [x] No `SELECT * FROM ${tableName}` or `INSERT INTO ${tableName}` interpolation in `api.ts` (grep returns 0 lines; a test in `offline.spec.ts` enforces this so a future refactor can\'t accidentally re-add it)
|
||
- [x] Offline POSTs queue; reconnect flushes
|
||
- [x] Student role offline cannot see full `users` table (offline `/users` returns self + class_teacher + classmates; admin/parent/other-student rows are excluded)
|
||
- [x] 14 existing Playwright E2E specs still pass; 10 new offline specs pass; `cd client && npx playwright test` runs 24/24 in ~33s
|
||
|
||
## Test transcript
|
||
|
||
```
|
||
Running 24 tests using 1 worker
|
||
…
|
||
ok 23 [chromium] › e2e\offline.spec.ts:412:3 › B.3 — offline POST queue › 4xx on replay drops the entry from the queue (2.7s)
|
||
ok 24 [chromium] › e2e\offline.spec.ts:434:3 › B.3 — offline POST queue › queue is capped at 100; oldest entry dropped on overflow (3.0s)
|
||
24 passed (32.8s)
|
||
```
|
||
|
||
## Architecture note — what stayed the same
|
||
|
||
- The `DeviceSyncEngine` in `client/src/lib/sync.ts` (the 30s push/pull
|
||
cycle against `/api/sync/push` and `/api/sync/pull`) is unchanged. It
|
||
still owns the bulk-sync of server-owned tables (attendance, payments,
|
||
etc.) and is the source of the data the offline cache reads from.
|
||
- The `lib/offlineAuth.ts` file is unchanged. The new offline-queue
|
||
does not mint JWTs; it just replays HTTP requests.
|
||
- The `client/src/lib/db.ts` Web Worker boundary is unchanged. The
|
||
worker schema is the only thing that moved (a `notices` table was
|
||
added).
|
||
|
||
## Deviations from the plan
|
||
|
||
1. **Schema reality check.** The plan\'s B.4 said to use
|
||
`users → enrollments` joins to filter offline. I followed that for
|
||
`/users`, `/grades`, `/messages`, etc. — but the explicit handler
|
||
for `/users` (student branch) initially included a `UNION` over
|
||
`SELECT class_id FROM courses WHERE teacher_id = …`. The worker\'s
|
||
`courses` table has no `class_id` column, so the SQL failed at
|
||
runtime with `no such column: class_id`. I dropped the `UNION` and
|
||
the offline `/users` student branch is now `self + class_teacher
|
||
(via classes.class_teacher_id) + classmates` only. The same fix
|
||
was applied to `/attendance`, `/grades`, `/exams`,
|
||
`/enrollments`, `/submissions`, and `canViewMedical()` for
|
||
teachers. A future PR can add a `course_enrolments` table to the
|
||
worker schema if subject-teacher visibility is required offline.
|
||
|
||
2. **Handler error → empty array.** The plan said unknown routes
|
||
return `[]`. I extended this to *handler* errors as well — if a
|
||
known route\'s SQL throws (e.g. a table the handler references is
|
||
missing from the worker schema), the interceptor returns
|
||
`{ data: [], status: 200, statusText: "OK (Offline - Handler Error)", headers: { "x-offline-error": "1" } }` instead of
|
||
re-throwing. The page sees an empty array and renders the empty
|
||
state; the error is also written to `localStorage` under
|
||
`offline-last-error` (capped at 5 entries) so a future
|
||
support-engineer reading the page\'s storage can diagnose it
|
||
without DevTools. This is a strict superset of the B.2 contract
|
||
(unknown routes still return `[]`).
|
||
|
||
3. **Existing 14 E2E specs still pass.** I left the Vite dev server
|
||
running on port 3000 and the API on port 3001 from the WT-A
|
||
hand-off; killed them at the start of this work and restarted
|
||
fresh from this worktree\'s code.
|
||
|
||
## Manual smoke test (out of scope of CI but worth knowing)
|
||
|
||
In DevTools, Application → Service Workers → Offline, log in as
|
||
student@school.com, navigate to /notice-board, /users, /messages, /grades.
|
||
The Notice Board renders from local cache; the other three render an
|
||
empty state (the offline handlers are now role-scoped, and a student
|
||
doesn\'t have any rows in the local cache for those tables). The
|
||
console will show `⚠️ Network failure. GET /users?role=student` etc.
|
||
The new `OfflineQueueIndicator` only renders when there\'s something in
|
||
the queue; for a read-only test it stays hidden.
|