If a record level hold is filled and waiting at Library A, and the item is checked in at Library B, AddReturn() calls RevertWaitingStatus(), which unsets found, and itemnumber ( if the hold was record level ). AddReturn() then retuns the message 'ResFound', which triggers the hold fulfillment dialog, which passes the reserve_id back to circulation.pl. Then, circulation.pl attempts to get the biblio via the itemnumber, which the hold doesn't have, causing an ISE.
So the root cause is that we are not passing the itemnumber *for the scanned barcode* back to the form, making itemnumber undef, so the Item object is empty, so calling the biblio method causes an ISE.
Created attachment 106758 [details] [review] Bug 25969: Checking in a found hold at a different branch then confirming the hold causes internal server error If a record level hold is filled and waiting at Library A, and the item is checked in at Library B, and an attempt is made to re-fill the hold with the item, Koha will return an ISE. Test Plan: 1) Place a hold for Library A, at Library A, for pickup at Library A 2) Check in the item at Library A and fill the hold so it is waiting at Library A 3) Log in as Library B, check in the same barcode 4) Note the request to fill the hold with the item 5) Choose to fill the hold 6) Note you get an internal server error 7) Apply this patch 8) Restart all the things! 9) Repeat steps 1-5 10) No ISE!
Created attachment 106771 [details] [review] Bug 25969: Checking in a found hold at a different branch then confirming the hold causes internal server error If a record level hold is filled and waiting at Library A, and the item is checked in at Library B, and an attempt is made to re-fill the hold with the item, Koha will return an ISE. Test Plan: 1) Place a hold for Library A, at Library A, for pickup at Library A 2) Check in the item at Library A and fill the hold so it is waiting at Library A 3) Log in as Library B, check in the same barcode 4) Note the request to fill the hold with the item 5) Choose to fill the hold 6) Note you get an internal server error 7) Apply this patch 8) Restart all the things! 9) Repeat steps 1-5 10) No ISE!
I'm still getting an error with the patch applied. "Can't call method "biblio" on an undefined value at /kohadevbox/koha/circ/returns.pl line 148"
Created attachment 107697 [details] [review] Bug 25969: Checking in a found hold at a different branch then confirming the hold causes internal server error If a record level hold is filled and waiting at Library A, and the item is checked in at Library B, and an attempt is made to re-fill the hold with the item, Koha will return an ISE. Test Plan: 1) Place a hold for Library A, at Library A, for pickup at Library A 2) Check in the item at Library A and fill the hold so it is waiting at Library A 3) Log in as Library B, check in the same barcode 4) Note the request to fill the hold with the item 5) Choose to fill the hold 6) Note you get an internal server error 7) Apply this patch 8) Restart all the things! 9) Repeat steps 1-5 10) No ISE!
This patch works if you check in using the the 'check in' function in the main search bar. However, if you use the check in at: cgi-bin/koha/circ/returns.pl - then you still get the same ISE as before the patch is applied: Can't call method "biblio" on an undefined value at /kohadevbox/koha/circ/returns.pl line 148 in (eval) at /kohadevbox/koha/circ/returns.pl line 148 145: my $cancel_reserve = $query->pa
(In reply to Sally from comment #6) > This patch works if you check in using the the 'check in' function in the > main search bar. > > However, if you use the check in at: cgi-bin/koha/circ/returns.pl - then you > still get the same ISE as before the patch is applied: > > Can't call method "biblio" on an undefined value at > /kohadevbox/koha/circ/returns.pl line 148 > in (eval) at /kohadevbox/koha/circ/returns.pl line 148 > 145: my $cancel_reserve = $query->pa I cannot recreate this, can you test again and explain the steps?
I'm also getting an error. To recreate: - Log in at Library A - Place hold on item at Library A, for pickup at Library A - Check item in at Library A - Click "Confirm hold" - Set library to Library B - Check item in again - Click "Confirm hold and transfer" - get error: Can't call method "biblio" on an undefined value at /kohadevbox/koha/circ/returns.pl line 147
Created attachment 109678 [details] [review] Bug 25969: Don't set itemnumber as reserve->itemnumber When a hold is checked in at the wrong destination we revert the found status For a title level hold this means we return it to a title level hold, with no itemnumber When we checkin the item we find the hold, but won't set the itemnumber in the hold until it is confirmed In the script we were setting the itemnumber param, but then setting it again using the reserve itemnumber. In all conditions before the itemnumber has been set, we don't need to set it again, especially when we do so incorrectly To test: - Log in at Library A - Place hold on item at Library A, for pickup at Library A - Check item in at Library A - Click "Confirm hold" - Set library to Library B - Check item in again (try with both the box on reutrns.pl and the box in the header) - Click "Confirm hold and transfer" - get error: Can't call method "biblio" on an undefined value at /kohadevbox/koha/circ/returns.pl line 147 - apply patch - repeat - success! - confirm trappings of holds works as normally otherwise too
*** Bug 26386 has been marked as a duplicate of this bug. ***
Created attachment 109755 [details] [review] Bug 25969: Checking in a found hold at a different branch then confirming the hold causes internal server error If a record level hold is filled and waiting at Library A, and the item is checked in at Library B, and an attempt is made to re-fill the hold with the item, Koha will return an ISE. Test Plan: 1) Place a hold for Library A, at Library A, for pickup at Library A 2) Check in the item at Library A and fill the hold so it is waiting at Library A 3) Log in as Library B, check in the same barcode 4) Note the request to fill the hold with the item 5) Choose to fill the hold 6) Note you get an internal server error 7) Apply this patch 8) Restart all the things! 9) Repeat steps 1-5 10) No ISE! Signed-off-by: Daniel Gaghan <daniel.gaghan@pueblolibrary.org>
Created attachment 109756 [details] [review] Bug 25969: Don't set itemnumber as reserve->itemnumber When a hold is checked in at the wrong destination we revert the found status For a title level hold this means we return it to a title level hold, with no itemnumber When we checkin the item we find the hold, but won't set the itemnumber in the hold until it is confirmed In the script we were setting the itemnumber param, but then setting it again using the reserve itemnumber. In all conditions before the itemnumber has been set, we don't need to set it again, especially when we do so incorrectly To test: - Log in at Library A - Place hold on item at Library A, for pickup at Library A - Check item in at Library A - Click "Confirm hold" - Set library to Library B - Check item in again (try with both the box on reutrns.pl and the box in the header) - Click "Confirm hold and transfer" - get error: Can't call method "biblio" on an undefined value at /kohadevbox/koha/circ/returns.pl line 147 - apply patch - repeat - success! - confirm trappings of holds works as normally otherwise too Signed-off-by: Daniel Gaghan <daniel.gaghan@pueblolibrary.org>
Created attachment 110006 [details] [review] Bug 25969: Checking in a found hold at a different branch then confirming the hold causes internal server error If a record level hold is filled and waiting at Library A, and the item is checked in at Library B, and an attempt is made to re-fill the hold with the item, Koha will return an ISE. Test Plan: 1) Place a hold for Library A, at Library A, for pickup at Library A 2) Check in the item at Library A and fill the hold so it is waiting at Library A 3) Log in as Library B, check in the same barcode 4) Note the request to fill the hold with the item 5) Choose to fill the hold 6) Note you get an internal server error 7) Apply this patch 8) Restart all the things! 9) Repeat steps 1-5 10) No ISE! Signed-off-by: Daniel Gaghan <daniel.gaghan@pueblolibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 110007 [details] [review] Bug 25969: Don't set itemnumber as reserve->itemnumber When a hold is checked in at the wrong destination we revert the found status For a title level hold this means we return it to a title level hold, with no itemnumber When we checkin the item we find the hold, but won't set the itemnumber in the hold until it is confirmed In the script we were setting the itemnumber param, but then setting it again using the reserve itemnumber. In all conditions before the itemnumber has been set, we don't need to set it again, especially when we do so incorrectly To test: - Log in at Library A - Place hold on item at Library A, for pickup at Library A - Check item in at Library A - Click "Confirm hold" - Set library to Library B - Check item in again (try with both the box on reutrns.pl and the box in the header) - Click "Confirm hold and transfer" - get error: Can't call method "biblio" on an undefined value at /kohadevbox/koha/circ/returns.pl line 147 - apply patch - repeat - success! - confirm trappings of holds works as normally otherwise too Signed-off-by: Daniel Gaghan <daniel.gaghan@pueblolibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Kyle, Nick, I have a follow-up patches for this, but I prefer to not push it as a RM follow-up but separated instead. Can you have a look at bug 26485 please?
Pushed to master for 20.11, thanks to everybody involved!
The patch "Bug 25969: Don't set itemnumber as reserve->itemnumber" here breaks printing hold slip in returns.pl page if you try to print it more than once. So if you return the hold first once, it prints the slip fine and goes to waiting state, then if you check in the item another time and try to print the slip the Internal server error comes.
(In reply to Joonas Kylmälä from comment #17) > The patch "Bug 25969: Don't set itemnumber as reserve->itemnumber" here > breaks printing hold slip in returns.pl page if you try to print it more > than once. So if you return the hold first once, it prints the slip fine and > goes to waiting state, then if you check in the item another time and try to > print the slip the Internal server error comes. Nick or Kyle, can you have a look at this ASAP please?
Created attachment 111328 [details] [review] Bug 25969: (follow-up) If not coming from hold modal find item by barcode
(In reply to Nick Clemens from comment #19) > Created attachment 111328 [details] [review] [review] > Bug 25969: (follow-up) If not coming from hold modal find item by barcode This fixes the ISE, but if you click "Print slip and confirm" the modal launches again, and again, and again... Pinging Kyle
(In reply to Nick Clemens from comment #20) > (In reply to Nick Clemens from comment #19) > > Created attachment 111328 [details] [review] [review] [review] > > Bug 25969: (follow-up) If not coming from hold modal find item by barcode > > This fixes the ISE, but if you click "Print slip and confirm" the modal > launches again, and again, and again... > > Pinging Kyle That repeat is due to the change from Bug 25907 that adds the barcode to the modal request. This makes the system think there is another book being returned after the currently processed one. Since it's the same one it just keeps repeating. I tried commenting out the barcode part from 25907 and that works (when itemnumber is provided), but the functionality from 25907 of course stops working properly then.
Created attachment 111344 [details] [review] Bug 26627: Print and confirming a hold can cause an infinite loop Test plan: 1) Place a hold for pickup at library A 2) Confirm the hold and print slip 3) Checkin the item again 4) Confirm the hold and print slip 5) The modal returns, do it again 6) Apply this patch 7) Reload the page 8) Repeat steps 1-3 9) No loop! Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(In reply to Joonas Kylmälä from comment #17) > The patch "Bug 25969: Don't set itemnumber as reserve->itemnumber" here > breaks printing hold slip in returns.pl page if you try to print it more > than once. So if you return the hold first once, it prints the slip fine and > goes to waiting state, then if you check in the item another time and try to > print the slip the Internal server error comes. This issue doesn't happen anymore with the latest master version. I'm not exactly sure which commit did the fix. So the patch "Bug 25969: (follow-up) If not coming from hold modal find item by barcode" is not at least required anymore to fix this, I don't know if it would be useful in other cases.
Comment on attachment 111328 [details] [review] Bug 25969: (follow-up) If not coming from hold modal find item by barcode This has not been pushed.