Split views_api into a domain package; unshadow /accounts/hierarchy #61
2 changed files with 105 additions and 88 deletions
fix(accounts): register /accounts/hierarchy before /accounts/{account_id}
FastAPI matches in registration order, and hierarchy was registered
~3,300 lines after the {account_id} route — every request resolved as
account_id="hierarchy" and 404'd, so the endpoint has been unreachable
since it was added. Moved above the param routes in the accounts
module, with a functional reachability test and an updated route
snapshot (literal-before-param is now asserted for both overlap
families).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
commit
a0166d4026
|
|
@ -10,14 +10,11 @@ Regenerate after a DELIBERATE routing change with:
|
||||||
from libra.views_api import libra_api_router
|
from libra.views_api import libra_api_router
|
||||||
for r in libra_api_router.routes:
|
for r in libra_api_router.routes:
|
||||||
print((",".join(sorted(r.methods)), r.path, r.endpoint.__name__))
|
print((",".join(sorted(r.methods)), r.path, r.endpoint.__name__))
|
||||||
|
|
||||||
Note the one known wart this table records: /api/v1/accounts/hierarchy
|
|
||||||
is registered AFTER /api/v1/accounts/{account_id}, so it is shadowed
|
|
||||||
(requests resolve as account_id="hierarchy"). Fixing that is a
|
|
||||||
deliberate behavior change with its own commit + snapshot update.
|
|
||||||
"""
|
"""
|
||||||
import importlib
|
import importlib
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
|
||||||
def _module(name: str):
|
def _module(name: str):
|
||||||
for prefix in ("lnbits.extensions.libra", "libra"):
|
for prefix in ("lnbits.extensions.libra", "libra"):
|
||||||
|
|
@ -34,10 +31,10 @@ EXPECTED_ROUTES = [
|
||||||
("GET", "/api/v1/currencies", "api_get_currencies"),
|
("GET", "/api/v1/currencies", "api_get_currencies"),
|
||||||
("GET", "/api/v1/accounts", "api_get_accounts"),
|
("GET", "/api/v1/accounts", "api_get_accounts"),
|
||||||
("POST", "/api/v1/accounts", "api_create_account"),
|
("POST", "/api/v1/accounts", "api_create_account"),
|
||||||
|
("GET", "/api/v1/accounts/hierarchy", "api_get_account_hierarchy"),
|
||||||
("GET", "/api/v1/accounts/{account_id}", "api_get_account"),
|
("GET", "/api/v1/accounts/{account_id}", "api_get_account"),
|
||||||
("GET", "/api/v1/accounts/{account_id}/balance", "api_get_account_balance"),
|
("GET", "/api/v1/accounts/{account_id}/balance", "api_get_account_balance"),
|
||||||
("GET", "/api/v1/accounts/{account_id}/transactions", "api_get_account_transactions"),
|
("GET", "/api/v1/accounts/{account_id}/transactions", "api_get_account_transactions"),
|
||||||
("GET", "/api/v1/accounts/hierarchy", "api_get_account_hierarchy"),
|
|
||||||
("GET", "/api/v1/entries", "api_get_journal_entries"),
|
("GET", "/api/v1/entries", "api_get_journal_entries"),
|
||||||
("GET", "/api/v1/entries/user", "api_get_user_entries"),
|
("GET", "/api/v1/entries/user", "api_get_user_entries"),
|
||||||
("GET", "/api/v1/entries/pending", "api_get_pending_entries"),
|
("GET", "/api/v1/entries/pending", "api_get_pending_entries"),
|
||||||
|
|
@ -125,11 +122,26 @@ def test_overlapping_route_order_is_preserved():
|
||||||
these are the two overlap families in the table. The package split
|
these are the two overlap families in the table. The package split
|
||||||
keeps each family inside one module so include order can't reorder
|
keeps each family inside one module so include order can't reorder
|
||||||
them."""
|
them."""
|
||||||
# Known wart carried over from before the split: hierarchy is
|
# The literal must precede the {param} sibling or it is unreachable.
|
||||||
# shadowed by the {account_id} route (fixed in a separate commit).
|
assert _index("/api/v1/accounts/hierarchy") < _index(
|
||||||
assert _index("/api/v1/accounts/{account_id}") < _index(
|
"/api/v1/accounts/{account_id}"
|
||||||
"/api/v1/accounts/hierarchy"
|
|
||||||
)
|
)
|
||||||
assert _index("/api/v1/admin/accounts/sync") < _index(
|
assert _index("/api/v1/admin/accounts/sync") < _index(
|
||||||
"/api/v1/admin/accounts/sync/{account_name:path}"
|
"/api/v1/admin/accounts/sync/{account_name:path}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_accounts_hierarchy_is_reachable(
|
||||||
|
client, configured_user, standard_accounts,
|
||||||
|
):
|
||||||
|
"""GET /accounts/hierarchy must reach the hierarchy endpoint. It was
|
||||||
|
registered after /accounts/{account_id} since it was added, so every
|
||||||
|
request resolved as account_id="hierarchy" and 404'd."""
|
||||||
|
_, wallet = configured_user
|
||||||
|
r = await client.get(
|
||||||
|
"/libra/api/v1/accounts/hierarchy",
|
||||||
|
headers={"X-Api-Key": wallet.inkey},
|
||||||
|
)
|
||||||
|
assert r.status_code == 200, f"hierarchy shadowed again? {r.status_code} {r.text}"
|
||||||
|
assert isinstance(r.json(), list)
|
||||||
|
|
|
||||||
|
|
@ -171,84 +171,10 @@ async def api_create_account(
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@router.get("/api/v1/accounts/{account_id}")
|
# NOTE: hierarchy must be registered BEFORE /accounts/{account_id} —
|
||||||
async def api_get_account(
|
# FastAPI matches in registration order, and the param route otherwise
|
||||||
account_id: str,
|
# swallows the literal path (it resolved as account_id="hierarchy" and
|
||||||
auth: AuthContext = Depends(require_authenticated),
|
# 404'd for as long as the endpoint existed).
|
||||||
) -> Account:
|
|
||||||
"""Get a specific account (requires authentication and account access)"""
|
|
||||||
account = await get_account(account_id)
|
|
||||||
if not account:
|
|
||||||
raise HTTPException(
|
|
||||||
status_code=HTTPStatus.NOT_FOUND, detail="Account not found"
|
|
||||||
)
|
|
||||||
# Check access permission
|
|
||||||
await require_account_access(auth, account_id, PermissionType.READ)
|
|
||||||
return account
|
|
||||||
|
|
||||||
|
|
||||||
@router.get("/api/v1/accounts/{account_id}/balance")
|
|
||||||
async def api_get_account_balance(
|
|
||||||
account_id: str,
|
|
||||||
auth: AuthContext = Depends(require_authenticated),
|
|
||||||
) -> dict:
|
|
||||||
"""Get account balance from Fava/Beancount (requires authentication and account access)"""
|
|
||||||
from ..fava_client import get_fava_client
|
|
||||||
|
|
||||||
# Get account to retrieve its name
|
|
||||||
account = await get_account(account_id)
|
|
||||||
if not account:
|
|
||||||
raise HTTPException(status_code=404, detail="Account not found")
|
|
||||||
|
|
||||||
# Check access permission
|
|
||||||
await require_account_access(auth, account_id, PermissionType.READ)
|
|
||||||
|
|
||||||
# Query Fava for balance
|
|
||||||
fava = get_fava_client()
|
|
||||||
balance_data = await fava.get_account_balance(account.name)
|
|
||||||
|
|
||||||
return {
|
|
||||||
"account_id": account_id,
|
|
||||||
"balance": balance_data["sats"], # Balance in satoshis
|
|
||||||
"fiat": float(balance_data.get("fiat", 0)), # Fiat amount
|
|
||||||
"fiat_currency": balance_data.get("fiat_currency", "EUR")
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
@router.get("/api/v1/accounts/{account_id}/transactions")
|
|
||||||
async def api_get_account_transactions(
|
|
||||||
account_id: str,
|
|
||||||
limit: int = 100,
|
|
||||||
auth: AuthContext = Depends(require_authenticated),
|
|
||||||
) -> list[dict]:
|
|
||||||
"""
|
|
||||||
Get all transactions for an account from Fava/Beancount.
|
|
||||||
|
|
||||||
Returns transactions affecting this account in reverse chronological order.
|
|
||||||
Requires authentication and account access.
|
|
||||||
"""
|
|
||||||
from ..fava_client import get_fava_client
|
|
||||||
|
|
||||||
# Get account details
|
|
||||||
account = await get_account(account_id)
|
|
||||||
if not account:
|
|
||||||
raise HTTPException(
|
|
||||||
status_code=HTTPStatus.NOT_FOUND,
|
|
||||||
detail=f"Account {account_id} not found"
|
|
||||||
)
|
|
||||||
|
|
||||||
# Check access permission
|
|
||||||
await require_account_access(auth, account_id, PermissionType.READ)
|
|
||||||
|
|
||||||
# Query Fava for transactions
|
|
||||||
fava = get_fava_client()
|
|
||||||
transactions = await fava.get_account_transactions(account.name, limit)
|
|
||||||
|
|
||||||
return transactions
|
|
||||||
|
|
||||||
|
|
||||||
# ===== JOURNAL ENTRY ENDPOINTS =====
|
|
||||||
|
|
||||||
@router.get("/api/v1/accounts/hierarchy")
|
@router.get("/api/v1/accounts/hierarchy")
|
||||||
async def api_get_account_hierarchy(
|
async def api_get_account_hierarchy(
|
||||||
root_account: str | None = None,
|
root_account: str | None = None,
|
||||||
|
|
@ -326,3 +252,82 @@ async def api_get_account_hierarchy(
|
||||||
|
|
||||||
|
|
||||||
# ===== ACCOUNT SYNC ENDPOINTS =====
|
# ===== ACCOUNT SYNC ENDPOINTS =====
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("/api/v1/accounts/{account_id}")
|
||||||
|
async def api_get_account(
|
||||||
|
account_id: str,
|
||||||
|
auth: AuthContext = Depends(require_authenticated),
|
||||||
|
) -> Account:
|
||||||
|
"""Get a specific account (requires authentication and account access)"""
|
||||||
|
account = await get_account(account_id)
|
||||||
|
if not account:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=HTTPStatus.NOT_FOUND, detail="Account not found"
|
||||||
|
)
|
||||||
|
# Check access permission
|
||||||
|
await require_account_access(auth, account_id, PermissionType.READ)
|
||||||
|
return account
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("/api/v1/accounts/{account_id}/balance")
|
||||||
|
async def api_get_account_balance(
|
||||||
|
account_id: str,
|
||||||
|
auth: AuthContext = Depends(require_authenticated),
|
||||||
|
) -> dict:
|
||||||
|
"""Get account balance from Fava/Beancount (requires authentication and account access)"""
|
||||||
|
from ..fava_client import get_fava_client
|
||||||
|
|
||||||
|
# Get account to retrieve its name
|
||||||
|
account = await get_account(account_id)
|
||||||
|
if not account:
|
||||||
|
raise HTTPException(status_code=404, detail="Account not found")
|
||||||
|
|
||||||
|
# Check access permission
|
||||||
|
await require_account_access(auth, account_id, PermissionType.READ)
|
||||||
|
|
||||||
|
# Query Fava for balance
|
||||||
|
fava = get_fava_client()
|
||||||
|
balance_data = await fava.get_account_balance(account.name)
|
||||||
|
|
||||||
|
return {
|
||||||
|
"account_id": account_id,
|
||||||
|
"balance": balance_data["sats"], # Balance in satoshis
|
||||||
|
"fiat": float(balance_data.get("fiat", 0)), # Fiat amount
|
||||||
|
"fiat_currency": balance_data.get("fiat_currency", "EUR")
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("/api/v1/accounts/{account_id}/transactions")
|
||||||
|
async def api_get_account_transactions(
|
||||||
|
account_id: str,
|
||||||
|
limit: int = 100,
|
||||||
|
auth: AuthContext = Depends(require_authenticated),
|
||||||
|
) -> list[dict]:
|
||||||
|
"""
|
||||||
|
Get all transactions for an account from Fava/Beancount.
|
||||||
|
|
||||||
|
Returns transactions affecting this account in reverse chronological order.
|
||||||
|
Requires authentication and account access.
|
||||||
|
"""
|
||||||
|
from ..fava_client import get_fava_client
|
||||||
|
|
||||||
|
# Get account details
|
||||||
|
account = await get_account(account_id)
|
||||||
|
if not account:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=HTTPStatus.NOT_FOUND,
|
||||||
|
detail=f"Account {account_id} not found"
|
||||||
|
)
|
||||||
|
|
||||||
|
# Check access permission
|
||||||
|
await require_account_access(auth, account_id, PermissionType.READ)
|
||||||
|
|
||||||
|
# Query Fava for transactions
|
||||||
|
fava = get_fava_client()
|
||||||
|
transactions = await fava.get_account_transactions(account.name, limit)
|
||||||
|
|
||||||
|
return transactions
|
||||||
|
|
||||||
|
|
||||||
|
# ===== JOURNAL ENTRY ENDPOINTS =====
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue