[Koha-patches] [PATCH] Bug 2505: adding warnings to opac-detail.pl

[Andrew Moore]

Before starting to edit opac/opac-detail.pl, I added 'use warnings' and
eliminated all of the warnings that I could get it to produce. I also
extracted two lines that prepared an amazon ISBN into a function. They
were causing a few warnings and might as well be extracted. They should
probably be moved to a module somewhere since these two lines are copied
all over our code.
---
 opac/opac-detail.pl |   65 +++++++++++++++++++++++++++++++++-----------------
 1 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl
index de61ce3..40688d2 100755
--- a/opac/opac-detail.pl
+++ b/opac/opac-detail.pl
@@ -19,6 +19,7 @@
 
 
 use strict;
+use warnings;
 
 use CGI;
 use C4::Auth;
@@ -116,23 +117,27 @@ my $norequests = 1;
 my $branches = GetBranches();
 my %itemfields;
 for my $itm (@items) {
-     $norequests = 0
+    $norequests = 0
        if ( (not $itm->{'wthdrawn'} )
          && (not $itm->{'itemlost'} )
          && ($itm->{'itemnotforloan'}<0 || not $itm->{'itemnotforloan'} )
 		 && (not $itemtypes->{$itm->{'itype'}}->{notforloan} )
          && ($itm->{'itemnumber'} ) );
 
-    $itm->{ $itm->{'publictype'} } = 1;
+    if ( defined $itm->{'publictype'} ) {
+        # I can't actually find any case in which this is defined. --amoore 2008-12-09
+        $itm->{ $itm->{'publictype'} } = 1;
+    }
     $itm->{datedue}      = format_date($itm->{datedue});
     $itm->{datelastseen} = format_date($itm->{datelastseen});
 
-    #get collection code description, too
-	my $ccode= $itm->{'ccode'};
-	$itm->{'ccode'} = $collections->{$ccode} if(defined($collections) && exists($collections->{$ccode}));
-    $itm->{'location_description'} = $shelflocations->{$itm->{'location'} };
-    $itm->{'imageurl'}    = getitemtypeimagelocation( 'opac', $itemtypes->{ $itm->{itype} }->{'imageurl'} );
-    $itm->{'description'} = $itemtypes->{$itemtype}->{'description'};
+    # get collection code description, too
+    if ( my $ccode = $itm->{'ccode'} ) {
+        $itm->{'ccode'} = $collections->{$ccode} if ( defined($collections) && exists( $collections->{$ccode} ) );
+    }
+    if ( defined $itm->{'location'} ) {
+        $itm->{'location_description'} = $shelflocations->{ $itm->{'location'} };
+    }
     foreach (qw(ccode enumchron copynumber itemnotes)) {
         $itemfields{$_} = 1 if ($itm->{$_});
     }
@@ -149,7 +154,7 @@ for my $itm (@items) {
 
     
      my ( $transfertwhen, $transfertfrom, $transfertto ) = GetTransfers($itm->{itemnumber});
-     if ( $transfertwhen ne '' ) {
+     if ( defined( $transfertwhen ) && $transfertwhen ne '' ) {
         $itm->{transfertwhen} = format_date($transfertwhen);
         $itm->{transfertfrom} = $branches->{$transfertfrom}{branchname};
         $itm->{transfertto}   = $branches->{$transfertto}{branchname};
@@ -184,7 +189,7 @@ my $subtitle         = C4::Biblio::get_koha_field_from_marc('bibliosubtitle', 's
     );
 
 foreach ( keys %{$dat} ) {
-    $template->param( "$_" => $dat->{$_} . "" );
+    $template->param( "$_" => defined $dat->{$_} ? $dat->{$_} : '' );
 }
 
 # COinS format FIXME: for books Only
@@ -229,24 +234,40 @@ $template->param(
 );
 
 sub isbn_cleanup ($) {
-	my $isbn=shift;
+    my $isbn = shift;
+    return unless $isbn;
     ($isbn) = $isbn =~ /([\d-]*[X]*)/;
     $isbn =~ s/-//g;
-	if (
-		$isbn =~ /\b(\d{13})\b/ or
-		$isbn =~ /\b(\d{10})\b/ or 
-		$isbn =~ /\b(\d{9}X)\b/i
-	) {
-		return $1;
-	}
-	return undef;
+    if (   $isbn =~ /\b(\d{13})\b/
+        or $isbn =~ /\b(\d{10})\b/
+        or $isbn =~ /\b(\d{9}X)\b/i ) {
+        return $1;
+    }
+    return;
+}
+
+=head3 get_amazon_isbn_from_isbn
+
+pass in an ISBN.
+
+This function returns that ISBN formatted for amazon lookups.
+
+# FIXME: so it is OK if the ISBN = 'XXXXX' ?
+
+=cut
+
+sub get_amazon_isbn_from_isbn {
+    my $isbn = shift;
+    return unless $isbn;
+
+    ( my $aisbn ) = $isbn =~ /([\d-]*[X]*)/;
+    $aisbn =~ s/-//g;
+    return $aisbn;
 }
 
 # XISBN Stuff
 my $xisbn=$dat->{'isbn'};
-(my $aisbn) = $xisbn =~ /([\d-]*[X]*)/;
-$aisbn =~ s/-//g;
-$template->param(amazonisbn => $aisbn);		# FIXME: so it is OK if the ISBN = 'XXXXX' ?
+$template->param( amazonisbn => get_amazon_isbn_from_isbn($xisbn) );
 my ($clean,$clean2);
 # these might be overkill, but they are better than the regexp above.
 if ($clean = isbn_cleanup($xisbn)){
-- 
1.5.6