fix: enforce transfer invariants when updating a transaction (#3)

The transfer rules — destination required, destination different from the
source account, no category, and no destination on a non-transfer — were
enforced by TransactionBase's validator on create, but the PATCH route only
re-checked category-vs-type.

So `PATCH /api/transactions/{id} {"type": "transfer"}` on an ordinary
expense committed a transfer row with transfer_account_id = NULL, and only
then failed serializing the response (TransactionRead inherits the same
validator). The caller got a 500 and would reasonably assume nothing was
saved — but from that point on every `GET /api/transactions` failed the same
validation, so the transactions list, the app's main screen, returned 500
permanently, with no way left in the UI to delete the offending row.

The rules now live in one place, transfer_rule_violation(), used by both the
create schema and the update route, so the two paths can't drift apart
again. The update path validates the row as it would look after the patch
rather than only the fields sent, since switching type alone invalidates
fields that were left untouched.

The frontend already sends a complete payload on edit (explicit nulls for
category_id / transfer_account_id) and does these same checks client-side,
so this only closes the hole for direct API calls — no UI change needed.


Claude-Session: https://claude.ai/code/session_01UmuMDFrwdvQkce885VaaTh

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
technosexual
2026-08-30 02:42:37 +03:00
committed by GitHub
parent c95eb12d57
commit 949d2df37f
3 changed files with 118 additions and 8 deletions

View File

@@ -18,6 +18,7 @@ from app.schemas.transaction import (
TransactionPage,
TransactionRead,
TransactionUpdate,
transfer_rule_violation,
)
router = APIRouter(prefix="/transactions", tags=["transactions"])
@@ -193,9 +194,22 @@ async def update_transaction(
if transaction is None:
raise HTTPException(status_code=404, detail="Transaction not found")
updates = payload.model_dump(exclude_unset=True, exclude={"tag_ids"})
# Checks run against the row as it would look after the patch, not just
# the fields sent: switching type alone can invalidate fields left
# untouched.
effective_type = updates.get("type", transaction.type)
effective_category_id = updates.get("category_id", transaction.category_id)
effective_account_id = updates.get("account_id", transaction.account_id)
effective_transfer_account_id = updates.get("transfer_account_id", transaction.transfer_account_id)
await _ensure_category_matches_type(session, effective_category_id, effective_type)
violation = transfer_rule_violation(
type=effective_type,
account_id=effective_account_id,
transfer_account_id=effective_transfer_account_id,
category_id=effective_category_id,
)
if violation:
raise HTTPException(status_code=400, detail=violation)
for field, value in updates.items():
setattr(transaction, field, value)
if payload.tag_ids is not None: