Bug 15532: Add ability to allow only items whose home/holding branch matches the...
authorKyle M Hall <kyle@bywatersolutions.com>
Sat, 19 Dec 2015 11:34:03 +0000 (11:34 +0000)
committerKyle M Hall <kyle@bywatersolutions.com>
Fri, 29 Apr 2016 09:49:07 +0000 (09:49 +0000)
Some libraries would like to be able to limit hold filling to items that
match the pickup library for a hold based on the item's home or holding
library. The patron's home library should not affect whether a patron
can place the hold, instead the hold will only be fillable when an item
matching the pickup location becomes available.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Note the new "Hold pickup library match" rules for "checkout, hold,
   and return policy" and for "holds policy by item type"
4) Set the policy to "item's holding library"
5) Place a hold where the item's holding branch does not match
   the pickup branch
6) Check in the item
7) Note it is not trapped for the hold
8) Update the item's holding branch to match the pickup branch
8) Check in the item
9) Note the item is trapped for the hold
10) Repeat steps 4-9 but for home branch instead

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as described

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
C4/Circulation.pm
C4/HoldsQueue.pm
C4/Reserves.pm
admin/smart-rules.pl
installer/data/mysql/atomicupdate/hold_fulfillment_policy.sql [new file with mode: 0644]
installer/data/mysql/kohastructure.sql
koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
t/db_dependent/Holds/HoldFulfillmentPolicy.t [new file with mode: 0755]
t/db_dependent/HoldsQueue.t

