Bug 3142: Use filter_by_for_holds where holdability is calculated

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

C4/Search.pm

@@ -1781,7 +1781,6 @@ sub searchResults {
         my $itembinding_count     = 0;
         my $itemdamaged_count     = 0;
         my $item_in_transit_count = 0;
-        my $can_place_holds       = 0;
         my $item_onhold_count     = 0;
         my $notforloan_count      = 0;
         my $items_count           = scalar(@fields);
@@ -1852,9 +1851,6 @@ sub searchResults {
                     $onloan_items->{$key}->{longoverdue}++;
                     $longoverdue_count++;
                 }
-                else {    # can place holds as long as item isn't lost
-                    $can_place_holds = 1;
-                }
             }
 
          # items not on loan, but still unavailable ( lost, withdrawn, damaged )
@@ -1916,18 +1912,6 @@ sub searchResults {
                     $item_onhold_count++     if $reservestatus eq 'Waiting';
                     $item->{status} = ($item->{withdrawn}//q{}) . "-" . ($item->{itemlost}//q{}) . "-" . ($item->{damaged}//q{}) . "-" . ($item->{notforloan}//q{});
 
-                    # can place a hold on a item if
-                    # not lost nor withdrawn
-                    # not damaged unless AllowHoldsOnDamagedItems is true
-                    # item is either for loan or on order (notforloan < 0)
-                    $can_place_holds = 1
-                      if (
-                           !$item->{itemlost}
-                        && !$item->{withdrawn}
-                        && ( !$item->{damaged} || C4::Context->preference('AllowHoldsOnDamagedItems') )
-                        && ( !$item->{notforloan} || $item->{notforloan} < 0 )
-                      );
-
                     $other_count++;
 
                     my $key = $prefix . $item->{status};
@@ -1944,7 +1928,6 @@ sub searchResults {
                 }
                 # item is available
                 else {
-                    $can_place_holds = 1;
                     $available_count++;
                     $available_items->{$prefix}->{count}++ if $item->{$hbranch};
                     foreach (qw(branchname itemcallnumber description)) {
@@ -2001,11 +1984,17 @@ sub searchResults {
             );
         }
 
+        my $biblio_object = Koha::Biblios->find( $oldbiblio->{biblionumber} );
+        $oldbiblio->{biblio_object} = $biblio_object;
+
+        my $can_place_holds = 1;
         # if biblio level itypes are used and itemtype is notforloan, it can't be reserved either
         if (!C4::Context->preference("item-level_itypes")) {
             if ($itemtype && $itemtype->{notforloan}) {
                 $can_place_holds = 0;
             }
+        } else {
+            $can_place_holds = $biblio_object->items->filter_by_for_hold()->count;
         }
         $oldbiblio->{norequests} = 1 unless $can_place_holds;
         $oldbiblio->{items_count}          = $items_count;
@@ -2054,8 +2043,6 @@ sub searchResults {
             $oldbiblio->{'alternateholdings_count'} = $alternateholdingscount;
         }
 
-        $oldbiblio->{biblio_object} = Koha::Biblios->find( $oldbiblio->{biblionumber} );
-
         push( @newresults, $oldbiblio );
     }
 

Koha/Holds.pm

@@ -130,26 +130,13 @@ sub get_items_that_can_fill {
           }
       );
 
-    my @hold_not_allowed_itypes = Koha::CirculationRules->search(
-        {
-            rule_name    => 'holdallowed',
-            branchcode   => undef,
-            categorycode => undef,
-            rule_value   => 'not_allowed',
-        }
-    )->get_column('itemtype');
-
     return Koha::Items->search(
         {
             biblionumber => { in => \@biblionumbers },
-            itemlost     => 0,
-            withdrawn    => 0,
-            notforloan   => 0,
-            onloan       => undef,
             itemnumber   => { -not_in => [ @branchtransfers, @waiting_holds ] },
-            itype        => { -not_in => \@hold_not_allowed_itypes },
+            onloan       => undef,
         }
-    );
+    )->filter_by_for_hold();
 }
 
 =head3 type

opac/opac-ISBDdetail.pl

