Skip to content

fix(registration): a name on the row means registered — and a SUCCESS screen says so - #686

Merged
hyasin270 merged 2 commits into
developfrom
bd-oak77.6-91071
Sep 6, 2026
Merged

fix(registration): a name on the row means registered — and a SUCCESS screen says so#686
hyasin270 merged 2 commits into
developfrom
bd-oak77.6-91071

Conversation

@hyasin270

Copy link
Copy Markdown
Contributor

The bug

The "is this teacher registered?" gate on develop keys on registration_completed. On production that flag is not the truth. Counted read-only on NIETE prod ihzciabopbttygxxgrkm, 2026-09-06:

count
users with a first_name 7,685 (212 of them '')
of those, registration_completed = true 440
of those, flag false (0 NULL) 7,245
…messaged the bot in the last 30 days 4,145 — 3,951 teachers, 105 principals, 61 coaches

registration_state is no fallback: it reads 'unregistered' for 7,684 of those 7,685 rows, including 439 of the 440 whose flag is true. Exactly one row on the whole database says 'completed'.

Staging never showed this because staging is a different database whose users all registered through the current Flow.

Two independent faults:

  1. A backfill gap — the July NIETE roster migration set names and never set the flag. The earliest rc = true row was created 2026-07-11.
  2. A live gap — the last-7-day completion rate is 76.7% (23 of 30 named new users carry the flag), so a path in use today still finishes without it.

The fix

Reader — bot/shared/utils/registration-status.js (new), one definition: registered = a completed run OR a name we already know. Both gates in text-message.handler.js read it:

  • :1777 — the /register copy branch (was showing "Welcome" instead of "Update your details").
  • :2275 — the "register" keyword branch. This is the harmful one: falling through it reaches FeatureRegistrationService.sendNameQuestion and re-asks the teacher for her name. That is the path 4,145 active users would have taken.

An empty-string first_name stays falsy, so the 212 '' rows are still unregistered — matching what the bot does today.

Writer — bot/shared/routes/registration-endpoint.js: handleRegistrationDataExchange returns screen: 'SUCCESS' from two places (the direct path and the org="other" path) and tells the teacher "Your registration is complete." Neither wrote the flag. It was set only when the terminal Flow payload later reached flow-response.handler.js:574 — the very payload this file's own header documents as unreliable ("the terminal Flow payload arrives with the earlier screens' values empty… verified in the logs 2026-09-03"), which is why bd-2480 already moved the name write per-screen. The completion flag was never moved with it. Both SUCCESS branches now call markRegistrationComplete(). flow-response.handler.js still writes it as well; a second write of true is a no-op.

Tests — red-first, and they execute the changed lines

tests/registration/bd-oak77-6-*.test.js, in the root tree so npm test actually gates them (bot/tests/ is a separate runner CI does not gate on). They drive the real handleTextMessage and the real endpoint.

Before the change: 5 of 9 failed — "already registered" never sent, sendNameQuestion called once, /register header "Welcome", and both SUCCESS branches wrote no flag. After: 9/9. The other 4 are the controls that stop the predicate widening too far (empty-string name, no name at all, flag-only-no-name, non-terminal screen).

bot/tests/registration/bd-2480-register-gate.test.js pinned the old contract with its own inline copy of the predicate — green the whole time the shipped handler was wrong on prod. It now requires the real helper.

Gate

Two full root runs on this branch against a fresh worktree of origin/develop aeb9862: no suite fails here that does not also fail on develop (run 2 is one suite better — the quarantined portal-capstone-submit flake did not fire). The repo gate reports no new failing suites and no new offenders; source-hygiene offenders 888 → 886. Run 1 showed tests/cache/language-writer.test.js, which passes 15/15 in isolation on both trees and does not recur — a parallel-run flake, not this change.

Staging checked before merge: niete_lp612_renders status='authoring' = 0, queued = 0.

Not in this PR

BACKFILL.sql (with its reversal) is written to prod_golive_2026-09-06/00_vehicle/ and is not run — it is a write to a pre-existing prod table and needs the operator's go. The code fix stands on its own without it; the backfill additionally repairs the coach portal's teacher list (dashboard/routes/hcp.routes.js filters .eq('registration_completed', true) and returns 440 of 7,685 today — that filter is identical on main, so it is a live defect rather than something this promotion introduces) and every registration/adoption metric.

Refs: bd-oak77.6, bd-oak77

Haroon Yasin added 2 commits September 6, 2026 15:53
… screen says so

Two faults, one symptom. On NIETE production 7,685 users carry a first_name and
only 440 carry registration_completed; 7,245 have it false (none NULL), and
4,145 of those messaged the bot in the last 30 days — 3,951 of them teachers.
Keying "is she registered?" on that flag alone sends every one of them back into
registration on a number where they are already registered and using lesson
plans. It never showed on staging because staging is a different database whose
users all registered through the current Flow.

READER. `bot/shared/utils/registration-status.js` is now the one definition:
a completed run OR a name we already know. Both gates in text-message.handler.js
read it — the /register copy branch and the "register" keyword branch, the second
of which falls through to sendNameQuestion and is what would have re-asked 4,145
teachers for their names. registration_state is kept in the union but is not a
fallback: on prod it reads 'unregistered' for 7,684 of those 7,685 rows,
including 439 of the 440 whose flag is true. An empty-string first_name stays
falsy — 212 prod rows carry one.

WRITER. handleRegistrationDataExchange returns screen SUCCESS from two places
and tells the teacher "Your registration is complete." Neither wrote the flag;
it was set only when the terminal Flow payload later reached
flow-response.handler.js — the very payload this file's own header documents as
unreliable, and the reason the NAME write was already moved per-screen. The
completion flag was never moved with it, which is why the last-7-day completion
rate is 76.7% and not 100%. Both SUCCESS branches now mark the account complete.
flow-response.handler.js still writes it too; a second write of true is a no-op.

TESTS. tests/registration/bd-oak77-6-*.test.js — in the ROOT tree, so the merge
gate actually runs them (bot/tests/ is a separate runner CI does not gate on).
They drive the real handleTextMessage and the real endpoint, so they execute the
changed lines; 5 of the 9 failed before this change and pass after, and the other
4 are the controls that stop the predicate widening too far (empty-string name,
no name, flag-only, non-terminal screen).

bd-2480-register-gate.test.js pinned the old contract with its own INLINE COPY of
the predicate — green while the shipped handler was wrong. It now requires the
real helper.

Gate: root suite, no new failing suites and no new offenders vs develop;
source-hygiene offenders 888 -> 886.

Refs: bd-oak77.6, bd-oak77
@hyasin270
hyasin270 merged commit 59d4675 into develop Sep 6, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant