Bug 2965: Allow due date in the past
[koha_gimpoz] / C4 / Circulation.pm
index 5930bed..0131e90 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;
@@ -32,6 +32,7 @@ use C4::Calendar;
 use C4::Accounts;
 use C4::ItemCirculationAlertPreference;
 use C4::Message;
+use C4::Debug;
 use Date::Calc qw(
   Today
   Today_and_Now
@@ -68,7 +69,6 @@ BEGIN {
                &AddRenewal
                &GetRenewCount
                &GetItemIssue
-                &GetOpenIssue
                &GetItemIssues
                &GetBorrowerIssues
                &GetIssuingCharges
@@ -76,6 +76,7 @@ BEGIN {
         &GetBranchBorrowerCircRule
         &GetBranchItemRule
                &GetBiblioIssues
+               &GetOpenIssue
                &AnonymiseIssueHistory
        );
 
@@ -116,11 +117,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.
@@ -131,8 +130,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.
@@ -140,6 +137,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') {
@@ -158,24 +156,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);
+  $str = &decode($chunk);
 
-=over 4
-
-=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 {
@@ -207,7 +209,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.
 
@@ -220,15 +223,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>
 
@@ -256,6 +261,8 @@ The item was eligible to be transferred. Barring problems communicating with the
 
 =back
 
+=back
+
 =cut
 
 sub transferbook {
@@ -325,8 +332,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 );
 }
 
@@ -531,7 +539,6 @@ sub itemissues {
             $data->{'date_due'} = ($data->{'wthdrawn'} eq '1') ? 'Cancelled' : 'Available';
         }
 
-        $sth2->finish;
 
         # Find the last 3 people who borrowed this item.
         $sth2 = $dbh->prepare(
@@ -551,20 +558,19 @@ sub itemissues {
             }    # if
         }    # for
 
-        $sth2->finish;
         $results[$i] = $data;
         $i++;
     }
 
-    $sth->finish;
     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.
 
@@ -621,7 +627,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
@@ -642,7 +650,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
 
@@ -680,7 +688,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
@@ -688,6 +701,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} ) {
@@ -730,7 +744,21 @@ sub CanBookBeIssued {
         }
     }
 
-    #
+    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
     #
        my $toomany = TooMany( $borrower, $item->{biblionumber}, $item );
@@ -789,8 +817,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} );
         }
     }
 
@@ -848,9 +878,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
 
