fix: Neue Tabellen ins Backup, toter Transfer-Code raus
Sieben nutzereigene Tabellen aus den Migrationen 0015-0020 standen in keiner Backup-Liste. Am schwersten wiegt inbox_items: Der Eingang haelt fest, welche Zahlungen von aussen schon erfasst wurden. Fehlt er nach dem Wiedereinspielen, erkennt der naechste Kontoauszug sie nicht wieder und bucht sie ein zweites Mal – genau der Fehler, den die Tabelle verhindern soll. Aufgenommen: inbox_items, transfer_links, link_ignored, api_tokens. Bewusst nicht: mail_sources/mail_senders (tragen das verschluesselte IMAP-Passwort; ein Backup ist eine Datei, die man herumreicht) und pending_uploads (fluechtige Rohdateien). Beides steht begruendet im Code. Ein Test laeuft jetzt ueber Base.registry und fordert fuer jedes Modell mit owner_id eine bewusste Entscheidung – sichern oder begruendet ausnehmen. Genau diese Pruefung fehlte, weshalb die sieben unbemerkt durchrutschten. Dazu der tote Code aus dem Transfer-Ausbau: is_counterpart und braucht_ziel waren feste False, _apply_counterpart_amount damit unerreichbar. Die Tests sichern dieselbe Garantie jetzt ueber das tatsaechliche Verhalten. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -35,6 +35,7 @@ from ..branding import VERSION
|
|||||||
from ..models import (
|
from ..models import (
|
||||||
Account,
|
Account,
|
||||||
AccountIdentifier,
|
AccountIdentifier,
|
||||||
|
ApiToken,
|
||||||
AppSetting,
|
AppSetting,
|
||||||
BudgetPeriod,
|
BudgetPeriod,
|
||||||
BudgetPeriodLine,
|
BudgetPeriodLine,
|
||||||
@@ -46,9 +47,12 @@ from ..models import (
|
|||||||
ExchangeRate,
|
ExchangeRate,
|
||||||
ImportBatch,
|
ImportBatch,
|
||||||
ImportTemplate,
|
ImportTemplate,
|
||||||
|
InboxItem,
|
||||||
|
LinkIgnored,
|
||||||
PlannedIncomeLine,
|
PlannedIncomeLine,
|
||||||
RecoveryCode,
|
RecoveryCode,
|
||||||
Transaction,
|
Transaction,
|
||||||
|
TransferLink,
|
||||||
User,
|
User,
|
||||||
UserSetting,
|
UserSetting,
|
||||||
)
|
)
|
||||||
@@ -113,8 +117,39 @@ USER_TABLES: list[TableSpec] = [
|
|||||||
{"category_id": "categories"}, label="Schlagwortregeln"),
|
{"category_id": "categories"}, label="Schlagwortregeln"),
|
||||||
TableSpec("user_settings", UserSetting, "user", has_id=False,
|
TableSpec("user_settings", UserSetting, "user", has_id=False,
|
||||||
label="Persönliche Einstellungen"),
|
label="Persönliche Einstellungen"),
|
||||||
|
# Der Eingang MUSS mitgesichert werden: Er hält fest, welche Zahlungen
|
||||||
|
# bereits von aussen erfasst wurden. Fehlt er nach dem Wiedereinspielen,
|
||||||
|
# erkennt der nächste Kontoauszug sie nicht wieder und bucht sie ein
|
||||||
|
# zweites Mal – genau der Fehler, den diese Tabelle verhindern soll.
|
||||||
|
TableSpec("inbox_items", InboxItem, "owner", {
|
||||||
|
"account_id": "accounts",
|
||||||
|
"category_id": "categories",
|
||||||
|
"transaction_id": "transactions",
|
||||||
|
}, label="Eingang"),
|
||||||
|
TableSpec("transfer_links", TransferLink, "owner",
|
||||||
|
{"from_txn_id": "transactions", "to_txn_id": "transactions"},
|
||||||
|
label="Übertrags-Verknüpfungen"),
|
||||||
|
TableSpec("link_ignored", LinkIgnored, "owner",
|
||||||
|
{"a_txn_id": "transactions", "b_txn_id": "transactions"},
|
||||||
|
label="Abgelehnte Übertrags-Vorschläge"),
|
||||||
|
# Ohne die Geräteschlüssel hört der Kurzbefehl auf dem Telefon nach einem
|
||||||
|
# Wiedereinspielen stillschweigend auf zu funktionieren. Gesichert wird nur
|
||||||
|
# der Hash – der Klartext existiert nirgends.
|
||||||
|
TableSpec("api_tokens", ApiToken, "owner",
|
||||||
|
{"default_account_id": "accounts"}, label="Geräteschlüssel"),
|
||||||
]
|
]
|
||||||
|
|
||||||
|
# Bewusst NICHT gesichert:
|
||||||
|
#
|
||||||
|
# ``mail_sources``/``mail_senders`` – die Quelle trägt das verschlüsselte
|
||||||
|
# IMAP-Passwort. Ein Backup ist eine Datei, die der Nutzer herumreicht; dort
|
||||||
|
# gehört keine Zugangsdatei hinein, auch keine verschlüsselte. Nach einem
|
||||||
|
# Wiedereinspielen richtet man das Postfach neu ein, das dauert zwei Minuten.
|
||||||
|
#
|
||||||
|
# ``pending_uploads`` – dort liegen rohe Dateien, die noch niemand geprüft hat.
|
||||||
|
# Sie sind flüchtig, blähen ein JSON-Backup um Megabytes auf und lassen sich
|
||||||
|
# jederzeit erneut vom Telefon schicken.
|
||||||
|
|
||||||
# Nur im systemweiten Backup, je Benutzer.
|
# Nur im systemweiten Backup, je Benutzer.
|
||||||
SYSTEM_USER_TABLES: list[TableSpec] = [
|
SYSTEM_USER_TABLES: list[TableSpec] = [
|
||||||
TableSpec("recovery_codes", RecoveryCode, "user", label="Wiederherstellungscodes"),
|
TableSpec("recovery_codes", RecoveryCode, "user", label="Wiederherstellungscodes"),
|
||||||
|
|||||||
@@ -766,9 +766,6 @@ def detect_rows(db: Session, user: User, account: Account, candidates: list[dict
|
|||||||
is_dup = True
|
is_dup = True
|
||||||
dup_grund = "inbox"
|
dup_grund = "inbox"
|
||||||
|
|
||||||
is_counterpart = False
|
|
||||||
braucht_ziel = False
|
|
||||||
|
|
||||||
out.append({
|
out.append({
|
||||||
"index": i,
|
"index": i,
|
||||||
"date": d.isoformat() if d else None,
|
"date": d.isoformat() if d else None,
|
||||||
@@ -791,11 +788,9 @@ def detect_rows(db: Session, user: User, account: Account, candidates: list[dict
|
|||||||
"import_hash": h,
|
"import_hash": h,
|
||||||
"is_duplicate": is_dup,
|
"is_duplicate": is_dup,
|
||||||
"duplicate_reason": dup_grund,
|
"duplicate_reason": dup_grund,
|
||||||
"is_counterpart": is_counterpart,
|
|
||||||
# Damit commit_import die geschätzte Gegenseite auf den echten Wert setzt.
|
# Damit commit_import die geschätzte Gegenseite auf den echten Wert setzt.
|
||||||
"counterpart_txn_id": None,
|
"counterpart_txn_id": None,
|
||||||
"include": valid and not is_dup and not is_counterpart and not braucht_ziel,
|
"include": valid and not is_dup,
|
||||||
"needs_target": braucht_ziel,
|
|
||||||
})
|
})
|
||||||
return out
|
return out
|
||||||
|
|
||||||
@@ -822,7 +817,7 @@ def reconcile(rows: list[dict]) -> dict:
|
|||||||
einer früher gebuchten Buchung. Eine Zeile, die auf ein Gegenkonto wartet,
|
einer früher gebuchten Buchung. Eine Zeile, die auf ein Gegenkonto wartet,
|
||||||
zählt dagegen NICHT – sie ist unterwegs verloren, und genau das soll der
|
zählt dagegen NICHT – sie ist unterwegs verloren, und genau das soll der
|
||||||
Abgleich melden."""
|
Abgleich melden."""
|
||||||
if not r.get("valid") or r.get("needs_target"):
|
if not r.get("valid"):
|
||||||
return 0
|
return 0
|
||||||
return r.get("amount_cents") or 0
|
return r.get("amount_cents") or 0
|
||||||
|
|
||||||
@@ -991,34 +986,6 @@ def _track_and_learn(db: Session, user: User, row: dict) -> None:
|
|||||||
_learn_rule(db, user, row)
|
_learn_rule(db, user, row)
|
||||||
|
|
||||||
|
|
||||||
def _apply_counterpart_amount(db: Session, user: User, account: Account, row: dict) -> bool:
|
|
||||||
"""Trägt den tatsächlichen Betrag einer Umtausch-Gegenbuchung in den bereits
|
|
||||||
gebuchten Transfer ein. Diese Zeile stammt aus dem Auszug der anderen Seite und
|
|
||||||
nennt exakt, was dort angekommen bzw. abgegangen ist."""
|
|
||||||
txn_id = row.get("counterpart_txn_id")
|
|
||||||
amount = row.get("amount_cents")
|
|
||||||
if not txn_id or not amount or not row.get("is_counterpart"):
|
|
||||||
return False
|
|
||||||
txn = db.get(Transaction, txn_id)
|
|
||||||
if txn is None or txn.owner_id != user.id or txn.type != TxnType.transfer:
|
|
||||||
return False
|
|
||||||
absamt = abs(amount)
|
|
||||||
if txn.to_account_id == account.id: # hier ist das Geld angekommen
|
|
||||||
if txn.to_amount_cents == absamt:
|
|
||||||
return False
|
|
||||||
txn.to_amount_cents = absamt
|
|
||||||
elif txn.account_id == account.id: # hier ist es abgegangen
|
|
||||||
if txn.amount_cents == absamt:
|
|
||||||
return False
|
|
||||||
txn.amount_cents = absamt
|
|
||||||
txn.amount_eur_cents = currency.compute_amount_eur_cents(
|
|
||||||
db, absamt, account.currency, txn.date
|
|
||||||
)
|
|
||||||
else:
|
|
||||||
return False
|
|
||||||
return True
|
|
||||||
|
|
||||||
|
|
||||||
def commit_import(db: Session, user: User, account: Account, template_id: int | None, filename: str,
|
def commit_import(db: Session, user: User, account: Account, template_id: int | None, filename: str,
|
||||||
rows: list[dict], preset: str | None = None) -> ImportBatch:
|
rows: list[dict], preset: str | None = None) -> ImportBatch:
|
||||||
accounts = {a.id: a for a in db.query(Account).filter(Account.owner_id == user.id)}
|
accounts = {a.id: a for a in db.query(Account).filter(Account.owner_id == user.id)}
|
||||||
@@ -1031,14 +998,8 @@ def commit_import(db: Session, user: User, account: Account, template_id: int |
|
|||||||
db.flush()
|
db.flush()
|
||||||
|
|
||||||
imported = 0
|
imported = 0
|
||||||
korrigiert = 0
|
|
||||||
for row in rows:
|
for row in rows:
|
||||||
if not row.get("include", True):
|
if not row.get("include", True):
|
||||||
# Gegenbuchung eines Umtauschs: nicht buchen, aber den echten Betrag
|
|
||||||
# dieser Seite in den bestehenden Transfer eintragen. Sonst bliebe dort
|
|
||||||
# die EZB-Schätzung stehen und der Saldo wiche um die Kursdifferenz ab.
|
|
||||||
if _apply_counterpart_amount(db, user, account, row):
|
|
||||||
korrigiert += 1
|
|
||||||
continue
|
continue
|
||||||
txn = _make_transaction(db, user, account, accounts, row, batch.id)
|
txn = _make_transaction(db, user, account, accounts, row, batch.id)
|
||||||
if txn is None:
|
if txn is None:
|
||||||
|
|||||||
@@ -52,3 +52,37 @@ def test_db_backup_admin_only(admin):
|
|||||||
assert other.get("/api/backup/db").status_code == 403
|
assert other.get("/api/backup/db").status_code == 403
|
||||||
# ...aber seinen eigenen Export schon (leer).
|
# ...aber seinen eigenen Export schon (leer).
|
||||||
assert other.get("/api/export/all.json").status_code == 200
|
assert other.get("/api/export/all.json").status_code == 200
|
||||||
|
|
||||||
|
|
||||||
|
def test_jede_nutzertabelle_ist_im_backup_beruecksichtigt():
|
||||||
|
"""Neue Tabellen dürfen nicht stillschweigend aus dem Backup fallen.
|
||||||
|
|
||||||
|
Sieben Tabellen sind genau so entstanden: angelegt, migriert, benutzt – und
|
||||||
|
in ``backup_data`` nie eingetragen. Aufgefallen wäre das erst beim
|
||||||
|
Wiedereinspielen, also im schlechtesten Moment. Dieser Test zwingt jede
|
||||||
|
künftige Tabelle mit ``owner_id`` zu einer bewussten Entscheidung: entweder
|
||||||
|
sichern oder hier begründet ausnehmen.
|
||||||
|
"""
|
||||||
|
from app.models import Base
|
||||||
|
from app.services import backup_data
|
||||||
|
|
||||||
|
gesichert = {
|
||||||
|
spec.model.__tablename__
|
||||||
|
for liste in (backup_data.USER_TABLES, backup_data.SYSTEM_USER_TABLES,
|
||||||
|
backup_data.GLOBAL_TABLES)
|
||||||
|
for spec in liste
|
||||||
|
}
|
||||||
|
# Bewusst ausgenommen – die Begründung steht in backup_data.py.
|
||||||
|
ausgenommen = {"mail_sources", "mail_senders", "pending_uploads"}
|
||||||
|
|
||||||
|
fehlend = sorted(
|
||||||
|
m.class_.__tablename__
|
||||||
|
for m in Base.registry.mappers
|
||||||
|
if "owner_id" in m.class_.__table__.columns
|
||||||
|
and m.class_.__tablename__ not in gesichert
|
||||||
|
and m.class_.__tablename__ not in ausgenommen
|
||||||
|
)
|
||||||
|
assert not fehlend, (
|
||||||
|
f"Diese Tabellen gehören dem Nutzer, stehen aber in keiner Backup-Liste: "
|
||||||
|
f"{fehlend}. Entweder in USER_TABLES aufnehmen oder oben begründet ausnehmen."
|
||||||
|
)
|
||||||
|
|||||||
@@ -341,7 +341,8 @@ def test_umtausch_wird_als_ausgabe_und_einnahme_gebucht(admin):
|
|||||||
# Der EUR-Auszug bringt den Zugang – als Einnahme, nicht als Gegenbuchung.
|
# Der EUR-Auszug bringt den Zugang – als Einnahme, nicht als Gegenbuchung.
|
||||||
rows2 = _rev_preview(admin, eur["id"], EUR_UMTAUSCH)
|
rows2 = _rev_preview(admin, eur["id"], EUR_UMTAUSCH)
|
||||||
assert rows2[0]["type"] == "income"
|
assert rows2[0]["type"] == "income"
|
||||||
assert rows2[0]["is_counterpart"] is False
|
assert rows2[0]["type"] != "transfer" # keine Kontoverknuepfung mehr
|
||||||
|
assert rows2[0]["include"] is True
|
||||||
assert rows2[0]["include"] is True
|
assert rows2[0]["include"] is True
|
||||||
r = admin.post("/api/imports/commit", json={"account_id": eur["id"], "filename": "eur.csv", "rows": rows2})
|
r = admin.post("/api/imports/commit", json={"account_id": eur["id"], "filename": "eur.csv", "rows": rows2})
|
||||||
assert r.json()["imported_count"] == 1
|
assert r.json()["imported_count"] == 1
|
||||||
@@ -367,7 +368,8 @@ def test_uebertrags_schlagwort_setzt_neutrale_kategorie(admin):
|
|||||||
rows = _rev_preview(admin, chf["id"], CHF_UMTAUSCH)
|
rows = _rev_preview(admin, chf["id"], CHF_UMTAUSCH)
|
||||||
assert rows[0]["type"] == "expense"
|
assert rows[0]["type"] == "expense"
|
||||||
assert rows[0]["category_id"] == neutral["id"]
|
assert rows[0]["category_id"] == neutral["id"]
|
||||||
assert rows[0]["needs_target"] is False
|
assert rows[0]["type"] != "transfer"
|
||||||
|
assert rows[0]["include"] is True
|
||||||
|
|
||||||
|
|
||||||
def test_expense_is_not_swallowed_by_incoming_transfer(admin):
|
def test_expense_is_not_swallowed_by_incoming_transfer(admin):
|
||||||
@@ -399,7 +401,7 @@ def test_expense_is_not_swallowed_by_incoming_transfer(admin):
|
|||||||
).json()["rows"]
|
).json()["rows"]
|
||||||
|
|
||||||
# Keine Zeile gilt mehr als Gegenbuchung – beide werden gebucht.
|
# Keine Zeile gilt mehr als Gegenbuchung – beide werden gebucht.
|
||||||
assert all(r["is_counterpart"] is False for r in rows)
|
assert all(r["type"] != "transfer" for r in rows)
|
||||||
assert all(r["include"] is True for r in rows)
|
assert all(r["include"] is True for r in rows)
|
||||||
assert rows[0]["type"] == "expense"
|
assert rows[0]["type"] == "expense"
|
||||||
assert rows[1]["type"] == "income"
|
assert rows[1]["type"] == "income"
|
||||||
@@ -512,7 +514,7 @@ def test_keine_zeile_bleibt_wegen_fehlendem_gegenkonto_liegen(admin):
|
|||||||
},
|
},
|
||||||
).json()
|
).json()
|
||||||
|
|
||||||
assert not [r for r in res["rows"] if r.get("needs_target")]
|
assert all(r["type"] != "transfer" for r in res["rows"])
|
||||||
uebertrag = res["rows"][1]
|
uebertrag = res["rows"][1]
|
||||||
assert uebertrag["type"] == "income" and uebertrag["include"] is True
|
assert uebertrag["type"] == "income" and uebertrag["include"] is True
|
||||||
|
|
||||||
|
|||||||
@@ -253,7 +253,7 @@ def test_import_verbindet_keine_konten_mehr(admin):
|
|||||||
rows = _preview_csv(admin, giro["id"], csv, COLMAP)
|
rows = _preview_csv(admin, giro["id"], csv, COLMAP)
|
||||||
assert [r["type"] for r in rows] == ["expense", "expense", "income"]
|
assert [r["type"] for r in rows] == ["expense", "expense", "income"]
|
||||||
assert all(r["to_account_id"] is None for r in rows)
|
assert all(r["to_account_id"] is None for r in rows)
|
||||||
assert all(r["needs_target"] is False for r in rows)
|
assert all(r["type"] != "transfer" for r in rows)
|
||||||
|
|
||||||
|
|
||||||
def test_beide_auszuege_werden_beide_gebucht(admin):
|
def test_beide_auszuege_werden_beide_gebucht(admin):
|
||||||
@@ -276,7 +276,8 @@ def test_beide_auszuege_werden_beide_gebucht(admin):
|
|||||||
rowsB = _preview_csv(admin, b["id"],
|
rowsB = _preview_csv(admin, b["id"],
|
||||||
"Datum;Betrag;Empfaenger;Zweck\n07.03.2026;100,00;Gutschrift;von KontoA\n", COLMAP)
|
"Datum;Betrag;Empfaenger;Zweck\n07.03.2026;100,00;Gutschrift;von KontoA\n", COLMAP)
|
||||||
assert rowsB[0]["type"] == "income"
|
assert rowsB[0]["type"] == "income"
|
||||||
assert rowsB[0]["is_counterpart"] is False
|
assert rowsB[0]["type"] != "transfer"
|
||||||
|
assert rowsB[0]["include"] is True
|
||||||
assert rowsB[0]["include"] is True
|
assert rowsB[0]["include"] is True
|
||||||
r = admin.post("/api/imports/commit", json={"account_id": b["id"], "filename": "b.csv", "rows": rowsB})
|
r = admin.post("/api/imports/commit", json={"account_id": b["id"], "filename": "b.csv", "rows": rowsB})
|
||||||
assert r.json()["imported_count"] == 1
|
assert r.json()["imported_count"] == 1
|
||||||
|
|||||||
Reference in New Issue
Block a user