Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Income summed under Expense #736

Closed
wants to merge 1 commit into from

Conversation

Harry-kp
Copy link
Contributor

@Harry-kp Harry-kp commented May 10, 2024

Description

  • Fixes Bug: Income transactions summed under Expenses in summary #721
  • Adjusted the transaction controller to ensure correct handling of income and expense amounts.
    • Incomes are now processed with positive amounts, aligning with the inflows scope.
    • Expenses are now processed with negative amounts, ensuring consistency.
Fix.Income.Sum.mov

@Harry-kp
Copy link
Contributor Author

Harry-kp commented May 10, 2024

@zachgoll The tests are written in the way that it expects income amount to be negative. Was this intentional?. If we are going to fix this issue, what should we going to modify, the scope inflows or the amount.

@Harry-kp Harry-kp marked this pull request as draft May 10, 2024 20:35
@zachgoll
Copy link
Collaborator

@Harry-kp yes, the convention that we're using is that any inflow to an account has a negative amount, while any outflow has a positive amount. This affects the account balance differently based on the classification of the account. You can find some examples of this described here.

@Harry-kp
Copy link
Contributor Author

@zachgoll I am slightly confused between With this approach, a positive value is an "inflow" and a negative value is an "outflow". and as you described above the convention that we're using is that any inflow to an account has a negative amount, while any outflow has a positive amount.
Also, Can you make me understand what happens in these scenarios

  1. Say, we added 100 USD as income in a Account whose classification is Asset. What value should be stored in the amount column of transaction.
  2. Say, we added 100 USD as income in a Account whose classification is Liability. What value should be stored in the amount column of the transaction.
  3. How are you mapping income and expense to inflow or outflow? Are these terms also dependent on the classification of account. Like, it differs when account is asset to when account is liability.

@zachgoll
Copy link
Collaborator

@Harry-kp that's my bad, at second glance, looks like the Wiki was not up-to-date with the convention we're using in the app.

I've re-written it with a bit more clarity, let me know if that helps!

https://github.com/maybe-finance/maybe/wiki/Vision#signage-of-money

And to your question about classification, yes, we're using it to determine how to sum account balances. Here's one example in the sync process:

net_transaction_flows *= -1 if @account.classification == "liability"

@Harry-kp
Copy link
Contributor Author

@zachgoll I found the updated wiki much clearer, thank you! I just want to clarify something: When we talk about income, is it the same as inflow? And similarly, is expense the same as outflow, or does it depend on how the accounts are classified?

@zachgoll
Copy link
Collaborator

@zachgoll I found the updated wiki much clearer, thank you! I just want to clarify something: When we talk about income, is it the same as inflow? And similarly, is expense the same as outflow, or does it depend on how the accounts are classified?

In theory, income and expense should be a subset of inflow and outflow. Right now, since we don't have the concept of transaction types yet, we're approximating anything with a negative amount as "income" and anything with a positive amount as "expense". This is an "interim" solution at best, but we don't have a much better one until those transaction types are introduced.

@zachgoll
Copy link
Collaborator

Covered by #759

@zachgoll zachgoll closed this May 17, 2024
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.

Bug: Income transactions summed under Expenses in summary
2 participants