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 <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Martin Renvoize 2026-09-02 17:13:26 +01:00 committed by Pedro Amorim
parent fa3aacffd8
commit d8c245b505
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
3 changed files with 11 additions and 6 deletions

View File

@ -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)

View File

@ -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'";
},
};

View File

@ -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 );