Bugzilla – Attachment 49225 Details for
Bug 15532
Add ability to allow only items whose home/holding branch matches the hold's pickup branch to fill a given hold
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 15532: Add ability to allow only items whose home/holding branch matches the hold's pickup branch to fill a given hold
Bug-15532-Add-ability-to-allow-only-items-whose-ho.patch (text/plain), 44.24 KB, created by
Kyle M Hall (khall)
on 2016-03-16 13:28:11 UTC
(
hide
)
Description:
Bug 15532: Add ability to allow only items whose home/holding branch matches the hold's pickup branch to fill a given hold
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2016-03-16 13:28:11 UTC
Size:
44.24 KB
patch
obsolete
>From 59b40f1e1b64b84a8e38971b2c7188e2820a7312 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Sat, 19 Dec 2015 11:34:03 +0000 >Subject: [PATCH] Bug 15532: Add ability to allow only items whose home/holding > branch matches the hold's pickup branch to fill a given hold > >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 >--- > C4/Circulation.pm | 10 +- > C4/HoldsQueue.pm | 54 ++++++-- > C4/Reserves.pm | 1 + > admin/smart-rules.pl | 86 ++++++------ > .../mysql/atomicupdate/hold_fulfillment_policy.sql | 4 + > installer/data/mysql/kohastructure.sql | 4 + > .../prog/en/modules/admin/smart-rules.tt | 58 ++++++++ > t/db_dependent/Holds/HoldFulfillmentPolicy.t | 152 +++++++++++++++++++++ > t/db_dependent/HoldsQueue.t | 119 +++++++++++++++- > 9 files changed, 427 insertions(+), 61 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/hold_fulfillment_policy.sql > create mode 100755 t/db_dependent/Holds/HoldFulfillmentPolicy.t > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index a1d7815..b3b6690 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1725,17 +1725,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'], > ); > >@@ -1748,11 +1748,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; >diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm >index 3cfe206..e547f10 100755 >--- a/C4/HoldsQueue.pm >+++ b/C4/HoldsQueue.pm >@@ -356,6 +356,7 @@ sub GetItemsAvailableToFillHoldRequestsForBib { > return [ grep { > my $rule = GetBranchItemRule($_->{homebranch}, $_->{itype}); > $_->{holdallowed} = $rule->{holdallowed}; >+ $_->{hold_fulfillment_policy} = $rule->{hold_fulfillment_policy}; > } @items ]; > } > >@@ -394,18 +395,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 { >@@ -438,7 +448,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; > } >@@ -454,6 +469,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; > } >@@ -482,6 +501,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; >@@ -492,6 +515,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 > } >@@ -508,6 +536,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; >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index e0d616b..fabc0ab 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -954,6 +954,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; >diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl >index da4e577..2cbee8a 100755 >--- a/admin/smart-rules.pl >+++ b/admin/smart-rules.pl >@@ -192,6 +192,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+/; >@@ -204,34 +205,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); > } > } > } >@@ -339,9 +340,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+/; > >@@ -350,34 +353,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 "*") { >@@ -385,17 +388,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 >@@ -403,19 +406,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); > } > } > } >@@ -548,8 +551,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); >@@ -575,12 +578,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 >index 0000000..ee514e2 >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/hold_fulfillment_policy.sql >@@ -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; >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index 63fefe7..263ed10 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -350,6 +350,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`), >@@ -683,6 +684,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`) >@@ -696,6 +698,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`) >@@ -712,6 +715,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; >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 c7e27ad..184258d 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 >@@ -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> </th> > <th> </th> >@@ -413,6 +414,39 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde > </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' ) %] > <option value="homebranch" selected="selected"> >@@ -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> </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' ) %] >@@ -590,6 +633,21 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde > </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> > <option value="holdingbranch">Item returns to issuing library</option> >diff --git a/t/db_dependent/Holds/HoldFulfillmentPolicy.t b/t/db_dependent/Holds/HoldFulfillmentPolicy.t >new file mode 100755 >index 0000000..513742e >--- /dev/null >+++ b/t/db_dependent/Holds/HoldFulfillmentPolicy.t >@@ -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; >diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t >index f0141fb..18049bb 100755 >--- a/t/db_dependent/HoldsQueue.t >+++ b/t/db_dependent/HoldsQueue.t >@@ -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; > >-- >2.1.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 15532
:
46461
|
46753
|
49225
|
50358
|
50838
|
50901
|
50902
|
50903