Conversation
POST /v1/onboarding/school creates or selects a school, promotes the authenticated user to SchoolAdmin, binds them, and sets the school to PENDING for admin review. Creates an event visible in the admin UI.
Tests cover: new school creation, existing school selection, active school rejection, missing fields, unauthenticated access, event creation, and user type promotion to SchoolAdmin.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a26e2459e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
app/api/onboarding.py
Outdated
|
|
||
| subclass_table = type_table_map.get(user.type) | ||
| if subclass_table: | ||
| from sqlalchemy import text |
There was a problem hiding this comment.
text import before unconditional SQL statements
_promote_to_school_admin imports text only inside if subclass_table, but text(...) is used unconditionally for the UPDATE users and subsequent inserts. When user.type is not in type_table_map (for example UserAccountType.WRIVETED), that branch is skipped and this path raises UnboundLocalError, causing /onboarding/school to fail with a 500 instead of completing onboarding.
Useful? React with 👍 / 👎.
app/api/onboarding.py
Outdated
| UserAccountType.PUBLIC: "public_readers", | ||
| UserAccountType.STUDENT: "students", | ||
| UserAccountType.EDUCATOR: "educators", | ||
| UserAccountType.PARENT: "parents", |
There was a problem hiding this comment.
Mapping UserAccountType.PARENT into the raw delete flow causes _promote_to_school_admin to execute DELETE FROM parents WHERE id = :uid; however Reader.parent_id (app/models/reader.py) references parents.id without ondelete, so parents with existing child readers will hit a foreign key violation and the onboarding request fails with an IntegrityError/500. This makes promotion unreliable for valid parent accounts unless child links are handled first or parent onboarding is explicitly blocked.
Useful? React with 👍 / 👎.
- Move sqlalchemy text import to top of function (P2 fix) - Exclude Parent type from raw subclass deletion to avoid FK cascade failure on readers table. Parents get a 409 with contact-support message. - Fix test to use fresh session for promotion verification
POST /v1/onboarding/family promotes PUBLIC user to PARENT and creates child reader accounts. Tests cover: basic flow, type promotion, and unauthenticated access.
hardbyte
force-pushed
the
feat/school-onboarding
branch
from
dfcc006 to
b2855eb
Compare
- Flow fixture: collects parent name, child name/age/reading level via conversational chat interface - Internal handler for /v1/onboarding/family: creates child reader profiles from chatflow api_call actions - Register flow in seed config
Critical: - Block WRIVETED/EDUCATOR/PARENT types from promotion (allowlist approach) - Check for existing admin when selecting existing school (prevent IDOR) High: - Cap children list at 10 in both endpoint and internal handler - Validate/truncate all string inputs in internal handler Medium: - Use SchoolLocationInput model instead of raw dict for location - Add country_code format validation (3-char constraint) - Add Field constraints on all string/int inputs - Sanitize event descriptions (no user-supplied data in description)
1 participant
Summary
New
POST /v1/onboarding/schoolendpoint for self-service school signup, replacing the Airtable form.What it does
infoJSONB underonboardingkeyAdmin approval flow
Test plan