We need a method that automatically applies outstanding credits to outstanding debits.
Created attachment 82684 [details] [review] Bug 21896: Add unit tests for Koha::Account::normalize_balance
Created attachment 82685 [details] [review] [SIGNED OFF]Bug 21896: Add Koha::Account::normalize_balance This patch adds the normalize_balance() method to Koha::Account. Its purpose is to apply outstanding credits (i.e. manual ones ore remaining amounts like in the case of refunds) to outstanding debts. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => SUCCESS: Tests pass! - Sign off :-D
Created attachment 82688 [details] [review] [SIGNED OFF]Bug 21896: Add unit tests for Koha::Account::normalize_balance
Created attachment 82689 [details] [review] [SIGNED OFF]Bug 21896: Add unit tests for Koha::Account::normalize_balance
Created attachment 82690 [details] [review] [SIGNED OFF]Bug 21896: Add unit tests for Koha::Account::normalize_balance
Created attachment 82691 [details] [review] [SIGNED OFF]Bug 21896: Add Koha::Account::normalize_balance This patch adds the normalize_balance() method to Koha::Account. Its purpose is to apply outstanding credits (i.e. manual ones ore remaining amounts like in the case of refunds) to outstanding debts. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => SUCCESS: Tests pass! - Sign off :-D
Created attachment 82692 [details] [review] Bug 21896: Add unit tests for Koha::Account::normalize_balance
Sorry, I am struggling with the sign off process still. :/ I hope I didn't screw anything up.
First comment is that I think I prefer the term 'reconcile' to the term 'normalize'.. To me reconcile means to apply one action against each other to tend towards a 'happy place', i.e. apply credits against debits to work towards balance zero, in the accounting sense... whereas normalize means to 'make the same for comparison' so you can continue with calculations, so the accounting example here would be to convert all accounts in various currencies into one 'normalized' currency before doing calculations upon them. Hope that makes sense.. maybe it's just my scientific background speaking out. Second comment is a question.. do we need to do anything logical in terms of the order of application of credits against debits.. i.e. should it be oldest debts first, smallest debts first, blocking debts first for example?
Created attachment 82701 [details] [review] Bug 21896: Add unit tests for Koha::Account::normalize_balance Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Created attachment 82702 [details] [review] Bug 21896: Add Koha::Account::normalize_balance This patch adds the normalize_balance() method to Koha::Account. Its purpose is to apply outstanding credits (i.e. manual ones ore remaining amounts like in the case of refunds) to outstanding debts. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Created attachment 82703 [details] [review] Bug 21896: (QA follow-up) normalize_balance -> reconcile_balance rename This was requested on the QA review and I agree. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 82720 [details] [review] Bug 21896: Add unit tests for Koha::Account::normalize_balance Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 82721 [details] [review] Bug 21896: Add Koha::Account::normalize_balance This patch adds the normalize_balance() method to Koha::Account. Its purpose is to apply outstanding credits (i.e. manual ones ore remaining amounts like in the case of refunds) to outstanding debts. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 82722 [details] [review] Bug 21896: (QA follow-up) normalize_balance -> reconcile_balance rename This was requested on the QA review and I agree. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 82735 [details] [review] Bug 21896: Add unit tests for Koha::Account::normalize_balance Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 82736 [details] [review] Bug 21896: Add Koha::Account::normalize_balance This patch adds the normalize_balance() method to Koha::Account. Its purpose is to apply outstanding credits (i.e. manual ones ore remaining amounts like in the case of refunds) to outstanding debts. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 82737 [details] [review] Bug 21896: (QA follow-up) normalize_balance -> reconcile_balance rename This was requested on the QA review and I agree. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 82738 [details] [review] Bug 21896: (QA follow-up) Document and Test for FIFO behaviour A short dicussion lead to the decision to make it explictly clear that this method will implicitly apply credits against debits in a 'First In First Out' manor, meaning oldest outstanding debits will be paid off first. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
I've added a followup to document the FIFO behaviour of this function and a supplemental test to guarantee the current implicit behaviour. Otherwise, I like this now.. Passing QA
Created attachment 82739 [details] [review] Bug 21896: (QA follow-up) Add tests for FIFO behaviour Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 82740 [details] [review] Bug 21896: (QA follow-up) Document and Test for FIFO behaviour A short dicussion lead to the decision to make it explictly clear that this method will implicitly apply credits against debits in a 'First In First Out' manor, meaning oldest outstanding debits will be paid off first. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 82742 [details] [review] Bug 21896: (QA follow-up) Document and Test for FIFO behaviour A short dicussion lead to the decision to make it explictly clear that this method will implicitly apply credits against debits in a 'First In First Out' manor, meaning oldest outstanding debits will be paid off first. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Awesome work everyone! Pushed to master for 19.05
Created attachment 82811 [details] [review] [18.05.x] Bug 21896: Add unit tests for Koha::Account::normalize_balance Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 82812 [details] [review] [18.05.x] Bug 21896: Add Koha::Account::normalize_balance This patch adds the normalize_balance() method to Koha::Account. Its purpose is to apply outstanding credits (i.e. manual ones ore remaining amounts like in the case of refunds) to outstanding debts. To test: - Apply this patchset - Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 82813 [details] [review] [18.05.x] Bug 21896: (QA follow-up) normalize_balance -> reconcile_balance rename This was requested on the QA review and I agree. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 82814 [details] [review] [18.05.x] Bug 21896: (QA follow-up) Add tests for FIFO behaviour Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 82815 [details] [review] [18.05.x] Bug 21896: (QA follow-up) Document and Test for FIFO behaviour A short dicussion lead to the decision to make it explictly clear that this method will implicitly apply credits against debits in a 'First In First Out' manor, meaning oldest outstanding debits will be paid off first. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 82820 [details] [review] Bug 21896: RM follow-up - fix tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Pushed to 18.11.x for 18.11.01. This is early enough in the release to be pushed even though it's technically an enhancement and as it is required as part of the fix for the critical bug 18805 I believe the justification is sound.
Will not be backported to 18.05
*** Bug 22233 has been marked as a duplicate of this bug. ***
We had a question on the mailing list and another in a bug report about this one for 18.05. Not sure how hard backporting would bea (lots of missing dependencies)?
(In reply to Katrin Fischer from comment #34) > We had a question on the mailing list and another in a bug report about this > one for 18.05. Not sure how hard backporting would bea (lots of missing > dependencies)? I don't think it is that bad, and we could have a 18.05 version of this if required. It was the RMaint call not to backport it. Maybe the new RMaints change their mindS?