Bug 31963: Only show hold fee msg on OPAC if patron will be charged

[koha-ffzg.git] / C4 / Reserves.pm
diff --git a/C4/Reserves.pm b/C4/Reserves.pm

index 20ad806..d96d6ce 100644 (file)
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -21,39 +21,33 @@ package C4::Reserves;
  # along with Koha; if not, see <http://www.gnu.org/licenses>.
  
  
  # along with Koha; if not, see <http://www.gnu.org/licenses>.
  
  
-use strict;
-#use warnings; FIXME - Bug 2505
-use C4::Context;
-use C4::Biblio;
-use C4::Members;
-use C4::Items;
-use C4::Circulation;
-use C4::Accounts;
+use Modern::Perl;
  
  
-# for _koha_notify_reserve
-use C4::Members::Messaging;
-use C4::Members qw();
+use C4::Accounts;
+use C4::Biblio qw( GetMarcFromKohaField );
+use C4::Circulation qw( CheckIfIssuedToPatron GetAgeRestriction GetBranchItemRule );
+use C4::Context;
+use C4::Items qw( CartToShelf get_hostitemnumbers_of );
  use C4::Letters;
  use C4::Letters;
-use C4::Log;
-
+use C4::Log qw( logaction );
+use C4::Members::Messaging;
+use C4::Members;
+use Koha::Account::Lines;
+use Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue;
  use Koha::Biblios;
  use Koha::Biblios;
-use Koha::DateUtils;
  use Koha::Calendar;
  use Koha::Calendar;
+use Koha::CirculationRules;
  use Koha::Database;
  use Koha::Database;
-use Koha::Hold;
-use Koha::Old::Hold;
+use Koha::DateUtils qw( dt_from_string output_pref );
  use Koha::Holds;
  use Koha::Holds;
-use Koha::Libraries;
-use Koha::IssuingRules;
-use Koha::Items;
  use Koha::ItemTypes;
  use Koha::ItemTypes;
+use Koha::Items;
+use Koha::Libraries;
+use Koha::Old::Holds;
  use Koha::Patrons;
  use Koha::Patrons;
+use Koha::Plugins;
  
  
-use List::MoreUtils qw( firstidx any );
-use Carp;
-use Data::Dumper;
-
-use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
+use List::MoreUtils qw( any );
  
  =head1 NAME
  
  
  =head1 NAME
  
@@ -71,10 +65,13 @@ 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 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
+  - 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
+            P(rocessing) : reserved item has been returned using self-check machine and reserve needs to be confirmed
+                           by librarian before notice is send and status changed to waiting.
+                           Applicable only if HoldsNeedProcessingSIP system preference is set.
    - itemnumber : empty : the reserve is still unaffected to an item
                   filled: the reserve is attached to an item
    The complete workflow is :
    - itemnumber : empty : the reserve is still unaffected to an item
                   filled: the reserve is attached to an item
    The complete workflow is :
@@ -98,56 +95,75 @@ This modules provides somes functions to deal with reservations.
  
  =cut
  
  
  =cut
  
+our (@ISA, @EXPORT_OK);
  BEGIN {
      require Exporter;
      @ISA = qw(Exporter);
  BEGIN {
      require Exporter;
      @ISA = qw(Exporter);
-    @EXPORT = qw(
-        &AddReserve
+    @EXPORT_OK = qw(
+      AddReserve
  
  
-        &GetReservesForBranch
-        &GetReserveStatus
+      GetReserveStatus
  
  
-        &GetOtherReserves
+      GetOtherReserves
+      ChargeReserveFee
+      GetReserveFee
  
  
-        &ModReserveFill
-        &ModReserveAffect
-        &ModReserve
-        &ModReserveStatus
-        &ModReserveCancelAll
-        &ModReserveMinusPriority
-        &MoveReserve
+      ModReserveAffect
+      ModReserve
+      ModReserveStatus
+      ModReserveCancelAll
+      ModReserveMinusPriority
+      MoveReserve
  
  
-        &CheckReserves
-        &CanBookBeReserved
-        &CanItemBeReserved
-        &CanReserveBeCanceledFromOpac
-        &CancelExpiredReserves
+      CheckReserves
+      CanBookBeReserved
+      CanItemBeReserved
+      CanReserveBeCanceledFromOpac
+      CancelExpiredReserves
  
  
-        &AutoUnsuspendReserves
+      AutoUnsuspendReserves
  
  
-        &IsAvailableForItemLevelRequest
+      IsAvailableForItemLevelRequest
+      ItemsAnyAvailableAndNotRestricted
  
  
-        &OPACItemHoldsAllowed
+      AlterPriority
+      ToggleLowestPriority
  
  
-        &AlterPriority
-        &ToggleLowestPriority
+      ReserveSlip
+      ToggleSuspend
+      SuspendAll
  
  
-        &ReserveSlip
-        &ToggleSuspend
-        &SuspendAll
+      GetReservesControlBranch
  
  
-        &GetReservesControlBranch
+      CalculatePriority
  
  
-        IsItemOnHoldAndFound
+      IsItemOnHoldAndFound
  
  
-        GetMaxPatronHoldsForRecord
+      GetMaxPatronHoldsForRecord
+
+      MergeHolds
+
+      RevertWaitingStatus
      );
      );
-    @EXPORT_OK = qw( MergeHolds );
  }
  
  =head2 AddReserve
  
  }
  
  =head2 AddReserve
  
-    AddReserve($branch,$borrowernumber,$biblionumber,$bibitems,$priority,$resdate,$expdate,$notes,$title,$checkitem,$found)
+    AddReserve(
+        {
+            branchcode       => $branchcode,
+            borrowernumber   => $borrowernumber,
+            biblionumber     => $biblionumber,
+            priority         => $priority,
+            reservation_date => $reservation_date,
+            expiration_date  => $expiration_date,
+            notes            => $notes,
+            title            => $title,
+            itemnumber       => $itemnumber,
+            found            => $found,
+            itemtype         => $itemtype,
+        }
+    );
  
  Adds reserve and generates HOLDPLACED message.
  
  
  Adds reserve and generates HOLDPLACED message.
  
@@ -163,27 +179,56 @@ The following tables are available witin the HOLDPLACED message:
  =cut
  
  sub AddReserve {
  =cut
  
  sub AddReserve {
-    my (
-        $branch,   $borrowernumber, $biblionumber, $bibitems,
-        $priority, $resdate,        $expdate,      $notes,
-        $title,    $checkitem,      $found,        $itemtype
-    ) = @_;
-
-    $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 ($params)       = @_;
+    my $branch         = $params->{branchcode};
+    my $borrowernumber = $params->{borrowernumber};
+    my $biblionumber   = $params->{biblionumber};
+    my $priority       = $params->{priority};
+    my $resdate        = $params->{reservation_date};
+    my $patron_expiration_date = $params->{expiration_date};
+    my $notes          = $params->{notes};
+    my $title          = $params->{title};
+    my $checkitem      = $params->{itemnumber};
+    my $found          = $params->{found};
+    my $itemtype       = $params->{itemtype};
+    my $non_priority   = $params->{non_priority};
+
+    $resdate ||= dt_from_string;
+
+    # if we have an item selectionned, and the pickup branch is the same as the holdingbranch
+    # of the document, we force the value $priority and $found .
+    if ( $checkitem and not C4::Context->preference('ReservesNeedReturns') ) {
+        my $item = Koha::Items->find( $checkitem ); # FIXME Prevent bad calls
+
+        if (
+            # If item is already checked out, it cannot be set waiting
+            !$item->onloan
+
+            # The item can't be waiting if it needs a transfer
+            && $item->holdingbranch eq $branch
+
+            # Similarly, if in transit it can't be waiting
+            && !$item->get_transfer
+
+            # If we can't hold damaged items, and it is damaged, it can't be waiting
+            && ( $item->damaged && C4::Context->preference('AllowHoldsOnDamagedItems') || !$item->damaged )
+
+            # Lastly, if this already has holds, we shouldn't make it waiting for the new hold
+            && !$item->current_holds->count )
+        {
+            $priority = 0;
+            $found = 'W';
+        }
+    }
+    if ( C4::Context->preference( 'AllowHoldDateInFuture' ) ) {
+    # Make room in reserves for this if passed a priority
+    $priority = _ShiftPriority( $biblionumber, $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
-    if ( $found eq 'W' ) {
+    if ( $found && $found eq 'W' ) {
          $waitingdate = $resdate;
      }
  
          $waitingdate = $resdate;
      }
  
@@ -202,12 +247,15 @@ sub AddReserve {
              itemnumber     => $checkitem,
              found          => $found,
              waitingdate    => $waitingdate,
              itemnumber     => $checkitem,
              found          => $found,
              waitingdate    => $waitingdate,
-            expirationdate => $expdate,
+            patron_expiration_date => $patron_expiration_date,
              itemtype       => $itemtype,
              itemtype       => $itemtype,
+            item_level_hold => $checkitem ? 1 : 0,
+            non_priority   => $non_priority ? 1 : 0,
          }
      )->store();
          }
      )->store();
+    $hold->set_waiting() if $found && $found eq 'W';
  
  
-    logaction( 'HOLDS', 'CREATE', $hold->id, Dumper($hold->unblessed) )
+    logaction( 'HOLDS', 'CREATE', $hold->id, $hold )
          if C4::Context->preference('HoldsLog');
  
      my $reserve_id = $hold->id();
          if C4::Context->preference('HoldsLog');
  
      my $reserve_id = $hold->id();
@@ -239,195 +287,362 @@ sub AddReserve {
              },
          ) ) {
  
              },
          ) ) {
  
-            my $admin_email_address = $library->branchemail || C4::Context->preference('KohaAdminEmailAddress');
+            my $branch_email_address = $library->inbound_email_address;
  
              C4::Letters::EnqueueLetter(
  
              C4::Letters::EnqueueLetter(
-                {   letter                 => $letter,
+                {
+                    letter                 => $letter,
                      borrowernumber         => $borrowernumber,
                      message_transport_type => 'email',
                      borrowernumber         => $borrowernumber,
                      message_transport_type => 'email',
-                    from_address           => $admin_email_address,
-                    to_address           => $admin_email_address,
+                    to_address             => $branch_email_address,
                  }
              );
          }
      }
  
                  }
              );
          }
      }
  
+    Koha::Plugins->call('after_hold_create', $hold);
+    Koha::Plugins->call(
+        'after_hold_action',
+        {
+            action  => 'place',
+            payload => { hold => $hold->get_from_storage }
+        }
+    );
+
+    Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue(
+        {
+            biblio_ids => [ $biblionumber ]
+        }
+    ) if C4::Context->preference('RealTimeHoldsQueue');
+
      return $reserve_id;
  }
  
  =head2 CanBookBeReserved
  
      return $reserve_id;
  }
  
  =head2 CanBookBeReserved
  
