envelope-zero / backend

Serves the API and contains the application logic
Other
6 stars 3 forks source link

Calculated income excludes off-budget to on-budget transactions #1007

Closed ekeih closed 7 months ago

ekeih commented 7 months ago

Describe the bug

To Reproduce

  1. Create an off-budget account and an on-budget account.
  2. Check the .income attribute of /api/v4/months of the budget.
  3. Add a transaction transferring money from the off-budget to the on-budget account.
  4. Check .income again. It is the same as in step 2, but I believe it should have increased by the transferred amount.

Expected behavior Off-budget to on-budget transactions without an envelope are included in the .income calculation of v4/months.

Additional context

morremeyer commented 7 months ago

Thanks for this detailed report.

Bug

You are indeed correct, this is a bug. Transactions from off-budget to on-budget accounts without an Envelope need to be considered for .income of the /v4/months endpoint.

Transaction filtering

The transaction filtering again has a different understanding, which I have come to consider wrong, too.

I thought about this for a while and arrived at the conclusion that we'll need two different filters, one direction filter and one type filter.

direction: purely for internal/external account considerations

Has the following filter values

type: For the effect a transaction has on the budget

Has the following filter values

I'm not yet happy with incoming and income being named this similarly. Any suggestions to improve this are very welcome.

morremeyer commented 7 months ago

I think Where("source_account.external = 1"). in budget.go should be Where("accounts_source.on_budget = true AND accounts_destination.on_budget = false"). instead.

@ekeih I think you meant that it should be

Where("source_account.on_budget = false AND destination_account.on_budget = true").

instead, right? You had the true and false condititons the other way around. I used the updated version in #1023, which resolves this issue.

I moved the changes to transaction filtering to #1024 to handle them separately.

ekeih commented 7 months ago

Oh, yes you are right, I mixed that up 😬 Thanks for the fix! 🚀