Bug 27069: Adapt uses of holdallowed

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

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

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';

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} };

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';
     }
 

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');
 

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();

koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt

@@ -564,34 +564,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>
@@ -916,17 +916,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>
@@ -970,10 +970,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>

koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt

@@ -1302,8 +1302,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?");