fix order form
This commit is contained in:
@@ -2,7 +2,12 @@
|
||||
|
||||
from typing import Any
|
||||
|
||||
from app.schemas.order import OrderCreateRequest, OrderCreateResponse
|
||||
from app.schemas.order import (
|
||||
OrderCreateRequest,
|
||||
OrderCreateResponse,
|
||||
OrderPackage,
|
||||
OrderParty,
|
||||
)
|
||||
|
||||
|
||||
class CDEKOrderMappingError(ValueError):
|
||||
@@ -10,7 +15,22 @@ class CDEKOrderMappingError(ValueError):
|
||||
|
||||
|
||||
def map_cdek_order_request(request: OrderCreateRequest) -> dict[str, Any]:
|
||||
return request.model_dump(mode="python", exclude_none=True)
|
||||
payload: dict[str, Any] = {
|
||||
"type": request.type,
|
||||
"tariff_code": request.tariff_code,
|
||||
"sender": _map_order_party(request.sender),
|
||||
"recipient": _map_order_party(request.recipient),
|
||||
"from_location": request.from_location.model_dump(mode="python"),
|
||||
"to_location": request.to_location.model_dump(mode="python"),
|
||||
"packages": [_map_order_package(package) for package in request.packages],
|
||||
}
|
||||
if request.comment is not None:
|
||||
payload["comment"] = request.comment
|
||||
if request.services is not None:
|
||||
payload["services"] = [
|
||||
service.model_dump(mode="python") for service in request.services
|
||||
]
|
||||
return payload
|
||||
|
||||
|
||||
def map_cdek_order_response(payload: dict[str, Any]) -> OrderCreateResponse:
|
||||
@@ -23,3 +43,17 @@ def map_cdek_order_response(payload: dict[str, Any]) -> OrderCreateResponse:
|
||||
raise CDEKOrderMappingError("CDEK order response must include entity.uuid.")
|
||||
|
||||
return OrderCreateResponse(provider="cdek", order_uuid=order_uuid)
|
||||
|
||||
|
||||
def _map_order_party(party: OrderParty) -> dict[str, Any]:
|
||||
return {
|
||||
"name": party.name,
|
||||
"email": party.email,
|
||||
"phones": [party.phone.model_dump(mode="python")],
|
||||
}
|
||||
|
||||
|
||||
def _map_order_package(package: OrderPackage) -> dict[str, Any]:
|
||||
payload = package.model_dump(mode="python", exclude_none=True)
|
||||
payload["weight"] = package.weight * 1000
|
||||
return payload
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
from typing import Literal
|
||||
|
||||
from pydantic import BaseModel, Field, model_validator
|
||||
from pydantic import BaseModel, ConfigDict, Field, model_validator
|
||||
|
||||
|
||||
class OrderPhone(BaseModel):
|
||||
@@ -10,9 +10,11 @@ class OrderPhone(BaseModel):
|
||||
|
||||
|
||||
class OrderParty(BaseModel):
|
||||
model_config = ConfigDict(extra="forbid")
|
||||
|
||||
name: str = Field(min_length=1)
|
||||
email: str = Field(min_length=1)
|
||||
phones: list[OrderPhone] = Field(min_length=1)
|
||||
phone: OrderPhone
|
||||
|
||||
@model_validator(mode="before")
|
||||
@classmethod
|
||||
@@ -50,7 +52,7 @@ class OrderCreateRequest(BaseModel):
|
||||
recipient: OrderParty
|
||||
from_location: OrderLocation
|
||||
to_location: OrderLocation
|
||||
services: list[OrderService] = Field(min_length=1)
|
||||
services: list[OrderService] | None = Field(default=None, min_length=1)
|
||||
packages: list[OrderPackage] = Field(min_length=1)
|
||||
|
||||
|
||||
|
||||
+7
-17
@@ -24,20 +24,16 @@ Content-Type: application/json
|
||||
"sender": {
|
||||
"name": "Петр Петров",
|
||||
"email": "sender@example.com",
|
||||
"phones": [
|
||||
{
|
||||
"number": "+79009876543"
|
||||
}
|
||||
]
|
||||
"phone": {
|
||||
"number": "+79009876543"
|
||||
}
|
||||
},
|
||||
"recipient": {
|
||||
"name": "Иван Иванов",
|
||||
"email": "ivan@example.com",
|
||||
"phones": [
|
||||
{
|
||||
"number": "+79001234567"
|
||||
}
|
||||
]
|
||||
"phone": {
|
||||
"number": "+79001234567"
|
||||
}
|
||||
},
|
||||
"from_location": {
|
||||
"address": "ул. Ленина, 1",
|
||||
@@ -49,16 +45,10 @@ Content-Type: application/json
|
||||
"city": "Новосибирск",
|
||||
"country_code": "RU"
|
||||
},
|
||||
"services": [
|
||||
{
|
||||
"code": "INSURANCE",
|
||||
"parameter": "1000"
|
||||
}
|
||||
],
|
||||
"packages": [
|
||||
{
|
||||
"number": "1",
|
||||
"weight": 1000,
|
||||
"weight": 1,
|
||||
"length": 20,
|
||||
"width": 15,
|
||||
"height": 10,
|
||||
|
||||
+3
-2
@@ -32,9 +32,10 @@
|
||||
| 023 | DONE | 2026-03-29 | Add Yandex Geosuggest address suggestion adapter and CIS routing | `spec/tasks/023_add_yandex_geosuggest_address_suggestion_adapter.md` |
|
||||
| 024 | DONE | 2026-03-29 | Add TomTom address suggestion adapter and Europe routing | `spec/tasks/024_add_tomtom_address_suggestion_adapter.md` |
|
||||
| 025 | DONE | 2026-04-03 | Remove company from Create Delivery Order parties | `spec/tasks/025_remove_company_from_create_delivery_order.md` |
|
||||
| 026 | DONE | 2026-04-05 | Align CDEK order contract with single phone and kilogram package weight | `spec/tasks/026_align_cdek_order_contract_single_phone_and_weight_units.md` |
|
||||
|
||||
## Summary
|
||||
|
||||
- Total: **26**
|
||||
- Total: **27**
|
||||
- TODO: **0**
|
||||
- DONE: **26**
|
||||
- DONE: **27**
|
||||
|
||||
+8
-3
@@ -97,6 +97,9 @@
|
||||
- Для расчёта тарифа CDEK adapter принимает city identifiers из `DeliveryCalculationRequest`, находит запись в `cities_map`, берёт `cdek.code` и передаёт его в CDEK API
|
||||
|
||||
Для сценария создания заказа CDEK adapter принимает валидированную order model, отправляет контракт `Регистрация заказа (тип "доставка", до двери)` из `http-client.http` и возвращает внутреннюю response model без утечки HTTP-деталей в Service.
|
||||
Во внутреннем order flow `sender` и `recipient` содержат ровно одно поле `phone`, а CDEK adapter сериализует его в provider payload `phones` с одним элементом.
|
||||
Поле `services` в order flow является необязательным; при отсутствии значения adapter не отправляет `services` в CDEK payload.
|
||||
Поле `packages[*].weight` во входном order request задаётся в килограммах, а CDEK adapter конвертирует его в граммы перед отправкой в provider API.
|
||||
|
||||
### Adapter (`app/adapters/address_suggestions`)
|
||||
- `base.py` — абстрактный интерфейс `AddressSuggestionProvider`:
|
||||
@@ -173,11 +176,11 @@ comment: str | None
|
||||
sender:
|
||||
name: str
|
||||
email: str
|
||||
phones: list[{number: str}]
|
||||
phone: {number: str}
|
||||
recipient:
|
||||
name: str
|
||||
email: str
|
||||
phones: list[{number: str}]
|
||||
phone: {number: str}
|
||||
from_location:
|
||||
address: str
|
||||
city: str
|
||||
@@ -186,11 +189,13 @@ to_location:
|
||||
address: str
|
||||
city: str
|
||||
country_code: str
|
||||
services: list[{code: str, parameter: str}]
|
||||
services: list[{code: str, parameter: str}] | None
|
||||
packages: list[{number: str, weight: int, length: int, width: int, height: int, comment: str | None}]
|
||||
```
|
||||
|
||||
`from_location.address` и `to_location.address` должны содержать точные значения адреса, выбранные клиентом; order flow не выполняет address suggestion lookup.
|
||||
`sender.phone` и `recipient.phone` представляют единственный телефон для соответствующей стороны заказа; передача нескольких телефонов во входном API не поддерживается.
|
||||
`packages[*].weight` в `OrderCreateRequest` задаётся в килограммах, а в payload CDEK должен передаваться в граммах.
|
||||
|
||||
### Выходная: `OrderCreateResponse`
|
||||
```
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
---
|
||||
id: 026
|
||||
title: Align CDEK order contract with single phone and kilogram package weight
|
||||
status: DONE
|
||||
created: 2026-04-05
|
||||
---
|
||||
|
||||
## Context
|
||||
Текущий order flow принимает `sender.phones` и `recipient.phones` как список, требует обязательное поле `services` и пробрасывает `packages.weight` в CDEK payload без явной фиксации единиц измерения. Новый контракт должен принимать один телефон на сторону, разрешать отсутствие `services` и гарантировать, что во входном API вес упаковки задаётся в килограммах, а в CDEK отправляется в граммах.
|
||||
|
||||
## Goal
|
||||
Обновить flow `POST /api/v1/delivery/order` по слоям Controller, Service и Adapter так, чтобы public/internal request contract использовал `phone` вместо `phones`, поле `services` было необязательным, а CDEK order mapper конвертировал `packages.weight` из килограммов во входном запросе в граммы во внешнем provider payload.
|
||||
|
||||
## Constraints
|
||||
- Изменения ограничены существующими модулями order flow: `app/schemas/order.py`, `app/controllers/v1/delivery.py`, `app/services/aggregator.py`, `app/adapters/delivery_providers/cdek/`, `http-client.http` и связанными тестами.
|
||||
- `POST /api/v1/delivery/order` должен оставаться в существующем controller и по-прежнему вызывать ровно один метод Service: `AggregatorService.create_order()`.
|
||||
- Service остаётся orchestration layer и не получает новую business logic; он только принимает обновлённую order model и делегирует её adapter.
|
||||
- Во внутреннем order flow и публичном API поле `phones` должно быть удалено; передача нескольких телефонов больше не поддерживается.
|
||||
- CDEK adapter может сериализовать внутренний `phone` в provider-specific поле `phones`, если это требуется внешним контрактом CDEK, но множественность телефонов не должна возвращаться во внутренние модели и controller contract.
|
||||
- `services` должно быть необязательным полем request schema; при отсутствии значения нельзя подставлять фиктивные service entries.
|
||||
- Конвертация единиц `packages.weight` должна выполняться только на границе CDEK adapter mapping: входной order request использует килограммы, исходящий payload в CDEK использует граммы.
|
||||
- Scope задачи не включает изменение response contract, price flow, address suggestion flow, provider routing, кеширование, новые провайдеры и расширение поддерживаемых `type`/`tariff_code`.
|
||||
- Не изменять файлы в `spec/`.
|
||||
|
||||
## Acceptance criteria
|
||||
- `OrderCreateRequest` использует `sender.phone` и `recipient.phone` вместо `sender.phones` и `recipient.phones`.
|
||||
- Если request payload содержит `sender.phones` или `recipient.phones`, endpoint возвращает 422 на уровне schema validation.
|
||||
- `POST /api/v1/delivery/order` принимает валидный payload без поля `services`.
|
||||
- Если `services` отсутствует или равен `null`, service и adapter flow успешно обрабатывают запрос без добавления `services` в исходящий CDEK payload.
|
||||
- `AggregatorService.create_order()` продолжает только оркестрировать вызов injected order adapter и не содержит преобразования `phone`/`phones` или килограммов в граммы.
|
||||
- CDEK order mapper формирует provider payload с полями `sender.phones` и `recipient.phones`, каждое из которых содержит ровно один элемент, полученный из соответствующего внутреннего поля `phone`.
|
||||
- Для каждого элемента `packages` исходящий payload CDEK содержит `weight`, равный значению входного `packages.weight`, умноженному на `1000`.
|
||||
- `http-client.http` содержит актуальный пример Create Delivery Order с `phone` вместо `phones`, без обязательного `services` и с весом, отражающим controller contract в килограммах.
|
||||
|
||||
## Definition of Done
|
||||
- [ ] Обновлён public/internal order request contract на `phone` вместо `phones`.
|
||||
- [ ] `services` сделано необязательным без изменения endpoint path и service orchestration.
|
||||
- [ ] Реализован mapping одного телефона в provider payload `phones` и конвертация `packages.weight` из килограммов в граммы.
|
||||
- [ ] Обновлены controller, service и adapter tests под новый контракт и unit conversion.
|
||||
- [ ] Обновлён пример запроса в `http-client.http`.
|
||||
|
||||
## Tests
|
||||
- Обновить `tests/controllers/v1/test_order.py` для success case с `phone`, сценариев 422 при передаче `sender.phones` и `recipient.phones`, а также для запроса без `services`.
|
||||
- Обновить `tests/services/test_order.py` для проверки, что service принимает обновлённую order model, делегирует её adapter без новой логики и корректно работает с `services=None`.
|
||||
- Обновить `tests/adapters/delivery_providers/cdek/test_order_client.py` для проверки mapping `phone -> phones[0]`, отсутствия `services` в исходящем payload при `None` и конвертации `packages.weight` из килограммов в граммы.
|
||||
- При необходимости обновить другие order-related tests и fixtures, завязанные на старые поля `phones` и обязательность `services`.
|
||||
|
||||
## Commands
|
||||
- `poetry run pytest tests/controllers/v1/test_order.py -q`
|
||||
- `poetry run pytest tests/services/test_order.py -q`
|
||||
- `poetry run pytest tests/adapters/delivery_providers/cdek/test_order_client.py -q`
|
||||
- `python3 spec/gen_spec_index.py --check`
|
||||
@@ -59,12 +59,12 @@ def _make_order_request(**overrides: object) -> OrderCreateRequest:
|
||||
"sender": {
|
||||
"name": "Petr Petrov",
|
||||
"email": "sender@example.com",
|
||||
"phones": [{"number": "+79009876543"}],
|
||||
"phone": {"number": "+79009876543"},
|
||||
},
|
||||
"recipient": {
|
||||
"name": "Ivan Ivanov",
|
||||
"email": "ivan@example.com",
|
||||
"phones": [{"number": "+79001234567"}],
|
||||
"phone": {"number": "+79001234567"},
|
||||
},
|
||||
"from_location": {
|
||||
"address": "Lenina 1",
|
||||
@@ -80,7 +80,7 @@ def _make_order_request(**overrides: object) -> OrderCreateRequest:
|
||||
"packages": [
|
||||
{
|
||||
"number": "1",
|
||||
"weight": 1000,
|
||||
"weight": 1,
|
||||
"length": 20,
|
||||
"width": 15,
|
||||
"height": 10,
|
||||
@@ -160,6 +160,70 @@ def test_provider_register_order_posts_cdek_contract_payload_and_maps_response()
|
||||
]
|
||||
|
||||
|
||||
def test_provider_register_order_omits_services_when_none_and_converts_package_weights() -> None:
|
||||
response = httpx.Response(
|
||||
200,
|
||||
json={"entity": {"uuid": "cdek-order-uuid"}},
|
||||
request=httpx.Request("POST", "https://api.cdek.test/v2/orders"),
|
||||
)
|
||||
http_client = SequenceHTTPClient([response])
|
||||
provider = CDEKProvider(
|
||||
CDEKClient(
|
||||
http_client=http_client, # type: ignore[arg-type]
|
||||
auth_client=StubAuthClient(), # type: ignore[arg-type]
|
||||
base_url="https://api.cdek.test/v2",
|
||||
timeout_seconds=7.5,
|
||||
retry_attempts=0,
|
||||
)
|
||||
)
|
||||
request = _make_order_request(
|
||||
services=None,
|
||||
packages=[
|
||||
{
|
||||
"number": "1",
|
||||
"weight": 1,
|
||||
"length": 20,
|
||||
"width": 15,
|
||||
"height": 10,
|
||||
"comment": "Package 1",
|
||||
},
|
||||
{
|
||||
"number": "2",
|
||||
"weight": 2,
|
||||
"length": 25,
|
||||
"width": 18,
|
||||
"height": 12,
|
||||
"comment": "Package 2",
|
||||
},
|
||||
],
|
||||
)
|
||||
|
||||
asyncio.run(provider.register_order(request))
|
||||
|
||||
payload = http_client.calls[0]["json"]
|
||||
assert "services" not in payload
|
||||
assert payload["sender"]["phones"] == [{"number": "+79009876543"}]
|
||||
assert payload["recipient"]["phones"] == [{"number": "+79001234567"}]
|
||||
assert payload["packages"] == [
|
||||
{
|
||||
"number": "1",
|
||||
"weight": 1000,
|
||||
"length": 20,
|
||||
"width": 15,
|
||||
"height": 10,
|
||||
"comment": "Package 1",
|
||||
},
|
||||
{
|
||||
"number": "2",
|
||||
"weight": 2000,
|
||||
"length": 25,
|
||||
"width": 18,
|
||||
"height": 12,
|
||||
"comment": "Package 2",
|
||||
},
|
||||
]
|
||||
|
||||
|
||||
def test_cdek_client_register_order_maps_4xx_to_request_error() -> None:
|
||||
rejected_response = httpx.Response(
|
||||
422,
|
||||
|
||||
@@ -39,12 +39,12 @@ def _valid_payload() -> dict[str, object]:
|
||||
"sender": {
|
||||
"name": "Petr Petrov",
|
||||
"email": "sender@example.com",
|
||||
"phones": [{"number": "+79009876543"}],
|
||||
"phone": {"number": "+79009876543"},
|
||||
},
|
||||
"recipient": {
|
||||
"name": "Ivan Ivanov",
|
||||
"email": "ivan@example.com",
|
||||
"phones": [{"number": "+79001234567"}],
|
||||
"phone": {"number": "+79001234567"},
|
||||
},
|
||||
"from_location": {
|
||||
"address": "Lenina 1",
|
||||
@@ -60,7 +60,7 @@ def _valid_payload() -> dict[str, object]:
|
||||
"packages": [
|
||||
{
|
||||
"number": "1",
|
||||
"weight": 1000,
|
||||
"weight": 1,
|
||||
"length": 20,
|
||||
"width": 15,
|
||||
"height": 10,
|
||||
@@ -136,6 +136,30 @@ def test_post_delivery_order_rejects_sender_company_field() -> None:
|
||||
assert service.calls == []
|
||||
|
||||
|
||||
def test_post_delivery_order_rejects_sender_phones_field() -> None:
|
||||
service = StubAggregatorService(response=None)
|
||||
app = create_app()
|
||||
_install_service_override(app, service)
|
||||
invalid_payload = _valid_payload()
|
||||
invalid_payload["sender"] = {
|
||||
**invalid_payload["sender"], # type: ignore[arg-type]
|
||||
"phones": [{"number": "+79009876543"}],
|
||||
}
|
||||
|
||||
async def run_request() -> httpx.Response:
|
||||
transport = httpx.ASGITransport(app=app)
|
||||
async with httpx.AsyncClient(
|
||||
transport=transport,
|
||||
base_url="http://testserver",
|
||||
) as client:
|
||||
return await client.post("/api/v1/delivery/order", json=invalid_payload)
|
||||
|
||||
response = asyncio.run(run_request())
|
||||
|
||||
assert response.status_code == 422
|
||||
assert service.calls == []
|
||||
|
||||
|
||||
def test_post_delivery_order_rejects_recipient_company_field() -> None:
|
||||
service = StubAggregatorService(response=None)
|
||||
app = create_app()
|
||||
@@ -160,6 +184,53 @@ def test_post_delivery_order_rejects_recipient_company_field() -> None:
|
||||
assert service.calls == []
|
||||
|
||||
|
||||
def test_post_delivery_order_rejects_recipient_phones_field() -> None:
|
||||
service = StubAggregatorService(response=None)
|
||||
app = create_app()
|
||||
_install_service_override(app, service)
|
||||
invalid_payload = _valid_payload()
|
||||
invalid_payload["recipient"] = {
|
||||
**invalid_payload["recipient"], # type: ignore[arg-type]
|
||||
"phones": [{"number": "+79001234567"}],
|
||||
}
|
||||
|
||||
async def run_request() -> httpx.Response:
|
||||
transport = httpx.ASGITransport(app=app)
|
||||
async with httpx.AsyncClient(
|
||||
transport=transport,
|
||||
base_url="http://testserver",
|
||||
) as client:
|
||||
return await client.post("/api/v1/delivery/order", json=invalid_payload)
|
||||
|
||||
response = asyncio.run(run_request())
|
||||
|
||||
assert response.status_code == 422
|
||||
assert service.calls == []
|
||||
|
||||
|
||||
def test_post_delivery_order_accepts_request_without_services() -> None:
|
||||
expected_response = OrderCreateResponse(provider="cdek", order_uuid="order-uuid-1")
|
||||
service = StubAggregatorService(response=expected_response)
|
||||
app = create_app()
|
||||
_install_service_override(app, service)
|
||||
payload = _valid_payload()
|
||||
del payload["services"]
|
||||
|
||||
async def run_request() -> httpx.Response:
|
||||
transport = httpx.ASGITransport(app=app)
|
||||
async with httpx.AsyncClient(
|
||||
transport=transport,
|
||||
base_url="http://testserver",
|
||||
) as client:
|
||||
return await client.post("/api/v1/delivery/order", json=payload)
|
||||
|
||||
response = asyncio.run(run_request())
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == expected_response.model_dump(mode="json")
|
||||
assert service.calls == [OrderCreateRequest(**payload)]
|
||||
|
||||
|
||||
def test_post_delivery_order_maps_invalid_request_to_400() -> None:
|
||||
service = StubAggregatorService(
|
||||
response=None,
|
||||
|
||||
@@ -39,12 +39,12 @@ def _make_order_request(**overrides: object) -> OrderCreateRequest:
|
||||
"sender": {
|
||||
"name": "Petr Petrov",
|
||||
"email": "sender@example.com",
|
||||
"phones": [{"number": "+79009876543"}],
|
||||
"phone": {"number": "+79009876543"},
|
||||
},
|
||||
"recipient": {
|
||||
"name": "Ivan Ivanov",
|
||||
"email": "ivan@example.com",
|
||||
"phones": [{"number": "+79001234567"}],
|
||||
"phone": {"number": "+79001234567"},
|
||||
},
|
||||
"from_location": {
|
||||
"address": "Lenina 1",
|
||||
@@ -60,7 +60,7 @@ def _make_order_request(**overrides: object) -> OrderCreateRequest:
|
||||
"packages": [
|
||||
{
|
||||
"number": "1",
|
||||
"weight": 1000,
|
||||
"weight": 1,
|
||||
"length": 20,
|
||||
"width": 15,
|
||||
"height": 10,
|
||||
@@ -96,6 +96,20 @@ def test_create_order_maps_provider_request_errors_to_invalid_order_error() -> N
|
||||
assert adapter.calls == [request]
|
||||
|
||||
|
||||
def test_create_order_delegates_request_with_services_none_without_new_logic() -> None:
|
||||
request = _make_order_request(services=None)
|
||||
adapter = StubOrderAdapter(
|
||||
response=OrderCreateResponse(provider="cdek", order_uuid="order-uuid-2")
|
||||
)
|
||||
service = AggregatorService(providers=[], order_adapter=adapter)
|
||||
|
||||
result = asyncio.run(service.create_order(request))
|
||||
|
||||
assert result == OrderCreateResponse(provider="cdek", order_uuid="order-uuid-2")
|
||||
assert adapter.calls == [request]
|
||||
assert adapter.calls[0].services is None
|
||||
|
||||
|
||||
def test_create_order_maps_transport_failures_to_unavailable_error() -> None:
|
||||
request = _make_order_request()
|
||||
adapter = StubOrderAdapter(error=RuntimeError("transport down"))
|
||||
|
||||
Reference in New Issue
Block a user