Bug 32624 - Patrons fines are not accurate in SIP2 when NoIssuesChargeGuarantorsWithGuarantees or NoIssuesChargeGuarantees are enabled
Summary: Patrons fines are not accurate in SIP2 when NoIssuesChargeGuarantorsWithGuara...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 29755
Blocks: 33055
  Show dependency treegraph
 
Reported: 2023-01-13 11:52 UTC by Nick Clemens
Modified: 2023-12-28 20:44 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.03, 22.05.10,21.11.18


Attachments
Bug 32624: Don't include guarantee or guarantor fines in BV (fines amount) in SIP messages (6.45 KB, patch)
2023-01-13 19:29 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 32624: Don't include guarantee or guarantor fines in BV (fines amount) in SIP messages (6.50 KB, patch)
2023-01-17 00:32 UTC, David Nind
Details | Diff | Splinter Review
Bug 32624: Don't include guarantee or guarantor fines in BV (fines amount) in SIP messages (6.56 KB, patch)
2023-02-07 13:31 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2023-01-13 11:52:18 UTC
When these prefs are active we add the fines for guarantors/guarantees to the patrons total fines in order to check against the limits.

We also then transmit this total as the fines owed by the patron.

On SIP machines that take payments, this is read as the amount owed, and when a patron pays this amount they end up with a credit on their account, and no way to transfer that balance to the charges for related patrons.

I think we should report when blocked by other patrons, but only transmit the patrons own fines
Comment 1 Nick Clemens 2023-01-13 19:29:50 UTC
Created attachment 145304 [details] [review]
Bug 32624: Don't include guarantee or guarantor fines in BV (fines amount) in SIP messages

This patch chanegs the code to report only the patron's personal fines, and to report a block
from other fines in the screen message

This is to prevent overpayment on accounts from SIP machines

To test:
1 - prove t/db_dependent/SIP/Patron.t
Comment 2 David Nind 2023-01-17 00:32:03 UTC
Created attachment 145345 [details] [review]
Bug 32624: Don't include guarantee or guarantor fines in BV (fines amount) in SIP messages

This patch chanegs the code to report only the patron's personal fines, and to report a block
from other fines in the screen message

This is to prevent overpayment on accounts from SIP machines

To test:
1 - prove t/db_dependent/SIP/Patron.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Suzanne 2023-02-03 03:05:18 UTC
This has caused a lot of problems at our library. Patrons end up trying to pay over and over and the fines won't go away (because they are fines from another account) and then they have a bunch of credit on their library account and we have to go find the other family member cards and resolve the fines manually. Would love to see this get fixed as soon as possible. Thank you for working on it!
Comment 4 Martin Renvoize 2023-02-07 06:48:04 UTC
My initial reaction to this is that really we aught to be allowing payment of guarantees fines via SIP as apposed to overruling the blocking rules the way we are here.

But, I need to dig through the code a bit here before I block it, I've not done that yet and this may be a decent intermediate work around whilst we work on a wider problem of guarantees fines.
Comment 5 Martin Renvoize 2023-02-07 08:16:06 UTC
OK.. looked at the code now.. seems reasonable.

Will work through QA today.
Comment 6 Martin Renvoize 2023-02-07 13:31:25 UTC
Created attachment 146329 [details] [review]
Bug 32624: Don't include guarantee or guarantor fines in BV (fines amount) in SIP messages

This patch chanegs the code to report only the patron's personal fines, and to report a block
from other fines in the screen message

This is to prevent overpayment on accounts from SIP machines

To test:
1 - prove t/db_dependent/SIP/Patron.t

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2023-02-07 13:31:54 UTC
Works as described, fixes a real issue and is well covered by tests.

Passing QA
Comment 8 Tomás Cohen Arazi 2023-02-09 13:01:40 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 9 Jacob O'Mara 2023-02-13 09:46:14 UTC
Nice work, thanks everyone!

Pushed to 22.11.x for the next release.
Comment 10 Lucas Gass 2023-02-14 22:47:13 UTC
Backported to 22.05.x for upcoming 22.05.10
Comment 11 Arthur Suzuki 2023-02-17 12:39:50 UTC
applied to 21.11.x for 21.11.18
Comment 12 wainuiwitikapark 2023-03-15 02:15:37 UTC
Not backported to 21.05.x