Add start balance to manual accounts

[jakubkottnauer]

This PR makes balance of manual accounts be automatically updated whenever a new valuation or a transaction is added to that account. In this first iteration, the solution "hijacks" the existing sync flow by first computing the correct current balance which is then treated as the "source of truth" in the rest of the sync (as if it was an account connected to a 3rd party provider).

Current balance of a manual account is calculated as follows:

• take the latest valuation's value, or start_balance if no valuation exists
• get the sum of all transactions that occurred between the valuation (or all transactions if no valuation exists) and today
• -> sum of these two numbers gives you the current balance

Changes

A new accounts.start_balance column has been added. I have considered making the migration populate the column for all accounts (as we only have manual accounts at the time of running the migration) but then to be consistent I would also have to change all of the seed data to create manual accounts, which doesn't sound like something we'd like to do. Happy to discuss this further.

All newly created accounts will be manual by default as we don't have support for connected accounts yet.

Next steps

• Start balance of a manual account should be made editable (are there designs for an account edit dialog yet?)
• Valuations and transactions can have their date set in the future. This means that accounts need to be synced periodically to make sure the balance is up to date every day the user opens Maybe. Somewhat similar to the issue Bug: Account group total value in sidebar ignores accounts in foreign currency #724 and can be fixed the same way.
• Come up with a better way of telling manual and connected accounts apart - currently it's done by checking account.start_balance.nil?. In the future it would be nicer to check if a provider connection exists for the account, but we're not quite there yet.
• We should have some UI elements distinguishing between manual/connected accounts - maybe we could temporarily show a fake-ish error message on connected accounts saying that the "provider for this account is inactive"? After this PR, some accounts will react to transactions/valuations being added (manual accounts) while others will not (connected accounts) and this might cause confusion without any way of telling them apart.

[jakubkottnauer]

@zachgoll After this is reviewed I will need some help with updating the Google Sheet to include the new manual account fixtures.

[zachgoll]

@jakubkottnauer will take a look this afternoon!

[zachgoll]

@jakubkottnauer thanks for the work on this! I've addressed some of the various questions below along with some overall thoughts:

Start balance of a manual account should be made editable (are there designs for an account edit dialog yet?)

We do have some designs, I'll be getting them out for dev soon!

Valuations and transactions can have their date set in the future.

Should future dates be valid? I know we're not enforcing this currently but it may be a good idea to set a max date of Date.current for valuations and transactions.

Come up with a better way of telling manual and connected accounts apart - currently it's done by checking account.start_balance.nil?. In the future it would be nicer to check if a provider connection exists for the account, but we're not quite there yet.

We should have some UI elements distinguishing between manual/connected accounts - maybe we could temporarily show a fake-ish error message on connected accounts saying that the "provider for this account is inactive"? After this PR, some accounts will react to transactions/valuations being added (manual accounts) while others will not (connected accounts) and this might cause confusion without any way of telling them apart.

There are some designs in the works to distinguish between the two, but I'm starting to think that we should simply overwrite the existing sync process and tests and assume that all accounts are manual for now. I'm completely fine deleting the existing sync logic and making this the new "base case" until we have connected accounts to work with. We can always go back in source control to find the "connected account logic". For now, I think it will be a lot simpler to deal with concrete flows rather than trying to design for the future.

Off the top of my head, I think the pieces that we'd need to address would be:

Strategy

In my head, a "start balance" is ultimately the same thing as a Valuation. It has a monetary value + date.

Rather than introducing a new DB field, I think we can just use the "oldest Valuation" as our start balance. When a user creates an account, we can ask them for this information and our system will automatically create the Valuation:

1. Current day balance
2. Account start value (we create a Valuation equal to the date + amount input)

If we did this, I think we'd be able to keep almost all of our existing balance calculator logic. The only thing that would then change would be this line:

maybe/app/models/account/balance/calculator.rb

Line 34 in 5ed2c47

{ date: Date.current, balance: @account.balance, currency: @account.currency, updated_at: Time.current } # Last balance must always match "source of truth"

We would then need to make sure that on every sync, @account.balance is updated to reflect the calculated series.

[jakubkottnauer]

Thank you for the review! I'll incorporate the changes you've suggested, making manual accounts the "base case" for now 👍

Should future dates be valid?

Some other apps I have used allow future transactions, though I don't see much value in allowing future valuations. And if we won't be allowing those, maybe it makes not to allow transactions either for consistency 😄 Up to you!

Rather than introducing a new DB field, I think we can just use the "oldest Valuation" as our start balance

I like the proposed solution, I'll make the change today/tomorrow

[zachgoll]

@jakubkottnauer I think for now we can enforce a max date of Date.current for simplicity. I could see a case for future dates for both, but it's not worth the complexity right now:

• Valuation - for example, a user provides a "projected" account balance
• Transaction - for example, a user provides a "recurring" transaction that is expected

[jakubkottnauer]

@zachgoll I've implemented the changes, seems to work well

[zachgoll]

Looks good! Can you update the tests to expect this new convention? Will get it merged after those pass.

[jakubkottnauer]

@zachgoll I've pushed a few new tests. Can you help out with updating the CSVs?

[zachgoll]

@jakubkottnauer here is the link to the Google sheet that I've been using to visualize the test cases:

https://docs.google.com/spreadsheets/d/18LN5N-VLq4b49Mq1fNwF7_eBiHSQB46qQduRtdAEN98/edit?usp=sharing

I'll go ahead and get this updated after this PR is merged. But for now, I think all we need to do is update the final row in each of these CSVs:

maybe/test/fixtures/family/expected_snapshots.csv

Line 32 in daf7ff8

0,47550.80,48550.80,1000.00,48000.80,0.00,0.00,1000.00,0.00,0.00,550.00,0.00,0,0,3214.48,2447.715,-0.3132574667

maybe/test/fixtures/account/expected_balances.csv

Line 32 in daf7ff8

0,550,5000,20000,1000,12000,13000.8,10000

[jakubkottnauer]

@zachgoll All tests are green now 😄

[zachgoll]

Awesome, looks great! Thanks for the work on this!