From d8c245b505a78df3dd7cf8e1a3be196191c299fd Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 2 Sep 2026 17:13:26 +0100 Subject: [PATCH] Bug 42490: (QA follow-up) Address QA review comments - Remove leftover skeleton boilerplate comments in the atomicupdate ("Do you stuffs here", "# sysprefs") - Fix typo in atomicupdate description ("preferene" -> "preference") - Update POD on MapItemsToHoldRequests to mention the new LocalHoldsPriorityScope gate - Explicitly mock LocalHoldsPriorityScope in the existing Local Holds Priority subtests that exercise CreateQueue/MapItemsToHoldRequests, instead of relying on it falling through to the real DB default Signed-off-by: Martin Renvoize Signed-off-by: Pedro Amorim --- C4/HoldsQueue.pm | 3 ++- installer/data/mysql/atomicupdate/bug_42490.pl | 4 +--- t/db_dependent/HoldsQueue.t | 10 ++++++++-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm index 4fb2bab26b..b5a889b922 100644 --- a/C4/HoldsQueue.pm +++ b/C4/HoldsQueue.pm @@ -725,7 +725,8 @@ RETRY: about the hold it has been mapped to. This routine attempts to match the holds in the following priority - 1 - If local holds priority is enabled we check all requests to see if local matches can be found + 1 - If local holds priority is enabled, and LocalHoldsPriorityScope includes the holds queue, + we check all requests to see if local matches can be found 2 - We check for item level matches and fill those 3 - We now loop the remaining requests in priority order attempting to fill with a - Items where HoldsQueuePrioritizeBranch matches either from items held at the pickup branch, or at the least cost branch (if Transport Cost Matrix is being used) diff --git a/installer/data/mysql/atomicupdate/bug_42490.pl b/installer/data/mysql/atomicupdate/bug_42490.pl index 377b71afa7..e9da94147a 100755 --- a/installer/data/mysql/atomicupdate/bug_42490.pl +++ b/installer/data/mysql/atomicupdate/bug_42490.pl @@ -3,12 +3,11 @@ use Koha::Installer::Output qw(say_warning say_success say_info); return { bug_number => "42490", - description => "Add system preferene LocalHoldsPriorityScope", + description => "Add system preference LocalHoldsPriorityScope", up => sub { my ($args) = @_; my ( $dbh, $out ) = @$args{qw(dbh out)}; - # Do you stuffs here $dbh->do( q{ INSERT IGNORE INTO systempreferences (variable, value, options, explanation, type) @@ -22,7 +21,6 @@ return { } ); - # sysprefs say $out "Added new system preference 'LocalHoldsPriorityScope'"; }, }; diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t index d9f2cfd814..db595a0f96 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -436,7 +436,8 @@ is( scalar(@$holds_queue), 2, "Holds not filled with items from closed libraries t::lib::Mocks::mock_preference( 'HoldsQueueSkipClosed', 0 ); ## Test LocalHoldsPriority -t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 'GiveLibrary' ); +t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 'GiveLibrary' ); +t::lib::Mocks::mock_preference( 'LocalHoldsPriorityScope', 'checkin_and_queue' ); $dbh->do("DELETE FROM circulation_rules"); Koha::CirculationRules->set_rule( @@ -1035,6 +1036,7 @@ subtest "Test Local Holds Priority - Bib level" => sub { Koha::Biblios->delete(); t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 'GiveLibrary' ); + t::lib::Mocks::mock_preference( 'LocalHoldsPriorityScope', 'checkin_and_queue' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'PickupLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' ); my $branch = $builder->build_object( { class => 'Koha::Libraries' } ); @@ -1196,7 +1198,7 @@ subtest "Test Local Holds Priority - Item level hold over Record level hold (Bug t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 'GiveLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'PickupLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' ); - t::lib::Mocks::mock_preference( 'LocalHoldsPriorityScope', 'checkin_and_queue' ); + t::lib::Mocks::mock_preference( 'LocalHoldsPriorityScope', 'checkin_and_queue' ); my $branch = $builder->build_object( { class => 'Koha::Libraries' } ); my $branch2 = $builder->build_object( { class => 'Koha::Libraries' } ); my $category = $builder->build_object( @@ -1266,6 +1268,7 @@ subtest "Test Local Holds Priority - Get correct item for item level hold" => su Koha::Biblios->delete(); t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 'GiveLibrary' ); + t::lib::Mocks::mock_preference( 'LocalHoldsPriorityScope', 'checkin_and_queue' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'PickupLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' ); my $branch = $builder->build_object( { class => 'Koha::Libraries' } ); @@ -1350,6 +1353,7 @@ subtest "Test Local Holds Priority - Ensure no duplicate requests in holds queue Koha::Biblios->delete(); t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 'GiveLibrary' ); + t::lib::Mocks::mock_preference( 'LocalHoldsPriorityScope', 'checkin_and_queue' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'PickupLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' ); my $branch = $builder->build_object( { class => 'Koha::Libraries' } ); @@ -1515,6 +1519,7 @@ subtest 'Excludes from local holds priority' => sub { Koha::Holds->delete; t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 'GiveLibrary' ); + t::lib::Mocks::mock_preference( 'LocalHoldsPriorityScope', 'checkin_and_queue' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'PickupLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' ); @@ -2687,6 +2692,7 @@ subtest "local_holdgroup_match uses exclusivity controls, not LocalHoldsPriority # Exclusivity (PickupLibrary/holdingbranch): pickup lib_a vs lib_c holdingbranch -> NOT in group -> flag = 0 t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 'GiveLibrary' ); + t::lib::Mocks::mock_preference( 'LocalHoldsPriorityScope', 'checkin_and_queue' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'HomeLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' ); t::lib::Mocks::mock_preference( 'LocalHoldsExclusivityPeriod', 7 );