Attachment 134839 Details for Bug 28529 – Bug 28529: Make biblio-level hold itemtype count against max rules

[patch] Bug 28529: Make biblio-level hold itemtype count against max rules

Bug-28529-Make-biblio-level-hold-itemtype-count-ag.patch (text/plain), 8.60 KB, created by David Nind on 2022-05-10 22:24:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Nind

Created: 2022-05-10 22:24:16 UTC

Size: 8.60 KB

patch

obsolete

>From 4afcd2fbfff66acb1be932fd40e6076934ec89cc Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Fri, 3 Sep 2021 09:55:01 -0300
>Subject: [PATCH] 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 <david@davidnind.com>
>---
> C4/Reserves.pm                                | 64 +++++++++++++++++--
> .../bootstrap/en/modules/opac-reserve.tt      |  6 +-
> opac/opac-reserve.pl                          | 25 ++++++--
> 3 files changed, 80 insertions(+), 15 deletions(-)
>
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 81fecaebe0..9b8c4353ab 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -344,6 +344,38 @@ sub CanBookBeReserved{
>         return { status =>'alreadypossession' };
>     }
> 
>+    if ( $params->{itemtype}
>+        and C4::Context->preference('BiblioHoldItemTypeUseForRules') )
>+    {
>+        # 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 $items;
>     #get items linked via host records
>     my @hostitemnumbers = get_hostitemnumbers_of($biblionumber);
>@@ -524,15 +556,35 @@ 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 and C4::Context->preference('BiblioHoldItemTypeUseForRules') ) {
>+                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 = ? )
>+                    };
>+                }
>+            }
>+            elsif ( defined $ruleitemtype ) {
>+                # 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 = ?";
>+            }
> 
>             my $sthcount = $dbh->prepare($querycount);
> 
>-            if ( defined $ruleitemtype ) {
>+            if ( defined $ruleitemtype and C4::Context->preference('BiblioHoldItemTypeUseForRules') ) {
>+                $sthcount->execute( $patron->borrowernumber, $reserves_control_branch, $ruleitemtype, $ruleitemtype );
>+            }
>+            elsif ( defined $ruleitemtype ) {
>                 $sthcount->execute( $patron->borrowernumber, $reserves_control_branch, $ruleitemtype );
>             }
>             else {
>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 6637cfab6f..95b56bd99b 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
>@@ -295,15 +295,11 @@
>                                                 </li>
> 
>                                                 [% IF Koha.Preference('AllowHoldItemTypeSelection') %]
>-                                                    [% itemtypes = [] %]
>-                                                    [% FOREACH item IN bibitemloo.itemLoop %]
>-                                                        [% itemtypes.push( item.itype ) %]
>-                                                    [%- END %]
>                                                     <li>
>                                                         <label for="itemtype">Request specific item type:</label>
>                                                         <select name="itemtype" id="itemtype">
>                                                             <option value="">Any item type</option>
>-                                                            [% FOREACH i IN itemtypes.unique.sort %]
>+                                                            [% FOREACH i IN bibitemloo.allowed_item_types %]
>                                                                 <option value="[% i | html %]">[% ItemTypes.GetDescription( i ) | html %]</option>
>                                                             [%- END %]
>                                                         </select>
>diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
>index b8ffca4e44..e62a3ba9af 100755
>--- a/opac/opac-reserve.pl
>+++ b/opac/opac-reserve.pl
>@@ -274,13 +274,17 @@ if ( $query->param('place_reserve') ) {
> 
>         my $patron_expiration_date = $query->param("expiration_date_$biblioNum");
> 
>+        my $itemtype = $query->param('itemtype') || undef;
>+        $itemtype = undef if $itemNum;
>+
>         my $rank = $biblioData->{rank};
>         my $patron = Koha::Patrons->find( $borrowernumber );
>         if ( $item ) {
>             $canreserve = 1 if CanItemBeReserved( $patron, $item, $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;
>@@ -302,9 +306,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(
>@@ -633,6 +634,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;
>+    }
>+
>     if ( $status eq 'recall' ) {
>         $biblioLoopIter{recall} = 1;
>     }
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 28529: 124505 | 124506 | 124507 | 134557 | 134558 | 134559 | 134562 | 134790 | 134791 | 134792 | 134809 | 134810 | 134811 | 134837 | 134838 | 134839 | 134991 | 134992 | 134993 | 135044 | 135045 | 137964 | 137965