Attachment 173427 Details for Bug 23732 – Bug 23732: Hold rules checker

[patch] Bug 23732: Hold rules checker

Bug-23732-Hold-rules-checker.patch (text/plain), 33.87 KB, created by Victor Grousset/tuxayo on 2024-10-28 04:18:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Victor Grousset/tuxayo

Created: 2024-10-28 04:18:35 UTC

Size: 33.87 KB

patch

obsolete

>From 617e8bac68172196d2de913aeb40a1ace487599c Mon Sep 17 00:00:00 2001
>From: Alex Arnaud <alex.arnaud@biblibre.com>
>Date: Wed, 12 Dec 2018 11:08:43 +0100
>Subject: [PATCH] Bug 23732: Hold rules checker
>
>Test plan:
>- apply patch
>- try to place a hold, on the Hold details page you should see a
>  "Holding rules" button. Click on it
>- check that the info is consistent with the holding rules, the syspref
>  and the current and future number of holds of the patron
>- place the hold
>- set maxreserves syspref to 1
>- try to place a hold
>- see in the holds rules checker that the maxreserves is highlighted in
>  red
>- restore maxreserves syspref
>- test that sabotaging the matched circulation rule or hold policy is
>  reflected in the hold rules checker
>
>Co-authored-by: Victor Grousset/tuxayo <victor@tuxayo.net>
>Co-authored-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
>Rebased-by: Julian Maurice <julian.maurice@biblibre.com>
>Co-authored-by: Julian Maurice <julian.maurice@biblibre.com>
>Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net>
>---
> C4/Reserves.pm                                | 244 +++++++++++++-----
> .../prog/en/modules/reserve/request.tt        | 150 +++++++++++
> reserve/request.pl                            |  11 +-
> t/db_dependent/Reserves/HoldRulesChecker.t    | 110 ++++++++
> 4 files changed, 453 insertions(+), 62 deletions(-)
> create mode 100644 t/db_dependent/Reserves/HoldRulesChecker.t
>
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 046bfd0235..cc01690f7b 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -370,19 +370,23 @@ sub CanBookBeReserved{
>         return { status =>'alreadypossession' };
>     }
> 
>+    my @circulation_rules;
>+
>     if ( $params->{itemtype} ) {
> 
>         # biblio-level, item type-contrained
>         my $patron          = Koha::Patrons->find($borrowernumber);
>-        my $reservesallowed = Koha::CirculationRules->get_effective_rule(
>+        my $reservesallowed_rule = Koha::CirculationRules->get_effective_rule(
>             {
>                 itemtype     => $params->{itemtype},
>                 categorycode => $patron->categorycode,
>                 branchcode   => $pickup_branchcode,
>                 rule_name    => 'reservesallowed',
>             }
>-        )->rule_value;
>+        );
>+        push @circulation_rules, $reservesallowed_rule if $reservesallowed_rule;
> 
>+        my $reservesallowed = $reservesallowed_rule ? $reservesallowed_rule->rule_value : undef;
>         $reservesallowed = ( $reservesallowed eq '' ) ? undef : $reservesallowed;
> 
>         my $count = $patron->holds->search(
>@@ -397,7 +401,7 @@ sub CanBookBeReserved{
>             }
>         )->count;
> 
>-        return { status => '' }
>+        return { status => '', circulation_rules => \@circulation_rules }
>           if defined $reservesallowed and $reservesallowed < $count + 1;
>     }
> 
>@@ -415,36 +419,100 @@ sub CanBookBeReserved{
>         $items = Koha::Items->search({ biblionumber => $biblionumber});
>     }
> 
>-    my $canReserve = { status => '' };
>+    my $response = { status => '', item_responses => [] };
>     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';
>+        my $itemResponse = CanItemBeReserved( $patron, $item, $pickup_branchcode, $params );
>+        push @{ $response->{item_responses} }, { %$itemResponse, item => $item };
>+
>+        if ($itemResponse->{status} eq 'OK') {
>+            $response->{status} = 'OK';
>+
>+            return $response;
>+        }
>     }
>-    return $canReserve;
>+
>+    return $response;
> }
> 
> =head2 CanItemBeReserved
> 
>-  $canReserve = &CanItemBeReserved($patron, $item, $branchcode, $params)
>+  $canReserve = CanItemBeReserved($patron, $item, $branchcode, $params)
>   if ($canReserve->{status} eq 'OK') { #We can reserve this Item! }
> 
>-  current params are:
>-  '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 care 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
>+Current params are:
>+
>+=over
>+
>+=item * C<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 care about reservability
>+
>+=back
>+
>+Returns a hashref with the following keys:
>+
>+=over
>+
>+=item * C<status>
>+
>+A string that can have the following values:
>+
>+=over
>+
>+=item * C<OK> if the item can be reserved
>+
>+=item * C<ageRestricted> if the item is aged restricted for this borrower
>+
>+=item * C<damaged> if the item is damaged
>+
>+=item * C<cannotReserveFromOtherBranches> if syspref
>+'canreservefromotherbranches' is OK
>+
>+=item * C<branchNotInHoldGroup> if borrower home library is not in hold group,
>+and holds are only allowed from hold groups.
>+
>+=item * C<tooManyReserves> if the borrower has exceeded their maximum reserve
>+amount.
>+
>+=item * C<tooManyHoldsForThisRecord> if the borrower has exceeded their maximum
>+reserve amount for this biblio record
>+
>+=item * C<tooManyReservesToday> if the borrower has exceeded their maximum
>+reserve amount for today
>+
>+=item * C<notReservable> if holds on this item are not allowed
>+
>+=item * C<libraryNotFound> if given branchcode is not an existing library
>+
>+=item * C<libraryNotPickupLocation> if given branchcode is not configured to be
>+a pickup location
>+
>+=item * C<cannotBeTransferred> if branch transfer limit applies on given item
>+and branchcode
>+
>+=item * C<pickupNotInHoldGroup> if pickup location is not in hold group, and
>+pickup locations are only allowed from hold groups.
>+
>+=item * C<recall> if the borrower has already placed a recall on this item
>+
>+=back
>+
>+=item * C<limit>
>+
>+Only if C<status> is equal to C<tooManyReserves>, C<tooManyHoldsForThisRecord>,
>+or C<tooManyReservesToday>.
>+
>+It contains the number of reserves allowed.
>+
>+=item * C<circulation_rules>
>+
>+An arrayref containing all circulations rules (L<Koha::CirculationRule>) that
>+have been checked
>+
>+=back
> 
> =cut
> 
>@@ -467,8 +535,6 @@ sub CanItemBeReserved {
>     }
> 
>     my $dbh = C4::Context->dbh;
>-    my $ruleitemtype;    # itemtype of the matching issuing rule
>-    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
>@@ -520,34 +586,21 @@ sub CanItemBeReserved {
>         $reserves_control_branch  = $patron->branchcode;
>     }
> 
>-    # we retrieve rights
>-    if (
>-        my $reservesallowed = Koha::CirculationRules->get_effective_rule({
>-                itemtype     => $item->effective_itemtype,
>-                categorycode => $patron->categorycode,
>-                branchcode   => $reserves_control_branch,
>-                rule_name    => 'reservesallowed',
>-        })
>-    ) {
>-        $ruleitemtype     = $reservesallowed->itemtype;
>-        $allowedreserves  = $reservesallowed->rule_value // 0; #undefined is 0, blank is unlimited
>-    }
>-    else {
>-        $ruleitemtype = undef;
>-    }
>+    my @circulation_rules;
> 
>-    my $rights = Koha::CirculationRules->get_effective_rules({
>+    my $holds_per_record_rule = Koha::CirculationRules->get_effective_rule({
>         categorycode => $patron->categorycode,
>         itemtype     => $item->effective_itemtype,
>         branchcode   => $reserves_control_branch,
>-        rules        => ['holds_per_record','holds_per_day']
>+        rule_name    => 'holds_per_record',
>     });
>-    my $holds_per_record = $rights->{holds_per_record} // 1;
>-    my $holds_per_day    = $rights->{holds_per_day};
>+    push @circulation_rules, $holds_per_record_rule if $holds_per_record_rule;
>+
>+    my $holds_per_record = $holds_per_record_rule ? $holds_per_record_rule->rule_value : 1;
> 
>     if (   defined $holds_per_record && $holds_per_record ne '' ){
>         if ( $holds_per_record == 0 ) {
>-            return _cache { status => "noReservesAllowed" };
>+            return _cache { status => "noReservesAllowed", circulation_rules => \@circulation_rules };
>         }
>         if ( !$params->{ignore_hold_counts} ) {
>             my $search_params = {
>@@ -555,24 +608,66 @@ sub CanItemBeReserved {
>                 biblionumber   => $item->biblionumber,
>             };
>             my $holds = Koha::Holds->search($search_params);
>-            return _cache { status => "tooManyHoldsForThisRecord", limit => $holds_per_record } if $holds->count() >= $holds_per_record;
>+
>+            if ($holds->count() >= $holds_per_record) {
>+                return _cache {
>+                    status => "tooManyHoldsForThisRecord",
>+                    limit  => $holds_per_record,
>+                    circulation_rules => \@circulation_rules,
>+                };
>+            }
>         }
>     }
> 
>+    my $holds_per_day_rule = Koha::CirculationRules->get_effective_rule({
>+        categorycode => $patron->categorycode,
>+        itemtype     => $item->effective_itemtype,
>+        branchcode   => $reserves_control_branch,
>+        rule_name    => 'holds_per_day',
>+    });
>+    push @circulation_rules, $holds_per_day_rule if $holds_per_day_rule;
>+
>+    my $holds_per_day = $holds_per_day_rule ? $holds_per_day_rule->rule_value : undef;
>     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 _cache { status => 'tooManyReservesToday', limit => $holds_per_day } if $today_holds->count() >= $holds_per_day;
>+
>+        if ($today_holds->count() >= $holds_per_day) {
>+            return _cache {
>+                status => 'tooManyReservesToday',
>+                limit  => $holds_per_day,
>+                circulation_rules => \@circulation_rules,
>+            };
>+        }
>+    }
>+
>+    my $reservesallowed_rule = Koha::CirculationRules->get_effective_rule({
>+        itemtype     => $item->effective_itemtype,
>+        categorycode => $patron->categorycode,
>+        branchcode   => $reserves_control_branch,
>+        rule_name    => 'reservesallowed',
>+    });
>+    push @circulation_rules, $reservesallowed_rule if $reservesallowed_rule;
>+
>+    my $ruleitemtype;    # itemtype of the matching issuing rule
>+    my $allowedreserves  = 0; # Total number of holds allowed across all records, default to none
>+    if ($reservesallowed_rule) {
>+        $ruleitemtype     = $reservesallowed_rule->itemtype;
>+        $allowedreserves  = $reservesallowed_rule->rule_value // 0; #undefined is 0, blank is unlimited
>     }
> 
>     # we check if it's ok or not
>     if ( defined $allowedreserves && $allowedreserves ne '' ){
>-        if( $allowedreserves == 0 ){
>-            return _cache { status => 'noReservesAllowed' };
>+        if ( $allowedreserves == 0 ) {
>+            return _cache {
>+                status            => 'noReservesAllowed',
>+                circulation_rules => \@circulation_rules,
>+            };
>         }
>+
>         if ( !$params->{ignore_hold_counts} ) {
>             # we retrieve count
>             my $querycount = q{
>@@ -616,45 +711,66 @@ sub CanItemBeReserved {
>                 $reservecount = $rowcount->{count};
>             }
> 
>-            return _cache { status => 'tooManyReserves', limit => $allowedreserves } if $reservecount >= $allowedreserves;
>+            if ($reservecount >= $allowedreserves) {
>+                return _cache {
>+                    status => 'tooManyReserves',
>+                    limit  => $allowedreserves,
>+                    circulation_rules => \@circulation_rules,
>+                };
>+            }
>         }
>     }
> 
>     # Now we need to check hold limits by patron category
>-    my $rule = Koha::CirculationRules->get_effective_rule(
>+    my $max_holds_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 '' ) {
>+    push @circulation_rules, $max_holds_rule if $max_holds_rule;
>+
>+    if (!$params->{ignore_hold_counts} && $max_holds_rule && defined( $max_holds_rule->rule_value ) && $max_holds_rule->rule_value ne '' ) {
>         my $total_holds_count = Koha::Holds->search(
>             {
>                 borrowernumber => $patron->borrowernumber
>             }
>         )->count();
> 
>-        return _cache { status => 'tooManyReserves', limit => $rule->rule_value} if $total_holds_count >= $rule->rule_value;
>+        if ($total_holds_count >= $max_holds_rule->rule_value) {
>+            return _cache {
>+                status => 'tooManyReserves',
>+                limit => $max_holds_rule->rule_value,
>+                circulation_rules => \@circulation_rules,
>+            };
>+        }
>     }
> 
>+    my $holdallowed_rule = Koha::CirculationRules->get_effective_rule({
>+        branchcode => $reserves_control_branch,
>+        itemtype => $item->effective_itemtype,
>+        rule_name => 'holdallowed',
>+    });
>+    push @circulation_rules, $holdallowed_rule if $holdallowed_rule;
>+
>     my $branchitemrule =
>       C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->effective_itemtype );
> 
>     if ( $branchitemrule->{holdallowed} eq 'not_allowed' ) {
>-        return _cache { status => 'notReservable' };
>+        return _cache { status => 'notReservable', circulation_rules => \@circulation_rules };
>     }
> 
>     if (   $branchitemrule->{holdallowed} eq 'from_home_library'
>         && $patron->branchcode ne $item->homebranch )
>     {
>-        return _cache { status => 'cannotReserveFromOtherBranches' };
>+        return _cache { status => 'cannotReserveFromOtherBranches', circulation_rules => \@circulation_rules };
>     }
> 
>     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 _cache { status => 'branchNotInHoldGroup' };
>+            return _cache { status => 'branchNotInHoldGroup', circulation_rules => \@circulation_rules };
>         }
>     }
> 
>@@ -672,15 +788,23 @@ sub CanItemBeReserved {
>         unless ($item->can_be_transferred({ to => $destination })) {
>             return _cache { status => 'cannotBeTransferred' };
>         }
>+
>+        my $hold_fulfillment_policy_rule = Koha::CirculationRules->get_effective_rule({
>+            branchcode => $reserves_control_branch,
>+            itemtype => $item->effective_itemtype,
>+            rule_name => 'hold_fulfillment_policy',
>+        });
>+        push @circulation_rules, $hold_fulfillment_policy_rule if $hold_fulfillment_policy_rule;
>+
>         if ($branchitemrule->{hold_fulfillment_policy} eq 'holdgroup' && !$item_library->validate_hold_sibling( {branchcode => $pickup_branchcode} )) {
>-            return _cache { status => 'pickupNotInHoldGroup' };
>+            return _cache { status => 'pickupNotInHoldGroup', circulation_rules => \@circulation_rules };
>         }
>         if ($branchitemrule->{hold_fulfillment_policy} eq 'patrongroup' && !Koha::Libraries->find({branchcode => $patron->branchcode})->validate_hold_sibling({branchcode => $pickup_branchcode})) {
>-            return _cache { status => 'pickupNotInHoldGroup' };
>+            return _cache { status => 'pickupNotInHoldGroup', circulation_rules => \@circulation_rules };
>         }
>     }
> 
>-    return _cache { status => 'OK' };
>+    return _cache { status => 'OK', circulation_rules => \@circulation_rules };
> }
> 
> =head2 ChargeReserveFee
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>index 2141e06c51..53ffe0d094 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>@@ -241,6 +241,10 @@
>                 </h2>
>             [% END %]
> 
>+            [% IF canbookbereserved_responses %]
>+                <button type="button" class="btn btn-sm" data-toggle="modal" data-target="#holdruleschecker">[% t('Show circulation rules') | html %]</button>
>+            [% END %]
>+
>             [% UNLESS club OR patron OR patron.borrowernumber OR noitems OR nobiblio %]
>                 [% IF ( messageborrower ) %]
>                     <div class="alert alert-warning">
>@@ -512,6 +516,7 @@
>                                 <li><strong>No items available: </strong>One or more records have no items that can be held</li>
>                             [% END # /IF exceeded_maxreserves %]
>                         [% END # /UNLESS multi_hold %]
>+
>                     </div>
>                 [% END # /IF ( exceeded_maxreserves || ... %]
> 
>@@ -2012,4 +2017,149 @@
>     </script>
> [% END %]
> 
>+<div class="modal fade" id="holdruleschecker" tabindex="-1" role="dialog" aria-labelledby="holdruleschecker-title">
>+    <div class="modal-dialog modal-lg" role="document">
>+        <div class="modal-content">
>+            <div class="modal-header">
>+                <h4 class="modal-title" id="holdruleschecker-title">[% t('Holding rules') | html %]</h4>
>+            </div>
>+            <div class="modal-body">
>+                <div>
>+                    <h4>[% t('Overall info') | html %]</h4>
>+
>+                    <ul>
>+                        <li>
>+                            [% tnx('Patron has {count} hold', 'Patron has {count} holds', reserves_count, { count = '<strong>' _ reserves_count _ '</strong>' }) | $raw %]
>+                            [% tx('and this request would raise the number to {count}.', { count = '<strong>' _ (new_reserves_count + reserves_count) _ '</strong>' }) | $raw %]
>+                        </li>
>+                        <li>
>+                            [% IF reserves_count >= Koha.Preference('maxreserves') %]
>+                                <span class="text-danger">
>+                            [% ELSE %]
>+                                <span>
>+                            [% END %]
>+                                [% tx('{preference_name} preference:', { preference_name = '<em>maxreserves</em>' }) | $raw %]
>+                                <strong>[% Koha.Preference('maxreserves') | html %]</strong>
>+                            </span>
>+                        </li>
>+                        <li>
>+                            [% tx('{preference_name} preference:', { preference_name = '<em>ReservesControlBranch</em>' }) | $raw %]
>+                            <strong>[% Koha.Preference('ReservesControlBranch') | html %]</strong>
>+                        </li
>+                    </ul>
>+                </div>
>+
>+                <h4>[% t('Per record info') | html %]</h4>
>+
>+                <div class="panel-group" id="accordion" role="tablist" aria-multiselectable="true">
>+                    [% FOREACH response IN canbookbereserved_responses %]
>+                        <div class="panel panel-default">
>+                            <div class="panel-heading" role="tab" id="heading-[% response.biblio.biblionumber | html %]">
>+                                <h4 class="panel-title">
>+                                    <a role="button" data-toggle="collapse" data-parent="#accordion" href="#collapse-[% response.biblio.biblionumber | html %]" aria-expanded="true" aria-controls="collapse-[% response.biblio.biblionumber | html %]">
>+                                        [% response.biblio.title | html %]
>+                                    </a>
>+                                </h4>
>+                            </div>
>+                            <div id="collapse-[% response.biblio.biblionumber | html %]" class="panel-collapse collapse in" role="tabpanel" aria-labelledby="heading-[% response.biblio.biblionumber | html %]">
>+                                <div class="panel-body">
>+                                    <h5>[% t('Circulation rules checked') | html %]</h5>
>+
>+                                    <table>
>+                                        <thead>
>+                                            <tr>
>+                                                <th>[% t('Item') | html %]</th>
>+                                                <th>[% t('Library') | html %]</th>
>+                                                <th>[% t('Category') | html %]</th>
>+                                                <th>[% t('Item type') | html %]</th>
>+                                                <th>[% t('Rule name') | html %]</th>
>+                                                <th>[% t('Rule value') | html %]</th>
>+                                            </tr>
>+                                        </thead>
>+                                        <tbody>
>+                                            [% FOREACH circulation_rule IN response.circulation_rules %]
>+                                                <tr>
>+                                                    <td></td>
>+                                                    <td>
>+                                                        [% IF circulation_rule.branchcode %]
>+                                                            [% Branches.GetName(circulation_rule.branchcode) | html %]
>+                                                            ([% circulation_rule.branchcode | html %])
>+                                                        [% ELSE %]
>+                                                            <em>[% t('All') | html %]</em>
>+                                                        [% END %]
>+                                                    </td>
>+                                                    <td>
>+                                                        [% IF circulation_rule.categorycode %]
>+                                                            [% Categories.GetName(circulation_rule.categorycode) | html %]
>+                                                        [% ELSE %]
>+                                                            <em>[% t('All') | html %]</em>
>+                                                        [% END %]
>+                                                    </td>
>+                                                    <td>
>+                                                        [% IF circulation_rule.itemtype %]
>+                                                            [% ItemTypes.GetDescription(circulation_rule.itemtype) | html %]
>+                                                        [% ELSE %]
>+                                                            <em>[% t('All') | html %]</em>
>+                                                        [% END %]
>+                                                    </td>
>+                                                    <td>
>+                                                        [% circulation_rule.rule_name | html %]
>+                                                    </td>
>+                                                    <td>
>+                                                        [% circulation_rule.rule_value | html %]
>+                                                    </td>
>+                                                </tr>
>+                                            [% END %]
>+                                            [% FOREACH item_response IN response.item_responses %]
>+                                                [% FOREACH circulation_rule IN item_response.circulation_rules %]
>+                                                    <tr>
>+                                                        <td>
>+                                                            [% item_response.item.barcode | html %] (#[% item_response.item.itemnumber | html %])
>+                                                        </td>
>+                                                        <td>
>+                                                            [% IF circulation_rule.branchcode %]
>+                                                                [% Branches.GetName(circulation_rule.branchcode) | html %]
>+                                                                ([% circulation_rule.branchcode | html %])
>+                                                            [% ELSE %]
>+                                                                <em>[% t('All') | html %]</em>
>+                                                            [% END %]
>+                                                        </td>
>+                                                        <td>
>+                                                            [% IF circulation_rule.categorycode %]
>+                                                                [% Categories.GetName(circulation_rule.categorycode) | html %]
>+                                                            [% ELSE %]
>+                                                                <em>[% t('All') | html %]</em>
>+                                                            [% END %]
>+                                                        </td>
>+                                                        <td>
>+                                                            [% IF circulation_rule.itemtype %]
>+                                                                [% ItemTypes.GetDescription(circulation_rule.itemtype) | html %]
>+                                                            [% ELSE %]
>+                                                                <em>[% t('All') | html %]</em>
>+                                                            [% END %]
>+                                                        </td>
>+                                                        <td>
>+                                                            [% circulation_rule.rule_name | html %]
>+                                                        </td>
>+                                                        <td>
>+                                                            [% circulation_rule.rule_value | html %]
>+                                                        </td>
>+                                                    </tr>
>+                                                [% END %]
>+                                            [% END %]
>+                                        </tbody>
>+                                    </table>
>+                                </div>
>+                            </div>
>+                        </div>
>+                    [% END %]
>+                </div>
>+            </div>
>+            <div class="modal-footer">
>+                <button type="button" class="btn btn-default" data-dismiss="modal">[% t('Close') | html %]</button>
>+            </div>
>+        </div>
>+    </div>
>+</div> 
>+
> [% INCLUDE 'intranet-bottom.inc' %]
>diff --git a/reserve/request.pl b/reserve/request.pl
>index b19fa3b868..6731ef3d9c 100755
>--- a/reserve/request.pl
>+++ b/reserve/request.pl
>@@ -186,8 +186,10 @@ if ($borrowernumber_hold && !$op) {
>     # FIXME At this time we have a simple count of reservs, but, later, we could improve the infos "title" ...
> 
>     my $reserves_count = $patron->holds->count;
>+    $template->param(reserves_count => $reserves_count);
> 
>     my $new_reserves_count = scalar( @biblionumbers );
>+    $template->param(new_reserves_count   => $new_reserves_count);
> 
>     my $maxreserves = C4::Context->preference('maxreserves');
>     $template->param( maxreserves => $maxreserves );
>@@ -203,8 +205,6 @@ if ($borrowernumber_hold && !$op) {
>         $exceeded_maxreserves = 1;
>         $template->param(
>             new_reserves_allowed => $new_reserves_allowed,
>-            new_reserves_count   => $new_reserves_count,
>-            reserves_count       => $reserves_count,
>             maxreserves          => $maxreserves,
>         );
>     }
>@@ -298,6 +298,7 @@ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
>     my @biblioloop = ();
>     my $no_reserves_allowed = 0;
>     my $num_bibs_available = 0;
>+    my @canbookbereserved_responses;
>     foreach my $biblionumber (@biblionumbers) {
>         next unless $biblionumber =~ m|^\d+$|;
> 
>@@ -316,6 +317,9 @@ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
>         if ( $patron ) {
>             { # CanBookBeReserved
>                 my $canReserve = CanBookBeReserved( $patron->borrowernumber, $biblionumber );
>+
>+                push @canbookbereserved_responses, { %$canReserve, biblio => $biblio };
>+
>                 if ( $canReserve->{status} eq 'OK' ) {
> 
>                     #All is OK and we can continue
>@@ -710,6 +714,9 @@ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
> 
>     $template->param( no_bibs_available => 1 ) unless $num_bibs_available > 0;
> 
>+    $Data::Dumper::Maxdepth = 3;
>+    use Data::Dumper; warn Dumper \@canbookbereserved_responses;
>+    $template->param( canbookbereserved_responses => \@canbookbereserved_responses );
>     $template->param( biblioloop => \@biblioloop );
>     $template->param( no_reserves_allowed => $no_reserves_allowed );
>     $template->param( exceeded_maxreserves => $exceeded_maxreserves );
>diff --git a/t/db_dependent/Reserves/HoldRulesChecker.t b/t/db_dependent/Reserves/HoldRulesChecker.t
>new file mode 100644
>index 0000000000..1bebad86ef
>--- /dev/null
>+++ b/t/db_dependent/Reserves/HoldRulesChecker.t
>@@ -0,0 +1,110 @@
>+#!/usr/bin/perl
>+
>+# Copyright 2019 Biblibre
>+#
>+# This file is part of Koha.
>+#
>+# Koha is free software; you can redistribute it and/or modify it
>+# under the terms of the GNU General Public License as published by
>+# the Free Software Foundation; either version 3 of the License, or
>+# (at your option) any later version.
>+#
>+# Koha is distributed in the hope that it will be useful, but
>+# WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+# GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License
>+# along with Koha; if not, see <http://www.gnu.org/licenses>.
>+
>+use Modern::Perl;
>+
>+use Test::More tests => 13;
>+use t::lib::TestBuilder;
>+
>+use C4::Reserves qw( CanBookBeReserved );
>+
>+my $schema = Koha::Database->new->schema;
>+$schema->storage->txn_begin;
>+
>+my $builder = t::lib::TestBuilder->new();
>+
>+my $library = $builder->build_object({ class => 'Koha::Libraries' });
>+my $category = $builder->build_object({ class => 'Koha::Patron::Categories' });
>+my $patron = $builder->build_object(
>+    {
>+        class => 'Koha::Patrons',
>+        value  => {
>+            categorycode => $category->categorycode,
>+            branchcode   => $library->branchcode,
>+        },
>+    }
>+);
>+
>+my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' });
>+
>+my $biblio = $builder->build_sample_biblio(
>+    {
>+        title => 'Title 1',
>+    }
>+);
>+my $item = $builder->build_sample_item(
>+    {
>+        biblionumber => $biblio->biblionumber,
>+        library      => $library->branchcode,
>+        itype        => $itemtype->itemtype,
>+        damaged      => 0,
>+    }
>+);
>+
>+my $rules_rs = Koha::Database->new()->schema()->resultset('CirculationRule');
>+$rules_rs->delete();
>+
>+my $default_reservesallowed_rule = $rules_rs->create(
>+    {
>+        categorycode     => undef,
>+        itemtype         => undef,
>+        branchcode       => undef,
>+        rule_name => 'reservesallowed',
>+        rule_value => 10,
>+    }
>+);
>+
>+my $reservesallowed_rule = $rules_rs->create(
>+    {
>+        categorycode     => $patron->categorycode,
>+        itemtype         => undef,
>+        branchcode       => $library->branchcode,
>+        rule_name => 'reservesallowed',
>+        rule_value => 0,
>+    }
>+);
>+
>+my $can = CanBookBeReserved($patron->borrowernumber, $biblio->biblionumber);
>+is($can->{status}, '');
>+is(scalar @{ $can->{circulation_rules} }, 0);
>+is(scalar @{ $can->{item_responses} }, 1);
>+is($can->{item_responses}->[0]->{status}, 'noReservesAllowed');
>+is(scalar @{ $can->{item_responses}->[0]->{circulation_rules} }, 1);
>+is($can->{item_responses}->[0]->{circulation_rules}->[0]->id, $reservesallowed_rule->id);
>+
>+my $holds_per_record_rule = $rules_rs->create(
>+    {
>+        categorycode     => $patron->categorycode,
>+        itemtype         => undef,
>+        branchcode       => $library->branchcode,
>+        rule_name => 'holds_per_record',
>+        rule_value => 1,
>+    }
>+);
>+
>+$can = CanBookBeReserved($patron->borrowernumber, $biblio->biblionumber);
>+is($can->{status}, '');
>+is(scalar @{ $can->{circulation_rules} }, 0);
>+is(scalar @{ $can->{item_responses} }, 1);
>+is($can->{item_responses}->[0]->{status}, 'noReservesAllowed');
>+is(scalar @{ $can->{item_responses}->[0]->{circulation_rules} }, 2);
>+is($can->{item_responses}->[0]->{circulation_rules}->[0]->id, $holds_per_record_rule->id);
>+is($can->{item_responses}->[0]->{circulation_rules}->[1]->id, $reservesallowed_rule->id);
>+
>+$schema->storage->txn_rollback;
>-- 
>2.47.0

Actions: View | Diff | Splinter Review

Attachments on bug 23732: 95435 | 95436 | 95443 | 95444 | 95445 | 95934 | 147501 | 147504 | 173427 | 173428