@@ -888,7 +918,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 );
@@ -999,6 +1028,9 @@ sub AddIssue {
             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'},
@@ -1055,9 +1087,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
 
@@ -1117,6 +1149,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.
@@ -1124,8 +1158,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 {
@@ -1172,11 +1204,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.  
@@ -1259,11 +1287,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.
@@ -1323,8 +1347,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.
 
@@ -1429,7 +1453,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
@@ -1447,7 +1472,13 @@ sub AddReturn {
             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
@@ -1458,10 +1489,12 @@ 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) {
@@ -1469,15 +1502,17 @@ sub AddReturn {
             $messages->{'WasReturned'} = 1;    # FIXME is the "= 1" right?  This could be the borrower hash.
         }
 
-        # the holdingbranch is updated if the document is returned to another location.
-        if ($item->{'holdingbranch'} ne $branch) {
-            UpdateHoldingbranch($branch, $item->{'itemnumber'});
-            $item->{'holdingbranch'} = $branch; # update item data holdingbranch too
-        }
-        ModDateLastSeen( $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'} );
 
@@ -1550,14 +1585,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 .
 
-    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
-             ! 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}
@@ -1568,11 +1603,7 @@ sub AddReturn {
 
 =head2 MarkIssueReturned
 
-=over 4
-
-MarkIssueReturned($borrowernumber, $itemnumber, $dropbox_branch, $returndate);
-
-=back
+  MarkIssueReturned($borrowernumber, $itemnumber, $dropbox_branch, $returndate);
 
 Unconditionally marks an issue as being returned by
 moving the C<issues> row to C<old_issues> and
@@ -1623,7 +1654,7 @@ sub MarkIssueReturned {
 
 =head2 _FixOverduesOnReturn
 
-    &_FixOverduesOnReturn($brn,$itm, $exemptfine, $dropboxmode);
+   &_FixOverduesOnReturn($brn,$itm, $exemptfine, $dropboxmode);
 
 C<$brn> borrowernumber
 
@@ -1687,7 +1718,7 @@ sub _FixOverduesOnReturn {
 
 =head2 _FixAccountForLostAndReturned
 
-       &_FixAccountForLostAndReturned($itemnumber, [$borrowernumber, $barcode]);
+  &_FixAccountForLostAndReturned($itemnumber, [$borrowernumber, $barcode]);
 
 Calculates the charge for a book lost and returned.
 
@@ -1818,7 +1849,7 @@ sub _GetCircControlBranch {
 
 =head2 GetItemIssue
 
-$issue = &GetItemIssue($itemnumber);
+  $issue = &GetItemIssue($itemnumber);
 
 Returns patron currently having a book, or undef if not checked out.
 
@@ -1845,7 +1876,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
 
@@ -1867,7 +1898,7 @@ sub GetOpenIssue {
 
 =head2 GetItemIssues
 
-$issues = &GetItemIssues($itemnumber, $history);
+  $issues = &GetItemIssues($itemnumber, $history);
 
 Returns patrons that have issued a book
 
@@ -1910,7 +1941,7 @@ sub GetItemIssues {
 
 =head2 GetBiblioIssues
 
-$issues = GetBiblioIssues($biblionumber);
+  $issues = GetBiblioIssues($biblionumber);
 
 this function get all issues from a biblionumber.
 
@@ -1954,11 +1985,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
 
@@ -1988,7 +2015,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.
 
@@ -2022,39 +2049,52 @@ sub CanBookBeRenewed {
     # 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";
                }
-        $sth2->finish;
+               
         my ( $resfound, $resrec ) = C4::Reserves::CheckReserves($itemnumber);
         if ($resfound) {
             $renewokay = 0;
@@ -2062,13 +2102,12 @@ sub CanBookBeRenewed {
         }
 
     }
-    $sth1->finish;
     return ($renewokay,$error);
 }
 
 =head2 AddRenewal
 
-&AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate]);
+  &AddRenewal($borrowernumber, $itemnumber, $branch, [$datedue], [$lastreneweddate]);
 
 Renews a loan.
 
@@ -2170,39 +2209,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.
@@ -2242,7 +2284,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 ) {
@@ -2261,7 +2303,7 @@ sub GetIssuingCharges {
 
 =head2 AddIssuingCharge
 
-&AddIssuingCharge( $itemno, $borrowernumber, $charge )
+  &AddIssuingCharge( $itemno, $borrowernumber, $charge )
 
 =cut
 
@@ -2283,7 +2325,7 @@ sub AddIssuingCharge {
 
 =head2 GetTransfers
 
-GetTransfers($itemnumber);
+  GetTransfers($itemnumber);
 
 =cut
 
@@ -2309,7 +2351,7 @@ sub GetTransfers {
 
 =head2 GetTransfersFromTo
 
-@results = GetTransfersFromTo($frombranch,$tobranch);
+  @results = GetTransfersFromTo($frombranch,$tobranch);
 
 Returns the list of pending transfers between $from and $to branch
 
@@ -2339,7 +2381,7 @@ sub GetTransfersFromTo {
 
 =head2 DeleteTransfer
 
-&DeleteTransfer($itemnumber);
+  &DeleteTransfer($itemnumber);
 
 =cut
 
@@ -2357,7 +2399,7 @@ sub DeleteTransfer {
 
 =head2 AnonymiseIssueHistory
 
-$rows = AnonymiseIssueHistory($borrowernumber,$date)
+  $rows = AnonymiseIssueHistory($borrowernumber,$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>.
@@ -2454,7 +2496,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 
 
@@ -2480,7 +2522,8 @@ sub updateWrongTransfer {
 
 =head2 UpdateHoldingbranch
 
-$items = UpdateHoldingbranch($branch,$itmenumber);
+  $items = UpdateHoldingbranch($branch,$itmenumber);
+
 Simple methode for updating hodlingbranch in items BDD line
 
 =cut
@@ -2532,15 +2575,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 {
@@ -2569,8 +2615,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 
@@ -2594,8 +2642,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
@@ -2623,8 +2673,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
@@ -2666,7 +2718,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
 
@@ -2694,7 +2746,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.
 
@@ -2713,7 +2765,7 @@ sub CreateBranchTransferLimit {
 
 =head2 DeleteBranchTransferLimits
 
-DeleteBranchTransferLimits();
+  DeleteBranchTransferLimits();
 
 =cut
 
@@ -2730,7 +2782,7 @@ __END__
 
 =head1 AUTHOR
 
-Koha Developement team <info@koha.org>
+Koha Development Team <http://koha-community.org/>
 
 =cut