-  $canReserve = &CanBookBeReserved($borrowernumber, $biblionumber)
+  $canReserve = &CanBookBeReserved($borrowernumber, $biblionumber, $branchcode, $params)
    if ($canReserve eq 'OK') { #We can reserve this Item! }
  
    if ($canReserve eq 'OK') { #We can reserve this Item! }
  
+  $params are passed directly through to CanItemBeReserved
+
  See CanItemBeReserved() for possible return values.
  
  =cut
  
  sub CanBookBeReserved{
  See CanItemBeReserved() for possible return values.
  
  =cut
  
  sub CanBookBeReserved{
-    my ($borrowernumber, $biblionumber) = @_;
+    my ($borrowernumber, $biblionumber, $pickup_branchcode, $params) = @_;
+
+    # Check that patron have not checked out this biblio (if AllowHoldsOnPatronsPossessions set)
+    if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions')
+        && C4::Circulation::CheckIfIssuedToPatron( $borrowernumber, $biblionumber ) ) {
+        return { status =>'alreadypossession' };
+    }
+
+    if ( $params->{itemtype} ) {
+
+        # biblio-level, item type-contrained
+        my $patron          = Koha::Patrons->find($borrowernumber);
+        my $reservesallowed = Koha::CirculationRules->get_effective_rule(
+            {
+                itemtype     => $params->{itemtype},
+                categorycode => $patron->categorycode,
+                branchcode   => $pickup_branchcode,
+                rule_name    => 'reservesallowed',
+            }
+        )->rule_value;
+
+        $reservesallowed = ( $reservesallowed eq '' ) ? undef : $reservesallowed;
+
+        my $count = $patron->holds->search(
+            {
+                '-or' => [
+                    { 'me.itemtype' => $params->{itemtype} },
+                    { 'item.itype'  => $params->{itemtype} }
+                ]
+            },
+            {
+                join => ['item']
+            }
+        )->count;
  
  
-    my $items = GetItemnumbersForBiblio($biblionumber);
+        return { status => '' }
+          if defined $reservesallowed and $reservesallowed < $count + 1;
+    }
+
+    my $items;
      #get items linked via host records
      #get items linked via host records
-    my @hostitems = get_hostitemnumbers_of($biblionumber);
-    if (@hostitems){
-    push (@$items,@hostitems);
+    my @hostitemnumbers = get_hostitemnumbers_of($biblionumber);
+    if (@hostitemnumbers){
+        $items = Koha::Items->search({
+            -or => [
+                biblionumber => $biblionumber,
+                itemnumber => { -in => @hostitemnumbers }
+            ]
+        });
+    } else {
+        $items = Koha::Items->search({ biblionumber => $biblionumber});
      }
  
      }
  
-    my $canReserve;
-    foreach my $item (@$items) {
-        $canReserve = CanItemBeReserved( $borrowernumber, $item );
-        return 'OK' if $canReserve eq 'OK';
+    my $canReserve = { status => '' };
+    my $patron = Koha::Patrons->find( $borrowernumber );
+    while ( my $item = $items->next ) {
+        $canReserve = CanItemBeReserved( $patron, $item, $pickup_branchcode, $params );
+        return { status => 'OK' } if $canReserve->{status} eq 'OK';
      }
      return $canReserve;
  }
  
  =head2 CanItemBeReserved
  
      }
      return $canReserve;
  }
  
  =head2 CanItemBeReserved
  
-  $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber)
-  if ($canReserve eq 'OK') { #We can reserve this Item! }
-
-@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
+  $canReserve = &CanItemBeReserved($patron, $item, $branchcode, $params)
+  if ($canReserve->{status} eq 'OK') { #We can reserve this Item! }
+
+  current params are:
+  'ignore_found_holds' - if true holds that have been trapped are not counted
+  toward the patron limit, used by checkHighHolds to avoid counting the hold we will fill with the
+  current checkout against the high holds threshold
+  'ignore_hold_counts' - we use this routine to check if an item can fill a hold - on this case we
+  should not check if there are too many holds as we only csre about reservability
+
+@RETURNS { status => OK },              if the Item can be reserved.
+         { status => ageRestricted },   if the Item is age restricted for this borrower.
+         { status => damaged },         if the Item is damaged.
+         { status => cannotReserveFromOtherBranches }, if syspref 'canreservefromotherbranches' is OK.
+         { status => branchNotInHoldGroup }, if borrower home library is not in hold group, and holds are only allowed from hold groups.
+         { status => tooManyReserves, limit => $limit }, if the borrower has exceeded their maximum reserve amount.
+         { status => notReservable },   if holds on this item are not allowed
+         { status => libraryNotFound },   if given branchcode is not an existing library
+         { status => libraryNotPickupLocation },   if given branchcode is not configured to be a pickup location
+         { status => cannotBeTransferred }, if branch transfer limit applies on given item and branchcode
+         { status => pickupNotInHoldGroup }, pickup location is not in hold group, and pickup locations are only allowed from hold groups.
+         { status => recall }, if the borrower has already placed a recall on this item
  
  =cut
  
  sub CanItemBeReserved {
  
  =cut
  
  sub CanItemBeReserved {
-    my ( $borrowernumber, $itemnumber ) = @_;
+    my ( $patron, $item, $pickup_branchcode, $params ) = @_;
  
      my $dbh = C4::Context->dbh;
      my $ruleitemtype;    # itemtype of the matching issuing rule
  
      my $dbh = C4::Context->dbh;
      my $ruleitemtype;    # itemtype of the matching issuing rule
-    my $allowedreserves  = 0; # Total number of holds allowed across all records
-    my $holds_per_record = 1; # Total number of holds allowed for this one given record
+    my $allowedreserves  = 0; # Total number of holds allowed across all records, default to none
+
+    # We check item branch if IndependentBranches is ON
+    # and canreservefromotherbranches is OFF
+    if ( C4::Context->preference('IndependentBranches')
+        and !C4::Context->preference('canreservefromotherbranches') )
+    {
+        if ( $item->homebranch ne $patron->branchcode ) {
+            return { status => 'cannotReserveFromOtherBranches' };
+        }
+    }
  
      # we retrieve borrowers and items informations #
      # item->{itype} will come for biblioitems if necessery
  
      # we retrieve borrowers and items informations #
      # item->{itype} will come for biblioitems if necessery
-    my $item       = GetItem($itemnumber);
-    my $biblio     = Koha::Biblios->find( $item->{biblionumber} );
-    my $patron = Koha::Patrons->find( $borrowernumber );
      my $borrower = $patron->unblessed;
  
      # If an item is damaged and we don't allow holds on damaged items, we can stop right here
      my $borrower = $patron->unblessed;
  
      # If an item is damaged and we don't allow holds on damaged items, we can stop right here
-    return 'damaged'
-      if ( $item->{damaged}
+    return { status =>'damaged' }
+      if ( $item->damaged
          && !C4::Context->preference('AllowHoldsOnDamagedItems') );
  
          && !C4::Context->preference('AllowHoldsOnDamagedItems') );
  
-    # Check for the age restriction
-    my ( $ageRestriction, $daysToAgeRestriction ) =
-      C4::Circulation::GetAgeRestriction( $biblio->biblioitem->agerestriction, $borrower );
-    return 'ageRestricted' if $daysToAgeRestriction && $daysToAgeRestriction > 0;
+    if( GetMarcFromKohaField('biblioitems.agerestriction') ){
+        my $biblio = $item->biblio;
+        # Check for the age restriction
+        my ( $ageRestriction, $daysToAgeRestriction ) =
+          C4::Circulation::GetAgeRestriction( $biblio->biblioitem->agerestriction, $borrower );
+        return { status => 'ageRestricted' } if $daysToAgeRestriction && $daysToAgeRestriction > 0;
+    }
  
      # Check that the patron doesn't have an item level hold on this item already
  
      # Check that the patron doesn't have an item level hold on this item already
-    return 'itemAlreadyOnHold'
-      if Koha::Holds->search( { borrowernumber => $borrowernumber, itemnumber => $itemnumber } )->count();
+    return { status =>'itemAlreadyOnHold' }
+      if ( !$params->{ignore_hold_counts} && Koha::Holds->search( { borrowernumber => $patron->borrowernumber, itemnumber => $item->itemnumber } )->count() );
  
  
-    my $controlbranch = C4::Context->preference('ReservesControlBranch');
+    # Check that patron have not checked out this biblio (if AllowHoldsOnPatronsPossessions set)
+    if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions')
+        && C4::Circulation::CheckIfIssuedToPatron( $patron->borrowernumber, $item->biblionumber ) ) {
+        return { status =>'alreadypossession' };
+    }
  
  
-    my $querycount = q{
-        SELECT count(*) AS count
-          FROM reserves
-     LEFT JOIN items USING (itemnumber)
-     LEFT JOIN biblioitems ON (reserves.biblionumber=biblioitems.biblionumber)
-     LEFT JOIN borrowers USING (borrowernumber)
-         WHERE borrowernumber = ?
-    };
+    # check if a recall exists on this item from this borrower
+    return { status => 'recall' }
+      if $patron->recalls->filter_by_current->search({ item_id => $item->itemnumber })->count;
  
  
-    my $branchcode  = "";
+    my $controlbranch = C4::Context->preference('ReservesControlBranch');
+
+    my $reserves_control_branch;
      my $branchfield = "reserves.branchcode";
  
      if ( $controlbranch eq "ItemHomeLibrary" ) {
          $branchfield = "items.homebranch";
      my $branchfield = "reserves.branchcode";
  
      if ( $controlbranch eq "ItemHomeLibrary" ) {
          $branchfield = "items.homebranch";
-        $branchcode  = $item->{homebranch};
+        $reserves_control_branch  = $item->homebranch;
      }
      elsif ( $controlbranch eq "PatronLibrary" ) {
          $branchfield = "borrowers.branchcode";
      }
      elsif ( $controlbranch eq "PatronLibrary" ) {
          $branchfield = "borrowers.branchcode";
-        $branchcode  = $borrower->{branchcode};
+        $reserves_control_branch  = $borrower->{branchcode};
      }
  
      # we retrieve rights
      }
  
      # we retrieve rights
-    if ( my $rights = GetHoldRule( $borrower->{'categorycode'}, $item->{'itype'}, $branchcode ) ) {
-        $ruleitemtype     = $rights->{itemtype};
-        $allowedreserves  = $rights->{reservesallowed};
-        $holds_per_record = $rights->{holds_per_record};
+    if (
+        my $reservesallowed = Koha::CirculationRules->get_effective_rule({
+                itemtype     => $item->effective_itemtype,
+                categorycode => $borrower->{categorycode},
+                branchcode   => $reserves_control_branch,
+                rule_name    => 'reservesallowed',
+        })
+    ) {
+        $ruleitemtype     = $reservesallowed->itemtype;
+        $allowedreserves  = $reservesallowed->rule_value // 0; #undefined is 0, blank is unlimited
      }
      else {
      }
      else {
-        $ruleitemtype = '*';
+        $ruleitemtype = undef;
      }
  
      }
  
-    $item = Koha::Items->find( $itemnumber );
-    my $holds = Koha::Holds->search(
-        {
-            borrowernumber => $borrowernumber,
-            biblionumber   => $item->biblionumber,
-            found          => undef, # Found holds don't count against a patron's holds limit
+    my $rights = Koha::CirculationRules->get_effective_rules({
+        categorycode => $borrower->{'categorycode'},
+        itemtype     => $item->effective_itemtype,
+        branchcode   => $reserves_control_branch,
+        rules        => ['holds_per_record','holds_per_day']
+    });
+    my $holds_per_record = $rights->{holds_per_record} // 1;
+    my $holds_per_day    = $rights->{holds_per_day};
+
+    if (   defined $holds_per_record && $holds_per_record ne '' ){
+        if ( $holds_per_record == 0 ) {
+            return { status => "noReservesAllowed" };
+        }
+        if ( !$params->{ignore_hold_counts} ) {
+            my $search_params = {
+                borrowernumber => $patron->borrowernumber,
+                biblionumber   => $item->biblionumber,
+            };
+            $search_params->{found} = undef if $params->{ignore_found_holds};
+            my $holds = Koha::Holds->search($search_params);
+            return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record } if $holds->count() >= $holds_per_record;
          }
          }
-    );
-    if ( $holds->count() >= $holds_per_record ) {
-        return "tooManyHoldsForThisRecord";
      }
  
      }
  
-    # we retrieve count
+    if (!$params->{ignore_hold_counts} && defined $holds_per_day && $holds_per_day ne '')
+    {
+        my $today_holds = Koha::Holds->search({
+            borrowernumber => $patron->borrowernumber,
+            reservedate    => dt_from_string->date
+        });
+        return { status => 'tooManyReservesToday', limit => $holds_per_day } if $today_holds->count() >= $holds_per_day;
+    }
  
  
-    $querycount .= "AND $branchfield = ?";
+    # we check if it's ok or not
+    if ( defined $allowedreserves && $allowedreserves ne '' ){
+        if( $allowedreserves == 0 ){
+            return { status => 'noReservesAllowed' };
+        }
+        if ( !$params->{ignore_hold_counts} ) {
+            # we retrieve count
+            my $querycount = q{
+                SELECT count(*) AS count
+                  FROM reserves
+             LEFT JOIN items USING (itemnumber)
+             LEFT JOIN biblioitems ON (reserves.biblionumber=biblioitems.biblionumber)
+             LEFT JOIN borrowers USING (borrowernumber)
+                 WHERE borrowernumber = ?
+            };
+            $querycount .= "AND ( $branchfield = ? OR $branchfield IS NULL )";
+
+            # If using item-level itypes, fall back to the record
+            # level itemtype if the hold has no associated item
+            if ( defined $ruleitemtype ) {
+                if ( C4::Context->preference('item-level_itypes') ) {
+                    $querycount .= q{
+                        AND ( COALESCE( items.itype, biblioitems.itemtype ) = ?
+                           OR reserves.itemtype = ? )
+                    };
+                }
+                else {
+                    $querycount .= q{
+                        AND ( biblioitems.itemtype = ?
+                           OR reserves.itemtype = ? )
+                    };
+                }
+            }
  
  
-    # 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 ( defined $ruleitemtype ) {
+                $sthcount->execute( $patron->borrowernumber, $reserves_control_branch, $ruleitemtype, $ruleitemtype );
+            }
+            else {
+                $sthcount->execute( $patron->borrowernumber, $reserves_control_branch );
+            }
  
  
-    if ( $ruleitemtype eq "*" ) {
-        $sthcount->execute( $borrowernumber, $branchcode );
-    }
-    else {
-        $sthcount->execute( $borrowernumber, $branchcode, $ruleitemtype );
-    }
+            my $reservecount = "0";
+            if ( my $rowcount = $sthcount->fetchrow_hashref() ) {
+                $reservecount = $rowcount->{count};
+            }
  
  
-    my $reservecount = "0";
-    if ( my $rowcount = $sthcount->fetchrow_hashref() ) {
-        $reservecount = $rowcount->{count};
+            return { status => 'tooManyReserves', limit => $allowedreserves } if $reservecount >= $allowedreserves;
+        }
      }
  
      }
  
-    # we check if it's ok or not
-    if ( $reservecount >= $allowedreserves ) {
-        return 'tooManyReserves';
+    # Now we need to check hold limits by patron category
+    my $rule = Koha::CirculationRules->get_effective_rule(
+        {
+            categorycode => $patron->categorycode,
+            branchcode   => $reserves_control_branch,
+            rule_name    => 'max_holds',
+        }
+    );
+    if (!$params->{ignore_hold_counts} && $rule && defined( $rule->rule_value ) && $rule->rule_value ne '' ) {
+        my $total_holds_count = Koha::Holds->search(
+            {
+                borrowernumber => $patron->borrowernumber
+            }
+        )->count();
+
+        return { status => 'tooManyReserves', limit => $rule->rule_value} if $total_holds_count >= $rule->rule_value;
      }
  
      }
  
-    my $circ_control_branch =
-      C4::Circulation::_GetCircControlBranch( $item->unblessed(), $borrower );
      my $branchitemrule =
      my $branchitemrule =
-      C4::Circulation::GetBranchItemRule( $circ_control_branch, $item->itype );
+      C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->effective_itemtype );
  
  
-    if ( $branchitemrule->{holdallowed} == 0 ) {
-        return 'notReservable';
+    if ( $branchitemrule->{holdallowed} eq 'not_allowed' ) {
+        return { status => 'notReservable' };
      }
  
      }
  
