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

Calculates trend based on previous transaction's balance on the same date #1483

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

nicogaldamez
Copy link
Contributor

Why?

What?

Add a new method to the Entry model to calculate the balance based on the last entry's balance from the same day

image

What should we test?

Create two entries: an account valuation and an expense (or income) on the same day. The account valuation entry's trend and amount should use the balance of the other entry instead of the previous day's balance.

Paired with @bruno-costanzo

@zachgoll
Copy link
Collaborator

@nicogaldamez I agree with this logic; definitely more intuitive!

I'll go ahead and merge this because I think it gets the logic in place that we'll need. Longer term (and this is something I'll be returning to shortly), I think all of this logic probably needs to be consolidated to the Account::Balance::Syncer, and we should store a end_balance directly on the Account::Entry model, which will be calculated in the background.

That way, in these views, we can just read that value rather than performing all these queries.

@zachgoll zachgoll merged commit 242eb5c into maybe-finance:main Nov 22, 2024
5 checks passed
@bruno-costanzo
Copy link
Contributor

@zachgoll Note that persisting the value may lead to having to recalculate the after_balance for each subsequent transaction when entering a new entry on an old date.

@zachgoll
Copy link
Collaborator

@bruno-costanzo yep, definitely a denormalization trade-off, but in this case, I think it's worth it given the cost of running all these queries for every single transaction on every page load. Our account sync process should handle it nicely though!

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: When balance and transaction exist on same day, balances not correctly calculated
3 participants