The ability to renew an item via SIP2 broke with version 23.05. To reproduce, run these commands (tailored for a KTD instance, so adjust them accordingly if necessary): misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m checkout --patron koha --item 3999900000001 misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m renew --patron koha --item 3999900000001 The last command will fail, and in /var/log/koha/kohadev/sip.log you will get: Use of uninitialized value in string eq at /kohadevbox/koha/C4/Circulation.pm line 1540
The related commit is ddc2906b77 from Bug 31735, where the file C4/SIP/ILS/Transaction/Renew.pm was modified in the following way: @@ -64,7 +66,7 @@ sub do_renew { my $self = shift; my $patron = Koha::Patrons->find( $self->{patron}->borrowernumber ); $patron or return; # FIXME we should log that - return $self->do_renew_for($patron->unblessed); + return $self->do_renew_for($patron); } As per https://perldoc.koha-community.org/C4/Circulation.html#AddIssue, AddIssue() expects its 1st argument ($borrower) to be " a hash with borrower informations (from Koha::Patron->unblessed)." This is trivial to fix, so expect a patch and updated unit tests soon.
Apparently, there's an earlier bug (Bug 32496, currently in "Signed Off" status) that attempts to "Reduce unnecessary unblessings of objects in Circulation.pm". Applying the changes from Bug 32496 fixes the problem with SIP2 renewals, so I'm marking my bug report as resolved.
Bug 32496 should not be backported IMO, so this is valid for stable versions.
(In reply to Jonathan Druart from comment #3) > Bug 32496 should not be backported IMO, so this is valid for stable versions. Thanks Jonathan, that makes sense. I'll re-open this report and upload my patches.
Created attachment 153416 [details] [review] Bug 34258: pass an unblessed hash to AddIssue() In Koha 23.05, we lost the ability to renew an item via SIP2. The relevant commit is ddc2906b77 from Bug 31735, where the file C4/SIP/ILS/Transaction/Renew.pm was modified to no longer pass an unblessed $patron hash to C4::Circulation::AddIssue() This patch fixes that. Test plan: 1) Using the SIP emulator, check out an item to a patron, then try to renew it. Example commands for a KTD instance: $ misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m checkout --patron koha --item 3999900000001 $ misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m renew --patron koha --item 3999900000001 Notice that the second command will fail! 2) Apply this patch. 3) Repeat the 2nd command -- this time the renewal should work. 4) Run the SIP-related unit tests, they should all pass: $ prove t/db_dependent/SIP/ t/db_dependent/SIP/ILS.t .......... ok t/db_dependent/SIP/Message.t ...... ok t/db_dependent/SIP/Patron.t ....... ok t/db_dependent/SIP/SIPServer.t .... ok t/db_dependent/SIP/Transaction.t .. ok
Created attachment 153417 [details] [review] Bug 34258: update SIP-related unit test Test plan: 1) Run the updated SIP-related unit test *without* having applied the other patch from this bug report -- it should fail: $ prove t/db_dependent/SIP/ILS.t 2) Apply the patch that fixes C4/SIP/ILS/Transaction/Renew.pm 3) Re-run the unit test -- it should pass.
Created attachment 153427 [details] [review] Bug 34258: pass an unblessed hash to AddIssue() In Koha 23.05, we lost the ability to renew an item via SIP2. The relevant commit is ddc2906b77 from Bug 31735, where the file C4/SIP/ILS/Transaction/Renew.pm was modified to no longer pass an unblessed $patron hash to C4::Circulation::AddIssue() This patch fixes that. Test plan: 1) Using the SIP emulator, check out an item to a patron, then try to renew it. Example commands for a KTD instance: $ misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m checkout --patron koha --item 3999900000001 $ misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m renew --patron koha --item 3999900000001 Notice that the second command will fail! 2) Apply this patch. 3) Repeat the 2nd command -- this time the renewal should work. 4) Run the SIP-related unit tests, they should all pass: $ prove t/db_dependent/SIP/ t/db_dependent/SIP/ILS.t .......... ok t/db_dependent/SIP/Message.t ...... ok t/db_dependent/SIP/Patron.t ....... ok t/db_dependent/SIP/SIPServer.t .... ok t/db_dependent/SIP/Transaction.t .. ok Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Created attachment 153428 [details] [review] Bug 34258: update SIP-related unit test Test plan: 1) Run the updated SIP-related unit test *without* having applied the other patch from this bug report -- it should fail: $ prove t/db_dependent/SIP/ILS.t 2) Apply the patch that fixes C4/SIP/ILS/Transaction/Renew.pm 3) Re-run the unit test -- it should pass. Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Created attachment 153663 [details] [review] Bug 34258: pass an unblessed hash to AddIssue() In Koha 23.05, we lost the ability to renew an item via SIP2. The relevant commit is ddc2906b77 from Bug 31735, where the file C4/SIP/ILS/Transaction/Renew.pm was modified to no longer pass an unblessed $patron hash to C4::Circulation::AddIssue() This patch fixes that. Test plan: 1) Using the SIP emulator, check out an item to a patron, then try to renew it. Example commands for a KTD instance: $ misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m checkout --patron koha --item 3999900000001 $ misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m renew --patron koha --item 3999900000001 Notice that the second command will fail! 2) Apply this patch. 3) Repeat the 2nd command -- this time the renewal should work. 4) Run the SIP-related unit tests, they should all pass: $ prove t/db_dependent/SIP/ t/db_dependent/SIP/ILS.t .......... ok t/db_dependent/SIP/Message.t ...... ok t/db_dependent/SIP/Patron.t ....... ok t/db_dependent/SIP/SIPServer.t .... ok t/db_dependent/SIP/Transaction.t .. ok Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 153664 [details] [review] Bug 34258: update SIP-related unit test Test plan: 1) Run the updated SIP-related unit test *without* having applied the other patch from this bug report -- it should fail: $ prove t/db_dependent/SIP/ILS.t 2) Apply the patch that fixes C4/SIP/ILS/Transaction/Renew.pm 3) Re-run the unit test -- it should pass. Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 153665 [details] [review] Bug 34258: (QA follow-up) Tidy the test Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
All working, tests passing.. Thanks for the quick backportable fix. Passing QA
Pushed to master for 23.11. Nice work everyone, thanks!
Pushed to 23.05.x for 23.05.03
Missing dependencies for 22.11.x. Not pushing.