geocrop-platform./apps/nextgen/.harness/plans/2026-07-22-phase1-pr2-pr3-execution.md

# Phase 1 — PR 2 closeout + PR 3 execution plan

**Date:** 2026-07-27
**Branch:** `feature/admin-cohorts-2026-07-22` (from `dev`)
**Companion:** [Phase 1 design](./2026-07-22-phase1-cohorts-assignments.md)

> **Why this doc exists:** the design plan lays out *what* ships. This doc is the
> ordered execution list for the closeout of PR 2 (cohorts frontend) and the
> full build of PR 3 (class + teacher assignment UX). Read top-to-bottom, do
> in order.

## Current state (re-verified 2026-07-27)

PR 1 (auxiliary roles) is merged + pushed on the feature branch.
PR 2 (cohorts) is **partially built** on the same branch — backend + Zustand
store + list page + detail page are written, but routes/nav/E2E are missing.

| Piece | State |
|---|---|
| Knex migration `2026072200000020_cohorts.js` | ✅ written, on disk |
| `server/src/controllers/cohorts.controller.js` | ✅ written, on disk (10 endpoints, audit-logged) |
| `server/src/index.js` mount at `/api/cohorts` | ✅ modified, uncommitted |
| `server/src/services/SyncEngine.js` `tablesToSync` (4 new tables) | ✅ modified, uncommitted |
| `client/src/store/cohorts.ts` (Zustand) | ✅ written, on disk |
| `client/src/pages/admin/Cohorts.tsx` (list) | ✅ written, on disk |
| `client/src/pages/admin/CohortDetail.tsx` (4 tabs + 3 modals) | ✅ written, on disk |
| `client/src/components/CohortForm.tsx` (create/edit) | ❌ missing |
| `client/src/App.tsx` routes (`/admin/cohorts`, `/admin/cohorts/:id`, `/admin/cohorts/new`) | ❌ missing |
| `client/src/components/Nav.tsx` Cohorts entry | ❌ missing |
| `client/e2e/helpers/nav.ts` ROUTES_PER_ROLE updates | ❌ missing |
| `client/e2e/cohorts.spec.ts` smoke | ❌ missing |
| `.harness/changelogs/2026-07-22-cohorts.md` | ❌ missing |
| Commit + push | ❌ pending |

**Backend smoke from prior session confirmed working:** create cohort, add student,
link class, link exam group, list, detail, remove student, unlink class.

## PR 2 — Cohorts closeout

### A. `client/src/components/CohortForm.tsx` (new)

A modal component that handles both create and edit. The plan's spec:
- programme dropdown drives `level` suggestions
- submit calls `createCohort` / `updateCohort` from store
- close on success

**Programme → level suggestions** (encoded as a constant, not magic strings):

```ts
const PROGRAMME_LEVELS: Record<string, string[]> = {
  zimsec:  ['ECD A', 'ECD B', 'Grade 1', 'Grade 2', 'Grade 3', 'Grade 4',
            'Grade 5', 'Grade 6', 'Grade 7', 'Form 1', 'Form 2', 'Form 3',
            'Form 4', 'Form 5', 'Form 6'],
  igcse:   ['Year 1', 'Year 2', 'IGCSE'],
  as:      ['Form 5', 'AS Level'],
  a2:      ['Form 6', 'A2 Level'],
  primary: ['Reception', 'Year 1', 'Year 2', 'Year 3', 'Year 4', 'Year 5', 'Year 6'],
  other:   [],  // free-text only
};
```

**UI shape** (mirrors `CoverAssignmentForm` + `UserRoles` patterns already in
the repo):
- ModalShell (fixed inset-0 overlay, inner `stopPropagation`)
- Form fields: name (required), programme (required, dropdown), level (datalist
  of suggestions + free-text), academic_year (text), description (textarea),
  start_date / end_date (date inputs, optional), is_active (toggle)
- Two action buttons: Cancel + "Save cohort" / "Create cohort"
- Loading + error states; on success, `onClose()` — parent navigates away

**Props:**
```ts
interface CohortFormProps {
  cohortId?: number;        // omit for create
  onClose: () => void;
  onSaved?: (cohort: Cohort) => void;
}
```

