scottohara / loot

An implementation of some of the core MS Money features in Ruby on Rails
MIT License
4 stars 3 forks source link

Only fetch reconciled/cleared totals when reconciling #194

Open scottohara opened 1 year ago

scottohara commented 1 year ago

Background

As part of the work to fix multi-page reconciliations (https://github.com/scottohara/loot/issues/114, https://github.com/scottohara/loot/commit/fcb9b6a39a4d0d8172771b4b358e8399360e5fdd and https://github.com/scottohara/loot/commit/6e92f3412f63ea87c8cd83e4a44c4edfaa11e2ce), two new properties were added to the account model:

Options and trade-offs

When exposing these properties to the UI, we were faced with two possible options:

  1. Compute and return the new properties as part of the existing GET /accounts/:id endpoint
  2. Add a new REST endpoint specifically for these properties, e.g. GET /accounts/:id/reconcile or similar

For option 1, the trade-off is potentially doing unnecessary extra work (wasting CPU cycles and the user's bandwidth). These particular values are only needed when reconciling, but we would be calculating and returning them any time an account is fetched.

For option 2, the trade-off is an expanded API surface and all of the work required to build and support the new endpoint, plus an extra API call that the front-end would need to make when reconciling.

Note:

There already exists a PUT /accounts/:id/reconcile endpoint. This is used to finalise a reconciliation by updating all transactions marked as Cleared -> Reconciled:

resources :accounts, concerns: [:favouritable] do
  resources :transactions, only: [:index], concerns: %i[reconcilable defaultable]
  put 'reconcile', on: :member
end

So, option 2 could presumably be implemented by promoting the concept of reconcile to a resource with #show and #update actions (and creating a corresponding ReconciliationsController to house these actions), e.g.

resources :accounts, concerns: [:favouritable] do
  resources :transactions, only: [:index], concerns: %i[reconcilable defaultable]
  resource :reconcile, only: %i[show update]
end

On the front-end, we would need to modify the accountModel with a new method to call the new endpoint.

Choice

For simplicity, a choice was made to go with the first option.

The thinking being that the reconciliation process is initiated from the /accounts/:id/transactions screen, at which point we have already fetched the account as part of resolving the UI state (i.e. this.context in the TransactionIndexController), and we can just reference the balances from there without having to make another API request.

It later became apparent that these balances can easily become stale, e.g. if the user manually toggles a transaction status by clicking the status icon, or if the user cancels and restarts a partial reconciliation.

This led us to add code in TransactionIndexController.reconcile() to flush the account from the cache and refetch it again, to ensure we start the reconciliation with the most up to date balances.

Given that we are now always refetching the account whenever we initiate the reconciliation process, the reasons that led us to choose option 1 are no longer valid; and we should consider whether it now makes more sense to go with option 2.

The advantages being:

  1. We no longer compute & return balances unnecessarily
  2. We don't need to flush the account cache (since the balances wouldn't be cached anyway)
  3. Given that we have to make an API call anyway when starting a reconciliation, we might as well target an endpoint that specifically relates to reconciliation