Summary: | SIP2 code does not correctly handle NoIssuesChargeGuarantees or NoIssuesChargeGuarantorsWithGuarantees | ||
---|---|---|---|
Product: | Koha | Reporter: | Nick Clemens <nick> |
Component: | SIP2 | Assignee: | Nick Clemens <nick> |
Status: | RESOLVED FIXED | QA Contact: | Testopia <testopia> |
Severity: | major | ||
Priority: | P5 - low | CC: | arthur.suzuki, david, magnus, martin.renvoize, wainuiwitikapark |
Version: | master | Keywords: | rel_21_11_candidate |
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29754 | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This fixes SIP2 so that it correctly determines if issues should be blocked for patrons when the NoIssuesChargeGuarantees and NoIssuesChargeGuarantorsWithGuarantees system preferences are set. Currently, it only checks the noissuescharge system preference as the limit for charges, and not the other 'No Issues charge' system preferences.
|
Version(s) released in: |
22.05.00,21.11.18
|
Bug Depends on: | |||
Bug Blocks: | 32624, 33411 | ||
Attachments: |
Bug 29755: Check each NoIssuesCharge separately
Bug 29755: Check each NoIssuesCharge separately Bug 29755: Check each NoIssuesCharge separately Bug 29755: (follow-up) Mock NoIssuesChargeGuarantorsWithGuarantees in test Bug 29755: Check each NoIssuesCharge separately Bug 29755: (follow-up) Mock NoIssuesChargeGuarantorsWithGuarantees in test Bug 29755: Check each NoIssuesCharge separately Bug 29755: (follow-up) Mock NoIssuesChargeGuarantorsWithGuarantees in test |
Description
Nick Clemens
2021-12-22 15:55:22 UTC
Created attachment 130449 [details] [review] Bug 29755: Check each NoIssuesCharge separately This patch updates SIP patron code to use account methods to calculate balances over the patronflags returns. It also checks if patron should be blocked for each 'No Issues charge' preference Tests are added for NoIssuesChargeGuarantees To test: 1 - Set noissuescharge preference to 5 2 - Add a $10 charge to a patron 3 - perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL -m patron_information --patron BARCODE 4 - Note the 64 message starts with Y's that mean patron is blocked 5 - Set noissuescharge to 11 6 - Repeat 3, patron is no longer blocked 7 - Set NoIssuesChargeGuarantees to 8 8 - Repeat 3, patron is blocked 9 - Pay $3 on patron so they owe 7 10 - Repeat 3, patron is not blocked 11 - Add a child account with patron as guarantor 12 - Repeat 3, patron is not blocked 13 - Add a $4 charge to child 14 - Repeat 3, patron is blocked 15 - Repeat 3, but with child barcode, child is not blocked 16 - Set NoIssuesChargeGuarantorsWithGuarantees to 11 17 - Repeat 3, patron is blocked 18 - Repeat 3 with child barcode, child is blocked Created attachment 134196 [details] [review] Bug 29755: Check each NoIssuesCharge separately This patch updates SIP patron code to use account methods to calculate balances over the patronflags returns. It also checks if patron should be blocked for each 'No Issues charge' preference Tests are added for NoIssuesChargeGuarantees To test: 1 - Set noissuescharge preference to 5 2 - Add a $10 charge to a patron 3 - perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL -m patron_information --patron BARCODE 4 - Note the 64 message starts with Y's that mean patron is blocked 5 - Set noissuescharge to 11 6 - Repeat 3, patron is no longer blocked 7 - Set NoIssuesChargeGuarantees to 8 8 - Repeat 3, patron is blocked 9 - Pay $3 on patron so they owe 7 10 - Repeat 3, patron is not blocked 11 - Add a child account with patron as guarantor 12 - Repeat 3, patron is not blocked 13 - Add a $4 charge to child 14 - Repeat 3, patron is blocked 15 - Repeat 3, but with child barcode, child is not blocked 16 - Set NoIssuesChargeGuarantorsWithGuarantees to 11 17 - Repeat 3, patron is blocked 18 - Repeat 3 with child barcode, child is blocked Signed-off-by: David Nind <david@davidnind.com> After signing off, I ran the tests and they failed: prove t/db_dependent/SIP/Patron.t t/db_dependent/SIP/Patron.t .. 8/10 # Failed test 'Guarantee only fines correctly counted' # at t/db_dependent/SIP/Patron.t line 318. # got: '11.11' # expected: '0.11' # Failed test 'Guarantee not blocked by guarantor fines' # at t/db_dependent/SIP/Patron.t line 319. # Looks like you failed 2 tests of 4. t/db_dependent/SIP/Patron.t .. 9/10 # Failed test 'NoIssuesChargeGuarantees tests' # at t/db_dependent/SIP/Patron.t line 322. t/db_dependent/SIP/Patron.t .. 10/10 # Looks like you failed 1 test of 10. t/db_dependent/SIP/Patron.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/10 subtests Test Summary Report ------------------- t/db_dependent/SIP/Patron.t (Wstat: 256 Tests: 10 Failed: 1) Failed test: 9 Non-zero exit status: 1 Files=1, Tests=10, 9 wallclock secs ( 0.04 usr 0.01 sys + 7.56 cusr 1.12 csys = 8.73 CPU) Result: FAIL Also, for steps 16-18 of the test plan, I think step 16 needs changing to "16 - Set NoIssuesChargeGuarantorsWithGuarantees to 10", rather than 11. - If 11 is used, the patrons and the child are not blocked. - The wording for the system preference is "Prevent a patron from checking out if the patron has guarantors and those guarantor's guarantees owing in total more than X USD in fines." - The key part is "MORE THAN X USD". In the test, the total is 11. Created attachment 134221 [details] [review] Bug 29755: Check each NoIssuesCharge separately This patch updates SIP patron code to use account methods to calculate balances over the patronflags returns. It also checks if patron should be blocked for each 'No Issues charge' preference Tests are added for NoIssuesChargeGuarantees To test: 1 - Set noissuescharge preference to 5 2 - Add a $10 charge to a patron 3 - perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL -m patron_information --patron BARCODE 4 - Note the 64 message starts with Y's that mean patron is blocked 5 - Set noissuescharge to 11 6 - Repeat 3, patron is no longer blocked 7 - Set NoIssuesChargeGuarantees to 8 8 - Repeat 3, patron is blocked 9 - Pay $3 on patron so they owe 7 10 - Repeat 3, patron is not blocked 11 - Add a child account with patron as guarantor 12 - Repeat 3, patron is not blocked 13 - Add a $4 charge to child 14 - Repeat 3, patron is blocked 15 - Repeat 3, but with child barcode, child is not blocked 16 - Set NoIssuesChargeGuarantorsWithGuarantees to 10 17 - Repeat 3, patron is blocked 18 - Repeat 3 with child barcode, child is blocked Signed-off-by: David Nind <david@davidnind.com> Created attachment 134222 [details] [review] Bug 29755: (follow-up) Mock NoIssuesChargeGuarantorsWithGuarantees in test Created attachment 134297 [details] [review] Bug 29755: Check each NoIssuesCharge separately This patch updates SIP patron code to use account methods to calculate balances over the patronflags returns. It also checks if patron should be blocked for each 'No Issues charge' preference Tests are added for NoIssuesChargeGuarantees To test: 1 - Set noissuescharge preference to 5 2 - Add a $10 charge to a patron 3 - perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL -m patron_information --patron BARCODE 4 - Note the 64 message starts with Y's that mean patron is blocked 5 - Set noissuescharge to 11 6 - Repeat 3, patron is no longer blocked 7 - Set NoIssuesChargeGuarantees to 8 8 - Repeat 3, patron is blocked 9 - Pay $3 on patron so they owe 7 10 - Repeat 3, patron is not blocked 11 - Add a child account with patron as guarantor 12 - Repeat 3, patron is not blocked 13 - Add a $4 charge to child 14 - Repeat 3, patron is blocked 15 - Repeat 3, but with child barcode, child is not blocked 16 - Set NoIssuesChargeGuarantorsWithGuarantees to 10 17 - Repeat 3, patron is blocked 18 - Repeat 3 with child barcode, child is blocked Signed-off-by: David Nind <david@davidnind.com> Created attachment 134299 [details] [review] Bug 29755: (follow-up) Mock NoIssuesChargeGuarantorsWithGuarantees in test Signed-off-by: David Nind <david@davidnind.com> Created attachment 134725 [details] [review] Bug 29755: Check each NoIssuesCharge separately This patch updates SIP patron code to use account methods to calculate balances over the patronflags returns. It also checks if patron should be blocked for each 'No Issues charge' preference Tests are added for NoIssuesChargeGuarantees To test: 1 - Set noissuescharge preference to 5 2 - Add a $10 charge to a patron 3 - perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL -m patron_information --patron BARCODE 4 - Note the 64 message starts with Y's that mean patron is blocked 5 - Set noissuescharge to 11 6 - Repeat 3, patron is no longer blocked 7 - Set NoIssuesChargeGuarantees to 8 8 - Repeat 3, patron is blocked 9 - Pay $3 on patron so they owe 7 10 - Repeat 3, patron is not blocked 11 - Add a child account with patron as guarantor 12 - Repeat 3, patron is not blocked 13 - Add a $4 charge to child 14 - Repeat 3, patron is blocked 15 - Repeat 3, but with child barcode, child is not blocked 16 - Set NoIssuesChargeGuarantorsWithGuarantees to 10 17 - Repeat 3, patron is blocked 18 - Repeat 3 with child barcode, child is blocked Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 134726 [details] [review] Bug 29755: (follow-up) Mock NoIssuesChargeGuarantorsWithGuarantees in test Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Nice to see this migration to using Koha::Patron objects and methods.. works as expected and no regressions found. Tests all passing and QA script is happy PQA Pushed to master for 22.05, thanks to everybody involved 🦄 applied to 21.11.x for 21.11.18 Not backported to 21.05.x |