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>
This commit is contained in:
parent
a2be532dc2
commit
ca0bde1e7e
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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 );
|
||||
|
|
|
|||
|
|
@ -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 );
|
||||
|
|
|
|||
|
|
@ -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 );
|
||||
|
|
|
|||
Loading…
Reference in New Issue