projects / koha.git / blobdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | inline | side by side
Bug 12748: Code tidy
[koha.git] / C4 / Reserves.pm
diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index ea46f2a..b83f8e1 100644 (file)
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -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;
@@ -34,14 +34,20 @@ use C4::Accounts;
 use C4::Members::Messaging;
 use C4::Members qw();
 use C4::Letters;
 use C4::Members::Messaging;
 use C4::Members qw();
 use C4::Letters;
-use C4::Branch qw( GetBranchDetail );
-use C4::Dates qw( format_date_in_iso );
 
 use Koha::DateUtils;
 
 use Koha::DateUtils;
+use Koha::Calendar;
+use Koha::Database;
+use Koha::Hold;
+use Koha::Holds;
+use Koha::Libraries;
+use Koha::Items;
+use Koha::ItemTypes;
 
 
-use List::MoreUtils qw( firstidx );
+use List::MoreUtils qw( firstidx any );
+use Carp;
 
 
-use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
+use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
 
 =head1 NAME
 
 
 =head1 NAME
 
@@ -59,7 +65,7 @@ This modules provides somes functions to deal with reservations.
   The following columns contains important values :
   - priority >0      : then the reserve is at 1st stage, and not yet affected to any item.
              =0      : then the reserve is being dealed
   The following columns contains important values :
   - priority >0      : then the reserve is at 1st stage, and not yet affected to any item.
              =0      : then the reserve is being dealed
-  - found : NULL       : means the patron requested the 1st available, and we haven't choosen the item
+  - found : NULL       : means the patron requested the 1st available, and we haven't chosen the item
             T(ransit)  : the reserve is linked to an item but is in transit to the pickup branch
             W(aiting)  : the reserve is linked to an item, is at the pickup branch, and is waiting on the hold shelf
             F(inished) : the reserve has been completed, and is done
             T(ransit)  : the reserve is linked to an item but is in transit to the pickup branch
             W(aiting)  : the reserve is linked to an item, is at the pickup branch, and is waiting on the hold shelf
             F(inished) : the reserve has been completed, and is done
@@ -68,13 +74,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
          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 pickup library receive the book, it check in       P =0, F=W,    I=filled
   The patron borrow the book                                      P =0, F=F,    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
