Cleanup: single source of truth for account roots; drop no-op per-test rate-limit reset #54

Open
opened 2026-06-16 21:59:39 +00:00 by padreug · 0 comments
Owner

Found in the high-effort code review of PR #46. Two minor cleanups.

1. The five account roots are encoded in three places.

  • static/js/index.js accountRootTypes()['Assets','Liabilities','Equity','Income','Expenses']
  • views_api.py:3661 _VALID_ACCOUNT_PREFIXES
  • account_utils.ACCOUNT_TYPE_ROOTS

The JS comment even says "Mirrors the server's _VALID_ACCOUNT_PREFIXES" — an explicit drift hazard. If a root is ever renamed/added, all three must change in lockstep. Derive the JS list from a server-provided value (e.g. an existing settings/info endpoint) or at least centralize the Python copies into one constant.

2. The per-test rate-limit reset is dead code. tests/conftest.py _settings_cleanup sets lnbits_rate_limit_no = 1_000_000 on every test, but the limiter is built once at app creation (the session settings fixture value is what matters). The per-test write does nothing — and would silently defeat any future test that deliberately lowers the limit to assert a 429. Drop it; keep only the session-fixture override.

Found in the high-effort code review of PR #46. Two minor cleanups. **1. The five account roots are encoded in three places.** - `static/js/index.js` `accountRootTypes()` — `['Assets','Liabilities','Equity','Income','Expenses']` - `views_api.py:3661` `_VALID_ACCOUNT_PREFIXES` - `account_utils.ACCOUNT_TYPE_ROOTS` The JS comment even says "Mirrors the server's `_VALID_ACCOUNT_PREFIXES`" — an explicit drift hazard. If a root is ever renamed/added, all three must change in lockstep. Derive the JS list from a server-provided value (e.g. an existing settings/info endpoint) or at least centralize the Python copies into one constant. **2. The per-test rate-limit reset is dead code.** `tests/conftest.py` `_settings_cleanup` sets `lnbits_rate_limit_no = 1_000_000` on every test, but the limiter is built once at app creation (the session `settings` fixture value is what matters). The per-test write does nothing — and would silently defeat any future test that deliberately lowers the limit to assert a 429. Drop it; keep only the session-fixture override.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
aiolabs/libra#54
No description provided.