[Bug 14015] New: Entering 0 (zero) as barcode reloads checkout page without patron information
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Bug ID: 14015 Summary: Entering 0 (zero) as barcode reloads checkout page without patron information Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: trivial Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: veron@veron.ch QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com To reproduce: - Go to Home > Circulation > Checkouts > [a patron] - Enter 0 (zero) in barcode field and hit Enter Result: circ/circulation.pl reloads without patron selected (Home > Circulation
Checkouts). You have to search the patron again to proceed.
-- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Entering 0 (zero) as |Entering 0 (zero) as |barcode reloads checkout |barcode on checkout page |page without patron |leads to software error |information | --- Comment #1 from Marc Véron <veron@veron.ch> --- New behaviour on current master: - Go to Home > Circulation > Checkouts > [a patron] - Enter 0 (zero) in barcode field and hit Enter Result: Can't use string ("1") as a HASH ref while "strict refs" in use at /usr/share/kohaclone/circ/circulation.pl line 473 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|trivial |normal -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Entering 0 (zero) as |Checkout: Fix software |barcode on checkout page |error if barcode '0' is |leads to software error |given -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |veron@veron.ch |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 --- Comment #2 from Marc Véron <veron@veron.ch> --- Created attachment 45660 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45660&action=edit Bug 14015 - Checkout: Fix software error if barcode '0' is given To reproduce the bug: - Go to Home > Circulation > Checkouts > [a patron] - Enter 0 (zero) in barcode field and hit Enter Result: Software error Can't use string ("1") as a HASH ref while "strict refs" in use at /usr/share/kohaclone/circ/circulation.pl line 473 To test: - Apply patch - Repeat steps above - Verify that the sofware error is gone and that you get a message as with other invalid barcodes. - Test with other values: '00', '000', existing barcode, barcode that does not exist - Test with empty barcode (for printing slip) - Search for regressions Note: I tried to create an item with barcode '0' to test that case as well, but it was not possible. Giving the possibility to create an item with barcode '0' is not in the scope of this bug. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45660|0 |1 is obsolete| | --- Comment #3 from Marc Véron <veron@veron.ch> --- Created attachment 45661 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45661&action=edit Bug 14015 - Checkout: Fix software error if barcode '0' is given To reproduce the bug: - Go to Home > Circulation > Checkouts > [a patron] - Enter 0 (zero) in barcode field and hit Enter Result: Software error Can't use string ("1") as a HASH ref while "strict refs" in use at /usr/share/kohaclone/circ/circulation.pl line 473 To test: - Apply patch - Repeat steps above - Verify that the sofware error is gone and that you get a message as with other invalid barcodes. - Test with other values: '00', '000', existing barcode, and a barcode that does not exist - Test with empty barcode (for printing slip) - Search for regressions Note: I tried to create an item with barcode '0' to test that case as well, but it was not possible. Giving the possibility to create an item with barcode '0' is not in the scope of this bug. (Amended for better formatting of commit message) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Chris Kirby <christopherlawrencekirby@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |christopherlawrencekirby@gm | |ail.com --- Comment #4 from Chris Kirby <christopherlawrencekirby@gmail.com> --- Hi Marc, I have tried testing this bug on Sandbox 6 (with patch applied) and Sandbox 7 (without patch applied), and I am not seeing the software error described. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 --- Comment #5 from Marc Véron <veron@veron.ch> --- Hi Chris,thanks for testing. What happens if you enter a 0 and then hit "return"? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Aleisha Amohia <aleishaamohia@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Aleisha Amohia <aleishaamohia@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45661|0 |1 is obsolete| | --- Comment #6 from Aleisha Amohia <aleishaamohia@hotmail.com> --- Created attachment 45873 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45873&action=edit Bug 14015 - Checkout: Fix software error if barcode '0' is given To reproduce the bug: - Go to Home > Circulation > Checkouts > [a patron] - Enter 0 (zero) in barcode field and hit Enter Result: Software error Can't use string ("1") as a HASH ref while "strict refs" in use at /usr/share/kohaclone/circ/circulation.pl line 473 To test: - Apply patch - Repeat steps above - Verify that the sofware error is gone and that you get a message as with other invalid barcodes. - Test with other values: '00', '000', existing barcode, and a barcode that does not exist - Test with empty barcode (for printing slip) - Search for regressions Note: I tried to create an item with barcode '0' to test that case as well, but it was not possible. Giving the possibility to create an item with barcode '0' is not in the scope of this bug. (Amended for better formatting of commit message) I was able to reproduce the warn and patch works correctly. Signed-off-by: Aleisha <aleishaamohia@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Comment on attachment 45873 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45873 Bug 14015 - Checkout: Fix software error if barcode '0' is given Review of attachment 45873: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=14015&attachment=45873) ----------------------------------------------------------------- ::: circ/circulation.pl @@ +217,5 @@
}
# check and see if we should print +# but do not set to 'yes' if barcode given by user is '0' +if ( ( @$barcodes == 0 ) && ( ! @$barcodes eq '0') && ( $print eq 'maybe' ) ) {
What is supposed to test !@$barcodes eq '0' ?? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 --- Comment #8 from Marc Véron <veron@veron.ch> --- (In reply to Jonathan Druart from comment #7)
Comment on attachment 45873 [details] [review] Bug 14015 - Checkout: Fix software error if barcode '0' is given
Review of attachment 45873 [details] [review]: -----------------------------------------------------------------
::: circ/circulation.pl @@ +217,5 @@
}
# check and see if we should print +# but do not set to 'yes' if barcode given by user is '0' +if ( ( @$barcodes == 0 ) && ( ! @$barcodes eq '0') && ( $print eq 'maybe' ) ) {
What is supposed to test !@$barcodes eq '0' ??
Hi Jonathan, The first (already existing) part @$barcodes == 0 tests @$barcodes for numerical equality (no barcodes given). While testing I found out that it evaluates to true for a barcode that is '0'. Because it evaluated to true with a barcode '0', $print was set to 'yes' (but we do not want to print, since we have a barcode). With $print set to 'yes' the borrowernumber is set to '' in line 235, and that leads to the software error in line 473. By adding a test for the barcode not to be equal to '0' by the string equality oparator eq ( ! @$barcodes eq '0'), the value of $print is not set to 'yes' and the barcode '0' is treated as such. Koha will then display the message 'The barcode was not found 0' (as expected) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marc Véron from comment #8)
(In reply to Jonathan Druart from comment #7)
Comment on attachment 45873 [details] [review] [review] Bug 14015 - Checkout: Fix software error if barcode '0' is given
Review of attachment 45873 [details] [review] [review]: -----------------------------------------------------------------
::: circ/circulation.pl @@ +217,5 @@
}
# check and see if we should print +# but do not set to 'yes' if barcode given by user is '0' +if ( ( @$barcodes == 0 ) && ( ! @$barcodes eq '0') && ( $print eq 'maybe' ) ) {
What is supposed to test !@$barcodes eq '0' ??
Hi Jonathan,
The first (already existing) part @$barcodes == 0 tests @$barcodes for numerical equality (no barcodes given). While testing I found out that it evaluates to true for a barcode that is '0'. Because it evaluated to true with a barcode '0', $print was set to 'yes' (but we do not want to print, since we have a barcode). With $print set to 'yes' the borrowernumber is set to '' in line 235, and that leads to the software error in line 473. By adding a test for the barcode not to be equal to '0' by the string equality oparator eq ( ! @$barcodes eq '0'), the value of $print is not set to 'yes' and the barcode '0' is treated as such. Koha will then display the message 'The barcode was not found 0' (as expected)
my $a = ['0']; say "1.is 0" if @$a eq '0'; $a = [0]; say "2.is 0" if @$a eq '0'; Does not print anything. I have tried your changes and remove the last 2 tests and everything looks ok. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Signed Off |Failed QA --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Please clarify which steps are needed to replicate the problem in current master. I do not see this error (anymore?).. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 --- Comment #11 from Marc Véron <veron@veron.ch> --- Hi Marcel, I re-tested and reproduced with 3.22.00.000 on our demo installation and with current master 3.23.00.000 on my development vm. Test 1: - Go to "My Checkouts" - Enter 0 (zero) in barcode field and hit Enter Expected result: Message that barcode 0 does not exist Result: Software error. Can't use string ("1") as a HASH ref while "strict refs" in use at /usr/share/kohaclone/circ/circulation.pl line 470 (Like in comment #2) Test 2: - Search a patron and got to the checkout page - Enter 0 (zero) in barcode filed and hit Enter Expected result: Message that barcode 0 does not exist Result: Checkout page loads (circ/circulation.pl). You have to search the patron again to proceed. (Like in comment #1) For some patrons I get the result 1 (patrons with staff permissions), for others I get the result 2 ('normal' patrons). The reason for both is that the borrowernumber is set to '' in circ/circulation.pl line 235 (because of $print eq 'yes' evaluates to true which is wrong, $print should not be 'yes' here). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> ---
Go to Home > Circulation > Checkouts > [a patron]
This was quite confusing actually. It is: Go to Home > Patrons > [enter patron] > Check out Back to SO -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- if ( ( @$barcodes == 0 ) && ( ! @$barcodes eq '0') && ( $print eq 'maybe' ) ) I agree with Jonathan. This code needs to be adjusted. If $barcodes == [ '0'], the second test cannot be true since @$barcodes will be 1. Failed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 --- Comment #14 from Marc Véron <veron@veron.ch> --- Hi Marcel, a question: Can you reproduce / confirm the problem as described in comment #11? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Marc, Try with only this change: diff --git a/circ/circulation.pl b/circ/circulation.pl index f16475e..fa62dcc 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -88,7 +88,9 @@ if (!C4::Context->userenv && !$branch){ } my $barcodes = []; -if ( my $barcode = $query->param('barcode') ) { +my $barcode = $query->param('barcode'); +# Barcode given by user could be '0' +if ( $barcode || $barcode eq '0' ) { $barcodes = [ $barcode ]; } else { my $filefh = $query->upload('uploadfile'); @@ -215,12 +217,13 @@ if( $onsite_checkout && !$duedatespec_allow ) { } It will work. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45873|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 --- Comment #16 from Marc Véron <veron@veron.ch> --- Created attachment 45999 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45999&action=edit Bug 14015 - Checkout: Fix software error if barcode '0' is given To reproduce the bug: - Go to Home > Circulation > Checkouts > [a patron] - Enter 0 (zero) in barcode field and hit Enter Result: Software error Can't use string ("1") as a HASH ref while "strict refs" in use at /usr/share/kohaclone/circ/circulation.pl line 473 To test: - Apply patch - Repeat steps above - Verify that the sofware error is gone and that you get a message as with other invalid barcodes. - Test with other values: '00', '000', existing barcode, and a barcode that does not exist - Test with empty barcode (for printing slip) - Search for regressions This patch implements Jonathan's solution (see comment #15) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45999|0 |1 is obsolete| | --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 46000 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46000&action=edit Bug 14015 - Checkout: Fix software error if barcode '0' is given To reproduce the bug: - Go to Home > Circulation > Checkouts > [a patron] - Enter 0 (zero) in barcode field and hit Enter Result: Software error Can't use string ("1") as a HASH ref while "strict refs" in use at /usr/share/kohaclone/circ/circulation.pl line 473 To test: - Apply patch - Repeat steps above - Verify that the sofware error is gone and that you get a message as with other invalid barcodes. - Test with other values: '00', '000', existing barcode, and a barcode that does not exist - Test with empty barcode (for printing slip) - Search for regressions This patch implements Jonathan's solution (see comment #15) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #46000|0 |1 is obsolete| | --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 46001 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46001&action=edit Bug 14015 - Checkout: Fix software error if barcode '0' is given To reproduce the bug: - Go to Home > Circulation > Checkouts > [a patron] - Enter 0 (zero) in barcode field and hit Enter Result: Software error Can't use string ("1") as a HASH ref while "strict refs" in use at /usr/share/kohaclone/circ/circulation.pl line 473 To test: - Apply patch - Repeat steps above - Verify that the sofware error is gone and that you get a message as with other invalid barcodes. - Test with other values: '00', '000', existing barcode, and a barcode that does not exist - Test with empty barcode (for printing slip) - Search for regressions This patch implements Jonathan's solution (see comment #15) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |11759 Referenced Bugs: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11759 [Bug 11759] Batch checkout -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Passed QA |In Discussion --- Comment #19 from Kyle M Hall <kyle@bywatersolutions.com> --- Not sure if I understand the problem correctly, but it seems that this patch doesn't work. I created an item with barcode 0. Without patch: Trying to check out gives blank screen Trying to check acts as if no barcode passed in With patch: Trying to check out gives "The barcode was not found 0" Trying to check acts as if no barcode passed in Is the intention of this patch to allow the use of '0' as an item barcode? Thanks! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 --- Comment #20 from Marc Véron <veron@veron.ch> --- Hi Kyle, You do not need an item with barcode '0' to reproduce the bug, and it is not the aim of this bug to make it possible to use a barcode '0' The aim of this bug is to get rid of the software error if you (per accident) enter 0 in the barcode field of the checkout page. I think the best description to reproduce is in comment #11 There are two different behaviours without patch: 1) If you try it for a patron with staff permissions, an error shows up: Can't use string ("1") as a HASH ref while "strict refs" in use at /usr/share/kohaclone/circ/circulation.pl line 470 2) If you try it for a user without staff permissions, the empty page appears (as in your test) With the patch applied, you should get the following message (for both cases): "The barcode was not found 0 Fast cataloging" -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 --- Comment #21 from Marc Véron <veron@veron.ch> --- I can still reprduce the error on current master. Patch still applies. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marc Véron from comment #21)
I can still reprduce the error on current master. Patch still applies.
It seems that Kyle missed the purpose of this report if I read comment19. If you reset the status, it will receive attention again from the RM. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Passed QA --- Comment #23 from Marc Véron <veron@veron.ch> --- Reset status as of comment #22 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14015 --- Comment #24 from Kyle M Hall <kyle@bywatersolutions.com> --- Thanks for the explanation Marc! Indeed I did misunderstand the intention of the patch. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org