@@ -87,8 +93,6 @@ This modules provides somes functions to deal with reservations.
 =cut
 
 BEGIN {
 =cut
 
 BEGIN {
-    # set the version for version checking
-    $VERSION = 3.07.00.049;
     require Exporter;
     @ISA = qw(Exporter);
     @EXPORT = qw(
     require Exporter;
     @ISA = qw(Exporter);
     @EXPORT = qw(
@@ -101,12 +105,11 @@ BEGIN {
         &GetReservesForBranch
         &GetReservesToBranch
         &GetReserveCount
         &GetReservesForBranch
         &GetReservesToBranch
         &GetReserveCount
-        &GetReserveFee
         &GetReserveInfo
         &GetReserveStatus
         &GetReserveInfo
         &GetReserveStatus
-        
+
         &GetOtherReserves
         &GetOtherReserves
-        
+
         &ModReserveFill
         &ModReserveAffect
         &ModReserve
         &ModReserveFill
         &ModReserveAffect
         &ModReserve
@@ -114,17 +117,20 @@ BEGIN {
         &ModReserveCancelAll
         &ModReserveMinusPriority
         &MoveReserve
         &ModReserveCancelAll
         &ModReserveMinusPriority
         &MoveReserve
-        
+
         &CheckReserves
         &CanBookBeReserved
         &CheckReserves
         &CanBookBeReserved
-       &CanItemBeReserved
+        &CanItemBeReserved
+        &CanReserveBeCanceledFromOpac
         &CancelReserve
         &CancelExpiredReserves
 
         &AutoUnsuspendReserves
 
         &IsAvailableForItemLevelRequest
         &CancelReserve
         &CancelExpiredReserves
 
         &AutoUnsuspendReserves
 
         &IsAvailableForItemLevelRequest
-        
+
+        &OPACItemHoldsAllowed
+
         &AlterPriority
         &ToggleLowestPriority
 
         &AlterPriority
         &ToggleLowestPriority
 
@@ -133,38 +139,53 @@ BEGIN {
         &SuspendAll
 
         &GetReservesControlBranch
         &SuspendAll
 
         &GetReservesControlBranch
+
+        IsItemOnHoldAndFound
     );
     @EXPORT_OK = qw( MergeHolds );
     );
     @EXPORT_OK = qw( MergeHolds );
-}    
+}
 
 =head2 AddReserve
 
 
 =head2 AddReserve
 
-    AddReserve($branch,$borrowernumber,$biblionumber,$constraint,$bibitems,$priority,$resdate,$expdate,$notes,$title,$checkitem,$found)
+    AddReserve($branch,$borrowernumber,$biblionumber,$bibitems,$priority,$resdate,$expdate,$notes,$title,$checkitem,$found)
+
+Adds reserve and generates HOLDPLACED message.
+
+The following tables are available witin the HOLDPLACED message:
+
+    branches
+    borrowers
+    biblio
+    biblioitems
+    items
 
 =cut
 
 sub AddReserve {
     my (
 
 =cut
 
 sub AddReserve {
     my (
-        $branch,    $borrowernumber, $biblionumber,
-        $constraint, $bibitems,  $priority, $resdate, $expdate, $notes,
-        $title,      $checkitem, $found
+        $branch,   $borrowernumber, $biblionumber, $bibitems,
+        $priority, $resdate,        $expdate,      $notes,
+        $title,    $checkitem,      $found,        $itemtype
     ) = @_;
     ) = @_;
-    my $fee =
-          GetReserveFee($borrowernumber, $biblionumber, $constraint,
-            $bibitems );
-    my $dbh     = C4::Context->dbh;
-    my $const   = lc substr( $constraint, 0, 1 );
-    $resdate = format_date_in_iso( $resdate ) if ( $resdate );
-    $resdate = C4::Dates->today( 'iso' ) unless ( $resdate );
-    if ($expdate) {
-        $expdate = format_date_in_iso( $expdate );
-    } else {
-        undef $expdate; # make reserves.expirationdate default to null rather than '0000-00-00'
+
+    if ( Koha::Holds->search( { borrowernumber => $borrowernumber, biblionumber => $biblionumber } )->count() > 0 ) {
+        carp("AddReserve: borrower $borrowernumber already has a hold for biblionumber $biblionumber");
+        return;
     }
     }
-    if ( C4::Context->preference( 'AllowHoldDateInFuture' ) ) {
-       # Make room in reserves for this before those of a later reserve date
-       $priority = _ShiftPriorityByDateAndPriority( $biblionumber, $resdate, $priority );
+
+    my $dbh     = C4::Context->dbh;
+
+    $resdate = output_pref( { str => dt_from_string( $resdate ), dateonly => 1, dateformat => 'iso' })
+        or output_pref({ dt => dt_from_string, dateonly => 1, dateformat => 'iso' });
+
+    $expdate = output_pref({ str => $expdate, dateonly => 1, dateformat => 'iso' });
+
+    if ( C4::Context->preference('AllowHoldDateInFuture') ) {
+
+        # Make room in reserves for this before those of a later reserve date
+        $priority = _ShiftPriorityByDateAndPriority( $biblionumber, $resdate, $priority );
     }
     }
+
     my $waitingdate;
 
     # If the reserv had the waiting status, we had the value of the resdate
     my $waitingdate;
 
     # If the reserv had the waiting status, we had the value of the resdate
@@ -172,54 +193,51 @@ sub AddReserve {
         $waitingdate = $resdate;
     }
 
         $waitingdate = $resdate;
     }
 
-    #eval {
+    # Don't add itemtype limit if specific item is selected
+    $itemtype = undef if $checkitem;
+
     # updates take place here
     # updates take place here
-    if ( $fee > 0 ) {
-        my $nextacctno = &getnextacctno( $borrowernumber );
-        my $query      = qq/
-        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 $query = qq/
-        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 $hold = Koha::Hold->new(
+        {
+            borrowernumber => $borrowernumber,
+            biblionumber   => $biblionumber,
+            reservedate    => $resdate,
+            branchcode     => $branch,
+            priority       => $priority,
+            reservenotes   => $notes,
+            itemnumber     => $checkitem,
+            found          => $found,
+            waitingdate    => $waitingdate,
+            expirationdate => $expdate,
+            itemtype       => $itemtype,
+        }
+    )->store();
+    my $reserve_id = $hold->id();
+
+    # add a reserve fee if needed
+    my $fee = GetReserveFee( $borrowernumber, $biblionumber );
+    ChargeReserveFee( $borrowernumber, $fee, $title );
+
+    _FixPriority({ biblionumber => $biblionumber});
 
     # Send e-mail to librarian if syspref is active
     if(C4::Context->preference("emailLibrarianWhenHoldIsPlaced")){
         my $borrower = C4::Members::GetMember(borrowernumber => $borrowernumber);
 
     # Send e-mail to librarian if syspref is active
     if(C4::Context->preference("emailLibrarianWhenHoldIsPlaced")){
         my $borrower = C4::Members::GetMember(borrowernumber => $borrowernumber);
-        my $branch_details = C4::Branch::GetBranchDetail($borrower->{branchcode});
+        my $library = Koha::Libraries->find($borrower->{branchcode})->unblessed;
         if ( my $letter =  C4::Letters::GetPreparedLetter (
             module => 'reserves',
             letter_code => 'HOLDPLACED',
             branchcode => $branch,
             tables => {
         if ( my $letter =  C4::Letters::GetPreparedLetter (
             module => 'reserves',
             letter_code => 'HOLDPLACED',
             branchcode => $branch,
             tables => {
-                'branches'  => $branch_details,
-                'borrowers' => $borrower,
-                'biblio'    => $biblionumber,
-                'items'     => $checkitem,
+                'branches'    => $library,
+                'borrowers'   => $borrower,
+                'biblio'      => $biblionumber,
+                'biblioitems' => $biblionumber,
+                'items'       => $checkitem,
             },
         ) ) {
 
             },
         ) ) {
 
-            my $admin_email_address =$branch_details->{'branchemail'} || C4::Context->preference('KohaAdminEmailAddress');
+            my $admin_email_address = $library->{'branchemail'} || C4::Context->preference('KohaAdminEmailAddress');
 
             C4::Letters::EnqueueLetter(
                 {   letter                 => $letter,
 
             C4::Letters::EnqueueLetter(
                 {   letter                 => $letter,
@@ -232,20 +250,7 @@ sub AddReserve {
         }
     }
 
         }
     }
 
-    #}
-    ($const eq "o" || $const eq "e") or return;   # FIXME: why not have a useful return value?
-    $query = qq/
-        INSERT INTO reserveconstraints
-            (borrowernumber,biblionumber,reservedate,biblioitemnumber)
-        VALUES
-            (?,?,?,?)
-    /;
-    $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
@@ -260,6 +265,7 @@ sub GetReserve {
     my ($reserve_id) = @_;
 
     my $dbh = C4::Context->dbh;
     my ($reserve_id) = @_;
 
     my $dbh = C4::Context->dbh;
+
     my $query = "SELECT * FROM reserves WHERE reserve_id = ?";
     my $sth = $dbh->prepare( $query );
     $sth->execute( $reserve_id );
     my $query = "SELECT * FROM reserves WHERE reserve_id = ?";
     my $sth = $dbh->prepare( $query );
     $sth->execute( $reserve_id );
@@ -268,17 +274,30 @@ sub GetReserve {
 
 =head2 GetReservesFromBiblionumber
 
 
 =head2 GetReservesFromBiblionumber
 
-  ($count, $title_reserves) = GetReservesFromBiblionumber($biblionumber);
+  my $reserves = GetReservesFromBiblionumber({
+    biblionumber => $biblionumber,
+    [ itemnumber => $itemnumber, ]
+    [ all_dates => 1|0 ]
+  });
+
+This function gets the list of reservations for one C<$biblionumber>,
+returning an arrayref pointing to the reserves for C<$biblionumber>.
 
 
-This function gets the list of reservations for one C<$biblionumber>, returning a count
-of the reserves and an arrayref pointing to the reserves for C<$biblionumber>.
+By default, only reserves whose start date falls before the current
+time are returned.  To return all reserves, including future ones,
+the C<all_dates> parameter can be included and set to a true value.
+
+If the C<itemnumber> parameter is supplied, reserves must be targeted
+to that item or not targeted to any item at all; otherwise, they
+are excluded from the list.
 
 =cut
 
 sub GetReservesFromBiblionumber {
 
 =cut
 
 sub GetReservesFromBiblionumber {
-    my ($biblionumber) = shift or return (0, []);
-    my ($all_dates) = shift;
-    my ($itemnumber) = shift;
+    my ( $params ) = @_;
+    my $biblionumber = $params->{biblionumber} or return [];
+    my $itemnumber = $params->{itemnumber};
+    my $all_dates = $params->{all_dates} // 0;
     my $dbh   = C4::Context->dbh;
 
     # Find the desired items in the reserves
     my $dbh   = C4::Context->dbh;
 
     # Find the desired items in the reserves
@@ -291,14 +310,14 @@ sub GetReservesFromBiblionumber {
                 biblionumber,
                 borrowernumber,
                 reservedate,
                 biblionumber,
                 borrowernumber,
                 reservedate,
-                constrainttype,
                 found,
                 itemnumber,
                 reservenotes,
                 expirationdate,
                 lowestPriority,
                 suspend,
                 found,
                 itemnumber,
                 reservenotes,
                 expirationdate,
                 lowestPriority,
                 suspend,
-                suspend_until
+                suspend_until,
+                itemtype
         FROM     reserves
         WHERE biblionumber = ? ";
     push( @params, $biblionumber );
         FROM     reserves
         WHERE biblionumber = ? ";
     push( @params, $biblionumber );
@@ -313,47 +332,10 @@ sub GetReservesFromBiblionumber {
     my $sth = $dbh->prepare($query);
     $sth->execute( @params );
     my @results;
     my $sth = $dbh->prepare($query);
     $sth->execute( @params );
     my @results;
-    my $i = 0;
     while ( my $data = $sth->fetchrow_hashref ) {
     while ( my $data = $sth->fetchrow_hashref ) {
-
-        # FIXME - What is this doing? How do constraints work?
-        if ($data->{constrainttype} eq 'o') {
-            $query = '
-                SELECT biblioitemnumber
-                FROM  reserveconstraints
-                WHERE  biblionumber   = ?
-                AND   borrowernumber = ?
-                AND   reservedate    = ?
-            ';
-            my $csth = $dbh->prepare($query);
-            $csth->execute($data->{biblionumber}, $data->{borrowernumber}, $data->{reservedate});
-            my @bibitemno;
-            while ( my $bibitemnos = $csth->fetchrow_array ) {
-                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 ( $count > 1 ) {
-                $bdata = GetBiblioItemData( $bibitemno[$i] );   # FIXME: This doesn't make sense.
-                $i++; #  $i can increase each pass, but the next @bibitemno might be smaller?
-            }
-            else {
-                # Look up the book we just found.
-                $bdata = GetBiblioItemData( $bibitemno[0] );
-            }
-            # Add the results of this latest search to the current
-            # results.
-            # FIXME - An 'each' would probably be more efficient.
-            foreach my $key ( keys %$bdata ) {
-                $data->{$key} = $bdata->{$key};
-            }
-        }
         push @results, $data;
     }
         push @results, $data;
     }
-    return ( $#results + 1, \@results );
+    return \@results;
 }
 
 =head2 GetReservesFromItemnumber
 }
 
 =head2 GetReservesFromItemnumber
@@ -367,19 +349,33 @@ The routine does not look at future reserves (read: item level holds), but DOES
 =cut
 
 sub GetReservesFromItemnumber {
 =cut
 
 sub GetReservesFromItemnumber {
-    my ( $itemnumber ) = @_;
-    my $dbh   = C4::Context->dbh;
-    my $query = "
-    SELECT reservedate,borrowernumber,branchcode,reserve_id,waitingdate
-    FROM   reserves
-    WHERE  itemnumber=? AND ( reservedate <= CURRENT_DATE() OR
-           waitingdate IS NOT NULL )
-    ORDER BY priority
-    ";
-    my $sth_res = $dbh->prepare($query);
-    $sth_res->execute($itemnumber);
-    my ( $reservedate, $borrowernumber,$branchcode, $reserve_id, $wait ) = $sth_res->fetchrow_array;
-    return ( $reservedate, $borrowernumber, $branchcode, $reserve_id, $wait );
+    my ($itemnumber) = @_;
+
+    my $schema = Koha::Database->new()->schema();
+
+    my $r = $schema->resultset('Reserve')->search(
+        {
+            itemnumber => $itemnumber,
+            suspend    => 0,
+            -or        => [
+                reservedate => \'<= CAST( NOW() AS DATE )',
+                waitingdate => { '!=', undef }
+            ]
+        },
+        {
+            order_by => 'priority',
+        }
+    )->first();
+
+    return unless $r;
+
+    return (
+        $r->reservedate(),
+        $r->get_column('borrowernumber'),
+        $r->get_column('branchcode'),
+        $r->reserve_id(),
+        $r->waitingdate(),
+    );
 }
 
 =head2 GetReservesFromBorrowernumber
 }
 
 =head2 GetReservesFromBorrowernumber
@@ -415,10 +411,13 @@ sub GetReservesFromBorrowernumber {
     my $data = $sth->fetchall_arrayref({});
     return @$data;
 }
     my $data = $sth->fetchall_arrayref({});
     return @$data;
 }
-#-------------------------------------------------------------------------------------
+
 =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
 
@@ -432,46 +431,63 @@ 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 $dbh             = C4::Context->dbh;
+    my $ruleitemtype; # itemtype of the matching issuing rule
     my $allowedreserves = 0;
             
     my $allowedreserves = 0;
             
+    # 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
+    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 $controlbranch = C4::Context->preference('ReservesControlBranch');
     my $controlbranch = C4::Context->preference('ReservesControlBranch');
-    my $itype         = C4::Context->preference('item-level_itypes') ? "itype" : "itemtype";
 
 
-    # we retrieve borrowers and items informations #
-    my $item     = GetItem($itemnumber);
-    my $borrower = C4::Members::GetMember('borrowernumber'=>$borrowernumber);     
-    
     # we retrieve user rights on this itemtype and branchcode
     # 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)
@@ -481,11 +497,9 @@ sub CanItemBeReserved{
                                 ";
     
     
                                 ";
     
     
-    my $itemtype     = $item->{$itype};
-    my $categorycode = $borrower->{categorycode};
     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};
@@ -495,35 +509,56 @@ sub CanItemBeReserved{
     }
     
     # we retrieve rights 
     }
     
     # we retrieve rights 
-    $sth->execute($categorycode, $itemtype, $branchcode);
+    $sth->execute($borrower->{'categorycode'}, $item->{'itype'}, $branchcode);
     if(my $rights = $sth->fetchrow_hashref()){
     if(my $rights = $sth->fetchrow_hashref()){
-        $itemtype        = $rights->{itemtype};
+        $ruleitemtype    = $rights->{itemtype};
         $allowedreserves = $rights->{reservesallowed}; 
     }else{
         $allowedreserves = $rights->{reservesallowed}; 
     }else{
-        $itemtype = '*';
+        $ruleitemtype = '*';
     }
     }
-    
+
     # we retrieve count
     # we retrieve count
-    
+
     $querycount .= "AND $branchfield = ?";
     
     $querycount .= "AND $branchfield = ?";
     
-    $querycount .= " AND $itype = ?" if ($itemtype ne "*");
+    # If using item-level itypes, fall back to the record
+    # level itemtype if the hold has no associated item
+    $querycount .=
+      C4::Context->preference('item-level_itypes')
+      ? " AND COALESCE( items.itype, biblioitems.itemtype ) = ?"
+      : " AND biblioitems.itemtype = ?"
+      if ( $ruleitemtype ne "*" );
+
     my $sthcount = $dbh->prepare($querycount);
     
     my $sthcount = $dbh->prepare($querycount);
     
-    if($itemtype eq "*"){
+    if($ruleitemtype eq "*"){
         $sthcount->execute($borrowernumber, $branchcode);
     }else{
         $sthcount->execute($borrowernumber, $branchcode);
     }else{
-        $sthcount->execute($borrowernumber, $branchcode, $itemtype);
+        $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
@@ -533,13 +568,36 @@ 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 'OK';
+}
+
+=head2 CanReserveBeCanceledFromOpac
+
+    $number = CanReserveBeCanceledFromOpac($reserve_id, $borrowernumber);
+
+    returns 1 if reserve can be cancelled by user from OPAC.
+    First check if reserve belongs to user, next checks if reserve is not in
+    transfer or waiting status
+
+=cut
+
+sub CanReserveBeCanceledFromOpac {
+    my ($reserve_id, $borrowernumber) = @_;
+
+    return unless $reserve_id and $borrowernumber;
+    my $reserve = GetReserve($reserve_id);
+
+    return 0 unless $reserve->{borrowernumber} == $borrowernumber;
+    return 0 if ( $reserve->{found} eq 'W' ) or ( $reserve->{found} eq 'T' );
+
     return 1;
     return 1;
+
 }
 }
-#--------------------------------------------------------------------------------
+
 =head2 GetReserveCount
 
   $number = &GetReserveCount($borrowernumber);
 =head2 GetReserveCount
 
   $number = &GetReserveCount($borrowernumber);
@@ -568,7 +626,7 @@ sub GetReserveCount {
 
   ($messages,$nextreservinfo)=$GetOtherReserves(itemnumber);
 
 
   ($messages,$nextreservinfo)=$GetOtherReserves(itemnumber);
 
-Check queued list of this document and check if this document must be  transfered
+Check queued list of this document and check if this document must be transferred
 
 =cut
 
 
 =cut
 
@@ -612,102 +670,62 @@ sub GetOtherReserves {
     return ( $messages, $nextreservinfo );
 }
 
     return ( $messages, $nextreservinfo );
 }
 
-=head2 GetReserveFee
+=head2 ChargeReserveFee
 
 
-  $fee = GetReserveFee($borrowernumber,$biblionumber,$constraint,$biblionumber);
+    $fee = ChargeReserveFee( $borrowernumber, $fee, $title );
 
 
-Calculate the fee for a reserve
+    Charge the fee for a reserve (if $fee > 0)
 
 =cut
 
 
 =cut
 
-sub GetReserveFee {
-    my ($borrowernumber, $biblionumber, $constraint, $bibitems ) = @_;
+sub ChargeReserveFee {
+    my ( $borrowernumber, $fee, $title ) = @_;
+    return if !$fee || $fee==0; # the last test is needed to include 0.00
+    my $accquery = qq{
+INSERT INTO accountlines ( borrowernumber, accountno, date, amount, description, accounttype, amountoutstanding ) VALUES (?, ?, NOW(), ?, ?, 'Res', ?)
+    };
+    my $dbh = C4::Context->dbh;
+    my $nextacctno = &getnextacctno( $borrowernumber );
+    $dbh->do( $accquery, undef, ( $borrowernumber, $nextacctno, $fee, "Reserve Charge - $title", $fee ) );
+}
 
 
-    #check for issues;
-    my $dbh   = C4::Context->dbh;
-    my $const = lc substr( $constraint, 0, 1 );
-    my $query = qq/
-      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 $fee      = $data->{'reservefee'};
-    my $cntitems = @- > $bibitems;
+=head2 GetReserveFee
 
 
-    if ( $fee > 0 ) {
+    $fee = GetReserveFee( $borrowernumber, $biblionumber );
 
 
-        # check for items on issue
-        # first find biblioitem records
-        my @biblioitems;
-        my $sth1 = $dbh->prepare(
-            "SELECT * FROM biblio LEFT JOIN biblioitems on biblio.biblionumber = biblioitems.biblionumber
-                   WHERE (biblio.biblionumber = ?)"
-        );
-        $sth1->execute($biblionumber);
-        while ( my $data1 = $sth1->fetchrow_hashref ) {
-            if ( $const eq "a" ) {
-                push @biblioitems, $data1;
-            }
-            else {
-                my $found = 0;
-                my $x     = 0;
-                while ( $x < $cntitems ) {
-                    if ( @$bibitems->{'biblioitemnumber'} ==
-                        $data->{'biblioitemnumber'} )
-                    {
-                        $found = 1;
-                    }
-                    $x++;
-                }
-                if ( $const eq 'o' ) {
-                    if ( $found == 1 ) {
-                        push @biblioitems, $data1;
-                    }
-                }
-                else {
-                    if ( $found == 0 ) {
-                        push @biblioitems, $data1;
-                    }
-                }
-            }
-        }
-        my $cntitemsfound = @biblioitems;
-        my $issues        = 0;
-        my $x             = 0;
-        my $allissued     = 1;
-        while ( $x < $cntitemsfound ) {
-            my $bitdata = $biblioitems[$x];
-            my $sth2    = $dbh->prepare(
-                "SELECT * FROM items
-                     WHERE biblioitemnumber = ?"
-            );
-            $sth2->execute( $bitdata->{'biblioitemnumber'} );
-            while ( my $itdata = $sth2->fetchrow_hashref ) {
-                my $sth3 = $dbh->prepare(
-                    "SELECT * FROM issues
-                       WHERE itemnumber = ?"
-                );
-                $sth3->execute( $itdata->{'itemnumber'} );
-                if ( my $isdata = $sth3->fetchrow_hashref ) {
-                }
-                else {
-                    $allissued = 0;
-                }
-            }
-            $x++;
-        }
-        if ( $allissued == 0 ) {
-            my $rsth =
-              $dbh->prepare("SELECT * FROM reserves WHERE biblionumber = ?");
-            $rsth->execute($biblionumber);
-            if ( my $rdata = $rsth->fetchrow_hashref ) {
-            }
-            else {
-                $fee = 0;
-            }
+    Calculate the fee for a reserve (if applicable).
+
+=cut
+
+sub GetReserveFee {
+    my ( $borrowernumber, $biblionumber ) = @_;
+    my $borquery = qq{
+SELECT reservefee FROM borrowers LEFT JOIN categories ON borrowers.categorycode = categories.categorycode WHERE borrowernumber = ?
+    };
+    my $issue_qry = qq{
+SELECT COUNT(*) FROM items
+LEFT JOIN issues USING (itemnumber)
+WHERE items.biblionumber=? AND issues.issue_id IS NULL
+    };
+    my $holds_qry = qq{
+SELECT COUNT(*) FROM reserves WHERE biblionumber=? AND borrowernumber<>?
+    };
+
+    my $dbh = C4::Context->dbh;
+    my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) );
+    my $hold_fee_mode = C4::Context->preference('HoldFeeMode') || 'not_always';
+    if( $fee and $fee > 0 and $hold_fee_mode ne 'always' ) {
+        # This is a reconstruction of the old code:
+        # Compare number of items with items issued, and optionally check holds
+        # If not all items are issued and there are no holds: charge no fee
+        # NOTE: Lost, damaged, not-for-loan, etc. are just ignored here
+        my ( $notissued, $reserved );
+        ( $notissued ) = $dbh->selectrow_array( $issue_qry, undef,
+            ( $biblionumber ) );
+        if( $notissued ) {
+            ( $reserved ) = $dbh->selectrow_array( $holds_qry, undef,
+                ( $biblionumber, $borrowernumber ) );
+            $fee = 0 if $reserved == 0;
         }
     }
     return $fee;
         }
     }
     return $fee;
@@ -777,9 +795,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
@@ -789,7 +807,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;
 
@@ -800,12 +818,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';
@@ -846,7 +858,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;
@@ -856,7 +868,10 @@ sub CheckReserves {
            items.biblioitemnumber,
            itemtypes.notforloan,
            items.notforloan AS itemnotforloan,
            items.biblioitemnumber,
            itemtypes.notforloan,
            items.notforloan AS itemnotforloan,
-           items.itemnumber
+           items.itemnumber,
+           items.damaged,
+           items.homebranch,
+           items.holdingbranch
            FROM   items
            LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
            LEFT JOIN itemtypes   ON items.itype   = itemtypes.itemtype
            FROM   items
            LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
            LEFT JOIN itemtypes   ON items.itype   = itemtypes.itemtype
@@ -868,13 +883,16 @@ sub CheckReserves {
            items.biblioitemnumber,
            itemtypes.notforloan,
            items.notforloan AS itemnotforloan,
            items.biblioitemnumber,
            itemtypes.notforloan,
            items.notforloan AS itemnotforloan,
-           items.itemnumber
+           items.itemnumber,
+           items.damaged,
+           items.homebranch,
+           items.holdingbranch
            FROM   items
            LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
            LEFT JOIN itemtypes   ON biblioitems.itemtype   = itemtypes.itemtype
         ";
     }
            FROM   items
            LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
            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);
@@ -884,16 +902,18 @@ sub CheckReserves {
         $sth->execute($barcode);
     }
     # note: we get the itemnumber because we might have started w/ just the barcode.  Now we know for sure we have it.
         $sth->execute($barcode);
     }
     # note: we get the itemnumber because we might have started w/ just the barcode.  Now we know for sure we have it.
-    my ( $biblio, $bibitem, $notforloan_per_itemtype, $notforloan_per_item, $itemnumber ) = $sth->fetchrow_array;
+    my ( $biblio, $bibitem, $notforloan_per_itemtype, $notforloan_per_item, $itemnumber, $damaged, $item_homebranch, $item_holdingbranch ) = $sth->fetchrow_array;
 
 
-    return ( '' ) unless $itemnumber; # bail if we got nothing.
+    return if ( $damaged && !C4::Context->preference('AllowHoldsOnDamagedItems') );
+
+    return unless $itemnumber; # bail if we got nothing.
 
     # if item is not for loan it cannot be reserved either.....
 
     # if item is not for loan it cannot be reserved either.....
-    #    execpt where items.notforloan < 0 :  This indicates the item is holdable. 
-    return ( '' ) if  ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;
+    # except where items.notforloan < 0 :  This indicates the item is holdable.
+    return if  ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;
 
     # Find this item in the reserves
 
     # 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,
@@ -901,21 +921,49 @@ sub CheckReserves {
     # $highest is the most important item we've seen so far.
     my $highest;
     if (scalar @reserves) {
     # $highest is the most important item we've seen so far.
     my $highest;
     if (scalar @reserves) {
+        my $LocalHoldsPriority = C4::Context->preference('LocalHoldsPriority');
+        my $LocalHoldsPriorityPatronControl = C4::Context->preference('LocalHoldsPriorityPatronControl');
+        my $LocalHoldsPriorityItemControl = C4::Context->preference('LocalHoldsPriorityItemControl');
+
         my $priority = 10000000;
         foreach my $res (@reserves) {
             if ( $res->{'itemnumber'} == $itemnumber && $res->{'priority'} == 0) {
                 return ( "Waiting", $res, \@reserves ); # Found it
             } else {
         my $priority = 10000000;
         foreach my $res (@reserves) {
             if ( $res->{'itemnumber'} == $itemnumber && $res->{'priority'} == 0) {
                 return ( "Waiting", $res, \@reserves ); # Found it
             } else {
+                my $borrowerinfo;
+                my $iteminfo;
+                my $local_hold_match;
+
+                if ($LocalHoldsPriority) {
+                    $borrowerinfo = C4::Members::GetMember( borrowernumber => $res->{'borrowernumber'} );
+                    $iteminfo = C4::Items::GetItem($itemnumber);
+
+                    my $local_holds_priority_item_branchcode =
+                      $iteminfo->{$LocalHoldsPriorityItemControl};
+                    my $local_holds_priority_patron_branchcode =
+                      ( $LocalHoldsPriorityPatronControl eq 'PickupLibrary' )
+                      ? $res->{branchcode}
+                      : ( $LocalHoldsPriorityPatronControl eq 'HomeLibrary' )
+                      ? $borrowerinfo->{branchcode}
+                      : undef;
+                    $local_hold_match =
+                      $local_holds_priority_item_branchcode eq
+                      $local_holds_priority_patron_branchcode;
+                }
+
                 # See if this item is more important than what we've got so far
                 # See if this item is more important than what we've got so far
-                if ( $res->{'priority'} && $res->{'priority'} < $priority ) {
-                    my $borrowerinfo=C4::Members::GetMember(borrowernumber => $res->{'borrowernumber'});
-                    my $iteminfo=C4::Items::GetItem($itemnumber);
+                if ( ( $res->{'priority'} && $res->{'priority'} < $priority ) || $local_hold_match ) {
+                    $iteminfo ||= C4::Items::GetItem($itemnumber);
+                    next if $res->{itemtype} && $res->{itemtype} ne _get_itype( $iteminfo );
+                    $borrowerinfo ||= C4::Members::GetMember( borrowernumber => $res->{'borrowernumber'} );
                     my $branch = GetReservesControlBranch( $iteminfo, $borrowerinfo );
                     my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$iteminfo->{'itype'});
                     next if ($branchitemrule->{'holdallowed'} == 0);
                     next if (($branchitemrule->{'holdallowed'} == 1) && ($branch ne $borrowerinfo->{'branchcode'}));
                     my $branch = GetReservesControlBranch( $iteminfo, $borrowerinfo );
                     my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$iteminfo->{'itype'});
                     next if ($branchitemrule->{'holdallowed'} == 0);
                     next if (($branchitemrule->{'holdallowed'} == 1) && ($branch ne $borrowerinfo->{'branchcode'}));
+                    next if ( ($branchitemrule->{hold_fulfillment_policy} ne 'any') && ($res->{branchcode} ne $iteminfo->{ $branchitemrule->{hold_fulfillment_policy} }) );
                     $priority = $res->{'priority'};
                     $highest  = $res;
                     $priority = $res->{'priority'};
                     $highest  = $res;
+                    last if $local_hold_match;
                 }
             }
         }
                 }
             }
         }
@@ -944,7 +992,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
     " );
@@ -953,22 +1001,32 @@ 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");
-        my $charge = C4::Context->preference("ExpireReservesMaxPickUpDelayCharge");
+        my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays');
+
+        my $today = dt_from_string();
 
         my $query = "SELECT * FROM reserves WHERE TO_DAYS( NOW() ) - TO_DAYS( waitingdate ) > ? AND found = 'W' AND priority = 0";
         $sth = $dbh->prepare( $query );
         $sth->execute( $max_pickup_delay );
 
 
         my $query = "SELECT * FROM reserves WHERE TO_DAYS( NOW() ) - TO_DAYS( waitingdate ) > ? AND found = 'W' AND priority = 0";
         $sth = $dbh->prepare( $query );
         $sth->execute( $max_pickup_delay );
 
-        while (my $res = $sth->fetchrow_hashref ) {
-            if ( $charge ) {
-                manualinvoice($res->{'borrowernumber'}, $res->{'itemnumber'}, 'Hold waiting too long', 'F', $charge);
+        while ( my $res = $sth->fetchrow_hashref ) {
+            my $do_cancel = 1;
+            unless ( $cancel_on_holidays ) {
+                my $calendar = Koha::Calendar->new( branchcode => $res->{'branchcode'} );
+                my $is_holiday = $calendar->is_holiday( $today );
+
+                if ( $is_holiday ) {
+                    $do_cancel = 0;
+                }
             }
 
             }
 
-            CancelReserve({ reserve_id => $res->{'reserve_id'} });
+            if ( $do_cancel ) {
+                CancelReserve({ reserve_id => $res->{'reserve_id'}, charge_cancel_fee => 1 });
+            }
         }
     }
 
         }
     }
 
@@ -983,20 +1041,18 @@ Unsuspends all suspended reserves with a suspend_until date from before today.
 =cut
 
 sub AutoUnsuspendReserves {
 =cut
 
 sub AutoUnsuspendReserves {
+    my $today = dt_from_string();
 
 
-    my $dbh = C4::Context->dbh;
-
-    my $query = "UPDATE reserves SET suspend = 0, suspend_until = NULL WHERE DATE( suspend_until ) < DATE( CURDATE() )";
-    my $sth = $dbh->prepare( $query );
-    $sth->execute();
+    my @holds = Koha::Holds->search( { suspend_until => { '<' => $today->ymd() } } );
 
 
+    map { $_->suspend(0)->suspend_until(undef)->store() } @holds;
 }
 
 =head2 CancelReserve
 
 }
 
 =head2 CancelReserve
 
-  CancelReserve({ reserve_id => $reserve_id, [ biblionumber => $biblionumber, borrowernumber => $borrrowernumber, itemnumber => $itemnumber ] });
+  CancelReserve({ reserve_id => $reserve_id, [ biblionumber => $biblionumber, borrowernumber => $borrrowernumber, itemnumber => $itemnumber, ] [ charge_cancel_fee => 1 ] });
 
 
-Cancels a reserve.
+Cancels a reserve. If C<charge_cancel_fee> is passed and the C<ExpireReservesMaxPickUpDelayCharge> syspref is set, charge that fee to the patron's account.
 
 =cut
 
 
 =cut
 
@@ -1004,41 +1060,52 @@ sub CancelReserve {
     my ( $params ) = @_;
 
     my $reserve_id = $params->{'reserve_id'};
     my ( $params ) = @_;
 
     my $reserve_id = $params->{'reserve_id'};
-    $reserve_id = GetReserveId( $params ) unless ( $reserve_id );
+    # Filter out only the desired keys; this will insert undefined values for elements missing in
+    # \%params, but GetReserveId filters them out anyway.
+    $reserve_id = GetReserveId( { biblionumber => $params->{'biblionumber'}, borrowernumber => $params->{'borrowernumber'}, itemnumber => $params->{'itemnumber'} } ) unless ( $reserve_id );
 
     return unless ( $reserve_id );
 
     my $dbh = C4::Context->dbh;
 
     my $reserve = GetReserve( $reserve_id );
 
     return unless ( $reserve_id );
 
     my $dbh = C4::Context->dbh;
 
     my $reserve = GetReserve( $reserve_id );
+    if ($reserve) {
+        my $query = "
+            UPDATE reserves
+            SET    cancellationdate = now(),
+                   found            = Null,
+                   priority         = 0
+            WHERE  reserve_id = ?
+        ";
+        my $sth = $dbh->prepare($query);
+        $sth->execute( $reserve_id );
 
 
-    my $query = "
-        UPDATE reserves
-        SET    cancellationdate = now(),
-               found            = Null,
-               priority         = 0
-        WHERE  reserve_id = ?
-    ";
-    my $sth = $dbh->prepare($query);
-    $sth->execute( $reserve_id );
+        $query = "
+            INSERT INTO old_reserves
+            SELECT * FROM reserves
+            WHERE  reserve_id = ?
+        ";
+        $sth = $dbh->prepare($query);
+        $sth->execute( $reserve_id );
 
 
-    $query = "
-        INSERT INTO old_reserves
-        SELECT * FROM reserves
-        WHERE  reserve_id = ?
-    ";
-    $sth = $dbh->prepare($query);
-    $sth->execute( $reserve_id );
+        $query = "
+            DELETE FROM reserves
+            WHERE  reserve_id = ?
+        ";
+        $sth = $dbh->prepare($query);
+        $sth->execute( $reserve_id );
 
 
-    $query = "
-        DELETE FROM reserves
-        WHERE  reserve_id = ?
-    ";
-    $sth = $dbh->prepare($query);
-    $sth->execute( $reserve_id );
+        # now fix the priority on the others....
+        _FixPriority({ biblionumber => $reserve->{biblionumber} });
+
+        # and, if desired, charge a cancel fee
+        my $charge = C4::Context->preference("ExpireReservesMaxPickUpDelayCharge");
+        if ( $charge && $params->{'charge_cancel_fee'} ) {
+            manualinvoice($reserve->{'borrowernumber'}, $reserve->{'itemnumber'}, '', 'HE', $charge);
+        }
+    }
 
 
-    # now fix the priority on the others....
-    _FixPriority({ biblionumber => $reserve->{biblionumber} });
+    return $reserve;
 }
 
 =head2 ModReserve
 }
 
 =head2 ModReserve
@@ -1061,12 +1128,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.
 
@@ -1099,19 +1166,26 @@ sub ModReserve {
         CancelReserve({ reserve_id => $reserve_id });
     }
     elsif ($rank =~ /^\d+/ and $rank > 0) {
         CancelReserve({ reserve_id => $reserve_id });
     }
     elsif ($rank =~ /^\d+/ and $rank > 0) {
-        my $query = "
-            UPDATE reserves SET priority = ? ,branchcode = ?, itemnumber = ?, found = NULL, waitingdate = NULL
-            WHERE reserve_id = ?
-        ";
-        my $sth = $dbh->prepare($query);
-        $sth->execute( $rank, $branchcode, $itemnumber, $reserve_id );
+        my $hold = Koha::Holds->find($reserve_id);
+
+        $hold->set(
+            {
+                priority    => $rank,
+                branchcode  => $branchcode,
+                itemnumber  => $itemnumber,
+                found       => undef,
+                waitingdate => undef
+            }
+        )->store();
 
         if ( defined( $suspend_until ) ) {
             if ( $suspend_until ) {
 
         if ( defined( $suspend_until ) ) {
             if ( $suspend_until ) {
-                $suspend_until = C4::Dates->new( $suspend_until )->output("iso");
-                $dbh->do("UPDATE reserves SET suspend = 1, suspend_until = ? WHERE reserve_id = ?", undef, ( $suspend_until, $reserve_id ) );
+                $suspend_until = eval { dt_from_string( $suspend_until ) };
+                $hold->suspend_hold( $suspend_until );
             } else {
             } else {
-                $dbh->do("UPDATE reserves SET suspend_until = NULL WHERE reserve_id = ?", undef, ( $reserve_id ) );
+                # If the hold is suspended leave the hold suspended, but convert it to an indefinite hold.
+                # If the hold is not suspended, this does nothing.
+                $hold->set( { suspend_until => undef } )->store();
             }
         }
 
             }
         }
 
@@ -1178,11 +1252,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 });
     }
 }
 
     }
 }
 
@@ -1223,7 +1297,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
@@ -1248,7 +1322,7 @@ sub ModReserveAffect {
     my $request = GetReserveInfo($reserve_id);
     my $already_on_shelf = ($request && $request->{found} eq 'W') ? 1 : 0;
 
     my $request = GetReserveInfo($reserve_id);
     my $already_on_shelf = ($request && $request->{found} eq 'W') ? 1 : 0;
 
-    # If we affect a reserve that has to be transfered, don't set to Waiting
+    # If we affect a reserve that has to be transferred, don't set to Waiting
     my $query;
     if ($transferToDo) {
     $query = "
     my $query;
     if ($transferToDo) {
     $query = "
@@ -1275,7 +1349,7 @@ sub ModReserveAffect {
     $sth = $dbh->prepare($query);
     $sth->execute( $itemnumber, $borrowernumber,$biblionumber);
     _koha_notify_reserve( $itemnumber, $borrowernumber, $biblionumber ) if ( !$transferToDo && !$already_on_shelf );
     $sth = $dbh->prepare($query);
     $sth->execute( $itemnumber, $borrowernumber,$biblionumber);
     _koha_notify_reserve( $itemnumber, $borrowernumber, $biblionumber ) if ( !$transferToDo && !$already_on_shelf );
-
+    _FixPriority( { biblionumber => $biblionumber } );
     if ( C4::Context->preference("ReturnToShelvingCart") ) {
       CartToShelf( $itemnumber );
     }
     if ( C4::Context->preference("ReturnToShelvingCart") ) {
       CartToShelf( $itemnumber );
     }
@@ -1384,22 +1458,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
@@ -1410,48 +1480,102 @@ 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;
-    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 = ?";
-    }
-    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;
+    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'));
+
+    my $on_shelf_holds = _OnShelfHoldsAllowed($itype,$borrower->{categorycode},$item->{holdingbranch});
+
+    if ( $on_shelf_holds == 1 ) {
+        return 1;
+    } elsif ( $on_shelf_holds == 2 ) {
+        my @items =
+          Koha::Items->search( { biblionumber => $item->{biblionumber} } );
+
+        my $any_available = 0;
+
+        foreach my $i (@items) {
+            $any_available = 1
+              unless $i->itemlost
+              || $i->{notforloan} > 0
+              || $i->withdrawn
+              || $i->onloan
+              || IsItemOnHoldAndFound( $i->id )
+              || ( $i->damaged
+                && !C4::Context->preference('AllowHoldsOnDamagedItems') )
+              || Koha::ItemTypes->find( $i->effective_itemtype() )->notforloan;
+        }
+
+        return $any_available ? 0 : 1;
     }
 
     }
 
-    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;
+    return $item->{onloan} || GetReserveStatus($item->{itemnumber}) eq "Waiting";
+}
 
 
+=head2 OnShelfHoldsAllowed
 
 
-    if (C4::Context->preference('AllowOnShelfHolds')) {
-        return $available_per_item;
-    } else {
-        return ($available_per_item and ($item->{onloan} or GetReserveStatus($itemnumber) eq "Waiting"));
+  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')) {
+        # We can't trust GetItem to honour the syspref, so safest to do it ourselves
+        # When GetItem is fixed, we can remove this
+        $itype = $item->{itype};
     }
     }
+    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;
+}
+
+sub _OnShelfHoldsAllowed {
+    my ($itype,$borrowercategory,$branchcode) = @_;
+
+    my $rule = C4::Circulation::GetIssuingRule($borrowercategory, $itype, $branchcode);
+    return $rule->{onshelfholds};
 }
 
 =head2 AlterPriority
 }
 
 =head2 AlterPriority
@@ -1524,23 +1648,15 @@ be cleared when it is unsuspended.
 sub ToggleSuspend {
     my ( $reserve_id, $suspend_until ) = @_;
 
 sub ToggleSuspend {
     my ( $reserve_id, $suspend_until ) = @_;
 
-    $suspend_until = output_pref({ dt => dt_from_string( $suspend_until ), dateformat => 'iso' }) if ( $suspend_until );
+    $suspend_until = dt_from_string($suspend_until) if ($suspend_until);
 
 
-    my $do_until = ( $suspend_until ) ? '?' : 'NULL';
+    my $hold = Koha::Holds->find( $reserve_id );
 
 
-    my $dbh = C4::Context->dbh;
-
-    my $sth = $dbh->prepare(
-        "UPDATE reserves SET suspend = NOT suspend,
-        suspend_until = CASE WHEN suspend = 0 THEN NULL ELSE $do_until END
-        WHERE reserve_id = ?
-    ");
-
-    my @params;
-    push( @params, $suspend_until ) if ( $suspend_until );
-    push( @params, $reserve_id );
-
-    $sth->execute( @params );
+    if ( $hold->is_suspended ) {
+        $hold->resume()
+    } else {
+        $hold->suspend_hold( $suspend_until );
+    }
 }
 
 =head2 SuspendAll
 }
 
 =head2 SuspendAll
@@ -1565,37 +1681,26 @@ sub SuspendAll {
     my $borrowernumber = $params{'borrowernumber'} || undef;
     my $biblionumber   = $params{'biblionumber'}   || undef;
     my $suspend_until  = $params{'suspend_until'}  || undef;
     my $borrowernumber = $params{'borrowernumber'} || undef;
     my $biblionumber   = $params{'biblionumber'}   || undef;
     my $suspend_until  = $params{'suspend_until'}  || undef;
-    my $suspend        = defined( $params{'suspend'} ) ? $params{'suspend'} :  1;
+    my $suspend = defined( $params{'suspend'} ) ? $params{'suspend'} : 1;
 
 
-    $suspend_until = C4::Dates->new( $suspend_until )->output("iso") if ( defined( $suspend_until ) );
+    $suspend_until = eval { dt_from_string($suspend_until) }
+      if ( defined($suspend_until) );
 
     return unless ( $borrowernumber || $biblionumber );
 
 
     return unless ( $borrowernumber || $biblionumber );
 
-    my ( $query, $sth, $dbh, @query_params );
+    my $params;
+    $params->{found}          = undef;
+    $params->{borrowernumber} = $borrowernumber if $borrowernumber;
+    $params->{biblionumber}   = $biblionumber if $biblionumber;
 
 
-    $query = "UPDATE reserves SET suspend = ? ";
-    push( @query_params, $suspend );
-    if ( !$suspend ) {
-        $query .= ", suspend_until = NULL ";
-    } elsif ( $suspend_until ) {
-        $query .= ", suspend_until = ? ";
-        push( @query_params, $suspend_until );
-    }
-    $query .= " WHERE ";
-    if ( $borrowernumber ) {
-        $query .= " borrowernumber = ? ";
-        push( @query_params, $borrowernumber );
+    my @holds = Koha::Holds->search($params);
+
+    if ($suspend) {
+        map { $_->suspend_hold($suspend_until) } @holds;
     }
     }
-    $query .= " AND " if ( $borrowernumber && $biblionumber );
-    if ( $biblionumber ) {
-        $query .= " biblionumber = ? ";
-        push( @query_params, $biblionumber );
+    else {
+        map { $_->resume() } @holds;
     }
     }
-    $query .= " AND found IS NULL ";
-
-    $dbh = C4::Context->dbh;
-    $sth = $dbh->prepare( $query );
-    $sth->execute( @query_params );
 }
 
 
 }
 
 
@@ -1667,7 +1772,7 @@ sub _FixPriority {
 
     # get whats left
     my $query = "
 
     # get whats left
     my $query = "
-        SELECT reserve_id, borrowernumber, reservedate, constrainttype
+        SELECT reserve_id, borrowernumber, reservedate
         FROM   reserves
         WHERE  biblionumber   = ?
           AND  ((found <> 'W' AND found <> 'T') OR found IS NULL)
         FROM   reserves
         WHERE  biblionumber   = ?
           AND  ((found <> 'W' AND found <> 'T') OR found IS NULL)
@@ -1713,7 +1818,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({
@@ -1727,15 +1832,12 @@ 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
-reserve constraints.
+Looks for a holds-queue based item-specific match first, then for a holds-queue title-level match, returning the
+first match found.  If neither, then we look for non-holds-queue based holds.
 Lookahead is the number of days to look in advance.
 
 Lookahead is the number of days to look in advance.
 
-TODO: add more explanation about reserve constraints
-
 C<&_Findgroupreserve> returns :
 C<@results> is an array of references-to-hash whose keys are mostly
 fields from the reserves table of the Koha database, plus
 C<&_Findgroupreserve> returns :
 C<@results> is an array of references-to-hash whose keys are mostly
 fields from the reserves table of the Koha database, plus
@@ -1744,12 +1846,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.
     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/
+    # check for exact targeted match
+    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,
@@ -1761,7 +1863,8 @@ sub _Findgroupreserve {
                reserves.timestamp           AS timestamp,
                biblioitems.biblioitemnumber AS biblioitemnumber,
                reserves.itemnumber          AS itemnumber,
                reserves.timestamp           AS timestamp,
                biblioitems.biblioitemnumber AS biblioitemnumber,
                reserves.itemnumber          AS itemnumber,
-               reserves.reserve_id          AS reserve_id
+               reserves.reserve_id          AS reserve_id,
+               reserves.itemtype            AS itemtype
         FROM reserves
         JOIN biblioitems USING (biblionumber)
         JOIN hold_fill_targets USING (biblionumber, borrowernumber, itemnumber)
         FROM reserves
         JOIN biblioitems USING (biblionumber)
         JOIN hold_fill_targets USING (biblionumber, borrowernumber, itemnumber)
@@ -1771,17 +1874,19 @@ sub _Findgroupreserve {
         AND itemnumber = ?
         AND reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY)
         AND suspend = 0
         AND itemnumber = ?
         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
-    my $title_level_target_query = qq/
+
+    # check for title-level targeted match
+    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,
@@ -1792,7 +1897,9 @@ sub _Findgroupreserve {
                reserves.priority            AS priority,
                reserves.timestamp           AS timestamp,
                biblioitems.biblioitemnumber AS biblioitemnumber,
                reserves.priority            AS priority,
                reserves.timestamp           AS timestamp,
                biblioitems.biblioitemnumber AS biblioitemnumber,
-               reserves.itemnumber          AS itemnumber
+               reserves.itemnumber          AS itemnumber,
+               reserves.reserve_id          AS reserve_id,
+               reserves.itemtype            AS itemtype
         FROM reserves
         JOIN biblioitems USING (biblionumber)
         JOIN hold_fill_targets USING (biblionumber, borrowernumber)
         FROM reserves
         JOIN biblioitems USING (biblionumber)
         JOIN hold_fill_targets USING (biblionumber, borrowernumber)
@@ -1802,16 +1909,18 @@ sub _Findgroupreserve {
         AND hold_fill_targets.itemnumber = ?
         AND reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY)
         AND suspend = 0
         AND hold_fill_targets.itemnumber = ?
         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,
@@ -1822,24 +1931,22 @@ sub _Findgroupreserve {
                reserves.reservenotes               AS reservenotes,
                reserves.priority                   AS priority,
                reserves.timestamp                  AS timestamp,
                reserves.reservenotes               AS reservenotes,
                reserves.priority                   AS priority,
                reserves.timestamp                  AS timestamp,
-               reserveconstraints.biblioitemnumber AS biblioitemnumber,
-               reserves.itemnumber                 AS itemnumber
+               reserves.itemnumber                 AS itemnumber,
+               reserves.reserve_id                 AS reserve_id,
+               reserves.itemtype                   AS itemtype
         FROM reserves
         FROM reserves
-          LEFT JOIN reserveconstraints ON reserves.biblionumber = reserveconstraints.biblionumber
         WHERE reserves.biblionumber = ?
         WHERE reserves.biblionumber = ?
-          AND ( ( reserveconstraints.biblioitemnumber = ?
-          AND reserves.borrowernumber = reserveconstraints.borrowernumber
-          AND reserves.reservedate    = reserveconstraints.reservedate )
-          OR  reserves.constrainttype='a' )
           AND (reserves.itemnumber IS NULL OR reserves.itemnumber = ?)
           AND reserves.reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY)
           AND suspend = 0
           AND (reserves.itemnumber IS NULL OR reserves.itemnumber = ?)
           AND reserves.reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY)
           AND suspend = 0
-    /;
+          ORDER BY priority
+    };
     $sth = $dbh->prepare($query);
     $sth = $dbh->prepare($query);
-    $sth->execute( $biblio, $bibitem, $itemnumber, $lookahead||0);
+    $sth->execute( $biblio, $itemnumber, $lookahead||0);
     @results = ();
     while ( my $data = $sth->fetchrow_hashref ) {
     @results = ();
     while ( my $data = $sth->fetchrow_hashref ) {
-        push( @results, $data );
+        push( @results, $data )
+          unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ;
     }
     return @results;
 }
     }
     return @results;
 }
@@ -1851,6 +1958,25 @@ sub _Findgroupreserve {
 Sends a notification to the patron that their hold has been filled (through
 ModReserveAffect, _not_ ModReserveFill)
 
 Sends a notification to the patron that their hold has been filled (through
 ModReserveAffect, _not_ ModReserveFill)
 
+The letter code for this notice may be found using the following query:
+
+    select distinct letter_code
+    from message_transports
+    inner join message_attributes using (message_attribute_id)
+    where message_name = 'Hold_Filled'
+
+This will probably sipmly be 'HOLD', but because it is defined in the database,
+it is subject to addition or change.
+
+The following tables are availalbe witin the notice:
+
+    branches
+    borrowers
+    biblio
+    biblioitems
+    reserves
+    items
+
 =cut
 
 sub _koha_notify_reserve {
 =cut
 
 sub _koha_notify_reserve {
@@ -1858,18 +1984,14 @@ 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);
-    
-    my $letter_code;
-    my $print_mode = 0;
-    my $messagingprefs;
-    if ( $to_address || $borrower->{'smsalertnumber'} ) {
-        $messagingprefs = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $borrowernumber, message_name => 'Hold_Filled' } );
-    } else {
-        $print_mode = 1;
-    }
+
+    my $messagingprefs = C4::Members::Messaging::GetMessagingPreferences( {
+            borrowernumber => $borrowernumber,
+            message_name => 'Hold_Filled'
+    } );
 
     my $sth = $dbh->prepare("
         SELECT *
 
     my $sth = $dbh->prepare("
         SELECT *
@@ -1879,61 +2001,59 @@ sub _koha_notify_reserve {
     ");
     $sth->execute( $borrowernumber, $biblionumber );
     my $reserve = $sth->fetchrow_hashref;
     ");
     $sth->execute( $borrowernumber, $biblionumber );
     my $reserve = $sth->fetchrow_hashref;
-    my $branch_details = GetBranchDetail( $reserve->{'branchcode'} );
+    my $library = Koha::Libraries->find( $reserve->{branchcode} )->unblessed;
 
 
-    my $admin_email_address = $branch_details->{'branchemail'} || C4::Context->preference('KohaAdminEmailAddress');
+    my $admin_email_address = $library->{branchemail} || C4::Context->preference('KohaAdminEmailAddress');
 
     my %letter_params = (
         module => 'reserves',
         branchcode => $reserve->{branchcode},
         tables => {
 
     my %letter_params = (
         module => 'reserves',
         branchcode => $reserve->{branchcode},
         tables => {
-            'branches'  => $branch_details,
-            'borrowers' => $borrower,
-            'biblio'    => $biblionumber,
-            'reserves'  => $reserve,
+            'branches'       => $library,
+            'borrowers'      => $borrower,
+            'biblio'         => $biblionumber,
+            'biblioitems'    => $biblionumber,
+            'reserves'       => $reserve,
             'items', $reserve->{'itemnumber'},
         },
             'items', $reserve->{'itemnumber'},
         },
-        substitute => { today => C4::Dates->new()->output() },
+        substitute => { today => output_pref( { dt => dt_from_string, dateonly => 1 } ) },
     );
 
     );
 
-
-    if ( $print_mode ) {
-        $letter_params{ 'letter_code' } = 'HOLD_PRINT';
-        my $letter =  C4::Letters::GetPreparedLetter ( %letter_params ) or die "Could not find a letter called '$letter_params{'letter_code'}' in the 'reserves' module";
+    my $notification_sent = 0; #Keeping track if a Hold_filled message is sent. If no message can be sent, then default to a print message.
+    my $send_notification = sub {
+        my ( $mtt, $letter_code ) = (@_);
+        return unless defined $letter_code;
+        $letter_params{letter_code} = $letter_code;
+        $letter_params{message_transport_type} = $mtt;
+        my $letter =  C4::Letters::GetPreparedLetter ( %letter_params );
+        unless ($letter) {
+            warn "Could not find a letter called '$letter_params{'letter_code'}' for $mtt in the 'reserves' module";
+            return;
+        }
 
         C4::Letters::EnqueueLetter( {
             letter => $letter,
             borrowernumber => $borrowernumber,
 
         C4::Letters::EnqueueLetter( {
             letter => $letter,
             borrowernumber => $borrowernumber,
-            message_transport_type => 'print',
+            from_address => $admin_email_address,
+            message_transport_type => $mtt,
         } );
         } );
-        
-        return;
-    }
-
-    if ( $to_address && defined $messagingprefs->{transports}->{'email'} ) {
-        $letter_params{ 'letter_code' } = $messagingprefs->{transports}->{'email'};
-        my $letter =  C4::Letters::GetPreparedLetter ( %letter_params ) or die "Could not find a letter called '$letter_params{'letter_code'}' in the 'reserves' module";
+    };
 
 
-        C4::Letters::EnqueueLetter(
-            {   letter                 => $letter,
-                borrowernumber         => $borrowernumber,
-                message_transport_type => 'email',
-                from_address           => $admin_email_address,
-            }
+    while ( my ( $mtt, $letter_code ) = each %{ $messagingprefs->{transports} } ) {
+        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
         );
         );
-    }
-
-    if ( $borrower->{'smsalertnumber'} && defined $messagingprefs->{transports}->{'sms'} ) {
-        $letter_params{ 'letter_code' } = $messagingprefs->{transports}->{'sms'};
-        my $letter =  C4::Letters::GetPreparedLetter ( %letter_params ) or die "Could not find a letter called '$letter_params{'letter_code'}' in the 'reserves' module";
 
 
-        C4::Letters::EnqueueLetter(
-            {   letter                 => $letter,
-                borrowernumber         => $borrowernumber,
-                message_transport_type => 'sms',
-            }
-        );
+        &$send_notification($mtt, $letter_code);
+        $notification_sent++;
     }
     }
+    #Making sure that a print notification is sent if no other transport types can be utilized.
+    if (! $notification_sent) {
+        &$send_notification('print', 'HOLD');
+    }
+    
 }
 
 =head2 _ShiftPriorityByDateAndPriority
 }
 
 =head2 _ShiftPriorityByDateAndPriority
@@ -1985,6 +2105,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 can't 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 )
@@ -1997,7 +2165,8 @@ If $cancelreserve boolean is set to true, it will remove existing reserve
 sub MoveReserve {
     my ( $itemnumber, $borrowernumber, $cancelreserve ) = @_;
 
 sub MoveReserve {
     my ( $itemnumber, $borrowernumber, $cancelreserve ) = @_;
 
-    my ( $restype, $res, $all_reserves ) = CheckReserves( $itemnumber );
+    my $lookahead = C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds
+    my ( $restype, $res, $all_reserves ) = CheckReserves( $itemnumber, undef, $lookahead );
     return unless $res;
 
     my $biblionumber     =  $res->{biblionumber};
     return unless $res;
 
     my $biblionumber     =  $res->{biblionumber};
@@ -2029,11 +2198,7 @@ sub MoveReserve {
             RevertWaitingStatus({ itemnumber => $itemnumber });
         }
         elsif ( $cancelreserve eq 'cancel' || $cancelreserve ) { # cancel reserves on this item
             RevertWaitingStatus({ itemnumber => $itemnumber });
         }
         elsif ( $cancelreserve eq 'cancel' || $cancelreserve ) { # cancel reserves on this item
-            CancelReserve({
-                biblionumber   => $res->{'biblionumber'},
-                itemnumber     => $res->{'itemnumber'},
-                borrowernumber => $res->{'borrowernumber'}
-            });
+            CancelReserve( { reserve_id => $res->{'reserve_id'} } );
         }
     }
 }
         }
     }
 }
@@ -2067,7 +2232,7 @@ sub MergeHolds {
         );
         my $upd_sth = $dbh->prepare(
 "UPDATE reserves SET priority = ? WHERE biblionumber = ? AND borrowernumber = ?
         );
         my $upd_sth = $dbh->prepare(
 "UPDATE reserves SET priority = ? WHERE biblionumber = ? AND borrowernumber = ?
-        AND reservedate = ? AND constrainttype = ? AND (itemnumber = ? or itemnumber is NULL) "
+        AND reservedate = ? AND (itemnumber = ? or itemnumber is NULL) "
         );
         $sth->execute( $to_biblio, 'W', 'T' );
         my $priority = 1;
         );
         $sth->execute( $to_biblio, 'W', 'T' );
         my $priority = 1;
@@ -2075,7 +2240,7 @@ sub MergeHolds {
             $upd_sth->execute(
                 $priority,                    $to_biblio,
                 $reserve->{'borrowernumber'}, $reserve->{'reservedate'},
             $upd_sth->execute(
                 $priority,                    $to_biblio,
                 $reserve->{'borrowernumber'}, $reserve->{'reservedate'},
-                $reserve->{'constrainttype'}, $reserve->{'itemnumber'}
+                $reserve->{'itemnumber'}
             );
             $priority++;
         }
             );
             $priority++;
         }
