Bug 13687: Move hold policy check into CanItemBeReserved
[srvgit] / C4 / Reserves.pm
index 032e044..9c760b6 100644 (file)
@@ -7,18 +7,18 @@ package C4::Reserves;
 #
 # This file is part of Koha.
 #
 #
 # This file is part of Koha.
 #
-# Koha is free software; you can redistribute it and/or modify it under the
-# terms of the GNU General Public License as published by the Free Software
-# Foundation; either version 2 of the License, or (at your option) any later
-# version.
+# Koha is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
 #
 #
-# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
-# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
-# A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
+# Koha is distributed in the hope that it will be useful, but
+# WITHOUT ANY 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.,
-# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+# You should have received a copy of the GNU General Public License
+# along with Koha; if not, see <http://www.gnu.org/licenses>.
 
 
 use strict;
 
 
 use strict;
@@ -41,7 +41,7 @@ use Koha::DateUtils;
 use Koha::Calendar;
 use Koha::Database;
 
 use Koha::Calendar;
 use Koha::Database;
 
-use List::MoreUtils qw( firstidx );
+use List::MoreUtils qw( firstidx any );
 
 use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
 
 
 use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
 
@@ -70,13 +70,13 @@ This modules provides somes functions to deal with reservations.
   The complete workflow is :
   ==== 1st use case ====
   patron request a document, 1st available :                      P >0, F=NULL, I=NULL
   The complete workflow is :
   ==== 1st use case ====
   patron request a document, 1st available :                      P >0, F=NULL, I=NULL
-  a library having it run "transfertodo", and clic on the list    
+  a library having it run "transfertodo", and clic on the list
          if there is no transfer to do, the reserve waiting
          if there is no transfer to do, the reserve waiting
-         patron can pick it up                                    P =0, F=W,    I=filled 
+         patron can pick it up                                    P =0, F=W,    I=filled
          if there is a transfer to do, write in branchtransfer    P =0, F=T,    I=filled
            The pickup library recieve the book, it check in       P =0, F=W,    I=filled
   The patron borrow the book                                      P =0, F=F,    I=filled
          if there is a transfer to do, write in branchtransfer    P =0, F=T,    I=filled
            The pickup library recieve the book, it check in       P =0, F=W,    I=filled
   The patron borrow the book                                      P =0, F=F,    I=filled
-  
+
   ==== 2nd use case ====
   patron requests a document, a given item,
     If pickup is holding branch                                   P =0, F=W,   I=filled
   ==== 2nd use case ====
   patron requests a document, a given item,
     If pickup is holding branch                                   P =0, F=W,   I=filled
@@ -106,9 +106,9 @@ BEGIN {
         &GetReserveFee
         &GetReserveInfo
         &GetReserveStatus
         &GetReserveFee
         &GetReserveInfo
         &GetReserveStatus
-        
+
         &GetOtherReserves
         &GetOtherReserves
-        
+
         &ModReserveFill
         &ModReserveAffect
         &ModReserve
         &ModReserveFill
         &ModReserveAffect
         &ModReserve
@@ -116,10 +116,10 @@ BEGIN {
         &ModReserveCancelAll
         &ModReserveMinusPriority
         &MoveReserve
         &ModReserveCancelAll
         &ModReserveMinusPriority
         &MoveReserve
-        
+
         &CheckReserves
         &CanBookBeReserved
         &CheckReserves
         &CanBookBeReserved
-       &CanItemBeReserved
+        &CanItemBeReserved
         &CanReserveBeCanceledFromOpac
         &CancelReserve
         &CancelExpiredReserves
         &CanReserveBeCanceledFromOpac
         &CancelReserve
         &CancelExpiredReserves
@@ -127,7 +127,9 @@ BEGIN {
         &AutoUnsuspendReserves
 
         &IsAvailableForItemLevelRequest
         &AutoUnsuspendReserves
 
         &IsAvailableForItemLevelRequest
-        
+
+        &OPACItemHoldsAllowed
+
         &AlterPriority
         &ToggleLowestPriority
 
         &AlterPriority
         &ToggleLowestPriority
 
@@ -136,9 +138,11 @@ BEGIN {
         &SuspendAll
 
         &GetReservesControlBranch
         &SuspendAll
 
         &GetReservesControlBranch
+
+        IsItemOnHoldAndFound
     );
     @EXPORT_OK = qw( MergeHolds );
     );
     @EXPORT_OK = qw( MergeHolds );
-}    
+}
 
 =head2 AddReserve
 
 
 =head2 AddReserve
 
