104 lines
5.8 KiB
Markdown
104 lines
5.8 KiB
Markdown
# 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
|