-    if (   $branchitemrule->{holdallowed} == 1
+    if (   $branchitemrule->{holdallowed} eq 'from_home_library'
          && $borrower->{branchcode} ne $item->homebranch )
      {
          && $borrower->{branchcode} ne $item->homebranch )
      {
-        return 'cannotReserveFromOtherBranches';
+        return { status => 'cannotReserveFromOtherBranches' };
      }
  
      }
  
-    # If reservecount is ok, we check item branch if IndependentBranches is ON
-    # and canreservefromotherbranches is OFF
-    if ( C4::Context->preference('IndependentBranches')
-        and !C4::Context->preference('canreservefromotherbranches') )
-    {
-        my $itembranch = $item->homebranch;
-        if ( $itembranch ne $borrower->{branchcode} ) {
-            return 'cannotReserveFromOtherBranches';
+    my $item_library = Koha::Libraries->find( {branchcode => $item->homebranch} );
+    if ( $branchitemrule->{holdallowed} eq 'from_local_hold_group') {
+        if($patron->branchcode ne $item->homebranch && !$item_library->validate_hold_sibling( {branchcode => $patron->branchcode} )) {
+            return { status => 'branchNotInHoldGroup' };
          }
      }
  
          }
      }
  
-    return 'OK';
+    if ($pickup_branchcode) {
+        my $destination = Koha::Libraries->find({
+            branchcode => $pickup_branchcode,
+        });
+
+        unless ($destination) {
+            return { status => 'libraryNotFound' };
+        }
+        unless ($destination->pickup_location) {
+            return { status => 'libraryNotPickupLocation' };
+        }
+        unless ($item->can_be_transferred({ to => $destination })) {
+            return { status => 'cannotBeTransferred' };
+        }
+        if ($branchitemrule->{hold_fulfillment_policy} eq 'holdgroup' && !$item_library->validate_hold_sibling( {branchcode => $pickup_branchcode} )) {
+            return { status => 'pickupNotInHoldGroup' };
+        }
+        if ($branchitemrule->{hold_fulfillment_policy} eq 'patrongroup' && !Koha::Libraries->find({branchcode => $borrower->{branchcode}})->validate_hold_sibling({branchcode => $pickup_branchcode})) {
+            return { status => 'pickupNotInHoldGroup' };
+        }
+    }
+
+    return { status => 'OK' };
  }
  
  =head2 CanReserveBeCanceledFromOpac
  }
  
  =head2 CanReserveBeCanceledFromOpac
@@ -444,13 +659,10 @@ sub CanReserveBeCanceledFromOpac {
      my ($reserve_id, $borrowernumber) = @_;
  
      return unless $reserve_id and $borrowernumber;
      my ($reserve_id, $borrowernumber) = @_;
  
      return unless $reserve_id and $borrowernumber;
-    my $reserve = Koha::Holds->find($reserve_id);
+    my $reserve = Koha::Holds->find($reserve_id) or return;
  
      return 0 unless $reserve->borrowernumber == $borrowernumber;
  
      return 0 unless $reserve->borrowernumber == $borrowernumber;
-    return 0 if ( $reserve->found eq 'W' ) or ( $reserve->found eq 'T' );
-
-    return 1;
-
+    return $reserve->is_cancelable_from_opac;
  }
  
  =head2 GetOtherReserves
  }
  
  =head2 GetOtherReserves
@@ -467,8 +679,8 @@ sub GetOtherReserves {
      my $nextreservinfo;
      my ( undef, $checkreserves, undef ) = CheckReserves($itemnumber);
      if ($checkreserves) {
      my $nextreservinfo;
      my ( undef, $checkreserves, undef ) = CheckReserves($itemnumber);
      if ($checkreserves) {
-        my $iteminfo = GetItem($itemnumber);
-        if ( $iteminfo->{'holdingbranch'} ne $checkreserves->{'branchcode'} ) {
+        my $item = Koha::Items->find($itemnumber);
+        if ( $item->holdingbranch ne $checkreserves->{'branchcode'} ) {
              $messages->{'transfert'} = $checkreserves->{'branchcode'};
              #minus priorities of others reservs
              ModReserveMinusPriority(
              $messages->{'transfert'} = $checkreserves->{'branchcode'};
              #minus priorities of others reservs
              ModReserveMinusPriority(
@@ -479,8 +691,9 @@ sub GetOtherReserves {
              #launch the subroutine dotransfer
              C4::Items::ModItemTransfer(
                  $itemnumber,
              #launch the subroutine dotransfer
              C4::Items::ModItemTransfer(
                  $itemnumber,
-                $iteminfo->{'holdingbranch'},
-                $checkreserves->{'branchcode'}
+                $item->holdingbranch,
+                $checkreserves->{'branchcode'},
+                'Reserve'
                ),
                ;
          }
                ),
                ;
          }
@@ -495,7 +708,7 @@ sub GetOtherReserves {
              ModReserveStatus($itemnumber,'W');
          }
  
              ModReserveStatus($itemnumber,'W');
          }
  
-        $nextreservinfo = $checkreserves->{'borrowernumber'};
+        $nextreservinfo = $checkreserves;
      }
  
      return ( $messages, $nextreservinfo );
      }
  
      return ( $messages, $nextreservinfo );
@@ -511,13 +724,20 @@ sub GetOtherReserves {
  
  sub ChargeReserveFee {
      my ( $borrowernumber, $fee, $title ) = @_;
  
  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 ) );
+    return if !$fee || $fee == 0;    # the last test is needed to include 0.00
+    Koha::Account->new( { patron_id => $borrowernumber } )->add_debit(
+        {
+            amount       => $fee,
+            description  => $title,
+            note         => undef,
+            user_id      => C4::Context->userenv ? C4::Context->userenv->{'number'} : undef,
+            library_id   => C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef,
+            interface    => C4::Context->interface,
+            invoice_type => undef,
+            type         => 'RESERVE',
+            item_id      => undef
+        }
+    );
  }
  
  =head2 GetReserveFee
  }
  
  =head2 GetReserveFee
@@ -553,50 +773,18 @@ SELECT COUNT(*) FROM reserves WHERE biblionumber=? AND borrowernumber<>?
          my ( $notissued, $reserved );
          ( $notissued ) = $dbh->selectrow_array( $issue_qry, undef,
              ( $biblionumber ) );
          my ( $notissued, $reserved );
          ( $notissued ) = $dbh->selectrow_array( $issue_qry, undef,
              ( $biblionumber ) );
-        if( $notissued ) {
+        if( $notissued == 0 ) {
+            # all items are issued
              ( $reserved ) = $dbh->selectrow_array( $holds_qry, undef,
                  ( $biblionumber, $borrowernumber ) );
              $fee = 0 if $reserved == 0;
              ( $reserved ) = $dbh->selectrow_array( $holds_qry, undef,
                  ( $biblionumber, $borrowernumber ) );
              $fee = 0 if $reserved == 0;
+        } else {
+            $fee = 0;
          }
      }
      return $fee;
  }
  
          }
      }
      return $fee;
  }
  
-=head2 GetReservesForBranch
-
-  @transreserv = GetReservesForBranch($frombranch);
-
-=cut
-
-sub GetReservesForBranch {
-    my ($frombranch) = @_;
-    my $dbh = C4::Context->dbh;
-
-    my $query = "
-        SELECT reserve_id,borrowernumber,reservedate,itemnumber,waitingdate, expirationdate
-        FROM   reserves
-        WHERE   priority='0'
-        AND found='W'
-    ";
-    $query .= " AND branchcode=? " if ( $frombranch );
-    $query .= "ORDER BY waitingdate" ;
-
-    my $sth = $dbh->prepare($query);
-    if ($frombranch){
-     $sth->execute($frombranch);
-    } else {
-        $sth->execute();
-    }
-
-    my @transreserv;
-    my $i = 0;
-    while ( my $data = $sth->fetchrow_hashref ) {
-        $transreserv[$i] = $data;
-        $i++;
-    }
-    return (@transreserv);
-}
-
  =head2 GetReserveStatus
  
    $reservestatus = GetReserveStatus($itemnumber);
  =head2 GetReserveStatus
  
    $reservestatus = GetReserveStatus($itemnumber);
@@ -624,19 +812,20 @@ sub GetReserveStatus {
  
      if(defined $found) {
          return 'Waiting'  if $found eq 'W' and $priority == 0;
  
      if(defined $found) {
          return 'Waiting'  if $found eq 'W' and $priority == 0;
+        return 'Processing'  if $found eq 'P';
          return 'Finished' if $found eq 'F';
      }
  
          return 'Finished' if $found eq 'F';
      }
  
-    return 'Reserved' if $priority > 0;
+    return 'Reserved' if defined $priority && $priority > 0;
  
      return ''; # empty string here will remove need for checking undef, or less log lines
  }
  
  =head2 CheckReserves
  
  
      return ''; # empty string here will remove need for checking undef, or less log lines
  }
  
  =head2 CheckReserves
  
-  ($status, $reserve, $all_reserves) = &CheckReserves($itemnumber);
-  ($status, $reserve, $all_reserves) = &CheckReserves(undef, $barcode);
-  ($status, $reserve, $all_reserves) = &CheckReserves($itemnumber,undef,$lookahead);
+  ($status, $matched_reserve, $possible_reserves) = &CheckReserves($itemnumber);
+  ($status, $matched_reserve, $possible_reserves) = &CheckReserves(undef, $barcode);
+  ($status, $matched_reserve, $possible_reserves) = &CheckReserves($itemnumber,undef,$lookahead);
  
  Find a book in the reserves.
  
  
  Find a book in the reserves.
  
@@ -707,14 +896,17 @@ sub CheckReserves {
      }
      # 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, $damaged, $item_homebranch, $item_holdingbranch ) = $sth->fetchrow_array;
      }
      # 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, $damaged, $item_homebranch, $item_holdingbranch ) = $sth->fetchrow_array;
