Summary: | Patron fines counted twice for SIP when NoIssuesChargeGuarantorsWithGuarantees is enabled | ||
---|---|---|---|
Product: | Koha | Reporter: | Nick Clemens <nick> |
Component: | SIP2 | Assignee: | Nick Clemens <nick> |
Status: | CLOSED FIXED | QA Contact: | Tomás Cohen Arazi <tomascohen> |
Severity: | major | ||
Priority: | P5 - low | CC: | andrewfh, kyle, martin.renvoize, tomascohen, victor, wainuiwitikapark |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29755 | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
22.05.00,21.11.03,21.05.11,20.11.17
|
|
Attachments: |
Bug 29754: Unit test
Bug 29754: Don't include patron when fetching guarantees fines Bug 29754: Unit test Bug 29754: Don't include patron when fetching guarantees fines Bug 29754: Unit test Bug 29754: Don't include patron when fetching guarantees fines Bug 29754: (QA follow-up) Minor tests tweaks |
Description
Nick Clemens
2021-12-22 15:23:02 UTC
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 🦄 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? |