Koha's SIP2 implementation has facilities to send the fee amount ( BV ) for checking out a given item, but in our code, the fee amount is never set!
Created attachment 31136 [details] [review] Bug 12820 - SIP2 does not send rental charge fee! Koha's SIP2 implementation has facilities to send the fee amount ( BV ) for checking out a given item, but in our code, the fee amount is never set! Test Plan: 1) Check out an item with a rental fee via SIP2. 2) Note the BV field is missing. 3) Apply this patch, restart your SIP2 server. 4) Repeat step 1. 5) Note the BV field is now set to the correct value.
Created attachment 31153 [details] [review] Bug 12820 - SIP2 does not send rental charge fee! Koha's SIP2 implementation has facilities to send the fee amount ( BV ) for checking out a given item, but in our code, the fee amount is never set! Test Plan: 1) Check out an item with a rental fee via SIP2. 2) Note the BV field is missing. 3) Apply this patch, restart your SIP2 server. 4) Repeat step 1. 5) Note the BV field is now set to the correct value. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Should we not be using C4::Circulation::GetIssuingCharges to get the amount? This returns the rental fee but also calculates the discount if one is applicable, that step is omitted in this patch
Hi Colin, thank you for taking a look. I think you are correct, it won't apply the correct rental charge.
Created attachment 33901 [details] [review] Bug 12820 [QA Followup] - Let Koha calculate fee for Checkout
Created attachment 33902 [details] [review] Bug 12820 [QA Followup] - Let Koha calculate fee for Checkout
Is this really a follow up or a new patch?
Created attachment 33911 [details] [review] Bug 12820 - SIP2 does not send rental charge fee! Koha's SIP2 implementation has facilities to send the fee amount ( BV ) for checking out a given item, but in our code, the fee amount is never set! It's even more complicated than that. If there is a fee, the sip server checks to see if BO ( fee acknowledged ) is set to Y. If it is not, the SIP server is supposed to block the checkout and send a screen message with the fee amount. If it is, the checkout proceeds as usual. Test Plan: 1) Apply the patch for bug 13159 2) Set a checkout fee for an itemtype 3) Find an item of the itemtype 4) Run the SIP cli utility: sip_cli_emulator.pl -a <address> -p <port> -su <sip_user> -sp <sip_user_password> -l <location> --patron <patron_cardnumber> -m checkout --item <item_barcode> 5) Note the server returns an AF field describing the fee and doesn't check out the item. 6) Rerun step 4 with the addition of the -fa flag 7) Note the item checks out to the patron this time
Created attachment 33994 [details] [review] Signed Off Patch Tested that with patch applied charge was blocked until fee was submitted and that charge could proceed after acknowledgement. Also tested that issue of items without fees proceeded as expected
Updated status to signed off
I tried using the patch on a site and it didnt accept the fee acknowledgement. I've got a fixed patch which a site is testing with some 3M machines, will upload if they dont find any further probs
Created attachment 35878 [details] [review] Revised Patch Revised patch. Found some problems with earlier patch, i.e. it was not allowing the checkout once the fee was acknowledged (it was bailing out of the routine too early). Also it didnt handle the scenario in renew (renew in MsgType was aware of the field but called routines that ignored it). Have handled it in a way that is consistent with how Renew originally intended. Patch functionality has been tested at Chshire Libraries in the UK who are now live with it
Created attachment 36155 [details] [review] Bug 12820: Handle rental fees in Sip issue and renew Implement correct handling of fees associated with checking out an item. This is associated with fee acknowledged field (BO) To quote from the Sip2 document " If this field is N in a Checkout message and there is a fee associated with checking out the item, the ACS should tell the SC in the Checkout Response that there is a fee, and refuse to check out the item. If the SC and the patron then interact and the patron agrees to pay the fee, this field will be set to Y on a second Checkout message, indicating to the ACS that the patron has acknowledged the fee and checkout of the item should not be refused just because there is a fee associated with the item" So there are two Checkout requests the first with BO not set to Y is rejected but the fee amount is returned. The Second Checkout with BO set to Y should succeed. Added a debug log message indicating why we block a checkout when we dont otherwise indicate Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> I tested with telnet and generated sip messages to get response.
Created attachment 37038 [details] [review] [PASSED QA] Bug 12820: Handle rental fees in Sip issue and renew Implement correct handling of fees associated with checking out an item. This is associated with fee acknowledged field (BO) To quote from the Sip2 document " If this field is N in a Checkout message and there is a fee associated with checking out the item, the ACS should tell the SC in the Checkout Response that there is a fee, and refuse to check out the item. If the SC and the patron then interact and the patron agrees to pay the fee, this field will be set to Y on a second Checkout message, indicating to the ACS that the patron has acknowledged the fee and checkout of the item should not be refused just because there is a fee associated with the item" So there are two Checkout requests the first with BO not set to Y is rejected but the fee amount is returned. The Second Checkout with BO set to Y should succeed. Added a debug log message indicating why we block a checkout when we dont otherwise indicate Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 37039 [details] [review] Bug 12820 [QA Followup] - Tab and whitespace cleanup Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Patches pushed to master. Thanks for making SIP support better Colin!
Pushed to 3.18.x will be in 3.18.6
Pushed to 3.16.x, will be in 3.16.10
Note that this patch was pushed in 3.16.10, whereas 7904 was pushed only in 3.16.11. We noticed SIP checkouts does not work on a 3.16.10. So upgrade is mandatory for SIP users.