From 959be3d1d0f85272511307bdf173b5f9bdea92cb Mon Sep 17 00:00:00 2001 From: Kyle Hall Date: Tue, 18 Oct 2022 12:25:35 -0400 Subject: [PATCH] Bug 31557: Add ability for holds queue builder to prioritize either matching a patron's home library to the item's home or holding library Right now the holds queue builder starts filling bib-level holds with items whose patron's home library matches the item's home library. It would be good and reasonable to have the option to prioritize item's whose patron's home library matches the item's holding library to minimize transfers. Signed-off-by: Andrew Fuerste-Henry --- C4/HoldsQueue.pm | 5 +- .../data/mysql/atomicupdate/bug_31557.pl | 14 +++ installer/data/mysql/mandatory/sysprefs.sql | 1 + .../admin/preferences/circulation.pref | 7 ++ t/db_dependent/HoldsQueue.t | 95 ++++++++++++++++++- 5 files changed, 119 insertions(+), 3 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_31557.pl diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm index 7d0d4abc25..d39d0cc9b6 100644 --- a/C4/HoldsQueue.pm +++ b/C4/HoldsQueue.pm @@ -560,12 +560,13 @@ sub MapItemsToHoldRequests { my ($itemnumber, $holdingbranch); my $holding_branch_items = $items_by_branch{$pickup_branch}; + my $priority_branch = C4::Context->preference('HoldsQueuePrioritizeBranch') // 'homebranch'; if ( $holding_branch_items ) { foreach my $item (@$holding_branch_items) { next unless $items_by_itemnumber{ $item->{itemnumber} }->{_object}->can_be_transferred( { to => $libraries->{ $request->{branchcode} } } ); if ( - $request->{borrowerbranch} eq $item->{homebranch} + $request->{borrowerbranch} eq $item->{$priority_branch} && _checkHoldPolicy($item, $request) # Don't fill item level holds that contravene the hold pickup policy at this time && ( !$request->{itemtype} # If hold itemtype is set, item's itemtype must match || ( $request->{itemnumber} && ( $items_by_itemnumber{ $request->{itemnumber} }->{itype} eq $request->{itemtype} ) ) ) @@ -584,7 +585,7 @@ sub MapItemsToHoldRequests { my $holding_branch_items = $items_by_branch{$holdingbranch}; foreach my $item (@$holding_branch_items) { - next if $request->{borrowerbranch} ne $item->{homebranch}; + next if $request->{borrowerbranch} ne $item->{$priority_branch}; next unless $items_by_itemnumber{ $item->{itemnumber} }->{_object}->can_be_transferred( { to => $libraries->{ $request->{branchcode} } } ); # Don't fill item level holds that contravene the hold pickup policy at this time diff --git a/installer/data/mysql/atomicupdate/bug_31557.pl b/installer/data/mysql/atomicupdate/bug_31557.pl new file mode 100755 index 0000000000..bbd4109bec --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_31557.pl @@ -0,0 +1,14 @@ +use Modern::Perl; + +return { + bug_number => "31557", + description => "Add syspref HoldsQueuePrioritizeBranch", + up => sub { + my ($args) = @_; + my ($dbh, $out) = @$args{qw(dbh out)}; + $dbh->do(q{ + INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES + ('HoldsQueuePrioritizeBranch','homebranch','holdingbranch|homebranch','Decides if holds queue builder patron home library match to home or holding branch','Choice') + }); + }, +}; diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index 47a49d331f..89c7a8c20d 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -264,6 +264,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('HoldsLog','0',NULL,'If ON, log create/cancel/suspend/resume actions on holds.','YesNo'), ('HoldsNeedProcessingSIP', '0', NULL, 'Require staff to check-in before hold is set to waiting state', 'YesNo' ), ('HoldsQueueSkipClosed', '0', NULL, 'If enabled, any libraries that are closed when the holds queue is built will be ignored for the purpose of filling holds.', 'YesNo'), +('HoldsQueuePrioritizeBranch','homebranch','holdingbranch|homebranch','Decides if holds queue builder patron home library match to home or holding branch','Choice'), ('HoldsSplitQueue','nothing','nothing|branch|itemtype|branch_itemtype','In the staff interface, split the holds view by the given criteria','Choice'), ('HoldsSplitQueueNumbering', 'actual', 'actual|virtual', 'If the holds queue is split, decide if the actual priorities should be displayed', 'Choice'), ('HoldsToPullStartDate','2',NULL,'Set the default start date for the Holds to pull list to this many days ago','Integer'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index c2652a77da..40f8336bcd 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -782,6 +782,13 @@ Circulation: - pref: ExpireReservesMaxPickUpDelayCharge class: currency - "." + - + - The holds queue should prioritize filling a hold by matching the patron's home library with an item having a matching + - pref: HoldsQueuePrioritizeBranch + type: choice + choices: + homebranch: home library (homebranch). + holdingbranch: holding library (holdingbranch). - - Satisfy holds using items from the libraries - pref: StaticHoldsQueueWeight diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t index 7067931c35..a2f10f74fc 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -8,7 +8,7 @@ use Modern::Perl; -use Test::More tests => 58; +use Test::More tests => 59; use Data::Dumper; use C4::Calendar qw( new insert_single_holiday ); @@ -1817,3 +1817,96 @@ subtest "GetHoldsQueueItems" => sub { $schema->storage->txn_rollback; }; + +subtest "Test HoldsQueuePrioritizeBranch" => sub { + plan tests => 4; + + Koha::Biblios->delete(); + t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 0 ); + t::lib::Mocks::mock_preference( 'HoldsQueuePrioritizeBranch', 'homebranch' ); + t::lib::Mocks::mock_preference('UseTransportCostMatrix', 0); + + my $branch1 = $builder->build_object( { class => 'Koha::Libraries' } ); + my $branch2 = $builder->build_object( { class => 'Koha::Libraries' } ); + my $category = $builder->build_object( { class => 'Koha::Patron::Categories' }); + my $patron = $builder->build_object( + { + class => "Koha::Patrons", + value => { + branchcode => $branch1->branchcode, + categorycode => $category->categorycode + } + } + ); + + my $biblio = $builder->build_sample_biblio(); + my $item1 = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + library => $branch1->branchcode, + } + )->holdingbranch( $branch2->id )->store(); + + my $item2 = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + library => $branch1->branchcode, + } + )->homebranch( $branch2->id )->store(); + + my $reserve_id = AddReserve( + { + branchcode => $branch1->branchcode, + borrowernumber => $patron->borrowernumber, + biblionumber => $biblio->biblionumber, + priority => 1, + } + ); + + C4::HoldsQueue::CreateQueue(); + + my $queue_rs = $schema->resultset('TmpHoldsqueue'); + my $target_rs = $schema->resultset('HoldFillTarget'); + is( + $queue_rs->next->itemnumber->itemnumber, + $item1->itemnumber, + "Picked the item whose homebranch matches the pickup branch" + ); + + t::lib::Mocks::mock_preference( 'HoldsQueuePrioritizeBranch', 'holdingbranch' ); + + C4::HoldsQueue::CreateQueue(); + + $queue_rs = $schema->resultset('TmpHoldsqueue'); + $target_rs = $schema->resultset('HoldFillTarget'); + is( + $queue_rs->next->itemnumber->itemnumber, + $item2->itemnumber, + "Picked the item whose holdingbranch matches the pickup branch" + ); + + t::lib::Mocks::mock_preference('UseTransportCostMatrix', 1); + t::lib::Mocks::mock_preference( 'HoldsQueuePrioritizeBranch', 'homebranch' ); + + C4::HoldsQueue::CreateQueue(); + + $queue_rs = $schema->resultset('TmpHoldsqueue'); + $target_rs = $schema->resultset('HoldFillTarget'); + is( + $queue_rs->next->itemnumber->itemnumber, + $item1->itemnumber, + "Picked the item whose homebranch matches the pickup branch" + ); + + t::lib::Mocks::mock_preference( 'HoldsQueuePrioritizeBranch', 'holdingbranch' ); + + C4::HoldsQueue::CreateQueue(); + + $queue_rs = $schema->resultset('TmpHoldsqueue'); + $target_rs = $schema->resultset('HoldFillTarget'); + is( + $queue_rs->next->itemnumber->itemnumber, + $item2->itemnumber, + "Picked the item whose holdingbranch matches the pickup branch" + ); +}; -- 2.30.2