**Why not edit-in-place on the detail page:** cards navigate to detail, and the
detail page is read-mostly with tab actions. The "Edit cohort" action is a
small button on the detail page header that opens this same modal. Simpler than
two separate code paths.

### B. `client/src/App.tsx` — add cohort routes

Three routes per role arm (school_admin, systems_admin, principal):
- `/admin/cohorts` → `Cohorts` (list)
- `/admin/cohorts/new` → a thin wrapper that renders `CohortForm` with no
  `cohortId` and on close navigates to `/admin/cohorts` (use a small inline
  component or pass a wrapper; the wrapper is ~10 lines)
- `/admin/cohorts/:id` → `CohortDetail`

Add an Edit button to the detail page header (admin only) that navigates to
`/admin/cohorts/:id/edit` (same wrapper but with `cohortId=id`).

**Add new page imports at the top of App.tsx:**
```ts
import Cohorts from './pages/admin/Cohorts';
import CohortDetail from './pages/admin/CohortDetail';
import CohortForm from './components/CohortForm';
```

Mount under the `case 'school_admin':`, `case 'systems_admin':`, and
`case 'principal':` arms. Principal is read-only — gate the routes with the
same `allowedRoles` array but the page checks `canManage` internally (it
already does).

### C. `client/src/components/Nav.tsx` — Cohorts entries

Add to all three role arms (school_admin, systems_admin, principal):

```ts
{ path: '/admin/cohorts', label: 'Cohorts', icon: GraduationCap },
```

The `GraduationCap` icon is already imported. Place it near the existing
`/students` entry for natural grouping.

### D. `client/e2e/helpers/nav.ts` — extend ROUTES_PER_ROLE

Add `/admin/cohorts` to `school_admin` and `systems_admin`. Skip principal
for now (no principal smoke spec yet — see `prinicpal: []` note in the file).

### E. `client/e2e/cohorts.spec.ts` (new)

Smoke + happy-path:

```ts
test.describe('Cohorts (Phase 1 PR 2) @flow', () => {
  test.beforeEach(async ({ page }) => { await loginAs(page, 'school_admin'); });

  test('list page mounts and shows the empty or cohort state', async ({ page }) => {
    await page.goto(`${BASE}/admin/cohorts`);
    await expect(page.locator('h1, h2').first()).toBeVisible();
    // Either a cohort card or the "No cohorts match" empty state
    const cardOrEmpty = page
      .getByText(/Form 4 IGCSE 2026|Grade 7 ZIMSEC|No cohorts/i)
      .or(page.getByRole('button', { name: /New Cohort/i }));
    await expect(cardOrEmpty.first()).toBeVisible({ timeout: 5_000 });
  });

  test('create new cohort end-to-end via the form', async ({ page }) => {
    await page.goto(`${BASE}/admin/cohorts/new`);
    const cohortName = `E2E ${Date.now()}`;
    await page.getByLabel(/name/i).fill(cohortName);
    await page.locator('select').first().selectOption('igcse');
    await page.getByLabel(/level/i).fill('Year 1');
    await page.getByLabel(/academic year/i).fill('2026');
    await page.getByRole('button', { name: /Create cohort|Save cohort/i }).click();
    // Should land back on list and show the new cohort
    await expect(page).toHaveURL(/\/admin\/cohorts$/);
    await expect(page.getByText(cohortName)).toBeVisible({ timeout: 5_000 });
  });

  test('principal can read cohort list but cannot see New Cohort', async ({ page }) => {
    // No principal demo seed; skip if not present
    const ok = await isRoleAvailable('principal');
    test.skip(!ok, 'principal demo account not in seed');
    await loginAs(page, 'principal');
    await page.goto(`${BASE}/admin/cohorts`);
    await expect(page.locator('h1, h2').first()).toBeVisible();
    await expect(page.getByRole('button', { name: /New Cohort/i })).toHaveCount(0);
  });

  test('teacher gets 403 on the cohort list API', async ({ page }) => {
    await loginAs(page, 'teacher');
    const res = await page.request.get(`${API}/cohorts`);
    expect(res.status()).toBe(403);
  });
});
```