-
      return if ( $damaged && !C4::Context->preference('AllowHoldsOnDamagedItems') );
  
      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.....
      # except where items.notforloan < 0 :  This indicates the item is holdable.
      # if item is not for loan it cannot be reserved either.....
      # except where items.notforloan < 0 :  This indicates the item is holdable.
-    return if  ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;
+
+    my @SkipHoldTrapOnNotForLoanValue = split( '\|', C4::Context->preference('SkipHoldTrapOnNotForLoanValue') );
+    return if grep { $_ eq $notforloan_per_item } @SkipHoldTrapOnNotForLoanValue;
+
+    my $dont_trap = C4::Context->preference('TrapHoldsOnOrder') ? ($notforloan_per_item > 0) : ($notforloan_per_item && 1 );
+    return if $dont_trap or $notforloan_per_itemtype;
  
      # Find this item in the reserves
      my @reserves = _Findgroupreserve( $bibitem, $biblio, $itemnumber, $lookahead_days, $ignore_borrowers);
  
      # Find this item in the reserves
      my @reserves = _Findgroupreserve( $bibitem, $biblio, $itemnumber, $lookahead_days, $ignore_borrowers);
@@ -724,6 +916,7 @@ sub CheckReserves {
      # the more important the item.)
      # $highest is the most important item we've seen so far.
      my $highest;
      # the more important the item.)
      # $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');
      if (scalar @reserves) {
          my $LocalHoldsPriority = C4::Context->preference('LocalHoldsPriority');
          my $LocalHoldsPriorityPatronControl = C4::Context->preference('LocalHoldsPriorityPatronControl');
@@ -731,40 +924,52 @@ sub CheckReserves {
  
          my $priority = 10000000;
          foreach my $res (@reserves) {
  
          my $priority = 10000000;
          foreach my $res (@reserves) {
-            if ( $res->{'itemnumber'} == $itemnumber && $res->{'priority'} == 0) {
-                return ( "Waiting", $res, \@reserves ); # Found it
+            if ($res->{'found'} && $res->{'found'} eq 'W') {
+                return ( "Waiting", $res, \@reserves ); # Found it, it is waiting
+            } elsif ($res->{'found'} && $res->{'found'} eq 'P') {
+                return ( "Processing", $res, \@reserves ); # Found determinated hold, e. g. the transferred one
+            } elsif ($res->{'found'} && $res->{'found'} eq 'T') {
+                return ( "Transferred", $res, \@reserves ); # Found determinated hold, e. g. the transferred one
              } else {
                  my $patron;
              } else {
                  my $patron;
-                my $iteminfo;
+                my $item;
                  my $local_hold_match;
  
                  if ($LocalHoldsPriority) {
                      $patron = Koha::Patrons->find( $res->{borrowernumber} );
                  my $local_hold_match;
  
                  if ($LocalHoldsPriority) {
                      $patron = Koha::Patrons->find( $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' )
-                      ? $patron->branchcode
-                      : undef;
-                    $local_hold_match =
-                      $local_holds_priority_item_branchcode eq
-                      $local_holds_priority_patron_branchcode;
+                    $item = Koha::Items->find($itemnumber);
+
+                    unless ($item->exclude_from_local_holds_priority || $patron->category->exclude_from_local_holds_priority) {
+                        my $local_holds_priority_item_branchcode =
+                            $item->$LocalHoldsPriorityItemControl;
+                        my $local_holds_priority_patron_branchcode =
+                            ( $LocalHoldsPriorityPatronControl eq 'PickupLibrary' )
+                            ? $res->{branchcode}
+                            : ( $LocalHoldsPriorityPatronControl eq 'HomeLibrary' )
+                            ? $patron->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
                  if ( ( $res->{'priority'} && $res->{'priority'} < $priority ) || $local_hold_match ) {
                  }
  
                  # See if this item is more important than what we've got so far
                  if ( ( $res->{'priority'} && $res->{'priority'} < $priority ) || $local_hold_match ) {
-                    $iteminfo ||= C4::Items::GetItem($itemnumber);
-                    next if $res->{itemtype} && $res->{itemtype} ne _get_itype( $iteminfo );
+                    $item ||= Koha::Items->find($itemnumber);
+                    next if $res->{itemtype} && $res->{itemtype} ne $item->effective_itemtype;
                      $patron ||= Koha::Patrons->find( $res->{borrowernumber} );
                      $patron ||= Koha::Patrons->find( $res->{borrowernumber} );
-                    my $branch = GetReservesControlBranch( $iteminfo, $patron->unblessed );
-                    my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$iteminfo->{'itype'});
-                    next if ($branchitemrule->{'holdallowed'} == 0);
-                    next if (($branchitemrule->{'holdallowed'} == 1) && ($branch ne $patron->branchcode));
-                    next if ( ($branchitemrule->{hold_fulfillment_policy} ne 'any') && ($res->{branchcode} ne $iteminfo->{ $branchitemrule->{hold_fulfillment_policy} }) );
+                    my $branch = GetReservesControlBranch( $item->unblessed, $patron->unblessed );
+                    my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$item->effective_itemtype);
+                    next if ($branchitemrule->{'holdallowed'} eq 'not_allowed');
+                    next if (($branchitemrule->{'holdallowed'} eq 'from_home_library') && ($item->homebranch ne $patron->branchcode));
+                    my $library = Koha::Libraries->find({branchcode=>$item->homebranch});
+                    next if (($branchitemrule->{'holdallowed'} eq 'from_local_hold_group') && (!$library->validate_hold_sibling({branchcode => $patron->branchcode}) ));
+                    my $hold_fulfillment_policy = $branchitemrule->{hold_fulfillment_policy};
+                    next if ( ($hold_fulfillment_policy eq 'holdgroup') && (!$library->validate_hold_sibling({branchcode => $res->{branchcode}})) );
+                    next if ( ($hold_fulfillment_policy eq 'homebranch') && ($res->{branchcode} ne $item->$hold_fulfillment_policy) );
+                    next if ( ($hold_fulfillment_policy eq 'holdingbranch') && ($res->{branchcode} ne $item->$hold_fulfillment_policy) );
+                    next unless $item->can_be_transferred( { to => Koha::Libraries->find( $res->{branchcode} ) } );
                      $priority = $res->{'priority'};
                      $highest  = $res;
                      last if $local_hold_match;
                      $priority = $res->{'priority'};
                      $highest  = $res;
                      last if $local_hold_match;
@@ -792,20 +997,23 @@ Cancels all reserves with an expiration date from before today.
  =cut
  
  sub CancelExpiredReserves {
  =cut
  
  sub CancelExpiredReserves {
-
+    my $cancellation_reason = shift;
      my $today = dt_from_string();
      my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays');
      my $today = dt_from_string();
      my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays');
-
-    my $dbh = C4::Context->dbh;
+    my $expireWaiting = C4::Context->preference('ExpireReservesMaxPickUpDelay');
  
      my $dtf = Koha::Database->new->schema->storage->datetime_parser;
  
      my $dtf = Koha::Database->new->schema->storage->datetime_parser;
-    my $today = dt_from_string;
+    my $params = {
+        -or => [
+            { expirationdate => { '<', $dtf->format_date($today) } },
+            { patron_expiration_date => { '<' => $dtf->format_date($today) } }
+        ]
+    };
+
+    $params->{found} = [ { '!=', 'W' }, undef ]  unless $expireWaiting;
+
      # FIXME To move to Koha::Holds->search_expired (?)
      # FIXME To move to Koha::Holds->search_expired (?)
-    my $holds = Koha::Holds->search(
-        {
-            expirationdate => { '<', $dtf->format_date($today) }
-        }
-    );
+    my $holds = Koha::Holds->search( $params );
  
      while ( my $hold = $holds->next ) {
          my $calendar = Koha::Calendar->new( branchcode => $hold->branchcode );
  
      while ( my $hold = $holds->next ) {
          my $calendar = Koha::Calendar->new( branchcode => $hold->branchcode );
@@ -813,11 +1021,12 @@ sub CancelExpiredReserves {
          next if !$cancel_on_holidays && $calendar->is_holiday( $today );
  
          my $cancel_params = {};
          next if !$cancel_on_holidays && $calendar->is_holiday( $today );
  
          my $cancel_params = {};
-        if ( $holds->found eq 'W' ) {
+        $cancel_params->{cancellation_reason} = $cancellation_reason if defined($cancellation_reason);
+        if ( defined($hold->found) && $hold->found eq 'W' ) {
              $cancel_params->{charge_cancel_fee} = 1;
          }
              $cancel_params->{charge_cancel_fee} = 1;
          }
+        $cancel_params->{autofill} = C4::Context->preference('ExpireReservesAutoFill');
          $hold->cancel( $cancel_params );
          $hold->cancel( $cancel_params );
-
      }
  }
  
      }
  }
  
@@ -832,70 +1041,9 @@ Unsuspends all suspended reserves with a suspend_until date from before today.
  sub AutoUnsuspendReserves {
      my $today = dt_from_string();
  
  sub AutoUnsuspendReserves {
      my $today = dt_from_string();
  
-    my @holds = Koha::Holds->search( { suspend_until => { '<' => $today->ymd() } } );
-
-    map { $_->suspend(0)->suspend_until(undef)->store() } @holds;
-}
-
-=head2 CancelReserve
-
-  CancelReserve({ reserve_id => $reserve_id, [ biblionumber => $biblionumber, borrowernumber => $borrrowernumber, itemnumber => $itemnumber, ] [ charge_cancel_fee => 1 ] });
-
-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
-
-sub CancelReserve {
-    my ( $params ) = @_;
-
-    my $reserve_id = $params->{'reserve_id'};
-    my $hold;
-    if ( $reserve_id ) {
-        $hold = Koha::Holds->find( $reserve_id );
-    } else {
-        $hold = Koha::Holds->search( $params ); # biblionumber, borrowernumber, itemnumber
-    }
-
-    return unless $hold;
-
-    logaction( 'HOLDS', 'CANCEL', $hold->reserve_id, Dumper($hold->unblessed) )
-        if C4::Context->preference('HoldsLog');
-
-    my $query = "
-        UPDATE reserves
-        SET    cancellationdate = now(),
-               priority         = 0
-        WHERE  reserve_id = ?
-    ";
-    my $dbh = C4::Context->dbh;
-    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 = "
-        DELETE FROM reserves
-        WHERE  reserve_id = ?
-    ";
-    $sth = $dbh->prepare($query);
-    $sth->execute( $reserve_id );
-
-    # now fix the priority on the others....
-    _FixPriority({ biblionumber => $hold->biblionumber });
+    my @holds = Koha::Holds->search( { suspend_until => { '<=' => $today->ymd() } } )->as_list;
  
  
-    # and, if desired, charge a cancel fee
-    my $charge = C4::Context->preference("ExpireReservesMaxPickUpDelayCharge");
-    if ( $charge && $params->{'charge_cancel_fee'} ) {
-        manualinvoice($hold->borrowernumber, $hold->itemnumber, '', 'HE', $charge);
-    }
-
-    return $hold->unblessed;
+    map { $_->resume() } @holds;
  }
  
  =head2 ModReserve
  }
  
  =head2 ModReserve
@@ -910,7 +1058,7 @@ sub CancelReserve {
  Change a hold request's priority or cancel it.
  
  C<$rank> specifies the effect of the change.  If C<$rank>
  Change a hold request's priority or cancel it.
  
  C<$rank> specifies the effect of the change.  If C<$rank>
-is 'W' or 'n', nothing happens.  This corresponds to leaving a
+is 'n', nothing happens.  This corresponds to leaving a
  request alone when changing its priority in the holds queue
  for a bib.
  
  request alone when changing its priority in the holds queue
  for a bib.
  
@@ -922,6 +1070,9 @@ that the item is not waiting on the hold shelf, setting the
  priority to a non-zero value also sets the request's found
  status and waiting date to NULL.
  
  priority to a non-zero value also sets the request's found
  status and waiting date to NULL.
  
+If the hold is 'found' (waiting, in-transit, processing) the
+only field that can be updated is the expiration date.
+
  The optional C<$itemnumber> parameter is used only when
  C<$rank> is a non-zero integer; if supplied, the itemnumber
  of the hold request is set accordingly; if omitted, the itemnumber
  The optional C<$itemnumber> parameter is used only when
  C<$rank> is a non-zero integer; if supplied, the itemnumber
  of the hold request is set accordingly; if omitted, the itemnumber
@@ -944,41 +1095,58 @@ sub ModReserve {
      my $suspend_until = $params->{'suspend_until'};
      my $borrowernumber = $params->{'borrowernumber'};
      my $biblionumber = $params->{'biblionumber'};
      my $suspend_until = $params->{'suspend_until'};
      my $borrowernumber = $params->{'borrowernumber'};
      my $biblionumber = $params->{'biblionumber'};
+    my $cancellation_reason = $params->{'cancellation_reason'};
+    my $date = $params->{expirationdate};
  
  
-    return if $rank eq "W";
-    return if $rank eq "n";
+    return if defined $rank && $rank eq "n";
  
      return unless ( $reserve_id || ( $borrowernumber && ( $biblionumber || $itemnumber ) ) );
  
      my $hold;
      unless ( $reserve_id ) {
  
      return unless ( $reserve_id || ( $borrowernumber && ( $biblionumber || $itemnumber ) ) );
  
      my $hold;
      unless ( $reserve_id ) {
-        $hold = Koha::Holds->search({ biblionumber => $biblionumber, borrowernumber => $borrowernumber, itemnumber => $itemnumber });
-        return unless $hold; # FIXME Should raise an exception
+        my $holds = Koha::Holds->search({ biblionumber => $biblionumber, borrowernumber => $borrowernumber, itemnumber => $itemnumber });
+        return unless $holds->count; # FIXME Should raise an exception
+        $hold = $holds->next;
          $reserve_id = $hold->reserve_id;
      }
  
      $hold ||= Koha::Holds->find($reserve_id);
  
          $reserve_id = $hold->reserve_id;
      }
  
      $hold ||= Koha::Holds->find($reserve_id);
  
+    # FIXME Other calls may fail
+    Koha::Exceptions::ObjectNotFound->throw( 'No hold with id ' . $reserve_id ) unless $hold;
+
      if ( $rank eq "del" ) {
      if ( $rank eq "del" ) {
-        $hold->cancel;
+        $hold->cancel({ cancellation_reason => $cancellation_reason });
+    }
+    elsif ($hold->found && $hold->priority eq '0' && $date) {
+        logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold )
+            if C4::Context->preference('HoldsLog');
+
+        # The only column that can be updated for a found hold is the expiration date
+        $hold->expirationdate($date)->store();
      }
      elsif ($rank =~ /^\d+/ and $rank > 0) {
      }
      elsif ($rank =~ /^\d+/ and $rank > 0) {
-        logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, Dumper($hold->unblessed) )
+        logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold )
              if C4::Context->preference('HoldsLog');
  
              if C4::Context->preference('HoldsLog');
  
-        $hold->set(
-            {
-                priority    => $rank,
-                branchcode  => $branchcode,
-                itemnumber  => $itemnumber,
-                found       => undef,
-                waitingdate => undef
-            }
-        )->store();
+        my $properties = {
+            priority    => $rank,
+            branchcode  => $branchcode,
+            itemnumber  => $itemnumber,
+            found       => undef,
+            waitingdate => undef
+        };
+        if (exists $params->{reservedate}) {
+            $properties->{reservedate} = $params->{reservedate} || undef;
+        }
+        if (exists $params->{expirationdate}) {
+            $properties->{expirationdate} = $params->{expirationdate} || undef;
+        }
+
+        $hold->set($properties)->store();
  
          if ( defined( $suspend_until ) ) {
              if ( $suspend_until ) {
  
          if ( defined( $suspend_until ) ) {
              if ( $suspend_until ) {
-                $suspend_until = eval { dt_from_string( $suspend_until ) };
                  $hold->suspend_hold( $suspend_until );
              } else {
                  # If the hold is suspended leave the hold suspended, but convert it to an indefinite hold.
                  $hold->suspend_hold( $suspend_until );
              } else {
                  # If the hold is suspended leave the hold suspended, but convert it to an indefinite hold.
@@ -991,52 +1159,6 @@ sub ModReserve {
      }
  }
  
      }
  }
  
-=head2 ModReserveFill
-
-  &ModReserveFill($reserve);
-
-Fill a reserve. If I understand this correctly, this means that the
-reserved book has been found and given to the patron who reserved it.
-
-C<$reserve> specifies the reserve to fill. It is a reference-to-hash
-whose keys are fields from the reserves table in the Koha database.
-
-=cut
-
-sub ModReserveFill {
-    my ($res) = @_;
-    my $reserve_id = $res->{'reserve_id'};
-
-    my $hold = Koha::Holds->find($reserve_id);
-
-    # get the priority on this record....
-    my $priority = $hold->priority;
-
-    # update the hold statuses, no need to store it though, we will be deleting it anyway
-    $hold->set(
-        {
-            found    => 'F',
-            priority => 0,
-        }
-    );
-
-    # FIXME Must call Koha::Hold->cancel ?
-    Koha::Old::Hold->new( $hold->unblessed() )->store();
-
-    $hold->delete();
-
-    if ( C4::Context->preference('HoldFeeMode') eq 'any_time_is_collected' ) {
-        my $reserve_fee = GetReserveFee( $hold->borrowernumber, $hold->biblionumber );
-        ChargeReserveFee( $hold->borrowernumber, $reserve_fee, $hold->biblio->title );
-    }
-
-    # now fix the priority on the others (if the priority wasn't
-    # already sorted!)....
-    unless ( $priority == 0 ) {
-        _FixPriority( { reserve_id => $reserve_id, biblionumber => $hold->biblionumber } );
-    }
-}
-
  =head2 ModReserveStatus
  
    &ModReserveStatus($itemnumber, $newstatus);
  =head2 ModReserveStatus
  
    &ModReserveStatus($itemnumber, $newstatus);
@@ -1059,14 +1181,17 @@ sub ModReserveStatus {
      my $sth_set = $dbh->prepare($query);
      $sth_set->execute( $newstatus, $itemnumber );
  
      my $sth_set = $dbh->prepare($query);
      $sth_set->execute( $newstatus, $itemnumber );
  
-    if ( C4::Context->preference("ReturnToShelvingCart") && $newstatus ) {
+    my $item = Koha::Items->find($itemnumber);
+    if ( $item->location && $item->location eq 'CART'
+        && ( !$item->permanent_location || $item->permanent_location ne 'CART' )
+        && $newstatus ) {
        CartToShelf( $itemnumber );
      }
  }
  
  =head2 ModReserveAffect
  
        CartToShelf( $itemnumber );
      }
  }
  
  =head2 ModReserveAffect
  
-  &ModReserveAffect($itemnumber,$borrowernumber,$diffBranchSend,$reserve_id);
+  &ModReserveAffect($itemnumber,$borrowernumber,$diffBranchSend,$reserve_id, $desk_id, $notify_library);
  
  This function affect an item and a status for a given reserve, either fetched directly
  by record_id, or by borrowernumber and itemnumber or biblionumber. If only biblionumber
  
  This function affect an item and a status for a given reserve, either fetched directly
  by record_id, or by borrowernumber and itemnumber or biblionumber. If only biblionumber
@@ -1077,10 +1202,12 @@ 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
  take care of the waiting status
  
  otherwise, a transfer is on the way, and the end of the transfer will
  take care of the waiting status
  
+This function also removes any entry of the hold in holds queue table.
+
  =cut
  
  sub ModReserveAffect {
  =cut
  
  sub ModReserveAffect {
-    my ( $itemnumber, $borrowernumber, $transferToDo, $reserve_id ) = @_;
+    my ( $itemnumber, $borrowernumber, $transferToDo, $reserve_id, $desk_id, $notify_library ) = @_;
      my $dbh = C4::Context->dbh;
  
      # we want to attach $itemnumber to $borrowernumber, find the biblionumber
      my $dbh = C4::Context->dbh;
  
      # we want to attach $itemnumber to $borrowernumber, find the biblionumber
@@ -1105,23 +1232,52 @@ sub ModReserveAffect {
      my $already_on_shelf = $hold->found && $hold->found eq 'W';
  
      $hold->itemnumber($itemnumber);
      my $already_on_shelf = $hold->found && $hold->found eq 'W';
  
      $hold->itemnumber($itemnumber);
-    $hold->set_waiting($transferToDo);
  
  
-    _koha_notify_reserve( $hold->reserve_id )
-      if ( !$transferToDo && !$already_on_shelf );
+    if ($transferToDo) {
+        $hold->set_transfer();
+    } elsif (C4::Context->preference('HoldsNeedProcessingSIP')
+             && C4::Context->interface eq 'sip'
+             && !$already_on_shelf) {
+        $hold->set_processing();
+    } else {
+        $hold->set_waiting($desk_id);
+        _koha_notify_reserve( $hold->reserve_id ) unless $already_on_shelf;
+        # Complete transfer if one exists
+        my $transfer = $hold->item->get_transfer;
+        $transfer->receive if $transfer;
+    }
+
+    _koha_notify_hold_changed( $hold ) if $notify_library;
  
      _FixPriority( { biblionumber => $biblionumber } );
  
      _FixPriority( { biblionumber => $biblionumber } );
-
-    if ( C4::Context->preference("ReturnToShelvingCart") ) {
-        CartToShelf($itemnumber);
+    my $item = Koha::Items->find($itemnumber);
+    if ( $item->location && $item->location eq 'CART'
+        && ( !$item->permanent_location || $item->permanent_location ne 'CART' ) ) {
+      CartToShelf( $itemnumber );
      }
  
      }
  
+    my $std = $dbh->prepare(q{
+        DELETE  q, t
+        FROM    tmp_holdsqueue q
+        INNER JOIN hold_fill_targets t
+        ON  q.borrowernumber = t.borrowernumber
+            AND q.biblionumber = t.biblionumber
+            AND q.itemnumber = t.itemnumber
+            AND q.item_level_request = t.item_level_request
+            AND q.holdingbranch = t.source_branchcode
+        WHERE t.reserve_id = ?
+    });
+    $std->execute($hold->reserve_id);
+
+    logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold )
+        if C4::Context->preference('HoldsLog');
+
      return;
  }
  
  =head2 ModReserveCancelAll
  
      return;
  }
  
  =head2 ModReserveCancelAll
  
-  ($messages,$nextreservinfo) = &ModReserveCancelAll($itemnumber,$borrowernumber);
+  ($messages,$nextreservinfo) = &ModReserveCancelAll($itemnumber,$borrowernumber,$reason);
  
  function to cancel reserv,check other reserves, and transfer document if it's necessary
  
  
  function to cancel reserv,check other reserves, and transfer document if it's necessary
  
@@ -1130,17 +1286,17 @@ function to cancel reserv,check other reserves, and transfer document if it's ne
  sub ModReserveCancelAll {
      my $messages;
      my $nextreservinfo;
  sub ModReserveCancelAll {
      my $messages;
      my $nextreservinfo;
-    my ( $itemnumber, $borrowernumber ) = @_;
+    my ( $itemnumber, $borrowernumber, $cancellation_reason ) = @_;
  
      #step 1 : cancel the reservation
      my $holds = Koha::Holds->search({ itemnumber => $itemnumber, borrowernumber => $borrowernumber });
      return unless $holds->count;
  
      #step 1 : cancel the reservation
      my $holds = Koha::Holds->search({ itemnumber => $itemnumber, borrowernumber => $borrowernumber });
      return unless $holds->count;
-    $holds->next->cancel;
+    $holds->next->cancel({ cancellation_reason => $cancellation_reason });
  
      #step 2 launch the subroutine of the others reserves
      ( $messages, $nextreservinfo ) = GetOtherReserves($itemnumber);
  
  
      #step 2 launch the subroutine of the others reserves
      ( $messages, $nextreservinfo ) = GetOtherReserves($itemnumber);
  
-    return ( $messages, $nextreservinfo );
+    return ( $messages, $nextreservinfo->{borrowernumber} );
  }
  
  =head2 ModReserveMinusPriority
  }
  
  =head2 ModReserveMinusPriority
@@ -1169,7 +1325,7 @@ sub ModReserveMinusPriority {
  
  =head2 IsAvailableForItemLevelRequest
  
  
  =head2 IsAvailableForItemLevelRequest
  
-  my $is_available = IsAvailableForItemLevelRequest($item_record,$borrower_record);
+  my $is_available = IsAvailableForItemLevelRequest( $item_record, $borrower_record, $pickup_branchcode );
  
  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
@@ -1177,6 +1333,7 @@ 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
+* a waiting or in transit reserve is placed on
  * does not have a not for loan value > 0
  
  Need to check the issuingrules onshelfholds column,
  * does not have a not for loan value > 0
  
  Need to check the issuingrules onshelfholds column,
@@ -1188,110 +1345,113 @@ a request on the item - in particular,
  this routine does not check IndependentBranches
  and canreservefromotherbranches.
  
  this routine does not check IndependentBranches
  and canreservefromotherbranches.
  
+Note also that this subroutine does not checks smart
+rules limits for item by reservesallowed/holds_per_record
+values, this complemented in calling code with calls and
+checks with CanItemBeReserved or CanBookBeReserved.
+
  =cut
  
  sub IsAvailableForItemLevelRequest {
  =cut
  
  sub IsAvailableForItemLevelRequest {
-    my $item = shift;
-    my $borrower = shift;
+    my $item                = shift;
+    my $patron              = shift;
+    my $pickup_branchcode   = shift;
+    # items_any_available is precalculated status passed from request.pl when set of items
+    # looped outside of IsAvailableForItemLevelRequest to avoid nested loops:
+    my $items_any_available = 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
  
      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
-    my $itype = _get_itype($item);
-    my $notforloan_per_itemtype
-      = $dbh->selectrow_array("SELECT notforloan FROM itemtypes WHERE itemtype = ?",
-                              undef, $itype);
+    my $itemtype = $item->effective_itemtype;
+    return 0
+      unless defined $itemtype;
+    my $notforloan_per_itemtype = Koha::ItemTypes->find($itemtype)->notforloan;
  
      return 0 if
          $notforloan_per_itemtype ||
  
      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});
+        $item->itemlost        ||
+        $item->notforloan > 0  || # item with negative or zero notforloan value is holdable
+        $item->withdrawn        ||
+        ($item->damaged && !C4::Context->preference('AllowHoldsOnDamagedItems'));
+
+    if ($pickup_branchcode) {
+        my $destination = Koha::Libraries->find($pickup_branchcode);
+        return 0 unless $destination;
+        return 0 unless $destination->pickup_location;
+        return 0 unless $item->can_be_transferred( { to => $destination } );
+        my $reserves_control_branch =
+            GetReservesControlBranch( $item->unblessed(), $patron->unblessed() );
+        my $branchitemrule =
+            C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->itype );
+        my $home_library = Koha::Libraries->find( {branchcode => $item->homebranch} );
+        return 0 unless $branchitemrule->{hold_fulfillment_policy} ne 'holdgroup' || $home_library->validate_hold_sibling( {branchcode => $pickup_branchcode} );
+    }
+
+    my $on_shelf_holds = Koha::CirculationRules->get_onshelfholds_policy( { item => $item, patron => $patron } );
  
      if ( $on_shelf_holds == 1 ) {
          return 1;
      } elsif ( $on_shelf_holds == 2 ) {
  
      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;
-        }
  
  
+        # if we have this param predefined from outer caller sub, we just need
+        # to return it, so we saving from having loop inside other loop:
+        return  $items_any_available ? 0 : 1
+            if defined $items_any_available;
+
+        my $any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron });
          return $any_available ? 0 : 1;
          return $any_available ? 0 : 1;
+    } else { # on_shelf_holds == 0 "If any unavailable" (the description is rather cryptic and could still be improved)
+        return $item->onloan || IsItemOnHoldAndFound( $item->itemnumber );
      }
      }
-
-    return $item->{onloan} || GetReserveStatus($item->{itemnumber}) eq "Waiting";
  }
  
  }
  
-=head2 OnShelfHoldsAllowed
+=head2 ItemsAnyAvailableAndNotRestricted
  
  
-  OnShelfHoldsAllowed($itemtype,$borrowercategory,$branchcode);
+  ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblionumber, patron => $patron });
  
  
-Checks issuingrules, using the borrowers categorycode, the itemtype, and branchcode to see if onshelf
-holds are allowed, returns true if so.
+This function checks all items for specified biblionumber (numeric) against patron (object)
+and returns true (1) if at least one item available for loan/check out/present/not held
+and also checks other parameters logic which not restricts item for hold at all (for ex.
+AllowHoldsOnDamagedItems or 'holdallowed' own/sibling library)
  
  =cut
  
  
  =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 $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule({ categorycode => $borrowercategory, itemtype => $itype, branchcode => $branchcode });
-    return $issuing_rule ? $issuing_rule->onshelfholds : undef;
+sub ItemsAnyAvailableAndNotRestricted {
+    my $param = shift;
+
+    my @items = Koha::Items->search( { biblionumber => $param->{biblionumber} } )->as_list;
+
+    foreach my $i (@items) {
+        my $reserves_control_branch =
+            GetReservesControlBranch( $i->unblessed(), $param->{patron}->unblessed );
+        my $branchitemrule =
+            C4::Circulation::GetBranchItemRule( $reserves_control_branch, $i->itype );
+        my $item_library = Koha::Libraries->find( { branchcode => $i->homebranch } );
+
+        # we can return (end the loop) when first one found:
+        return 1
+            unless $i->itemlost
+            || $i->notforloan # items with non-zero notforloan cannot be checked out
+            || $i->withdrawn
+            || $i->onloan
+            || IsItemOnHoldAndFound( $i->id )
+            || ( $i->damaged
+                 && ! C4::Context->preference('AllowHoldsOnDamagedItems') )
+            || Koha::ItemTypes->find( $i->effective_itemtype() )->notforloan
+            || $branchitemrule->{holdallowed} eq 'from_home_library' && $param->{patron}->branchcode ne $i->homebranch
+            || $branchitemrule->{holdallowed} eq 'from_local_hold_group' && ! $item_library->validate_hold_sibling( { branchcode => $param->{patron}->branchcode } )
+            || CanItemBeReserved( $param->{patron}, $i )->{status} ne 'OK';
+    }
+
+    return 0;
  }
  
  =head2 AlterPriority
  
  }
  
  =head2 AlterPriority
  
-  AlterPriority( $where, $reserve_id );
+  AlterPriority( $where, $reserve_id, $prev_priority, $next_priority, $first_priority, $last_priority );
  
  This function changes a reserve's priority up, down, to the top, or to the bottom.
  Input: $where is 'up', 'down', 'top' or 'bottom'. Biblionumber, Date reserve was placed
  
  This function changes a reserve's priority up, down, to the top, or to the bottom.
  Input: $where is 'up', 'down', 'top' or 'bottom'. Biblionumber, Date reserve was placed
@@ -1299,7 +1459,7 @@ Input: $where is 'up', 'down', 'top' or 'bottom'. Biblionumber, Date reserve was
  =cut
  
  sub AlterPriority {
  =cut
  
  sub AlterPriority {
-    my ( $where, $reserve_id ) = @_;
+    my ( $where, $reserve_id, $prev_priority, $next_priority, $first_priority, $last_priority ) = @_;
  
      my $hold = Koha::Holds->find( $reserve_id );
      return unless $hold;
  
      my $hold = Koha::Holds->find( $reserve_id );
      return unless $hold;
@@ -1309,21 +1469,23 @@ sub AlterPriority {
          return;
      }
  
          return;
      }
  
-    if ( $where eq 'up' || $where eq 'down' ) {
-
-      my $priority = $hold->priority;
-      $priority = $where eq 'up' ? $priority - 1 : $priority + 1;
-      _FixPriority({ reserve_id => $reserve_id, rank => $priority })
-
+    if ( $where eq 'up' ) {
+      return unless $prev_priority;
+      _FixPriority({ reserve_id => $reserve_id, rank => $prev_priority })
+    } elsif ( $where eq 'down' ) {
+      return unless $next_priority;
+      _FixPriority({ reserve_id => $reserve_id, rank => $next_priority })
      } elsif ( $where eq 'top' ) {
      } elsif ( $where eq 'top' ) {
-
-      _FixPriority({ reserve_id => $reserve_id, rank => '1' })
-
+      _FixPriority({ reserve_id => $reserve_id, rank => $first_priority })
      } elsif ( $where eq 'bottom' ) {
      } elsif ( $where eq 'bottom' ) {
-
-      _FixPriority({ reserve_id => $reserve_id, rank => '999999' });
-
+      _FixPriority({ reserve_id => $reserve_id, rank => $last_priority });
      }
      }
+
+    Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue(
+        {
+            biblio_ids => [ $hold->biblionumber ]
+        }
+    ) if C4::Context->preference('RealTimeHoldsQueue');
      # FIXME Should return the new priority
  }
  
      # FIXME Should return the new priority
  }
  
@@ -1359,8 +1521,6 @@ be cleared when it is unsuspended.
  sub ToggleSuspend {
      my ( $reserve_id, $suspend_until ) = @_;
  
  sub ToggleSuspend {
      my ( $reserve_id, $suspend_until ) = @_;
  
-    $suspend_until = dt_from_string($suspend_until) if ($suspend_until);
-
      my $hold = Koha::Holds->find( $reserve_id );
  
      if ( $hold->is_suspended ) {
      my $hold = Koha::Holds->find( $reserve_id );
  
      if ( $hold->is_suspended ) {
@@ -1394,9 +1554,6 @@ sub SuspendAll {
      my $suspend_until  = $params{'suspend_until'}  || undef;
      my $suspend = defined( $params{'suspend'} ) ? $params{'suspend'} : 1;
  
      my $suspend_until  = $params{'suspend_until'}  || undef;
      my $suspend = defined( $params{'suspend'} ) ? $params{'suspend'} : 1;
  
-    $suspend_until = eval { dt_from_string($suspend_until) }
-      if ( defined($suspend_until) );
-
      return unless ( $borrowernumber || $biblionumber );
  
      my $params;
      return unless ( $borrowernumber || $biblionumber );
  
      my $params;
@@ -1404,7 +1561,7 @@ sub SuspendAll {
      $params->{borrowernumber} = $borrowernumber if $borrowernumber;
      $params->{biblionumber}   = $biblionumber if $biblionumber;
  
      $params->{borrowernumber} = $borrowernumber if $borrowernumber;
      $params->{biblionumber}   = $biblionumber if $biblionumber;
  
-    my @holds = Koha::Holds->search($params);
+    my @holds = Koha::Holds->search($params)->as_list;
  
      if ($suspend) {
          map { $_->suspend_hold($suspend_until) } @holds;
  
      if ($suspend) {
          map { $_->suspend_hold($suspend_until) } @holds;
@@ -1462,6 +1619,10 @@ sub _FixPriority {
      my $hold;
      if ( $reserve_id ) {
          $hold = Koha::Holds->find( $reserve_id );
      my $hold;
      if ( $reserve_id ) {
          $hold = Koha::Holds->find( $reserve_id );
+        if (!defined $hold){
+            # may have already been checked out and hold fulfilled
+            $hold = Koha::Old::Holds->find( $reserve_id );
+        }
          return unless $hold;
      }
  
          return unless $hold;
      }
  
@@ -1472,14 +1633,14 @@ sub _FixPriority {
      if ( $rank eq "del" ) { # FIXME will crash if called without $hold
          $hold->cancel;
      }
      if ( $rank eq "del" ) { # FIXME will crash if called without $hold
          $hold->cancel;
      }
-    elsif ( $rank eq "W" || $rank eq "0" ) {
+    elsif ( $reserve_id && ( $rank eq "W" || $rank eq "0" ) ) {
  
          # make sure priority for waiting or in-transit items is 0
          my $query = "
              UPDATE reserves
              SET    priority = 0
              WHERE reserve_id = ?
  
          # make sure priority for waiting or in-transit items is 0
          my $query = "
              UPDATE reserves
              SET    priority = 0
              WHERE reserve_id = ?
-            AND found IN ('W', 'T')
+            AND found IN ('W', 'T', 'P')
          ";
          my $sth = $dbh->prepare($query);
          $sth->execute( $reserve_id );
          ";
          my $sth = $dbh->prepare($query);
          $sth->execute( $reserve_id );
@@ -1491,7 +1652,7 @@ sub _FixPriority {
          SELECT reserve_id, borrowernumber, reservedate
          FROM   reserves
          WHERE  biblionumber   = ?
          SELECT reserve_id, borrowernumber, reservedate
          FROM   reserves
          WHERE  biblionumber   = ?
-          AND  ((found <> 'W' AND found <> 'T') OR found IS NULL)
+          AND  ((found <> 'W' AND found <> 'T' AND found <> 'P') OR found IS NULL)
          ORDER BY priority ASC
      ";
      my $sth = $dbh->prepare($query);
          ORDER BY priority ASC
      ";
      my $sth = $dbh->prepare($query);
@@ -1500,11 +1661,12 @@ sub _FixPriority {
          push( @priority,     $line );
      }
  
          push( @priority,     $line );
      }
  
+    # FIXME This whole sub must be rewritten, especially to highlight what is done when reserve_id is not given
      # To find the matching index
      my $i;
      my $key = -1;    # to allow for 0 to be a valid result
      for ( $i = 0 ; $i < @priority ; $i++ ) {
      # To find the matching index
      my $i;
      my $key = -1;    # to allow for 0 to be a valid result
      for ( $i = 0 ; $i < @priority ; $i++ ) {
-        if ( $reserve_id == $priority[$i]->{'reserve_id'} ) {
+        if ( $reserve_id && $reserve_id == $priority[$i]->{'reserve_id'} ) {
              $key = $i;    # save the index
              last;
          }
              $key = $i;    # save the index
              last;
          }
@@ -1512,9 +1674,9 @@ sub _FixPriority {
  
      # if index exists in array then move it to new position
      if ( $key > -1 && $rank ne 'del' && $rank > 0 ) {
  
      # if index exists in array then move it to new position
      if ( $key > -1 && $rank ne 'del' && $rank > 0 ) {
-        my $new_rank = $rank -
-          1;    # $new_rank is what you want the new index to be in the array
+        my $new_rank = $rank - 1; # $new_rank is what you want the new index to be in the array
          my $moving_item = splice( @priority, $key, 1 );
          my $moving_item = splice( @priority, $key, 1 );
+        $new_rank = scalar @priority if $new_rank > scalar @priority;
          splice( @priority, $new_rank, 0, $moving_item );
      }
  
          splice( @priority, $new_rank, 0, $moving_item );
      }
  
@@ -1532,10 +1694,9 @@ sub _FixPriority {
          );
      }
  
          );
      }
  
-    $sth = $dbh->prepare( "SELECT reserve_id FROM reserves WHERE lowestPriority = 1 ORDER BY priority" );
-    $sth->execute();
-
      unless ( $ignoreSetLowestRank ) {
      unless ( $ignoreSetLowestRank ) {
+        $sth = $dbh->prepare( "SELECT reserve_id FROM reserves WHERE lowestPriority = 1 AND biblionumber = ? ORDER BY priority" );
+        $sth->execute($biblionumber);
        while ( my $res = $sth->fetchrow_hashref() ) {
          _FixPriority({
              reserve_id => $res->{'reserve_id'},
        while ( my $res = $sth->fetchrow_hashref() ) {
          _FixPriority({
              reserve_id => $res->{'reserve_id'},
@@ -1559,6 +1720,13 @@ C<@results> is an array of references-to-hash whose keys are mostly
  fields from the reserves table of the Koha database, plus
  C<biblioitemnumber>.
  
  fields from the reserves table of the Koha database, plus
  C<biblioitemnumber>.
  
+This routine with either return:
+1 - Item specific holds from the holds queue
+2 - Title level holds from the holds queue
+3 - All holds for this biblionumber
+
+All return values will respect any borrowernumbers passed as arrayref in $ignore_borrowers
+
  =cut
  
  sub _Findgroupreserve {
  =cut
  
  sub _Findgroupreserve {
@@ -1580,14 +1748,15 @@ sub _Findgroupreserve {
                 biblioitems.biblioitemnumber AS biblioitemnumber,
                 reserves.itemnumber          AS itemnumber,
                 reserves.reserve_id          AS reserve_id,
                 biblioitems.biblioitemnumber AS biblioitemnumber,
                 reserves.itemnumber          AS itemnumber,
                 reserves.reserve_id          AS reserve_id,
-               reserves.itemtype            AS itemtype
+               reserves.itemtype            AS itemtype,
+               reserves.non_priority        AS non_priority
          FROM reserves
          JOIN biblioitems USING (biblionumber)
          FROM reserves
          JOIN biblioitems USING (biblionumber)
-        JOIN hold_fill_targets USING (biblionumber, borrowernumber, itemnumber)
+        JOIN hold_fill_targets USING (reserve_id)
          WHERE found IS NULL
          AND priority > 0
          AND item_level_request = 1
          WHERE found IS NULL
          AND priority > 0
          AND item_level_request = 1
-        AND itemnumber = ?
+        AND hold_fill_targets.itemnumber = ?
          AND reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY)
          AND suspend = 0
          ORDER BY priority
          AND reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY)
          AND suspend = 0
          ORDER BY priority
@@ -1615,10 +1784,11 @@ sub _Findgroupreserve {
                 biblioitems.biblioitemnumber AS biblioitemnumber,
                 reserves.itemnumber          AS itemnumber,
                 reserves.reserve_id          AS reserve_id,
                 biblioitems.biblioitemnumber AS biblioitemnumber,
                 reserves.itemnumber          AS itemnumber,
                 reserves.reserve_id          AS reserve_id,
-               reserves.itemtype            AS itemtype
+               reserves.itemtype            AS itemtype,
+               reserves.non_priority        AS non_priority
          FROM reserves
          JOIN biblioitems USING (biblionumber)
          FROM reserves
          JOIN biblioitems USING (biblionumber)
-        JOIN hold_fill_targets USING (biblionumber, borrowernumber)
+        JOIN hold_fill_targets USING (reserve_id)
          WHERE found IS NULL
          AND priority > 0
          AND item_level_request = 0
          WHERE found IS NULL
          AND priority > 0
          AND item_level_request = 0
@@ -1649,7 +1819,8 @@ sub _Findgroupreserve {
                 reserves.timestamp                  AS timestamp,
                 reserves.itemnumber                 AS itemnumber,
                 reserves.reserve_id                 AS reserve_id,
                 reserves.timestamp                  AS timestamp,
                 reserves.itemnumber                 AS itemnumber,
                 reserves.reserve_id                 AS reserve_id,
-               reserves.itemtype                   AS itemtype
+               reserves.itemtype                   AS itemtype,
+               reserves.non_priority        AS non_priority
          FROM reserves
          WHERE reserves.biblionumber = ?
            AND (reserves.itemnumber IS NULL OR reserves.itemnumber = ?)
          FROM reserves
          WHERE reserves.biblionumber = ?
            AND (reserves.itemnumber IS NULL OR reserves.itemnumber = ?)
@@ -1672,7 +1843,7 @@ sub _Findgroupreserve {
    _koha_notify_reserve( $hold->reserve_id );
  
  Sends a notification to the patron that their hold has been filled (through
    _koha_notify_reserve( $hold->reserve_id );
  
  Sends a notification to the patron that their hold has been filled (through
-ModReserveAffect, _not_ ModReserveFill)
+ModReserveAffect)
  
  The letter code for this notice may be found using the following query:
  
  
  The letter code for this notice may be found using the following query:
  
@@ -1697,29 +1868,29 @@ The following tables are availalbe witin the notice:
  
  sub _koha_notify_reserve {
      my $reserve_id = shift;
  
  sub _koha_notify_reserve {
      my $reserve_id = shift;
+
      my $hold = Koha::Holds->find($reserve_id);
      my $borrowernumber = $hold->borrowernumber;
  
      my $patron = Koha::Patrons->find( $borrowernumber );
  
      # Try to get the borrower's email address
      my $hold = Koha::Holds->find($reserve_id);
      my $borrowernumber = $hold->borrowernumber;
  
      my $patron = Koha::Patrons->find( $borrowernumber );
  
      # Try to get the borrower's email address
-    my $to_address = C4::Members::GetNoticeEmailAddress($borrowernumber);
+    my $to_address = $patron->notice_email_address;
  
      my $messagingprefs = C4::Members::Messaging::GetMessagingPreferences( {
              borrowernumber => $borrowernumber,
              message_name => 'Hold_Filled'
      } );
  
  
      my $messagingprefs = C4::Members::Messaging::GetMessagingPreferences( {
              borrowernumber => $borrowernumber,
              message_name => 'Hold_Filled'
      } );
  
-    my $library = Koha::Libraries->find( $hold->branchcode )->unblessed;
-
-    my $admin_email_address = $library->{branchemail} || C4::Context->preference('KohaAdminEmailAddress');
+    my $library = Koha::Libraries->find( $hold->branchcode );
+    my $inbound_email_address = $library->inbound_email_address;
  
      my %letter_params = (
          module => 'reserves',
          branchcode => $hold->branchcode,
          lang => $patron->lang,
          tables => {
  
      my %letter_params = (
          module => 'reserves',
          branchcode => $hold->branchcode,
          lang => $patron->lang,
          tables => {
-            'branches'       => $library,
+            'branches'       => $library->unblessed,
              'borrowers'      => $patron->unblessed,
              'biblio'         => $hold->biblionumber,
              'biblioitems'    => $hold->biblionumber,
              'borrowers'      => $patron->unblessed,
              'biblio'         => $hold->biblionumber,
              'biblioitems'    => $hold->biblionumber,
@@ -1743,7 +1914,7 @@ sub _koha_notify_reserve {
          C4::Letters::EnqueueLetter( {
              letter => $letter,
              borrowernumber => $borrowernumber,
          C4::Letters::EnqueueLetter( {
              letter => $letter,
              borrowernumber => $borrowernumber,
-            from_address => $admin_email_address,
+            from_address => $inbound_email_address,
              message_transport_type => $mtt,
          } );
      };
              message_transport_type => $mtt,
          } );
      };
@@ -1752,7 +1923,8 @@ sub _koha_notify_reserve {
          next if (
                 ( $mtt eq 'email' and not $to_address ) # No email address
              or ( $mtt eq 'sms'   and not $patron->smsalertnumber ) # No SMS number
          next if (
                 ( $mtt eq 'email' and not $to_address ) # No email address
              or ( $mtt eq 'sms'   and not $patron->smsalertnumber ) # No SMS number
-            or ( $mtt eq 'phone' and C4::Context->preference('TalkingTechItivaPhoneNotification') ) # Notice is handled by TalkingTech_itiva_outbound.pl
+            or ( $mtt eq 'itiva' and C4::Context->preference('TalkingTechItivaPhoneNotification') ) # Notice is handled by TalkingTech_itiva_outbound.pl
+            or ( $mtt eq 'phone' and not $patron->phone ) # No phone number to call
          );
  
          &$send_notification($mtt, $letter_code);
          );
  
          &$send_notification($mtt, $letter_code);
@@ -1765,9 +1937,53 @@ sub _koha_notify_reserve {
  
  }
  
  
  }
  
-=head2 _ShiftPriorityByDateAndPriority
+=head2 _koha_notify_hold_changed
+
+  _koha_notify_hold_changed( $hold_object );
+
+=cut
+
+sub _koha_notify_hold_changed {
+    my $hold = shift;
+
+    my $patron = $hold->patron;
+    my $library = $hold->branch;
+
+    my $letter = C4::Letters::GetPreparedLetter(
+        module      => 'reserves',
+        letter_code => 'HOLD_CHANGED',
+        branchcode  => $hold->branchcode,
+        substitute  => { today => output_pref( dt_from_string ) },
+        tables      => {
+            'branches'    => $library->unblessed,
+            'borrowers'   => $patron->unblessed,
+            'biblio'      => $hold->biblionumber,
+            'biblioitems' => $hold->biblionumber,
+            'reserves'    => $hold->unblessed,
+            'items'       => $hold->itemnumber,
+        },
+    );
+
+    return unless $letter;
+
+    my $email =
+         C4::Context->preference('ExpireReservesAutoFillEmail')
+      || $library->inbound_email_address;
+
+    C4::Letters::EnqueueLetter(
+        {
+            letter                 => $letter,
+            borrowernumber         => $patron->id,
+            message_transport_type => 'email',
+            from_address           => $email,
+            to_address             => $email,
+        }
+    );
+}
  
  
-  $new_priority = _ShiftPriorityByDateAndPriority( $biblionumber, $reservedate, $priority );
+=head2 _ShiftPriority
+
+  $new_priority = _ShiftPriority( $biblionumber, $priority );
  
  This increments the priority of all reserves after the one
  with either the lowest date after C<$reservedate>
  
  This increments the priority of all reserves after the one
  with either the lowest date after C<$reservedate>
@@ -1783,13 +1999,13 @@ the sub accounts for that too.
  
  =cut
  
  
  =cut
  
-sub _ShiftPriorityByDateAndPriority {
-    my ( $biblio, $resdate, $new_priority ) = @_;
+sub _ShiftPriority {
+    my ( $biblio, $new_priority ) = @_;
  
      my $dbh = C4::Context->dbh;
  
      my $dbh = C4::Context->dbh;
-    my $query = "SELECT priority FROM reserves WHERE biblionumber = ? AND ( reservedate > ? OR priority > ? ) ORDER BY priority ASC LIMIT 1";
+    my $query = "SELECT priority FROM reserves WHERE biblionumber = ? AND priority > ? ORDER BY priority ASC LIMIT 1";
      my $sth = $dbh->prepare( $query );
      my $sth = $dbh->prepare( $query );
-    $sth->execute( $biblio, $resdate, $new_priority );
+    $sth->execute( $biblio, $new_priority );
      my $min_priority = $sth->fetchrow;
      # if no such matches are found, $new_priority remains as original value
      $new_priority = $min_priority if ( $min_priority );
      my $min_priority = $sth->fetchrow;
      # if no such matches are found, $new_priority remains as original value
      $new_priority = $min_priority if ( $min_priority );
@@ -1814,54 +2030,6 @@ 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 )
@@ -1874,32 +2042,33 @@ If $cancelreserve boolean is set to true, it will remove existing reserve
  sub MoveReserve {
      my ( $itemnumber, $borrowernumber, $cancelreserve ) = @_;
  
  sub MoveReserve {
      my ( $itemnumber, $borrowernumber, $cancelreserve ) = @_;
  
+    $cancelreserve //= 0;
+
      my $lookahead = C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds
      my $lookahead = C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds
-    my ( $restype, $res, $all_reserves ) = CheckReserves( $itemnumber, undef, $lookahead );
+    my ( $restype, $res, undef ) = CheckReserves( $itemnumber, undef, $lookahead );
      return unless $res;
  
      return unless $res;
  
-    my $biblionumber     =  $res->{biblionumber};
+    my $biblionumber = $res->{biblionumber};
  
      if ($res->{borrowernumber} == $borrowernumber) {
  
      if ($res->{borrowernumber} == $borrowernumber) {
-        ModReserveFill($res);
+        my $hold = Koha::Holds->find( $res->{reserve_id} );
+        $hold->fill;
      }
      else {
          # warn "Reserved";
          # The item is reserved by someone else.
          # Find this item in the reserves
  
      }
      else {
          # warn "Reserved";
          # The item is reserved by someone else.
          # Find this item in the reserves
  
-        my $borr_res;
-        foreach (@$all_reserves) {
-            $_->{'borrowernumber'} == $borrowernumber or next;
-            $_->{'biblionumber'}   == $biblionumber   or next;
-
-            $borr_res = $_;
-            last;
-        }
+        my $borr_res  = Koha::Holds->search({
+            borrowernumber => $borrowernumber,
+            biblionumber   => $biblionumber,
+        },{
+            order_by       => 'priority'
+        })->next();
  
          if ( $borr_res ) {
              # The item is reserved by the current patron
  
          if ( $borr_res ) {
              # The item is reserved by the current patron
-            ModReserveFill($borr_res);
+            $borr_res->fill;
          }
  
          if ( $cancelreserve eq 'revert' ) { ## Revert waiting reserve to priority 1
          }
  
          if ( $cancelreserve eq 'revert' ) { ## Revert waiting reserve to priority 1
@@ -1937,13 +2106,13 @@ sub MergeHolds {
          # don't reorder those already waiting
  
          $sth = $dbh->prepare(
          # don't reorder those already waiting
  
          $sth = $dbh->prepare(
-"SELECT * FROM reserves WHERE biblionumber = ? AND (found <> ? AND found <> ? OR found is NULL) ORDER BY reservedate ASC"
+"SELECT * FROM reserves WHERE biblionumber = ? AND (found NOT IN ('W', 'T', 'P') OR found is NULL) ORDER BY reservedate ASC"
          );
          my $upd_sth = $dbh->prepare(
  "UPDATE reserves SET priority = ? WHERE biblionumber = ? AND borrowernumber = ?
          AND reservedate = ? AND (itemnumber = ? or itemnumber is NULL) "
          );
          );
          my $upd_sth = $dbh->prepare(
  "UPDATE reserves SET priority = ? WHERE biblionumber = ? AND borrowernumber = ?
          AND reservedate = ? AND (itemnumber = ? or itemnumber is NULL) "
          );
-        $sth->execute( $to_biblio, 'W', 'T' );
+        $sth->execute( $to_biblio );
          my $priority = 1;
          while ( my $reserve = $sth->fetchrow_hashref() ) {
              $upd_sth->execute(
          my $priority = 1;
          while ( my $reserve = $sth->fetchrow_hashref() ) {
              $upd_sth->execute(
@@ -1979,47 +2148,52 @@ sub RevertWaitingStatus {
      my $dbh = C4::Context->dbh;
  
      ## Get the waiting reserve we want to revert
      my $dbh = C4::Context->dbh;
  
      ## Get the waiting reserve we want to revert
-    my $query = "
-        SELECT * FROM reserves
-        WHERE itemnumber = ?
-        AND found IS NOT NULL
-    ";
-    my $sth = $dbh->prepare( $query );
-    $sth->execute( $itemnumber );
-    my $reserve = $sth->fetchrow_hashref();
+    my $hold = Koha::Holds->search(
+        {
+            itemnumber => $itemnumber,
+            found => { not => undef },
+        }
+    )->next;
  
      ## Increment the priority of all other non-waiting
      ## reserves for this bib record
  
      ## Increment the priority of all other non-waiting
      ## reserves for this bib record
-    $query = "
-        UPDATE reserves
-        SET
-          priority = priority + 1
-        WHERE
-          biblionumber =  ?
-        AND
-          priority > 0
-    ";
-    $sth = $dbh->prepare( $query );
-    $sth->execute( $reserve->{'biblionumber'} );
+    my $holds = Koha::Holds->search({ biblionumber => $hold->biblionumber, priority => { '>' => 0 } })
+                           ->update({ priority => \'priority + 1' }, { no_triggers => 1 });
  
      ## Fix up the currently waiting reserve
  
      ## Fix up the currently waiting reserve
-    $query = "
-    UPDATE reserves
-    SET
-      priority = 1,
-      found = NULL,
-      waitingdate = NULL
-    WHERE
-      reserve_id = ?
-    ";
-    $sth = $dbh->prepare( $query );
-    $sth->execute( $reserve->{'reserve_id'} );
-    _FixPriority( { biblionumber => $reserve->{biblionumber} } );
+    $hold->set(
+        {
+            priority    => 1,
+            found       => undef,
+            waitingdate => undef,
+            expirationdate => $hold->patron_expiration_date,
+            itemnumber  => $hold->item_level_hold ? $hold->itemnumber : undef,
+        }
+    )->store();
+
+    _FixPriority( { biblionumber => $hold->biblionumber } );
+
+    Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue(
+        {
+            biblio_ids => [ $hold->biblionumber ]
+        }
+    ) if C4::Context->preference('RealTimeHoldsQueue');
+
+
+    return $hold;
  }
  
  =head2 ReserveSlip
  
  }
  
  =head2 ReserveSlip
  
-  ReserveSlip($branchcode, $borrowernumber, $biblionumber)
+ReserveSlip(
+    {
+        branchcode     => $branchcode,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        [ itemnumber   => $itemnumber, ]
+        [ barcode      => $barcode, ]
+    }
+  )
  
  Returns letter hash ( see C4::Letters::GetPreparedLetter ) or undef
  
  
  Returns letter hash ( see C4::Letters::GetPreparedLetter ) or undef
  
@@ -2036,19 +2210,20 @@ available within the slip:
  =cut
  
  sub ReserveSlip {
  =cut
  
  sub ReserveSlip {
-    my ($branch, $borrowernumber, $biblionumber) = @_;
-
-#   return unless ( C4::Context->boolean_preference('printreserveslips') );
-    my $patron = Koha::Patrons->find( $borrowernumber );
+    my ($args) = @_;
+    my $branchcode     = $args->{branchcode};
+    my $reserve_id = $args->{reserve_id};
  
  
-    my $hold = Koha::Holds->search({biblionumber => $biblionumber, borrowernumber => $borrowernumber })->next;
+    my $hold = Koha::Holds->find($reserve_id);
      return unless $hold;
      return unless $hold;
+
+    my $patron = $hold->borrower;
      my $reserve = $hold->unblessed;
  
      return  C4::Letters::GetPreparedLetter (
          module => 'circulation',
          letter_code => 'HOLD_SLIP',
      my $reserve = $hold->unblessed;
  
      return  C4::Letters::GetPreparedLetter (
          module => 'circulation',
          letter_code => 'HOLD_SLIP',
-        branchcode => $branch,
+        branchcode => $branchcode,
          lang => $patron->lang,
          tables => {
              'reserves'    => $reserve,
          lang => $patron->lang,
          tables => {
              'reserves'    => $reserve,
@@ -2101,7 +2276,7 @@ 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
  the parameter value.
  
  After calculation of this priority, it is recommended to call
-_ShiftPriorityByDateAndPriority. Note that this is currently done in
+_ShiftPriority. Note that this is currently done in
  AddReserves.
  
  =cut
  AddReserves.
  
  =cut
@@ -2115,7 +2290,7 @@ sub CalculatePriority {
          AND   priority > 0
          AND   (found IS NULL OR found = '')
      };
          AND   priority > 0
          AND   (found IS NULL OR found = '')
      };
-    #skip found==W or found==T (waiting or transit holds)
+    #skip found==W or found==T or found==P (waiting, transit or processing holds)
      if( $resdate ) {
          $sql.= ' AND ( reservedate <= ? )';
      }
      if( $resdate ) {
          $sql.= ' AND ( reservedate <= ? )';
      }
@@ -2172,7 +2347,7 @@ sub GetMaxPatronHoldsForRecord {
      my ( $borrowernumber, $biblionumber ) = @_;
  
      my $patron = Koha::Patrons->find($borrowernumber);
      my ( $borrowernumber, $biblionumber ) = @_;
  
      my $patron = Koha::Patrons->find($borrowernumber);
-    my @items = Koha::Items->search( { biblionumber => $biblionumber } );
+    my @items = Koha::Items->search( { biblionumber => $biblionumber } )->as_list;
  
      my $controlbranch = C4::Context->preference('ReservesControlBranch');
  
  
      my $controlbranch = C4::Context->preference('ReservesControlBranch');
  
@@ -2186,46 +2361,19 @@ sub GetMaxPatronHoldsForRecord {
  
          $branchcode = $item->homebranch if ( $controlbranch eq "ItemHomeLibrary" );
  
  
          $branchcode = $item->homebranch if ( $controlbranch eq "ItemHomeLibrary" );
  
-        my $rule = GetHoldRule( $categorycode, $itemtype, $branchcode );
-        my $holds_per_record = $rule ? $rule->{holds_per_record} : 0;
+        my $rule = Koha::CirculationRules->get_effective_rule({
+            categorycode => $categorycode,
+            itemtype     => $itemtype,
+            branchcode   => $branchcode,
+            rule_name    => 'holds_per_record'
+        });
+        my $holds_per_record = $rule ? $rule->rule_value : 0;
          $max = $holds_per_record if $holds_per_record > $max;
      }
  
      return $max;
  }
  
          $max = $holds_per_record if $holds_per_record > $max;
      }
  
      return $max;
  }
  
-=head2 GetHoldRule
-
-my $rule = GetHoldRule( $categorycode, $itemtype, $branchcode );
-
-Returns the matching hold related issuingrule fields for a given
-patron category, itemtype, and library.
-
-=cut
-
-sub GetHoldRule {
-    my ( $categorycode, $itemtype, $branchcode ) = @_;
-
-    my $dbh = C4::Context->dbh;
-
-    my $sth = $dbh->prepare(
-        q{
-         SELECT categorycode, itemtype, branchcode, reservesallowed, holds_per_record
-           FROM issuingrules
-          WHERE (categorycode in (?,'*') )
-            AND (itemtype IN (?,'*'))
-            AND (branchcode IN (?,'*'))
-       ORDER BY categorycode DESC,
-                itemtype     DESC,
-                branchcode   DESC
-        }
-    );
-
-    $sth->execute( $categorycode, $itemtype, $branchcode );
-
-    return $sth->fetchrow_hashref();
-}
-
  =head1 AUTHOR
  
  Koha Development Team <http://koha-community.org/>
  =head1 AUTHOR
  
  Koha Development Team <http://koha-community.org/>