mirror of
https://github.com/Zproger/Aurum.git
synced 2026-10-13 02:56:39 +07:00
fix: don't enforce write-time transfer rules when reading transactions (#7)
TransactionRead inherited TransactionBase's model_validator, so the rules a new row must satisfy were also applied on the way out, to rows already in the database. A row can stop satisfying them through no fault of its own: transactions.transfer_account_id is ON DELETE SET NULL, so deleting an account leaves every transfer that pointed at it without a destination. The result was that deleting an account could 500 the entire transactions list — permanently, and with the list gone there was no way in the UI to find and delete the offending row. The fields move to TransactionFields, which reading uses; TransactionBase keeps the invariants for the write path. The frontend already tolerates a missing destination (TransactionsTable renders the transfer suffix only when transfer_account_id is set, and the edit modal makes you pick a destination before saving), so such a row is now visible, editable and deletable instead of fatal.
This commit is contained in:
@@ -39,7 +39,15 @@ def transfer_rule_violation(
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
class TransactionBase(BaseModel):
|
class TransactionFields(BaseModel):
|
||||||
|
"""A transaction's shape, without the rules that only make sense while
|
||||||
|
writing one. Reading goes through this: rows already in the database can
|
||||||
|
stop satisfying a write-time rule through no fault of their own — the
|
||||||
|
transfer_account_id FK is ON DELETE SET NULL, so deleting an account
|
||||||
|
leaves the transfers that pointed at it with no destination — and
|
||||||
|
refusing to serialize such a row would take the whole transactions list
|
||||||
|
down with it, leaving no way in the UI to find and delete the row."""
|
||||||
|
|
||||||
account_id: int
|
account_id: int
|
||||||
category_id: int | None = None
|
category_id: int | None = None
|
||||||
transfer_account_id: int | None = None
|
transfer_account_id: int | None = None
|
||||||
@@ -57,6 +65,11 @@ class TransactionBase(BaseModel):
|
|||||||
def _capitalize_description(cls, value: str) -> str:
|
def _capitalize_description(cls, value: str) -> str:
|
||||||
return capitalize_first_letter(value)
|
return capitalize_first_letter(value)
|
||||||
|
|
||||||
|
|
||||||
|
class TransactionBase(TransactionFields):
|
||||||
|
"""The write-side shape: the fields plus the invariants a new or edited
|
||||||
|
row has to satisfy."""
|
||||||
|
|
||||||
@model_validator(mode="after")
|
@model_validator(mode="after")
|
||||||
def _validate_type_specific_fields(self) -> "TransactionBase":
|
def _validate_type_specific_fields(self) -> "TransactionBase":
|
||||||
violation = transfer_rule_violation(
|
violation = transfer_rule_violation(
|
||||||
@@ -93,7 +106,7 @@ class TransactionUpdate(BaseModel):
|
|||||||
return capitalize_first_letter(value) if value is not None else None
|
return capitalize_first_letter(value) if value is not None else None
|
||||||
|
|
||||||
|
|
||||||
class TransactionRead(TransactionBase):
|
class TransactionRead(TransactionFields):
|
||||||
model_config = ConfigDict(from_attributes=True)
|
model_config = ConfigDict(from_attributes=True)
|
||||||
|
|
||||||
id: int
|
id: int
|
||||||
|
|||||||
@@ -378,3 +378,26 @@ async def test_update_rejects_a_category_on_a_transfer(client: AsyncClient, acco
|
|||||||
)
|
)
|
||||||
|
|
||||||
assert resp.status_code == 400
|
assert resp.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
|
async def test_a_transfer_whose_destination_account_was_deleted_can_still_be_listed(
|
||||||
|
client: AsyncClient, account_id
|
||||||
|
):
|
||||||
|
"""Deleting an account nulls transfer_account_id on the transfers that
|
||||||
|
pointed at it (the FK is ON DELETE SET NULL), leaving a row that create
|
||||||
|
would reject. Refusing to *serialize* such a row takes the whole
|
||||||
|
transactions list down with a 500 — and with the list gone there is no
|
||||||
|
way left in the UI to delete the row and recover. Reading has to stay
|
||||||
|
possible; only writing is guarded."""
|
||||||
|
other = (await client.post("/accounts", json={"name": "Savings", "type": "savings"})).json()
|
||||||
|
created = await client.post(
|
||||||
|
"/transactions", json=_txn(account_id, type="transfer", transfer_account_id=other["id"], category_id=None)
|
||||||
|
)
|
||||||
|
txn_id = created.json()["id"]
|
||||||
|
assert (await client.delete(f"/accounts/{other['id']}")).status_code == 204
|
||||||
|
|
||||||
|
listing = await client.get("/transactions")
|
||||||
|
|
||||||
|
assert listing.status_code == 200
|
||||||
|
assert txn_id in [item["id"] for item in listing.json()["items"]]
|
||||||
|
assert (await client.delete(f"/transactions/{txn_id}")).status_code == 204
|
||||||
|
|||||||
Reference in New Issue
Block a user