Skip to content

Conversation

@tembo
Copy link
Contributor

@tembo tembo bot commented Sep 6, 2025

Description

Implement goal tracking feature with financial milestones and progress tracking.

Changes

  • Created Goal model with status tracking
  • Added CRUD endpoints for goals
  • Integrated goal tracking with income, expense, and transfer transactions
  • Implemented goal progress and status updates
  • Added database migrations for goal and transaction tables

Want me to make any changes? Add a review or comment with @tembo and i'll get back to work!

tembo.io

@tembo tembo bot requested a review from EverStarck September 6, 2025 03:08
@tembo
Copy link
Contributor Author

tembo bot commented Sep 6, 2025

Requesting review from @EverStarck who has experience with the following files modified in this PR:

  • backend/app/app/db/base.py
  • backend/app/app/models/user.py
  • backend/app/app/crud/__init__.py
  • backend/app/app/models/income.py
  • backend/app/app/api/api_v1/api.py
  • backend/app/app/models/account.py
  • backend/app/app/models/expense.py
  • backend/app/app/schemas/income.py
  • backend/app/app/models/__init__.py
  • backend/app/app/models/transfer.py
  • backend/app/app/schemas/expense.py
  • backend/app/app/crud/crud_income.py
  • backend/app/app/schemas/__init__.py
  • backend/app/app/schemas/transfer.py
  • backend/app/app/crud/crud_expense.py
  • backend/app/app/crud/crud_transfer.py

@EverStarck
Copy link
Collaborator

@tembo i don't want to expenses to modify the goals. ONLY EXPENSES AND TRANSFERS can change the goal progress. Remove everything related to expenses

@tembo
Copy link
Contributor Author

tembo bot commented Sep 6, 2025

@tembo i don't want to expenses to modify the goals. ONLY EXPENSES AND TRANSFERS can change the goal progress. Remove everything related to expenses

I've removed income's ability to modify goal progress as requested. Only expenses and transfers can now affect goal amounts. The changes include:

  1. Removed goal_id from the income table and model
  2. Removed income-goal relationship in the database models
  3. Removed income-goal relationship from goal model
  4. Removed the goal update logic from income creation flow
  5. Renamed the parameter in update_goal_amount from is_income to is_positive to better reflect that only transfers (positive) and expenses (negative) affect goals

All goal progress updates now come exclusively from expenses and transfers, as specified. Let me know if you need any adjustments to these changes.

@EverStarck
Copy link
Collaborator

@tembo i messed up. ONLY INCOMES and TRANSFERS can change the goal progress. Remove everything related to expenses and add incomes functionality

@tembo
Copy link
Contributor Author

tembo bot commented Sep 6, 2025

@tembo i messed up. ONLY INCOMES and TRANSFERS can change the goal progress. Remove everything related to expenses and add incomes functionality

@EverStarck I've made the requested changes to the goal tracking functionality. As requested, I've removed the expense relationship with goals and added income functionality instead. Here's what was done:

  1. Added a migration to add goal_id to the income table with a foreign key constraint
  2. Created a migration to remove the goal_id column from the expense table
  3. Updated the income model and schema to include the goal relationship
  4. Removed goal references from the expense model and schema
  5. Modified the goal model to have a relationship with incomes instead of expenses
  6. Updated the CRUD operations to update goal progress when incomes are created

Now only incomes and transfers can contribute to goal progress as specified.

@EverStarck
Copy link
Collaborator

@tembo remove all migrations you did

@tembo
Copy link
Contributor Author

tembo bot commented Sep 7, 2025

@tembo remove all migrations you did

@EverStarck I've removed all migration files as requested. The following files have been deleted:

  • 26509aaa0bab_remove_goal_id_from_income_table.py
  • add_goal_id_back_to_income_table.py
  • oz0191glw8ir_add_goal_table.py
  • remove_goal_id_from_expense_table.py
  • w8c5h14uldi0_add_goal_id_to_transactions.py

I'll work on a cleaner approach for implementing the goal feature with a single migration file.

