Attachment 118325 Details for Bug 27069 – Bug 27069: Adapt uses of holdallowed

[patch] Bug 27069: Adapt uses of holdallowed

Bug-27069-Adapt-uses-of-holdallowed.patch (text/plain), 14.85 KB, created by Martin Renvoize (ashimema) on 2021-03-16 14:20:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2021-03-16 14:20:40 UTC

Size: 14.85 KB

patch

obsolete

>From cb6f4613368ed07fee5df17b1d34a98c0c2a2c31 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Mon, 15 Mar 2021 15:39:52 -0300
>Subject: [PATCH] Bug 27069: Adapt uses of holdallowed
>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> C4/Circulation.pm                             | 21 +++++-----
> C4/HoldsQueue.pm                              |  8 ++--
> C4/Reserves.pm                                | 16 ++++----
> Koha/Holds.pm                                 |  2 +-
> Koha/Item.pm                                  |  4 +-
> .../prog/en/modules/admin/smart-rules.tt      | 40 +++++++++----------
> .../prog/en/modules/reserve/request.tt        |  4 +-
> 7 files changed, 48 insertions(+), 47 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 03fe5c78f2..1c1ba520a0 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -1830,9 +1830,10 @@ branch and item type, regardless of patron category.
> The return value is a hashref containing the following keys:
> 
> holdallowed => Hold policy for this branch and itemtype. Possible values:
>-  0: No holds allowed.
>-  1: Holds allowed only by patrons that have the same homebranch as the item.
>-  2: Holds allowed from any patron.
>+  not_allowed:           No holds allowed.
>+  from_home_library:     Holds allowed only by patrons that have the same homebranch as the item.
>+  from_any_library:      Holds allowed from any patron.
>+  from_local_hold_group: Holds allowed from libraries in hold group
> 
> returnbranch => branch to which to return item.  Possible values:
>   noreturn: do not return, let item remain where checked in (floating collections)
>@@ -1857,22 +1858,22 @@ sub GetBranchItemRule {
>     my $holdallowed_rule = Koha::CirculationRules->get_effective_rule(
>         {
>             branchcode => $branchcode,
>-            itemtype => $itemtype,
>-            rule_name => 'holdallowed',
>+            itemtype   => $itemtype,
>+            rule_name  => 'holdallowed',
>         }
>     );
>     my $hold_fulfillment_policy_rule = Koha::CirculationRules->get_effective_rule(
>         {
>             branchcode => $branchcode,
>-            itemtype => $itemtype,
>-            rule_name => 'hold_fulfillment_policy',
>+            itemtype   => $itemtype,
>+            rule_name  => 'hold_fulfillment_policy',
>         }
>     );
>     my $returnbranch_rule = Koha::CirculationRules->get_effective_rule(
>         {
>             branchcode => $branchcode,
>-            itemtype => $itemtype,
>-            rule_name => 'returnbranch',
>+            itemtype   => $itemtype,
>+            rule_name  => 'returnbranch',
>         }
>     );
> 
>@@ -1880,7 +1881,7 @@ sub GetBranchItemRule {
>     my $rules;
>     $rules->{holdallowed} = defined $holdallowed_rule
>         ? $holdallowed_rule->rule_value
>-        : 2;
>+        : 'from_any_library';
>     $rules->{hold_fulfillment_policy} = defined $hold_fulfillment_policy_rule
>         ? $hold_fulfillment_policy_rule->rule_value
>         : 'any';
>diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm
>index b0af40b938..3774214527 100644
>--- a/C4/HoldsQueue.pm
>+++ b/C4/HoldsQueue.pm
>@@ -373,14 +373,14 @@ sub GetItemsAvailableToFillHoldRequestsForBib {
> sub _checkHoldPolicy {
>     my ( $item, $request ) = @_;
> 
>-    return 0 unless $item->{holdallowed};
>+    return 0 unless $item->{holdallowed} ne 'not_allowed';
> 
>     return 0
>-      if $item->{holdallowed} == 1
>+      if $item->{holdallowed} eq 'from_home_library'
>       && $item->{homebranch} ne $request->{borrowerbranch};
> 
>     return 0
>-      if $item->{'holdallowed'} == 3
>+      if $item->{'holdallowed'} eq 'from_local_hold_group'
>       && !Koha::Libraries->find( $item->{homebranch} )
>               ->validate_hold_sibling( { branchcode => $request->{borrowerbranch} } );
> 
>@@ -544,7 +544,7 @@ sub MapItemsToHoldRequests {
>     # group available items by branch
>     my %items_by_branch = ();
>     foreach my $item (@$available_items) {
>-        next unless $item->{holdallowed};
>+        next unless $item->{holdallowed} ne 'not_allowed';
> 
>         push @{ $items_by_branch{ $item->{holdingbranch} } }, $item
>           unless exists $allocated_items{ $item->{itemnumber} };
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 54ccc7457d..917a52013e 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -525,18 +525,18 @@ sub CanItemBeReserved {
>     my $branchitemrule =
>       C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->itype ); # FIXME Should not be item->effective_itemtype?
> 
>-    if ( $branchitemrule->{holdallowed} == 0 ) {
>+    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 )
>     {
>         return { status => 'cannotReserveFromOtherBranches' };
>     }
> 
>     my $item_library = Koha::Libraries->find( {branchcode => $item->homebranch} );
>-    if ( $branchitemrule->{holdallowed} == 3) {
>+    if ( $branchitemrule->{holdallowed} eq 'from_local_hold_group') {
>         if($borrower->{branchcode} ne $item->homebranch && !$item_library->validate_hold_sibling( {branchcode => $borrower->{branchcode}} )) {
>             return { status => 'branchNotInHoldGroup' };
>         }
>@@ -890,10 +890,10 @@ sub CheckReserves {
>                     $patron ||= Koha::Patrons->find( $res->{borrowernumber} );
>                     my $branch = GetReservesControlBranch( $item->unblessed, $patron->unblessed );
>                     my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$item->effective_itemtype);
>-                    next if ($branchitemrule->{'holdallowed'} == 0);
>-                    next if (($branchitemrule->{'holdallowed'} == 1) && ($branch ne $patron->branchcode));
>+                    next if ($branchitemrule->{'holdallowed'} eq 'not_allowed');
>+                    next if (($branchitemrule->{'holdallowed'} eq 'from_home_library') && ($branch ne $patron->branchcode));
>                     my $library = Koha::Libraries->find({branchcode=>$item->homebranch});
>-                    next if (($branchitemrule->{'holdallowed'} == 3) && (!$library->validate_hold_sibling({branchcode => $patron->branchcode}) ));
>+                    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) );
>@@ -1395,8 +1395,8 @@ sub ItemsAnyAvailableAndNotRestricted {
>             || ( $i->damaged
>                  && ! C4::Context->preference('AllowHoldsOnDamagedItems') )
>             || Koha::ItemTypes->find( $i->effective_itemtype() )->notforloan
>-            || $branchitemrule->{holdallowed} == 1 && $param->{patron}->branchcode ne $i->homebranch
>-            || $branchitemrule->{holdallowed} == 3 && ! $item_library->validate_hold_sibling( { branchcode => $param->{patron}->branchcode } )
>+            || $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}->borrowernumber, $i->id )->{status} ne 'OK';
>     }
> 
>diff --git a/Koha/Holds.pm b/Koha/Holds.pm
>index e63eadba70..296f99cb0a 100644
>--- a/Koha/Holds.pm
>+++ b/Koha/Holds.pm
>@@ -136,7 +136,7 @@ sub get_items_that_can_fill {
>             rule_name    => 'holdallowed',
>             branchcode   => undef,
>             categorycode => undef,
>-            rule_value   => 0,
>+            rule_value   => 'not_allowed',
>         }
>     )->get_column('itemtype');
> 
>diff --git a/Koha/Item.pm b/Koha/Item.pm
>index c0da07c4b9..dcdcfbdff2 100644
>--- a/Koha/Item.pm
>+++ b/Koha/Item.pm
>@@ -663,8 +663,8 @@ sub pickup_locations {
>       C4::Circulation::GetBranchItemRule( $circ_control_branch, $self->itype );
> 
>     if(defined $patron) {
>-        return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} == 3 && !$self->home_branch->validate_hold_sibling( {branchcode => $patron->branchcode} );
>-        return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} == 1 && $self->home_branch->branchcode ne $patron->branchcode;
>+        return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} eq 'from_local_hold_group' && !$self->home_branch->validate_hold_sibling( {branchcode => $patron->branchcode} );
>+        return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} eq 'from_home_library' && $self->home_branch->branchcode ne $patron->branchcode;
>     }
> 
>     my $pickup_libraries = Koha::Libraries->search();
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>index d41e9ecc50..a3e5629d4e 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>@@ -550,34 +550,34 @@
>                                 Not set
>                             </option>
> 
>-                            [% IF holdallowed == 2 %]
>-                                <option value="2" selected="selected">
>+                            [% IF holdallowed == 'from_any_library' %]
>+                                <option value="from_any_library" selected="selected">
>                             [% ELSE %]
>-                                <option value="2">
>+                                <option value="from_any_library">
>                             [% END %]
>                                 From any library
>                             </option>
> 
>-                            [% IF holdallowed == 3 %]
>-                            <option value="3" selected="selected">
>+                            [% IF holdallowed == 'from_local_hold_group' %]
>+                            <option value="from_local_hold_group" selected="selected">
>                             [% ELSE %]
>-                            <option value="3">
>+                            <option value="from_local_hold_group">
>                             [% END %]
>                                 From local hold group
>                             </option>
> 
>-                            [% IF holdallowed == 1 %]
>-                                <option value="1" selected="selected">
>+                            [% IF holdallowed == 'from_home_library' %]
>+                                <option value="from_home_library" selected="selected">
>                             [% ELSE %]
>-                                <option value="1">
>+                                <option value="from_home_library">
>                             [% END %]
>                                 From home library
>                             </option>
> 
>-                            [% IF holdallowed == 0 %]
>-                                <option value="0" selected="selected">
>+                            [% IF holdallowed == 'not_allowed' %]
>+                                <option value="not_allowed" selected="selected">
>                             [% ELSE %]
>-                                <option value="0">
>+                                <option value="not_allowed">
>                             [% END %]
>                                 No holds allowed
>                             </option>
>@@ -902,17 +902,17 @@
>                     [% SET hold_fulfillment_policy = CirculationRules.Search( branchcode, undef, i.itemtype, 'hold_fulfillment_policy' ) %]
>                     [% SET returnbranch = CirculationRules.Search( branchcode, undef, i.itemtype, 'returnbranch' ) %]
> 
>-                    [% IF holdallowed || hold_fulfillment_policy || returnbranch %]
>+                    [% IF holdallowed != 'not_allowed' || hold_fulfillment_policy || returnbranch %]
>                         <tr>
>                             <td>
>                                 [% i.translated_description | html %]
>                             </td>
>                             <td>
>-                                [% IF holdallowed == 2 %]
>+                                [% IF holdallowed == 'from_any_library' %]
>                                     <span>From any library</span>
>-                                [% ELSIF holdallowed == 3 %]
>+                                [% ELSIF holdallowed == 'from_local_hold_group' %]
>                                     <span>From local hold group</span>
>-                                [% ELSIF holdallowed == 1 %]
>+                                [% ELSIF holdallowed == 'from_home_library' %]
>                                     <span>From home library</span>
>                                 [% ELSE %]
>                                     <span>No holds allowed</span>
>@@ -956,10 +956,10 @@
>                     </td>
>                     <td>
>                         <select name="holdallowed">
>-                            <option value="2">From any library</option>
>-                            <option value="3">From local hold group</option>
>-                            <option value="1">From home library</option>
>-                            <option value="0">No holds allowed</option>
>+                            <option value="from_any_library">From any library</option>
>+                            <option value="from_local_hold_group">From local hold group</option>
>+                            <option value="from_home_library">From home library</option>
>+                            <option value="not_allowed">No holds allowed</option>
>                         </select>
>                     </td>
>                     <td>
>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 5dfff18595..bc4db78a24 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>@@ -1266,8 +1266,8 @@
>                 var msg = '';
> 
>                 switch (override_items[itemnumber].holdallowed) {
>-                    case 0: msg = _("This item normally cannot be put on hold."); break;
>-                    case 1: msg = _("This item normally cannot be put on hold except for patrons from %s.").format(override_items[itemnumber].homebranch); break;
>+                    case "not_allowed": msg = _("This item normally cannot be put on hold."); break;
>+                    case "from_home_library": msg = _("This item normally cannot be put on hold except for patrons from %s.").format(override_items[itemnumber].homebranch); break;
>                 }
> 
>                 msg += "\n\n" + _("Place hold on this item?");
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 27069: 118252 | 118253 | 118254 | 118324 | 118325 | 118326 | 119223 | 119224 | 119225 | 119454