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 <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Works as expected. Fixes the problem and no regressions found.
It even has regression tests :-D

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

diff --git a/C4/ItemType.pm b/C4/ItemType.pm
index 3a15287..a59923e 100644 (file)
--- 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 558bba9..238fe2c 100755 (executable)
--- 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' );