From ca0bde1e7e168982efc8917c5836aeedea183621 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 23 Jun 2017 10:43:16 +0200 Subject: [PATCH] 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 --- Koha/Template/Plugin/ItemTypes.pm | 5 +++-- members/summary-print.pl | 2 +- opac/opac-shelves.pl | 8 +++++--- virtualshelves/shelves.pl | 6 +++--- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Koha/Template/Plugin/ItemTypes.pm b/Koha/Template/Plugin/ItemTypes.pm index 74ccacc653..20910562b0 100644 --- 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 2d5eb478de..a3d438c65e 100755 --- 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 b608657474..85bc8e60eb 100755 --- 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 a868c24604..86b7852b2b 100755 --- 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 ); -- 2.39.5