nazrinharris / pecunia

Massively ambitious personal finance app.
1 stars 0 forks source link

Reported balance and actual balance may look unequal when it actually is #127

Closed nazrinharris closed 10 months ago

nazrinharris commented 10 months ago

I'm guessing this is also an issue with riverpod refresh. A restart of the app shows that the balance are the same. Happened when creating a transfer transaction. Reported balance is whats wrong.

nazrinharris commented 10 months ago

Okay, I think this stems from the account retrieved being stale. Maybe a watchAllWrites in getAccountById is needed

nazrinharris commented 10 months ago

This is very related to #106, as this issue pops back up when I added a watchAllWrites in getAccountByIdProvider. The culprit:

class SourceAccountDropdown extends HookConsumerWidget {
  const SourceAccountDropdown(this.chosenSourceAccount, this.chosenDestinationAccount, this.accountsList,
      {super.key});

  final ValueNotifier<Account> chosenSourceAccount;
  final ValueNotifier<Account?> chosenDestinationAccount;
  final List<Account> accountsList;

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final ids = accountsList.map((account) => account.id);
    final uniqueIds = ids.toSet();
    if (ids.length != uniqueIds.length) {
      print('There are duplicate accounts in the list');
    }
    if (!accountsList.contains(chosenSourceAccount.value)) {
      print('The chosen source account is not in the accounts list');
      // You may want to handle this case appropriately
    }

    return DropdownButtonFormField<Account>(
      isExpanded: true,
      value: chosenSourceAccount.value,
      onChanged: (Account? newSource) {
        if (chosenDestinationAccount.value != null && newSource!.id == chosenDestinationAccount.value!.id) {
          chosenDestinationAccount.value = chosenSourceAccount.value;
        }
        chosenSourceAccount.value = newSource!;
      },
      items: accountsList.map<DropdownMenuItem<Account>>((Account value) {
        return DropdownMenuItem<Account>(
          value: value,
          child: Text(value.name),
        );
      }).toList(),
      hint: Text('Select Source Account'),
      decoration: InputDecoration(),
    );
  }
}

So, the problem here is chosenSourceAccount is stale, while accountsList is new. That is, in the way that I have made it, when getAccountById gets refreshed (like when the TransferTransaction gets created), it gets an up-to-date account, and for a moment, the accountsList (which hasn't yet been refreshed), still has the old list, causing a mismatch in balance.

A simple workaround is to have the Dropdown take just the id and name, rather than a whole Account object. But there may be times where I DO want the whole account object (say in the future I'd enhance the choose account list to include balance).

nazrinharris commented 10 months ago

I think the best way to do this is to merge the provision of chosenSourceAccount and accountsList in such a way that when either one of them is loading, the form itself wouldn't show.

nazrinharris commented 10 months ago

This solution is pretty neat. At the cost of retrieving the list of accounts ahead of time, I'm able to make the CreateTransferTxnForm not depend on any providers.

nazrinharris commented 10 months ago

Should be completed with ea99b17 in #118