[Bug 34258] New: Cannot renew item via SIP2
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Bug ID: 34258 Summary: Cannot renew item via SIP2 Change sponsored?: --- Product: Koha Version: 23.05 Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: SIP2 Assignee: koha-bugs@lists.koha-community.org Reporter: a.roussos@dataly.gr QA Contact: testopia@bugs.koha-community.org 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 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Andreas Roussos <a.roussos@dataly.gr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=31735 Assignee|koha-bugs@lists.koha-commun |a.roussos@dataly.gr |ity.org | --- Comment #1 from Andreas Roussos <a.roussos@dataly.gr> --- 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. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Andreas Roussos <a.roussos@dataly.gr> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |INVALID Status|ASSIGNED |RESOLVED See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=32496 --- Comment #2 from Andreas Roussos <a.roussos@dataly.gr> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #3 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Bug 32496 should not be backported IMO, so this is valid for stable versions. -- You are receiving this mail because: You are watching all bug changes.
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
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Andreas Roussos <a.roussos@dataly.gr> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|INVALID |--- Status|RESOLVED |REOPENED --- Comment #4 from Andreas Roussos <a.roussos@dataly.gr> --- (In reply to Jonathan Druart from comment #3) patches. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Andreas Roussos <a.roussos@dataly.gr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 --- Comment #5 from Andreas Roussos <a.roussos@dataly.gr> --- Created attachment 153416 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153416&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Andreas Roussos <a.roussos@dataly.gr> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 --- Comment #6 from Andreas Roussos <a.roussos@dataly.gr> --- Created attachment 153417 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153417&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153416|0 |1 is obsolete| | --- Comment #7 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 153427 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153427&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153417|0 |1 is obsolete| | --- Comment #8 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 153428 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153428&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |emily.lamancusa@montgomeryc | |ountymd.gov -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153427|0 |1 is obsolete| | --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 153663 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153663&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153428|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 153664 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153664&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 153665 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153665&action=edit Bug 34258: (QA follow-up) Tidy the test Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Signed Off |Passed QA --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- All working, tests passing.. Thanks for the quick backportable fix. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |fridolin.somers@biblibre.co | |m Version(s)|23.11.00 |23.11.00,23.05.03 released in| | --- Comment #14 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.05.x for 23.05.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@ptfs-europe.co | |m Resolution|--- |FIXED Status|Pushed to stable |RESOLVED --- Comment #15 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Missing dependencies for 22.11.x. Not pushing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34258 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34846 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34846 [Bug 34846] SIP/ILS.t is failing if the DB has been upgraded -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org