feat: add goal-based savings tracking with milestones#608
feat: add goal-based savings tracking with milestones#608GPradaT wants to merge 5 commits intorohitdash08:mainfrom
Conversation
Implements savings goal tracking (rohitdash08#133): - SavingsGoal and GoalContribution models - Full CRUD for goals with status management (ACTIVE/COMPLETED/CANCELLED) - Contribution tracking with auto-complete when target is reached - Progress endpoint with milestone tracking (25/50/75/100%) - Deadline-aware daily savings calculation - 15 comprehensive tests covering CRUD, contributions, and progress Endpoints: GET/POST /goals/ GET/PUT/DELETE /goals/:id GET /goals/:id/progress POST /goals/:id/contribute GET /goals/:id/contributions Closes rohitdash08#133
There was a problem hiding this comment.
Pull request overview
Adds a new “savings goals” feature to the backend, introducing goal CRUD, contribution tracking, and a progress endpoint with milestone calculations.
Changes:
- Added
SavingsGoal/GoalContributionmodels and corresponding DB schema tables. - Implemented goals service logic (create/update/cancel, contribute, progress + milestones).
- Added
/goalsroutes and a full test suite for the new feature.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/backend/app/models.py | Adds new goal/contribution models and status enum. |
| packages/backend/app/db/schema.sql | Adds new tables for goals and contributions. |
| packages/backend/app/services/goals.py | Implements goal lifecycle, contributions, and progress/milestones. |
| packages/backend/app/routes/goals.py | Adds HTTP endpoints for goals CRUD, contributions, and progress. |
| packages/backend/app/routes/init.py | Registers the new goals blueprint. |
| packages/backend/tests/test_goals.py | Adds test coverage for goals CRUD, contributions, and progress. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/backend/app/routes/goals.py
Outdated
|
|
||
| from ..services import goals as goal_service | ||
|
|
||
| bp = Blueprint("goals", __name__, url_prefix="/goals") |
There was a problem hiding this comment.
The goals blueprint is created with url_prefix="/goals", but register_routes() also registers the same blueprint with url_prefix="/goals". In Flask these prefixes stack, so the effective routes become /goals/goals/... (breaking the intended paths and likely causing 404s for /goals). Align with the existing route modules by removing the blueprint-level url_prefix and using @bp.get("")/@bp.post("") etc (or alternatively remove the url_prefix from register_routes, but that would diverge from the established pattern).
| amount=amount, | ||
| note=data.get("note"), | ||
| ) | ||
| if error: |
There was a problem hiding this comment.
add_contribution() returns a generic 400 for all service errors, including "Goal not found". That makes the API inconsistent with the other endpoints in this module (which return 404 for missing goals) and prevents clients from distinguishing not-found vs validation/business-rule failures. Map the not-found case to 404 (e.g., have the service return a sentinel error type/code, or re-check existence in the route when error == "Goal not found").
| if error: | |
| if error: | |
| if error == "Goal not found": | |
| return jsonify(error=error), 404 |
| CREATE TABLE IF NOT EXISTS goal_contributions ( | ||
| id SERIAL PRIMARY KEY, | ||
| goal_id INT NOT NULL REFERENCES savings_goals(id) ON DELETE CASCADE, | ||
| amount NUMERIC(12,2) NOT NULL, | ||
| note VARCHAR(200), |
There was a problem hiding this comment.
goal_contributions will be queried by goal_id and ordered by contribution time, but the table has no index to support that access pattern. Add an index such as (goal_id, contributed_at DESC) (or (goal_id, created_at DESC)) to avoid slow scans as contributions grow.
packages/backend/tests/test_goals.py
Outdated
| r = client.post("/goals/", json={ | ||
| "name": "Emergency Fund", | ||
| "target_amount": 10000, | ||
| "currency": "USD", | ||
| "deadline": "2026-12-31", | ||
| }, headers=auth_header) |
There was a problem hiding this comment.
These tests use the /goals/ trailing-slash form, but the existing API (and openapi.yaml) consistently uses no trailing slash for collection endpoints (e.g., /categories, /expenses, /bills). Consider switching the goals routes/tests to /goals for consistency (and to avoid Flask redirect behavior from strict slashes).
| def list_goals(): | ||
| user_id = get_jwt_identity() | ||
| status = request.args.get("status") | ||
| items = goal_service.get_goals(user_id, status) | ||
| return jsonify([_serialize_goal(g) for g in items]), 200 |
There was a problem hiding this comment.
get_jwt_identity() returns a string in this codebase (see auth.py where tokens are created with identity=str(user.id)). Here user_id is left as a string and is passed into DB filters/columns that are integers; this will work in SQLite tests but can raise type errors in Postgres (or produce incorrect filtering). Cast user_id to int (as done in other routes) before calling the service layer in this module.
| db.session.add(contribution) | ||
|
|
||
| goal.current_amount = Decimal(str(goal.current_amount)) + Decimal(str(amount)) | ||
|
|
||
| # Auto-complete if target reached | ||
| if goal.current_amount >= goal.target_amount: | ||
| goal.status = GoalStatus.COMPLETED.value | ||
|
|
||
| db.session.commit() |
There was a problem hiding this comment.
Updating goal.current_amount by reading it into Python and writing back is not safe under concurrent requests: two contributions can race and one update can be lost. Use an atomic DB-side update (e.g., UPDATE savings_goals SET current_amount = current_amount + :amount ...) or lock the goal row (SELECT ... FOR UPDATE) within a transaction before recomputing current_amount and status.
| pct = round((current / target) * 100, 2) if target > 0 else 0 | ||
|
|
||
| result = { | ||
| "goal_id": goal.id, | ||
| "name": goal.name, | ||
| "target": target, | ||
| "current": current, | ||
| "remaining": round(target - current, 2), |
There was a problem hiding this comment.
remaining is computed as target - current and can become negative if a contribution overshoots the target (which is allowed). Returning a negative remaining amount is confusing for clients; clamp it to 0 (and consider also clamping progress_pct to 100).
| pct = round((current / target) * 100, 2) if target > 0 else 0 | |
| result = { | |
| "goal_id": goal.id, | |
| "name": goal.name, | |
| "target": target, | |
| "current": current, | |
| "remaining": round(target - current, 2), | |
| pct_raw = round((current / target) * 100, 2) if target > 0 else 0 | |
| pct = min(pct_raw, 100.0) | |
| remaining_raw = round(target - current, 2) | |
| remaining = max(remaining_raw, 0.0) | |
| result = { | |
| "goal_id": goal.id, | |
| "name": goal.name, | |
| "target": target, | |
| "current": current, | |
| "remaining": remaining, |
| "on_track": True, | ||
| } | ||
|
|
||
| if goal.deadline: | ||
| days_left = (goal.deadline - date.today()).days | ||
| result["deadline"] = goal.deadline.isoformat() | ||
| result["days_left"] = max(days_left, 0) | ||
|
|
||
| if days_left > 0 and current < target: | ||
| daily_needed = round((target - current) / days_left, 2) | ||
| result["daily_savings_needed"] = daily_needed | ||
| elif days_left <= 0 and current < target: | ||
| result["on_track"] = False | ||
|
|
There was a problem hiding this comment.
The PR description says the progress response includes on-track/off-track status "based on deadline proximity", but this implementation only flips on_track to False after the deadline has already passed. If on/off-track is meant to indicate whether the user is on pace before the deadline, compute expected progress vs time remaining (or compare daily_savings_needed against a configured/observed savings rate) and set on_track accordingly.
| CREATE TABLE IF NOT EXISTS savings_goals ( | ||
| id SERIAL PRIMARY KEY, | ||
| user_id INT NOT NULL REFERENCES users(id) ON DELETE CASCADE, | ||
| name VARCHAR(200) NOT NULL, | ||
| target_amount NUMERIC(12,2) NOT NULL, |
There was a problem hiding this comment.
This feature introduces queries that filter goals by user_id (and optionally status) and order by created_at, but savings_goals is created without any supporting indexes. Add an index such as (user_id, status, created_at DESC) (or at least (user_id, created_at DESC)) to keep list/progress operations fast as the table grows.
| def update_goal(goal_id): | ||
| user_id = get_jwt_identity() | ||
| data = request.get_json() or {} | ||
| goal = goal_service.update_goal(goal_id, user_id, **data) |
There was a problem hiding this comment.
update_goal forwards the raw JSON payload directly into goal_service.update_goal without validating or normalizing fields. Unlike create_goal, this means clients can set target_amount to a negative/non-numeric value or set deadline to a non-date string, which will either violate business rules or trigger a 500 on commit. Mirror the validation/parsing from create_goal here (and restrict status to the known GoalStatus values if you intend to allow status updates at all).
| goal = goal_service.update_goal(goal_id, user_id, **data) | |
| # Disallow direct status updates from this endpoint unless explicitly supported | |
| if "status" in data: | |
| return jsonify(error="status cannot be updated via this endpoint"), 400 | |
| updates = {} | |
| if "name" in data: | |
| updates["name"] = data["name"] | |
| if "target_amount" in data: | |
| target = data["target_amount"] | |
| if not isinstance(target, (int, float)) or target <= 0: | |
| return jsonify(error="target_amount must be a positive number"), 400 | |
| updates["target_amount"] = target | |
| if "currency" in data: | |
| updates["currency"] = data["currency"] | |
| if "deadline" in data: | |
| # Allow clearing the deadline by sending null/empty string | |
| raw_deadline = data["deadline"] | |
| if raw_deadline in (None, ""): | |
| updates["deadline"] = None | |
| else: | |
| try: | |
| updates["deadline"] = date.fromisoformat(raw_deadline) | |
| except (TypeError, ValueError): | |
| return jsonify(error="deadline must be YYYY-MM-DD format"), 400 | |
| goal = goal_service.update_goal(goal_id, user_id, **updates) |
…for not found, clamp remaining
- Frontend: Goals page with progress bars, milestone badges, contribute dialog - Frontend: API client (goals.ts) following existing pattern - Frontend: Route in App.tsx + navigation link in Navbar - OpenAPI: Full /goals paths and SavingsGoal/GoalProgress/Contribution schemas - Auto-complete visual feedback when goal reaches target - Daily savings needed calculation displayed on cards
Demo VideoFull demo showing all three features working against a local dev server (SQLite + fakeredis):
Recorded on local Kubuntu dev environment, March 23 2026. |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Implements savings goal tracking with milestones (#133).
Features:
Endpoints:
/goals//goals//goals/:id/goals/:id/goals/:id/goals/:id/progress/goals/:id/contribute/goals/:id/contributionsTests: 15 tests covering CRUD, contributions, auto-completion, progress, and edge cases.
Closes #133
/claim #133