Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed

This patchset move The OPACItemHoldsAllowed logic
(issuingrules.opacitemholds) to a new class method of
Koha::IssuingRules: get_opacitemholds_policy

On the way, this patch will certainly fix the same problem as bug
19298 with onshelfholds.

Test plan:
Make sure the opacitemholds policy is correct when placing a hold at the
OPAC or the staff interface.

Followed test plan which worked as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

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

diff --git a/Koha/IssuingRules.pm b/Koha/IssuingRules.pm
index 20bbb45..f3da6e2 100644 (file)
--- a/Koha/IssuingRules.pm
+++ b/Koha/IssuingRules.pm
@@ -71,6 +71,16 @@ sub get_effective_issuing_rule {
     return $rule;
 }
 
+=head3 get_opacitemholds_policy
+
+my $can_place_a_hold_at_item_level = Koha::IssuingRules->get_opacitemholds_policy( { patron => $patron, item => $item } );
+
+Return 'Y' or 'F' if the patron can place a hold on this item according to the issuing rules
+and the "Item level holds" (opacitemholds).
+Can be 'N' - Don't allow, 'Y' - Allow, and 'F' - Force
+
+=cut
+
 sub get_opacitemholds_policy {
     my ( $class, $params ) = @_;
 
@@ -79,8 +89,15 @@ sub get_opacitemholds_policy {
 
     return unless $item or $patron;
 
-    require C4::Reserves;
-    return C4::Reserves::OPACItemHoldsAllowed( $item->unblessed, $patron->unblessed );
+    my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule(
+        {
+            categorycode => $patron->categorycode,
+            itemtype     => $item->effective_itemtype,
+            branchcode   => $item->homebranch,
+        }
+    );
+
+    return $issuing_rule ? $issuing_rule->opacitemholds : undef;
 }
 
 =head3 type

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 8639b40..1f7679c 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
@@ -604,7 +604,7 @@ function checkMultiHold() {
                     Not on hold
                 [% END %]
 
-                [% IF itemloo.item_level_holds == "" %]
+                [% IF itemloo.item_level_holds == "N" %]
                     <br/>Item level hold not allowed from OPAC
                 [% ELSIF itemloo.item_level_holds == "F" %]
                     <br/>Item level hold forced from OPAC

diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
index 95204b3..dcaec35 100755 (executable)
--- a/opac/opac-reserve.pl
+++ b/opac/opac-reserve.pl
@@ -36,6 +36,7 @@ use C4::Debug;
 use Koha::AuthorisedValues;
 use Koha::Biblios;
 use Koha::DateUtils;
+use Koha::IssuingRules;
 use Koha::Items;
 use Koha::ItemTypes;
 use Koha::Checkouts;
@@ -446,6 +447,7 @@ foreach my $biblioNum (@biblionumbers) {
     my $numCopiesOPACAvailable = 0;
     foreach my $itemInfo (@{$biblioData->{itemInfos}}) {
         my $itemNum = $itemInfo->{itemnumber};
+        my $item = Koha::Items->find( $itemNum );
         my $itemLoopIter = {};
 
         $itemLoopIter->{itemnumber} = $itemNum;
@@ -475,7 +477,6 @@ foreach my $biblioNum (@biblionumbers) {
         }
 
         # checking reserve
-        my $item = Koha::Items->find( $itemNum );
         my $holds = $item->current_holds;
 
         if ( my $first_hold = $holds->next ) {
@@ -539,10 +540,11 @@ foreach my $biblioNum (@biblionumbers) {
             CanItemBeReserved($borrowernumber,$itemNum) eq 'OK';
 
         if ($policy_holdallowed) {
-            if ( my $hold_allowed = OPACItemHoldsAllowed( $itemInfo, $patron_unblessed ) ) {
+            my $opac_hold_policy = Koha::IssuingRules->get_opacitemholds_policy( { item => $item, patron => $patron } );
+            if ( $opac_hold_policy ne 'N' ) { # If Y or F
                 $itemLoopIter->{available} = 1;
                 $numCopiesOPACAvailable++;
-                $biblioLoopIter{force_hold} = 1 if $hold_allowed eq 'F';
+                $biblioLoopIter{force_hold} = 1 if $opac_hold_policy eq 'F';
             }
             $numCopiesAvailable++;
 

diff --git a/reserve/request.pl b/reserve/request.pl
index 919de1f..7a60b85 100755 (executable)
--- a/reserve/request.pl
+++ b/reserve/request.pl
@@ -47,6 +47,7 @@ use Koha::Biblios;
 use Koha::DateUtils;
 use Koha::Checkouts;
 use Koha::Holds;
+use Koha::IssuingRules;
 use Koha::Items;
 use Koha::ItemTypes;
 use Koha::Libraries;
@@ -395,7 +396,8 @@ foreach my $biblionumber (@biblionumbers) {
             }
 
             # checking reserve
-            my $holds = Koha::Items->find( $itemnumber )->current_holds;
+            my $item_object = Koha::Items->find( $itemnumber );
+            my $holds = $item_object->current_holds;
             if ( my $first_hold = $holds->next ) {
                 my $p = Koha::Patrons->find( $first_hold->borrowernumber );
 
@@ -467,7 +469,7 @@ foreach my $biblionumber (@biblionumbers) {
                 my $can_item_be_reserved = CanItemBeReserved( $patron->borrowernumber, $itemnumber );
                 $item->{not_holdable} = $can_item_be_reserved unless ( $can_item_be_reserved eq 'OK' );
 
-                $item->{item_level_holds} = OPACItemHoldsAllowed( $item, $patron_unblessed);
+                $item->{item_level_holds} = Koha::IssuingRules->get_opacitemholds_policy( { item => $item_object, patron => $patron } );
 
                 if (
                        !$item->{cantreserve}

diff --git a/t/db_dependent/Koha/IssuingRules.t b/t/db_dependent/Koha/IssuingRules.t
index 0907a94..b1c97bf 100644 (file)
--- a/t/db_dependent/Koha/IssuingRules.t
+++ b/t/db_dependent/Koha/IssuingRules.t
@@ -274,7 +274,7 @@ subtest 'get_opacitemholds_policy' => sub {
     is ( $opacitemholds, 'Y', 'Patrons can place a hold on this itype');
     t::lib::Mocks::mock_preference('item-level_itypes', 0);
     $opacitemholds = Koha::IssuingRules->get_opacitemholds_policy( { item => $item, patron => $patron } );
-    is ( $opacitemholds, '', 'Patrons cannot place a hold on this itemtype');
+    is ( $opacitemholds, 'N', 'Patrons cannot place a hold on this itemtype');
 
     Koha::IssuingRules->delete;
     Koha::IssuingRule->new({categorycode => '*', itemtype => '*',                 branchcode => '*', opacitemholds => "N"})->store;
@@ -282,7 +282,7 @@ subtest 'get_opacitemholds_policy' => sub {
     Koha::IssuingRule->new({categorycode => '*', itemtype => $itemtype->itemtype, branchcode => '*', opacitemholds => "Y"})->store;
     t::lib::Mocks::mock_preference('item-level_itypes', 1);
     $opacitemholds = Koha::IssuingRules->get_opacitemholds_policy( { item => $item, patron => $patron } );
-    is ( $opacitemholds, '', 'Patrons cannot place a hold on this itype');
+    is ( $opacitemholds, 'N', 'Patrons cannot place a hold on this itype');
     t::lib::Mocks::mock_preference('item-level_itypes', 0);
     $opacitemholds = Koha::IssuingRules->get_opacitemholds_policy( { item => $item, patron => $patron } );
     is ( $opacitemholds, 'Y', 'Patrons can place a hold on this itemtype');