index 81c6947..650dc12 100644 (file)
@@ -1724,17 +1724,17 @@ sub GetBranchItemRule {
     my $result = {};
 
     my @attempts = (
-        ['SELECT holdallowed, returnbranch
+        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
             FROM branch_item_rules
             WHERE branchcode = ?
               AND itemtype = ?', $branchcode, $itemtype],
-        ['SELECT holdallowed, returnbranch
+        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
             FROM default_branch_circ_rules
             WHERE branchcode = ?', $branchcode],
-        ['SELECT holdallowed, returnbranch
+        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
             FROM default_branch_item_rules
             WHERE itemtype = ?', $itemtype],
-        ['SELECT holdallowed, returnbranch
+        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
             FROM default_circ_rules'],
     );
 
@@ -1747,11 +1747,13 @@ sub GetBranchItemRule {
         # defaults tables, we have to check that the key we want is set, not
         # just that a row was returned
         $result->{'holdallowed'}  = $search_result->{'holdallowed'}  unless ( defined $result->{'holdallowed'} );
+        $result->{'hold_fulfillment_policy'} = $search_result->{'hold_fulfillment_policy'} unless ( defined $result->{'hold_fulfillment_policy'} );
         $result->{'returnbranch'} = $search_result->{'returnbranch'} unless ( defined $result->{'returnbranch'} );
     }
     
     # built-in default circulation rule
     $result->{'holdallowed'} = 2 unless ( defined $result->{'holdallowed'} );
+    $result->{'hold_fulfillment_policy'} = 'any' unless ( defined $result->{'hold_fulfillment_policy'} );
     $result->{'returnbranch'} = 'homebranch' unless ( defined $result->{'returnbranch'} );
 
     return $result;
index 4fda2c6..7cadfec 100755 (executable)
@@ -355,6 +355,7 @@ sub GetItemsAvailableToFillHoldRequestsForBib {
     return [ grep {
         my $rule = GetBranchItemRule($_->{homebranch}, $_->{itype});
         $_->{holdallowed} = $rule->{holdallowed};
+        $_->{hold_fulfillment_policy} = $rule->{hold_fulfillment_policy};
     } @items ];
 }
 
@@ -393,18 +394,27 @@ sub MapItemsToHoldRequests {
         # is this an item-level request?
         if (defined($request->{itemnumber})) {
             # fill it if possible; if not skip it
-            if (exists $items_by_itemnumber{$request->{itemnumber}} and
-                not exists $allocated_items{$request->{itemnumber}}) {
-                $item_map{$request->{itemnumber}} = {
+            if (
+                    exists $items_by_itemnumber{ $request->{itemnumber} }
+                and not exists $allocated_items{ $request->{itemnumber} }
+                and ( # Don't fill item level holds that contravene the hold pickup policy at this time
+                    ( $items_by_itemnumber{ $request->{itemnumber} }->{hold_fulfillment_policy} eq 'any' )
+                    || ( $request->{branchcode} eq $items_by_itemnumber{ $request->{itemnumber} }->{ $items_by_itemnumber{ $request->{itemnumber} }->{hold_fulfillment_policy} }  )
+                )
+
+              )
+            {
+
+                $item_map{ $request->{itemnumber} } = {
                     borrowernumber => $request->{borrowernumber},
-                    biblionumber => $request->{biblionumber},
-                    holdingbranch =>  $items_by_itemnumber{$request->{itemnumber}}->{holdingbranch},
-                    pickup_branch => $request->{branchcode} || $request->{borrowerbranch},
-                    item_level => 1,
-                    reservedate => $request->{reservedate},
-                    reservenotes => $request->{reservenotes},
+                    biblionumber   => $request->{biblionumber},
+                    holdingbranch  => $items_by_itemnumber{ $request->{itemnumber} }->{holdingbranch},
+                    pickup_branch  => $request->{branchcode} || $request->{borrowerbranch},
+                    item_level     => 1,
+                    reservedate    => $request->{reservedate},
+                    reservenotes   => $request->{reservenotes},
                 };
-                $allocated_items{$request->{itemnumber}}++;
+                $allocated_items{ $request->{itemnumber} }++;
                 $num_items_remaining--;
             }
         } else {
@@ -437,7 +447,12 @@ sub MapItemsToHoldRequests {
         my $holding_branch_items = $items_by_branch{$pickup_branch};
         if ( $holding_branch_items ) {
             foreach my $item (@$holding_branch_items) {
-                if ( $request->{borrowerbranch} eq $item->{homebranch} ) {
+                if (
+                    $request->{borrowerbranch} eq $item->{homebranch}
+                    && ( ( $item->{hold_fulfillment_policy} eq 'any' ) # Don't fill item level holds that contravene the hold pickup policy at this time
+                        || $request->{branchcode} eq $item->{ $item->{hold_fulfillment_policy} } )
+                  )
+                {
                     $itemnumber = $item->{itemnumber};
                     last;
                 }
@@ -453,6 +468,10 @@ sub MapItemsToHoldRequests {
                 foreach my $item (@$holding_branch_items) {
                     next if $request->{borrowerbranch} ne $item->{homebranch};
 
+                    # Don't fill item level holds that contravene the hold pickup policy at this time
+                    next unless $item->{hold_fulfillment_policy} eq 'any'
+                        || $request->{branchcode} eq $item->{ $item->{hold_fulfillment_policy} };
+
                     $itemnumber = $item->{itemnumber};
                     last;
                 }
@@ -481,6 +500,10 @@ sub MapItemsToHoldRequests {
                     next if $pickup_branch ne $item->{homebranch};
                     next if ( $item->{holdallowed} == 1 && $item->{homebranch} ne $request->{borrowerbranch} );
 
+                    # Don't fill item level holds that contravene the hold pickup policy at this time
+                    next unless $item->{hold_fulfillment_policy} eq 'any'
+                        || $request->{branchcode} eq $item->{ $item->{hold_fulfillment_policy} };
+
                     $itemnumber = $item->{itemnumber};
                     $holdingbranch = $branch;
                     last PULL_BRANCHES;
@@ -491,6 +514,11 @@ sub MapItemsToHoldRequests {
             unless ( $itemnumber ) {
                 foreach my $current_item ( @{ $items_by_branch{$holdingbranch} } ) {
                     if ( $holdingbranch && ( $current_item->{holdallowed} == 2 || $request->{borrowerbranch} eq $current_item->{homebranch} ) ) {
+
+                        # Don't fill item level holds that contravene the hold pickup policy at this time
+                        next unless $current_item->{hold_fulfillment_policy} eq 'any'
+                            || $request->{branchcode} eq $current_item->{ $current_item->{hold_fulfillment_policy} };
+
                         $itemnumber = $current_item->{itemnumber};
                         last; # quit this loop as soon as we have a suitable item
                     }
@@ -507,6 +535,10 @@ sub MapItemsToHoldRequests {
                     foreach my $item (@$holding_branch_items) {
                         next if ( $item->{holdallowed} == 1 && $item->{homebranch} ne $request->{borrowerbranch} );
 
+                        # Don't fill item level holds that contravene the hold pickup policy at this time
+                        next unless $item->{hold_fulfillment_policy} eq 'any'
+                            || $request->{branchcode} eq $item->{ $item->{hold_fulfillment_policy} };
+
                         $itemnumber = $item->{itemnumber};
                         $holdingbranch = $branch;
                         last PULL_BRANCHES2;
index fbeca06..44e384c 100644 (file)
@@ -952,6 +952,7 @@ sub CheckReserves {
                     my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$iteminfo->{'itype'});
                     next if ($branchitemrule->{'holdallowed'} == 0);
                     next if (($branchitemrule->{'holdallowed'} == 1) && ($branch ne $borrowerinfo->{'branchcode'}));
+                    next if ( ($branchitemrule->{hold_fulfillment_policy} ne 'any') && ($res->{branchcode} ne $iteminfo->{ $branchitemrule->{hold_fulfillment_policy} }) );
                     $priority = $res->{'priority'};
                     $highest  = $res;
                     last if $local_hold_match;
index ade2993..11a5637 100755 (executable)
@@ -193,6 +193,7 @@ elsif ($op eq "set-branch-defaults") {
     my $maxissueqty   = $input->param('maxissueqty');
     my $maxonsiteissueqty = $input->param('maxonsiteissueqty');
     my $holdallowed   = $input->param('holdallowed');
+    my $hold_fulfillment_policy = $input->param('hold_fulfillment_policy');
     my $returnbranch  = $input->param('returnbranch');
     $maxissueqty =~ s/\s//g;
     $maxissueqty = undef if $maxissueqty !~ /^\d+/;
@@ -205,34 +206,34 @@ elsif ($op eq "set-branch-defaults") {
         my $sth_search = $dbh->prepare("SELECT count(*) AS total
                                         FROM default_circ_rules");
         my $sth_insert = $dbh->prepare("INSERT INTO default_circ_rules
-                                        (maxissueqty, maxonsiteissueqty, holdallowed, returnbranch)
-                                        VALUES (?, ?, ?, ?)");
+                                        (maxissueqty, maxonsiteissueqty, holdallowed, hold_fulfillment_policy, returnbranch)
+                                        VALUES (?, ?, ?, ?, ?)");
         my $sth_update = $dbh->prepare("UPDATE default_circ_rules
-                                        SET maxissueqty = ?, maxonsiteissueqty = ?, holdallowed = ?, returnbranch = ?");
+                                        SET maxissueqty = ?, maxonsiteissueqty = ?, holdallowed = ?, hold_fulfillment_policy = ?, returnbranch = ?");
 
         $sth_search->execute();
         my $res = $sth_search->fetchrow_hashref();
         if ($res->{total}) {
-            $sth_update->execute($maxissueqty, $maxonsiteissueqty, $holdallowed, $returnbranch);
+            $sth_update->execute($maxissueqty, $maxonsiteissueqty, $holdallowed, $hold_fulfillment_policy, $returnbranch);
         } else {
-            $sth_insert->execute($maxissueqty, $maxonsiteissueqty, $holdallowed, $returnbranch);
+            $sth_insert->execute($maxissueqty, $maxonsiteissueqty, $holdallowed, $hold_fulfillment_policy, $returnbranch);
         }
     } else {
         my $sth_search = $dbh->prepare("SELECT count(*) AS total
                                         FROM default_branch_circ_rules
                                         WHERE branchcode = ?");
         my $sth_insert = $dbh->prepare("INSERT INTO default_branch_circ_rules
-                                        (branchcode, maxissueqty, maxonsiteissueqty, holdallowed, returnbranch)
-                                        VALUES (?, ?, ?, ?, ?)");
+                                        (branchcode, maxissueqty, maxonsiteissueqty, holdallowed, hold_fulfillment_policy, returnbranch)
+                                        VALUES (?, ?, ?, ?, ?, ?)");
         my $sth_update = $dbh->prepare("UPDATE default_branch_circ_rules
-                                        SET maxissueqty = ?, maxonsiteissueqty = ?, holdallowed = ?, returnbranch = ?
+                                        SET maxissueqty = ?, maxonsiteissueqty = ?, holdallowed = ?, hold_fulfillment_policy = ?, returnbranch = ?
                                         WHERE branchcode = ?");
         $sth_search->execute($branch);
         my $res = $sth_search->fetchrow_hashref();
         if ($res->{total}) {
-            $sth_update->execute($maxissueqty, $maxonsiteissueqty, $holdallowed, $returnbranch, $branch);
+            $sth_update->execute($maxissueqty, $maxonsiteissueqty, $holdallowed, $hold_fulfillment_policy, $returnbranch, $branch);
         } else {
-            $sth_insert->execute($branch, $maxissueqty, $maxonsiteissueqty, $holdallowed, $returnbranch);
+            $sth_insert->execute($branch, $maxissueqty, $maxonsiteissueqty, $holdallowed, $hold_fulfillment_policy, $returnbranch);
         }
     }
 }
@@ -340,9 +341,11 @@ elsif ($op eq "add-branch-cat") {
     }
 }
 elsif ($op eq "add-branch-item") {
-    my $itemtype  = $input->param('itemtype');
-    my $holdallowed   = $input->param('holdallowed');
-    my $returnbranch  = $input->param('returnbranch');
+    my $itemtype                = $input->param('itemtype');
+    my $holdallowed             = $input->param('holdallowed');
+    my $hold_fulfillment_policy = $input->param('hold_fulfillment_policy');
+    my $returnbranch            = $input->param('returnbranch');
+
     $holdallowed =~ s/\s//g;
     $holdallowed = undef if $holdallowed !~ /^\d+/;
 
@@ -351,34 +354,34 @@ elsif ($op eq "add-branch-item") {
             my $sth_search = $dbh->prepare("SELECT count(*) AS total
                                             FROM default_circ_rules");
             my $sth_insert = $dbh->prepare("INSERT INTO default_circ_rules
-                                            (holdallowed, returnbranch)
-                                            VALUES (?, ?)");
+                                            (holdallowed, hold_fulfillment_policy, returnbranch)
+                                            VALUES (?, ?, ?)");
             my $sth_update = $dbh->prepare("UPDATE default_circ_rules
-                                            SET holdallowed = ?, returnbranch = ?");
+                                            SET holdallowed = ?, hold_fulfillment_policy = ?, returnbranch = ?");
 
             $sth_search->execute();
             my $res = $sth_search->fetchrow_hashref();
             if ($res->{total}) {
-                $sth_update->execute($holdallowed, $returnbranch);
+                $sth_update->execute($holdallowed, $hold_fulfillment_policy, $returnbranch);
             } else {
-                $sth_insert->execute($holdallowed, $returnbranch);
+                $sth_insert->execute($holdallowed, $hold_fulfillment_policy, $returnbranch);
             }
         } else {
             my $sth_search = $dbh->prepare("SELECT count(*) AS total
                                             FROM default_branch_item_rules
                                             WHERE itemtype = ?");
             my $sth_insert = $dbh->prepare("INSERT INTO default_branch_item_rules
-                                            (itemtype, holdallowed, returnbranch)
-                                            VALUES (?, ?, ?)");
+                                            (itemtype, holdallowed, hold_fulfillment_policy, returnbranch)
+                                            VALUES (?, ?, ?, ?)");
             my $sth_update = $dbh->prepare("UPDATE default_branch_item_rules
-                                            SET holdallowed = ?, returnbranch = ?
+                                            SET holdallowed = ?, hold_fulfillment_policy = ?, returnbranch = ?
                                             WHERE itemtype = ?");
             $sth_search->execute($itemtype);
             my $res = $sth_search->fetchrow_hashref();
             if ($res->{total}) {
-                $sth_update->execute($holdallowed, $returnbranch, $itemtype);
+                $sth_update->execute($holdallowed, $hold_fulfillment_policy, $returnbranch, $itemtype);
             } else {
-                $sth_insert->execute($itemtype, $holdallowed, $returnbranch);
+                $sth_insert->execute($itemtype, $holdallowed, $hold_fulfillment_policy, $returnbranch);
             }
         }
     } elsif ($itemtype eq "*") {
@@ -386,17 +389,17 @@ elsif ($op eq "add-branch-item") {
                                         FROM default_branch_circ_rules
                                         WHERE branchcode = ?");
         my $sth_insert = $dbh->prepare("INSERT INTO default_branch_circ_rules
-                                        (branchcode, holdallowed, returnbranch)
-                                        VALUES (?, ?, ?)");
+                                        (branchcode, holdallowed, hold_fulfillment_policy, returnbranch)
+                                        VALUES (?, ?, ?, ?)");
         my $sth_update = $dbh->prepare("UPDATE default_branch_circ_rules
-                                        SET holdallowed = ?, returnbranch = ?
+                                        SET holdallowed = ?, hold_fulfillment_policy = ?, returnbranch = ?
                                         WHERE branchcode = ?");
         $sth_search->execute($branch);
         my $res = $sth_search->fetchrow_hashref();
         if ($res->{total}) {
-            $sth_update->execute($holdallowed, $returnbranch, $branch);
+            $sth_update->execute($holdallowed, $hold_fulfillment_policy, $returnbranch, $branch);
         } else {
-            $sth_insert->execute($branch, $holdallowed, $returnbranch);
+            $sth_insert->execute($branch, $holdallowed, $hold_fulfillment_policy, $returnbranch);
         }
     } else {
         my $sth_search = $dbh->prepare("SELECT count(*) AS total
@@ -404,19 +407,19 @@ elsif ($op eq "add-branch-item") {
                                         WHERE branchcode = ?
                                         AND   itemtype = ?");
         my $sth_insert = $dbh->prepare("INSERT INTO branch_item_rules
-                                        (branchcode, itemtype, holdallowed, returnbranch)
-                                        VALUES (?, ?, ?, ?)");
+                                        (branchcode, itemtype, holdallowed, hold_fulfillment_policy, returnbranch)
+                                        VALUES (?, ?, ?, ?, ?)");
         my $sth_update = $dbh->prepare("UPDATE branch_item_rules
-                                        SET holdallowed = ?, returnbranch = ?
+                                        SET holdallowed = ?, hold_fulfillment_policy = ?, returnbranch = ?
                                         WHERE branchcode = ?
                                         AND itemtype = ?");
 
         $sth_search->execute($branch, $itemtype);
         my $res = $sth_search->fetchrow_hashref();
         if ($res->{total}) {
-            $sth_update->execute($holdallowed, $returnbranch, $branch, $itemtype);
+            $sth_update->execute($holdallowed, $hold_fulfillment_policy, $returnbranch, $branch, $itemtype);
         } else {
-            $sth_insert->execute($branch, $itemtype, $holdallowed, $returnbranch);
+            $sth_insert->execute($branch, $itemtype, $holdallowed, $hold_fulfillment_policy, $returnbranch);
         }
     }
 }
@@ -549,8 +552,8 @@ my @sorted_branch_item_rules = sort { lc $a->{translated_description} cmp lc $b-
 
 # note undef holdallowed so that template can deal with them
 foreach my $entry (@sorted_branch_item_rules) {
-    $entry->{holdallowed_any} = 1 if($entry->{holdallowed} == 2);
-    $entry->{holdallowed_same} = 1 if($entry->{holdallowed} == 1);
+    $entry->{holdallowed_any}  = 1 if ( $entry->{holdallowed} == 2 );
+    $entry->{holdallowed_same} = 1 if ( $entry->{holdallowed} == 1 );
 }
 
 $template->param(show_branch_cat_rule_form => 1);
@@ -576,12 +579,13 @@ if ($branch eq "*") {
 my $defaults = $sth_defaults->fetchrow_hashref;
 
 if ($defaults) {
-    $template->param(default_holdallowed_none => 1) if($defaults->{holdallowed} == 0);
-    $template->param(default_holdallowed_same => 1) if($defaults->{holdallowed} == 1);
-    $template->param(default_holdallowed_any => 1) if($defaults->{holdallowed} == 2);
-    $template->param(default_maxissueqty => $defaults->{maxissueqty});
-    $template->param(default_maxonsiteissueqty => $defaults->{maxonsiteissueqty});
-    $template->param(default_returnbranch => $defaults->{returnbranch});
+    $template->param( default_holdallowed_none => 1 ) if ( $defaults->{holdallowed} == 0 );
+    $template->param( default_holdallowed_same => 1 ) if ( $defaults->{holdallowed} == 1 );
+    $template->param( default_holdallowed_any  => 1 ) if ( $defaults->{holdallowed} == 2 );
+    $template->param( default_hold_fulfillment_policy => $defaults->{hold_fulfillment_policy} );
+    $template->param( default_maxissueqty      => $defaults->{maxissueqty} );
+    $template->param( default_maxonsiteissueqty => $defaults->{maxonsiteissueqty} );
+    $template->param( default_returnbranch      => $defaults->{returnbranch} );
 }
 
 $template->param(default_rules => ($defaults ? 1 : 0));
diff --git a/installer/data/mysql/atomicupdate/hold_fulfillment_policy.sql b/installer/data/mysql/atomicupdate/hold_fulfillment_policy.sql
new file mode 100644 (file)
index 0000000..ee514e2
--- /dev/null
@@ -0,0 +1,4 @@
+ALTER TABLE branch_item_rules         ADD COLUMN hold_fulfillment_policy ENUM('any', 'homebranch', 'holdingbranch') NOT NULL DEFAULT 'any' AFTER holdallowed;
+ALTER TABLE default_branch_circ_rules ADD COLUMN hold_fulfillment_policy ENUM('any', 'homebranch', 'holdingbranch') NOT NULL DEFAULT 'any' AFTER holdallowed;
+ALTER TABLE default_branch_item_rules ADD COLUMN hold_fulfillment_policy ENUM('any', 'homebranch', 'holdingbranch') NOT NULL DEFAULT 'any' AFTER holdallowed;
+ALTER TABLE default_circ_rules        ADD COLUMN hold_fulfillment_policy ENUM('any', 'homebranch', 'holdingbranch') NOT NULL DEFAULT 'any' AFTER holdallowed;
index 028c85f..3bec3e5 100644 (file)
@@ -348,6 +348,7 @@ CREATE TABLE `branch_item_rules` ( -- information entered in the circulation and
   `branchcode` varchar(10) NOT NULL, -- the branch this rule is for (branches.branchcode)
   `itemtype` varchar(10) NOT NULL, -- the item type this rule applies to (items.itype)
   `holdallowed` tinyint(1) default NULL, -- the number of holds allowed
+  hold_fulfillment_policy ENUM('any', 'homebranch', 'holdingbranch') NOT NULL DEFAULT 'any', -- limit trapping of holds by branchcode
   `returnbranch` varchar(15) default NULL, -- the branch the item returns to (homebranch, holdingbranch, noreturn)
   PRIMARY KEY  (`itemtype`,`branchcode`),
   KEY `branch_item_rules_ibfk_2` (`branchcode`),
@@ -681,6 +682,7 @@ CREATE TABLE `default_branch_circ_rules` (
   `maxissueqty` int(4) default NULL,
   `maxonsiteissueqty` int(4) default NULL,
   `holdallowed` tinyint(1) default NULL,
+  hold_fulfillment_policy ENUM('any', 'homebranch', 'holdingbranch') NOT NULL DEFAULT 'any', -- limit trapping of holds by branchcode
   `returnbranch` varchar(15) default NULL,
   PRIMARY KEY (`branchcode`),
   CONSTRAINT `default_branch_circ_rules_ibfk_1` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`)
@@ -694,6 +696,7 @@ DROP TABLE IF EXISTS `default_branch_item_rules`;
 CREATE TABLE `default_branch_item_rules` (
   `itemtype` varchar(10) NOT NULL,
   `holdallowed` tinyint(1) default NULL,
+  hold_fulfillment_policy ENUM('any', 'homebranch', 'holdingbranch') NOT NULL DEFAULT 'any', -- limit trapping of holds by branchcode
   `returnbranch` varchar(15) default NULL,
   PRIMARY KEY  (`itemtype`),
   CONSTRAINT `default_branch_item_rules_ibfk_1` FOREIGN KEY (`itemtype`) REFERENCES `itemtypes` (`itemtype`)
@@ -710,6 +713,7 @@ CREATE TABLE `default_circ_rules` (
     `maxissueqty` int(4) default NULL,
     `maxonsiteissueqty` int(4) default NULL,
     `holdallowed` int(1) default NULL,
+    hold_fulfillment_policy ENUM('any', 'homebranch', 'holdingbranch') NOT NULL DEFAULT 'any', -- limit trapping of holds by branchcode
     `returnbranch` varchar(15) default NULL,
     PRIMARY KEY (`singleton`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
index c7e27ad..184258d 100644 (file)
@@ -379,6 +379,7 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde
                     <th>Total current checkouts allowed</th>
                     <th>Total current on-site checkouts allowed</th>
                     <th>Hold policy</th>
+                    <th>Hold pickup library match</th>
                     <th>Return policy</th>
                     <th>&nbsp;</th>
                     <th>&nbsp;</th>
@@ -412,6 +413,39 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde
                             </option>
                         </select>
                     </td>
+                    <td>
+                        <select name="hold_fulfillment_policy">
+                            [% IF default_hold_fulfillment_policy == 'any' %]
+                                <option value="any" selected="selected">
+                                    any library
+                                </option>
+                            [% ELSE %]
+                                <option value="any">
+                                    any library
+                                </option>
+                            [% END %]
+
+                            [% IF default_hold_fulfillment_policy == 'homebranch' %]
+                                <option value="homebranch" selected="selected">
+                                    item's home library
+                                </option>
+                            [% ELSE %]
+                                <option value="homebranch">
+                                    item's home library
+                                </option>
+                            [% END %]
+
+                            [% IF default_hold_fulfillment_policy == 'holdingbranch' %]
+                                <option value="holdingbranch" selected="selected">
+                                    item's holding library
+                                </option>
+                            [% ELSE %]
+                                <option value="holdingbranch">
+                                    item's holding library
+                                </option>
+                            [% END %]
+                        </select>
+                    </td>
                     <td>
                         <select name="returnbranch">
                             [% IF ( default_returnbranch == 'homebranch' ) %]
@@ -536,6 +570,7 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde
                 <tr>
                     <th>Item type</th>
                     <th>Hold policy</th>
+                    <th>Hold pickup library match</th>
                     <th>Return policy</th>
                     <th>&nbsp;</th>
                 </tr>
@@ -559,6 +594,14 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde
                                 No holds allowed
                             [% END %]
                         </td>
+                        <td>[% IF ( branch_item_rule_loo.hold_fulfillment_policy == 'any' ) %]
+                                any library
+                            [% ELSIF ( branch_item_rule_loo.hold_fulfillment_policy == 'homebranch' ) %]
+                                item's home library
+                            [% ELSIF ( branch_item_rule_loo.hold_fulfillment_policy == 'holdingbranch' ) %]
+                                item's holding library
+                            [% END %]
+                        </td>
                         <td>[% IF ( branch_item_rule_loo.returnbranch == 'homebranch' ) %]
                                 Item returns home
                             [% ELSIF ( branch_item_rule_loo.returnbranch == 'holdingbranch' ) %]
@@ -589,6 +632,21 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde
                             <option value="0">No holds allowed</option>
                         </select>
                     </td>
+                    <td>
+                        <select name="hold_fulfillment_policy">
+                            <option value="any">
+                                any library
+                            </option>
+
+                            <option value="homebranch">
+                                item's home library
+                            </option>
+
+                            <option value="holdingbranch">
+                                item's holding library
+                            </option>
+                        </select>
+                    </td>
                     <td>
                         <select name="returnbranch">
                             <option value="homebranch">Item returns home</option>
diff --git a/t/db_dependent/Holds/HoldFulfillmentPolicy.t b/t/db_dependent/Holds/HoldFulfillmentPolicy.t
new file mode 100755 (executable)
index 0000000..513742e
--- /dev/null
@@ -0,0 +1,152 @@
+#!/usr/bin/perl
+
+use Modern::Perl;
+
+use C4::Context;
+
+use Test::More tests => 10;
+
+use t::lib::TestBuilder;
+
+BEGIN {
+    use FindBin;
+    use lib $FindBin::Bin;
+    use_ok('C4::Reserves');
+}
+
+my $schema = Koha::Database->schema;
+$schema->storage->txn_begin;
+my $dbh = C4::Context->dbh;
+
+my $builder = t::lib::TestBuilder->new;
+
+my $library1 = $builder->build({
+    source => 'Branch',
+});
+my $library2 = $builder->build({
+    source => 'Branch',
+});
+my $library3 = $builder->build({
+    source => 'Branch',
+});
+
+my $bib_title = "Test Title";
+
+my $borrower = $builder->build({
+    source => 'Borrower',
+    value => {
+        categorycode => 'S',
+        branchcode => $library1->{branchcode},
+    }
+});
+
+# Test hold_fulfillment_policy
+my ( $itemtype ) = @{ $dbh->selectrow_arrayref("SELECT itemtype FROM itemtypes LIMIT 1") };
+my $borrowernumber = $borrower->{borrowernumber};
+my $library_A = $library1->{branchcode};
+my $library_B = $library2->{branchcode};
+my $library_C = $library3->{branchcode};
+$dbh->do("DELETE FROM reserves");
+$dbh->do("DELETE FROM issues");
+$dbh->do("DELETE FROM items");
+$dbh->do("DELETE FROM biblio");
+$dbh->do("DELETE FROM biblioitems");
+$dbh->do("DELETE FROM transport_cost");
+$dbh->do("DELETE FROM tmp_holdsqueue");
+$dbh->do("DELETE FROM hold_fill_targets");
+$dbh->do("DELETE FROM default_branch_circ_rules");
+$dbh->do("DELETE FROM default_branch_item_rules");
+$dbh->do("DELETE FROM default_circ_rules");
+$dbh->do("DELETE FROM branch_item_rules");
+
+$dbh->do("INSERT INTO biblio (frameworkcode, author, title, datecreated) VALUES ('', 'Koha test', '$bib_title', '2011-02-01')");
+
+my $biblionumber = $dbh->selectrow_array("SELECT biblionumber FROM biblio WHERE title = '$bib_title'")
+  or BAIL_OUT("Cannot find newly created biblio record");
+
+$dbh->do("INSERT INTO biblioitems (biblionumber, marcxml, itemtype) VALUES ($biblionumber, '', '$itemtype')");
+
+my $biblioitemnumber =
+  $dbh->selectrow_array("SELECT biblioitemnumber FROM biblioitems WHERE biblionumber = $biblionumber")
+  or BAIL_OUT("Cannot find newly created biblioitems record");
+
+$dbh->do("
+    INSERT INTO items (biblionumber, biblioitemnumber, homebranch, holdingbranch, notforloan, damaged, itemlost, withdrawn, onloan, itype)
+    VALUES ($biblionumber, $biblioitemnumber, '$library_A', '$library_B', 0, 0, 0, 0, NULL, '$itemtype')
+");
+
+my $itemnumber =
+  $dbh->selectrow_array("SELECT itemnumber FROM items WHERE biblionumber = $biblionumber")
+  or BAIL_OUT("Cannot find newly created item");
+
+# With hold_fulfillment_policy = homebranch, hold should only be picked up if pickup branch = homebranch
+$dbh->do("DELETE FROM default_circ_rules");
+$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'homebranch' )");
+
+my $holds_queue;
+# Home branch matches pickup branch
+my $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
+my ( $status ) = CheckReserves($itemnumber);
+is( $status, 'Reserved', "Hold where pickup branch matches home branch targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Holding branch matches pickup branch
+$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
+( $status ) = CheckReserves($itemnumber);
+is($status, q{}, "Hold where pickup ne home, pickup eq home not targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Neither branch matches pickup branch
+$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
+( $status ) = CheckReserves($itemnumber);
+is( $status, q{}, "Hold where pickup ne home, pickup ne holding not targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# With hold_fulfillment_policy = holdingbranch, hold should only be picked up if pickup branch = holdingbranch
+$dbh->do("DELETE FROM default_circ_rules");
+$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'holdingbranch' )");
+
+# Home branch matches pickup branch
+$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
+( $status ) = CheckReserves($itemnumber);
+is( $status, q{}, "Hold where pickup eq home, pickup ne holding not targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Holding branch matches pickup branch
+$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
+( $status ) = CheckReserves($itemnumber);
+is( $status, 'Reserved', "Hold where pickup ne home, pickup eq holding targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Neither branch matches pickup branch
+$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
+( $status ) = CheckReserves($itemnumber);
+is( $status, q{}, "Hold where pickup ne home, pickup ne holding not targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# With hold_fulfillment_policy = any, hold should be pikcup up reguardless of matching home or holding branch
+$dbh->do("DELETE FROM default_circ_rules");
+$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'any' )");
+
+# Home branch matches pickup branch
+$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
+( $status ) = CheckReserves($itemnumber);
+is( $status, 'Reserved', "Hold where pickup eq home, pickup ne holding targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Holding branch matches pickup branch
+$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
+( $status ) = CheckReserves($itemnumber);
+is( $status, 'Reserved', "Hold where pickup ne home, pickup eq holding targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Neither branch matches pickup branch
+$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
+( $status ) = CheckReserves($itemnumber);
+is( $status, 'Reserved', "Hold where pickup ne home, pickup ne holding targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# End testing hold_fulfillment_policy
+
+# Cleanup
+$schema->storage->txn_rollback;
index f0141fb..18049bb 100755 (executable)
@@ -8,9 +8,10 @@
 
 use Modern::Perl;
 
-use Test::More tests => 26;
+use Test::More tests => 35;
 use Data::Dumper;
 
+
 use C4::Branch;
 use C4::Calendar;
 use C4::Context;
@@ -51,7 +52,6 @@ my $TITLE = "Test Holds Queue XXX";
 my $borrower = $builder->build({
     source => 'Borrower',
     value => {
-        categorycode => 'S',
         branchcode => $library1->{branchcode},
     }
 });
@@ -196,21 +196,18 @@ $library3 = $builder->build({
 my $borrower1 = $builder->build({
     source => 'Borrower',
     value => {
-        categorycode => 'S',
         branchcode => $branchcodes[0],
     },
 });
 my $borrower2 = $builder->build({
     source => 'Borrower',
     value => {
-        categorycode => 'S',
         branchcode => $branchcodes[1],
     },
 });
 my $borrower3 = $builder->build({
     source => 'Borrower',
     value => {
-        categorycode => 'S',
         branchcode => $branchcodes[2],
     },
 });
@@ -446,6 +443,118 @@ $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice
 is( @$holds_queue, 0, "Bug 15062 - Holds queue with Transport Cost Matrix will transfer item even if transfers disabled");
 # End Bug 15062
 
+# Test hold_fulfillment_policy
+C4::Context->set_preference( "UseTransportCostMatrix", 0 );
+$borrowernumber = $borrower3->{borrowernumber};
+$library_A = $library1->{branchcode};
+$library_B = $library2->{branchcode};
+$library_C = $library3->{branchcode};
+$dbh->do("DELETE FROM reserves");
+$dbh->do("DELETE FROM issues");
+$dbh->do("DELETE FROM items");
+$dbh->do("DELETE FROM biblio");
+$dbh->do("DELETE FROM biblioitems");
+$dbh->do("DELETE FROM transport_cost");
+$dbh->do("DELETE FROM tmp_holdsqueue");
+$dbh->do("DELETE FROM hold_fill_targets");
+$dbh->do("DELETE FROM default_branch_circ_rules");
+$dbh->do("DELETE FROM default_branch_item_rules");
+$dbh->do("DELETE FROM default_circ_rules");
+$dbh->do("DELETE FROM branch_item_rules");
+
+$dbh->do("INSERT INTO biblio (frameworkcode, author, title, datecreated) VALUES ('', 'Koha test', '$TITLE', '2011-02-01')");
+
+$biblionumber = $dbh->selectrow_array("SELECT biblionumber FROM biblio WHERE title = '$TITLE'")
+  or BAIL_OUT("Cannot find newly created biblio record");
+
+$dbh->do("INSERT INTO biblioitems (biblionumber, marcxml, itemtype) VALUES ($biblionumber, '', '$itemtype')");
+
+$biblioitemnumber =
+  $dbh->selectrow_array("SELECT biblioitemnumber FROM biblioitems WHERE biblionumber = $biblionumber")
+  or BAIL_OUT("Cannot find newly created biblioitems record");
+
+$dbh->do("
+    INSERT INTO items (biblionumber, biblioitemnumber, homebranch, holdingbranch, notforloan, damaged, itemlost, withdrawn, onloan, itype)
+    VALUES ($biblionumber, $biblioitemnumber, '$library_A', '$library_B', 0, 0, 0, 0, NULL, '$itemtype')
+");
+
+# With hold_fulfillment_policy = homebranch, hold should only be picked up if pickup branch = homebranch
+$dbh->do("DELETE FROM default_circ_rules");
+$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'homebranch' )");
+
+# Home branch matches pickup branch
+$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
+C4::HoldsQueue::CreateQueue();
+$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
+is( @$holds_queue, 1, "Hold where pickup branch matches home branch targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Holding branch matches pickup branch
+$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
+C4::HoldsQueue::CreateQueue();
+$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
+is( @$holds_queue, 0, "Hold where pickup ne home, pickup eq home not targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Neither branch matches pickup branch
+$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
+C4::HoldsQueue::CreateQueue();
+$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
+is( @$holds_queue, 0, "Hold where pickup ne home, pickup ne holding not targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# With hold_fulfillment_policy = holdingbranch, hold should only be picked up if pickup branch = holdingbranch
+$dbh->do("DELETE FROM default_circ_rules");
+$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'holdingbranch' )");
+
+# Home branch matches pickup branch
+$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
+C4::HoldsQueue::CreateQueue();
+$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
+is( @$holds_queue, 0, "Hold where pickup eq home, pickup ne holding not targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Holding branch matches pickup branch
+$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
+C4::HoldsQueue::CreateQueue();
+$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
+is( @$holds_queue, 1, "Hold where pickup ne home, pickup eq holding targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Neither branch matches pickup branch
+$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
+C4::HoldsQueue::CreateQueue();
+$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
+is( @$holds_queue, 0, "Hold where pickup ne home, pickup ne holding not targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# With hold_fulfillment_policy = any, hold should be pikcup up reguardless of matching home or holding branch
+$dbh->do("DELETE FROM default_circ_rules");
+$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'any' )");
+
+# Home branch matches pickup branch
+$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
+C4::HoldsQueue::CreateQueue();
+$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
+is( @$holds_queue, 1, "Hold where pickup eq home, pickup ne holding targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Holding branch matches pickup branch
+$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
+C4::HoldsQueue::CreateQueue();
+$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
+is( @$holds_queue, 1, "Hold where pickup ne home, pickup eq holding targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# Neither branch matches pickup branch
+$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
+C4::HoldsQueue::CreateQueue();
+$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
+is( @$holds_queue, 1, "Hold where pickup ne home, pickup ne holding targeted" );
+CancelReserve( { reserve_id => $reserve_id } );
+
+# End testing hold_fulfillment_policy
+
 # Cleanup
 $schema->storage->txn_rollback;