BugFixing : 3306
[koha.git] / C4 / Circulation.pm
index f899089..7c9fe89 100644 (file)
@@ -318,16 +318,7 @@ sub TooMany {
     my $dbh             = C4::Context->dbh;
        my $branch;
        # Get which branchcode we need
-       if (C4::Context->preference('CircControl') eq 'PickupLibrary'){
-               $branch = C4::Context->userenv->{'branch'}; 
-       }
-       elsif (C4::Context->preference('CircControl') eq 'PatronLibrary'){
-        $branch = $borrower->{'branchcode'}; 
-       }
-       else {
-               # items home library
-               $branch = $item->{'homebranch'};
-       }
+       $branch = _GetCircControlBranch($item,$borrower);
        my $type = (C4::Context->preference('item-level_itypes')) 
                        ? $item->{'itype'}         # item-level
                        : $item->{'itemtype'};     # biblio-level
@@ -659,9 +650,8 @@ sub CanBookBeIssued {
     #
     unless ( $duedate ) {
         my $issuedate = strftime( "%Y-%m-%d", localtime );
-        my $branch = (C4::Context->preference('CircControl') eq 'PickupLibrary') ? C4::Context->userenv->{'branch'} :
-                     (C4::Context->preference('CircControl') eq 'PatronLibrary') ? $borrower->{'branchcode'}        :
-                     $item->{'homebranch'};     # fallback to item's homebranch
+
+        my $branch = _GetCircControlBranch($item,$borrower);
         my $itype = ( C4::Context->preference('item-level_itypes') ) ? $item->{'itype'} : $biblioitem->{'itemtype'};
         my $loanlength = GetLoanLength( $borrower->{'categorycode'}, $itype, $branch );
         $duedate = CalcDateDue( C4::Dates->new( $issuedate, 'iso' ), $loanlength, $branch, $borrower );
@@ -898,9 +888,8 @@ sub AddIssue {
        if ($borrower and $barcode and $barcodecheck ne '0'){
                # find which item we issue
                my $item = GetItem('', $barcode) or return undef;       # if we don't get an Item, abort.
-               my $branch = (C4::Context->preference('CircControl') eq 'PickupLibrary') ? C4::Context->userenv->{'branch'} :
-                     (C4::Context->preference('CircControl') eq 'PatronLibrary') ? $borrower->{'branchcode'}        : 
-                     $item->{'homebranch'};     # fallback to item's homebranch
+               my $hbr = C4::Context->preference("HomeOrHoldingBranch")||"homebranch";
+               my $branch = _GetCircControlBranch($item,$borrower);
                
                # get actual issuing if there is one
                my $actualissue = GetItemIssue( $item->{itemnumber});
@@ -990,7 +979,7 @@ sub AddIssue {
         unless ($datedue) {
             my $itype = ( C4::Context->preference('item-level_itypes') ) ? $biblio->{'itype'} : $biblio->{'itemtype'};
             my $loanlength = GetLoanLength( $borrower->{'categorycode'}, $itype, $branch );
-            $datedue = CalcDateDue( C4::Dates->new( $issuedate, 'iso' ), $loanlength, $branch );
+            $datedue = CalcDateDue( C4::Dates->new( $issuedate, 'iso' ), $loanlength, $branch, $borrower );
 
             # if ReturnBeforeExpiry ON the datedue can't be after borrower expirydate
             if ( C4::Context->preference('ReturnBeforeExpiry') && $datedue->output('iso') gt $borrower->{dateexpiry} ) {
@@ -1002,12 +991,11 @@ sub AddIssue {
             $item->{'itemnumber'},              # itemnumber
             $issuedate,                         # issuedate
             $datedue->output('iso'),            # date_due
-            C4::Context->userenv->{'branch'}    # branchcode
+            $branch                                                            # branchcode
         );
-        $sth->finish;
         $item->{'issues'}++;
         ModItem({ issues           => $item->{'issues'},
-                  holdingbranch    => C4::Context->userenv->{'branch'},
+                  holdingbranch    => C4::Context->userenv->{branch},
                   itemlost         => 0,
                   datelastborrowed => C4::Dates->new()->output('iso'),
                   onloan           => $datedue->output('iso'),
@@ -1315,65 +1303,66 @@ patron who last borrowed the book.
 
 sub AddReturn {
     my ( $barcode, $branch, $exemptfine, $dropbox ) = @_;
-    if ($branch and not GetBranchDetail($branch)) {
-        warn "AddReturn error: branch '$branch' not found.  Reverting to " . C4::Context->userenv->{'branch'};
-        undef $branch;
-    }
-    $branch = C4::Context->userenv->{'branch'} unless $branch;  # we trust userenv to be a safe fallback/default
+    my $dbh      = C4::Context->dbh;
     my $messages;
+    my $doreturn = 1;
     my $borrower;
-    my $doreturn       = 1;
     my $validTransfert = 0;
-    my $reserveDone    = 0;
+    my $reserveDone = 0;
+       $branch ||=C4::Context->userenv->{'branch'};
     
     # get information on item
-    my $itemnumber      = GetItemnumberFromBarcode( $barcode );
-    my $iteminformation = GetItemIssue($itemnumber);
-    my $biblio          = GetBiblioItemData($iteminformation->{'biblioitemnumber'});
+    my $itemnumber = GetItemnumberFromBarcode($barcode);
+    my $iteminformation = GetItemIssue( $itemnumber );
+    my $biblio = GetBiblioItemData($iteminformation->{'biblioitemnumber'});
 #     use Data::Dumper;warn Data::Dumper::Dumper($iteminformation);  
-    unless ($itemnumber) {
+    unless ( $iteminformation->{'itemnumber'} or $itemnumber) {
         $messages->{'BadBarcode'} = $barcode;
         $doreturn = 0;
     } else {
-        if ( not $iteminformation ) {
+        # find the borrower
+        if ( not $iteminformation->{borrowernumber} ) {
             $messages->{'NotIssued'} = $barcode;
-            # even though item is not on loan, it may still
-            # be transferred; therefore, get current branch information
-            my $curr_iteminfo = GetItem($iteminformation->{'itemnumber'});
-            $iteminformation->{'homebranch'}    = $curr_iteminfo->{'homebranch'};
-            $iteminformation->{'holdingbranch'} = $curr_iteminfo->{'holdingbranch'};
-            $iteminformation->{'itemlost'}      = $curr_iteminfo->{'itemlost'};
-            # These lines patch up $iteminformation enough so it can be used below for other messages
             $doreturn = 0;
         }
-
-        my $hbr = $iteminformation->{C4::Context->preference("HomeOrHoldingBranch")} || '';
+        
+        # even though item is not on loan, it may still
+        # be transferred; therefore, get current branch information
+        my $curr_iteminfo = GetItem($itemnumber);
+        $iteminformation->{'homebranch'} = $curr_iteminfo->{'homebranch'};
+        $iteminformation->{'holdingbranch'} = $curr_iteminfo->{'holdingbranch'};
+        $iteminformation->{'itemlost'} = $curr_iteminfo->{'itemlost'};
+        
         # check if the book is in a permanent collection....
-        # FIXME -- This 'PE' attribute is largely undocumented.  afaict, there's no user interface that reflects this functionality.
-        if ( $hbr ) {
-            my $branches = GetBranches();    # a potentially expensive call for a non-feature.
-            $branches->{$hbr}->{PE} and $messages->{'IsPermanent'} = $hbr;
-        }
-
-        # if indy branches and returning to different branch, refuse the return
-        if ($hbr ne $branch && C4::Context->preference("IndependantBranches")){
-            $messages->{'Wrongbranch'} = 1;
-            $doreturn = 0;
+        my $hbr      = $iteminformation->{C4::Context->preference("HomeOrHoldingBranch")};
+        my $branches = GetBranches();
+               # FIXME -- This 'PE' attribute is largely undocumented.  afaict, there's no user interface that reflects this functionality.
+        if ( $hbr && $branches->{$hbr}->{'PE'} ) {
+            $messages->{'IsPermanent'} = $hbr;
         }
-
-        if ( $iteminformation->{'wthdrawn'} ) { # book has been cancelled
+               
+                   # if independent branches are on and returning to different branch, refuse the return
+        if ($hbr ne $branch && C4::Context->preference("IndependantBranches") && $iteminformation->{borrowernumber}){
+                         $messages->{'Wrongbranch'} = 1;
+                         $doreturn=0;
+                   }
+                       
+        # check that the book has been cancelled
+        if ( $iteminformation->{'wthdrawn'} ) {
             $messages->{'wthdrawn'} = 1;
             $doreturn = 0;
         }
+    
 
-        # if the book returned in an other branch, update the holding branch
-        # update issues, thereby returning book (should push this out into another subroutine
+    #     new op dev : if the book returned in an other branch update the holding branch
+    
+    # update issues, thereby returning book (should push this out into another subroutine
         $borrower = C4::Members::GetMemberDetails( $iteminformation->{borrowernumber}, 0 );
-
-        # case of a return of document (deal with issues and holdingbranch)
+    
+    # case of a return of document (deal with issues and holdingbranch)
     
         if ($doreturn) {
-                       my $circControlBranch = GetCirculationBranch($iteminformation,$borrower);
+                       my $circControlBranch = _GetCircControlBranch($iteminformation,$borrower);
                        if($dropbox) {
                                # don't allow dropbox mode to create an invalid entry in issues (issuedate > returndate) FIXME: actually checks eq, not gt
                                undef($dropbox) if ( $iteminformation->{'issuedate'} eq C4::Dates->today('iso') );
@@ -1382,54 +1371,60 @@ sub AddReturn {
             $messages->{'WasReturned'} = 1;    # FIXME is the "= 1" right?  
             # continue to deal with returns cases, but not only if we have an issue
             
-            # the holdingbranch is updated if the document is returned in an other location .
-            if ( $iteminformation->{'holdingbranch'} ne $branch ) {
-                UpdateHoldingbranch($branch, $iteminformation->{'itemnumber'});
-                $iteminformation->{'holdingbranch'} = $branch; # update iteminformation holdingbranch too
-            }
+            
+            # We update the holdingbranch from circControlBranch variable
+            UpdateHoldingbranch($branch,$iteminformation->{'itemnumber'});
+            $iteminformation->{'holdingbranch'} = $branch;
+        
+            
             ModDateLastSeen( $iteminformation->{'itemnumber'} );
             ModItem({ onloan => undef }, $biblio->{'biblionumber'}, $iteminformation->{'itemnumber'});
-          
+
             if ($iteminformation->{borrowernumber}){
-                $borrower = C4::Members::GetMemberDetails( $iteminformation->{borrowernumber}, 0 ); # FIXME: we shouldn't need to make the same call twice
+              ($borrower) = C4::Members::GetMemberDetails( $iteminformation->{borrowernumber}, 0 );
             }
         }
     
-        # check if we have a transfer for this document
+    # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
+    #     check if we have a transfer for this document
         my ($datesent,$frombranch,$tobranch) = GetTransfers( $iteminformation->{'itemnumber'} );
     
-        # if we have a transfer to do, we update the line of transfers with the datearrived
+    #     if we have a transfer to do, we update the line of transfers with the datearrived
         if ($datesent) {
             if ( $tobranch eq $branch ) {
-                my $sth = C4::Context->dbh->prepare(
-                    "UPDATE branchtransfers SET datearrived = now() WHERE itemnumber= ? AND datearrived IS NULL"
-                );
-                $sth->execute( $iteminformation->{'itemnumber'} );
-                # if we have a reservation with the validate of transfer, we can set it's status to 'W'
-                C4::Reserves::ModReserveStatus($iteminformation->{'itemnumber'}, 'W');
-            } else {
-                $messages->{'WrongTransfer'}     = $tobranch;
-                $messages->{'WrongTransferItem'} = $iteminformation->{'itemnumber'};
+                    my $sth =
+                    $dbh->prepare(
+                            "UPDATE branchtransfers SET datearrived = now() WHERE itemnumber= ? AND datearrived IS NULL"
+                    );
+                    $sth->execute( $iteminformation->{'itemnumber'} );
+                    $sth->finish;
+    #         now we check if there is a reservation with the validate of transfer if we have one, we can         set it with the status 'W'
+            C4::Reserves::ModReserveStatus( $iteminformation->{'itemnumber'},'W' );
             }
-            $validTransfert = 1;
+        else {
+            $messages->{'WrongTransfer'} = $tobranch;
+            $messages->{'WrongTransferItem'} = $iteminformation->{'itemnumber'};
+        }
+        $validTransfert = 1;
         }
     
+    # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # 
         # fix up the accounts.....
         if ($iteminformation->{'itemlost'}) {
                 FixAccountForLostAndReturned($iteminformation, $borrower);
                 ModItem({ itemlost => '0' }, $biblio->{'biblionumber'}, $iteminformation->{'itemnumber'});
                 $messages->{'WasLost'} = 1;
         }
-
         # fix up the overdues in accounts...
         FixOverduesOnReturn( $borrower->{'borrowernumber'},
             $iteminformation->{'itemnumber'}, $exemptfine, $dropbox );
     
-        # find reserves.....
-        # if we don't have a reserve with the status W, we launch the Checkreserves routine
-        my ( $resfound, $resrec ) = C4::Reserves::CheckReserves( $iteminformation->{'itemnumber'} );
+    # find reserves.....
+    #     if we don't have a reserve with the status W, we launch the Checkreserves routine
+        my ( $resfound, $resrec ) = 
+        C4::Reserves::CheckReserves( $itemnumber, $barcode );
         if ($resfound) {
-              $resrec->{'ResFound'} = $resfound;
+            $resrec->{'ResFound'}   = $resfound;
             $messages->{'ResFound'} = $resrec;
             $reserveDone = 1;
         }
@@ -1449,16 +1444,12 @@ sub AddReturn {
         #adding message if holdingbranch is non equal a userenv branch to return the document to homebranch
         #we check, if we don't have reserv or transfert for this document, if not, return it to homebranch .
         
-        if ($doreturn and ($branch ne $iteminformation->{$hbr}) and not $messages->{'WrongTransfer'} and ($validTransfert ne 1) and ($reserveDone ne 1) ){
+        if (($doreturn or $messages->{'NotIssued'}) and ($branch ne $iteminformation->{$hbr}) and not $messages->{'WrongTransfer'} and ($validTransfert ne 1) and ($reserveDone ne 1) ){
                        if (C4::Context->preference("AutomaticItemReturn") == 1) {
-                               ModItemTransfer($iteminformation->{'itemnumber'}, C4::Context->userenv->{'branch'}, $iteminformation->{$hbr});
+                               ModItemTransfer($iteminformation->{'itemnumber'}, $branch, $iteminformation->{$hbr});
                                $messages->{'WasTransfered'} = 1;
-                       } elsif ( C4::Context->preference("UseBranchTransferLimits") == 1 
-                                       && ! IsBranchTransferAllowed( $branch, $iteminformation->{'homebranch'}, $iteminformation->{ C4::Context->preference("BranchTransferLimitsType") } )
-                               ) {
-                               ModItemTransfer($iteminformation->{'itemnumber'}, C4::Context->userenv->{'branch'}, $iteminformation->{'homebranch'});
-                                $messages->{'WasTransfered'} = 1;
-                       } else {
+                       }
+                       else {
                                $messages->{'NeedsTransfer'} = 1;
                        }
         }
@@ -1671,32 +1662,36 @@ sub FixAccountForLostAndReturned {
        return;
 }
 
-=head2 GetCirculationBranch
+=head2 _GetCircControlBranch
 
-       &GetCirculationBranch($iteminfos,$borrower);
+   my $circ_control_branch = _GetCircControlBranch($iteminfos, $borrower);
 
-Return the branchcode that must be used for an item circulation
+Internal function : 
+
+Return the library code to be used to determine which circulation
+policy applies to a transaction.  Looks up the CircControl and
+HomeOrHoldingBranch system preferences.
 
 C<$iteminfos> is a hashref to iteminfo. Only {itemnumber} is used.
 
 C<$borrower> is a hashref to borrower. Only {borrowernumber is used.
 
-Internal function, called by AddReturn
-
 =cut
 
-sub GetCirculationBranch {
+sub _GetCircControlBranch {
     my ($iteminfos, $borrower) = @_;
     my $circcontrol = C4::Context->preference('CircControl');
-    
-    if($circcontrol eq 'ItemHomeLibrary'){
-        return $iteminfos->{C4::Context->preference("HomeOrHoldingBranch")};
-    }elsif($circcontrol eq 'PatronLibrary'){
-        return $borrower->{branchcode};
-    }elsif($circcontrol eq 'PickupLibrary'){
-        return C4::Context->userenv->{'branch'};
+    my $branch;
+
+    if ($circcontrol eq 'PickupLibrary') {
+        $branch= C4::Context->userenv->{'branch'};
+    } elsif ($circcontrol eq 'PatronLibrary') {
+        $branch=$borrower->{branchcode};
+    } else {
+        my $branchfield = C4::Context->preference('HomeOrHoldingBranch') || 'homebranch';
+        $branch = $iteminfos->{$branchfield};
     }
-    
+    return $branch;
 }
 
 =head2 GetItemIssue
@@ -1940,7 +1935,8 @@ has the item.
 
 C<$itemnumber> is the number of the item to renew.
 
-C<$branch> is the library branch.  Defaults to the homebranch of the ITEM.
+C<$branch> is the library where the renewal took place (if any).
+           The library that controls the circ policies for the renewal is retrieved from the issues record.
 
 C<$datedue> can be a C4::Dates object used to set the due date.
 
@@ -1953,13 +1949,16 @@ from the book's item type.
 =cut
 
 sub AddRenewal {
-       my $borrowernumber = shift or return undef;
-       my     $itemnumber = shift or return undef;
+    
+    my $borrowernumber  = shift or return undef;
+    my $itemnumber      = shift or return undef;
     my $item   = GetItem($itemnumber) or return undef;
-    my $biblio = GetBiblioFromItemNumber($itemnumber) or return undef;
     my $branch  = (@_) ? shift : $item->{homebranch};  # opac-renew doesn't send branch
-    my $datedue = shift;
-    my $lastreneweddate = shift;
+    my $datedue         = shift;
+    my $lastreneweddate = shift || C4::Dates->new()->output('iso');
+
+
+    my $biblio = GetBiblioFromItemNumber($itemnumber) or return undef;
 
     # If the due date wasn't specified, calculate it by adding the
     # book's loan length to today's date.
@@ -1972,7 +1971,7 @@ sub AddRenewal {
                        $item->{homebranch}                     # item's homebranch determines loanlength OR do we want the branch specified by the AddRenewal argument?
         );
                #FIXME -- use circControl?
-               $datedue =  CalcDateDue(C4::Dates->new(),$loanlength,$branch);  # this branch is the transactional branch.
+               $datedue =  CalcDateDue(C4::Dates->new(),$loanlength,$branch,$borrower);        # this branch is the transactional branch.
                                                                # The question of whether to use item's homebranch calendar is open.
     }
 
@@ -1991,25 +1990,25 @@ sub AddRenewal {
     $sth->execute( $borrowernumber, $itemnumber );
     my $issuedata = $sth->fetchrow_hashref;
     $sth->finish;
-
+    if($datedue && ! $datedue->output('iso')){
+        warn "Invalid date passed to AddRenewal.";
+        return undef;
+    }
     # If the due date wasn't specified, calculate it by adding the
     # book's loan length to today's date or the current due date
     # based on the value of the RenewalPeriodBase syspref.
-    unless ($datedue && $datedue->output('iso')) {
+    unless ($datedue) {
 
         my $borrower = C4::Members::GetMemberDetails( $borrowernumber, 0 ) or return undef;
         my $loanlength = GetLoanLength(
-            $borrower->{'categorycode'},
-             (C4::Context->preference('item-level_itypes')) ? $biblio->{'itype'} : $biblio->{'itemtype'} ,
-                       $item->{homebranch}     # item's homebranch determines loanlength OR do we want the branch specified by the AddRenewal argument?
-        );
+                    $borrower->{'categorycode'},
+                    (C4::Context->preference('item-level_itypes')) ? $biblio->{'itype'} : $biblio->{'itemtype'} ,
+                               $issuedata->{'branchcode'}  );   # that's the circ control branch.
 
         $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
                                         C4::Dates->new($issuedata->{date_due}, 'iso') :
                                         C4::Dates->new();
-        #FIXME -- use circControl?
-        $datedue =  CalcDateDue($datedue,$loanlength,$branch,$borrower);    # this branch is the transactional branch.
-        # The question of whether to use item's homebranch calendar is open.
+        $datedue =  CalcDateDue($datedue,$loanlength,$issuedata->{'branchcode'},$borrower);
     }
 
     # Update the issues record to have the new due date, and a new count