mirror of
https://github.com/Zproger/Aurum.git
synced 2026-10-13 02:56:39 +07:00
fix: actually reject zero-amount goal contributions (#4)
`GoalContributionCreate.amount` used `Field(ne=0)`, but pydantic has no
"not equal" constraint — the keyword was accepted as unknown schema
metadata and never validated anything. `POST /api/goals/{id}/contributions`
with `"amount": "0"` returned 201 and wrote a contribution that moves the
goal's progress by nothing, and the comment right above the field says zero
is pointless.
Replaced with a field_validator so the rule is enforced, deliberately not
with `gt=0`: negative contributions are intentional (money taken back out of
a goal), and the added tests pin both halves of that.
This also clears the `PydanticDeprecatedSince20: Using extra keyword
arguments on Field (Extra keys: 'ne')` warning that the test suite printed
on every run — that warning was the symptom.
Claude-Session: https://claude.ai/code/session_01UmuMDFrwdvQkce885VaaTh
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
from datetime import date as date_
|
||||
from decimal import Decimal
|
||||
|
||||
from pydantic import BaseModel, Field
|
||||
from pydantic import BaseModel, Field, field_validator
|
||||
|
||||
|
||||
class GoalCreate(BaseModel):
|
||||
@@ -19,10 +19,20 @@ class GoalUpdate(BaseModel):
|
||||
class GoalContributionCreate(BaseModel):
|
||||
# Negative allowed — a withdrawal from the goal is still a contribution
|
||||
# to its running total, just in the other direction. Zero is pointless.
|
||||
amount: Decimal = Field(ne=0)
|
||||
amount: Decimal
|
||||
date: date_
|
||||
note: str | None = Field(default=None, max_length=200)
|
||||
|
||||
@field_validator("amount")
|
||||
@classmethod
|
||||
def _reject_zero(cls, value: Decimal) -> Decimal:
|
||||
# Has to be a validator: pydantic has no "not equal" constraint, so
|
||||
# the Field(ne=0) this replaces was parsed as an unknown keyword,
|
||||
# stored as schema metadata, and never actually checked anything.
|
||||
if value == 0:
|
||||
raise ValueError("amount must not be zero")
|
||||
return value
|
||||
|
||||
|
||||
class GoalRead(BaseModel):
|
||||
id: int
|
||||
|
||||
34
backend/tests/test_goals.py
Normal file
34
backend/tests/test_goals.py
Normal file
@@ -0,0 +1,34 @@
|
||||
"""Savings goals: contribution rules.
|
||||
|
||||
The contribution log accepts negative amounts on purpose (taking money back
|
||||
out of a goal is still an entry in its running total), which is exactly why
|
||||
zero has to be rejected explicitly rather than by a `gt=0` constraint.
|
||||
"""
|
||||
from httpx import AsyncClient
|
||||
|
||||
from tests.helpers import money
|
||||
|
||||
|
||||
async def _goal(client: AsyncClient) -> dict:
|
||||
return (await client.post("/goals", json={"name": "Emergency fund", "target_amount": "1000"})).json()
|
||||
|
||||
|
||||
async def test_zero_contribution_is_rejected(client: AsyncClient):
|
||||
goal = await _goal(client)
|
||||
|
||||
resp = await client.post(f"/goals/{goal['id']}/contributions", json={"amount": "0", "date": "2026-01-15"})
|
||||
|
||||
assert resp.status_code == 422
|
||||
assert money((await client.get("/goals")).json()[0]["current_amount"]) == money(0)
|
||||
|
||||
|
||||
async def test_negative_contribution_is_allowed(client: AsyncClient):
|
||||
"""Withdrawing from a goal is a legitimate entry — the zero check must not
|
||||
turn into a positive-only constraint."""
|
||||
goal = await _goal(client)
|
||||
await client.post(f"/goals/{goal['id']}/contributions", json={"amount": "250", "date": "2026-01-15"})
|
||||
|
||||
resp = await client.post(f"/goals/{goal['id']}/contributions", json={"amount": "-100", "date": "2026-02-01"})
|
||||
|
||||
assert resp.status_code == 201
|
||||
assert money(resp.json()["current_amount"]) == money("150")
|
||||
Reference in New Issue
Block a user