The current implementation checks 'amountoutstanding' matches what's expected for debits (i.e. > 0) and credits (i.e. < 0). But there are pathological use cases in Koha (that we need to deal with), specially related to accruing fines and back dated returns (and so dropbox mode) that trigger false positives (specifically debits with amountoutstanding being negative, then meaning they are actually credits).
Created attachment 82939 [details] [review] Bug 21969: Regression tests This patch introduces regression tests for Koha::Account::outstanding_* methods so they don't pick wrong lines when amountoutstanding matches what we are looking for (i.e. negative for credits and positive for debits). To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => FAIL: Tests fail because pathological account lines are wrongly picked.
Created attachment 82940 [details] [review] Bug 21969: Fix Koha::Account->outstanding_* matching the opposite on pathological cases This patch makes outstanding_* methods be safe regarding pathological account lines that get converted into another type because of the value of amountoutstanding To test: - Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => FAIL: Tests fail because pathological account lines are wrongly picked. - Apply this patch - Run: k$ prove t/db_dependent/Koha/Account.t =>SUCCESS: All green!
Created attachment 83011 [details] [review] Bug 21969: Regression tests This patch introduces regression tests for Koha::Account::outstanding_* methods so they don't pick wrong lines when amountoutstanding matches what we are looking for (i.e. negative for credits and positive for debits). To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => FAIL: Tests fail because pathological account lines are wrongly picked. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 83012 [details] [review] Bug 21969: Fix Koha::Account->outstanding_* matching the opposite on pathological cases This patch makes outstanding_* methods be safe regarding pathological account lines that get converted into another type because of the value of amountoutstanding To test: - Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => FAIL: Tests fail because pathological account lines are wrongly picked. - Apply this patch - Run: k$ prove t/db_dependent/Koha/Account.t =>SUCCESS: All green! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
A very sensible change that makes these methods considerable more robust... Signing Off.
Created attachment 83047 [details] [review] Bug 21969: Regression tests This patch introduces regression tests for Koha::Account::outstanding_* methods so they don't pick wrong lines when amountoutstanding matches what we are looking for (i.e. negative for credits and positive for debits). To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => FAIL: Tests fail because pathological account lines are wrongly picked. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 83048 [details] [review] Bug 21969: Fix Koha::Account->outstanding_* matching the opposite on pathological cases This patch makes outstanding_* methods be safe regarding pathological account lines that get converted into another type because of the value of amountoutstanding To test: - Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => FAIL: Tests fail because pathological account lines are wrongly picked. - Apply this patch - Run: k$ prove t/db_dependent/Koha/Account.t =>SUCCESS: All green! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.01
missing dependency for 18.05, won't backport