From 6dfb21cf11bf4da610285fbd171fd7bf94ed5239 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Tue, 25 Aug 2020 10:35:02 +0000 Subject: [PATCH] Bug 18958: (follow-up) Ensure hold fill target reserve_id is set for all hold types MapItemsToHoldRequests has three sections: Local holds, item level holds, bib level holds Only one of them was setting the reserve_id. This patch makes al three set it and adds tests To test: 1 - Repeat test plan on bug 2 - sudo koha-mysql kohadev SELECT * FROM hold_fill_targets 3 - Ensure reserve_id is set at appropriate times 4 - prove -v t/db_dependent/HoldsQueue.t Signed-off-by: Marcel de Rooy Bug 18958: (QA follow-up) Fix number of tests In HoldsQueue.t Signed-off-by: Marcel de Rooy Signed-off-by: Jonathan Druart Signed-off-by: Jonathan Druart Signed-off-by: Kyle M Hall https://bugs.koha-community.org/show_bug.cgi?id=25265 --- C4/HoldsQueue.pm | 2 ++ t/db_dependent/HoldsQueue.t | 12 +++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm index 08f8a4eb94..b891afc437 100644 --- a/C4/HoldsQueue.pm +++ b/C4/HoldsQueue.pm @@ -481,6 +481,7 @@ sub MapItemsToHoldRequests { biblionumber => $request->{biblionumber}, holdingbranch => $items_by_itemnumber{ $request->{itemnumber} }->{holdingbranch}, pickup_branch => $request->{branchcode} || $request->{borrowerbranch}, + reserve_id => $request->{reserve_id}, item_level => $request->{item_level_hold}, reservedate => $request->{reservedate}, reservenotes => $request->{reservenotes}, @@ -656,6 +657,7 @@ sub MapItemsToHoldRequests { biblionumber => $request->{biblionumber}, holdingbranch => $holdingbranch, pickup_branch => $pickup_branch, + reserve_id => $request->{reserve_id}, item_level => $request->{item_level_hold}, reservedate => $request->{reservedate}, reservenotes => $request->{reservenotes}, diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t index 518f6d3fb4..06047c647f 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -8,7 +8,7 @@ use Modern::Perl; -use Test::More tests => 54; +use Test::More tests => 55; use Data::Dumper; use C4::Calendar; @@ -669,6 +669,8 @@ $reserve_id = AddReserve( 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" ); +my $target_rs = $schema->resultset('HoldFillTarget'); +is( $target_rs->next->reserve_id, $reserve_id, "Reserve id correctly set in hold fill target for title level hold" ); Koha::Holds->find( $reserve_id )->cancel; # Holding branch matches pickup branch @@ -915,7 +917,7 @@ Koha::Holds->find( $reserve_id )->cancel; subtest "Test Local Holds Priority - Bib level" => sub { - plan tests => 2; + plan tests => 3; Koha::Biblios->delete(); t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 1 ); @@ -971,6 +973,7 @@ subtest "Test Local Holds Priority - Bib level" => sub { C4::HoldsQueue::CreateQueue(); my $queue_rs = $schema->resultset('TmpHoldsqueue'); + my $target_rs = $schema->resultset('HoldFillTarget'); is( $queue_rs->count(), 1, "Hold queue contains one hold" ); is( @@ -978,6 +981,7 @@ subtest "Test Local Holds Priority - Bib level" => sub { $local_patron->borrowernumber, "We should pick the local hold over the next available" ); + is( $target_rs->next->reserve_id, $reserve_id2, "Reserve id correctly set in hold fill target" ); }; subtest "Test Local Holds Priority - Item level" => sub { @@ -1263,7 +1267,7 @@ subtest "Test Local Holds Priority - Ensure no duplicate requests in holds queue subtest "Item level holds info is preserved (Bug 25738)" => sub { - plan tests => 3; + plan tests => 4; $dbh->do("DELETE FROM tmp_holdsqueue"); $dbh->do("DELETE FROM hold_fill_targets"); @@ -1336,6 +1340,8 @@ subtest "Item level holds info is preserved (Bug 25738)" => sub { my $queue_line_1 = $queue_rs->next; is( $queue_line_1->item_level_request, 1, 'Request is correctly advertised as item-level' ); + my $target_rs = $schema->resultset('HoldFillTarget')->search({borrowernumber=>$patron_1->borrowernumber});; + is( $target_rs->next->reserve_id, $reserve_id_1, "Reserve id correctly set in hold fill target for item level hold" ); my $queue_line_2 = $queue_rs->next; is( $queue_line_2->item_level_request, 0, 'Request is correctly advertised as biblio-level' ); -- 2.24.1 (Apple Git-126)