This file follows the same patterns as `client/e2e/rbac.spec.ts` (uses
`loginAs`, `isRoleAvailable`, base URL constants).

### F. `.harness/changelogs/2026-07-22-cohorts.md` (new)

Brief changelog per project convention (see
`.harness/changelogs/2026-07-22-user-roles.md` for the template).

### G. Commit + push

Single feature commit for PR 2:
- Subject: `feat(cohorts): frontend + routes + e2e (PR 2 closeout)`
- Body references the plan doc and lists the new files + key behaviors
- Push to `origin/feature/admin-cohorts-2026-07-22`

## PR 3 — Class + teacher assignment UX

### Backend (additive, no schema changes)

The plan calls for 5 new endpoints, all in existing controllers. Each one is
small and audited. No new migration needed — `enrollments`, `classes`, and
`subjects` already have the right columns (`class_teacher_id`, `teacher_id`,
`status`, `request_status`).

| Endpoint | Controller | Notes |
|---|---|---|
| `POST /api/students/bulk-enroll` | students.controller.js | `{ studentIds, classId, academicYear }` — bulk insert, `status='active', request_status='approved'`. Idempotent on existing rows. Wrapped in `db.transaction()`. Admin-only. |
| `POST /api/enrollments/:id/transfer` | enrollments.controller.js | `{ newClassId }` — sets old to `transferred` (request_status), creates new `active`/`approved` row. Wrapped in `db.transaction()`. Admin/teacher. |
| `PUT /api/classes/:id/class-teacher` | classes.controller.js | `{ teacherId }` — sets `classes.class_teacher_id`, validates teacher exists + role='teacher'. Admin-only. |
| `GET /api/classes/:id/teachers` | classes.controller.js | Returns form tutor + per-subject teachers. Used by ClassDetail's Teachers tab. |
| `POST /api/classes/:id/subjects/:subjectId/teacher` | classes.controller.js | `{ teacherId }` — sets `subjects.teacher_id`. Admin-only. |
| `POST /api/classes?withAssignments=1` (create with assignments) | classes.controller.js | Accepts `{ name, section, capacity, classTeacherId, initialStudentIds }`. Wrapped in `db.transaction()`. Admin-only. |

**Note on the transfer route:** the existing
`enrollments.controller.js` already has a `/withdraw` endpoint at
`POST /:id/withdraw` and a `/approve`, `/reject`. The plan's "withdraw" is
already in place — no new work needed there. The new endpoints are just
`transfer` and the four class-side ones.

**E2E for backend:** covered by the frontend spec below (the happy-path tests
exercise the real endpoints, not mocks).

### Frontend (5 new files + 2 modified)

**New files:**
- `client/src/pages/admin/ClassDetail.tsx` — 5 tabs (Overview / Roster / Teachers / Subjects / Cohorts). "Currently covering" indicator on Teachers tab uses the existing `GET /api/user-roles/active-covers` endpoint from PR 1.
- `client/src/components/CreateClassWizard.tsx` — multi-step modal: 1) class details, 2) form tutor, 3) initial roster (optional). On submit, calls `POST /api/classes?withAssignments=1`.
- `client/src/store/classDetail.ts` — Zustand slice for class detail (parallel to the cohorts pattern).

**Modified files:**
- `client/src/pages/Classes.tsx` — replace existing simple create form with `CreateClassWizard`. Add "View detail" button on each row → `/admin/classes/:id`.
- `client/src/pages/Students.tsx` — add "Bulk assign" toolbar button → modal. Add enrollment status column. Pending-approval rows with Approve/Reject.
- `client/src/components/CohortDetail.tsx` — on the Classes tab, link each class to `/admin/classes/:id` (cross-link to PR 3 surface).
- `client/src/App.tsx` — add `/admin/classes/:id` route (admin + principal).
- `client/src/components/Nav.tsx` — add `/admin/classes/:id` (only meaningful when filtered by id, so this is more of a deep-link target — the main nav entry stays `/classes`).
- `client/e2e/helpers/nav.ts` — add `/admin/classes/1` to `school_admin`.

### File-order guidance (conventions: 5-file commit)

PR 3 ships in two commits:
1. **Backend commit** — 5 controller changes + AuditService calls. Backend
   smoke test via curl.
