diff --git a/crud.py b/crud.py index 0692806..b2b43dd 100644 --- a/crud.py +++ b/crud.py @@ -250,13 +250,9 @@ async def get_or_create_user_account( if not fava_account_exists: # Create account in Fava/Beancount via Open directive logger.info(f"[FAVA CREATE] Creating account in Fava: {account_name}") - # Unconstrained Open: a per-user receivable/payable legitimately - # holds arbitrary fiat (CAD/GBP/JPY/…). Constraining it to - # EUR/SATS/USD made any posting in another currency fail - # bean-check (the errors this account path originally exhibited). await fava.add_account( account_name=account_name, - currencies=None, + currencies=["EUR", "SATS", "USD"], # Support common currencies metadata={ "user_id": user_id, "description": f"User-specific {account_type.value} account" diff --git a/fava_client.py b/fava_client.py index 053ed0d..f176031 100644 --- a/fava_client.py +++ b/fava_client.py @@ -60,30 +60,6 @@ def _escape_beancount_string(value: str) -> str: ) -def _open_directive_exists(source: str, account_name: str) -> bool: - """Return True if `source` already contains an Open directive for exactly - `account_name`. - - Anchored to a real `YYYY-MM-DD open ` directive line (re.MULTILINE) - so the account name can't match text inside another account's description - metadata or a comment (false positive → spurious 409). The trailing - negative-lookahead `(?![\\w:-])` requires the next char not to be an - account-continuation char, so: - - a prefix (Expenses:Gas) does not match a longer sibling - (Expenses:GasStation / Expenses:Gas:Vehicle), and - - a real directive with an inline comment and no space - (`open Expenses:Gas;legacy`) is still detected (`;` ends the name), - which the previous `(?:\\s|$)` boundary missed → duplicate write. - """ - return bool( - re.search( - rf"^\d{{4}}-\d{{2}}-\d{{2}} open {re.escape(account_name)}(?![\w:-])", - source, - re.MULTILINE, - ) - ) - - class FavaClient: """ Async client for Fava REST API. @@ -1668,9 +1644,15 @@ class FavaClient: source = source_data["source"] # Step 2: Check if account already exists (may have been - # created by a concurrent request). See - # _open_directive_exists for the anchoring rationale. - if _open_directive_exists(source, account_name): + # created by a concurrent request). Anchor on the Open + # directive and require the account to be followed by + # whitespace/end-of-line so a prefix (Expenses:Gas) does + # not match a longer sibling (Expenses:GasStation). + if re.search( + rf"open {re.escape(account_name)}(?:\s|$)", + source, + re.MULTILINE, + ): logger.info(f"Account {account_name} already exists in {target_file}") return { "data": sha256sum, diff --git a/tests/test_admin_chart_accounts_api.py b/tests/test_admin_chart_accounts_api.py index 023835f..1f574f9 100644 --- a/tests/test_admin_chart_accounts_api.py +++ b/tests/test_admin_chart_accounts_api.py @@ -93,32 +93,6 @@ async def test_add_chart_account_duplicate_returns_409( assert "already exists" in second.json().get("detail", "").lower() -@pytest.mark.anyio -async def test_add_chart_account_recovers_ledger_only_account( - client, super_user_headers, -): - """An account present in the ledger but absent from libra's DB (prior sync - failure / out-of-band edit) is recovered (synced), not 409'd — otherwise it - would be permanently un-grantable with no path back. - - Reproduce the ledger-only state by creating normally (so Fava parses the - Open) then deleting only the libra-DB row — appending to the ledger file - directly would race Fava's parse cache.""" - from ..crud import db # the same singleton the app uses - - name = _unique("Expenses:Recover") - first = await add_chart_account(client, super_user_headers=super_user_headers, name=name) - assert first.status_code == 201, f"setup create failed: {first.status_code} {first.text}" - - await db.execute("DELETE FROM accounts WHERE name = :name", {"name": name}) - - r = await add_chart_account(client, super_user_headers=super_user_headers, name=name) - assert r.status_code == 201, f"expected 201 recovery, got {r.status_code}: {r.text}" - body = r.json() - assert body.get("already_existed") is True, body - assert body["synced_to_libra_db"] is True, body - - @pytest.mark.anyio async def test_add_chart_account_invalid_prefix_returns_400( client, super_user_headers, fava_ledger_path, diff --git a/tests/test_unit.py b/tests/test_unit.py index 4f97b03..2fa41ec 100644 --- a/tests/test_unit.py +++ b/tests/test_unit.py @@ -31,59 +31,9 @@ bf = _module("beancount_format") au = _module("account_utils") val = _module("core.validation") mdl = _module("models") -fc = _module("fava_client") AccountType = mdl.AccountType -# --------------------------------------------------------------------------- -# fava_client._open_directive_exists — duplicate-account detection -# --------------------------------------------------------------------------- - - -def test_open_directive_exists_matches_real_directive(): - src = "2020-01-01 open Expenses:Vehicle:Gas\n" - assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True - - -def test_open_directive_exists_matches_currency_constrained_and_metadata(): - src = ( - "2020-01-01 open Expenses:Vehicle:Gas EUR, SATS\n" - ' added_by: "abc"\n' - ) - assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True - - -def test_open_directive_exists_matches_inline_comment_without_space(): - # Valid Beancount: the account token ends at ';'. The old (?:\\s|$) boundary - # missed this → duplicate Open written → bean-check breaks. - src = "2020-01-01 open Expenses:Vehicle:Gas;legacy-import\n" - assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True - - -def test_open_directive_exists_ignores_name_inside_description(): - # The name appears only inside another account's description metadata. - src = ( - "2020-01-01 open Expenses:Notes\n" - ' description: "remember to open Expenses:Vehicle:Gas next month"\n' - ) - assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False - - -def test_open_directive_exists_ignores_comment_line(): - src = "; TODO: open Expenses:Vehicle:Gas eventually\n" - assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False - - -def test_open_directive_exists_does_not_match_longer_sibling(): - src = "2020-01-01 open Expenses:Vehicle:GasStation\n" - assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False - - -def test_open_directive_exists_does_not_match_deeper_child(): - src = "2020-01-01 open Expenses:Vehicle:Gas:Premium\n" - assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False - - # --------------------------------------------------------------------------- # beancount_format.sanitize_link # --------------------------------------------------------------------------- diff --git a/views_api.py b/views_api.py index 1b3149a..3e8647a 100644 --- a/views_api.py +++ b/views_api.py @@ -3750,31 +3750,14 @@ async def api_admin_add_chart_account( metadata=metadata, ) - from .account_sync import sync_single_account_from_beancount - if result.get("already_existed"): - # The Open directive is already in the ledger. If it's also already - # mirrored into libra's DB, it's a true duplicate → 409. If not (a prior - # sync failed — there's no cross-DB atomicity — or it was opened out of - # band), mirror it now so it becomes grantable instead of being stranded - # with no recovery path. - from .crud import get_account_by_name - - if await get_account_by_name(payload.name) is not None: - raise HTTPException( - status_code=HTTPStatus.CONFLICT, - detail=f"Account {payload.name} already exists", - ) - - synced = await sync_single_account_from_beancount(payload.name) - return { - "success": True, - "account_name": payload.name, - "synced_to_libra_db": synced, - "already_existed": True, - } + raise HTTPException( + status_code=HTTPStatus.CONFLICT, + detail=f"Account {payload.name} already exists", + ) # Mirror into libra DB so permissions / metadata layer sees it. + from .account_sync import sync_single_account_from_beancount synced = await sync_single_account_from_beancount(payload.name) return {