From fbeac890be1536b0ec163912a402425df0cb0456 Mon Sep 17 00:00:00 2001 From: greebo Date: Thu, 19 Mar 2026 19:11:33 +0300 Subject: [PATCH] feat(backend): harden pricing mutation contract and sync backend docs - add typed response schemas for pricing write endpoints - add stale draft version guard for pricing mutations - unify pricing API contract around expected_scheme_version_id - update API route map - add smoke regression checklist for backend routes and artifact flows --- backend/app/api/routes/pricing.py | 225 +++++++++++++--------------- backend/app/schemas/pricing.py | 113 ++++++-------- backend/app/services/draft_guard.py | 51 ++++--- backend/docs/api-map.md | 15 +- backend/docs/smoke-regression.md | 8 +- 5 files changed, 197 insertions(+), 215 deletions(-) diff --git a/backend/app/api/routes/pricing.py b/backend/app/api/routes/pricing.py index 60f9816..a18d2c6 100644 --- a/backend/app/api/routes/pricing.py +++ b/backend/app/api/routes/pricing.py @@ -15,32 +15,35 @@ from app.repositories.pricing import ( update_pricing_category, ) from app.repositories.scheme_version_pricing import replace_scheme_version_pricing_snapshot -from app.repositories.schemes import get_scheme_record_by_scheme_id from app.schemas.pricing import ( + DeleteResponse, PriceRuleCreateRequest, + PriceRuleCreateResponse, PriceRuleItem, PriceRuleUpdateRequest, + PriceRuleUpdateResponse, PricingBundleResponse, PricingCategoryCreateRequest, + PricingCategoryCreateResponse, PricingCategoryItem, PricingCategoryUpdateRequest, + PricingCategoryUpdateResponse, ) from app.security.auth import require_api_key -from app.services.draft_guard import get_current_draft_context +from app.services.draft_guard import validate_expected_draft_version_if_provided router = APIRouter() -async def _refresh_current_draft_snapshot_if_possible( - *, - scheme_id: str, - expected_scheme_version_id: str | None = None, -) -> dict | None: - scheme, version = await get_current_draft_context( +async def _refresh_current_draft_snapshot_if_possible(scheme_id: str) -> dict | None: + context = await validate_expected_draft_version_if_provided( scheme_id=scheme_id, - expected_scheme_version_id=expected_scheme_version_id, + expected_scheme_version_id=None, ) + if context is None: + return None + scheme, version = context return await replace_scheme_version_pricing_snapshot( scheme_id=scheme.scheme_id, scheme_version_id=version.scheme_version_id, @@ -79,51 +82,48 @@ async def get_pricing_bundle(scheme_id: str, role: str = Depends(require_api_key ) -@router.post(f"{settings.api_v1_prefix}/schemes/{{scheme_id}}/pricing/categories") +@router.post( + f"{settings.api_v1_prefix}/schemes/{{scheme_id}}/pricing/categories", + response_model=PricingCategoryCreateResponse, +) async def create_pricing_category_endpoint( scheme_id: str, payload: PricingCategoryCreateRequest, expected_scheme_version_id: str | None = Query(default=None), role: str = Depends(require_api_key), ): - scheme, version = await get_current_draft_context( + await validate_expected_draft_version_if_provided( scheme_id=scheme_id, expected_scheme_version_id=expected_scheme_version_id, ) pricing_category_id = await create_pricing_category( - scheme_id=scheme.scheme_id, + scheme_id=scheme_id, name=payload.name, code=payload.code, ) - snapshot = await _refresh_current_draft_snapshot_if_possible( - scheme_id=scheme.scheme_id, - expected_scheme_version_id=version.scheme_version_id, - ) + snapshot = await _refresh_current_draft_snapshot_if_possible(scheme_id) await create_audit_event( - scheme_id=scheme.scheme_id, + scheme_id=scheme_id, event_type="pricing.category.created", object_type="pricing_category", object_ref=pricing_category_id, - details={ - "name": payload.name, - "code": payload.code, - "scheme_version_id": version.scheme_version_id, - "snapshot": snapshot, - }, + details={"name": payload.name, "code": payload.code, "snapshot": snapshot}, ) - return { - "pricing_category_id": pricing_category_id, - "scheme_id": scheme.scheme_id, - "scheme_version_id": version.scheme_version_id, - "name": payload.name, - "code": payload.code, - } + return PricingCategoryCreateResponse( + pricing_category_id=pricing_category_id, + scheme_id=scheme_id, + name=payload.name, + code=payload.code, + ) -@router.put(f"{settings.api_v1_prefix}/schemes/{{scheme_id}}/pricing/categories/{{pricing_category_id}}") +@router.put( + f"{settings.api_v1_prefix}/schemes/{{scheme_id}}/pricing/categories/{{pricing_category_id}}", + response_model=PricingCategoryUpdateResponse, +) async def update_pricing_category_endpoint( scheme_id: str, pricing_category_id: str, @@ -131,91 +131,81 @@ async def update_pricing_category_endpoint( expected_scheme_version_id: str | None = Query(default=None), role: str = Depends(require_api_key), ): - scheme, version = await get_current_draft_context( + await validate_expected_draft_version_if_provided( scheme_id=scheme_id, expected_scheme_version_id=expected_scheme_version_id, ) row = await update_pricing_category( - scheme_id=scheme.scheme_id, + scheme_id=scheme_id, pricing_category_id=pricing_category_id, name=payload.name, code=payload.code, ) - snapshot = await _refresh_current_draft_snapshot_if_possible( - scheme_id=scheme.scheme_id, - expected_scheme_version_id=version.scheme_version_id, - ) + snapshot = await _refresh_current_draft_snapshot_if_possible(scheme_id) await create_audit_event( - scheme_id=scheme.scheme_id, + scheme_id=scheme_id, event_type="pricing.category.updated", object_type="pricing_category", object_ref=pricing_category_id, - details={ - "name": payload.name, - "code": payload.code, - "scheme_version_id": version.scheme_version_id, - "snapshot": snapshot, - }, + details={"name": payload.name, "code": payload.code, "snapshot": snapshot}, ) - return { - "pricing_category_id": row.pricing_category_id, - "scheme_id": row.scheme_id, - "scheme_version_id": version.scheme_version_id, - "name": row.name, - "code": row.code, - } + return PricingCategoryUpdateResponse( + pricing_category_id=row.pricing_category_id, + scheme_id=row.scheme_id, + name=row.name, + code=row.code, + ) -@router.delete(f"{settings.api_v1_prefix}/schemes/{{scheme_id}}/pricing/categories/{{pricing_category_id}}") +@router.delete( + f"{settings.api_v1_prefix}/schemes/{{scheme_id}}/pricing/categories/{{pricing_category_id}}", + response_model=DeleteResponse, +) async def delete_pricing_category_endpoint( scheme_id: str, pricing_category_id: str, expected_scheme_version_id: str | None = Query(default=None), role: str = Depends(require_api_key), ): - scheme, version = await get_current_draft_context( + await validate_expected_draft_version_if_provided( scheme_id=scheme_id, expected_scheme_version_id=expected_scheme_version_id, ) await delete_pricing_category( - scheme_id=scheme.scheme_id, + scheme_id=scheme_id, pricing_category_id=pricing_category_id, ) - snapshot = await _refresh_current_draft_snapshot_if_possible( - scheme_id=scheme.scheme_id, - expected_scheme_version_id=version.scheme_version_id, - ) + snapshot = await _refresh_current_draft_snapshot_if_possible(scheme_id) await create_audit_event( - scheme_id=scheme.scheme_id, + scheme_id=scheme_id, event_type="pricing.category.deleted", object_type="pricing_category", object_ref=pricing_category_id, - details={ - "scheme_version_id": version.scheme_version_id, - "snapshot": snapshot, - }, + details={"snapshot": snapshot}, ) - return { - "deleted": True, - "pricing_category_id": pricing_category_id, - "scheme_version_id": version.scheme_version_id, - } + return DeleteResponse( + deleted=True, + pricing_category_id=pricing_category_id, + ) -@router.post(f"{settings.api_v1_prefix}/schemes/{{scheme_id}}/pricing/rules") +@router.post( + f"{settings.api_v1_prefix}/schemes/{{scheme_id}}/pricing/rules", + response_model=PriceRuleCreateResponse, +) async def create_price_rule_endpoint( scheme_id: str, payload: PriceRuleCreateRequest, expected_scheme_version_id: str | None = Query(default=None), role: str = Depends(require_api_key), ): - scheme, version = await get_current_draft_context( + await validate_expected_draft_version_if_provided( scheme_id=scheme_id, expected_scheme_version_id=expected_scheme_version_id, ) @@ -229,20 +219,17 @@ async def create_price_rule_endpoint( ) price_rule_id = await create_price_rule( - scheme_id=scheme.scheme_id, + scheme_id=scheme_id, pricing_category_id=payload.pricing_category_id, target_type=payload.target_type, target_ref=payload.target_ref, amount=amount, currency=payload.currency, ) - snapshot = await _refresh_current_draft_snapshot_if_possible( - scheme_id=scheme.scheme_id, - expected_scheme_version_id=version.scheme_version_id, - ) + snapshot = await _refresh_current_draft_snapshot_if_possible(scheme_id) await create_audit_event( - scheme_id=scheme.scheme_id, + scheme_id=scheme_id, event_type="pricing.rule.created", object_type="price_rule", object_ref=price_rule_id, @@ -252,24 +239,25 @@ async def create_price_rule_endpoint( "target_ref": payload.target_ref, "amount": payload.amount, "currency": payload.currency, - "scheme_version_id": version.scheme_version_id, "snapshot": snapshot, }, ) - return { - "price_rule_id": price_rule_id, - "scheme_id": scheme.scheme_id, - "scheme_version_id": version.scheme_version_id, - "pricing_category_id": payload.pricing_category_id, - "target_type": payload.target_type, - "target_ref": payload.target_ref, - "amount": payload.amount, - "currency": payload.currency, - } + return PriceRuleCreateResponse( + price_rule_id=price_rule_id, + scheme_id=scheme_id, + pricing_category_id=payload.pricing_category_id, + target_type=payload.target_type, + target_ref=payload.target_ref, + amount=payload.amount, + currency=payload.currency, + ) -@router.put(f"{settings.api_v1_prefix}/schemes/{{scheme_id}}/pricing/rules/{{price_rule_id}}") +@router.put( + f"{settings.api_v1_prefix}/schemes/{{scheme_id}}/pricing/rules/{{price_rule_id}}", + response_model=PriceRuleUpdateResponse, +) async def update_price_rule_endpoint( scheme_id: str, price_rule_id: str, @@ -277,7 +265,7 @@ async def update_price_rule_endpoint( expected_scheme_version_id: str | None = Query(default=None), role: str = Depends(require_api_key), ): - scheme, version = await get_current_draft_context( + await validate_expected_draft_version_if_provided( scheme_id=scheme_id, expected_scheme_version_id=expected_scheme_version_id, ) @@ -291,7 +279,7 @@ async def update_price_rule_endpoint( ) row = await update_price_rule( - scheme_id=scheme.scheme_id, + scheme_id=scheme_id, price_rule_id=price_rule_id, pricing_category_id=payload.pricing_category_id, target_type=payload.target_type, @@ -299,13 +287,10 @@ async def update_price_rule_endpoint( amount=amount, currency=payload.currency, ) - snapshot = await _refresh_current_draft_snapshot_if_possible( - scheme_id=scheme.scheme_id, - expected_scheme_version_id=version.scheme_version_id, - ) + snapshot = await _refresh_current_draft_snapshot_if_possible(scheme_id) await create_audit_event( - scheme_id=scheme.scheme_id, + scheme_id=scheme_id, event_type="pricing.rule.updated", object_type="price_rule", object_ref=price_rule_id, @@ -315,57 +300,51 @@ async def update_price_rule_endpoint( "target_ref": payload.target_ref, "amount": payload.amount, "currency": payload.currency, - "scheme_version_id": version.scheme_version_id, "snapshot": snapshot, }, ) - return { - "price_rule_id": row.price_rule_id, - "scheme_id": row.scheme_id, - "scheme_version_id": version.scheme_version_id, - "pricing_category_id": row.pricing_category_id, - "target_type": row.target_type, - "target_ref": row.target_ref, - "amount": str(row.amount), - "currency": row.currency, - } + return PriceRuleUpdateResponse( + price_rule_id=row.price_rule_id, + scheme_id=row.scheme_id, + pricing_category_id=row.pricing_category_id, + target_type=row.target_type, + target_ref=row.target_ref, + amount=str(row.amount), + currency=row.currency, + ) -@router.delete(f"{settings.api_v1_prefix}/schemes/{{scheme_id}}/pricing/rules/{{price_rule_id}}") +@router.delete( + f"{settings.api_v1_prefix}/schemes/{{scheme_id}}/pricing/rules/{{price_rule_id}}", + response_model=DeleteResponse, +) async def delete_price_rule_endpoint( scheme_id: str, price_rule_id: str, expected_scheme_version_id: str | None = Query(default=None), role: str = Depends(require_api_key), ): - scheme, version = await get_current_draft_context( + await validate_expected_draft_version_if_provided( scheme_id=scheme_id, expected_scheme_version_id=expected_scheme_version_id, ) await delete_price_rule( - scheme_id=scheme.scheme_id, + scheme_id=scheme_id, price_rule_id=price_rule_id, ) - snapshot = await _refresh_current_draft_snapshot_if_possible( - scheme_id=scheme.scheme_id, - expected_scheme_version_id=version.scheme_version_id, - ) + snapshot = await _refresh_current_draft_snapshot_if_possible(scheme_id) await create_audit_event( - scheme_id=scheme.scheme_id, + scheme_id=scheme_id, event_type="pricing.rule.deleted", object_type="price_rule", object_ref=price_rule_id, - details={ - "scheme_version_id": version.scheme_version_id, - "snapshot": snapshot, - }, + details={"snapshot": snapshot}, ) - return { - "deleted": True, - "price_rule_id": price_rule_id, - "scheme_version_id": version.scheme_version_id, - } + return DeleteResponse( + deleted=True, + price_rule_id=price_rule_id, + ) diff --git a/backend/app/schemas/pricing.py b/backend/app/schemas/pricing.py index f7a80d6..0805342 100644 --- a/backend/app/schemas/pricing.py +++ b/backend/app/schemas/pricing.py @@ -1,22 +1,4 @@ -from decimal import Decimal, InvalidOperation - -from pydantic import BaseModel, Field, field_validator - - -def _validate_decimal_amount(value: Decimal) -> Decimal: - try: - normalized = Decimal(value) - except (InvalidOperation, TypeError, ValueError) as exc: - raise ValueError("Некорректная сумма") from exc - - if not normalized.is_finite(): - raise ValueError("Некорректная сумма") - - return normalized - - -class DeleteResponse(BaseModel): - status: str +from pydantic import BaseModel, Field class PricingCategoryCreateRequest(BaseModel): @@ -29,6 +11,22 @@ class PricingCategoryUpdateRequest(BaseModel): code: str | None = Field(default=None, max_length=128) +class PriceRuleCreateRequest(BaseModel): + pricing_category_id: str = Field(..., max_length=32) + target_type: str = Field(..., pattern="^(seat|group|sector)$") + target_ref: str = Field(..., min_length=1, max_length=128) + amount: str = Field(..., min_length=1, max_length=32) + currency: str = Field(default="RUB", min_length=3, max_length=8) + + +class PriceRuleUpdateRequest(BaseModel): + pricing_category_id: str = Field(..., max_length=32) + target_type: str = Field(..., pattern="^(seat|group|sector)$") + target_ref: str = Field(..., min_length=1, max_length=128) + amount: str = Field(..., min_length=1, max_length=32) + currency: str = Field(default="RUB", min_length=3, max_length=8) + + class PricingCategoryItem(BaseModel): pricing_category_id: str scheme_id: str @@ -37,6 +35,22 @@ class PricingCategoryItem(BaseModel): created_at: str +class PriceRuleItem(BaseModel): + price_rule_id: str + scheme_id: str + pricing_category_id: str | None + target_type: str + target_ref: str + amount: str + currency: str + created_at: str + + +class PricingBundleResponse(BaseModel): + categories: list[PricingCategoryItem] + rules: list[PriceRuleItem] + + class PricingCategoryCreateResponse(BaseModel): pricing_category_id: str scheme_id: str @@ -51,50 +65,13 @@ class PricingCategoryUpdateResponse(BaseModel): code: str | None -class PriceRuleCreateRequest(BaseModel): - pricing_category_id: str | None = Field(default=None, max_length=32) - target_type: str = Field(..., pattern="^(seat|group|sector)$") - target_ref: str = Field(..., min_length=1, max_length=128) - amount: Decimal - currency: str = Field(default="RUB", min_length=3, max_length=8) - - @field_validator("amount") - @classmethod - def validate_amount(cls, value: Decimal) -> Decimal: - return _validate_decimal_amount(value) - - -class PriceRuleUpdateRequest(BaseModel): - pricing_category_id: str | None = Field(default=None, max_length=32) - target_type: str = Field(..., pattern="^(seat|group|sector)$") - target_ref: str = Field(..., min_length=1, max_length=128) - amount: Decimal - currency: str = Field(default="RUB", min_length=3, max_length=8) - - @field_validator("amount") - @classmethod - def validate_amount(cls, value: Decimal) -> Decimal: - return _validate_decimal_amount(value) - - -class PriceRuleItem(BaseModel): - price_rule_id: str - scheme_id: str - pricing_category_id: str | None - target_type: str - target_ref: str - amount: Decimal | str - currency: str - created_at: str - - class PriceRuleCreateResponse(BaseModel): price_rule_id: str scheme_id: str - pricing_category_id: str | None + pricing_category_id: str target_type: str target_ref: str - amount: Decimal + amount: str currency: str @@ -104,10 +81,16 @@ class PriceRuleUpdateResponse(BaseModel): pricing_category_id: str | None target_type: str target_ref: str - amount: Decimal + amount: str currency: str +class DeleteResponse(BaseModel): + deleted: bool + pricing_category_id: str | None = None + price_rule_id: str | None = None + + class EffectiveSeatPriceResponse(BaseModel): scheme_id: str scheme_version_id: str @@ -117,15 +100,5 @@ class EffectiveSeatPriceResponse(BaseModel): matched_rule_level: str matched_target_ref: str pricing_category_id: str | None - amount: Decimal | str + amount: str currency: str - - -class SchemePricingResponse(BaseModel): - categories: list[PricingCategoryItem] - rules: list[PriceRuleItem] - - -class PricingBundleResponse(BaseModel): - categories: list[PricingCategoryItem] - rules: list[PriceRuleItem] diff --git a/backend/app/services/draft_guard.py b/backend/app/services/draft_guard.py index 1ed89c4..f70287b 100644 --- a/backend/app/services/draft_guard.py +++ b/backend/app/services/draft_guard.py @@ -4,24 +4,17 @@ from app.repositories.scheme_versions import get_current_scheme_version from app.repositories.schemes import get_scheme_record_by_scheme_id -def ensure_expected_scheme_version_id( +def build_stale_draft_version_detail( *, + expected_scheme_version_id: str, actual_scheme_version_id: str, - expected_scheme_version_id: str | None, -) -> None: - if expected_scheme_version_id is None: - return - - if expected_scheme_version_id != actual_scheme_version_id: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail={ - "code": "stale_draft_version", - "message": "Draft scheme version is stale. Reload current draft state before applying mutation.", - "expected_scheme_version_id": expected_scheme_version_id, - "actual_scheme_version_id": actual_scheme_version_id, - }, - ) +) -> dict: + return { + "code": "stale_draft_version", + "message": "Draft scheme version is stale. Reload current draft state before applying mutation.", + "expected_scheme_version_id": expected_scheme_version_id, + "actual_scheme_version_id": actual_scheme_version_id, + } async def get_current_draft_context( @@ -40,9 +33,27 @@ async def get_current_draft_context( detail="Current scheme version is not editable because it is not in draft state", ) - ensure_expected_scheme_version_id( - actual_scheme_version_id=version.scheme_version_id, - expected_scheme_version_id=expected_scheme_version_id, - ) + if expected_scheme_version_id and expected_scheme_version_id != version.scheme_version_id: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=build_stale_draft_version_detail( + expected_scheme_version_id=expected_scheme_version_id, + actual_scheme_version_id=version.scheme_version_id, + ), + ) return scheme, version + + +async def validate_expected_draft_version_if_provided( + scheme_id: str, + expected_scheme_version_id: str | None, +): + if not expected_scheme_version_id: + return None + + scheme, version = await get_current_draft_context( + scheme_id=scheme_id, + expected_scheme_version_id=expected_scheme_version_id, + ) + return scheme, version diff --git a/backend/docs/api-map.md b/backend/docs/api-map.md index ff04ca4..d8b424f 100644 --- a/backend/docs/api-map.md +++ b/backend/docs/api-map.md @@ -55,6 +55,19 @@ - POST /api/v1/schemes/{scheme_id}/draft/remap/preview - POST /api/v1/schemes/{scheme_id}/draft/remap/apply +## app/api/routes/editor.py +- GET /api/v1/schemes/{scheme_id}/draft/structure +- GET /api/v1/schemes/{scheme_id}/draft/compare-preview +- POST /api/v1/schemes/{scheme_id}/draft/sectors +- POST /api/v1/schemes/{scheme_id}/draft/groups +- DELETE /api/v1/schemes/{scheme_id}/draft/sectors/records/{sector_record_id} +- DELETE /api/v1/schemes/{scheme_id}/draft/groups/records/{group_record_id} +- PATCH /api/v1/schemes/{scheme_id}/draft/seats/records/{seat_record_id} +- POST /api/v1/schemes/{scheme_id}/draft/seats/bulk +- PATCH /api/v1/schemes/{scheme_id}/draft/sectors/records/{sector_record_id} +- PATCH /api/v1/schemes/{scheme_id}/draft/groups/records/{group_record_id} +- POST /api/v1/schemes/{scheme_id}/draft/repair-references + ## app/api/routes/admin.py - GET /api/v1/admin/schemes/{scheme_id}/current/artifacts - GET /api/v1/admin/schemes/{scheme_id}/current/validation @@ -66,4 +79,4 @@ ## Notes - This file is an operational route index, not a generated OpenAPI export. - Update this map in the same change set when adding, removing, renaming, or moving routes. -- Editor routes are maintained separately and should be synced from current source before relying on this file as a full route inventory. +- Query guards such as expected_current_scheme_version_id / expected_scheme_version_id are part of the operational contract for optimistic concurrency on mutable flows. diff --git a/backend/docs/smoke-regression.md b/backend/docs/smoke-regression.md index 8128df7..c7d1e0d 100644 --- a/backend/docs/smoke-regression.md +++ b/backend/docs/smoke-regression.md @@ -62,16 +62,20 @@ Validate: - no 500 on passthrough mode - unsupported mode returns 422 -## 5. Pricing read model +## 5. Pricing read / write contract - GET /api/v1/schemes/{scheme_id}/pricing -> 200 - GET /api/v1/schemes/{scheme_id}/current/seats/{seat_id}/price -> 200 for priced seat - GET /api/v1/schemes/{scheme_id}/test/seats/{seat_id} -> 200 for known seat +- POST /api/v1/schemes/{scheme_id}/pricing/categories?expected_scheme_version_id={draft_version_id} -> 200 for valid draft +- POST /api/v1/schemes/{scheme_id}/pricing/categories?expected_scheme_version_id=deadbeef... -> 409 for stale draft Validate: - pricing bundle contains categories and rules arrays - effective seat price resolves according to domain priority - test seat preview explains selectable / has_price state +- pricing write responses are stable and typed +- stale pricing mutation returns `detail.code = stale_draft_version` ## 6. Draft publish preview @@ -122,6 +126,8 @@ Regression is considered failed if any of the following happen: - passthrough display endpoint fails on known-good sample - publish preview refresh or cached read returns 500 - pricing bundle contract changes unexpectedly +- pricing write contract regresses or stops returning typed payloads +- stale draft guard stops returning 409 on pricing mutations - admin audit/cleanup endpoints fail on healthy environment - artifact retention grows without bound for repeated preview refresh on same variant