From ef5c2cda9a60b362a9139be9c4298369e4f687f4 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 1 Apr 2015 10:12:05 +0200 Subject: [PATCH] Bug 13934: C4::ItemType->get should return undef if no parameter given The issue: If you try to check in an item with a non existent barcode, the application will exploded with a software error: "Can't bless non-reference at .../ItemType.pm Line 64". It's caused by: commit 7431f8cfe29e330e2232b0df591afc4d923b0a52 Bug 11944: Fix encoding issue in C4::ItemType and the following change: @@ -105,9 +104,6 @@ sub get { my $data = $dbh->selectrow_hashref( "SELECT * FROM itemtypes WHERE itemtype = ?", undef, $itemtype ); - if ( $data->{description} ) { - $data->{description} = Encode::encode('UTF-8', $data->{description}); - } because of the following: my $s; $s->{foo} = "bar" if $s->{foo}; use Data::Dumper;warn Dumper $s; => {} # not undef So later, bless $opts => $class; will fail because $opts is undef and was not (i.e. {}) before. More explicit test plan: 1) Log in to staff client 2) Circulation -> Check in 3) Type a non-existent barcode into 'Enter item barcode:' textbox 4) Click 'Submit' -- Should receive nasty error. 5) apply patch 6) repeat steps 2-4 -- Should be told 'No item with barcode: {what you typed}' 7) prove -v t/ItemType.t -- All tests should run successfully. 7) run koha qa test tools Note: Having tried to create and use an itemtype '0', this only demonstrates a lack of validation on the itemtype creation screen. Unable to use it without tweaking back end. That is beyond the scope of this bug. Note for QA: C4::ItemType->get is only uses in circ/return.pl. So even if the behavior is changed, it should not introduce any regression somewhere else. Signed-off-by: Josef Moravec Signed-off-by: Tomas Cohen Arazi Works as expected. Fixes the problem and no regressions found. It even has regression tests :-D Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi (cherry picked from commit 68e4f96511b070eb9f9ccfd5e31c6f51a70943fd) Signed-off-by: Chris Cormack --- C4/ItemType.pm | 3 +++ t/ItemType.t | 8 +++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/C4/ItemType.pm b/C4/ItemType.pm index ad253a7741..2a82b01573 100644 --- a/C4/ItemType.pm +++ b/C4/ItemType.pm @@ -99,6 +99,9 @@ an object. sub get { my ($class, $itemtype) = @_; + + return unless defined $itemtype; + my $dbh = C4::Context->dbh; my $data = $dbh->selectrow_hashref( diff --git a/t/ItemType.t b/t/ItemType.t index 558bba9bf5..238fe2ce6c 100755 --- a/t/ItemType.t +++ b/t/ItemType.t @@ -1,9 +1,8 @@ #!/usr/bin/perl -use strict; -use warnings; +use Modern::Perl; use DBI; -use Test::More tests => 26; +use Test::More tests => 27; use Test::MockModule; BEGIN { @@ -105,3 +104,6 @@ is( $itemtype->notforloan, '0', 'not for loan is 0' ); is( $itemtype->imageurl, '', ' not for loan is undef' ); is( $itemtype->checkinmsg, 'foo', 'checkinmsg is foo' ); + +$itemtype = C4::ItemType->get; +is( $itemtype, undef, 'C4::ItemType->get should return unless if no parameter is given' ); -- 2.39.5