Topic: Edit bank transfer

I've implemented a bank transfer edit feature.  The patch is available here.

If you have some mechanism for accepting pull requests via either git or hg do let me know.

E2E tests are also available in my frontaccounting-wrapper repo.  If you're interested in incorporating the e2e tests it would be nice to push the current code down one level and have the tests in a folder that is not in the main src (htdocs) tree.

Cambell https://github.com/cambell-prince

Re: Edit bank transfer

Joe please add this.

Re: Edit bank transfer

Janusz is about to look at this.

Joe

Re: Edit bank transfer

@cambell

Nice feature, however it needs some cleanups to be integrated in FA sources.

1. Edition can easily lead to negative balance on account which is not acceptable especially accounts of  on cash type. At least check_valid_entries() has to be  modified to forbid such situations testing change in transfer value against future account history.  But generally I guess it will be hard to achieve when edition is done as void/insert sequence instead of transaction modification, and without any limits (keep in mind situation, when one changes not only amount but also to/from accounts during edition).
2. Voided entries are still visible as zero amount transfers in bank account history.
3. There are a couple of small problems with variables declarations, which are not visible until debugging mode is on. Please set  $go_debug=1 in your config.php file during development to see it.
4. For better UI consistency I would also add edition links to bank transfers in 'Bank Account Inquiry' page.

Janusz

Re: Edit bank transfer

itronics wrote:

1. Edition can easily lead to negative balance on account which is not acceptable especially accounts of  on cash type. At least check_valid_entries() has to be  modified to forbid such situations testing change in transfer value against future account history.  But generally I guess it will be hard to achieve when edition is done as void/insert sequence instead of transaction modification, and without any limits (keep in mind situation, when one changes not only amount but also to/from accounts during edition).

In my testing cash accounts cannot go below zero when submitting the edition (red error box is displayed), but the current account can.  I think this is consistent with the current behavior of FA which allows -ve balance on the current account.   Let me know if there's a particular scenario you've got in mind.

itronics wrote:

2. Voided entries are still visible as zero amount transfers in bank account history.

Can you help me to see that?  Where do I get that view?  I see zero amounts for all manner of voided items in the bank reconciliation screen, and I can see zero amount transfers in the reconciliation also.

What's the preferred way to not display them.  a) delete them? (I'm guessing not)  b) not display them via some 'if voided' type function the don't display. c) something else?

itronics wrote:

3. There are a couple of small problems with variables declarations, which are not visible until debugging mode is on. Please set  $go_debug=1 in your config.php file during development to see it.

Yes, I can see those.  I'll go fix them.

itronics wrote:

4. For better UI consistency I would also add edition links to bank transfers in 'Bank Account Inquiry' page.

Will do.
Janusz

Cambell https://github.com/cambell-prince

Re: Edit bank transfer

Consider following simple scenario:
1. Create new Bank Account of cash type;
2. Transfer $100 to this account from any other account;
3. Entry $60 payment to supplier from the cash account;
4. Edit transfer created in step 2 changing amount from $100 to $50

and you will end with -$10 balance on the cash account without any warning nor error.
The same negative balance issue you can trigger by simply changing target account to any other one during edition.

Regarding the zero entries displayed in bank account inquire they just should be omitted in Bank Inquiry listing. This can be achieved by sql modification in get_bank_trans_for_bank_account() function.

Janusz

Re: Edit bank transfer

Is it completed now at:
https://github.com/cambell-prince/frontaccounting/commits/feature/tempBankTransferEdit
?

8 (edited by cambell 10/08/2014 12:03:17 pm)

Re: Edit bank transfer

No not yet.  See my Trello board if you want to track progress on anything I'm doing with FrontAccounting.

Cambell https://github.com/cambell-prince

Re: Edit bank transfer

@Janusz Items 2, 3, and 4 are complete.  I've attempted 1 but I'm not that satisfied.  For example consider the following scenario:

1. Add a transfer $100 from current -> cash account at date d
2. Add a transfer $60 from cash -> current account at date d + 2 weeks
3. Add a transfer $80 from cash -> current account at date d + 1 week.

This succeeds and results in a -ve balance in the cash account of -40 in the existing code.

I've attempted a fix by setting a future date in the validation.  Please have a look and review the fix here.

This fix would fix your review #1 but would also change the existing functionality.

Note that I'm not proposing hard coding the date 2099.  I'd like to know what future date from the perspective of the transfer the date should be.  End of FY?  Current date?  Something else?

The bank edit feature can be compared to current master here.

Cambell https://github.com/cambell-prince

Re: Edit bank transfer

Please look closer on the check_bank_account_history() algorithm. This function scans all the bank history to find whether atomic change in balance made on some date is safe or not.

If you try implement transfer edition as void/entry sequence in general you have take into consideration two dates:
1. original date for which old transaction is voided (check against -ve balance in target account);
2. new date for which new transaction is entered (check against -ve balance in source account).

In addition you should consider special situation, when only amount changes (but from/to account and date don't): in this case only change in amount on the old transaction date should be tested, otherwise you can have 'false positive' results in case of small change made on big transfer amount.

Janusz

Re: Edit bank transfer

Ok. I think I've got it.  I'll add a check to ensure that voiding the original transaction does not result in a -ve balance unless the new transaction would correct that before it happens.  This should cover the special case.  The new transaction is covered by existing code.

Cambell https://github.com/cambell-prince

Re: Edit bank transfer

Updated code is here.  Unit tests are here.

Cambell https://github.com/cambell-prince

Re: Edit bank transfer

@Janusz - please check to see if it can be included in the core with or without a flag in the config.