2. **Frontend commit** — 3 new files + 3 modified files + App.tsx + Nav +
   nav.ts helper.

**Why split:** easier to review the API contract before the UI. The backend
contract is what the frontend signs against, and a coherent API surface is
easier to land when reviewed in isolation.

### Out of scope for PR 3 (explicit)

- **Auto-creating a cohort when a class is created.** Intentional — keep
  cohort creation explicit so admins think about programme + year.
- **Cross-class bulk transfer.** Single-class transfer only; bulk transfer
  is a follow-up.
- **Subject teacher UI rewrite.** The class detail's Subjects tab shows
  current teachers and lets admin change them — the underlying `subjects`
  table is the source of truth; no separate "subject catalog" page is in
  scope.
- **Per-cohort enrollment report.** A read-only view of "students in cohort X
  enrolled in class Y" is a useful Phase 2 dashboard, not a PR 3 deliverable.

## Files to change (cumulative)

### PR 2 closeout
- `client/src/components/CohortForm.tsx` (new)
- `client/src/App.tsx` (modify — 3 routes × 3 role arms + 2 page imports + 1 component import)
- `client/src/components/Nav.tsx` (modify — 3 entries)
- `client/e2e/helpers/nav.ts` (modify — 2 entries)
- `client/e2e/cohorts.spec.ts` (new)
- `.harness/changelogs/2026-07-22-cohorts.md` (new)
- Commit + push

### PR 3
- `server/src/controllers/students.controller.js` (modify — `POST /bulk-enroll`)
- `server/src/controllers/enrollments.controller.js` (modify — `POST /:id/transfer`)
- `server/src/controllers/classes.controller.js` (modify — 3 new routes)
- `client/src/pages/admin/ClassDetail.tsx` (new)
- `client/src/components/CreateClassWizard.tsx` (new)
- `client/src/store/classDetail.ts` (new)
- `client/src/pages/Classes.tsx` (modify)
- `client/src/pages/Students.tsx` (modify)
- `client/src/pages/admin/CohortDetail.tsx` (modify — cross-link to /admin/classes/:id)
- `client/src/App.tsx` (modify)
- `client/src/components/Nav.tsx` (modify)
- `client/e2e/helpers/nav.ts` (modify)
- `client/e2e/class-assignments.spec.ts` (new)
- `.harness/changelogs/2026-07-22-class-assignments.md` (new)
- Commit + push (×2 — backend, then frontend)

## Verification

- [ ] `npm run db:init` runs cleanly on a fresh DB (idempotent)
- [ ] Backend smoke: create class, assign form tutor, bulk-enroll 3 students
- [ ] Backend smoke: transfer student (old → transferred, new → active)
- [ ] Backend smoke: link subject teacher
- [ ] Frontend smoke (admin): navigate to `/admin/classes/1`, see all 5 tabs,
      switch a form tutor, see it update
- [ ] Frontend smoke (admin): bulk-assign 2 unassigned students
- [ ] Frontend smoke (principal): read-only on ClassDetail (no Edit / Change
      buttons; can see Roster + Teachers + Cohorts)
- [ ] Frontend smoke (teacher): `GET /api/classes/1/teachers` works for a
      class they teach, gets 403 for a class they don't
- [ ] Portal smoke (`@smoke`) for all three role arms walks the new routes
      without errors
- [ ] Cohorts smoke (`@flow`) from PR 2 still passes
- [ ] `git log --oneline origin/feature/admin-cohorts-2026-07-22` shows the
      three new commits (cohorts closeout, PR 3 backend, PR 3 frontend) on
      top of the merge commit

## Order of execution

1. PR 2 closeout, top to bottom (A → G). Commit + push.
2. PR 3 backend (4 controller changes, all in one commit). Smoke via curl.
3. PR 3 frontend (top to bottom). Commit + push.
4. Run full Playwright suite (portal smokes + cohorts flow + class-assignments
   flow) and confirm green.

**Why this order:** PR 2's frontend is the smaller lift and is half-done
already; shipping it unblocks the cross-link from CohortDetail to ClassDetail
(PR 3). PR 3's backend is small and isolated — getting the API surface
locked first lets the frontend sign against a known contract.