Compare commits
No commits in common. "39440b75a7f3a076a522dfa3277c24b66999d20d" and "89f0f8ac3a8cee2014ef502771a8757791f46f89" have entirely different histories.
39440b75a7
...
89f0f8ac3a
5 changed files with 15 additions and 130 deletions
6
crud.py
6
crud.py
|
|
@ -250,13 +250,9 @@ async def get_or_create_user_account(
|
||||||
if not fava_account_exists:
|
if not fava_account_exists:
|
||||||
# Create account in Fava/Beancount via Open directive
|
# Create account in Fava/Beancount via Open directive
|
||||||
logger.info(f"[FAVA CREATE] Creating account in Fava: {account_name}")
|
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(
|
await fava.add_account(
|
||||||
account_name=account_name,
|
account_name=account_name,
|
||||||
currencies=None,
|
currencies=["EUR", "SATS", "USD"], # Support common currencies
|
||||||
metadata={
|
metadata={
|
||||||
"user_id": user_id,
|
"user_id": user_id,
|
||||||
"description": f"User-specific {account_type.value} account"
|
"description": f"User-specific {account_type.value} account"
|
||||||
|
|
|
||||||
|
|
@ -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 <account>` 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:
|
class FavaClient:
|
||||||
"""
|
"""
|
||||||
Async client for Fava REST API.
|
Async client for Fava REST API.
|
||||||
|
|
@ -1668,9 +1644,15 @@ class FavaClient:
|
||||||
source = source_data["source"]
|
source = source_data["source"]
|
||||||
|
|
||||||
# Step 2: Check if account already exists (may have been
|
# Step 2: Check if account already exists (may have been
|
||||||
# created by a concurrent request). See
|
# created by a concurrent request). Anchor on the Open
|
||||||
# _open_directive_exists for the anchoring rationale.
|
# directive and require the account to be followed by
|
||||||
if _open_directive_exists(source, account_name):
|
# 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}")
|
logger.info(f"Account {account_name} already exists in {target_file}")
|
||||||
return {
|
return {
|
||||||
"data": sha256sum,
|
"data": sha256sum,
|
||||||
|
|
|
||||||
|
|
@ -93,32 +93,6 @@ async def test_add_chart_account_duplicate_returns_409(
|
||||||
assert "already exists" in second.json().get("detail", "").lower()
|
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
|
@pytest.mark.anyio
|
||||||
async def test_add_chart_account_invalid_prefix_returns_400(
|
async def test_add_chart_account_invalid_prefix_returns_400(
|
||||||
client, super_user_headers, fava_ledger_path,
|
client, super_user_headers, fava_ledger_path,
|
||||||
|
|
|
||||||
|
|
@ -31,59 +31,9 @@ bf = _module("beancount_format")
|
||||||
au = _module("account_utils")
|
au = _module("account_utils")
|
||||||
val = _module("core.validation")
|
val = _module("core.validation")
|
||||||
mdl = _module("models")
|
mdl = _module("models")
|
||||||
fc = _module("fava_client")
|
|
||||||
AccountType = mdl.AccountType
|
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
|
# beancount_format.sanitize_link
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
|
||||||
19
views_api.py
19
views_api.py
|
|
@ -3750,31 +3750,14 @@ async def api_admin_add_chart_account(
|
||||||
metadata=metadata,
|
metadata=metadata,
|
||||||
)
|
)
|
||||||
|
|
||||||
from .account_sync import sync_single_account_from_beancount
|
|
||||||
|
|
||||||
if result.get("already_existed"):
|
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(
|
raise HTTPException(
|
||||||
status_code=HTTPStatus.CONFLICT,
|
status_code=HTTPStatus.CONFLICT,
|
||||||
detail=f"Account {payload.name} already exists",
|
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,
|
|
||||||
}
|
|
||||||
|
|
||||||
# Mirror into libra DB so permissions / metadata layer sees it.
|
# 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)
|
synced = await sync_single_account_from_beancount(payload.name)
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue