Persist quick-start onboarding state in the API#297
Conversation
Summary: - add user onboarding JSONB state plus a migration and shared payload helpers for the authenticated quick-start contract - expose backend-owned quick-start media config on `/auth/me` and `/v1/me`, and add `/account/onboarding` to persist state updates - cover the new response shape and persisted state writes in backend route tests Rationale: - onboarding state has to live server-side to stay consistent across sessions and devices instead of depending on browser-local storage - the embed target needs a backend-owned config path so the video can be changed without coupling every change to a dashboard redeploy - normalizing and JSON-encoding the payload before storage preserves compatibility with legacy local state while avoiding JSONB datetime serialization failures Tests: - `source venv/bin/activate && pytest tests/test_account_routes.py -v` - `source venv/bin/activate && pytest tests/test_auth_routes.py -v` - `source venv/bin/activate && ruff check ace_platform/ tests/` - `source venv/bin/activate && ruff format ace_platform/ tests/` - `source venv/bin/activate && pytest tests/ -v` Co-authored-by: Codex <codex@openai.com>
|
@codex review |
![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: b27e3c934e
ℹ️ 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".
| if status == "dismissed": | ||
| normalized["status"] = "minimized" | ||
| normalized.setdefault("minimized_at", normalized.get("dismissed_at")) |
There was a problem hiding this comment.
The legacy conversion path sets status from dismissed to minimized, but it keeps dismissed_at in the payload; because QuickStartOnboardingState has extra="forbid", validation then fails and the except path silently resets the user to the default active state. Any user record carrying legacy dismissed fields will lose its intended minimized state and see onboarding reopen.
Useful? React with 👍 / 👎.
| normalized_state = normalize_quick_start_state(body.model_dump(exclude_none=True)) | ||
| current_user.onboarding_state = normalized_state.model_dump(mode="json", exclude_none=True) |
There was a problem hiding this comment.
This PATCH handler replaces current_user.onboarding_state with a dump of only the request model, but the request model gives status a default of active; when a client sends a partial update (for example only last_seen_at, which the schema allows), the dump can overwrite prior minimized/completed state and drop stored timestamps. This causes unintended state regressions even though the endpoint is declared as a partial update.
Useful? React with 👍 / 👎.
1 participant
Summary
users.onboarding_statewith a migration and normalized API payload helpers/auth/meand/v1/me, and addPATCH /account/onboardingfor authenticated state updatesCoordinated change
Test plan
source venv/bin/activate && pytest tests/test_account_routes.py -vsource venv/bin/activate && pytest tests/test_auth_routes.py -vsource venv/bin/activate && ruff check ace_platform/ tests/source venv/bin/activate && ruff format ace_platform/ tests/source venv/bin/activate && pytest tests/ -vPATCH /account/onboardingandGET /v1/mereturned matching minimized state plus backend-owned embed config