A Sip Renew request (29) for an item with an associated fee should behave similarly to a checkout request i.e. the initial request has fee acknowledged set to N - server rejects the transaction an responds indicating the fee amount. Self check unit has the responsibility of collecting that fee and can then proceed repeating the request but with fee acknowledged set to Y, this request should then succeed. Currently this is not happening on older versions renew requests are always rejected if there is a fee irrespective of the value of the fee acknowledged field. On master there is an additional problem that the server is crashing due to a runtime error when processing the fee. This bug has not manifested for many users as 3m and most other self-check units actually send a checkout request not a renewal request when renewing items. Newer biblioteca units do send a renew request and expect this functionality
Example of correct responses: Item has a rental fee of 2.00 renew request sent with field BO set to N 300NUN20181025 110552AA23529001223675|AB502326000833|AJAlien nation|AH|BV2.00|BHUSD|BT06|AOCPL| same request sent with field BO now set to Y 301YNN20181025 110552AA23529001223675|AB502326000833|AJAlien nation|AH20181030 235900|CK|BV2.00|BHUSD|BT06|AOCPL| Key difference is first field after the response id '30' 0 == renew rejected, 1 == renew succeeded
Created attachment 81126 [details] [review] patch to stop sipserver crash handling fees This patch is required on master otherwise runtime error occurs in handling fees as C4::Circulation::getnextacctno is not found (i.e. import of routine in package namespace has been overwritten) (bug 21486 was also causing problems but is now in master)
Created attachment 81127 [details] [review] patch to suppress runtime warning Fix runtime error caused by assumption that only errors (with accompanying message) block renewal
Created attachment 81128 [details] [review] Fix error in value not passed to transaction Renewal transaction blocks/allows based on fee_acknowledge field as value was not passed to the renewal transaction code it was always N blocking the transaction. Pass the value similarly to ILS.pm's checkout code for the same purpose
see also bug 21683
(In reply to Josef Moravec from comment #5) > see also bug 21683 Yes if bug 21683 is applied the first if these 3 patches is no longer required
Hi, is there a test plan for this?
I tested as follows, with a set up running SIP in my testing setup (docker, this time): sent this: kohadev-koha@b46acddf5bb7:/kohadevbox/koha$ perl misc/sip_cli_emulator.pl --sip_user=term1 --sip_pass=term1 --patron=2 --item='502326000048' -m renew -a 127.0.0.1 -p 6001 -l MAIN Trying 'renew' SEND: 29NN20190508 23053520190508 230535AOMAIN|AA2|AB502326000048|AJ|ACterm1|BON| READ: 300NUN20190508 230535AA2|AB502326000048|AJWorking in health care :|AH|BV3.00|BHUSD|BT06|AOCPL| Item was not renewed. I couldn't get the emulator to do something sensible, so I used telnet for the next part: root@b46acddf5bb7:koha(bug21658-qa)$ telnet localhost 6001 Trying 127.0.0.1... Connected to localhost. Escape character is '^]'. 9300CNterm1|COterm1|CPMAIN| 941 29NN20190508 23055020190508 230550AOMAIN|AA2|AB502326000048|AJ|ACterm1|BOY| 301YNN20190508 230653AA2|AB502326000048|AJWorking in health care :|AH20190523 235900|CK|BV3.00|BHUSD|BT06|AOCPL| The item was renewed, and the fee was accepted (evidence in the interface and in the string), like I think I was expecting. The SIP server also didn't crash at any point here, so I think that's winning. Liz
Created attachment 89487 [details] [review] Bug 21658 Define string to avoid run time warnings In addition to an explicit error a renewal may be rejected because a fee is due and has not yet been flagged as paid. In this case define the error message as empty string to avoid runtime errors in the regexes performed on the standard error messages Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 89488 [details] [review] Bug 21658 Pass fee_ack value to Renew transaction Response to a renew request should mirror that of checkout However fee_ack was not being passed to the renewal transaction so that it was always 'N' not allowing the renewal to proceed even although the fee was paid This bug was not seen with many units as they use checkout requests both for an initial issue and renewal. Code now passes fee_ack as is done for checkout Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Fixed by bug 21683