Classes: sections + shifts vocabulary, one teacher per subject, /class alias - #231
Merged
Conversation
…r costs the class TWO BUGS, both found by using it rather than reading it. 1. `/classes` opened the manager and `/class` did not. Rather than appending one more alternative to an inline regex — which is how that gap appeared — the rule now lives in class-command.js with its near misses as tests. The rule: slash forms match as a PREFIX (`/class`, `/classes`, `/classes please`); plain words match only as the WHOLE message (`class`, `my classes`, `add a class`, `show my classes`, plus Urdu جماعت / جماعتیں / jamaat). The whole-message restriction is the load-bearing half. "my class is too noisy" is a teacher talking, and answering it with a form is a hijack — she asked a question and got a spreadsheet. Two collisions the tests pin: the attendance router is checked LATER in the handler, so `attendance` / `roll call` / حاضری must never match here; and "Classroom Management" is a real training course title, so the pattern anchors on the word and never the substring (which also rules out `classy`, `subclass`, `first class`). 2. A teacher joining a class a COLLEAGUE already created, while ticking "I am the class teacher", got NOTHING: no class_teachers row, her chosen subject discarded, the class absent from her list — and a stray legacy mirror row written for a class she was not assigned to. She would reasonably conclude the feature was broken. Found by creating the same class as two different teachers against staging, not by inspection. assignTeacher() returned early on the role conflict, before creating her assignment. A REFUSED ROLE MUST NOT COST HER THE CLASS. The role is now simply declined — she is assigned as a subject teacher, her subjects are kept, the original class teacher stays put — and `classTeacherTaken` tells the caller what to say. The invariant is unchanged: still exactly one class teacher per class, still enforced by the partial unique index. The earlier test asserting `error: 'class_teacher_exists'` encoded the old contract and is updated to assert the new one, keeping the invariant it cared about. TESTS 274 across the model, all green (52 new for the command rule, 5 for the join-a-colleague's-class case). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… subject
Three rules from the operator, and two bugs they exposed.
RULE 1 — SECTIONS ARE A CLOSED SET (A-E). They were the one axis still free text,
which is the same disease grades and subjects were just cured of — except worse.
Section is part of class IDENTITY, so a spelling variance does not merely mislabel:
it SPLITS a class in two, with the roster divided between them. A table rather than
a CHECK because "ask NIETE support to add one" has to be cheap; a CHECK would make
it a migration and a deploy, so the answer would become "no" in practice.
RULE 2 — SHIFT IS A NEW AXIS OF IDENTITY. Morning and evening are different classes
with different students and teachers, so identity is now
(school, grade, section, shift, session). shift_code is NOT NULL defaulting to
morning: a nullable shift would be an "unspecified" third value that merges with
neither.
RULE 3 — ONE TEACHER PER SUBJECT PER CLASS. Two teachers could both be recorded
teaching Maths to 4-A. Enforcing it needed the class ON the subject row, so class_id
is denormalised onto class_teacher_subjects to carry a real unique index — an
application check would let two simultaneous submissions both pass and both insert.
BUG 1 — THE MIRROR MERGED DISTINCT CLASSES. The legacy roster name came from the
GRADE alone ("Grade 9"), and student_lists is unique on
(user_id, LOWER(class_name), academic_year). So one teacher's 4-A and 4-B collided
and the second class silently ADOPTED the first's roster. Shift would have made it
worse. The mirror name now carries section and shift.
Consequence, faced squarely: adoption now matches only rows this code wrote. A
roster the teacher hand-named ("4th grade morning") is left ALONE rather than
claimed on a fuzzy match — claiming it would silently move someone else's
students. Tested both ways.
BUG 2 — A DECLINED CLAIM COST HER THE WORK. Following the class-teacher fix, a
subject a colleague already teaches is now declined INDIVIDUALLY: the free subjects
are still assigned, and `subjectsTaken` names what was not. This deliberately
differs from `unknown_subject`, which stays all-or-nothing — that is a caller bug,
whereas this is a legitimate clash between colleagues. The internal API's 409 is
gone: the class WAS saved, and a 409 lost the work and read as "nothing happened".
SURFACES The section TextInput becomes a Dropdown on both WhatsApp and the portal —
free text would now be REFUSED by the database, so a text box was an error loop, not
a nicety. Shift joins both forms. The helper text names the support route, because a
closed set without a stated escape hatch is a dead end. SAVED and the portal toast
now confirm the save AND name any declined claim, additively.
VERIFIED ON STAGING, not asserted: section F and ALPHA refused, shift "night"
refused, evening 8-D accepted as a distinct class, duplicate morning 8-D refused,
two teachers on one class allowed, teacher B refused Maths but granted Urdu, and the
same subject accepted in the evening class. Every test row cleaned up, no orphans.
TESTS 302 in the model suite + 13 portal, all green. tsc clean. Full run: 31
failing suites against develop's 32 — zero newly failing, one improved.
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.
Follow-up to #230, on the same branch. Three rules from the operator, plus two bugs they exposed and one reported from staging.
The rules
(school, grade, section, shift, session).shift_codeis NOT NULL defaulting tomorning— a nullable shift is an "unspecified" third value that merges with neither.class_idis denormalised ontoclass_teacher_subjectsto carry a real unique index. An application-level check would let two simultaneous submissions both pass.Bugs these exposed
Grade 9), andstudent_listsis unique on(user_id, LOWER(class_name), academic_year)— so one teacher's 4-A and 4-B collided and the second silently adopted the first's roster. Shift would have made it worse. The name now carries section and shift. Consequence faced squarely: adoption now matches only rows this code wrote; a hand-named roster is left alone rather than claimed on a fuzzy match, because claiming it would move someone else's students.subjectsTakennames what wasn't. The internal API's 409 is gone: the class was saved, and a 409 lost the work and read as "nothing happened".Also fixed
/classesworked and/classdidn't. The rule now lives inclass-command.jswith its near misses as tests: slash forms match as a prefix, plain words only as the whole message — so "my class is too noisy" still reaches the assistant instead of opening a form,attendance/roll call/حاضری stay with the attendance router, and "Classroom Management" (a real course title) doesn't hijack.Surfaces
The section TextInput becomes a Dropdown on both WhatsApp and the portal — free text is now refused by the database, so a text box was an error loop rather than a nicety. Shift joins both forms. Helper text names the support route, because a closed set without a stated escape hatch is a dead end.
Verified on staging, not asserted
V1.1.4applied to the staging database (separate Supabase project; the runner asserts the staging ref and hard-stops on production's). Then, against that live database:F/ALPHAnightEvery test row cleaned up; zero orphan subject rows.
Tests
302 in the model suite + 13 portal, all green;
tsc --noEmitclean. Full run: 31 failing suites against develop's 32 — zero newly failing, one improved.Sequencing note
The Flow JSON changed, so it needs a re-upload and republish after this deploys —
register-all-flowsskips flows that already exist, so the registrar alone won't pick it up. Doing that after the deploy lands, since Meta probes the endpoint on publish.🤖 Generated with Claude Code