Merge remote branch 'kc/new/bug_3326' into kcmaster

[koha.git] / C4 / Circulation.pm

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index ab4f7a5..516a111 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -1,6 +1,7 @@
 package C4::Circulation;
 
 # Copyright 2000-2002 Katipo Communications
 package C4::Circulation;
 
 # Copyright 2000-2002 Katipo Communications

+# copyright 2010 BibLibre

 #
 # This file is part of Koha.
 #
 #
 # This file is part of Koha.
 #


@@ -13,13 +14,13 @@ package C4::Circulation;
 # WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
 # A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
 #
 # WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
 # A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
 #

-# You should have received a copy of the GNU General Public License along with
-# Koha; if not, write to the Free Software Foundation, Inc., 59 Temple Place,
-# Suite 330, Boston, MA  02111-1307 USA
+# You should have received a copy of the GNU General Public License along
+# with Koha; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.

 
 
 use strict;
 
 
 use strict;

-#use warnings;  # soon!
+#use warnings; FIXME - Bug 2505

 use C4::Context;
 use C4::Stats;
 use C4::Reserves;
 use C4::Context;
 use C4::Stats;
 use C4::Reserves;


@@ -31,7 +32,9 @@ use C4::Dates;
 use C4::Calendar;
 use C4::Accounts;
 use C4::ItemCirculationAlertPreference;
 use C4::Calendar;
 use C4::Accounts;
 use C4::ItemCirculationAlertPreference;

+use C4::Dates qw(format_date);

 use C4::Message;
 use C4::Message;

+use C4::Debug;

 use Date::Calc qw(
   Today
   Today_and_Now
 use Date::Calc qw(
   Today
   Today_and_Now


@@ -68,7 +71,6 @@ BEGIN {
                &AddRenewal
                &GetRenewCount
                &GetItemIssue
                &AddRenewal
                &GetRenewCount
                &GetItemIssue

-                &GetOpenIssue

                &GetItemIssues
                &GetBorrowerIssues
                &GetIssuingCharges
                &GetItemIssues
                &GetBorrowerIssues
                &GetIssuingCharges


@@ -76,6 +78,7 @@ BEGIN {
         &GetBranchBorrowerCircRule
         &GetBranchItemRule
                &GetBiblioIssues
         &GetBranchBorrowerCircRule
         &GetBranchItemRule
                &GetBiblioIssues

+               &GetOpenIssue

                &AnonymiseIssueHistory
        );
 
                &AnonymiseIssueHistory
        );
 


@@ -116,11 +119,9 @@ Also deals with stocktaking.
 
 =head2 barcodedecode
 
 
 =head2 barcodedecode
 

-=head3 $str = &barcodedecode($barcode, [$filter]);
+  $str = &barcodedecode($barcode, [$filter]);

 
 

-=over 4
-
-=item Generic filter function for barcode string.
+Generic filter function for barcode string.

 Called on every circ if the System Pref itemBarcodeInputFilter is set.
 Will do some manipulation of the barcode for systems that deliver a barcode
 to circulation.pl that differs from the barcode stored for the item.
 Called on every circ if the System Pref itemBarcodeInputFilter is set.
 Will do some manipulation of the barcode for systems that deliver a barcode
 to circulation.pl that differs from the barcode stored for the item.


@@ -131,8 +132,6 @@ The optional $filter argument is to allow for testing or explicit
 behavior that ignores the System Pref.  Valid values are the same as the 
 System Pref options.
 
 behavior that ignores the System Pref.  Valid values are the same as the 
 System Pref options.
 

-=back
-

 =cut
 
 # FIXME -- the &decode fcn below should be wrapped into this one.
 =cut
 
 # FIXME -- the &decode fcn below should be wrapped into this one.


@@ -140,6 +139,7 @@ System Pref options.
 #
 sub barcodedecode {
     my ($barcode, $filter) = @_;
 #
 sub barcodedecode {
     my ($barcode, $filter) = @_;

+    my $branch = C4::Branch::mybranch();

     $filter = C4::Context->preference('itemBarcodeInputFilter') unless $filter;
     $filter or return $barcode;     # ensure filter is defined, else return untouched barcode
        if ($filter eq 'whitespace') {
     $filter = C4::Context->preference('itemBarcodeInputFilter') unless $filter;
     $filter or return $barcode;     # ensure filter is defined, else return untouched barcode
        if ($filter eq 'whitespace') {


@@ -158,24 +158,28 @@ sub barcodedecode {
         # FIXME: $barcode could be "T1", causing warning: substr outside of string
         # Why drop the nonzero digit after the T?
         # Why pass non-digits (or empty string) to "T%07d"?
         # FIXME: $barcode could be "T1", causing warning: substr outside of string
         # Why drop the nonzero digit after the T?
         # Why pass non-digits (or empty string) to "T%07d"?

+       } elsif ($filter eq 'libsuite8') {
+               unless($barcode =~ m/^($branch)-/i){    #if barcode starts with branch code its in Koha style. Skip it.
+                       if($barcode =~ m/^(\d)/i){      #Some barcodes even start with 0's & numbers and are assumed to have b as the item type in the libsuite8 software
+                                $barcode =~ s/^[0]*(\d+)$/$branch-b-$1/i;
+                        }else{
+                               $barcode =~ s/^(\D+)[0]*(\d+)$/$branch-$1-$2/i;
+                       }
+               }

        }
     return $barcode;    # return barcode, modified or not
 }
 
 =head2 decode
 
        }
     return $barcode;    # return barcode, modified or not
 }
 
 =head2 decode
 

-=head3 $str = &decode($chunk);
-
-=over 4
+  $str = &decode($chunk);

 
 

-=item Decodes a segment of a string emitted by a CueCat barcode scanner and
+Decodes a segment of a string emitted by a CueCat barcode scanner and

 returns it.
 
 FIXME: Should be replaced with Barcode::Cuecat from CPAN
 or Javascript based decoding on the client side.
 
 returns it.
 
 FIXME: Should be replaced with Barcode::Cuecat from CPAN
 or Javascript based decoding on the client side.
 

-=back
-

 =cut
 
 sub decode {
 =cut
 
 sub decode {


@@ -207,7 +211,8 @@ sub decode {
 
 =head2 transferbook
 
 
 =head2 transferbook
 

-($dotransfer, $messages, $iteminformation) = &transferbook($newbranch, $barcode, $ignore_reserves);
+  ($dotransfer, $messages, $iteminformation) = &transferbook($newbranch, 
+                                            $barcode, $ignore_reserves);

 
 Transfers an item to a new branch. If the item is currently on loan, it is automatically returned before the actual transfer.
 
 
 Transfers an item to a new branch. If the item is currently on loan, it is automatically returned before the actual transfer.
 


@@ -220,15 +225,17 @@ Otherwise, if an item is reserved, the transfer fails.
 
 Returns three values:
 
 
 Returns three values:
 

-=head3 $dotransfer 
+=over
+
+=item $dotransfer 

 
 is true if the transfer was successful.
 
 
 is true if the transfer was successful.
 

-=head3 $messages
+=item $messages

 
 is a reference-to-hash which may have any of the following keys:
 
 
 is a reference-to-hash which may have any of the following keys:
 

-=over 4
+=over

 
 =item C<BadBarcode>
 
 
 =item C<BadBarcode>
 


@@ -256,6 +263,8 @@ The item was eligible to be transferred. Barring problems communicating with the
 
 =back
 
 
 =back
 

+=back
+

 =cut
 
 sub transferbook {
 =cut
 
 sub transferbook {


@@ -325,8 +334,9 @@ sub transferbook {
 
         # don't need to update MARC anymore, we do it in batch now
         $messages->{'WasTransfered'} = 1;
 
         # don't need to update MARC anymore, we do it in batch now
         $messages->{'WasTransfered'} = 1;

-               ModDateLastSeen( $itemnumber );
+

     }
     }

+    ModDateLastSeen( $itemnumber );

     return ( $dotransfer, $messages, $biblio );
 }
 
     return ( $dotransfer, $messages, $biblio );
 }
 


@@ -339,7 +349,7 @@ sub TooMany {
     my $dbh             = C4::Context->dbh;
        my $branch;
        # Get which branchcode we need
     my $dbh             = C4::Context->dbh;
        my $branch;
        # Get which branchcode we need

-       $branch=_GetCirculationBranch($item,$borrower);
+       $branch = _GetCircControlBranch($item,$borrower);

        my $type = (C4::Context->preference('item-level_itypes')) 
                        ? $item->{'itype'}         # item-level
                        : $item->{'itemtype'};     # biblio-level
        my $type = (C4::Context->preference('item-level_itypes')) 
                        ? $item->{'itype'}         # item-level
                        : $item->{'itemtype'};     # biblio-level


@@ -413,7 +423,7 @@ sub TooMany {
 
         my $max_loans_allowed = $issuing_rule->{'maxissueqty'};
         if ($current_loan_count >= $max_loans_allowed) {
 
         my $max_loans_allowed = $issuing_rule->{'maxissueqty'};
         if ($current_loan_count >= $max_loans_allowed) {

-            return "$current_loan_count / $max_loans_allowed";
+            return ($current_loan_count, $max_loans_allowed);

         }
     }
 
         }
     }
 


@@ -441,7 +451,7 @@ sub TooMany {
 
         my $max_loans_allowed = $branch_borrower_circ_rule->{maxissueqty};
         if ($current_loan_count >= $max_loans_allowed) {
 
         my $max_loans_allowed = $branch_borrower_circ_rule->{maxissueqty};
         if ($current_loan_count >= $max_loans_allowed) {

-            return "$current_loan_count / $max_loans_allowed";
+            return ($current_loan_count, $max_loans_allowed);

         }
     }
 
         }
     }
 


@@ -531,7 +541,6 @@ sub itemissues {
             $data->{'date_due'} = ($data->{'wthdrawn'} eq '1') ? 'Cancelled' : 'Available';
         }
 
             $data->{'date_due'} = ($data->{'wthdrawn'} eq '1') ? 'Cancelled' : 'Available';
         }
 

-        $sth2->finish;

 
         # Find the last 3 people who borrowed this item.
         $sth2 = $dbh->prepare(
 
         # Find the last 3 people who borrowed this item.
         $sth2 = $dbh->prepare(


@@ -551,20 +560,19 @@ sub itemissues {
             }    # if
         }    # for
 
             }    # if
         }    # for
 

-        $sth2->finish;

         $results[$i] = $data;
         $i++;
     }
 
         $results[$i] = $data;
         $i++;
     }
 