@@ -179,32 +183,33 @@ sub AddReserve {
     # updates take place here
     if ( $fee > 0 ) {
         my $nextacctno = &getnextacctno( $borrowernumber );
     # updates take place here
     if ( $fee > 0 ) {
         my $nextacctno = &getnextacctno( $borrowernumber );
-        my $query      = qq/
+        my $query      = qq{
         INSERT INTO accountlines
             (borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding)
         VALUES
             (?,?,now(),?,?,'Res',?)
         INSERT INTO accountlines
             (borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding)
         VALUES
             (?,?,now(),?,?,'Res',?)
-    /;
+    };
         my $usth = $dbh->prepare($query);
         $usth->execute( $borrowernumber, $nextacctno, $fee,
             "Reserve Charge - $title", $fee );
     }
 
     #if ($const eq 'a'){
         my $usth = $dbh->prepare($query);
         $usth->execute( $borrowernumber, $nextacctno, $fee,
             "Reserve Charge - $title", $fee );
     }
 
     #if ($const eq 'a'){
-    my $query = qq/
+    my $query = qq{
         INSERT INTO reserves
             (borrowernumber,biblionumber,reservedate,branchcode,constrainttype,
             priority,reservenotes,itemnumber,found,waitingdate,expirationdate)
         VALUES
              (?,?,?,?,?,
              ?,?,?,?,?,?)
         INSERT INTO reserves
             (borrowernumber,biblionumber,reservedate,branchcode,constrainttype,
             priority,reservenotes,itemnumber,found,waitingdate,expirationdate)
         VALUES
              (?,?,?,?,?,
              ?,?,?,?,?,?)
-    /;
+             };
     my $sth = $dbh->prepare($query);
     $sth->execute(
         $borrowernumber, $biblionumber, $resdate, $branch,
         $const,          $priority,     $notes,   $checkitem,
         $found,          $waitingdate, $expdate
     );
     my $sth = $dbh->prepare($query);
     $sth->execute(
         $borrowernumber, $biblionumber, $resdate, $branch,
         $const,          $priority,     $notes,   $checkitem,
         $found,          $waitingdate, $expdate
     );
+    my $reserve_id = $sth->{mysql_insertid};
 
     # Send e-mail to librarian if syspref is active
     if(C4::Context->preference("emailLibrarianWhenHoldIsPlaced")){
 
     # Send e-mail to librarian if syspref is active
     if(C4::Context->preference("emailLibrarianWhenHoldIsPlaced")){
@@ -237,18 +242,18 @@ sub AddReserve {
 
     #}
     ($const eq "o" || $const eq "e") or return;   # FIXME: why not have a useful return value?
 
     #}
     ($const eq "o" || $const eq "e") or return;   # FIXME: why not have a useful return value?
-    $query = qq/
+    $query = qq{
         INSERT INTO reserveconstraints
             (borrowernumber,biblionumber,reservedate,biblioitemnumber)
         VALUES
             (?,?,?,?)
         INSERT INTO reserveconstraints
             (borrowernumber,biblionumber,reservedate,biblioitemnumber)
         VALUES
             (?,?,?,?)
-    /;
+    };
     $sth = $dbh->prepare($query);    # keep prepare outside the loop!
     foreach (@$bibitems) {
         $sth->execute($borrowernumber, $biblionumber, $resdate, $_);
     }
         
     $sth = $dbh->prepare($query);    # keep prepare outside the loop!
     foreach (@$bibitems) {
         $sth->execute($borrowernumber, $biblionumber, $resdate, $_);
     }
         
-    return;     # FIXME: why not have a useful return value?
+    return $reserve_id;
 }
 
 =head2 GetReserve
 }
 
 =head2 GetReserve
@@ -348,7 +353,7 @@ sub GetReservesFromBiblionumber {
                 push( @bibitemno, $bibitemnos );    # FIXME: inefficient: use fetchall_arrayref
             }
             my $count = scalar @bibitemno;
                 push( @bibitemno, $bibitemnos );    # FIXME: inefficient: use fetchall_arrayref
             }
             my $count = scalar @bibitemno;
-    
+
             # if we have two or more different specific itemtypes
             # reserved by same person on same day
             my $bdata;
             # if we have two or more different specific itemtypes
             # reserved by same person on same day
             my $bdata;
@@ -448,7 +453,10 @@ sub GetReservesFromBorrowernumber {
 #-------------------------------------------------------------------------------------
 =head2 CanBookBeReserved
 
 #-------------------------------------------------------------------------------------
 =head2 CanBookBeReserved
 
-  $error = &CanBookBeReserved($borrowernumber, $biblionumber)
+  $canReserve = &CanBookBeReserved($borrowernumber, $biblionumber)
+  if ($canReserve eq 'OK') { #We can reserve this Item! }
+
+See CanItemBeReserved() for possible return values.
 
 =cut
 
 
 =cut
 
@@ -462,23 +470,31 @@ sub CanBookBeReserved{
     push (@$items,@hostitems);
     }
 
     push (@$items,@hostitems);
     }
 
-    foreach my $item (@$items){
-        return 1 if CanItemBeReserved($borrowernumber, $item);
+    my $canReserve;
+    foreach my $item (@$items) {
+        $canReserve = CanItemBeReserved( $borrowernumber, $item );
+        return 'OK' if $canReserve eq 'OK';
     }
     }
-    return 0;
+    return $canReserve;
 }
 
 =head2 CanItemBeReserved
 
 }
 
 =head2 CanItemBeReserved
 
