@@ -, +, @@ home/holding branch matches the hold's pickup branch to fill a given hold and return policy" and for "holds policy by item type" the pickup branch --- 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 | 115 +++++++++++++++- 9 files changed, 427 insertions(+), 57 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/hold_fulfillment_policy.sql create mode 100755 t/db_dependent/Holds/HoldFulfillmentPolicy.t --- a/C4/Circulation.pm +++ a/C4/Circulation.pm @@ -1675,17 +1675,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'], ); @@ -1698,11 +1698,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; --- a/C4/HoldsQueue.pm +++ a/C4/HoldsQueue.pm @@ -343,6 +343,7 @@ sub GetItemsAvailableToFillHoldRequestsForBib { return [ grep { my $rule = GetBranchItemRule($_->{homebranch}, $_->{itype}); $_->{holdallowed} = $rule->{holdallowed}; + $_->{hold_fulfillment_policy} = $rule->{hold_fulfillment_policy}; } @items ]; } @@ -381,18 +382,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 { @@ -425,7 +435,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; } @@ -441,6 +456,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; } @@ -469,6 +488,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; @@ -479,6 +502,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 } @@ -495,6 +523,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; --- a/C4/Reserves.pm +++ a/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; --- a/admin/smart-rules.pl +++ a/admin/smart-rules.pl @@ -189,6 +189,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+/; @@ -201,34 +202,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); } } } @@ -336,9 +337,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+/; @@ -347,34 +350,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 "*") { @@ -382,17 +385,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 @@ -400,19 +403,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); } } } @@ -536,8 +539,8 @@ my @sorted_branch_item_rules = sort { $a->{'humanitemtype'} cmp $b->{'humanitemt # 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); @@ -563,12 +566,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)); --- a/installer/data/mysql/atomicupdate/hold_fulfillment_policy.sql +++ a/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; --- a/installer/data/mysql/kohastructure.sql +++ a/installer/data/mysql/kohastructure.sql @@ -347,6 +347,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`), @@ -667,6 +668,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`) @@ -680,6 +682,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`) @@ -696,6 +699,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; --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -379,6 +379,7 @@ for="tobranch">Clone these rules to: Clone these rules to: + [% IF default_hold_fulfillment_policy == 'any' %] + + [% ELSE %] + + [% END %] + + [% IF default_hold_fulfillment_policy == 'homebranch' %] + + [% ELSE %] + + [% END %] + + [% IF default_hold_fulfillment_policy == 'holdingbranch' %] + + [% ELSE %] + + [% END %] + + + Clone these rules to: Clone these rules to: + + + + + + + +