Bug 17698: (follow-up) Fixing things in Comment 57
[koha.git] / circ / returns.pl
index ddced56..103d290 100755 (executable)
@@ -27,11 +27,9 @@ script to execute returns of books
 
 =cut
 
-use strict;
-use warnings;
+use Modern::Perl;
 
-use Carp 'verbose';
-$SIG{ __DIE__ } = sub { Carp::confess( @_ ) };
+# FIXME There are weird things going on with $patron and $borrowernumber in this script
 
 use CGI qw ( -utf8 );
 use DateTime;
@@ -51,9 +49,10 @@ use Koha::AuthorisedValues;
 use Koha::DateUtils;
 use Koha::Calendar;
 use Koha::BiblioFrameworks;
-use Koha::Checkouts;
+use Koha::Holds;
 use Koha::Items;
 use Koha::Patrons;
+use Koha::Checkouts;
 
 my $query = new CGI;
 
@@ -80,8 +79,9 @@ if ($session->param('branch') eq 'NO_LIBRARY_SET'){
 if ( $query->param('print_slip') ) {
     $template->param(
         print_slip     => 1,
-        borrowernumber => scalar $query->param('borrowernumber'),
+        borrowernumber => scalar $query->param('borrowernumber'), # FIXME We should send a Koha::Patron and raise an error if not exist.
         biblionumber   => scalar $query->param('biblionumber'),
+        itemnumber     => scalar $query->param('itemnumber'),
     );
 }
 
@@ -157,7 +157,10 @@ if ( $query->param('reserve_id') ) {
     my $biblio = $item->biblio;
 
     if ( $cancel_reserve ) {
-        CancelReserve({ reserve_id => $reserve_id, charge_cancel_fee => !$forgivemanualholdsexpire });
+        my $hold = Koha::Holds->find( $reserve_id );
+        if ( $hold ) {
+            $hold->cancel( { charge_cancel_fee => !$forgivemanualholdsexpire } );
+        } # FIXME else?
     } else {
         my $diffBranchSend = ($userenv_branch ne $diffBranchReturned) ? $diffBranchReturned : undef;
         # diffBranchSend tells ModReserveAffect whether document is expected in this library or not,
@@ -167,8 +170,8 @@ if ( $query->param('reserve_id') ) {
 #   check if we have other reserves for this document, if we have a return send the message of transfer
     my ( $messages, $nextreservinfo ) = GetOtherReserves($itemnumber);
 
-    my $borr = GetMember( borrowernumber => $nextreservinfo );
-    my $name   = $borr->{'surname'} . ", " . $borr->{'title'} . " " . $borr->{'firstname'};
+    my $patron = Koha::Patrons->find( $nextreservinfo );
+    my $name   = $patron ? $patron->surname . ", " . $patron->title . " " . $patron->firstname : '';
     if ( $messages->{'transfert'} ) {
         $template->param(
             itemtitle      => $biblio->title,
@@ -176,11 +179,7 @@ if ( $query->param('reserve_id') ) {
             itembiblionumber => $biblio->biblionumber,
             iteminfo       => $biblio->author,
             name           => $name,
-            borrowernumber => $borrowernumber,
-            borcnum        => $borr->{'cardnumber'},
-            borfirstname   => $borr->{'firstname'},
-            borsurname     => $borr->{'surname'},
-            borcategory    => $borr->{'description'},
+            patron         => $patron,
             diffbranch     => 1,
         );
     }
@@ -259,42 +258,43 @@ if ($barcode) {
     $barcode = barcodedecode($barcode) if C4::Context->preference('itemBarcodeInputFilter');
     my $item = Koha::Items->find({ barcode => $barcode });
 
-    # Check if we should display a checkin message, based on the the item
-    # type of the checked in item
-    my $itemtype = Koha::ItemTypes->find( $item->effective_itemtype );
-    if ( $itemtype && $itemtype->checkinmsg ) {
-        $template->param(
-            checkinmsg     => $itemtype->checkinmsg,
-            checkinmsgtype => $itemtype->checkinmsgtype,
-        );
-    }
-
-    # make sure return branch respects home branch circulation rules, default to homebranch
-    my $hbr = GetBranchItemRule($item->homebranch, $itemtype ? $itemtype->itemtype : undef )->{'returnbranch'} || "homebranch";
-    $returnbranch = $item->$hbr;
+    if ( $item ) {
+        $itemnumber = $item->itemnumber;
+        # Check if we should display a checkin message, based on the the item
+        # type of the checked in item
+        my $itemtype = Koha::ItemTypes->find( $item->effective_itemtype );
+        if ( $itemtype && $itemtype->checkinmsg ) {
+            $template->param(
+                checkinmsg     => $itemtype->checkinmsg,
+                checkinmsgtype => $itemtype->checkinmsgtype,
+            );
+        }
 
-    my $materials = $item->materials;
-    my $descriptions = Koha::AuthorisedValues->get_description_by_koha_field({frameworkcode => '', kohafield =>'items.materials', authorised_value => $materials });
-    $materials = $descriptions->{lib} // $materials;
+        # make sure return branch respects home branch circulation rules, default to homebranch
+        my $hbr = GetBranchItemRule($item->homebranch, $itemtype ? $itemtype->itemtype : undef )->{'returnbranch'} || "homebranch";
+        $returnbranch = $hbr ne 'noreturn' ? $item->$hbr : $userenv_branch; # can be noreturn, homebranch or holdingbranch
 
-    my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } );
+        my $materials = $item->materials;
+        my $descriptions = Koha::AuthorisedValues->get_description_by_koha_field({frameworkcode => '', kohafield =>'items.materials', authorised_value => $materials });
+        $materials = $descriptions->{lib} // $materials;
 
-    my $biblio = $item->biblio;
-    $template->param(
-        title            => $biblio->title,
-        homebranch       => $item->homebranch,
-        holdingbranch    => $item->holdingbranch,
-        returnbranch     => $returnbranch,
-        author           => $biblio->author,
-        itembarcode      => $item->barcode,
-        itemtype         => $item->effective_itemtype,
-        ccode            => $item->ccode,
-        itembiblionumber => $biblio->biblionumber,
-        biblionumber     => $biblio->biblionumber,
-        borrower         => $borrower,
-        additional_materials => $materials,
-        issue            => $issue,
-    );
+        my $checkout = $item->checkout;
+        my $biblio   = $item->biblio;
+        $template->param(
+            title            => $biblio->title,
+            homebranch       => $item->homebranch,
+            holdingbranch    => $item->holdingbranch,
+            returnbranch     => $returnbranch,
+            author           => $biblio->author,
+            itembarcode      => $item->barcode,
+            itemtype         => $item->effective_itemtype,
+            ccode            => $item->ccode,
+            itembiblionumber => $biblio->biblionumber,
+            biblionumber     => $biblio->biblionumber,
+            additional_materials => $materials,
+            issue            => $checkout,
+        );
+    } # FIXME else we should not call AddReturn but set BadBarcode directly instead
 
     my %input = (
         counter => 0,
@@ -302,6 +302,7 @@ if ($barcode) {
         barcode => $barcode,
     );
 
+
     # do the return
     ( $returned, $messages, $issue, $borrower ) =
       AddReturn( $barcode, $userenv_branch, $exemptfine, $dropboxmode, $return_date_override, $dropboxdate );
@@ -396,27 +397,15 @@ if ( $messages->{'WrongTransfer'} and not $messages->{'WasTransfered'}) {
     );
 
     my $reserve    = $messages->{'ResFound'};
-    my $borr = C4::Members::GetMember( borrowernumber => $reserve->{'borrowernumber'} );
-    my $name = $borr->{'surname'} . ", " . $borr->{'title'} . " " . $borr->{'firstname'};
+    if ( $reserve ) {
+        my $patron = Koha::Patrons->find( $reserve->{'borrowernumber'} );
+        my $name = $patron->surname . ", " . $patron->title . " " . $patron->firstname;
+        $template->param(
+            patron => $patron,
+        );
+    }
     $template->param(
-            wname           => $name,
-            wborfirstname   => $borr->{'firstname'},
-            wborsurname     => $borr->{'surname'},
-            wborcategory    => $borr->{'description'},
-            wbortitle       => $borr->{'title'},
-            wborphone       => $borr->{'phone'},
-            wboremail       => $borr->{'email'},
-            streetnumber    => $borr->{streetnumber},
-            streettype      => $borr->{streettype},
-            address         => $borr->{'address'},
-            address2        => $borr->{'address2'},
-            city            => $borr->{'city'},
-            zipcode         => $borr->{'zipcode'},
-            state           => $borr->{'state'},
-            country         => $borr->{'country'},
-            wborrowernumber => $reserve->{'borrowernumber'},
-            wborcnum        => $borr->{'cardnumber'},
-            wtransfertFrom  => $userenv_branch,
+        wtransfertFrom  => $userenv_branch,
     );
 }
 
@@ -425,7 +414,7 @@ if ( $messages->{'WrongTransfer'} and not $messages->{'WasTransfered'}) {
 #
 if ( $messages->{'ResFound'}) {
     my $reserve    = $messages->{'ResFound'};
-    my $borr = C4::Members::GetMember( borrowernumber => $reserve->{'borrowernumber'} );
+    my $patron = Koha::Patrons->find( $reserve->{borrowernumber} );
     my $holdmsgpreferences =  C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $reserve->{'borrowernumber'}, message_name   => 'Hold_Filled' } );
     if ( $reserve->{'ResFound'} eq "Waiting" or $reserve->{'ResFound'} eq "Reserved" ) {
         if ( $reserve->{'ResFound'} eq "Waiting" ) {
@@ -443,28 +432,11 @@ if ( $messages->{'ResFound'}) {
 
         # same params for Waiting or Reserved
         $template->param(
+            # FIXME The full patron object should be passed to the template
             found          => 1,
-            name           => $borr->{'surname'} . ", " . $borr->{'title'} . " " . $borr->{'firstname'},
-            borfirstname   => $borr->{'firstname'},
-            borsurname     => $borr->{'surname'},
-            borcategory    => $borr->{'description'},
-            bortitle       => $borr->{'title'},
-            borphone       => $borr->{'phone'},
-            boremail       => $borr->{'email'},
-            streetnumber   => $borr->{streetnumber},
-            streettype     => $borr->{streettype},
-            address        => $borr->{'address'},
-            address2       => $borr->{'address2'},
-            city           => $borr->{'city'},
-            zipcode        => $borr->{'zipcode'},
-            state          => $borr->{'state'},
-            country        => $borr->{'country'},
-            borcnum        => $borr->{'cardnumber'},
-            debarred       => $borr->{'debarred'},
-            gonenoaddress  => $borr->{'gonenoaddress'},
+            patron         => $patron,
             barcode        => $barcode,
             destbranch     => $reserve->{'branchcode'},
-            borrowernumber => $reserve->{'borrowernumber'},
             itemnumber     => $reserve->{'itemnumber'},
             reservenotes   => $reserve->{'reservenotes'},
             reserve_id     => $reserve->{reserve_id},
@@ -485,13 +457,13 @@ foreach my $code ( keys %$messages ) {
     elsif ( $code eq 'NotIssued' ) {
         $err{notissued} = 1;
         $err{msg} = '';
-        $err{msg} = $messages->{'IsPermanent'} if $messages->{'IsPermanent'};
     }
     elsif ( $code eq 'LocalUse' ) {
         $err{localuse} = 1;
     }
     elsif ( $code eq 'WasLost' ) {
         $err{waslost} = 1;
+        $exit_required_p = 1 if C4::Context->preference("BlockReturnOfLostItems");
     }
     elsif ( $code eq 'LostItemFeeRefunded' ) {
         $template->param( LostItemFeeRefunded => 1 );
@@ -509,12 +481,6 @@ foreach my $code ( keys %$messages ) {
         $err{withdrawn} = 1;
         $exit_required_p = 1 if C4::Context->preference("BlockReturnOfWithdrawnItems");
     }
-    elsif ( ( $code eq 'IsPermanent' ) && ( not $messages->{'ResFound'} ) ) {
-        if ( $messages->{'IsPermanent'} ne $userenv_branch ) {
-            $err{ispermanent} = 1;
-            $err{msg}         = $messages->{'IsPermanent'};
-        }
-    }
     elsif ( $code eq 'WrongTransfer' ) {
         ;    # FIXME... anything to do here?
     }
@@ -540,6 +506,9 @@ foreach my $code ( keys %$messages ) {
     elsif ( $code eq 'NotForLoanStatusUpdated' ) {
         $err{NotForLoanStatusUpdated} = $messages->{NotForLoanStatusUpdated};
     }
+    elsif ( $code eq 'DataCorrupted' ) {
+        $err{data_corrupted} = 1;
+    }
     else {
         die "Unknown error code $code";    # note we need all the (empty) elsif's above, or we die.
         # This forces the issue of staying in sync w/ Circulation.pm
@@ -569,20 +538,14 @@ foreach ( sort { $a <=> $b } keys %returneditems ) {
             $ri{hour}   = $duedate->hour();
             $ri{minute}   = $duedate->minute();
             $ri{duedate} = output_pref($duedate);
-            my $b      = C4::Members::GetMember( borrowernumber => $riborrowernumber{$_} );
+            my $patron = Koha::Patrons->find( $riborrowernumber{$_} );
             unless ( $dropboxmode ) {
                 $ri{return_overdue} = 1 if (DateTime->compare($duedate, DateTime->now()) == -1);
             } else {
                 $ri{return_overdue} = 1 if (DateTime->compare($duedate, $dropboxdate) == -1);
             }
-            $ri{borrowernumber} = $b->{'borrowernumber'};
-            $ri{borcnum}        = $b->{'cardnumber'};
-            $ri{borfirstname}   = $b->{'firstname'};
-            $ri{borsurname}     = $b->{'surname'};
-            $ri{bortitle}       = $b->{'title'};
-            $ri{bornote}        = $b->{'borrowernotes'};
-            $ri{borcategorycode}= $b->{'categorycode'};
-            $ri{borissuescount} = Koha::Checkouts->count( { borrowernumber => $b->{'borrowernumber'} } );
+            $ri{patron} = $patron,
+            $ri{borissuescount} = $patron->checkouts->count;
         }
         else {
             $ri{borrowernumber} = $riborrowernumber{$_};
@@ -606,7 +569,7 @@ foreach ( sort { $a <=> $b } keys %returneditems ) {
         $ri{homebranch}          = $item->homebranch;
         $ri{holdingbranch}       = $item->holdingbranch;
 
-        $ri{location}         = $biblio->{'location'};
+        $ri{location} = $item->location;
         my $shelfcode = $ri{'location'};
         $ri{'location'} = $shelflocations->{$shelfcode} if ( defined( $shelfcode ) && defined($shelflocations) && exists( $shelflocations->{$shelfcode} ) );
 
@@ -617,6 +580,8 @@ foreach ( sort { $a <=> $b } keys %returneditems ) {
     push @riloop, \%ri;
 }
 
+my $pending_checkout_notes = Koha::Checkouts->search({ noteseen => 0 })->count;
+
 $template->param(
     riloop         => \@riloop,
     printer        => $printer,
@@ -627,7 +592,7 @@ $template->param(
     forgivemanualholdsexpire => $forgivemanualholdsexpire,
     overduecharges => $overduecharges,
     AudioAlerts        => C4::Context->preference("AudioAlerts"),
-    BlockReturnOfWithdrawnItems => C4::Context->preference("BlockReturnOfWithdrawnItems"),
+    pending_checkout_notes => $pending_checkout_notes,
 );
 
 $itemnumber = GetItemnumberFromBarcode( $barcode );