Classes as a first-level entity: model, WhatsApp CRUD, portal CRUD - #230
Merged
Conversation
…resolvers
A "class" was a row in `student_lists`, a table the attendance feature created
to hold a roster to mark attendance against. Class identity was a side effect of
a feature, so it inherited that feature's shape: owned by ONE teacher, named by
free text, with no school, no subject, no session, and no student who exists
independently of it.
Measured on production before this change:
1 student_lists rows 465 schools
29 students 8,797 users with school_id
21 of those with NO list_id 7,149 leader_teachers
1 attendance_sessions 433 leader_schools
943 quizzes — of which 0 carry a list_id
The roster model is unused, and the institutional spine it should have hung off
already exists and is fully populated. Meanwhile class identity has degraded
into a string: those 21 rostered-nowhere students carry 18 distinct spellings of
what are really about three classes (3 · 3b · 3-c · Class:3 · 4A · 4-A · ⁴ A ·
5th A · class 5-c …).
WHAT LANDS
Seven tables (V1.1.3, mirrored into the bootstrap schema, RLS, and seed):
academic_sessions the period a class sits for — duration-agnostic
(annual/semester/term + real dates), so a non-year
academic system is representable
grade_levels 14 canonical grades with ordinal + band + aliases
subjects 6 codes, scoped to what the LP corpus can serve
classes school × grade × section × session
class_teachers one row per (class, teacher); the ROLE lives here
class_teacher_subjects which subjects that one assignment covers
class_enrollments membership as a row with a date range
Plus two services: a pure free-text label parser, and a cached resolver that
maps the legacy encodings onto the new vocabulary.
DESIGN NOTES
- The role is on the (class, teacher) pair, not the subject row: a teacher
taking Math AND Science for 4-A who is also its class teacher would otherwise
carry an ambiguous flag on two rows. At most one prime-responsible teacher per
class, enforced by a partial unique index.
- Enrollment is a row, not a pointer. A class ends with its session; students
are then promoted, or retained into a same-grade class in the NEXT session.
With a pointer column every rollover duplicates the child.
- Reference tables hold NO display labels. Field caps are an outage class here
and the cap audit measures source, so a label in the database is invisible to
it. Copy goes in ux-strings keyed by these codes.
- The parser refuses rather than guesses: a digit inside junk text ("Test Class
3A") is incidental, not a grade, and a wrong grade silently picks the wrong
reading passage. The resolver fails CLOSED for the same reason.
- Subjects are deliberately LP-corpus-scoped, so "teaches Islamiat to 4-A" is
not yet representable. Adding one is a one-row INSERT.
NOT in this change: no feature is migrated. attendance / quizzes / reading keep
reading `student_lists`, untouched. Nothing is dropped or altered. The migration
has NOT been applied to any database.
TESTS 95 new, all green (48 parser, 47 resolver). Suite delta is clean: 32
failed suites / 89 failed tests both before and after, all pre-existing.
Mutation-checked the band-ordering guard (4 red when broken, restored green).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e legacy mirror
The write surface for the classes model, plus the bridge that keeps the old world
working while the new one is adopted.
THE MIRROR. The new class CRUD becomes the single teacher-facing way to create a
class, but attendance and 943 existing quizzes still read `student_lists`. So
createClass() also writes — or ADOPTS — a mirror roster row and links it through
the new student_lists.class_id column.
Adoption matters more than insertion: student_lists is unique on
(user_id, LOWER(class_name), academic_year) WHERE is_active, so a teacher who
already added "Grade 4" the old way must have that row adopted with its students
intact, not hit a duplicate-key error. Another teacher's identically-named roster
is deliberately left alone.
A failed mirror does NOT fail the class. Losing the class a teacher just created
is unrecoverable; a missing mirror only degrades attendance visibility and is
repairable. The caller learns via `mirrored: false`.
The mirror, the class_id column, and mirrorLabel() are scaffolding and are all
removed by the cutover PR that moves attendance onto class_id.
API. createClass / assignTeacher / listClassesForTeacher / enrollStudent.
Errors are values ('unknown_grade', 'class_teacher_exists', 'unknown_subject', …)
rather than throws, because every caller is a Flow endpoint or an HTTP route that
must render a screen, not a stack trace.
Two deliberately opposite failure directions, since both were tempting:
- the vocabulary resolver fails CLOSED — a wrong grade silently picks the wrong
reading passage, so no answer beats a guess
- listClassesForTeacher fails to an EMPTY LIST — an empty "my classes" screen is
a state a teacher can act on; an error screen is a dead end
assignTeacher validates every subject BEFORE writing any, so a typo in the third
code cannot leave the first two assigned. It also catches a second
prime-responsible teacher in code, so the caller shows a sentence instead of
surfacing a 23505 from the partial unique index.
TESTS 24 new (119 across the model now), all green. Suite delta clean: 32 failed
suites / 89 failed tests both before and after, all pre-existing.
Includes tests/fixtures/fake-supabase.js — a small in-memory client that actually
applies filters, so "the mirror was adopted rather than duplicated" asserts
behaviour instead of mock choreography. Mutation-tested both ways: disabling
adoption and removing the class-teacher guard each turn exactly one test red.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… guard
The reference tables hold identity and structure and no display copy, so the
labels the teacher actually sees live in ux-strings keyed by those codes — same
pattern as languageLabelFor.
WHY NOT name_en / name_ur COLUMNS. Field caps are an outage class here, and the
cap audit measures SOURCE. A label stored in a database column is invisible to
it, so nothing would catch an over-cap grade name before Meta rejected the
message. Choosing name_ur over name_en at render time would also be a second
clamp implementation, which is the structural defect the catalog exists to remove.
13 grades and 6 subjects, en + ur. Urdu uses the standard جماعت + ordinal form
(اول، دوم، سوم …) rather than transliterated digits, which is how grades are named
in Pakistani classrooms.
THREE THINGS THE TESTS ENFORCE, because the decision only pays off if they are:
- COVERAGE: every seeded code has a label in BOTH offered languages, and the
key sets EQUAL the codes parsed straight out of 02_seed-data.sql. Add a
subject to the seed and forget the label, and the build fails instead of the
picker rendering a blank row.
- NO LAZY ur: a label whose Urdu equals its English is rejected. That is the
partial-map failure in disguise — it looks complete and reads as broken.
- CAPS in CODE POINTS, against the 20-point button cap, the tightest
teacher-facing field. Short enough for a button is short enough for a list row
and a Flow dropdown, so these are safe anywhere. The test also asserts the
measurement itself ([...s].length, not s.length), since .length would let an
Urdu label near the cap pass locally and fail at the Graph API.
CORRECTION: earlier commits in this branch describe grade_levels as 14 rows. It
is 13 — early_years plus grades 1 to 12. The seed and the labels always agreed
with each other; the prose count was wrong. Comments corrected here.
TESTS 52 new (171 across the model), all green. Existing ux-strings consumers
(settings, remark) still green at 60/60. Suite delta clean: 32 failed suites /
89 failed tests before and after.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… not April
The session spans were seeded April–March, copied from the other deployment. This
one rolls in AUGUST: getCurrentAcademicYear() in bot/shared/routes/
attendance-setup-endpoint.js returns `${y}-${y+1}` when getMonth() >= 7. It is the
only definition of an academic year in this repo, and the seed disagreed with it.
Not cosmetic. The legacy roster mirror files student_lists.academic_year =
sessionCode, and student_lists is unique on (user_id, LOWER(class_name),
academic_year) WHERE is_active. A year's disagreement means the mirror is filed
under a different year than its class, the unique index stops matching, and the
adoption path inserts duplicates instead of adopting. A class created in, say, May
would have been filed a full year off, and nothing would have said so.
Spans are now 1 Aug – 31 Jul for 2025-2026, 2026-2027 and 2027-2028.
New test pins the two answers together rather than trusting either: it parses the
spans out of the seed SQL and asserts the date predicate and
getCurrentAcademicYear() name the SAME session, sampled on both sides of the
rollover (2026-07-31 → 2025-2026, 2026-08-01 → 2026-2027, and April deliberately
mid-year rather than a new one). It also asserts the spans are non-overlapping and
contiguous, so no date falls in a gap, and that the months are 08-01/07-31 —
stated as its own assertion because April is the mistake that was actually made.
Mutation-checked by restoring the April spans: 7 of the 17 tests go red.
TESTS 17 new (188 across the model), all green. Suite delta unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d a new one
The teacher-facing surface for the classes model. CLASSES → ADD → SUBJECTS →
SAVED, where CLASSES doubles as "view my classes" and its footer walks into the
add path.
EVERY STRING COMES FROM THE ENDPOINT, NOT THE FLOW JSON. The existing Flows here
hardcode English copy, which is fine for a single-language deployment and wrong
for this one: a Flow asset is per-WABA and cannot be re-rendered per teacher, so
an Urdu-preferring teacher only ever sees Urdu if the endpoint sends it. Every
heading, label, helper and footer is `${data.*}`, resolved through the catalog for
that teacher. 18 new catalog keys, all inside the caps that bind (footer 35,
button 20, header 60).
THREE DEAD ENDS DELIBERATELY CLOSED, each one a pattern that has already cost this
deployment or the other:
- INIT only ever answers CLASSES. Meta refuses to OPEN a Flow on a screen with
incoming routes, and the branch most likely to break that rule is the
"graceful" empty-state one — which is exactly how a kindness became the only
hard failure last time. A test asserts the returned screen is one the routing
model has no edges into, rather than just checking the string.
- An empty class list is a sentence plus the add button, never an error.
- A teacher with no school on file (roughly one in eight) gets a CHAT message
naming the fix, because classes.school_id is NOT NULL and opening a Flow that
cannot succeed wastes her taps. The check sits in the trigger, not the
endpoint.
Also: an expired in-flight choice (30 min TTL) returns her to the ADD form rather
than a mid-flow screen with no context, and the subject selection is normalized
from array / JSON-string / bare-string payloads with unknown codes dropped, so a
stale published asset cannot inject a subject the table has never heard of.
WIRING docs/flows/class-manager-flow.json · class-manager-endpoint.js ·
/api/flows/class-manager · CLASS_MANAGER_FLOW_ID in constants + .env.template ·
FLOW_CONFIGS entry · a screen-contract pair · /classes (and "my classes" /
"add a class") in the text handler.
NOT DONE: the Flow is not published to any WABA, so nothing is live. Editing an
existing class is also not in this pass — view and add are, which is what was
asked for; edit is worth its own screens.
TESTS 29 new (217 across the model), all green, including an assertion that every
screen the endpoint can return is declared by the Flow JSON.
Fixed a stale tripwire while here: register-all-flows asserted 13 flow configs
against an actual 14 and had been red on develop before this branch. Now 15 and
green, plus a uniqueness check on name/envVar/endpointPath that cannot go stale.
Verified by set-diffing the failing-suite list against develop: one suite
improved, zero regressions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…al API
A "My Classes" page: the classes this teacher is assigned to, and a form to add
one. Teacher-owned classes only — a principal's or coach's view of a school's
classes is deliberately out of scope for this pass.
BOTH ROUTES PROXY TO THE BOT rather than touching the class tables from the
dashboard process. Requiring the bot's ClassService directly was the other option
and is a known trap here: that require throws when a bot module reaches a bot-only
dependency, and the throw gets swallowed — the same shape as the render route that
wrote 21 orphan rows in two days while answering "queued: true".
Two things that follow from proxying, both deliberate:
- ONE WRITER. createClass() also writes the legacy student_lists mirror and
adopts a colliding roster. A second implementation of that in the portal is
exactly how the training rules rotted while this file's own comments claimed
parity.
- ONE COPY CATALOG. Grade and subject labels resolve in the bot process, so the
API hands the page finished, already-localised labels. The page keeps no
vocabulary of its own — which matters, because the seed already had to
reconcile five competing spellings and a sixth would be the portal's.
NEW POST /api/internal/classes/{list,options,create} (key-guarded) ·
GET + POST /api/portal/classes (session-scoped) · PortalClasses.tsx ·
/portal/classes route · a "My Classes" nav item.
The teacher id comes from the SESSION, never the request body, so one teacher
cannot add a class against another's account.
FAILURES ARE DISTINCT, NOT COLLAPSED. 422 no-school, 409 class-teacher-taken, 503
no-session and 400 unknown-grade each carry a sentence naming what would fix it,
and the page shows the API's wording verbatim. Collapsing them into one "failed"
is how a teacher creates the same class three times. On a 409 the class WAS
created and only the role claim refused, so the page reloads rather than implying
nothing happened.
The add form is hidden entirely when the account cannot satisfy it — no school on
file, roughly one teacher in eight — with a sentence instead. A form that always
fails is worse than an explanation.
The new nav item lands in the mobile overflow tray, not the four-item primary bar,
so no labels crop.
TESTS 8 new portal tests (225 across the model). tsc --noEmit clean. One test
feeds the API Urdu labels and asserts they render verbatim, so the page hardcoding
English would go red.
Fixed en route: five new ❌ logs did not pass level='error', which the
logger-level ratchet caught — a ❌ at info level is a bug that never reaches the
dashboard's error filter. Verified across three full runs that the failing-suite
set is a strict subset of develop's: zero regressions, one improvement. A
one-off appearance of tests/cache/language-writer.test.js was order-dependent
flakiness — it passes in isolation and did not recur.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…built yet
docs/classes-model.md, linked from the root navigation table.
Written because the interesting half of this work is the half that is not built.
The tables landed; promotion, rollover and the feature migrations did not, and the
reasoning behind the shape is the thing that decays first.
WHAT IT RECORDS
- why a class became an entity: the roster model was unused (every live quiz
carried no roster link, most student rows belonged to no list) while the
institutional spine it should have hung off was already fully populated
- the three decisions worth not re-litigating: the role sits on the
(class, teacher) pair rather than the subject row; enrollment is a row rather
than a pointer; the reference tables hold no display copy
- the two opposite failure directions and why they differ — the vocabulary
resolver fails CLOSED because a wrong grade picks the wrong reading passage,
while the class list fails to an EMPTY LIST because an error screen is a dead
end and an empty one is not
WHAT IT PLANS
- promotion as close+open of an enrollment, with retention as the same operation
pointed at a same-grade class in the next session — no flag, no special case
- the four ordering constraints when it is built: target classes must exist
first, it is a batch op with per-student exceptions, it must be idempotent and
reversible because someone will run it twice and someone will run it early,
and teaching-assignment roll-forward is a separate question
- the mixed annual/semester question, named as unresolved rather than guessed:
the likely answer is that the cycle is a property of the school, which is a
schema change that should wait for a second cycle to actually exist
- the migration order for the features, where each step removes a reason for the
mirror to exist, and grades_taught becomes derivable rather than hand-typed
It also names the scaffolding to delete together (the mirror write, the bridge
column, mirrorLabel) and seven things deliberately not done — including the one
worth knowing before anyone promises a teacher lesson plans for a class outside the
corpus's grade × subject coverage.
Verified across a full run: failing-suite set is a strict subset of develop's —
zero regressions, one improvement. The link-integrity and source-hygiene failures
this touches were red before this branch and my additions are not in either
offender list.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…apply it
Applied V1.1.3 to the STAGING database and it failed:
0A000 EXECUTE of transaction commands is not implemented
`npm run migrate` goes through the `exec_sql` RPC, whose body is `EXECUTE query`.
Postgres cannot EXECUTE a transaction command, so any migration containing `BEGIN;`
raises 0A000 and never applies. The file followed house style and house style does
not work through the repo's own runner.
Removing the wrapper loses nothing: a plpgsql function body is itself a single
transaction, so the batch still commits or rolls back together. Applying by hand
(psql / SQL editor) runs in autocommit, which the header now says.
THIS AFFECTS SIX OTHERS. V1.0.8, V1.0.9, V1.0.10, V1.1.0, V1.1.1 and V1.1.2 all
contain BEGIN;, so none can go through the runner as written — the likely reason
`schema_versions` lags well behind the files on disk and those changes were applied
by hand. Documented in docs/classes-model.md; fixing them is not this change's job.
STAGING IS MIGRATED. Confirmed a separate Supabase project from production before
writing anything — the runner script asserts the staging ref and hard-stops on
production's, and prod was never contacted. All 7 tables present, 1.1.3 recorded,
13 grades / 6 subjects / 3 sessions seeded with the August–July spans, and the
student_lists.class_id bridge in place.
Then verified the constraints actually BITE against that database rather than
trusting the DDL — 14 checks, all passing, every created row cleaned up:
- unknown grade_code / session_code refused (FKs are real)
- lower-case and padded sections refused (the CHECK is applied)
- duplicate (school, grade, section, session) refused
- the SAME class in the NEXT session allowed — this is what makes rollover work
- a second prime-responsible class teacher refused (the partial unique index is
valid, not merely defined), while a second SUBJECT teacher is still allowed
- a subject outside the seeded set refused; a seeded one accepted
- inverted session span and unknown session kind both refused
One trap worth recording: the post-migration table checks reported 404 while the
seed reads in the same script succeeded. That is PostgREST's schema cache
reloading asynchronously after the NOTIFY, not a partial apply — a re-check once
warm shows all seven present.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…green The sprint-0 security scan forbids references to the SQL-exec RPC anywhere under bot/, dashboard/ or infrastructure/ — it skips `//`, `*` and `#` comment lines, but not SQL's `--`, so the explanatory header I added tripped it. Reworded to describe the mechanism without naming the identifier. The actionable part is kept: the error code and the reason a BEGIN-wrapped file cannot apply through the runner. The full detail stays in docs/classes-model.md, which that scan does not cover. Rewording rather than adding an exemption: the guard exists because that RPC executes arbitrary SQL, and widening it for a comment would make the next widening easier to argue for. Suite delta re-verified: 31 failing suites against develop's 32 — one improvement (register-all-flows), none added. Two single-run appearances during this work were order-dependent flakiness, both passing in isolation and not recurring (voice-language-floor, and language-writer earlier); the security-scan hit was real and is what this fixes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
A class stops being a side effect of the attendance feature and becomes an entity: it belongs to a school, sits for a session, is at one grade, and is taught by one-or-more teachers across one-or-more subjects, at most one of whom is the prime-responsible class teacher. Students are enrolled into it rather than being rows inside it.
V1.1.3__classes_model.sqlis committed and unapplied. Nothing in this branch has touched a live database. Two things follow:CLASS_MANAGER_FLOW_IDis unset and the tables do not exist, so/classesanswers "not available yet" and/portal/classeswill 502. The code is inert, not broken..envhere resolves to a single project ref, so "apply it on staging" and "apply it on production" may be the same act. That needs answering before the migration runs anywhere.Why
Measured on production before this branch:
student_listsschoolsstudentslist_id)school_idattendance_sessionsleader_teachersquizzeslist_idleader_schoolsThe roster model was unused: every live quiz routed around it. Meanwhile the institutional spine it should have hung off already existed and was fully populated. Class identity had degraded into a string — those 21 rostered-nowhere students carry 18 distinct spellings of about three classes, including a superscript digit.
What landed
7 tables (mirrored into bootstrap schema, RLS and seed):
academic_sessions,grade_levels,subjects,classes,class_teachers,class_teacher_subjects,class_enrollments, plus a temporarystudent_lists.class_idbridge.Services: a pure free-text label parser, a cached vocabulary resolver, and
ClassService(create / assignTeacher / enroll / list) with mirror-and-adopt.WhatsApp: a Class Manager Flow — CLASSES → ADD → SUBJECTS → SAVED — whose every string comes from the endpoint, not the Flow JSON, because a Flow asset is per-WABA and an Urdu-preferring teacher only sees Urdu if the endpoint sends it.
Portal: a teacher-only My Classes page, proxied to the bot's internal API so there is one writer and one copy catalog.
Docs:
docs/classes-model.md— the design record plus the promotion/rollover plan, which is deliberately not built.Decisions worth reviewing
repeated_yearflag.ux-stringskeyed by these codes, with a test asserting the key sets equal the seeded codes.Bugs the tests caught while writing this
'0'silently resolved toearly_years— a teacher typing a bare zero got a KG class.academic_year = sessionCodeandstudent_listsis unique on(user_id, LOWER(class_name), academic_year), a class created in spring would have been filed a year off and the adoption path would have inserted duplicates. A test now pins the seed against the one existing academic-year function.❌logs did not passlevel='error', so genuine failures would never have reached the dashboard's error filter.Tests
225 new, all green. Failing-suite set verified as a strict subset of
develop's across three full runs: zero regressions, one improvement (register-all-flowsasserted 13 flow configs against an actual 14 and was already red; now 15 and green, plus a uniqueness check that cannot go stale).Mutation-tested rather than assumed: disabling mirror adoption, removing the class-teacher guard, dropping canonical band ordering, and restoring the April spans each turn the expected tests red.
Pre-existing and not absorbed into this branch: the baseline snapshot reports REGRESSION on clean
develop(785 offender lines, 31 red suites from merged pic-to-lp/video/quiz work), and the language audit shows new debt in 9 files, none mine. Both want a deliberate re-baseline by whoever owns that debt.Not in this pass
Editing a class; principal/coach views; any feature migrated onto
class_id(attendance, quizzes and reading all still readstudent_lists, untouched); publishing the Flow to any WABA.🤖 Generated with Claude Code