The check for fines amount when using this pref fetches the patrons fines, then adds the total fines with guarantors, including this borrowers fines. This means the patrons own fines are doubled
Created attachment 128863 [details] [review] Bug 29754: Unit test This adds a unit test to the SIP code It also adds some spacing and dividers to make the tests for 'relationships_debt' easier to read To test: prove -v t/db_dependent/Koha/Patron.t - passes and is readable prove -v t/db_dependent/SIP/Patron.t - fails
Created attachment 128864 [details] [review] Bug 29754: Don't include patron when fetching guarantees fines This patch removes the flag to fetch the patrons own debts as we have already calculated them This test plan highlights another issue - the value of NoIssuesChargeGuarantorsWithGuarantees is not checked in SIP code - we merely get the total owed including guarantees and compares to noissuescharge See bug 29755 To test: 1 - Apply first patch only 2 - prove t/db_dependent/SIP/Patron.t 3 - It fails 4 - Enable NoIssuesChargeGuarantorsWithGuarantees, you can set to 1 5 - Create/find a patron 6 - Add a $10 charge to parons account 7 - Add a guarantee to patron 8 - Add a $5 charge to guarantee 9 - Set noissuescharge to 15.01 10 - perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL -m patron_information --patron BARCODE 11 - Note the 64 message starts with Y's that mean patron is blocked 12 - Patron shouldn't be blocked as under 15.01 13 - Apply patch 14 - Restart all 15 - repeat 10 16 - Patron is no longer blocked
Ah, simpler to check correct total, 'BV' field in SIP response will be 25 before patch, 15 after
Created attachment 129732 [details] [review] Bug 29754: Unit test This adds a unit test to the SIP code It also adds some spacing and dividers to make the tests for 'relationships_debt' easier to read To test: prove -v t/db_dependent/Koha/Patron.t - passes and is readable prove -v t/db_dependent/SIP/Patron.t - fails Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 129733 [details] [review] Bug 29754: Don't include patron when fetching guarantees fines This patch removes the flag to fetch the patrons own debts as we have already calculated them This test plan highlights another issue - the value of NoIssuesChargeGuarantorsWithGuarantees is not checked in SIP code - we merely get the total owed including guarantees and compares to noissuescharge See bug 29755 To test: 1 - Apply first patch only 2 - prove t/db_dependent/SIP/Patron.t 3 - It fails 4 - Enable NoIssuesChargeGuarantorsWithGuarantees, you can set to 1 5 - Create/find a patron 6 - Add a $10 charge to parons account 7 - Add a guarantee to patron 8 - Add a $5 charge to guarantee 9 - Set noissuescharge to 15.01 10 - perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL -m patron_information --patron BARCODE 11 - Note the 64 message starts with Y's that mean patron is blocked 12 - Patron shouldn't be blocked as under 15.01 13 - Apply patch 14 - Restart all 15 - repeat 10 16 - Patron is no longer blocked Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Logic follows, this works as expected and includes the requisite unit tests. Signing off.
Created attachment 130213 [details] [review] Bug 29754: Unit test This adds a unit test to the SIP code It also adds some spacing and dividers to make the tests for 'relationships_debt' easier to read To test: prove -v t/db_dependent/Koha/Patron.t - passes and is readable prove -v t/db_dependent/SIP/Patron.t - fails Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 130214 [details] [review] Bug 29754: Don't include patron when fetching guarantees fines This patch removes the flag to fetch the patrons own debts as we have already calculated them This test plan highlights another issue - the value of NoIssuesChargeGuarantorsWithGuarantees is not checked in SIP code - we merely get the total owed including guarantees and compares to noissuescharge See bug 29755 To test: 1 - Apply first patch only 2 - prove t/db_dependent/SIP/Patron.t 3 - It fails 4 - Enable NoIssuesChargeGuarantorsWithGuarantees, you can set to 1 5 - Create/find a patron 6 - Add a $10 charge to parons account 7 - Add a guarantee to patron 8 - Add a $5 charge to guarantee 9 - Set noissuescharge to 15.01 10 - perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL -m patron_information --patron BARCODE 11 - Note the 64 message starts with Y's that mean patron is blocked 12 - Patron shouldn't be blocked as under 15.01 13 - Apply patch 14 - Restart all 15 - repeat 10 16 - Patron is no longer blocked Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 130215 [details] [review] Bug 29754: (QA follow-up) Minor tests tweaks This patch does two things: - Wraps the new SIP tests in a single transaction. We do that to avoid sub-test interference, specially when test files get big. The provided tests are correct and don't require previous tests data. - Migrated from the introduced 'diag' calls to 'note', as I just learned they are the same, but 'note' is only displayed when in verbose mode. Which is great to avoid unnecessary noise in Jenkins. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Pushed to master for 22.05, thanks to everybody involved [U+1F984]
Pushed to 21.11.x for 21.11.03
Pushed to 21.05.x for 21.05.11
Backported: Pushed to 20.11.x branch for 20.11.17
Does this need to be backported to 19.11.x?