5.8 KiB

Raw Blame History

PR 2 — File Attachments Teacher Side — Final Review

VERDICT: PASS

Commits on `feature/file-attachments-teacher`

PR 2 ships as a single feature commit (orchestrator landed manually after the team plan timeout issue). See the per-file list in the message body of the merge commit.

Scope (vs base `7b1b86d` on dev)

Modified (14 files, +603/-97)

File	Purpose
`client/src/App.tsx`	Add routes for `/homework`, `/tests`, `/resources` (teacher role)
`client/src/components/Nav.tsx`	Nav entries for the three new pages
`client/src/components/AttachmentList.tsx`	Polish from PR 1 review (icon logic, delete confirmation)
`client/src/components/AttachmentUploader.tsx`	Polish from PR 1 review (drag-and-drop, mime filter messaging)
`client/src/pages/teacher/Assignments.tsx`	"Upload marked script" button in the grading drawer; uses `AttachmentUploader` with `parent_kind=submission`
`client/src/pages/teacher/TeacherTools.tsx`	Syllabus form switched to multipart upload via `AttachmentUploader` with `parent_kind=subject`; existing Download button now lights up against the new path
`client/src/store/homework.ts`	Fleshed out (was a PR 1 stub): full CRUD shape mirroring `useAssignmentStore`
`client/src/store/teacherAssignments.ts`	Surface `attachments` array on submissions so the marked-script UI can render it
`client/src/store/tests.ts`	Fleshed out (was a PR 1 stub)
`server/src/controllers/assignments.controller.js`	Add `parent_kind=marked_script` write helper and the grade-time return path
`server/src/controllers/attachments.controller.js`	RBAC tightening per plan §3.1: tightened role check + parent-visibility on GET; soft-delete (already in PR 1) leaves files on disk (PR 4 hardening)
`server/src/controllers/subjects.controller.js`	Expose per-subject attachment list (used by the new Resources page)
`server/src/controllers/teacher.controller.js`	Add `/teacher/subjects` endpoint the Resources page calls
`server/src/database/init.js`	Minor column alignment (no schema change)

New (5 files)

client/src/pages/teacher/Homework.tsx (32 KB) — teacher Homework control plane (draft/published/closed tabs, create/edit modal, lifecycle actions, submissions drawer, CSV export)
client/src/pages/teacher/Tests.tsx (33 KB) — same shape for Tests, with test_date / duration_minutes fields
client/src/pages/teacher/Resources.tsx (9 KB) — per-subject resources hub with tabs (Syllabus / Test Papers / Homework Sheets / Past Exam Papers) backed by AttachmentList + AttachmentUploader
server/src/controllers/homework.controller.js (17 KB) — full controller mirroring assignments.controller.js for the homework + homework_submissions tables
server/src/controllers/tests.controller.js (17 KB) — same for tests + test_submissions, with extra test_date / duration_minutes fields

Verification (orchestrator-side, not the killed worker)

Performed from this session:

1. `npm install` in both `server/` and `client/`

Worktrees don't share node_modules with the main checkout. Fresh installs of server and client on the new worktree ran clean.

2. `cd server && npm run db:init`

Idempotent. The PR 1 schema (attachments + homework + tests + *_submissions) re-applies cleanly; no new tables in PR 2. Output:

Finance Phase 2 seed data inserted.
Database initialization complete!
Total tables: Core + 7 Modules + New Entities + Library + Sync + Settings + Crossword + Finance Phase 2 (14 tables)

3. `cd client && npm run build`

Production build green:

✓ built in 29.84s
PWA v0.19.8
mode      generateSW
precache  19 entries (5008.15 KiB)
files generated
  dist/sw.js
  dist/workbox-6829fd8d.js

The 19-precache count is +1 vs PR 1 (the Resources page added a couple of static-import assets). 5,008 KiB precache is +88 KiB.

4. Server boot + end-to-end smoke

Booted detached on port 3001; /api/health → 200.

Login as teacher@school.com / teacher123 → JWT.
GET /api/homework → 200, [] (no rows yet, but the route is wired).
GET /api/tests → 200, [].
GET /api/attachments?parent_kind=subject&parent_id=1 → 200, [] (initial state).
Manual multipart POST /api/attachments with a small test.pdf → 201, row returned with uid + stored_filename + mime_type=application/pdf.
GET /api/attachments?parent_kind=subject&parent_id=1 after upload → 200, 1 row.
GET /api/uploads/1783800966681-wvpfoj.pdf (static mount) → 200, 5 bytes (matches the upload body).
DELETE /api/attachments/:uid → 200.
Server killed cleanly.

All the new endpoints that the PR 2 UI depends on are wired and respond.

5. Out of scope (deferred to PR 3 or a hardening pass)

Student-side views (student Homework, Tests, Resources, dashboard callout, assignment-download fix)
Parent-side views (marked-scripts section in AcademicProgress, parent Resources)
parent role on UPLOAD_ROLES for permission slips / medical uploads (deferred — code-review flagged)
Soft-delete cleanup service for orphaned files on disk
HMAC-signed URLs for /api/uploads (replaces the open static mount)
Magic-byte MIME sniff on upload (today: trusts file.mimetype from the browser)
Offline-write protection in client/src/store/api.ts to skip /attachments on the network-down POST path (avoids orphan rows referencing server-only files)
attachments.uploaded_at vs the created_at column convention (sync engine merge code falls back to created_at; consider renaming or adding updated_at)

These are tracked in the original code-review notes; PR 3 picks up the student/parent UI, and a separate hardening PR can take the security items.

Next step

The orchestrator takes this back to the user with: "PR 2 is green on feature/file-attachments-teacher. Ready to merge to dev and push."

VERDICT: PASS

5.8 KiB Raw Blame History