Bug 3881: OPAC Privacy reimplementation
[koha_gimpoz] / C4 / Circulation.pm
index 98f324c..f7846ac 100644 (file)
@@ -13,13 +13,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.
 #
-# 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 warnings;  # soon!
+#use warnings; FIXME - Bug 2505
 use C4::Context;
 use C4::Stats;
 use C4::Reserves;
@@ -31,6 +31,7 @@ use C4::Dates;
 use C4::Calendar;
 use C4::Accounts;
 use C4::ItemCirculationAlertPreference;
+use C4::Dates qw(format_date);
 use C4::Message;
 use C4::Debug;
 use Date::Calc qw(
@@ -69,7 +70,6 @@ BEGIN {
                &AddRenewal
                &GetRenewCount
                &GetItemIssue
-                &GetOpenIssue
                &GetItemIssues
                &GetBorrowerIssues
                &GetIssuingCharges
@@ -77,6 +77,7 @@ BEGIN {
         &GetBranchBorrowerCircRule
         &GetBranchItemRule
                &GetBiblioIssues
+               &GetOpenIssue
                &AnonymiseIssueHistory
        );
 
@@ -117,11 +118,9 @@ Also deals with stocktaking.
 
 =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.
@@ -132,8 +131,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.
 
-=back
-
 =cut
 
 # FIXME -- the &decode fcn below should be wrapped into this one.
@@ -141,6 +138,7 @@ System Pref options.
 #
 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') {
@@ -159,24 +157,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"?
+       } 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
 
-=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.
 
-=back
-
 =cut
 
 sub decode {
@@ -208,7 +210,8 @@ sub decode {
 
 =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.
 
@@ -221,15 +224,17 @@ Otherwise, if an item is reserved, the transfer fails.
 
 Returns three values:
 
-=head3 $dotransfer 
+=over
+
+=item $dotransfer 
 
 is true if the transfer was successful.
 
-=head3 $messages
+=item $messages
 
 is a reference-to-hash which may have any of the following keys:
 
-=over 4
+=over
 
 =item C<BadBarcode>
 
@@ -257,6 +262,8 @@ The item was eligible to be transferred. Barring problems communicating with the
 
 =back
 
+=back
+
 =cut
 
 sub transferbook {
@@ -326,8 +333,9 @@ sub transferbook {
 
         # don't need to update MARC anymore, we do it in batch now
         $messages->{'WasTransfered'} = 1;
-               ModDateLastSeen( $itemnumber );
+
     }
+    ModDateLastSeen( $itemnumber );
     return ( $dotransfer, $messages, $biblio );
 }
 
@@ -414,7 +422,7 @@ sub TooMany {
 
         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);
         }
     }
 
@@ -442,7 +450,7 @@ sub TooMany {
 
         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);
         }
     }
 
@@ -560,9 +568,10 @@ sub itemissues {
 
 =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.
 
@@ -619,7 +628,9 @@ item withdrawn.
 
 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
@@ -640,7 +651,7 @@ reserved for someone else.
 
 =head3 INVALID_DATE
 
-sticky due date is invalid
+sticky due date is invalid or due date in the past
 
 =head3 TOO_MANY
 
@@ -678,7 +689,12 @@ sub CanBookBeIssued {
         # 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
@@ -686,6 +702,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'});
+        ModDateLastSeen( $item->{'itemnumber'} );
         return( { STATS => 1 }, {});
     }
     if ( $borrower->{flags}->{GNA} ) {
@@ -729,23 +746,32 @@ sub CanBookBeIssued {
     }
 
     my ($blocktype, $count) = C4::Members::IsMemberBlocked($borrower->{'borrowernumber'});
-    if($blocktype == -1){
-        ## remaining overdue documents
+    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;
-    }elsif($blocktype == 1){
-        ## blocked because of overdue return
-        $issuingimpossible{USERBLOCKEDOVERDUE} = $count;
     }
 
 #
     # 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{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;
+        }
     }
 
     #
@@ -796,8 +822,10 @@ sub CanBookBeIssued {
     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} );
+            $needsconfirmation{BORRNOTSAMEBRANCH} = GetBranchName( $borrower->{'branchcode'} )
+              if ( $borrower->{'branchcode'} ne $userenv->{branch} );
         }
     }
 
