bug_5245 Fix SQL syntax in GetItems
[koha_gimpoz] / C4 / Circulation.pm
index ab4f7a5..644ab27 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.
@@ -164,18 +161,14 @@ sub barcodedecode {
 
 =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 {
@@ -207,7 +200,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 +214,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 +252,8 @@ The item was eligible to be transferred. Barring problems communicating with the
 
 =back
 
+=back
+
 =cut
 
 sub transferbook {
@@ -325,8 +323,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 );
 }
 
@@ -339,7 +338,7 @@ sub TooMany {
     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
@@ -531,7 +530,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 +549,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 +618,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
@@ -672,7 +671,7 @@ sub CanBookBeIssued {
     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 );
@@ -688,6 +687,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 +730,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 +803,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 +864,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 +904,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 );
@@ -898,7 +913,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.
-               my $branch = _GetCirculationBranch($item,$borrower);
+               my $branch = _GetCircControlBranch($item,$borrower);
                
                # get actual issuing if there is one
                my $actualissue = GetItemIssue( $item->{itemnumber});
@@ -999,6 +1014,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 +1073,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 +1135,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 +1144,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 +1190,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 +1273,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 +1333,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.
 
@@ -1447,7 +1457,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 +1474,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 = _GetCirculationBranch($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,17 +1487,17 @@ sub AddReturn {
             $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'});
     }
 
+    # 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'} );
 
@@ -1552,14 +1570,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}
@@ -1570,11 +1588,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
@@ -1625,7 +1639,7 @@ sub MarkIssueReturned {
 
 =head2 _FixOverduesOnReturn
 
-    &_FixOverduesOnReturn($brn,$itm, $exemptfine, $dropboxmode);
+   &_FixOverduesOnReturn($brn,$itm, $exemptfine, $dropboxmode);
 
 C<$brn> borrowernumber
 
@@ -1689,7 +1703,7 @@ sub _FixOverduesOnReturn {
 
 =head2 _FixAccountForLostAndReturned
 
-       &_FixAccountForLostAndReturned($itemnumber, [$borrowernumber, $barcode]);
+  &_FixAccountForLostAndReturned($itemnumber, [$borrowernumber, $barcode]);
 
 Calculates the charge for a book lost and returned.
 
@@ -1776,37 +1790,41 @@ sub _FixAccountForLostAndReturned {
     return;
 }
 
-=head2 _GetCirculationBranch
+=head2 _GetCircControlBranch
 
-   _GetCirculationBranch($iteminfos,$borrower);
+   my $circ_control_branch = _GetCircControlBranch($iteminfos, $borrower);
 
 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.
 
-Internal function, called by AddReturn
-
 =cut
 
-sub _GetCirculationBranch {
-    my ($iteminfos, $borrower) = @_;
+sub _GetCircControlBranch {
+    my ($item, $borrower) = @_;
     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};
+    } 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 +1834,7 @@ sub _GetCirculationBranch {
 
 =head2 GetItemIssue
 
-$issue = &GetItemIssue($itemnumber);
+  $issue = &GetItemIssue($itemnumber);
 
 Returns patron currently having a book, or undef if not checked out.
 
@@ -1843,7 +1861,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
 
@@ -1865,7 +1883,7 @@ sub GetOpenIssue {
 
 =head2 GetItemIssues
 
-$issues = &GetItemIssues($itemnumber, $history);
+  $issues = &GetItemIssues($itemnumber, $history);
 
 Returns patrons that have issued a book
 
@@ -1908,7 +1926,7 @@ sub GetItemIssues {
 
 =head2 GetBiblioIssues
 
-$issues = GetBiblioIssues($biblionumber);
+  $issues = GetBiblioIssues($biblionumber);
 
 this function get all issues from a biblionumber.
 
@@ -1952,11 +1970,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
 
@@ -1986,7 +2000,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.
 
@@ -2020,39 +2034,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;
@@ -2060,13 +2087,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.
 
@@ -2168,39 +2194,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.
@@ -2240,7 +2269,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 ) {
@@ -2259,7 +2288,7 @@ sub GetIssuingCharges {
 
 =head2 AddIssuingCharge
 
-&AddIssuingCharge( $itemno, $borrowernumber, $charge )
+  &AddIssuingCharge( $itemno, $borrowernumber, $charge )
 
 =cut
 
@@ -2281,7 +2310,7 @@ sub AddIssuingCharge {
 
 =head2 GetTransfers
 
-GetTransfers($itemnumber);
+  GetTransfers($itemnumber);
 
 =cut
 
@@ -2307,7 +2336,7 @@ sub GetTransfers {
 
 =head2 GetTransfersFromTo
 
-@results = GetTransfersFromTo($frombranch,$tobranch);
+  @results = GetTransfersFromTo($frombranch,$tobranch);
 
 Returns the list of pending transfers between $from and $to branch
 
@@ -2337,7 +2366,7 @@ sub GetTransfersFromTo {
 
 =head2 DeleteTransfer
 
-&DeleteTransfer($itemnumber);
+  &DeleteTransfer($itemnumber);
 
 =cut
 
@@ -2355,7 +2384,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>.
@@ -2452,7 +2481,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 
 
@@ -2478,7 +2507,8 @@ sub updateWrongTransfer {
 
 =head2 UpdateHoldingbranch
 
-$items = UpdateHoldingbranch($branch,$itmenumber);
+  $items = UpdateHoldingbranch($branch,$itmenumber);
+
 Simple methode for updating hodlingbranch in items BDD line
 
 =cut
@@ -2530,15 +2560,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 {
@@ -2567,8 +2600,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 
@@ -2592,8 +2627,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
@@ -2621,8 +2658,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
@@ -2664,7 +2703,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
 
@@ -2692,7 +2731,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.
 
@@ -2711,7 +2750,7 @@ sub CreateBranchTransferLimit {
 
 =head2 DeleteBranchTransferLimits
 
-DeleteBranchTransferLimits();
+  DeleteBranchTransferLimits();
 
 =cut
 
@@ -2728,7 +2767,7 @@ __END__
 
 =head1 AUTHOR
 
-Koha Developement team <info@koha.org>
+Koha Development Team <http://koha-community.org/>
 
 =cut