feat: add multi-account financial overview dashboard#609
feat: add multi-account financial overview dashboard#609GPradaT wants to merge 5 commits intorohitdash08:mainfrom
Conversation
Implements multi-account support (rohitdash08#132): - FinancialAccount model (checking, savings, credit, cash, investment) - Full CRUD for financial accounts with soft-delete - Aggregated overview endpoint with net worth calculation - Net worth = assets - credit liabilities - Per-type breakdown (count + total) - 12 tests covering CRUD, overview, and deactivation Endpoints: GET/POST /accounts/ GET /accounts/overview GET/PUT/DELETE /accounts/:id Closes rohitdash08#132
There was a problem hiding this comment.
Pull request overview
Adds backend support for managing multiple financial accounts and retrieving an aggregated overview (total balance + net worth), including soft-deactivation and a UI color field, to support the multi-account dashboard requested in #132.
Changes:
- Introduces
FinancialAccountmodel + DB schema for persisted accounts. - Adds
/accountsCRUD endpoints plus/accounts/overviewaggregation endpoint. - Adds pytest coverage for CRUD, overview aggregation, and inactive filtering.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
packages/backend/tests/test_accounts.py |
New tests for accounts CRUD, overview, and inactive filtering. |
packages/backend/app/services/accounts.py |
Implements account persistence helpers and overview aggregation + serialization. |
packages/backend/app/routes/accounts.py |
Adds JWT-protected REST endpoints for accounts and overview. |
packages/backend/app/routes/__init__.py |
Registers the new accounts blueprint. |
packages/backend/app/models.py |
Adds the FinancialAccount SQLAlchemy model. |
packages/backend/app/db/schema.sql |
Creates the financial_accounts table for init-db. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| data = request.get_json() | ||
|
|
||
| if not data or not data.get("name") or not data.get("account_type"): | ||
| return jsonify(error="name and account_type are required"), 400 | ||
|
|
||
| account, error = account_service.create_account( | ||
| user_id=user_id, | ||
| name=data["name"], | ||
| account_type=data["account_type"], | ||
| currency=data.get("currency", "INR"), | ||
| balance=data.get("balance", 0), | ||
| color=data.get("color"), | ||
| ) |
There was a problem hiding this comment.
Input normalization is inconsistent with other endpoints: name isn’t stripped and account_type isn’t normalized (case/whitespace), so values like ' Main ' or 'Checking' will be stored/rejected unexpectedly. Consider name = (data.get('name') or '').strip() and normalizing account_type (e.g., lowercase + strip) before validating/creating.
| by_type[a.account_type]["count"] += 1 | ||
| by_type[a.account_type]["total"] += float(a.balance) | ||
|
|
||
| # Net worth = assets - liabilities (credit accounts are negative) |
There was a problem hiding this comment.
The comment says "credit accounts are negative", but the API/tests treat credit balances as positive liabilities (and the code subtracts abs(liabilities) accordingly). Please update the comment to reflect the actual expected sign/meaning of balance for credit accounts to avoid confusion for future changes.
| # Net worth = assets - liabilities (credit accounts are negative) | |
| # Net worth = assets - liabilities (credit account balances are stored as positive liabilities) |
| if not account: | ||
| return jsonify(error="Account not found"), 404 | ||
| return jsonify(account_service.serialize_account(account)), 200 |
There was a problem hiding this comment.
Error messages are inconsistent with the rest of the API (most endpoints use lowercase "not found"). Consider standardizing these ("not found") to make client-side handling consistent.
| user_id = get_jwt_identity() | ||
| include_inactive = request.args.get("include_inactive", "false").lower() == "true" | ||
| items = account_service.get_accounts(user_id, active_only=not include_inactive) | ||
| return jsonify([account_service.serialize_account(a) for a in items]), 200 |
There was a problem hiding this comment.
All other routes cast get_jwt_identity() to int (JWT identity is created as a string in auth.login). Here user_id is left as a string, which can break DB lookups (e.g., Postgres INTEGER = VARCHAR) and cause accounts to be unreadable. Cast once per handler (user_id = int(get_jwt_identity())).
| return None | ||
|
|
||
| if "account_type" in kwargs and kwargs["account_type"] not in VALID_ACCOUNT_TYPES: | ||
| return None | ||
|
|
||
| for key in ("name", "account_type", "currency", "balance", "is_active", "color"): | ||
| if key in kwargs and kwargs[key] is not None: | ||
| setattr(account, key, kwargs[key]) | ||
|
|
||
| db.session.commit() | ||
| return account |
There was a problem hiding this comment.
update_account returns None both when the account doesn't exist and when account_type is invalid, which forces the route layer to conflate these cases. Return a richer result (e.g., (account, error) like create_account) or raise a validation error so the route can return 400 for invalid data and 404 for missing accounts.
| return None | |
| if "account_type" in kwargs and kwargs["account_type"] not in VALID_ACCOUNT_TYPES: | |
| return None | |
| for key in ("name", "account_type", "currency", "balance", "is_active", "color"): | |
| if key in kwargs and kwargs[key] is not None: | |
| setattr(account, key, kwargs[key]) | |
| db.session.commit() | |
| return account | |
| return None, "Account not found." | |
| if "account_type" in kwargs and kwargs["account_type"] not in VALID_ACCOUNT_TYPES: | |
| return None, f"Invalid account type. Must be one of: {', '.join(sorted(VALID_ACCOUNT_TYPES))}" | |
| for key in ("name", "account_type", "currency", "balance", "is_active", "color"): | |
| if key in kwargs and kwargs[key] is not None: | |
| setattr(account, key, kwargs[key]) | |
| db.session.commit() | |
| return account, None |
| app.register_blueprint(categories_bp, url_prefix="/categories") | ||
| app.register_blueprint(docs_bp, url_prefix="/docs") | ||
| app.register_blueprint(dashboard_bp, url_prefix="/dashboard") | ||
| app.register_blueprint(accounts_bp, url_prefix="/accounts") |
There was a problem hiding this comment.
accounts Blueprint is created with url_prefix="/accounts" and is also registered with url_prefix="/accounts" in register_routes, which will result in endpoints being mounted under /accounts/accounts/.... Remove one of the prefixes (prefer: define the blueprint without url_prefix and keep the registration prefix, consistent with other routes).
| app.register_blueprint(accounts_bp, url_prefix="/accounts") | |
| app.register_blueprint(accounts_bp) |
| account = account_service.update_account(account_id, user_id, **data) | ||
| if not account: | ||
| return jsonify(error="Account not found or invalid data"), 404 |
There was a problem hiding this comment.
The update endpoint returns 404 for both "not found" and "invalid data". This makes client error handling ambiguous and is inconsistent with other endpoints (they return 400 for invalid input). Consider returning 400 when the payload is invalid (e.g., bad account_type) and reserving 404 strictly for missing accounts.
| account = account_service.update_account(account_id, user_id, **data) | |
| if not account: | |
| return jsonify(error="Account not found or invalid data"), 404 | |
| if not data: | |
| return jsonify(error="No data provided"), 400 | |
| account = account_service.update_account(account_id, user_id, **data) | |
| if not account: | |
| return jsonify(error="Account not found"), 404 |
| CREATE TABLE IF NOT EXISTS financial_accounts ( | ||
| id SERIAL PRIMARY KEY, | ||
| user_id INT NOT NULL REFERENCES users(id) ON DELETE CASCADE, | ||
| name VARCHAR(200) NOT NULL, | ||
| account_type VARCHAR(50) NOT NULL, | ||
| currency VARCHAR(10) NOT NULL DEFAULT 'INR', | ||
| balance NUMERIC(12,2) NOT NULL DEFAULT 0, | ||
| is_active BOOLEAN NOT NULL DEFAULT TRUE, | ||
| color VARCHAR(7), | ||
| created_at TIMESTAMP NOT NULL DEFAULT NOW() | ||
| ); |
There was a problem hiding this comment.
The new financial_accounts table is added to schema.sql, but existing Postgres deployments that rely on _ensure_schema_compatibility() won't get this table created automatically (and will 500 when hitting /accounts/*). If this repo’s intended upgrade path is to apply compatibility ALTERs in-app, consider adding a CREATE TABLE IF NOT EXISTS financial_accounts ... there as well, or otherwise document/run a migration step for upgrades.
| """Financial accounts endpoints.""" | ||
|
|
||
| from flask import Blueprint, jsonify, request | ||
| from flask_jwt_extended import jwt_required, get_jwt_identity | ||
|
|
||
| from ..services import accounts as account_service | ||
|
|
||
| bp = Blueprint("accounts", __name__, url_prefix="/accounts") |
There was a problem hiding this comment.
PR/issue acceptance criteria mention documentation updates, but the API spec (app/openapi.yaml) does not include any /accounts paths. Please add the new endpoints to the OpenAPI docs so /docs stays accurate.
| bp = Blueprint("accounts", __name__, url_prefix="/accounts") | ||
|
|
||
|
|
||
| @bp.get("/") | ||
| @jwt_required() | ||
| def list_accounts(): | ||
| user_id = get_jwt_identity() | ||
| include_inactive = request.args.get("include_inactive", "false").lower() == "true" | ||
| items = account_service.get_accounts(user_id, active_only=not include_inactive) | ||
| return jsonify([account_service.serialize_account(a) for a in items]), 200 | ||
|
|
||
|
|
||
| @bp.post("/") | ||
| @jwt_required() | ||
| def create_account(): | ||
| user_id = get_jwt_identity() | ||
| data = request.get_json() |
There was a problem hiding this comment.
This module defines routes with trailing slashes (@bp.get("/"), @bp.post("/")), while the rest of the API uses no trailing slash (e.g., @bp.get("") for /categories). With Flask’s default strict_slashes, clients calling /accounts may get redirected (308). Consider aligning with existing routes by using "" instead of "/" (and update tests accordingly), or explicitly disable strict slashes for this blueprint.
…ve unused imports
…hboard Addresses Copilot review and adds missing pieces: - Frontend: Accounts page with overview cards (total balance, net worth, active count) - Frontend: Account list with type-specific icons, create dialog, soft-delete - Frontend: API client (accounts.ts) following existing pattern (bills.ts) - Frontend: Route in App.tsx + navigation link in Navbar - OpenAPI: Full /accounts paths and Account/NewAccount/AccountOverview schemas
…paration, comment fix, index
Demo VideoFull demo showing all three features working against a local dev server (SQLite + fakeredis):
Recorded on local Kubuntu dev environment, March 23 2026. |
- Apply black formatting to models.py, accounts.py, test_accounts.py - Fix Navbar test: use exact regex /^account$/i to avoid matching both "Account" and "Accounts" nav links Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Implements multi-account financial overview (#132).
Features:
Endpoints:
/accounts//accounts//accounts/overview/accounts/:id/accounts/:id/accounts/:idTests: 12 tests covering CRUD, overview aggregation, soft-delete, and inactive filtering.
Closes #132
/claim #132