When trying to checkout an unknown barcode, I receive an internal server error: Can't call method "materials" on an undefined value at /home/vagrant/kohaclone/circ/circulation.pl line 387. It should ask me to do a fast add instead.
(In reply to Katrin Fischer from comment #0) > When trying to checkout an unknown barcode, I receive an internal server > error: > > Can't call method "materials" on an undefined value at > /home/vagrant/kohaclone/circ/circulation.pl line 387. > > It should ask me to do a fast add instead. For me, it asks for fast cataloguing the item - I can't reproduce it on my devbox...
Created attachment 67857 [details] [review] Bug 19431: Fix error when checking out an unknown barcode It seems this has been caused by commit 1544f9a5d4a8acd47c97d7c6ac55dee8e759d3ff Bug 18276: Remove GetBiblioFromItemNumber - circulation pages To reproduce the problem you need switch on the two prefs - OnSiteCheckouts - OnSiteCheckoutsForce Test plan: Try to check an item out using an unknown barcode With the 2 prefs set to on and without this patch, you will get the following error in the log Can't call method "materials" on an undefined value at /home/vagrant/kohaclone/circ/circulation.pl line 387. With this patch applied you should not get this error and a correct behaviour for the different pref combinations.
Created attachment 67863 [details] [review] Bug 19431: Fix error when checking out an unknown barcode It seems this has been caused by commit 1544f9a5d4a8acd47c97d7c6ac55dee8e759d3ff Bug 18276: Remove GetBiblioFromItemNumber - circulation pages To reproduce the problem you need switch on the two prefs - OnSiteCheckouts - OnSiteCheckoutsForce Test plan: Try to check an item out using an unknown barcode With the 2 prefs set to on and without this patch, you will get the following error in the log Can't call method "materials" on an undefined value at /home/vagrant/kohaclone/circ/circulation.pl line 387. With this patch applied you should not get this error and a correct behaviour for the different pref combinations. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 67917 [details] [review] Bug 19431: Fix error when checking out an unknown barcode It seems this has been caused by commit 1544f9a5d4a8acd47c97d7c6ac55dee8e759d3ff Bug 18276: Remove GetBiblioFromItemNumber - circulation pages To reproduce the problem you need switch on the two prefs - OnSiteCheckouts - OnSiteCheckoutsForce Test plan: Try to check an item out using an unknown barcode With the 2 prefs set to on and without this patch, you will get the following error in the log Can't call method "materials" on an undefined value at /home/vagrant/kohaclone/circ/circulation.pl line 387. With this patch applied you should not get this error and a correct behaviour for the different pref combinations. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Works in my testing. I would suggest adding parens to the second conditional: if ( (a and b) or (c) ) just for clarity, but not necessary if you don't want to
I will take a look again later but I think the condition is wrong, I guess it should be if( $item and ( !$blocker or $force_allow_issue ) ){
Created attachment 67966 [details] [review] Bug 19431: Fix error when checking out an unknown barcode It seems this has been caused by commit 1544f9a5d4a8acd47c97d7c6ac55dee8e759d3ff Bug 18276: Remove GetBiblioFromItemNumber - circulation pages To reproduce the problem you need switch on the two prefs - OnSiteCheckouts - OnSiteCheckoutsForce Test plan: Try to check an item out using an unknown barcode With the 2 prefs set to on and without this patch, you will get the following error in the log Can't call method "materials" on an undefined value at /home/vagrant/kohaclone/circ/circulation.pl line 387. With this patch applied you should not get this error and a correct behaviour for the different pref combinations.
Sorry about that, the condition was wrong!
Created attachment 68069 [details] [review] Bug 19431: Fix error when checking out an unknown barcode It seems this has been caused by commit 1544f9a5d4a8acd47c97d7c6ac55dee8e759d3ff Bug 18276: Remove GetBiblioFromItemNumber - circulation pages To reproduce the problem you need switch on the two prefs - OnSiteCheckouts - OnSiteCheckoutsForce Test plan: Try to check an item out using an unknown barcode With the 2 prefs set to on and without this patch, you will get the following error in the log Can't call method "materials" on an undefined value at /home/vagrant/kohaclone/circ/circulation.pl line 387. With this patch applied you should not get this error and a correct behaviour for the different pref combinations. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
QA: Looking here now
- unless( $onsite_checkout and C4::Context->preference("OnSiteCheckoutsForce") ) { + if ( $error->{UNKNOWN_BARCODE} or not $onsite_checkout or not C4::Context->preference("OnSiteCheckoutsForce") ) { If CanBookBeIssued sets the UNKNOWN_BARCODE flag, it returns no DEBT question and no other impossible errors. But since there is one error, it will set $blocker to 1. Just wondering if there are other errors that should trigger the blocker flag when doing onsite checkouts here. If CanBookBeIssued said No, when is an onsite checkout still possible? - if( !$blocker || $force_allow_issue ){ + + if( $item and ( !$blocker or $force_allow_issue ) ){ Since you raised the blocker flag in the unknown barcode case, you do not need to test for $item here. Note that item is found by barcode and CanBookBeIssued calls GetItem with barcode parameter. Please clarify/adjust.
(In reply to Marcel de Rooy from comment #11) > - unless( $onsite_checkout and > C4::Context->preference("OnSiteCheckoutsForce") ) { > + if ( $error->{UNKNOWN_BARCODE} or not $onsite_checkout or not > C4::Context->preference("OnSiteCheckoutsForce") ) { > > If CanBookBeIssued sets the UNKNOWN_BARCODE flag, it returns no DEBT > question and no other impossible errors. But since there is one error, it > will set $blocker to 1. > Just wondering if there are other errors that should trigger the blocker > flag when doing onsite checkouts here. If CanBookBeIssued said No, when is > an onsite checkout still possible? I put the test there to avoid to copy paste the blocker=1 and the IMPOSSIBLE flag. Do you have something better to suggest? > - if( !$blocker || $force_allow_issue ){ > + > + if( $item and ( !$blocker or $force_allow_issue ) ){ > > Since you raised the blocker flag in the unknown barcode case, you do not > need to test for $item here. Note that item is found by barcode and > CanBookBeIssued calls GetItem with barcode parameter. Yes definitely, but this code is very confusing and I think explicitly tell we need $item here makes sense (even if useless in a algorithm pov, in my opinion it helps the read).
(In reply to Jonathan Druart from comment #12) > (In reply to Marcel de Rooy from comment #11) > > - unless( $onsite_checkout and > > C4::Context->preference("OnSiteCheckoutsForce") ) { > > + if ( $error->{UNKNOWN_BARCODE} or not $onsite_checkout or not > > C4::Context->preference("OnSiteCheckoutsForce") ) { > > > > If CanBookBeIssued sets the UNKNOWN_BARCODE flag, it returns no DEBT > > question and no other impossible errors. But since there is one error, it > > will set $blocker to 1. > > Just wondering if there are other errors that should trigger the blocker > > flag when doing onsite checkouts here. If CanBookBeIssued said No, when is > > an onsite checkout still possible? > > I put the test there to avoid to copy paste the blocker=1 and the IMPOSSIBLE > flag. Do you have something better to suggest? My question was: Is UNKNOWN_BARCODE the only one needed here? Should the blocker flag be set (among other things) for other types of errors ?
UNKNOWN_BARCODE is the one we need to add to fix this bug.
(In reply to Jonathan Druart from comment #14) > UNKNOWN_BARCODE is the one we need to add to fix this bug. Hmm I never thought of that ;) I am not blocking this patch but I raised a valid question and got no answer.
Created attachment 68462 [details] [review] Bug 19431: Fix error when checking out an unknown barcode It seems this has been caused by commit 1544f9a5d4a8acd47c97d7c6ac55dee8e759d3ff Bug 18276: Remove GetBiblioFromItemNumber - circulation pages To reproduce the problem you need switch on the two prefs - OnSiteCheckouts - OnSiteCheckoutsForce Test plan: Try to check an item out using an unknown barcode With the 2 prefs set to on and without this patch, you will get the following error in the log Can't call method "materials" on an undefined value at /home/vagrant/kohaclone/circ/circulation.pl line 387. With this patch applied you should not get this error and a correct behaviour for the different pref combinations. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Marcel de Rooy from comment #15) > (In reply to Jonathan Druart from comment #14) > > UNKNOWN_BARCODE is the one we need to add to fix this bug. > > Hmm I never thought of that ;) > I am not blocking this patch but I raised a valid question and got no answer. To be honest I have no idea what we need to do more. The subroutine and the script are very ugly and need a full refurnish to be readable and less error-prone.
Pushed to master for 17.11, thanks to everybody involved!
Does not appear in 17.05.x