@@ -826,8 +854,11 @@ sub CanBookBeIssued {
         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.
@@ -841,13 +872,23 @@ sub CanBookBeIssued {
         {
             # 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.
-            $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 );
@@ -855,9 +896,9 @@ sub CanBookBeIssued {
 
 =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
 
@@ -895,7 +936,6 @@ sub AddIssue {
     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 );
@@ -1065,9 +1105,9 @@ sub AddIssue {
 
 =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
 
@@ -1127,6 +1167,8 @@ sub GetLoanLength {
 
 =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.
@@ -1134,8 +1176,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.
 
-my $irule = &GetIssuingRule($borrowertype,$itemtype,branchcode)
-
 =cut
 
 sub GetIssuingRule {
@@ -1182,11 +1222,7 @@ sub GetIssuingRule {
 
 =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.  
@@ -1269,11 +1305,7 @@ sub GetBranchBorrowerCircRule {
 
 =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.
@@ -1333,8 +1365,8 @@ sub GetBranchItemRule {
 
 =head2 AddReturn
 
-($doreturn, $messages, $iteminformation, $borrower) =
-    &AddReturn($barcode, $branch, $exemptfine, $dropbox);
+  ($doreturn, $messages, $iteminformation, $borrower) =
+      &AddReturn($barcode, $branch, $exemptfine, $dropbox);
 
 Returns a book.
 
@@ -1439,7 +1471,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 $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
@@ -1474,14 +1507,16 @@ sub AddReturn {
     # case of a return of document (deal with issues and holdingbranch)
     if ($doreturn) {
         $borrower or warn "AddReturn without current borrower";
-               my $circControlBranch = _GetCircControlBranch($item,$borrower);
+               my $circControlBranch;
         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) {
-            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.
         }
 
@@ -1586,11 +1621,7 @@ sub AddReturn {
 
 =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
@@ -1602,6 +1633,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.
 
+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>.
@@ -1609,7 +1643,7 @@ routine in C<C4::Accounts>.
 =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;
@@ -1633,6 +1667,16 @@ sub MarkIssueReturned {
                                   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 = ?");
@@ -1641,7 +1685,7 @@ sub MarkIssueReturned {
 
 =head2 _FixOverduesOnReturn
 
-    &_FixOverduesOnReturn($brn,$itm, $exemptfine, $dropboxmode);
+   &_FixOverduesOnReturn($brn,$itm, $exemptfine, $dropboxmode);
 
 C<$brn> borrowernumber
 
@@ -1705,7 +1749,7 @@ sub _FixOverduesOnReturn {
 
 =head2 _FixAccountForLostAndReturned
 
-       &_FixAccountForLostAndReturned($itemnumber, [$borrowernumber, $barcode]);
+  &_FixAccountForLostAndReturned($itemnumber, [$borrowernumber, $barcode]);
 
 Calculates the charge for a book lost and returned.
 
@@ -1836,7 +1880,7 @@ sub _GetCircControlBranch {
 
 =head2 GetItemIssue
 
-$issue = &GetItemIssue($itemnumber);
+  $issue = &GetItemIssue($itemnumber);
 
 Returns patron currently having a book, or undef if not checked out.
 
@@ -1863,7 +1907,7 @@ sub GetItemIssue {
 
 =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
 
@@ -1885,7 +1929,7 @@ sub GetOpenIssue {
 
 =head2 GetItemIssues
 
-$issues = &GetItemIssues($itemnumber, $history);
+  $issues = &GetItemIssues($itemnumber, $history);
 
 Returns patrons that have issued a book
 
@@ -1928,7 +1972,7 @@ sub GetItemIssues {
 
 =head2 GetBiblioIssues
 
-$issues = GetBiblioIssues($biblionumber);
+  $issues = GetBiblioIssues($biblionumber);
 
 this function get all issues from a biblionumber.
 
@@ -1972,11 +2016,7 @@ sub GetBiblioIssues {
 
 =head2 GetUpcomingDueIssues
 
-=over 4
-my $upcoming_dues = GetUpcomingDueIssues( { days_in_advance => 4 } );
-
-=back
+  my $upcoming_dues = GetUpcomingDueIssues( { days_in_advance => 4 } );
 
 =cut
 
@@ -2006,7 +2046,7 @@ END_SQL
 
 =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.
 
@@ -2060,9 +2100,9 @@ sub CanBookBeRenewed {
                    LEFT JOIN biblioitems USING (biblioitemnumber)
                    
                    WHERE
-                    issuingrules.categorycode = borrowers.categorycode
+                    (issuingrules.categorycode = borrowers.categorycode OR issuingrules.categorycode = '*')
                    AND
-                    issuingrules.itemtype = $itype
+                    (issuingrules.itemtype = $itype OR issuingrules.itemtype = '*')
                    AND
                     (issuingrules.branchcode = $controlbranch OR issuingrules.branchcode = '*') 
                    AND 
@@ -2098,7 +2138,7 @@ sub CanBookBeRenewed {
 
 =head2 AddRenewal
 
-&AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate]);
+  &AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate]);
 
 Renews a loan.
 
@@ -2200,39 +2240,42 @@ sub AddRenewal {
 
 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.
-    my $sth = $dbh->prepare("select * from issues
+    my $sth = $dbh->prepare(
+        "select * from issues
                                 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 $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
 
-($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.
@@ -2272,7 +2315,7 @@ sub GetIssuingCharges {
         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 ) {
@@ -2291,7 +2334,7 @@ sub GetIssuingCharges {
 
 =head2 AddIssuingCharge
 
-&AddIssuingCharge( $itemno, $borrowernumber, $charge )
+  &AddIssuingCharge( $itemno, $borrowernumber, $charge )
 
 =cut
 
@@ -2313,7 +2356,7 @@ sub AddIssuingCharge {
 
 =head2 GetTransfers
 
-GetTransfers($itemnumber);
+  GetTransfers($itemnumber);
 
 =cut
 
@@ -2339,7 +2382,7 @@ sub GetTransfers {
 
 =head2 GetTransfersFromTo
 
-@results = GetTransfersFromTo($frombranch,$tobranch);
+  @results = GetTransfersFromTo($frombranch,$tobranch);
 
 Returns the list of pending transfers between $from and $to branch
 
@@ -2369,7 +2412,7 @@ sub GetTransfersFromTo {
 
 =head2 DeleteTransfer
 
-&DeleteTransfer($itemnumber);
+  &DeleteTransfer($itemnumber);
 
 =cut
 
@@ -2387,11 +2430,14 @@ sub DeleteTransfer {
 
 =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>.
 
+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
@@ -2402,12 +2448,24 @@ sub AnonymiseIssueHistory {
     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
     ";
-    $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;
 }
 
@@ -2484,7 +2542,7 @@ sub SendCirculationAlert {
 
 =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 
 
@@ -2510,7 +2568,8 @@ sub updateWrongTransfer {
 
 =head2 UpdateHoldingbranch
 
-$items = UpdateHoldingbranch($branch,$itmenumber);
+  $items = UpdateHoldingbranch($branch,$itmenumber);
+
 Simple methode for updating hodlingbranch in items BDD line
 
 =cut
@@ -2562,15 +2621,18 @@ sub CalcDateDue {
 }
 
 =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
+
 =cut
 
 sub CheckValidDatedue {
@@ -2599,8 +2661,10 @@ return $newdatedue;
 
 =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 
@@ -2624,8 +2688,10 @@ return $result;
 
 =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
@@ -2653,8 +2719,10 @@ return $countspecial;
 
 =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
@@ -2696,7 +2764,7 @@ return $exist;
 
 =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
 
@@ -2724,7 +2792,7 @@ sub IsBranchTransferAllowed {
 
 =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.
 
@@ -2743,7 +2811,7 @@ sub CreateBranchTransferLimit {
 
 =head2 DeleteBranchTransferLimits
 
-DeleteBranchTransferLimits();
+  DeleteBranchTransferLimits();
 
 =cut
 
@@ -2760,7 +2828,7 @@ __END__
 
 =head1 AUTHOR
 
-Koha Developement team <info@koha.org>
+Koha Development Team <http://koha-community.org/>
 
 =cut