@@ -2084,7 +2249,7 @@ sub MergeHolds {
 
 =head2 RevertWaitingStatus
 
 
 =head2 RevertWaitingStatus
 
-  $success = RevertWaitingStatus({ itemnumber => $itemnumber });
+  RevertWaitingStatus({ itemnumber => $itemnumber });
 
   Reverts a 'waiting' hold back to a regular hold with a priority of 1.
 
 
   Reverts a 'waiting' hold back to a regular hold with a priority of 1.
 
@@ -2139,7 +2304,8 @@ sub RevertWaitingStatus {
       reserve_id = ?
     ";
     $sth = $dbh->prepare( $query );
       reserve_id = ?
     ";
     $sth = $dbh->prepare( $query );
-    return $sth->execute( $reserve->{'reserve_id'} );
+    $sth->execute( $reserve->{'reserve_id'} );
+    _FixPriority( { biblionumber => $reserve->{biblionumber} } );
 }
 
 =head2 GetReserveId
 }
 
 =head2 GetReserveId
@@ -2181,7 +2347,17 @@ sub GetReserveId {
 
   ReserveSlip($branchcode, $borrowernumber, $biblionumber)
 
 
   ReserveSlip($branchcode, $borrowernumber, $biblionumber)
 
-  Returns letter hash ( see C4::Letters::GetPreparedLetter ) or undef
+Returns letter hash ( see C4::Letters::GetPreparedLetter ) or undef
+
+The letter code will be HOLD_SLIP, and the following tables are
+available within the slip:
+
+    reserves
+    branches
+    borrowers
+    biblio
+    biblioitems
+    items
 
 =cut
 
 
 =cut
 
@@ -2198,13 +2374,14 @@ sub ReserveSlip {
 
     return  C4::Letters::GetPreparedLetter (
         module => 'circulation',
 
     return  C4::Letters::GetPreparedLetter (
         module => 'circulation',
-        letter_code => 'RESERVESLIP',
+        letter_code => 'HOLD_SLIP',
         branchcode => $branch,
         tables => {
             'reserves'    => $reserve,
             'branches'    => $reserve->{branchcode},
             'borrowers'   => $reserve->{borrowernumber},
             'biblio'      => $reserve->{biblionumber},
         branchcode => $branch,
         tables => {
             'reserves'    => $reserve,
             'branches'    => $reserve->{branchcode},
             'borrowers'   => $reserve->{borrowernumber},
             'biblio'      => $reserve->{biblionumber},
+            'biblioitems' => $reserve->{biblionumber},
             'items'       => $reserve->{itemnumber},
         },
     );
             'items'       => $reserve->{itemnumber},
         },
     );
@@ -2236,6 +2413,75 @@ sub GetReservesControlBranch {
     return $branchcode;
 }
 
     return $branchcode;
 }
 
+=head2 CalculatePriority
+
+    my $p = CalculatePriority($biblionumber, $resdate);
+
+Calculate priority for a new reserve on biblionumber, placing it at
+the end of the line of all holds whose start date falls before
+the current system time and that are neither on the hold shelf
+or in transit.
+
+The reserve date parameter is optional; if it is supplied, the
+priority is based on the set of holds whose start date falls before
+the parameter value.
+
+After calculation of this priority, it is recommended to call
+_ShiftPriorityByDateAndPriority. Note that this is currently done in
+AddReserves.
+
+=cut
+
+sub CalculatePriority {
+    my ( $biblionumber, $resdate ) = @_;
+
+    my $sql = q{
+        SELECT COUNT(*) FROM reserves
+        WHERE biblionumber = ?
+        AND   priority > 0
+        AND   (found IS NULL OR found = '')
+    };
+    #skip found==W or found==T (waiting or transit holds)
+    if( $resdate ) {
+        $sql.= ' AND ( reservedate <= ? )';
+    }
+    else {
+        $sql.= ' AND ( reservedate < NOW() )';
+    }
+    my $dbh = C4::Context->dbh();
+    my @row = $dbh->selectrow_array(
+        $sql,
+        undef,
+        $resdate ? ($biblionumber, $resdate) : ($biblionumber)
+    );
+
+    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/>
Koha ILS @ FFZG - based on git://git.koha.org/pub/scm/koha.git