-  $error = &CanItemBeReserved($borrowernumber, $itemnumber)
+  $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber)
+  if ($canReserve eq 'OK') { #We can reserve this Item! }
 
 
-This function return 1 if an item can be issued by this borrower.
+@RETURNS OK,              if the Item can be reserved.
+         ageRestricted,   if the Item is age restricted for this borrower.
+         damaged,         if the Item is damaged.
+         cannotReserveFromOtherBranches, if syspref 'canreservefromotherbranches' is OK.
+         tooManyReserves, if the borrower has exceeded his maximum reserve amount.
+         notReservable,   if holds on this item are not allowed
 
 =cut
 
 sub CanItemBeReserved{
     my ($borrowernumber, $itemnumber) = @_;
 
 =cut
 
 sub CanItemBeReserved{
     my ($borrowernumber, $itemnumber) = @_;
-    
+
     my $dbh             = C4::Context->dbh;
     my $ruleitemtype; # itemtype of the matching issuing rule
     my $allowedreserves = 0;
     my $dbh             = C4::Context->dbh;
     my $ruleitemtype; # itemtype of the matching issuing rule
     my $allowedreserves = 0;
@@ -486,28 +502,32 @@ sub CanItemBeReserved{
     # we retrieve borrowers and items informations #
     # item->{itype} will come for biblioitems if necessery
     my $item = GetItem($itemnumber);
     # we retrieve borrowers and items informations #
     # item->{itype} will come for biblioitems if necessery
     my $item = GetItem($itemnumber);
+    my $biblioData = C4::Biblio::GetBiblioData( $item->{biblionumber} );
+    my $borrower = C4::Members::GetMember('borrowernumber'=>$borrowernumber);
 
     # If an item is damaged and we don't allow holds on damaged items, we can stop right here
 
     # If an item is damaged and we don't allow holds on damaged items, we can stop right here
-    return 0 if ( $item->{damaged} && !C4::Context->preference('AllowHoldsOnDamagedItems') );
+    return 'damaged' if ( $item->{damaged} && !C4::Context->preference('AllowHoldsOnDamagedItems') );
+
+    #Check for the age restriction
+    my ($ageRestriction, $daysToAgeRestriction) = C4::Circulation::GetAgeRestriction( $biblioData->{agerestriction}, $borrower );
+    return 'ageRestricted' if $daysToAgeRestriction && $daysToAgeRestriction > 0;
 
 
-    my $borrower = C4::Members::GetMember('borrowernumber'=>$borrowernumber);     
-    
     my $controlbranch = C4::Context->preference('ReservesControlBranch');
     my $itemtypefield = C4::Context->preference('item-level_itypes') ? "itype" : "itemtype";
 
     # we retrieve user rights on this itemtype and branchcode
     my $controlbranch = C4::Context->preference('ReservesControlBranch');
     my $itemtypefield = C4::Context->preference('item-level_itypes') ? "itype" : "itemtype";
 
     # we retrieve user rights on this itemtype and branchcode
-    my $sth = $dbh->prepare("SELECT categorycode, itemtype, branchcode, reservesallowed 
-                             FROM issuingrules 
-                             WHERE (categorycode in (?,'*') ) 
-                             AND (itemtype IN (?,'*')) 
-                             AND (branchcode IN (?,'*')) 
-                             ORDER BY 
-                               categorycode DESC, 
-                               itemtype     DESC, 
+    my $sth = $dbh->prepare("SELECT categorycode, itemtype, branchcode, reservesallowed
+                             FROM issuingrules
+                             WHERE (categorycode in (?,'*') )
+                             AND (itemtype IN (?,'*'))
+                             AND (branchcode IN (?,'*'))
+                             ORDER BY
+                               categorycode DESC,
+                               itemtype     DESC,
                                branchcode   DESC;"
                            );
                                branchcode   DESC;"
                            );
-                           
-    my $querycount ="SELECT 
+
+    my $querycount ="SELECT
                             count(*) as count
                             FROM reserves
                                 LEFT JOIN items USING (itemnumber)
                             count(*) as count
                             FROM reserves
                                 LEFT JOIN items USING (itemnumber)
@@ -519,7 +539,7 @@ sub CanItemBeReserved{
     
     my $branchcode   = "";
     my $branchfield  = "reserves.branchcode";
     
     my $branchcode   = "";
     my $branchfield  = "reserves.branchcode";
-    
+
     if( $controlbranch eq "ItemHomeLibrary" ){
         $branchfield = "items.homebranch";
         $branchcode = $item->{homebranch};
     if( $controlbranch eq "ItemHomeLibrary" ){
         $branchfield = "items.homebranch";
         $branchcode = $item->{homebranch};
@@ -536,9 +556,9 @@ sub CanItemBeReserved{
     }else{
         $ruleitemtype = '*';
     }
     }else{
         $ruleitemtype = '*';
     }
-    
+
     # we retrieve count
     # we retrieve count
-    
+
     $querycount .= "AND $branchfield = ?";
     
     $querycount .= " AND $itemtypefield = ?" if ($ruleitemtype ne "*");
     $querycount .= "AND $branchfield = ?";
     
     $querycount .= " AND $itemtypefield = ?" if ($ruleitemtype ne "*");
@@ -549,15 +569,29 @@ sub CanItemBeReserved{
     }else{
         $sthcount->execute($borrowernumber, $branchcode, $ruleitemtype);
     }
     }else{
         $sthcount->execute($borrowernumber, $branchcode, $ruleitemtype);
     }
-    
+
     my $reservecount = "0";
     if(my $rowcount = $sthcount->fetchrow_hashref()){
         $reservecount = $rowcount->{count};
     }
     my $reservecount = "0";
     if(my $rowcount = $sthcount->fetchrow_hashref()){
         $reservecount = $rowcount->{count};
     }
-    
     # we check if it's ok or not
     if( $reservecount >= $allowedreserves ){
     # we check if it's ok or not
     if( $reservecount >= $allowedreserves ){
-        return 0;
+        return 'tooManyReserves';
+    }
+
+    my $circ_control_branch = C4::Circulation::_GetCircControlBranch($item,
+        $borrower);
+    my $branchitemrule = C4::Circulation::GetBranchItemRule($circ_control_branch,
+        $item->{itype});
+
+    if ( $branchitemrule->{holdallowed} == 0 ) {
+        return 'notReservable';
+    }
+
+    if (   $branchitemrule->{holdallowed} == 1
+        && $borrower->{branchcode} ne $item->{homebranch} )
+    {
+          return 'cannotReserveFromOtherBranches';
     }
 
     # If reservecount is ok, we check item branch if IndependentBranches is ON
     }
 
     # If reservecount is ok, we check item branch if IndependentBranches is ON
@@ -567,11 +601,11 @@ sub CanItemBeReserved{
     {
         my $itembranch = $item->{homebranch};
         if ($itembranch ne $borrower->{branchcode}) {
     {
         my $itembranch = $item->{homebranch};
         if ($itembranch ne $borrower->{branchcode}) {
-            return 0;
+            return 'cannotReserveFromOtherBranches';
         }
     }
 
         }
     }
 
-    return 1;
+    return 'OK';
 }
 
 =head2 CanReserveBeCanceledFromOpac
 }
 
 =head2 CanReserveBeCanceledFromOpac
@@ -684,11 +718,11 @@ sub GetReserveFee {
     #check for issues;
     my $dbh   = C4::Context->dbh;
     my $const = lc substr( $constraint, 0, 1 );
     #check for issues;
     my $dbh   = C4::Context->dbh;
     my $const = lc substr( $constraint, 0, 1 );
-    my $query = qq/
+    my $query = qq{
       SELECT * FROM borrowers
     LEFT JOIN categories ON borrowers.categorycode = categories.categorycode
     WHERE borrowernumber = ?
       SELECT * FROM borrowers
     LEFT JOIN categories ON borrowers.categorycode = categories.categorycode
     WHERE borrowernumber = ?
-    /;
+    };
     my $sth = $dbh->prepare($query);
     $sth->execute($borrowernumber);
     my $data = $sth->fetchrow_hashref;
     my $sth = $dbh->prepare($query);
     $sth->execute($borrowernumber);
     my $data = $sth->fetchrow_hashref;
@@ -835,9 +869,9 @@ sub GetReservesForBranch {
 
 =head2 GetReserveStatus
 
 
 =head2 GetReserveStatus
 
-  $reservestatus = GetReserveStatus($itemnumber, $biblionumber);
+  $reservestatus = GetReserveStatus($itemnumber);
 
 
-Take an itemnumber or a biblionumber and return the status of the reserve places on it.
+Takes an itemnumber and returns the status of the reserve placed on it.
 If several reserves exist, the reserve with the lower priority is given.
 
 =cut
 If several reserves exist, the reserve with the lower priority is given.
 
 =cut
@@ -847,7 +881,7 @@ If several reserves exist, the reserve with the lower priority is given.
 ## multiple reserves for that bib can have the itemnumber set
 ## the sub is only used once in the codebase.
 sub GetReserveStatus {
 ## multiple reserves for that bib can have the itemnumber set
 ## the sub is only used once in the codebase.
 sub GetReserveStatus {
-    my ($itemnumber, $biblionumber) = @_;
+    my ($itemnumber) = @_;
 
     my $dbh = C4::Context->dbh;
 
 
     my $dbh = C4::Context->dbh;
 
@@ -858,12 +892,6 @@ sub GetReserveStatus {
         ($found, $priority) = $sth->fetchrow_array;
     }
 
         ($found, $priority) = $sth->fetchrow_array;
     }
 
-    if ( $biblionumber and not defined $found and not defined $priority ) {
-        $sth = $dbh->prepare("SELECT found, priority FROM reserves WHERE biblionumber = ? order by priority LIMIT 1");
-        $sth->execute($biblionumber);
-        ($found, $priority) = $sth->fetchrow_array;
-    }
-
     if(defined $found) {
         return 'Waiting'  if $found eq 'W' and $priority == 0;
         return 'Finished' if $found eq 'F';
     if(defined $found) {
         return 'Waiting'  if $found eq 'W' and $priority == 0;
         return 'Finished' if $found eq 'F';
@@ -904,7 +932,7 @@ table in the Koha database.
 =cut
 
 sub CheckReserves {
 =cut
 
 sub CheckReserves {
-    my ( $item, $barcode, $lookahead_days) = @_;
+    my ( $item, $barcode, $lookahead_days, $ignore_borrowers) = @_;
     my $dbh = C4::Context->dbh;
     my $sth;
     my $select;
     my $dbh = C4::Context->dbh;
     my $sth;
     my $select;
@@ -938,7 +966,7 @@ sub CheckReserves {
            LEFT JOIN itemtypes   ON biblioitems.itemtype   = itemtypes.itemtype
         ";
     }
            LEFT JOIN itemtypes   ON biblioitems.itemtype   = itemtypes.itemtype
         ";
     }
-   
+
     if ($item) {
         $sth = $dbh->prepare("$select WHERE itemnumber = ?");
         $sth->execute($item);
     if ($item) {
         $sth = $dbh->prepare("$select WHERE itemnumber = ?");
         $sth->execute($item);
@@ -959,7 +987,7 @@ sub CheckReserves {
     return if  ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;
 
     # Find this item in the reserves
     return if  ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;
 
     # Find this item in the reserves
-    my @reserves = _Findgroupreserve( $bibitem, $biblio, $itemnumber, $lookahead_days);
+    my @reserves = _Findgroupreserve( $bibitem, $biblio, $itemnumber, $lookahead_days, $ignore_borrowers);
 
     # $priority and $highest are used to find the most important item
     # in the list returned by &_Findgroupreserve. (The lower $priority,
 
     # $priority and $highest are used to find the most important item
     # in the list returned by &_Findgroupreserve. (The lower $priority,
@@ -1036,7 +1064,7 @@ sub CancelExpiredReserves {
     # Cancel reserves that have passed their expiration date.
     my $dbh = C4::Context->dbh;
     my $sth = $dbh->prepare( "
     # Cancel reserves that have passed their expiration date.
     my $dbh = C4::Context->dbh;
     my $sth = $dbh->prepare( "
-        SELECT * FROM reserves WHERE DATE(expirationdate) < DATE( CURDATE() ) 
+        SELECT * FROM reserves WHERE DATE(expirationdate) < DATE( CURDATE() )
         AND expirationdate IS NOT NULL
         AND found IS NULL
     " );
         AND expirationdate IS NOT NULL
         AND found IS NULL
     " );
@@ -1045,7 +1073,7 @@ sub CancelExpiredReserves {
     while ( my $res = $sth->fetchrow_hashref() ) {
         CancelReserve({ reserve_id => $res->{'reserve_id'} });
     }
     while ( my $res = $sth->fetchrow_hashref() ) {
         CancelReserve({ reserve_id => $res->{'reserve_id'} });
     }
-  
+
     # Cancel reserves that have been waiting too long
     if ( C4::Context->preference("ExpireReservesMaxPickUpDelay") ) {
         my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay");
     # Cancel reserves that have been waiting too long
     if ( C4::Context->preference("ExpireReservesMaxPickUpDelay") ) {
         my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay");
@@ -1171,12 +1199,12 @@ If C<$rank> is 'del', the hold request is cancelled.
 
 If C<$rank> is an integer greater than zero, the priority of
 the request is set to that value.  Since priority != 0 means
 
 If C<$rank> is an integer greater than zero, the priority of
 the request is set to that value.  Since priority != 0 means
-that the item is not waiting on the hold shelf, setting the 
+that the item is not waiting on the hold shelf, setting the
 priority to a non-zero value also sets the request's found
 priority to a non-zero value also sets the request's found
-status and waiting date to NULL. 
+status and waiting date to NULL.
 
 The optional C<$itemnumber> parameter is used only when
 
 The optional C<$itemnumber> parameter is used only when
-C<$rank> is a non-zero integer; if supplied, the itemnumber 
+C<$rank> is a non-zero integer; if supplied, the itemnumber
 of the hold request is set accordingly; if omitted, the itemnumber
 is cleared.
 
 of the hold request is set accordingly; if omitted, the itemnumber
 is cleared.
 
@@ -1288,11 +1316,11 @@ sub ModReserveFill {
                 ";
     $sth = $dbh->prepare($query);
     $sth->execute( $biblionumber, $resdate, $borrowernumber );
                 ";
     $sth = $dbh->prepare($query);
     $sth->execute( $biblionumber, $resdate, $borrowernumber );
-    
+
     # now fix the priority on the others (if the priority wasn't
     # already sorted!)....
     unless ( $priority == 0 ) {
     # now fix the priority on the others (if the priority wasn't
     # already sorted!)....
     unless ( $priority == 0 ) {
-        _FixPriority({ reserve_id => $reserve_id });
+        _FixPriority({ reserve_id => $reserve_id, biblionumber => $biblionumber });
     }
 }
 
     }
 }
 
@@ -1333,7 +1361,7 @@ with the biblionumber & the borrowernumber, we can affect the itemnumber
 to the correct reserve.
 
 if $transferToDo is not set, then the status is set to "Waiting" as well.
 to the correct reserve.
 
 if $transferToDo is not set, then the status is set to "Waiting" as well.
-otherwise, a transfer is on the way, and the end of the transfer will 
+otherwise, a transfer is on the way, and the end of the transfer will
 take care of the waiting status
 
 =cut
 take care of the waiting status
 
 =cut
@@ -1494,22 +1522,18 @@ sub GetReserveInfo {
 
 =head2 IsAvailableForItemLevelRequest
 
 
 =head2 IsAvailableForItemLevelRequest
 
-  my $is_available = IsAvailableForItemLevelRequest($itemnumber);
+  my $is_available = IsAvailableForItemLevelRequest($item_record,$borrower_record);
 
 Checks whether a given item record is available for an
 item-level hold request.  An item is available if
 
 
 Checks whether a given item record is available for an
 item-level hold request.  An item is available if
 
-* it is not lost AND 
-* it is not damaged AND 
-* it is not withdrawn AND 
+* it is not lost AND
+* it is not damaged AND
+* it is not withdrawn AND
 * does not have a not for loan value > 0
 
 * does not have a not for loan value > 0
 
-Whether or not the item is currently on loan is 
-also checked - if the AllowOnShelfHolds system preference
-is ON, an item can be requested even if it is currently
-on loan to somebody else.  If the system preference
-is OFF, an item that is currently checked out cannot
-be the target of an item-level hold request.
+Need to check the issuingrules onshelfholds column,
+if this is set items on the shelf can be placed on hold
 
 Note that IsAvailableForItemLevelRequest() does not
 check if the staff operator is authorized to place
 
 Note that IsAvailableForItemLevelRequest() does not
 check if the staff operator is authorized to place
@@ -1520,48 +1544,80 @@ and canreservefromotherbranches.
 =cut
 
 sub IsAvailableForItemLevelRequest {
 =cut
 
 sub IsAvailableForItemLevelRequest {
-    my $itemnumber = shift;
-   
-    my $item = GetItem($itemnumber);
+    my $item = shift;
+    my $borrower = shift;
 
 
+    my $dbh = C4::Context->dbh;
     # must check the notforloan setting of the itemtype
     # FIXME - a lot of places in the code do this
     #         or something similar - need to be
     #         consolidated
     # must check the notforloan setting of the itemtype
     # FIXME - a lot of places in the code do this
     #         or something similar - need to be
     #         consolidated
-    my $dbh = C4::Context->dbh;
-    my $notforloan_query;
+    my $itype = _get_itype($item);
+    my $notforloan_per_itemtype
+      = $dbh->selectrow_array("SELECT notforloan FROM itemtypes WHERE itemtype = ?",
+                              undef, $itype);
+
+    return 0 if
+        $notforloan_per_itemtype ||
+        $item->{itemlost}        ||
+        $item->{notforloan} > 0  ||
+        $item->{withdrawn}        ||
+        ($item->{damaged} && !C4::Context->preference('AllowHoldsOnDamagedItems'));
+
+
+    return 1 if _OnShelfHoldsAllowed($itype,$borrower->{categorycode},$item->{holdingbranch});
+
+    return $item->{onloan} || GetReserveStatus($item->{itemnumber}) eq "Waiting";
+}
+
+=head2 OnShelfHoldsAllowed
+
+  OnShelfHoldsAllowed($itemtype,$borrowercategory,$branchcode);
+
+Checks issuingrules, using the borrowers categorycode, the itemtype, and branchcode to see if onshelf
+holds are allowed, returns true if so.
+
+=cut
+
+sub OnShelfHoldsAllowed {
+    my ($item, $borrower) = @_;
+
+    my $itype = _get_itype($item);
+    return _OnShelfHoldsAllowed($itype,$borrower->{categorycode},$item->{holdingbranch});
+}
+
+sub _get_itype {
+    my $item = shift;
+
+    my $itype;
     if (C4::Context->preference('item-level_itypes')) {
     if (C4::Context->preference('item-level_itypes')) {
-        $notforloan_query = "SELECT itemtypes.notforloan
-                             FROM items
-                             JOIN itemtypes ON (itemtypes.itemtype = items.itype)
-                             WHERE itemnumber = ?";
-    } else {
-        $notforloan_query = "SELECT itemtypes.notforloan
-                             FROM items
-                             JOIN biblioitems USING (biblioitemnumber)
-                             JOIN itemtypes USING (itemtype)
-                             WHERE itemnumber = ?";
+        # We cant trust GetItem to honour the syspref, so safest to do it ourselves
+        # When GetItem is fixed, we can remove this
+        $itype = $item->{itype};
     }
     }
-    my $sth = $dbh->prepare($notforloan_query);
-    $sth->execute($itemnumber);
-    my $notforloan_per_itemtype = 0;
-    if (my ($notforloan) = $sth->fetchrow_array) {
-        $notforloan_per_itemtype = 1 if $notforloan;
+    else {
+        # XXX This is a bit dodgy. It relies on biblio itemtype column having different name.
+        # So if we already have a biblioitems join when calling this function,
+        # we don't need to access the database again
+        $itype = $item->{itemtype};
+    }
+    unless ($itype) {
+        my $dbh = C4::Context->dbh;
+        my $query = "SELECT itemtype FROM biblioitems WHERE biblioitemnumber = ? ";
+        my $sth = $dbh->prepare($query);
+        $sth->execute($item->{biblioitemnumber});
+        if (my $data = $sth->fetchrow_hashref()){
+            $itype = $data->{itemtype};
+        }
     }
     }
+    return $itype;
+}
 
 
-    my $available_per_item = 1;
-    $available_per_item = 0 if $item->{itemlost} or
-                               ( $item->{notforloan} > 0 ) or
-                               ($item->{damaged} and not C4::Context->preference('AllowHoldsOnDamagedItems')) or
-                               $item->{withdrawn} or
-                               $notforloan_per_itemtype;
-
+sub _OnShelfHoldsAllowed {
+    my ($itype,$borrowercategory,$branchcode) = @_;
 
 
-    if (C4::Context->preference('AllowOnShelfHolds')) {
-        return $available_per_item;
-    } else {
-        return ($available_per_item and ($item->{onloan} or GetReserveStatus($itemnumber) eq "Waiting"));
-    }
+    my $rule = C4::Circulation::GetIssuingRule($borrowercategory, $itype, $branchcode);
+    return $rule->{onshelfholds};
 }
 
 =head2 AlterPriority
 }
 
 =head2 AlterPriority
@@ -1829,7 +1885,7 @@ sub _FixPriority {
     
     $sth = $dbh->prepare( "SELECT reserve_id FROM reserves WHERE lowestPriority = 1 ORDER BY priority" );
     $sth->execute();
     
     $sth = $dbh->prepare( "SELECT reserve_id FROM reserves WHERE lowestPriority = 1 ORDER BY priority" );
     $sth->execute();
-    
+
     unless ( $ignoreSetLowestRank ) {
       while ( my $res = $sth->fetchrow_hashref() ) {
         _FixPriority({
     unless ( $ignoreSetLowestRank ) {
       while ( my $res = $sth->fetchrow_hashref() ) {
         _FixPriority({
@@ -1843,7 +1899,7 @@ sub _FixPriority {
 
 =head2 _Findgroupreserve
 
 
 =head2 _Findgroupreserve
 
-  @results = &_Findgroupreserve($biblioitemnumber, $biblionumber, $itemnumber, $lookahead);
+  @results = &_Findgroupreserve($biblioitemnumber, $biblionumber, $itemnumber, $lookahead, $ignore_borrowers);
 
 Looks for an item-specific match first, then for a title-level match, returning the
 first match found.  If neither, then we look for a 3rd kind of match based on
 
 Looks for an item-specific match first, then for a title-level match, returning the
 first match found.  If neither, then we look for a 3rd kind of match based on
@@ -1860,12 +1916,12 @@ C<biblioitemnumber>.
 =cut
 
 sub _Findgroupreserve {
 =cut
 
 sub _Findgroupreserve {
-    my ( $bibitem, $biblio, $itemnumber, $lookahead) = @_;
+    my ( $bibitem, $biblio, $itemnumber, $lookahead, $ignore_borrowers) = @_;
     my $dbh   = C4::Context->dbh;
 
     # TODO: consolidate at least the SELECT portion of the first 2 queries to a common $select var.
     # check for exact targetted match
     my $dbh   = C4::Context->dbh;
 
     # TODO: consolidate at least the SELECT portion of the first 2 queries to a common $select var.
     # check for exact targetted match
-    my $item_level_target_query = qq/
+    my $item_level_target_query = qq{
         SELECT reserves.biblionumber        AS biblionumber,
                reserves.borrowernumber      AS borrowernumber,
                reserves.reservedate         AS reservedate,
         SELECT reserves.biblionumber        AS biblionumber,
                reserves.borrowernumber      AS borrowernumber,
                reserves.reservedate         AS reservedate,
@@ -1888,17 +1944,18 @@ sub _Findgroupreserve {
         AND reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY)
         AND suspend = 0
         ORDER BY priority
         AND reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY)
         AND suspend = 0
         ORDER BY priority
-    /;
+    };
     my $sth = $dbh->prepare($item_level_target_query);
     $sth->execute($itemnumber, $lookahead||0);
     my @results;
     if ( my $data = $sth->fetchrow_hashref ) {
     my $sth = $dbh->prepare($item_level_target_query);
     $sth->execute($itemnumber, $lookahead||0);
     my @results;
     if ( my $data = $sth->fetchrow_hashref ) {
-        push( @results, $data );
+        push( @results, $data )
+          unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ;
     }
     return @results if @results;
     }
     return @results if @results;
-    
+
     # check for title-level targetted match
     # check for title-level targetted match
-    my $title_level_target_query = qq/
+    my $title_level_target_query = qq{
         SELECT reserves.biblionumber        AS biblionumber,
                reserves.borrowernumber      AS borrowernumber,
                reserves.reservedate         AS reservedate,
         SELECT reserves.biblionumber        AS biblionumber,
                reserves.borrowernumber      AS borrowernumber,
                reserves.reservedate         AS reservedate,
@@ -1921,16 +1978,17 @@ sub _Findgroupreserve {
         AND reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY)
         AND suspend = 0
         ORDER BY priority
         AND reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY)
         AND suspend = 0
         ORDER BY priority
-    /;
+    };
     $sth = $dbh->prepare($title_level_target_query);
     $sth->execute($itemnumber, $lookahead||0);
     @results = ();
     if ( my $data = $sth->fetchrow_hashref ) {
     $sth = $dbh->prepare($title_level_target_query);
     $sth->execute($itemnumber, $lookahead||0);
     @results = ();
     if ( my $data = $sth->fetchrow_hashref ) {
-        push( @results, $data );
+        push( @results, $data )
+          unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ;
     }
     return @results if @results;
 
     }
     return @results if @results;
 
-    my $query = qq/
+    my $query = qq{
         SELECT reserves.biblionumber               AS biblionumber,
                reserves.borrowernumber             AS borrowernumber,
                reserves.reservedate                AS reservedate,
         SELECT reserves.biblionumber               AS biblionumber,
                reserves.borrowernumber             AS borrowernumber,
                reserves.reservedate                AS reservedate,
@@ -1955,12 +2013,13 @@ sub _Findgroupreserve {
           AND reserves.reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY)
           AND suspend = 0
           ORDER BY priority
           AND reserves.reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY)
           AND suspend = 0
           ORDER BY priority
-    /;
+    };
     $sth = $dbh->prepare($query);
     $sth->execute( $biblio, $bibitem, $itemnumber, $lookahead||0);
     @results = ();
     while ( my $data = $sth->fetchrow_hashref ) {
     $sth = $dbh->prepare($query);
     $sth->execute( $biblio, $bibitem, $itemnumber, $lookahead||0);
     @results = ();
     while ( my $data = $sth->fetchrow_hashref ) {
-        push( @results, $data );
+        push( @results, $data )
+          unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ;
     }
     return @results;
 }
     }
     return @results;
 }
@@ -1979,7 +2038,7 @@ sub _koha_notify_reserve {
 
     my $dbh = C4::Context->dbh;
     my $borrower = C4::Members::GetMember(borrowernumber => $borrowernumber);
 
     my $dbh = C4::Context->dbh;
     my $borrower = C4::Members::GetMember(borrowernumber => $borrowernumber);
-    
+
     # Try to get the borrower's email address
     my $to_address = C4::Members::GetNoticeEmailAddress($borrowernumber);
 
     # Try to get the borrower's email address
     my $to_address = C4::Members::GetNoticeEmailAddress($borrowernumber);
 
@@ -2034,10 +2093,12 @@ sub _koha_notify_reserve {
     };
 
     while ( my ( $mtt, $letter_code ) = each %{ $messagingprefs->{transports} } ) {
     };
 
     while ( my ( $mtt, $letter_code ) = each %{ $messagingprefs->{transports} } ) {
-        if ( ($mtt eq 'email' and not $to_address) or ($mtt eq 'sms' and not $borrower->{smsalertnumber}) ) {
-            # email or sms is requested but not exist
-            next;
-        }
+        next if (
+               ( $mtt eq 'email' and not $to_address ) # No email address
+            or ( $mtt eq 'sms'   and not $borrower->{smsalertnumber} ) # No SMS number
+            or ( $mtt eq 'phone' and C4::Context->preference('TalkingTechItivaPhoneNotification') ) # Notice is handled by TalkingTech_itiva_outbound.pl
+        );
+
         &$send_notification($mtt, $letter_code);
         $notification_sent++;
     }
         &$send_notification($mtt, $letter_code);
         $notification_sent++;
     }
@@ -2097,6 +2158,54 @@ sub _ShiftPriorityByDateAndPriority {
     return $new_priority;  # so the caller knows what priority they wind up receiving
 }
 
     return $new_priority;  # so the caller knows what priority they wind up receiving
 }
 
+=head2 OPACItemHoldsAllowed
+
+  OPACItemHoldsAllowed($item_record,$borrower_record);
+
+Checks issuingrules, using the borrowers categorycode, the itemtype, and branchcode to see
+if specific item holds are allowed, returns true if so.
+
+=cut
+
+sub OPACItemHoldsAllowed {
+    my ($item,$borrower) = @_;
+
+    my $branchcode = $item->{homebranch} or die "No homebranch";
+    my $itype;
+    my $dbh = C4::Context->dbh;
+    if (C4::Context->preference('item-level_itypes')) {
+       # We cant trust GetItem to honour the syspref, so safest to do it ourselves
+       # When GetItem is fixed, we can remove this
+       $itype = $item->{itype};
+    }
+    else {
+       my $query = "SELECT itemtype FROM biblioitems WHERE biblioitemnumber = ? ";
+       my $sth = $dbh->prepare($query);
+       $sth->execute($item->{biblioitemnumber});
+       if (my $data = $sth->fetchrow_hashref()){
+           $itype = $data->{itemtype};
+       }
+    }
+
+    my $query = "SELECT opacitemholds,categorycode,itemtype,branchcode FROM issuingrules WHERE
+          (issuingrules.categorycode = ? OR issuingrules.categorycode = '*')
+        AND
+          (issuingrules.itemtype = ? OR issuingrules.itemtype = '*')
+        AND
+          (issuingrules.branchcode = ? OR issuingrules.branchcode = '*')
+        ORDER BY
+          issuingrules.categorycode desc,
+          issuingrules.itemtype desc,
+          issuingrules.branchcode desc
+       LIMIT 1";
+    my $sth = $dbh->prepare($query);
+    $sth->execute($borrower->{categorycode},$itype,$branchcode);
+    my $data = $sth->fetchrow_hashref;
+    my $opacitemholds = uc substr ($data->{opacitemholds}, 0, 1);
+    return '' if $opacitemholds eq 'N';
+    return $opacitemholds;
+}
+
 =head2 MoveReserve
 
   MoveReserve( $itemnumber, $borrowernumber, $cancelreserve )
 =head2 MoveReserve
 
   MoveReserve( $itemnumber, $borrowernumber, $cancelreserve )
@@ -2394,6 +2503,30 @@ sub CalculatePriority {
     return @row ? $row[0]+1 : 1;
 }
 
     return @row ? $row[0]+1 : 1;
 }
 
+=head2 IsItemOnHoldAndFound
+
+    my $bool = IsItemFoundHold( $itemnumber );
+
+    Returns true if the item is currently on hold
+    and that hold has a non-null found status ( W, T, etc. )
+
+=cut
+
+sub IsItemOnHoldAndFound {
+    my ($itemnumber) = @_;
+
+    my $rs = Koha::Database->new()->schema()->resultset('Reserve');
+
+    my $found = $rs->count(
+        {
+            itemnumber => $itemnumber,
+            found      => { '!=' => undef }
+        }
+    );
+
+    return $found;
+}
+
 =head1 AUTHOR
 
 Koha Development Team <http://koha-community.org/>
 =head1 AUTHOR
 
 Koha Development Team <http://koha-community.org/>