@@ -167,7 +167,6 @@ $template->param(
     subscriptionsnumber => $subscriptionsnumber,
 );
 
-my $norequests = 1;
 my $allow_onshelf_holds;
 my $res = GetISBDView({
     'record'    => $record,
@@ -177,13 +176,6 @@ my $res = GetISBDView({
 
 my $items = $biblio->items;
 while ( my $item = $items->next ) {
-    $norequests = 0
-      if $norequests
-        && !$item->withdrawn
-        && !$item->itemlost
-        && ($item->notforloan < 0 || not $item->notforloan )
-        && !Koha::ItemTypes->find($item->effective_itemtype)->notforloan
-        && $item->itemnumber;
 
     $allow_onshelf_holds = Koha::CirculationRules->get_onshelfholds_policy( { item => $item, patron => $patron } )
       unless $allow_onshelf_holds;
@@ -193,6 +185,8 @@ if( $allow_onshelf_holds || CountItemsIssued($biblionumber) || $biblio->has_item
     $template->param( ReservableItems => 1 );
 }
 
+my $norequests = ! $items->filter_by_for_hold->count;
+
 $template->param(
     RequestOnOpac       => C4::Context->preference("RequestOnOpac"),
     norequests   => $norequests,

opac/opac-MARCdetail.pl

@@ -143,19 +143,10 @@ $template->param(
 ) if $tagslib->{$bt_tag}->{$bt_subtag}->{hidden} <= 0 && # <=0 OPAC visible.
      $tagslib->{$bt_tag}->{$bt_subtag}->{hidden} > -8;   # except -8;
 
-my $norequests = 1;
 my $allow_onshelf_holds;
 my $items = $biblio->items;
 
 while ( my $item = $items->next ) {
-    $norequests = 0
-      if $norequests
-        && !$item->withdrawn
-        && !$item->itemlost
-        && ($item->notforloan < 0 || not $item->notforloan )
-        && !Koha::ItemTypes->find($item->effective_itemtype)->notforloan
-        && $item->itemnumber;
-
     $allow_onshelf_holds = Koha::CirculationRules->get_onshelfholds_policy( { item => $item, patron => $patron } )
       unless $allow_onshelf_holds;
 }
@@ -391,6 +382,7 @@ if( C4::Context->preference('ArticleRequests') ) {
     $template->param( artreqpossible => $artreqpossible );
 }
 
+my $norequests = ! $biblio->items->filter_by_for_hold->count;
 $template->param(
     item_loop           => \@item_loop,
     item_header_loop    => \@item_header_loop,

opac/opac-detail.pl

@@ -668,7 +668,6 @@ if ( $show_holds_count || $show_priority) {
 }
 $template->param( show_priority => $has_hold ) ;
 
-my $norequests = 1;
 my %itemfields;
 my (@itemloop, @otheritemloop);
 my $currentbranch = C4::Context->userenv ? C4::Context->userenv->{branch} : undef;
@@ -712,13 +711,6 @@ if ( not $viewallitems and @items > $max_items_to_display ) {
     my $item = Koha::Items->find( $itm->{itemnumber} );
     $itm->{holds_count} = $item_reserves{ $itm->{itemnumber} };
     $itm->{priority} = $priority{ $itm->{itemnumber} };
-    $norequests = 0
-      if $norequests
-        && !$itm->{'withdrawn'}
-        && !$itm->{'itemlost'}
-        && ($itm->{'itemnotforloan'}<0 || not $itm->{'itemnotforloan'})
-        && !$itemtypes->{$itm->{'itype'}}->{notforloan}
-        && $itm->{'itemnumber'};
 
     $allow_onshelf_holds = Koha::CirculationRules->get_onshelfholds_policy( { item => $item, patron => $patron } )
       unless $allow_onshelf_holds;
@@ -805,6 +797,7 @@ if( C4::Context->preference('ArticleRequests') ) {
     $template->param( artreqpossible => $artreqpossible );
 }
 
+my $norequests = ! $biblio->items->filter_by_for_hold->count;
     $template->param(
                      MARCNOTES               => $marcnotesarray,
                      norequests              => $norequests,