fix(ledger): split dividend response schema and harden decimal tests
Co-authored-by: genichin <4282227+genichin@users.noreply.github.com>
This commit is contained in:
co-authored by
genichin
parent
d3601d32dc
commit
e261a41b7f
+56
-22
@@ -27,6 +27,8 @@ from app.domain.models import Portfolio, PortfolioComment
|
|||||||
from app.domain.pnl import calculate_current_wac, calculate_realized_pnl
|
from app.domain.pnl import calculate_current_wac, calculate_realized_pnl
|
||||||
from app.repositories.factory import get_portfolio_repository_from_settings
|
from app.repositories.factory import get_portfolio_repository_from_settings
|
||||||
from app.schemas.v1 import (
|
from app.schemas.v1 import (
|
||||||
|
DividendLedgerHistoryItem,
|
||||||
|
DividendManualLedgerResponse,
|
||||||
HoldingsCostBasisResponse,
|
HoldingsCostBasisResponse,
|
||||||
LedgerHistoryItem,
|
LedgerHistoryItem,
|
||||||
LedgerHistoryResponse,
|
LedgerHistoryResponse,
|
||||||
@@ -686,25 +688,40 @@ def record_ledger_trade_v1(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def _manual_entry_to_response(entry) -> ManualLedgerResponse:
|
def _manual_entry_to_response(entry) -> ManualLedgerResponse | DividendManualLedgerResponse:
|
||||||
action = (entry.action or entry.side or "").upper()
|
action = (entry.action or entry.side or "").upper()
|
||||||
amount: float | Decimal | None = None
|
|
||||||
if action == "DIVIDEND":
|
if action == "DIVIDEND":
|
||||||
amount = entry.price_decimal if entry.price_decimal is not None else Decimal(str(entry.price))
|
amount_decimal = entry.price_decimal if entry.price_decimal is not None else Decimal(str(entry.price))
|
||||||
elif action in {"INVEST", "BUFFER_OUT", "BUFFER_IN", "WITHDRAW", "PROFIT"}:
|
tax_decimal = entry.tax_amount_decimal if entry.tax_amount_decimal is not None else Decimal(str(entry.tax_amount))
|
||||||
amount = float(entry.price)
|
return DividendManualLedgerResponse(
|
||||||
tax_value: float | Decimal = float(entry.tax_amount)
|
ledger_entry_id=entry.ledger_entry_id,
|
||||||
if action == "DIVIDEND":
|
portfolio_id=entry.portfolio_id,
|
||||||
tax_value = entry.tax_amount_decimal if entry.tax_amount_decimal is not None else Decimal(str(entry.tax_amount))
|
action=action,
|
||||||
|
amount=str(amount_decimal),
|
||||||
|
quantity=entry.quantity,
|
||||||
|
price=entry.price,
|
||||||
|
fee=entry.fee_amount,
|
||||||
|
tax=str(tax_decimal),
|
||||||
|
executed_at=entry.executed_at,
|
||||||
|
note=entry.note,
|
||||||
|
recorded_at=entry.recorded_at,
|
||||||
|
cash_balance_after=entry.cash_balance_after,
|
||||||
|
asset_quantity_after=entry.asset_quantity_after,
|
||||||
|
profit_buffer_after=entry.profit_buffer_after,
|
||||||
|
total_value_after=entry.total_value_after,
|
||||||
|
source=entry.source or "manual",
|
||||||
|
target_ledger_entry_id=entry.target_ledger_entry_id,
|
||||||
|
confirmation_evidence=entry.confirmation_evidence,
|
||||||
|
)
|
||||||
return ManualLedgerResponse(
|
return ManualLedgerResponse(
|
||||||
ledger_entry_id=entry.ledger_entry_id,
|
ledger_entry_id=entry.ledger_entry_id,
|
||||||
portfolio_id=entry.portfolio_id,
|
portfolio_id=entry.portfolio_id,
|
||||||
action=action,
|
action=action,
|
||||||
amount=amount,
|
amount=float(entry.price) if action in {"INVEST", "BUFFER_OUT", "BUFFER_IN", "WITHDRAW", "PROFIT"} else None,
|
||||||
quantity=entry.quantity,
|
quantity=entry.quantity,
|
||||||
price=entry.price,
|
price=entry.price,
|
||||||
fee=entry.fee_amount,
|
fee=entry.fee_amount,
|
||||||
tax=tax_value,
|
tax=float(entry.tax_amount),
|
||||||
executed_at=entry.executed_at,
|
executed_at=entry.executed_at,
|
||||||
note=entry.note,
|
note=entry.note,
|
||||||
recorded_at=entry.recorded_at,
|
recorded_at=entry.recorded_at,
|
||||||
@@ -792,16 +809,33 @@ def _decode_ledger_cursor(cursor: str, *, portfolio_id: str, source: str | None)
|
|||||||
raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail="invalid ledger cursor") from None
|
raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail="invalid ledger cursor") from None
|
||||||
|
|
||||||
|
|
||||||
def _ledger_entry_to_history_item(entry) -> LedgerHistoryItem:
|
def _ledger_entry_to_history_item(entry) -> LedgerHistoryItem | DividendLedgerHistoryItem:
|
||||||
action = (entry.action or entry.side or "").upper()
|
action = (entry.action or entry.side or "").upper()
|
||||||
amount: float | Decimal | None = None
|
|
||||||
if action == "DIVIDEND":
|
if action == "DIVIDEND":
|
||||||
amount = entry.price_decimal if entry.price_decimal is not None else Decimal(str(entry.price))
|
amount_decimal = entry.price_decimal if entry.price_decimal is not None else Decimal(str(entry.price))
|
||||||
elif action in {"INVEST", "BUFFER_OUT", "BUFFER_IN", "WITHDRAW", "PROFIT"}:
|
tax_decimal = entry.tax_amount_decimal if entry.tax_amount_decimal is not None else Decimal(str(entry.tax_amount))
|
||||||
amount = float(entry.price)
|
return DividendLedgerHistoryItem(
|
||||||
tax_amount: float | Decimal | None = entry.tax_amount
|
ledger_entry_id=entry.ledger_entry_id,
|
||||||
if action == "DIVIDEND":
|
portfolio_id=entry.portfolio_id,
|
||||||
tax_amount = entry.tax_amount_decimal if entry.tax_amount_decimal is not None else Decimal(str(entry.tax_amount))
|
source=entry.source or "broker",
|
||||||
|
action=action,
|
||||||
|
recorded_at=entry.recorded_at,
|
||||||
|
executed_at=entry.executed_at,
|
||||||
|
order_id=entry.order_id,
|
||||||
|
execution_id=entry.execution_id,
|
||||||
|
quantity=entry.quantity,
|
||||||
|
amount=str(amount_decimal),
|
||||||
|
price=entry.price,
|
||||||
|
fee_amount=entry.fee_amount,
|
||||||
|
tax_amount=str(tax_decimal),
|
||||||
|
note=entry.note,
|
||||||
|
cash_balance_after=entry.cash_balance_after,
|
||||||
|
asset_quantity_after=entry.asset_quantity_after,
|
||||||
|
profit_buffer_after=entry.profit_buffer_after,
|
||||||
|
total_value_after=entry.total_value_after,
|
||||||
|
target_ledger_entry_id=entry.target_ledger_entry_id,
|
||||||
|
confirmation_evidence=entry.confirmation_evidence,
|
||||||
|
)
|
||||||
return LedgerHistoryItem(
|
return LedgerHistoryItem(
|
||||||
ledger_entry_id=entry.ledger_entry_id,
|
ledger_entry_id=entry.ledger_entry_id,
|
||||||
portfolio_id=entry.portfolio_id,
|
portfolio_id=entry.portfolio_id,
|
||||||
@@ -812,10 +846,10 @@ def _ledger_entry_to_history_item(entry) -> LedgerHistoryItem:
|
|||||||
order_id=entry.order_id,
|
order_id=entry.order_id,
|
||||||
execution_id=entry.execution_id,
|
execution_id=entry.execution_id,
|
||||||
quantity=entry.quantity,
|
quantity=entry.quantity,
|
||||||
amount=amount,
|
amount=float(entry.price) if action in {"INVEST", "BUFFER_OUT", "BUFFER_IN", "WITHDRAW", "PROFIT"} else None,
|
||||||
price=entry.price,
|
price=entry.price,
|
||||||
fee_amount=entry.fee_amount,
|
fee_amount=entry.fee_amount,
|
||||||
tax_amount=tax_amount,
|
tax_amount=float(entry.tax_amount),
|
||||||
note=entry.note,
|
note=entry.note,
|
||||||
cash_balance_after=entry.cash_balance_after,
|
cash_balance_after=entry.cash_balance_after,
|
||||||
asset_quantity_after=entry.asset_quantity_after,
|
asset_quantity_after=entry.asset_quantity_after,
|
||||||
@@ -828,14 +862,14 @@ def _ledger_entry_to_history_item(entry) -> LedgerHistoryItem:
|
|||||||
|
|
||||||
@router.post(
|
@router.post(
|
||||||
"/{portfolio_id}/ledger/manual",
|
"/{portfolio_id}/ledger/manual",
|
||||||
response_model=ManualLedgerResponse,
|
response_model=ManualLedgerResponse | DividendManualLedgerResponse,
|
||||||
status_code=status.HTTP_201_CREATED,
|
status_code=status.HTTP_201_CREATED,
|
||||||
)
|
)
|
||||||
def record_manual_ledger_v1(
|
def record_manual_ledger_v1(
|
||||||
portfolio_id: str,
|
portfolio_id: str,
|
||||||
body: ManualLedgerRequest,
|
body: ManualLedgerRequest,
|
||||||
auth: AuthContext = Depends(require_read),
|
auth: AuthContext = Depends(require_read),
|
||||||
) -> ManualLedgerResponse:
|
) -> ManualLedgerResponse | DividendManualLedgerResponse:
|
||||||
if auth.token_type in {"service", "client"} or auth.user_id is None:
|
if auth.token_type in {"service", "client"} or auth.user_id is None:
|
||||||
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="manual ledger requires user/session token")
|
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="manual ledger requires user/session token")
|
||||||
|
|
||||||
|
|||||||
+53
-7
@@ -219,12 +219,33 @@ class ManualLedgerRequest(BaseModel):
|
|||||||
class ManualLedgerResponse(BaseModel):
|
class ManualLedgerResponse(BaseModel):
|
||||||
ledger_entry_id: int
|
ledger_entry_id: int
|
||||||
portfolio_id: str
|
portfolio_id: str
|
||||||
action: str
|
action: Literal["INVEST", "BUFFER_OUT", "BUFFER_IN", "WITHDRAW", "PROFIT", "BUY", "SELL", "REVERSAL"]
|
||||||
amount: float | Decimal | None = None
|
amount: float | None = None
|
||||||
quantity: float = 0.0
|
quantity: float = 0.0
|
||||||
price: float = 0.0
|
price: float = 0.0
|
||||||
fee: float = 0.0
|
fee: float = 0.0
|
||||||
tax: float | Decimal = 0.0
|
tax: float = 0.0
|
||||||
|
executed_at: datetime | None = None
|
||||||
|
note: str = ""
|
||||||
|
recorded_at: datetime | None = None
|
||||||
|
cash_balance_after: float = 0.0
|
||||||
|
asset_quantity_after: float = 0.0
|
||||||
|
profit_buffer_after: float = 0.0
|
||||||
|
total_value_after: float = 0.0
|
||||||
|
source: str = "manual"
|
||||||
|
target_ledger_entry_id: int | None = None
|
||||||
|
confirmation_evidence: str | None = None
|
||||||
|
|
||||||
|
|
||||||
|
class DividendManualLedgerResponse(BaseModel):
|
||||||
|
ledger_entry_id: int
|
||||||
|
portfolio_id: str
|
||||||
|
action: Literal["DIVIDEND"]
|
||||||
|
amount: str
|
||||||
|
quantity: float = 0.0
|
||||||
|
price: float = 0.0
|
||||||
|
fee: float = 0.0
|
||||||
|
tax: str = "0"
|
||||||
executed_at: datetime | None = None
|
executed_at: datetime | None = None
|
||||||
note: str = ""
|
note: str = ""
|
||||||
recorded_at: datetime | None = None
|
recorded_at: datetime | None = None
|
||||||
@@ -238,7 +259,7 @@ class ManualLedgerResponse(BaseModel):
|
|||||||
|
|
||||||
|
|
||||||
class ManualLedgerHistoryResponse(BaseModel):
|
class ManualLedgerHistoryResponse(BaseModel):
|
||||||
items: list[ManualLedgerResponse]
|
items: list[ManualLedgerResponse | DividendManualLedgerResponse]
|
||||||
total: int
|
total: int
|
||||||
|
|
||||||
|
|
||||||
@@ -254,10 +275,35 @@ class LedgerHistoryItem(BaseModel):
|
|||||||
order_id: str | None = None
|
order_id: str | None = None
|
||||||
execution_id: str | None = None
|
execution_id: str | None = None
|
||||||
quantity: float | None = None
|
quantity: float | None = None
|
||||||
amount: float | Decimal | None = None
|
amount: float | None = None
|
||||||
price: float | None = None
|
price: float | None = None
|
||||||
fee_amount: float | None = None
|
fee_amount: float | None = None
|
||||||
tax_amount: float | Decimal | None = None
|
tax_amount: float | None = None
|
||||||
|
note: str | None = None
|
||||||
|
cash_balance_after: float | None = None
|
||||||
|
asset_quantity_after: float | None = None
|
||||||
|
profit_buffer_after: float | None = None
|
||||||
|
total_value_after: float | None = None
|
||||||
|
target_ledger_entry_id: int | None = None
|
||||||
|
confirmation_evidence: str | None = None
|
||||||
|
|
||||||
|
|
||||||
|
class DividendLedgerHistoryItem(BaseModel):
|
||||||
|
"""Source-neutral, owner-visible ledger fact for GET /ledger."""
|
||||||
|
|
||||||
|
ledger_entry_id: int
|
||||||
|
portfolio_id: str
|
||||||
|
source: str
|
||||||
|
action: Literal["DIVIDEND"]
|
||||||
|
recorded_at: datetime
|
||||||
|
executed_at: datetime | None = None
|
||||||
|
order_id: str | None = None
|
||||||
|
execution_id: str | None = None
|
||||||
|
quantity: float | None = None
|
||||||
|
amount: str
|
||||||
|
price: float | None = None
|
||||||
|
fee_amount: float | None = None
|
||||||
|
tax_amount: str | None = None
|
||||||
note: str | None = None
|
note: str | None = None
|
||||||
cash_balance_after: float | None = None
|
cash_balance_after: float | None = None
|
||||||
asset_quantity_after: float | None = None
|
asset_quantity_after: float | None = None
|
||||||
@@ -268,7 +314,7 @@ class LedgerHistoryItem(BaseModel):
|
|||||||
|
|
||||||
|
|
||||||
class LedgerHistoryResponse(BaseModel):
|
class LedgerHistoryResponse(BaseModel):
|
||||||
items: list[LedgerHistoryItem]
|
items: list[LedgerHistoryItem | DividendLedgerHistoryItem]
|
||||||
next_cursor: str | None = None
|
next_cursor: str | None = None
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -292,7 +292,7 @@ def test_dividend_precision_and_reversal_round_trip_on_real_postgres(
|
|||||||
|
|
||||||
assert dividend.price == 0.00000001
|
assert dividend.price == 0.00000001
|
||||||
assert dividend.tax_amount == 0.00000001
|
assert dividend.tax_amount == 0.00000001
|
||||||
assert str(dividend.tax_amount_decimal) == "0.00000001"
|
assert dividend.tax_amount_decimal == Decimal("0.00000001")
|
||||||
assert dividend.profit_buffer_after == 0.00000001
|
assert dividend.profit_buffer_after == 0.00000001
|
||||||
with repo._connect().cursor() as cur:
|
with repo._connect().cursor() as cur:
|
||||||
cur.execute(
|
cur.execute(
|
||||||
@@ -330,7 +330,7 @@ def test_dividend_tax_omitted_persists_zero_on_real_postgres(
|
|||||||
note="tax-omitted",
|
note="tax-omitted",
|
||||||
)
|
)
|
||||||
assert dividend.tax_amount == 0.0
|
assert dividend.tax_amount == 0.0
|
||||||
assert str(dividend.tax_amount_decimal) == "0E-8"
|
assert dividend.tax_amount_decimal == Decimal("0.00000000")
|
||||||
with repo._connect().cursor() as cur:
|
with repo._connect().cursor() as cur:
|
||||||
cur.execute("SELECT tax_amount::text FROM portfolio_ledger WHERE ledger_entry_id = %s", (dividend.ledger_entry_id,))
|
cur.execute("SELECT tax_amount::text FROM portfolio_ledger WHERE ledger_entry_id = %s", (dividend.ledger_entry_id,))
|
||||||
assert cur.fetchone()[0] == "0.00000000"
|
assert cur.fetchone()[0] == "0.00000000"
|
||||||
@@ -431,7 +431,7 @@ def test_large_invest_and_large_dividend_tax_metadata_are_accepted_when_snapshot
|
|||||||
note="large-tax-metadata",
|
note="large-tax-metadata",
|
||||||
)
|
)
|
||||||
assert dividend.tax_amount == 50000000.0
|
assert dividend.tax_amount == 50000000.0
|
||||||
assert str(dividend.tax_amount_decimal) == "50000000.00000000"
|
assert dividend.tax_amount_decimal == Decimal("50000000.00000000")
|
||||||
repo.close()
|
repo.close()
|
||||||
|
|
||||||
|
|
||||||
@@ -458,8 +458,8 @@ def test_dividend_tax_alias_prone_decimals_round_trip_distinctly_on_real_postgre
|
|||||||
executed_at=datetime(2026, 9, 2, 0, 0, 0, tzinfo=UTC),
|
executed_at=datetime(2026, 9, 2, 0, 0, 0, tzinfo=UTC),
|
||||||
note="alias-tax-2",
|
note="alias-tax-2",
|
||||||
)
|
)
|
||||||
assert str(first.tax_amount_decimal) == "90000000.00000001"
|
assert first.tax_amount_decimal == Decimal("90000000.00000001")
|
||||||
assert str(second.tax_amount_decimal) == "90000000.00000002"
|
assert second.tax_amount_decimal == Decimal("90000000.00000002")
|
||||||
repo.close()
|
repo.close()
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -981,9 +981,12 @@ def test_unified_ledger_openapi_contains_nullable_amount() -> None:
|
|||||||
response = client.get("/openapi.json")
|
response = client.get("/openapi.json")
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
schemas = response.json()["components"]["schemas"]
|
schemas = response.json()["components"]["schemas"]
|
||||||
assert "amount" in schemas["LedgerHistoryItem"]["properties"]
|
manual_amount_types = {option.get("type") for option in schemas["ManualLedgerResponse"]["properties"]["amount"]["anyOf"]}
|
||||||
amount_schema = schemas["ManualLedgerResponse"]["properties"]["amount"]
|
assert manual_amount_types == {"number", "null"}
|
||||||
assert any(option.get("type") == "number" for option in amount_schema.get("anyOf", []))
|
assert schemas["DividendManualLedgerResponse"]["properties"]["amount"]["type"] == "string"
|
||||||
|
history_amount_types = {option.get("type") for option in schemas["LedgerHistoryItem"]["properties"]["amount"]["anyOf"]}
|
||||||
|
assert history_amount_types == {"number", "null"}
|
||||||
|
assert schemas["DividendLedgerHistoryItem"]["properties"]["amount"]["type"] == "string"
|
||||||
|
|
||||||
|
|
||||||
def test_reversal_rejects_fee_tax_and_missing_target() -> None:
|
def test_reversal_rejects_fee_tax_and_missing_target() -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user