feat(restapi): add Pydantic models for top REST responses (C3) — Cata…#153
Merged
Conversation
…log vertical slice
Lays the foundation for typed REST responses (C3 in CODE_REVIEW.md), with
Catalog as the first end-to-end migration. Establishes the pattern for
follow-up entity migrations.
Foundation
- restapi/types/{__init__.py, base.py} — `RestModel` base mirroring
gql/types/GqlModel: snake_case fields, to_camel aliasing, populate_by_name=True,
extra="ignore" so unknown server fields are dropped silently.
- restapi/types/{catalog,category,product,user,role,order}.py — conservative
hand-written models covering only the fields the test suite actually touches.
Field set sourced from grep across tests/restapi/. Future direction
(out of scope here): codegen from the platform's OpenAPI schema.
Catalog vertical slice (the demo)
- restapi/operations/catalog_operations.py — `create()` and `update()` now
return `Catalog` (model_validate on the wire response). `search()` still
returns dict (response shape needs a separate SearchResult model — defer).
`update()` accepts the typed model and dumps via model_dump(by_alias=True).
- tests/restapi/catalog/conftest.py — `make_catalog` factory now returns
`Catalog`. `make_category` accepts either `catalog: Catalog` or
`catalog_id: str` (the latter for cases where you only have an id).
- tests/restapi/catalog/test_catalog.py — full sweep from `catalog["id"]`
to `catalog.id`, etc.
- tests/restapi/catalog/test_category.py — fixed test_category_nested to
pass `catalog_id=` instead of the previous `catalog={"id": ...}` hack.
- tests/restapi/catalog/test_product.py — `target_catalog["id"]` →
`target_catalog.id` in the move_to_catalog test.
Verified
- 30/30 catalog tests pass on vcptcore-demo
- pytest --collect-only clean
- pyright errors on catalog_operations.py: 3 → 1 (the remaining one is
search() returning dict, deferred)
Deferred to follow-up PRs (one per entity, mechanical pattern repeat)
- Category, Product, User, Role, Order operation+consumer migrations.
Models are already in place; each follow-up just updates the operation
to model_validate() and sweeps test consumers to attribute access.
- The 5 remaining entities will lift the bulk of the 129 reportReturnType
pyright errors surfaced by C2 in the previous PR.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…er, Role, User Extends the Catalog vertical slice (commit bfffa4f) to all 6 review-recommended entities. The original PR scoped down to Catalog only; this push expands to the full plan per user direction ("review all remain test"). Foundation tweak - restapi/types/base.py: switch RestModel to extra="allow" so server fields outside the typed schema round-trip through model_dump() — needed for update() flows that re-send the full server response with edits. Tightening to extra="forbid" (S21) is a separate follow-up. - restapi/types/category.py: add `parent_id` field (used by test_category_nested). Operations migrated (return typed models for the natural read paths) - category_operations.py: create/get_by_id/update return Category. - product_operations.py: create/get_by_ids/get_by_id/update/create_or_update_with_body return Product. get_clone() left as dict (it returns a partially-built template). - order_operations.py: create/get_by_id/get_by_number return CustomerOrder. update/recalculate kept dict-friendly because the order test flow round-trips the response with deep edits (items[0].quantity, inPayments, shipments) where forcing model_dump on every step is more friction than value. - role_operations.py: create/get_by_name return Role. update accepts Role. - user_operations.py: get_by_name returns User. create/delete return their status dicts ({succeeded, errors}) — that's the platform's contract for those endpoints, not a User resource. update accepts User. Test consumer sweeps - tests/restapi/catalog/{conftest,test_catalog,test_category,test_product,test_assets}.py: attribute access throughout. make_category accepts catalog_id directly (cleaner than the previous {"id": ...} dict hack at test_category_nested). Server-only fields (weight, images, assets) read via reloaded.model_extra. - tests/restapi/orders/{conftest,test_orders}.py: attribute access for read paths; mutation flows use order.model_dump(by_alias=True) before edits. test_order_get_not_found now also catches Pydantic ValidationError (the platform returns 200 + null body for missing id, which fails model_validate). - tests/restapi/platform/{conftest,test_user,test_user_lock,test_user_password, test_api_key,test_roles}.py: full_user, reloaded, fetched all switch to attribute access (.id, .user_name, .email, .user_type for User; .id, .name for Role). make_user keeps returning dict because the create endpoint returns a status dict (not a user); credentials remain dict-injected. Verified - pytest --collect-only on tests/restapi/: 295 tests collected cleanly - Live run on vcptcore-demo: 286 passed, 5 failed - The 5 failures are the pre-existing personalization async-index issue (tracked in project memory; same as previous baseline) - The 2 transient order failures from the previous baseline now pass - No new regressions Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Continues the typed-response migration to the 9 remaining factory-backed
entities, completing the bulk of the C3 work. Pyright reportReturnType
errors should drop substantially after this lands.
New models (restapi/types/)
- contact.py — Contact (id, first_name, last_name, name, member_type)
- organization.py — Organization (id, name, member_type)
- employee.py — Employee (same shape as Contact)
- member.py — Member (id, name, member_type) — generic /api/members
- vendor.py — Vendor (id, name, member_type)
- pricelist.py — Pricelist + PricelistAssignment
- promotion.py — Promotion (id, name, is_active)
- store.py — Store (id, name, catalog, store_state)
- __init__.py — re-exports updated
Operations migrated (read paths return typed models, update returns None)
- contact_operations.py, organization_operations.py, employee_operations.py,
member_operations.py, vendor_operations.py: create/get_by_id/get_by_ids
return typed models; update returns None (PUT returns 204).
bulk variants typed where the server returns the entity list.
- pricelist_operations.py, pricelist_assignment_operations.py: same pattern.
search() and get_new() kept dict (search shape varies, new is a template).
- promotion_operations.py: same. get_new() kept dict (template).
- store_operations.py: same; get_all() returns list[Store]; get_accessible
kept list[dict] (response shape unclear).
Factory fixtures
- contacts/conftest.py — make_contact, make_organization, make_employee,
make_member return typed models.
- pricing/conftest.py — make_pricelist, make_assignment return typed models.
make_assignment now takes pricelist: Pricelist (was dict).
- marketing/conftest.py — make_promotion returns Promotion.
- store/conftest.py — make_store returns Store.
Test consumer sweeps (~13 test files)
- tests/restapi/contacts/{test_contact,test_organization,test_employee,
test_member}.py: attribute access throughout. Bulk update flows use
model_dump(by_alias=True) before mutating. test_contact_get_not_found
and test_organization_cycle catch ValidationError for null bodies.
- tests/restapi/pricing/{test_pricelist,test_assignments,test_prices}.py:
same.
- tests/restapi/marketing/test_promotions.py: same. Server-only fields
(description) read via reloaded.model_extra.
- tests/restapi/store/test_store.py: same.
Verified
- pytest --collect-only on tests/restapi/: 295 tests collected
- Live run on vcptcore-demo: 286 passed, 5 failed
- 5 failures are the pre-existing personalization async-index issue
(same baseline as before this PR's earlier commits)
- Zero new regressions
Tier 2 (settings, notifications, oauth, api_key, content/cms, dynamic
content sub-types) and Tier 3 (health checks, dashboards) intentionally
not migrated — low typing payoff per file.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
REST API Test Results295 tests 292 ✅ 26s ⏱️ Results for commit 5ed191b. ♻️ This comment has been updated with latest results. |
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.
…log vertical slice
Lays the foundation for typed REST responses (C3 in CODE_REVIEW.md), with Catalog as the first end-to-end migration. Establishes the pattern for follow-up entity migrations.
Foundation
RestModelbase mirroring gql/types/GqlModel: snake_case fields, to_camel aliasing, populate_by_name=True, extra="ignore" so unknown server fields are dropped silently.Catalog vertical slice (the demo)
create()andupdate()now returnCatalog(model_validate on the wire response).search()still returns dict (response shape needs a separate SearchResult model — defer).update()accepts the typed model and dumps via model_dump(by_alias=True).make_catalogfactory now returnsCatalog.make_categoryaccepts eithercatalog: Catalogorcatalog_id: str(the latter for cases where you only have an id).catalog["id"]tocatalog.id, etc.catalog_id=instead of the previouscatalog={"id": ...}hack.target_catalog["id"]→target_catalog.idin the move_to_catalog test.Verified
Deferred to follow-up PRs (one per entity, mechanical pattern repeat)