-    $sth->finish;

     return (@results);
 }
 
 =head2 CanBookBeIssued
 
     return (@results);
 }
 
 =head2 CanBookBeIssued
 

-Check if a book can be issued.
+  ( $issuingimpossible, $needsconfirmation ) =  CanBookBeIssued( $borrower, 
+                                      $barcode, $duedatespec, $inprocess );

 
 

-( $issuingimpossible, $needsconfirmation ) =  CanBookBeIssued( $borrower, $barcode, $duedatespec, $inprocess );
+Check if a book can be issued.

 
 C<$issuingimpossible> and C<$needsconfirmation> are some hashref.
 
 
 C<$issuingimpossible> and C<$needsconfirmation> are some hashref.
 


@@ -621,7 +629,9 @@ item withdrawn.
 
 item is restricted (set by ??)
 
 
 item is restricted (set by ??)
 

-C<$issuingimpossible> a reference to a hash. It contains reasons why issuing is impossible.
+C<$needsconfirmation> a reference to a hash. It contains reasons why the loan 
+could be prevented, but ones that can be overriden by the operator.
+

 Possible values are :
 
 =head3 DEBT
 Possible values are :
 
 =head3 DEBT


@@ -642,7 +652,7 @@ reserved for someone else.
 
 =head3 INVALID_DATE
 
 
 =head3 INVALID_DATE
 

-sticky due date is invalid
+sticky due date is invalid or due date in the past

 
 =head3 TOO_MANY
 
 
 =head3 TOO_MANY
 


@@ -672,7 +682,7 @@ sub CanBookBeIssued {
     unless ( $duedate ) {
         my $issuedate = strftime( "%Y-%m-%d", localtime );
 
     unless ( $duedate ) {
         my $issuedate = strftime( "%Y-%m-%d", localtime );
 

-        my $branch = _GetCirculationBranch($item,$borrower);
+        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 );
         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 );


@@ -680,7 +690,12 @@ sub CanBookBeIssued {
         # Offline circ calls AddIssue directly, doesn't run through here
         #  So issuingimpossible should be ok.
     }
         # Offline circ calls AddIssue directly, doesn't run through here
         #  So issuingimpossible should be ok.
     }

-    $issuingimpossible{INVALID_DATE} = $duedate->output('syspref') unless ( $duedate && $duedate->output('iso') ge C4::Dates->today('iso') );
+    if ($duedate) {
+        $needsconfirmation{INVALID_DATE} = $duedate->output('syspref')
+          unless $duedate->output('iso') ge C4::Dates->today('iso');
+    } else {
+        $issuingimpossible{INVALID_DATE} = $duedate->output('syspref');
+    }

 
     #
     # BORROWER STATUS
 
     #
     # BORROWER STATUS