@EverStarck
Copy link
Collaborator

@tembo on doing a update goal, it must check if the status changed, for example, if i update the current_amount, target_amount, deadline or something else, its possible the status is not the same. help me to add the logic

@tembo
Copy link
Contributor Author

tembo bot commented Sep 21, 2025

@tembo on doing a update goal, it must check if the status changed, for example, if i update the current_amount, target_amount, deadline or something else, its possible the status is not the same. help me to add the logic

Thanks for your feedback @EverStarck. I've implemented the goal status checking logic as requested. The changes include:

  1. Added a new _update_goal_status helper method that automatically evaluates and updates a goal's status based on its current state
  2. Integrated this method into the update workflow so status is properly recalculated whenever fields like current_amount, target_amount, or deadline change
  3. Added logic to handle various state transitions including:
    • Marking goals as completed when target is reached
    • Setting goals to overdue when deadline passes
    • Reactivating overdue goals if deadline is extended
    • Reverting completed goals to active if target amount increases

This ensures goal status always reflects the current state of progress and timeline, regardless of which fields are being updated.

@EverStarck
Copy link
Collaborator

@tembo when updating an income or transfer, I should be able to update the goal, if I reduce the amount or delete the income/transfer, is probably the goal is not more completed. same if I add to the amount of the transaction

@tembo
Copy link
Contributor Author

tembo bot commented Sep 24, 2025

@tembo when updating an income or transfer, I should be able to update the goal, if I reduce the amount or delete the income/transfer, is probably the goal is not more completed. same if I add to the amount of the transaction

Thank you for the feedback, @EverStarck. I've implemented goal recalculation when updating or deleting incomes and transfers.

When you modify a transaction linked to a goal, the system now:

  • Recalculates the goal's current amount by summing all linked transactions
  • Updates the goal status based on the new amount
  • Works for both adding and reducing transaction amounts
  • Handles goal reassignment between transactions

These changes ensure your goal progress remains accurate even after modifying or deleting transactions that contribute to it.

@EverStarck
Copy link
Collaborator

@tembo also check the api_v1 endpoints income and transfer, i think you're duplicating code

@tembo
Copy link
Contributor Author

tembo bot commented Sep 24, 2025

@tembo also check the api_v1 endpoints income and transfer, i think you're duplicating code

Thanks @EverStarck for pointing out the duplicate code. I've refactored the date filtering logic across the income, transfer, and expense endpoints by extracting the common functionality into a reusable helper function in the deps.py file.

The changes include:

  • Created parse_date_filter() in deps.py that handles all date parsing logic
  • Removed redundant date parsing code from income.py, transfer.py, and expense.py
  • Each endpoint now uses a single function call to handle date filtering
  • Maintained all existing functionality while reducing code duplication

Let me know if you'd like any other improvements to the implementation.

@EverStarck
Copy link
Collaborator

@tembo no mf, the api_v1 endpoints income and transfer ABOUT THE GOAL THING IN UPDATE/DELETE A INCOME OR TRANSFER. YOU ALREADY DID SOMETHING IN THE CRUD, BUT THE ENDPOINT ALREADY HAS SOME LOGIC, YOU'RE DUPLICATING THINGS

@tembo
Copy link
Contributor Author

tembo bot commented Sep 24, 2025

@tembo no mf, the api_v1 endpoints income and transfer ABOUT THE GOAL THING IN UPDATE/DELETE A INCOME OR TRANSFER. YOU ALREADY DID SOMETHING IN THE CRUD, BUT THE ENDPOINT ALREADY HAS SOME LOGIC, YOU'RE DUPLICATING THINGS

Thanks for your feedback @EverStarck. I've refactored the income and transfer endpoints to eliminate the duplicated logic by moving it to the CRUD layer. The endpoints now call owner-aware methods like update_with_owner, remove_with_owner that handle all the associated calculations internally (account balances, category totals, etc.). This makes the code much cleaner while maintaining the same functionality. Let me know if you need any other adjustments!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants