Bug 17843: [QA Follow-up] Some polishing

Resolve warning from members/summary-print.pl:
    "my" variable $itemtype masks earlier declaration in same scope

Test if find returns a Koha object in GetDescription.
Test if find returns a Koha object too in shelves.pl. While testing, I had
a crash on a biblioitem with itemtype NULL (bad record, but these things
tend to happen somehow.)
Can't call method "imageurl" on an undefined value at virtualshelves/shelves.pl line 253.
Same for opac/opac-shelves.pl.

Note: Did not add tests everywhere but generally, I have the impression that
we do not sufficiently test on the results of Koha::Object->find. Mostly we
just assume that it will find a record. Several reports include fixes to
resolve that wrong assumption.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

diff --git a/Koha/Template/Plugin/ItemTypes.pm b/Koha/Template/Plugin/ItemTypes.pm
index 74ccacc..2091056 100644 (file)
--- a/Koha/Template/Plugin/ItemTypes.pm
+++ b/Koha/Template/Plugin/ItemTypes.pm
@@ -25,8 +25,9 @@ use base qw( Template::Plugin );
 use Koha::ItemTypes;
 
 sub GetDescription {
-    my ( $self, $itemtype ) = @_;
-    return Koha::ItemTypes->find( $itemtype )->translated_description;
+    my ( $self, $itemtypecode ) = @_;
+    my $itemtype = Koha::ItemTypes->find( $itemtypecode );
+    return $itemtype ? $itemtype->translated_description : q{};
 }
 
 sub Get {

diff --git a/members/summary-print.pl b/members/summary-print.pl
index 2d5eb47..a3d438c 100755 (executable)
--- a/members/summary-print.pl
+++ b/members/summary-print.pl
@@ -94,7 +94,7 @@ sub build_issue_data {
         my ( $charge, $itemtype ) =
           GetIssuingCharges( $issue->{itemnumber}, $borrowernumber );
 
-        my $itemtype = Koha::ItemTypes->find( $itemtype );
+        $itemtype = Koha::ItemTypes->find( $itemtype );
         $row{'itemtype_description'} = $itemtype->description; #FIXME Should not it be translated_description
 
         $row{'charge'} = sprintf( "%.2f", $charge );

diff --git a/opac/opac-shelves.pl b/opac/opac-shelves.pl
index b608657..85bc8e6 100755 (executable)
--- a/opac/opac-shelves.pl
+++ b/opac/opac-shelves.pl
@@ -285,9 +285,11 @@ if ( $op eq 'view' ) {
                 my $marcflavour = C4::Context->preference("marcflavour");
                 my $itemtype = Koha::Biblioitems->search({ biblionumber => $content->biblionumber })->next->itemtype;
                 $itemtype = Koha::ItemTypes->find( $itemtype );
-                $this_item->{imageurl}          = C4::Koha::getitemtypeimagelocation( 'opac', $itemtype->imageurl );
-                $this_item->{description}       = $itemtype->description; #FIXME Should not it be translated_description?
-                $this_item->{notforloan}        = $itemtype->notforloan;
+                if( $itemtype ) {
+                    $this_item->{imageurl}          = C4::Koha::getitemtypeimagelocation( 'opac', $itemtype->imageurl );
+                    $this_item->{description}       = $itemtype->description; #FIXME Should not it be translated_description?
+                    $this_item->{notforloan}        = $itemtype->notforloan;
+                }
                 $this_item->{'coins'}           = GetCOinSBiblio($record);
                 $this_item->{'subtitle'}        = GetRecordValue( 'subtitle', $record, GetFrameworkCode( $biblionumber ) );
                 $this_item->{'normalized_upc'}  = GetNormalizedUPC( $record, $marcflavour );

diff --git a/virtualshelves/shelves.pl b/virtualshelves/shelves.pl
index a868c24..86b7852 100755 (executable)
--- a/virtualshelves/shelves.pl
+++ b/virtualshelves/shelves.pl
@@ -253,9 +253,9 @@ if ( $op eq 'view' ) {
                 $this_item->{title}             = $biblio->title;
                 $this_item->{author}            = $biblio->author;
                 $this_item->{dateadded}         = $content->dateadded;
-                $this_item->{imageurl}          = C4::Koha::getitemtypeimagelocation( 'intranet', $itemtype->imageurl );
-                $this_item->{description}       = $itemtype->description; #FIXME Should not it be translated_description
-                $this_item->{notforloan}        = $itemtype->notforloan;
+                $this_item->{imageurl}          = $itemtype ? C4::Koha::getitemtypeimagelocation( 'intranet', $itemtype->imageurl ) : q{};
+                $this_item->{description}       = $itemtype ? $itemtype->description : q{}; #FIXME Should this be translated_description ?
+                $this_item->{notforloan}        = $itemtype->notforloan if $itemtype;
                 $this_item->{'coins'}           = GetCOinSBiblio($record);
                 $this_item->{'subtitle'}        = GetRecordValue( 'subtitle', $record, GetFrameworkCode( $biblionumber ) );
                 $this_item->{'normalized_upc'}  = GetNormalizedUPC( $record, $marcflavour );