@@ -688,6 +703,7 @@ sub CanBookBeIssued {
     if ( $borrower->{'category_type'} eq 'X' && (  $item->{barcode}  )) { 
        # stats only borrower -- add entry to statistics table, and return issuingimpossible{STATS} = 1  .
         &UpdateStats(C4::Context->userenv->{'branch'},'localuse','','',$item->{'itemnumber'},$item->{'itemtype'},$borrower->{'borrowernumber'});
     if ( $borrower->{'category_type'} eq 'X' && (  $item->{barcode}  )) { 
        # stats only borrower -- add entry to statistics table, and return issuingimpossible{STATS} = 1  .
         &UpdateStats(C4::Context->userenv->{'branch'},'localuse','','',$item->{'itemnumber'},$item->{'itemtype'},$borrower->{'borrowernumber'});

+        ModDateLastSeen( $item->{'itemnumber'} );

         return( { STATS => 1 }, {});
     }
     if ( $borrower->{flags}->{GNA} ) {
         return( { STATS => 1 }, {});
     }
     if ( $borrower->{flags}->{GNA} ) {


@@ -715,30 +731,55 @@ sub CanBookBeIssued {
     # DEBTS
     my ($amount) =
       C4::Members::GetMemberAccountRecords( $borrower->{'borrowernumber'}, '' && $duedate->output('iso') );
     # DEBTS
     my ($amount) =
       C4::Members::GetMemberAccountRecords( $borrower->{'borrowernumber'}, '' && $duedate->output('iso') );

+    my $amountlimit = C4::Context->preference("noissuescharge");
+    my $allowfineoverride = C4::Context->preference("AllowFineOverride");
+    my $allfinesneedoverride = C4::Context->preference("AllFinesNeedOverride");

     if ( C4::Context->preference("IssuingInProcess") ) {
     if ( C4::Context->preference("IssuingInProcess") ) {

-        my $amountlimit = C4::Context->preference("noissuescharge");
-        if ( $amount > $amountlimit && !$inprocess ) {
+        if ( $amount > $amountlimit && !$inprocess && !$allowfineoverride) {

             $issuingimpossible{DEBT} = sprintf( "%.2f", $amount );
             $issuingimpossible{DEBT} = sprintf( "%.2f", $amount );

-        }
-        elsif ( $amount > 0 && $amount <= $amountlimit && !$inprocess ) {
+        } elsif ( $amount > $amountlimit && !$inprocess && $allowfineoverride) {
+            $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
+        } elsif ( $allfinesneedoverride && $amount > 0 && $amount <= $amountlimit && !$inprocess ) {

             $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
         }
     }
     else {
             $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
         }
     }
     else {

-        if ( $amount > 0 ) {
+        if ( $amount > $amountlimit && $allowfineoverride ) {
+            $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
+        } elsif ( $amount > $amountlimit && !$allowfineoverride) {
+            $issuingimpossible{DEBT} = sprintf( "%.2f", $amount );
+        } elsif ( $amount > 0 && $allfinesneedoverride ) {

             $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
         }
     }
 
             $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
         }
     }
 

-    #
+    my ($blocktype, $count) = C4::Members::IsMemberBlocked($borrower->{'borrowernumber'});
+    if ($blocktype == -1) {
+        ## patron has outstanding overdue loans
+           if ( C4::Context->preference("OverduesBlockCirc") eq 'block'){
+               $issuingimpossible{USERBLOCKEDOVERDUE} = $count;
+           }
+           elsif ( C4::Context->preference("OverduesBlockCirc") eq 'confirmation'){
+               $needsconfirmation{USERBLOCKEDOVERDUE} = $count;
+           }
+    } elsif($blocktype == 1) {
+        # patron has accrued fine days
+        $issuingimpossible{USERBLOCKEDREMAINING} = $count;
+    }
+
+#

     # JB34 CHECKS IF BORROWERS DONT HAVE ISSUE TOO MANY BOOKS
     #
     # JB34 CHECKS IF BORROWERS DONT HAVE ISSUE TOO MANY BOOKS
     #

-       my $toomany = TooMany( $borrower, $item->{biblionumber}, $item );
-    # if TooMany return / 0, then the user has no permission to check out this book
-    if ($toomany =~ /\/ 0/) {
+       my ($current_loan_count, $max_loans_allowed) = TooMany( $borrower, $item->{biblionumber}, $item );
+    # if TooMany max_loans_allowed returns 0 the user doesn't have permission to check out this book
+    if ($max_loans_allowed eq 0) {

         $needsconfirmation{PATRON_CANT} = 1;
     } else {
         $needsconfirmation{PATRON_CANT} = 1;
     } else {

-        $needsconfirmation{TOO_MANY} = $toomany if $toomany;
+        if($max_loans_allowed){
+            $needsconfirmation{TOO_MANY} = 1;
+            $needsconfirmation{current_loan_count} = $current_loan_count;
+            $needsconfirmation{max_loans_allowed} = $max_loans_allowed;
+        }

     }
 
     #
     }
 
     #


@@ -789,8 +830,10 @@ sub CanBookBeIssued {
     if ( C4::Context->preference("IndependantBranches") ) {
         my $userenv = C4::Context->userenv;
         if ( ($userenv) && ( $userenv->{flags} % 2 != 1 ) ) {
     if ( C4::Context->preference("IndependantBranches") ) {
         my $userenv = C4::Context->userenv;
         if ( ($userenv) && ( $userenv->{flags} % 2 != 1 ) ) {

-            $issuingimpossible{NOTSAMEBRANCH} = 1
+            $issuingimpossible{ITEMNOTSAMEBRANCH} = 1

               if ( $item->{C4::Context->preference("HomeOrHoldingBranch")} ne $userenv->{branch} );
               if ( $item->{C4::Context->preference("HomeOrHoldingBranch")} ne $userenv->{branch} );

+            $needsconfirmation{BORRNOTSAMEBRANCH} = GetBranchName( $borrower->{'branchcode'} )
+              if ( $borrower->{'branchcode'} ne $userenv->{branch} );

         }
     }
 
         }
     }
 


@@ -819,8 +862,11 @@ sub CanBookBeIssued {
         my $currborinfo =    C4::Members::GetMemberDetails( $issue->{borrowernumber} );
 
 #        warn "=>.$currborinfo->{'firstname'} $currborinfo->{'surname'} ($currborinfo->{'cardnumber'})";
         my $currborinfo =    C4::Members::GetMemberDetails( $issue->{borrowernumber} );
 
 #        warn "=>.$currborinfo->{'firstname'} $currborinfo->{'surname'} ($currborinfo->{'cardnumber'})";

-        $needsconfirmation{ISSUED_TO_ANOTHER} =
-"$currborinfo->{'reservedate'} : $currborinfo->{'firstname'} $currborinfo->{'surname'} ($currborinfo->{'cardnumber'})";
+        $needsconfirmation{ISSUED_TO_ANOTHER} = 1;
+        $needsconfirmation{issued_firstname} = $currborinfo->{'firstname'};
+        $needsconfirmation{issued_surname} = $currborinfo->{'surname'};
+        $needsconfirmation{issued_cardnumber} = $currborinfo->{'cardnumber'};
+        $needsconfirmation{issued_borrowernumber} = $currborinfo->{'borrowernumber'};

     }
 
     # See if the item is on reserve.
     }
 
     # See if the item is on reserve.


@@ -834,13 +880,23 @@ sub CanBookBeIssued {
         {
             # The item is on reserve and waiting, but has been
             # reserved by some other patron.
         {
             # The item is on reserve and waiting, but has been
             # reserved by some other patron.

-            $needsconfirmation{RESERVE_WAITING} =
-"$resborrower->{'firstname'} $resborrower->{'surname'} ($resborrower->{'cardnumber'}, $branchname)";
+            $needsconfirmation{RESERVE_WAITING} = 1;
+            $needsconfirmation{'resfirstname'} = $resborrower->{'firstname'};
+            $needsconfirmation{'ressurname'} = $resborrower->{'surname'};
+            $needsconfirmation{'rescardnumber'} = $resborrower->{'cardnumber'};
+            $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'};
+            $needsconfirmation{'resbranchname'} = $branchname;
+            $needsconfirmation{'reswaitingdate'} = format_date($res->{'waitingdate'});

         }
         elsif ( $restype eq "Reserved" ) {
             # The item is on reserve for someone else.
         }
         elsif ( $restype eq "Reserved" ) {
             # The item is on reserve for someone else.

-            $needsconfirmation{RESERVED} =
-"$res->{'reservedate'} : $resborrower->{'firstname'} $resborrower->{'surname'} ($resborrower->{'cardnumber'})";
+            $needsconfirmation{RESERVED} = 1;
+            $needsconfirmation{'resfirstname'} = $resborrower->{'firstname'};
+            $needsconfirmation{'ressurname'} = $resborrower->{'surname'};
+            $needsconfirmation{'rescardnumber'} = $resborrower->{'cardnumber'};
+            $needsconfirmation{'resborrowernumber'} = $resborrower->{'borrowernumber'};
+            $needsconfirmation{'resbranchname'} = $branchname;
+            $needsconfirmation{'resreservedate'} = format_date($res->{'reservedate'});

         }
     }
        return ( \%issuingimpossible, \%needsconfirmation );
         }
     }
        return ( \%issuingimpossible, \%needsconfirmation );


@@ -848,9 +904,9 @@ sub CanBookBeIssued {
 
 =head2 AddIssue
 
 
 =head2 AddIssue
 

-Issue a book. Does no check, they are done in CanBookBeIssued. If we reach this sub, it means the user confirmed if needed.
+  &AddIssue($borrower, $barcode, [$datedue], [$cancelreserve], [$issuedate])

 
 

-&AddIssue($borrower, $barcode, [$datedue], [$cancelreserve], [$issuedate])
+Issue a book. Does no check, they are done in CanBookBeIssued. If we reach this sub, it means the user confirmed if needed.

 
 =over 4
 
 
 =over 4
 


@@ -888,7 +944,6 @@ sub AddIssue {
     my ( $borrower, $barcode, $datedue, $cancelreserve, $issuedate, $sipmode) = @_;
     my $dbh = C4::Context->dbh;
        my $barcodecheck=CheckValidBarcode($barcode);
     my ( $borrower, $barcode, $datedue, $cancelreserve, $issuedate, $sipmode) = @_;
     my $dbh = C4::Context->dbh;
        my $barcodecheck=CheckValidBarcode($barcode);

-

     # $issuedate defaults to today.
     if ( ! defined $issuedate ) {
         $issuedate = strftime( "%Y-%m-%d", localtime );
     # $issuedate defaults to today.
     if ( ! defined $issuedate ) {
         $issuedate = strftime( "%Y-%m-%d", localtime );


@@ -898,7 +953,7 @@ 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.
        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 = _GetCirculationBranch($item,$borrower);
+               my $branch = _GetCircControlBranch($item,$borrower);

                
                # get actual issuing if there is one
                my $actualissue = GetItemIssue( $item->{itemnumber});
                
                # get actual issuing if there is one
                my $actualissue = GetItemIssue( $item->{itemnumber});


@@ -999,6 +1054,9 @@ sub AddIssue {
             C4::Context->userenv->{'branch'}    # branchcode
         );
         $sth->finish;
             C4::Context->userenv->{'branch'}    # branchcode
         );
         $sth->finish;

+        if ( C4::Context->preference('ReturnToShelvingCart') ) { ## ReturnToShelvingCart is on, anything issued should be taken off the cart.
+          CartToShelf( $item->{'itemnumber'} );
+        }

         $item->{'issues'}++;
         ModItem({ issues           => $item->{'issues'},
                   holdingbranch    => C4::Context->userenv->{'branch'},
         $item->{'issues'}++;
         ModItem({ issues           => $item->{'issues'},
                   holdingbranch    => C4::Context->userenv->{'branch'},


@@ -1055,9 +1113,9 @@ sub AddIssue {
 
 =head2 GetLoanLength
 
 
 =head2 GetLoanLength
 

-Get loan length for an itemtype, a borrower type and a branch
+  my $loanlength = &GetLoanLength($borrowertype,$itemtype,branchcode)

 
 

-my $loanlength = &GetLoanLength($borrowertype,$itemtype,branchcode)
+Get loan length for an itemtype, a borrower type and a branch

 
 =cut
 
 
 =cut
 


@@ -1117,6 +1175,8 @@ sub GetLoanLength {
 
 =head2 GetIssuingRule
 
 
 =head2 GetIssuingRule
 

+  my $irule = &GetIssuingRule($borrowertype,$itemtype,branchcode)
+

 FIXME - This is a copy-paste of GetLoanLength
 as a stop-gap.  Do not wish to change API for GetLoanLength 
 this close to release, however, Overdues::GetIssuingRules is broken.
 FIXME - This is a copy-paste of GetLoanLength
 as a stop-gap.  Do not wish to change API for GetLoanLength 
 this close to release, however, Overdues::GetIssuingRules is broken.


@@ -1124,8 +1184,6 @@ this close to release, however, Overdues::GetIssuingRules is broken.
 Get the issuing rule for an itemtype, a borrower type and a branch
 Returns a hashref from the issuingrules table.
 
 Get the issuing rule for an itemtype, a borrower type and a branch
 Returns a hashref from the issuingrules table.
 

-my $irule = &GetIssuingRule($borrowertype,$itemtype,branchcode)
-

 =cut
 
 sub GetIssuingRule {
 =cut
 
 sub GetIssuingRule {


@@ -1172,11 +1230,7 @@ sub GetIssuingRule {
 
 =head2 GetBranchBorrowerCircRule
 
 
 =head2 GetBranchBorrowerCircRule
 

-=over 4
-
-my $branch_cat_rule = GetBranchBorrowerCircRule($branchcode, $categorycode);
-
-=back
+  my $branch_cat_rule = GetBranchBorrowerCircRule($branchcode, $categorycode);

 
 Retrieves circulation rule attributes that apply to the given
 branch and patron category, regardless of item type.  
 
 Retrieves circulation rule attributes that apply to the given
 branch and patron category, regardless of item type.  


@@ -1259,11 +1313,7 @@ sub GetBranchBorrowerCircRule {
 
 =head2 GetBranchItemRule
 
 
 =head2 GetBranchItemRule
 

-=over 4
-
-my $branch_item_rule = GetBranchItemRule($branchcode, $itemtype);
-
-=back
+  my $branch_item_rule = GetBranchItemRule($branchcode, $itemtype);

 
 Retrieves circulation rule attributes that apply to the given
 branch and item type, regardless of patron category.
 
 Retrieves circulation rule attributes that apply to the given
 branch and item type, regardless of patron category.


@@ -1323,8 +1373,8 @@ sub GetBranchItemRule {
 
 =head2 AddReturn
 
 
 =head2 AddReturn
 

-($doreturn, $messages, $iteminformation, $borrower) =
-    &AddReturn($barcode, $branch, $exemptfine, $dropbox);
+  ($doreturn, $messages, $iteminformation, $borrower) =
+      &AddReturn($barcode, $branch, $exemptfine, $dropbox);

 
 Returns a book.
 
 
 Returns a book.
 


@@ -1429,7 +1479,8 @@ sub AddReturn {
     my $item = GetItem($itemnumber) or die "GetItem($itemnumber) failed";
         # full item data, but no borrowernumber or checkout info (no issue)
         # we know GetItem should work because GetItemnumberFromBarcode worked
     my $item = GetItem($itemnumber) or die "GetItem($itemnumber) failed";
         # full item data, but no borrowernumber or checkout info (no issue)
         # we know GetItem should work because GetItemnumberFromBarcode worked

-    my $hbr = $item->{C4::Context->preference("HomeOrHoldingBranch")} || '';
+    my $hbr      = C4::Context->preference("HomeOrHoldingBranchReturn") || "homebranch";
+    $hbr = $item->{$hbr} || '';

         # item must be from items table -- issues table has branchcode and issuingbranch, not homebranch nor holdingbranch
 
     my $borrowernumber = $borrower->{'borrowernumber'} || undef;    # we don't know if we had a borrower or not
         # item must be from items table -- issues table has branchcode and issuingbranch, not homebranch nor holdingbranch
 
     my $borrowernumber = $borrower->{'borrowernumber'} || undef;    # we don't know if we had a borrower or not


@@ -1447,7 +1498,13 @@ sub AddReturn {
             Wrongbranch => $branch,
             Rightbranch => $hbr,
         };
             Wrongbranch => $branch,
             Rightbranch => $hbr,
         };

-        $doreturn = 0;  # Could we bail here?
+        $doreturn = 0;
+        # bailing out here - in this case, current desired behavior
+        # is to act as if no return ever happened at all.
+        # FIXME - even in an indy branches situation, there should
+        # still be an option for the library to accept the item
+        # and transfer it to its owning library.
+        return ( $doreturn, $messages, $issue, $borrower );

     }
 
     if ( $item->{'wthdrawn'} ) { # book has been cancelled
     }
 
     if ( $item->{'wthdrawn'} ) { # book has been cancelled


@@ -1458,28 +1515,30 @@ sub AddReturn {
     # case of a return of document (deal with issues and holdingbranch)
     if ($doreturn) {
         $borrower or warn "AddReturn without current borrower";
     # case of a return of document (deal with issues and holdingbranch)
     if ($doreturn) {
         $borrower or warn "AddReturn without current borrower";

-               my $circControlBranch = _GetCirculationBranch($item,$borrower);
+               my $circControlBranch;

         if ($dropbox) {
         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 ( $item->{'issuedate'} eq C4::Dates->today('iso') );
+            # define circControlBranch only if dropbox mode is set
+            # don't allow dropbox mode to create an invalid entry in issues (issuedate > today)
+            # FIXME: check issuedate > returndate, factoring in holidays
+            $circControlBranch = _GetCircControlBranch($item,$borrower) unless ( $item->{'issuedate'} eq C4::Dates->today('iso') );;

         }
 
         if ($borrowernumber) {
         }
 
         if ($borrowernumber) {

-            MarkIssueReturned($borrowernumber, $item->{'itemnumber'}, $circControlBranch);
+            MarkIssueReturned($borrowernumber, $item->{'itemnumber'}, $circControlBranch, '', $borrower->{'privacy'});

             $messages->{'WasReturned'} = 1;    # FIXME is the "= 1" right?  This could be the borrower hash.
         }
 
             $messages->{'WasReturned'} = 1;    # FIXME is the "= 1" right?  This could be the borrower hash.
         }
 

-            
-            
-        # We update the holdingbranch from circControlBranch variable
-        UpdateHoldingbranch($circControlBranch,$item->{'itemnumber'});
-        $item->{'holdingbranch'} = $circControlBranch;
-        
-               
-        ModDateLastSeen( $item->{'itemnumber'} );

         ModItem({ onloan => undef }, $issue->{'biblionumber'}, $item->{'itemnumber'});
     }
 
         ModItem({ onloan => undef }, $issue->{'biblionumber'}, $item->{'itemnumber'});
     }
 

+    # the holdingbranch is updated if the document is returned to another location.
+    # this is always done regardless of whether the item was on loan or not
+    if ($item->{'holdingbranch'} ne $branch) {
+        UpdateHoldingbranch($branch, $item->{'itemnumber'});
+        $item->{'holdingbranch'} = $branch; # update item data holdingbranch too
+    }
+    ModDateLastSeen( $item->{'itemnumber'} );
+

     # check if we have a transfer for this document
     my ($datesent,$frombranch,$tobranch) = GetTransfers( $item->{'itemnumber'} );
 
     # check if we have a transfer for this document
     my ($datesent,$frombranch,$tobranch) = GetTransfers( $item->{'itemnumber'} );
 


@@ -1552,14 +1611,14 @@ 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 .
 
     #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 $item->{$hbr}) and not $messages->{'WrongTransfer'} and ($validTransfert ne 1) ){
+    if ($doreturn and ($branch ne $hbr) and not $messages->{'WrongTransfer'} and ($validTransfert ne 1) ){

         if ( C4::Context->preference("AutomaticItemReturn"    ) or
             (C4::Context->preference("UseBranchTransferLimits") and
         if ( C4::Context->preference("AutomaticItemReturn"    ) or
             (C4::Context->preference("UseBranchTransferLimits") and

-             ! IsBranchTransferAllowed($branch, $item->{$hbr}, $item->{C4::Context->preference("BranchTransferLimitsType")} )
+             ! IsBranchTransferAllowed($branch, $hbr, $item->{C4::Context->preference("BranchTransferLimitsType")} )

            )) {
            )) {

-            warn sprintf "about to call ModItemTransfer(%s, %s, %s)", $item->{'itemnumber'},$branch, $item->{$hbr};
-            warn "item: " . Dumper($item);
-            ModItemTransfer($item->{'itemnumber'}, $branch, $item->{$hbr});
+            $debug and warn sprintf "about to call ModItemTransfer(%s, %s, %s)", $item->{'itemnumber'},$branch, $hbr;
+            $debug and warn "item: " . Dumper($item);
+            ModItemTransfer($item->{'itemnumber'}, $branch, $hbr);

             $messages->{'WasTransfered'} = 1;
         } else {
             $messages->{'NeedsTransfer'} = 1;   # TODO: instead of 1, specify branchcode that the transfer SHOULD go to, $item->{homebranch}
             $messages->{'WasTransfered'} = 1;
         } else {
             $messages->{'NeedsTransfer'} = 1;   # TODO: instead of 1, specify branchcode that the transfer SHOULD go to, $item->{homebranch}


@@ -1570,11 +1629,7 @@ sub AddReturn {
 
 =head2 MarkIssueReturned
 
 
 =head2 MarkIssueReturned
 

-=over 4
-
-MarkIssueReturned($borrowernumber, $itemnumber, $dropbox_branch, $returndate);
-
-=back
+  MarkIssueReturned($borrowernumber, $itemnumber, $dropbox_branch, $returndate, $privacy);

 
 Unconditionally marks an issue as being returned by
 moving the C<issues> row to C<old_issues> and
 
 Unconditionally marks an issue as being returned by
 moving the C<issues> row to C<old_issues> and


@@ -1586,6 +1641,9 @@ it's safe to do this, i.e. last non-holiday > issuedate.
 if C<$returndate> is specified (in iso format), it is used as the date
 of the return. It is ignored when a dropbox_branch is passed in.
 
 if C<$returndate> is specified (in iso format), it is used as the date
 of the return. It is ignored when a dropbox_branch is passed in.
 

+C<$privacy> contains the privacy parameter. If the patron has set privacy to 2,
+the old_issue is immediately anonymised
+

 Ideally, this function would be internal to C<C4::Circulation>,
 not exported, but it is currently needed by one 
 routine in C<C4::Accounts>.
 Ideally, this function would be internal to C<C4::Circulation>,
 not exported, but it is currently needed by one 
 routine in C<C4::Accounts>.


@@ -1593,7 +1651,7 @@ routine in C<C4::Accounts>.
 =cut
 
 sub MarkIssueReturned {
 =cut
 
 sub MarkIssueReturned {

-    my ( $borrowernumber, $itemnumber, $dropbox_branch, $returndate ) = @_;
+    my ( $borrowernumber, $itemnumber, $dropbox_branch, $returndate, $privacy ) = @_;

     my $dbh   = C4::Context->dbh;
     my $query = "UPDATE issues SET returndate=";
     my @bind;
     my $dbh   = C4::Context->dbh;
     my $query = "UPDATE issues SET returndate=";
     my @bind;


@@ -1617,6 +1675,16 @@ sub MarkIssueReturned {
                                   WHERE borrowernumber = ?
                                   AND itemnumber = ?");
     $sth_copy->execute($borrowernumber, $itemnumber);
                                   WHERE borrowernumber = ?
                                   AND itemnumber = ?");
     $sth_copy->execute($borrowernumber, $itemnumber);

+    # anonymise patron checkout immediately if $privacy set to 2 and AnonymousPatron is set to a valid borrowernumber
+    if ( $privacy == 2) {
+        # The default of 0 does not work due to foreign key constraints
+        # The anonymisation will fail quietly if AnonymousPatron is not a valid entry
+        my $anonymouspatron = (C4::Context->preference('AnonymousPatron')) ? C4::Context->preference('AnonymousPatron') : 0;
+        my $sth_ano = $dbh->prepare("UPDATE old_issues SET borrowernumber=?
+                                  WHERE borrowernumber = ?
+                                  AND itemnumber = ?");
+       $sth_ano->execute($anonymouspatron, $borrowernumber, $itemnumber);
+    }

     my $sth_del  = $dbh->prepare("DELETE FROM issues
                                   WHERE borrowernumber = ?
                                   AND itemnumber = ?");
     my $sth_del  = $dbh->prepare("DELETE FROM issues
                                   WHERE borrowernumber = ?
                                   AND itemnumber = ?");


@@ -1625,7 +1693,7 @@ sub MarkIssueReturned {
 
 =head2 _FixOverduesOnReturn
 
 
 =head2 _FixOverduesOnReturn
 

-    &_FixOverduesOnReturn($brn,$itm, $exemptfine, $dropboxmode);
+   &_FixOverduesOnReturn($brn,$itm, $exemptfine, $dropboxmode);

 
 C<$brn> borrowernumber
 
 
 C<$brn> borrowernumber
 


@@ -1689,7 +1757,7 @@ sub _FixOverduesOnReturn {
 
 =head2 _FixAccountForLostAndReturned
 
 
 =head2 _FixAccountForLostAndReturned
 

-       &_FixAccountForLostAndReturned($itemnumber, [$borrowernumber, $barcode]);
+  &_FixAccountForLostAndReturned($itemnumber, [$borrowernumber, $barcode]);

 
 Calculates the charge for a book lost and returned.
 
 
 Calculates the charge for a book lost and returned.
 


@@ -1776,37 +1844,41 @@ sub _FixAccountForLostAndReturned {
     return;
 }
 
     return;
 }
 

-=head2 _GetCirculationBranch
+=head2 _GetCircControlBranch

 
 

-   _GetCirculationBranch($iteminfos,$borrower);
+   my $circ_control_branch = _GetCircControlBranch($iteminfos, $borrower);

 
 Internal function : 
 
 Internal function : 

-Return the branchcode that must be used for an item circulation
+
+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 {homebranch or holdingbranch} is used.
 
 C<$borrower> is a hashref to borrower. Only {branchcode} is used.
 
 
 C<$iteminfos> is a hashref to iteminfo. Only {homebranch or holdingbranch} is used.
 
 C<$borrower> is a hashref to borrower. Only {branchcode} is used.
 

-Internal function, called by AddReturn
-

 =cut
 
 =cut
 

-sub _GetCirculationBranch {
-    my ($iteminfos, $borrower) = @_;
+sub _GetCircControlBranch {
+    my ($item, $borrower) = @_;

     my $circcontrol = C4::Context->preference('CircControl');
     my $circcontrol = C4::Context->preference('CircControl');

-       my $branch;
+    my $branch;

 
 

-    if ($circcontrol eq 'PickupLibrary'){
-       $branch= C4::Context->userenv->{'branch'};
-    }
-       elsif($circcontrol eq 'PatronLibrary'){
+    if ($circcontrol eq 'PickupLibrary') {
+        $branch= C4::Context->userenv->{'branch'};
+    } elsif ($circcontrol eq 'PatronLibrary') {

         $branch=$borrower->{branchcode};
         $branch=$borrower->{branchcode};

+    } else {
+        my $branchfield = C4::Context->preference('HomeOrHoldingBranch') || 'homebranch';
+        $branch = $item->{$branchfield};
+        # default to item home branch if holdingbranch is used
+        # and is not defined
+        if (!defined($branch) && $branchfield eq 'holdingbranch') {
+            $branch = $item->{homebranch};
+        }

     }
     }

-       else {
-               my $branchfield=C4::Context->preference("HomeOrHoldingBranch")||"homebranch";
-        $branch= $iteminfos->{$branchfield};
-       }
-       return $branch;
+    return $branch;

 }
 
 
 }
 
 


@@ -1816,7 +1888,7 @@ sub _GetCirculationBranch {
 
 =head2 GetItemIssue
 
 
 =head2 GetItemIssue
 

-$issue = &GetItemIssue($itemnumber);
+  $issue = &GetItemIssue($itemnumber);

 
 Returns patron currently having a book, or undef if not checked out.
 
 
 Returns patron currently having a book, or undef if not checked out.
 


@@ -1843,7 +1915,7 @@ sub GetItemIssue {
 
 =head2 GetOpenIssue
 
 
 =head2 GetOpenIssue
 

-$issue = GetOpenIssue( $itemnumber );
+  $issue = GetOpenIssue( $itemnumber );

 
 Returns the row from the issues table if the item is currently issued, undef if the item is not currently issued
 
 
 Returns the row from the issues table if the item is currently issued, undef if the item is not currently issued
 


@@ -1865,7 +1937,7 @@ sub GetOpenIssue {
 
 =head2 GetItemIssues
 
 
 =head2 GetItemIssues
 

-$issues = &GetItemIssues($itemnumber, $history);
+  $issues = &GetItemIssues($itemnumber, $history);

 
 Returns patrons that have issued a book
 
 
 Returns patrons that have issued a book
 


@@ -1908,7 +1980,7 @@ sub GetItemIssues {
 
 =head2 GetBiblioIssues
 
 
 =head2 GetBiblioIssues
 

-$issues = GetBiblioIssues($biblionumber);
+  $issues = GetBiblioIssues($biblionumber);

 
 this function get all issues from a biblionumber.
 
 
 this function get all issues from a biblionumber.
 


@@ -1952,11 +2024,7 @@ sub GetBiblioIssues {
 
 =head2 GetUpcomingDueIssues
 
 
 =head2 GetUpcomingDueIssues
 

-=over 4
- 
-my $upcoming_dues = GetUpcomingDueIssues( { days_in_advance => 4 } );
-
-=back
+  my $upcoming_dues = GetUpcomingDueIssues( { days_in_advance => 4 } );

 
 =cut
 
 
 =cut
 


@@ -1986,7 +2054,7 @@ END_SQL
 
 =head2 CanBookBeRenewed
 
 
 =head2 CanBookBeRenewed
 

-($ok,$error) = &CanBookBeRenewed($borrowernumber, $itemnumber[, $override_limit]);
+  ($ok,$error) = &CanBookBeRenewed($borrowernumber, $itemnumber[, $override_limit]);

 
 Find out whether a borrowed item may be renewed.
 
 
 Find out whether a borrowed item may be renewed.
 


@@ -2020,39 +2088,52 @@ sub CanBookBeRenewed {
     # Look in the issues table for this item, lent to this borrower,
     # and not yet returned.
 
     # Look in the issues table for this item, lent to this borrower,
     # and not yet returned.
 

-    # FIXME - I think this function could be redone to use only one SQL call.
-    my $sth1 = $dbh->prepare(
-        "SELECT * FROM issues
-            WHERE borrowernumber = ?
-            AND itemnumber = ?"
-    );
-    $sth1->execute( $borrowernumber, $itemnumber );
-    if ( my $data1 = $sth1->fetchrow_hashref ) {
-
-        # Found a matching item
-
-        # See if this item may be renewed. This query is convoluted
-        # because it's a bit messy: given the item number, we need to find
-        # the biblioitem, which gives us the itemtype, which tells us
-        # whether it may be renewed.
-        my $query = "SELECT renewalsallowed FROM items ";
-        $query .= (C4::Context->preference('item-level_itypes'))
-                    ? "LEFT JOIN itemtypes ON items.itype = itemtypes.itemtype "
-                    : "LEFT JOIN biblioitems on items.biblioitemnumber = biblioitems.biblioitemnumber
-                       LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype ";
-        $query .= "WHERE items.itemnumber = ?";
-        my $sth2 = $dbh->prepare($query);
-        $sth2->execute($itemnumber);
-        if ( my $data2 = $sth2->fetchrow_hashref ) {
-            $renews = $data2->{'renewalsallowed'};
-        }
-        if ( ( $renews && $renews > $data1->{'renewals'} ) || $override_limit ) {
+    # Look in the issues table for this item, lent to this borrower,
+    # and not yet returned.
+    my %branch = (
+            'ItemHomeLibrary' => 'items.homebranch',
+            'PickupLibrary'   => 'items.holdingbranch',
+            'PatronLibrary'   => 'borrowers.branchcode'
+            );
+    my $controlbranch = $branch{C4::Context->preference('CircControl')};
+    my $itype         = C4::Context->preference('item-level_itypes') ? 'items.itype' : 'biblioitems.itemtype';
+    
+    my $sthcount = $dbh->prepare("
+                   SELECT 
+                    borrowers.categorycode, biblioitems.itemtype, issues.renewals, renewalsallowed, $controlbranch
+                   FROM  issuingrules, 
+                   issues 
+                   LEFT JOIN items USING (itemnumber) 
+                   LEFT JOIN borrowers USING (borrowernumber) 
+                   LEFT JOIN biblioitems USING (biblioitemnumber)
+                   
+                   WHERE
+                    (issuingrules.categorycode = borrowers.categorycode OR issuingrules.categorycode = '*')
+                   AND
+                    (issuingrules.itemtype = $itype OR issuingrules.itemtype = '*')
+                   AND
+                    (issuingrules.branchcode = $controlbranch OR issuingrules.branchcode = '*') 
+                   AND 
+                    borrowernumber = ? 
+                   AND
+                    itemnumber = ?
+                   ORDER BY
+                    issuingrules.categorycode desc,
+                    issuingrules.itemtype desc,
+                    issuingrules.branchcode desc
+                   LIMIT 1;
+                  ");
+
+    $sthcount->execute( $borrowernumber, $itemnumber );
+    if ( my $data1 = $sthcount->fetchrow_hashref ) {
+        
+        if ( ( $data1->{renewalsallowed} && $data1->{renewalsallowed} > $data1->{renewals} ) || $override_limit ) {

             $renewokay = 1;
         }
         else {
                        $error="too_many";
                }
             $renewokay = 1;
         }
         else {
                        $error="too_many";
                }

-        $sth2->finish;
+               

         my ( $resfound, $resrec ) = C4::Reserves::CheckReserves($itemnumber);
         if ($resfound) {
             $renewokay = 0;
         my ( $resfound, $resrec ) = C4::Reserves::CheckReserves($itemnumber);
         if ($resfound) {
             $renewokay = 0;


@@ -2060,13 +2141,12 @@ sub CanBookBeRenewed {
         }
 
     }
         }
 
     }

-    $sth1->finish;

     return ($renewokay,$error);
 }
 
 =head2 AddRenewal
 
     return ($renewokay,$error);
 }
 
 =head2 AddRenewal
 

-&AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate]);
+  &AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate]);

 
 Renews a loan.
 
 
 Renews a loan.
 


@@ -2168,39 +2248,42 @@ sub AddRenewal {
 
 sub GetRenewCount {
     # check renewal status
 
 sub GetRenewCount {
     # check renewal status

-    my ($bornum,$itemno)=@_;
-    my $dbh = C4::Context->dbh;
-    my $renewcount = 0;
-        my $renewsallowed = 0;
-        my $renewsleft = 0;
+    my ( $bornum, $itemno ) = @_;
+    my $dbh           = C4::Context->dbh;
+    my $renewcount    = 0;
+    my $renewsallowed = 0;
+    my $renewsleft    = 0;
+
+    my $borrower = C4::Members::GetMemberDetails($bornum);
+    my $item     = GetItem($itemno); 
+

     # Look in the issues table for this item, lent to this borrower,
     # and not yet returned.
 
     # FIXME - I think this function could be redone to use only one SQL call.
     # Look in the issues table for this item, lent to this borrower,
     # and not yet returned.
 
     # FIXME - I think this function could be redone to use only one SQL call.

-    my $sth = $dbh->prepare("select * from issues
+    my $sth = $dbh->prepare(
+        "select * from issues

                                 where (borrowernumber = ?)
                                 where (borrowernumber = ?)

-                                and (itemnumber = ?)");
-    $sth->execute($bornum,$itemno);
+                                and (itemnumber = ?)"
+    );
+    $sth->execute( $bornum, $itemno );

     my $data = $sth->fetchrow_hashref;
     $renewcount = $data->{'renewals'} if $data->{'renewals'};
     $sth->finish;
     my $data = $sth->fetchrow_hashref;
     $renewcount = $data->{'renewals'} if $data->{'renewals'};
     $sth->finish;

-    my $query = "SELECT renewalsallowed FROM items ";
-    $query .= (C4::Context->preference('item-level_itypes'))
-                ? "LEFT JOIN itemtypes ON items.itype = itemtypes.itemtype "
-                : "LEFT JOIN biblioitems on items.biblioitemnumber = biblioitems.biblioitemnumber
-                   LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype ";
-    $query .= "WHERE items.itemnumber = ?";
-    my $sth2 = $dbh->prepare($query);
-    $sth2->execute($itemno);
-    my $data2 = $sth2->fetchrow_hashref();
-    $renewsallowed = $data2->{'renewalsallowed'};
-    $renewsleft = $renewsallowed - $renewcount;
-    return ($renewcount,$renewsallowed,$renewsleft);
+    # $item and $borrower should be calculated
+    my $branchcode = _GetCircControlBranch($item, $borrower);
+    
+    my $issuingrule = GetIssuingRule($borrower->{categorycode}, $item->{itype}, $branchcode);
+    
+    $renewsallowed = $issuingrule->{'renewalsallowed'};
+    $renewsleft    = $renewsallowed - $renewcount;
+    if($renewsleft < 0){ $renewsleft = 0; }
+    return ( $renewcount, $renewsallowed, $renewsleft );

 }
 
 =head2 GetIssuingCharges
 
 }
 
 =head2 GetIssuingCharges
 

-($charge, $item_type) = &GetIssuingCharges($itemnumber, $borrowernumber);
+  ($charge, $item_type) = &GetIssuingCharges($itemnumber, $borrowernumber);

 
 Calculate how much it would cost for a given patron to borrow a given
 item, including any applicable discounts.
 
 Calculate how much it would cost for a given patron to borrow a given
 item, including any applicable discounts.


@@ -2240,7 +2323,7 @@ sub GetIssuingCharges {
         my $q2 = "SELECT rentaldiscount FROM borrowers
             LEFT JOIN issuingrules ON borrowers.categorycode = issuingrules.categorycode
             WHERE borrowers.borrowernumber = ?
         my $q2 = "SELECT rentaldiscount FROM borrowers
             LEFT JOIN issuingrules ON borrowers.categorycode = issuingrules.categorycode
             WHERE borrowers.borrowernumber = ?

-            AND issuingrules.itemtype = ?";
+            AND (issuingrules.itemtype = ? OR issuingrules.itemtype = '*')";

         my $sth2 = $dbh->prepare($q2);
         $sth2->execute( $borrowernumber, $item_type );
         if ( my $data2 = $sth2->fetchrow_hashref ) {
         my $sth2 = $dbh->prepare($q2);
         $sth2->execute( $borrowernumber, $item_type );
         if ( my $data2 = $sth2->fetchrow_hashref ) {


@@ -2259,7 +2342,7 @@ sub GetIssuingCharges {
 
 =head2 AddIssuingCharge
 
 
 =head2 AddIssuingCharge
 

-&AddIssuingCharge( $itemno, $borrowernumber, $charge )
+  &AddIssuingCharge( $itemno, $borrowernumber, $charge )

 
 =cut
 
 
 =cut
 


@@ -2281,7 +2364,7 @@ sub AddIssuingCharge {
 
 =head2 GetTransfers
 
 
 =head2 GetTransfers
 

-GetTransfers($itemnumber);
+  GetTransfers($itemnumber);

 
 =cut
 
 
 =cut
 


@@ -2307,7 +2390,7 @@ sub GetTransfers {
 
 =head2 GetTransfersFromTo
 
 
 =head2 GetTransfersFromTo
 

-@results = GetTransfersFromTo($frombranch,$tobranch);
+  @results = GetTransfersFromTo($frombranch,$tobranch);

 
 Returns the list of pending transfers between $from and $to branch
 
 
 Returns the list of pending transfers between $from and $to branch
 


@@ -2337,7 +2420,7 @@ sub GetTransfersFromTo {
 
 =head2 DeleteTransfer
 
 
 =head2 DeleteTransfer
 

-&DeleteTransfer($itemnumber);
+  &DeleteTransfer($itemnumber);

 
 =cut
 
 
 =cut
 


@@ -2355,11 +2438,14 @@ sub DeleteTransfer {
 
 =head2 AnonymiseIssueHistory
 
 
 =head2 AnonymiseIssueHistory
 

-$rows = AnonymiseIssueHistory($borrowernumber,$date)
+  $rows = AnonymiseIssueHistory($date,$borrowernumber)

 
 This function write NULL instead of C<$borrowernumber> given on input arg into the table issues.
 if C<$borrowernumber> is not set, it will delete the issue history for all borrower older than C<$date>.
 
 
 This function write NULL instead of C<$borrowernumber> given on input arg into the table issues.
 if C<$borrowernumber> is not set, it will delete the issue history for all borrower older than C<$date>.
 

+If c<$borrowernumber> is set, it will delete issue history for only that borrower, regardless of their opac privacy
+setting (force delete).
+

 return the number of affected rows.
 
 =cut
 return the number of affected rows.
 
 =cut


@@ -2370,12 +2456,24 @@ sub AnonymiseIssueHistory {
     my $dbh            = C4::Context->dbh;
     my $query          = "
         UPDATE old_issues
     my $dbh            = C4::Context->dbh;
     my $query          = "
         UPDATE old_issues

-        SET    borrowernumber = NULL
-        WHERE  returndate < '".$date."'
+        SET    borrowernumber = ?
+        WHERE  returndate < ?

           AND borrowernumber IS NOT NULL
     ";
           AND borrowernumber IS NOT NULL
     ";

-    $query .= " AND borrowernumber = '".$borrowernumber."'" if defined $borrowernumber;
-    my $rows_affected = $dbh->do($query);
+
+    # The default of 0 does not work due to foreign key constraints
+    # The anonymisation will fail quietly if AnonymousPatron is not a valid entry
+    my $anonymouspatron = (C4::Context->preference('AnonymousPatron')) ? C4::Context->preference('AnonymousPatron') : 0;
+    my @bind_params = ($anonymouspatron, $date);
+    if (defined $borrowernumber) {
+       $query .= " AND borrowernumber = ?";
+       push @bind_params, $borrowernumber;
+    } else {
+       $query .= " AND (SELECT privacy FROM borrowers WHERE borrowers.borrowernumber=old_issues.borrowernumber) <> 0";
+    }
+    my $sth = $dbh->prepare($query);
+    $sth->execute(@bind_params);
+    my $rows_affected = $sth->rows;  ### doublecheck row count return function

     return $rows_affected;
 }
 
     return $rows_affected;
 }
 


@@ -2452,7 +2550,7 @@ sub SendCirculationAlert {
 
 =head2 updateWrongTransfer
 
 
 =head2 updateWrongTransfer
 

-$items = updateWrongTransfer($itemNumber,$borrowernumber,$waitingAtLibrary,$FromLibrary);
+  $items = updateWrongTransfer($itemNumber,$borrowernumber,$waitingAtLibrary,$FromLibrary);

 
 This function validate the line of brachtransfer but with the wrong destination (mistake from a librarian ...), and create a new line in branchtransfer from the actual library to the original library of reservation 
 
 
 This function validate the line of brachtransfer but with the wrong destination (mistake from a librarian ...), and create a new line in branchtransfer from the actual library to the original library of reservation 
 


@@ -2478,7 +2576,8 @@ sub updateWrongTransfer {
 
 =head2 UpdateHoldingbranch
 
 
 =head2 UpdateHoldingbranch
 

-$items = UpdateHoldingbranch($branch,$itmenumber);
+  $items = UpdateHoldingbranch($branch,$itmenumber);
+

 Simple methode for updating hodlingbranch in items BDD line
 
 =cut
 Simple methode for updating hodlingbranch in items BDD line
 
 =cut


@@ -2530,15 +2629,18 @@ sub CalcDateDue {
 }
 
 =head2 CheckValidDatedue
 }
 
 =head2 CheckValidDatedue

-       This function does not account for holiday exceptions nor does it handle the 'useDaysMode' syspref .
-       To be replaced by CalcDateDue() once C4::Calendar use is tested.

 
 

-$newdatedue = CheckValidDatedue($date_due,$itemnumber,$branchcode);
+  $newdatedue = CheckValidDatedue($date_due,$itemnumber,$branchcode);
+
+This function does not account for holiday exceptions nor does it handle the 'useDaysMode' syspref .
+To be replaced by CalcDateDue() once C4::Calendar use is tested.
+

 this function validates the loan length against the holidays calendar, and adjusts the due date as per the 'useDaysMode' syspref.
 C<$date_due>   = returndate calculate with no day check
 C<$itemnumber>  = itemnumber
 C<$branchcode>  = location of issue (affected by 'CircControl' syspref)
 C<$loanlength>  = loan length prior to adjustment
 this function validates the loan length against the holidays calendar, and adjusts the due date as per the 'useDaysMode' syspref.
 C<$date_due>   = returndate calculate with no day check
 C<$itemnumber>  = itemnumber
 C<$branchcode>  = location of issue (affected by 'CircControl' syspref)
 C<$loanlength>  = loan length prior to adjustment

+

 =cut
 
 sub CheckValidDatedue {
 =cut
 
 sub CheckValidDatedue {


@@ -2567,8 +2669,10 @@ return $newdatedue;
 
 =head2 CheckRepeatableHolidays
 
 
 =head2 CheckRepeatableHolidays
 

-$countrepeatable = CheckRepeatableHoliday($itemnumber,$week_day,$branchcode);
-this function checks if the date due is a repeatable holiday
+  $countrepeatable = CheckRepeatableHoliday($itemnumber,$week_day,$branchcode);
+
+This function checks if the date due is a repeatable holiday
+

 C<$date_due>   = returndate calculate with no day check
 C<$itemnumber>  = itemnumber
 C<$branchcode>  = localisation of issue 
 C<$date_due>   = returndate calculate with no day check
 C<$itemnumber>  = itemnumber
 C<$branchcode>  = localisation of issue 


@@ -2592,8 +2696,10 @@ return $result;
 
 =head2 CheckSpecialHolidays
 
 
 =head2 CheckSpecialHolidays
 

-$countspecial = CheckSpecialHolidays($years,$month,$day,$itemnumber,$branchcode);
-this function check if the date is a special holiday
+  $countspecial = CheckSpecialHolidays($years,$month,$day,$itemnumber,$branchcode);
+
+This function check if the date is a special holiday
+

 C<$years>   = the years of datedue
 C<$month>   = the month of datedue
 C<$day>     = the day of datedue
 C<$years>   = the years of datedue
 C<$month>   = the month of datedue
 C<$day>     = the day of datedue


@@ -2621,8 +2727,10 @@ return $countspecial;
 
 =head2 CheckRepeatableSpecialHolidays
 
 
 =head2 CheckRepeatableSpecialHolidays
 

-$countspecial = CheckRepeatableSpecialHolidays($month,$day,$itemnumber,$branchcode);
-this function check if the date is a repeatble special holidays
+  $countspecial = CheckRepeatableSpecialHolidays($month,$day,$itemnumber,$branchcode);
+
+This function check if the date is a repeatble special holidays
+

 C<$month>   = the month of datedue
 C<$day>     = the day of datedue
 C<$itemnumber>  = itemnumber
 C<$month>   = the month of datedue
 C<$day>     = the day of datedue
 C<$itemnumber>  = itemnumber


@@ -2664,7 +2772,7 @@ return $exist;
 
 =head2 IsBranchTransferAllowed
 
 
 =head2 IsBranchTransferAllowed
 

-$allowed = IsBranchTransferAllowed( $toBranch, $fromBranch, $code );
+  $allowed = IsBranchTransferAllowed( $toBranch, $fromBranch, $code );

 
 Code is either an itemtype or collection doe depending on the pref BranchTransferLimitsType
 
 
 Code is either an itemtype or collection doe depending on the pref BranchTransferLimitsType
 


@@ -2692,7 +2800,7 @@ sub IsBranchTransferAllowed {
 
 =head2 CreateBranchTransferLimit
 
 
 =head2 CreateBranchTransferLimit
 

-CreateBranchTransferLimit( $toBranch, $fromBranch, $code );
+  CreateBranchTransferLimit( $toBranch, $fromBranch, $code );

 
 $code is either itemtype or collection code depending on what the pref BranchTransferLimitsType is set to.
 
 
 $code is either itemtype or collection code depending on what the pref BranchTransferLimitsType is set to.
 


@@ -2711,7 +2819,7 @@ sub CreateBranchTransferLimit {
 
 =head2 DeleteBranchTransferLimits
 
 
 =head2 DeleteBranchTransferLimits
 

-DeleteBranchTransferLimits();
+  DeleteBranchTransferLimits();

 
 =cut
 
 
 =cut
 


@@ -2728,7 +2836,7 @@ __END__
 
 =head1 AUTHOR
 
 
 =head1 AUTHOR
 

-Koha Developement team <info@koha.org>
+Koha Development Team <http://koha-community.org/>

 
 =cut
 
 
 =cut