C4/SIP/ILS.pm 436 sub renew { 437 my ($self, $patron_id, $patron_pwd, $item_id, $title_id, 438 $no_block, $nb_due_date, $third_party, 439 $item_props, $fee_ack) = @_; The $fee_ack variable is lost in the transition to C4::SIP::ILS::Transaction::Renew.. we need to clone it across as we do for the checkout message transaction. This is important, as without passing this on, one cannot renew items where charges apply. To test: 1) Add a rental charge to an itemtype 2) Checkout an item of that itemtype to a user 3) Attempt a renewal of that item via SIP2 and note that it fails 4) Pass the fee_acknowledgement bit in renewal and note the renewal still fails. This can be achieved using the sip_cli_emulator (after applying bug 34764 to fix it's mishandling of passing the fee_ack bit)
Created attachment 155559 [details] [review] Bug 34767: Pass fee_ack into ::Transaction::Renew(All) This patch copies the $fee_ack field into the generated ::Transaction::Renew|All objects such that the fee acknowldegement flag is respected for renewals. Test plan To test: 1) Add a rental charge to an itemtype 2) Checkout an item of that itemtype to a user 3) Attempt a renewal of that item via SIP2 and note that it fails sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL --patron 23529000035676 --item 39999000007756 -m renew 4) Pass the fee_acknowledgement bit in renewal and note the renewal still fails. sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL --fee-acknowledged Y --patron 23529000035676 --item 39999000007756 -m renew 5) Apply patch and note the above now succeeds sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL --fee-acknowledged Y --patron 23529000035676 --item 39999000007756 -m renew
Created attachment 155567 [details] [review] Bug 34767: Pass fee_ack into ::Transaction::Renew(All) This patch copies the $fee_ack field into the generated ::Transaction::Renew|All objects such that the fee acknowldegement flag is respected for renewals. Test plan To test: 1) Add a rental charge to an itemtype 2) Checkout an item of that itemtype to a user 3) Attempt a renewal of that item via SIP2 and note that it fails sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL --patron 23529000035676 --item 39999000007756 -m renew 4) Pass the fee_acknowledgement bit in renewal and note the renewal still fails. sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL --fee-acknowledged Y --patron 23529000035676 --item 39999000007756 -m renew 5) Apply patch and note the above now succeeds sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL --fee-acknowledged Y --patron 23529000035676 --item 39999000007756 -m renew Signed-off-by: David Nind <david@davidnind.com>
Testing notes (using KTD): 1. Applied bug 34764 before starting (not sure whether I should have applied it before starting or at step 5 of the test plan, as not listed as a dependency for this bug). 2. Response from - step 3 (renewal does not work - check details under checkouts for Henry in the staff interface): ./misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL --patron 23529000035676 --item 39999000007756 -m renew Attempting socket connection to localhost:6001...connected! Use of uninitialized value $hold_mode in string eq at ./misc/sip_cli_emulator.pl line 123. SEND: 9300CNterm1|COterm1|CPCPL| READ: 941 Trying 'renew' SEND: 29NN20230912 17483420230912 174834AOCPL|AA23529000035676|AB39999000007756|AJ|ACterm1| READ: 300NUN20230912 174834AA23529000035676|AB39999000007756|AJScarlet and black /|AH|BV1.00|BHUSD|BT06|AOCPL| 3. Response - step 4 (renewal does not work - check details under checkouts for Henry in the staff interface): ./misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL --fee-acknowledged Y --patron 23529000035676 --item 39999000007756 -m renew Attempting socket connection to localhost:6001...connected! Use of uninitialized value $hold_mode in string eq at ./misc/sip_cli_emulator.pl line 123. SEND: 9300CNterm1|COterm1|CPCPL| READ: 941 Trying 'renew' SEND: 29NN20230912 17493120230912 174931AOCPL|AA23529000035676|AB39999000007756|AJ|ACterm1|BOY| READ: 300NUN20230912 174931AA23529000035676|AB39999000007756|AJScarlet and black /|AH|BV1.00|BHUSD|BT06|AOCPL| 4. Response from step 5 (renewal now works - showing 1 renewal with 4 of 5 renewals remaining): ./misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL --fee-acknowledged Y --patron 23529000035676 --item 39999000007756 -m renew Attempting socket connection to localhost:6001...connected! Use of uninitialized value $hold_mode in string eq at ./misc/sip_cli_emulator.pl line 123. SEND: 9300CNterm1|COterm1|CPCPL| READ: 941 Trying 'renew' SEND: 29NN20230912 17525220230912 175252AOCPL|AA23529000035676|AB39999000007756|AJ|ACterm1|BOY| READ: 301YNN20230912 175253AA23529000035676|AB39999000007756|AJScarlet and black /|AH20230917 235900|CK|BV1.00|BHUSD|BT06|AOCPL|
Thanks so much for testing David, another perfect job. SIP tends to scare people, so you've reapply mad emy day keeping this marching forward.
Created attachment 155636 [details] [review] Bug 34767: Pass fee_ack into ::Transaction::Renew(All) This patch copies the $fee_ack field into the generated ::Transaction::Renew|All objects such that the fee acknowldegement flag is respected for renewals. Test plan To test: 1) Add a rental charge to an itemtype 2) Checkout an item of that itemtype to a user 3) Attempt a renewal of that item via SIP2 and note that it fails sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL --patron 23529000035676 --item 39999000007756 -m renew 4) Pass the fee_acknowledgement bit in renewal and note the renewal still fails. sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL --fee-acknowledged Y --patron 23529000035676 --item 39999000007756 -m renew 5) Apply patch and note the above now succeeds sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL --fee-acknowledged Y --patron 23529000035676 --item 39999000007756 -m renew Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 23.11. Nice work everyone, thanks!
Pushed to 23.05.x for 23.05.04
Nice work everyone! Pushed to oldstable for 22.11.x