From 69ccb3c6c1ade44f8b470c350abad999331eb175 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 1 Sep 2021 18:00:59 -0300 Subject: [PATCH] [21.11.x] Bug 28529: Make biblio-level hold itemtype count against max rules The current situation is that biblio-level holds can be assigned an item type, so they can only be fulfilled by items matching that specified item type (be it item-level itype or the fallback to biblio-level). But there's the situation in which max holds limits for a specific item type can be overridden by using biblio-level holds with item type selection (AllowHoldItemTypeSelection) enabled. To test: 1. Have a patron of category 'Staff' (S) 2. Have 3 records with items with the 'BK' item type, and maybe others 3. Enable AllowHoldItemTypeSelection 4. Set a limit of 2 max holds for that category+item type 5. In the OPAC. Place bibio-level holds, with item type contraint to 'BK' on those 3 records => FAIL: You can place the 3 holds 6. Cancel the holds 7. Apply this patch and restart all 8. Repeat 5 => SUCCESS: You can only place 2 holds 9. Run: $ kshell t/db_dependent/Reserves.t => SUCCESS: Tests pass! 10. Sign off :-D Signed-off-by: David Nind Signed-off-by: Martin Renvoize --- C4/Reserves.pm | 52 ++++++++++++++++--- .../bootstrap/en/modules/opac-reserve.tt | 6 +-- opac/opac-reserve.pl | 25 +++++++-- 3 files changed, 68 insertions(+), 15 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index b959b2215c9..73bc99fe219 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -331,6 +331,37 @@ sub CanBookBeReserved{ 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; + + return { status => '' } + if defined $reservesallowed and $reservesallowed < $count + 1; + } + my @itemnumbers = Koha::Items->search({ biblionumber => $biblionumber})->get_column("itemnumber"); #get items linked via host records my @hostitems = get_hostitemnumbers_of($biblionumber); @@ -489,16 +520,25 @@ sub CanItemBeReserved { # 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 defined $ruleitemtype; + 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 = ? ) + }; + } + } my $sthcount = $dbh->prepare($querycount); if ( defined $ruleitemtype ) { - $sthcount->execute( $borrowernumber, $branchcode, $ruleitemtype ); + $sthcount->execute( $borrowernumber, $branchcode, $ruleitemtype, $ruleitemtype ); } else { $sthcount->execute( $borrowernumber, $branchcode ); diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt index 99e09cc2458..4372e295ba3 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -293,15 +293,11 @@ [% IF Koha.Preference('AllowHoldItemTypeSelection') %] - [% itemtypes = [] %] - [% FOREACH item IN bibitemloo.itemLoop %] - [% itemtypes.push( item.itype ) %] - [%- END %]
  • diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 4ce329962fa..366ff0d6bd2 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -277,12 +277,16 @@ if ( $query->param('place_reserve') ) { my $expiration_date = $query->param("expiration_date_$biblioNum"); + my $itemtype = $query->param('itemtype') || undef; + $itemtype = undef if $itemNum; + my $rank = $biblioData->{rank}; if ( $itemNum ne '' ) { $canreserve = 1 if CanItemBeReserved( $borrowernumber, $itemNum, $branch )->{status} eq 'OK'; } else { - $canreserve = 1 if CanBookBeReserved( $borrowernumber, $biblioNum, $branch )->{status} eq 'OK'; + $canreserve = 1 + if CanBookBeReserved( $borrowernumber, $biblioNum, $branch, { itemtype => $itemtype } )->{status} eq 'OK'; # Inserts a null into the 'itemnumber' field of 'reserves' table. $itemNum = undef; @@ -304,9 +308,6 @@ if ( $query->param('place_reserve') ) { } } - my $itemtype = $query->param('itemtype') || undef; - $itemtype = undef if $itemNum; - # Here we actually do the reserveration. Stage 3. if ($canreserve) { my $reserve_id = AddReserve( @@ -635,6 +636,22 @@ foreach my $biblioNum (@biblionumbers) { $biblioLoopIter{holdable} &&= $status eq 'OK'; $biblioLoopIter{already_patron_possession} = $status eq 'alreadypossession'; + if ( $biblioLoopIter{holdable} and C4::Context->preference('AllowHoldItemTypeSelection') ) { + # build the allowed item types loop + my $rs = $biblio->items->search( + undef, + { select => [ { distinct => 'itype' } ], + as => 'item_type' + } + ); + + my @item_types = + grep { CanBookBeReserved( $borrowernumber, $biblioNum, $branch, { itemtype => $_ } )->{status} eq 'OK' } + $rs->get_column('item_type'); + + $biblioLoopIter{allowed_item_types} = \@item_types; + } + # For multiple holds per record, if a patron has previously placed a hold, # the patron can only place more holds of the same type. That is, if the # patron placed a record level hold, all the holds the patron places must -- 2.34.1