Attachment 109644 Details for Bug 18958 – Bug 18958: (follow-up) Ensure hold fill target reserve_id is set for all hold types

[patch] Bug 18958: (follow-up) Ensure hold fill target reserve_id is set for all hold types

Bug-18958-follow-up-Ensure-hold-fill-target-reserv.patch (text/plain), 4.46 KB, created by Marcel de Rooy on 2020-09-04 07:10:38 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2020-09-04 07:10:38 UTC

Size: 4.46 KB

patch

obsolete

>From 1b31e1511966a438ab1abc343b79fab400e5a499 Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>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
>Content-Type: text/plain; charset=utf-8
>
>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 <m.de.rooy@rijksmuseum.nl>
>---
> C4/HoldsQueue.pm            |  2 ++
> t/db_dependent/HoldsQueue.t | 10 ++++++++--
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
>diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm
>index 08f8a4eb94..b891afc437 100755
>--- 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..089630329c 100755
>--- a/t/db_dependent/HoldsQueue.t
>+++ b/t/db_dependent/HoldsQueue.t
>@@ -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.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 18958: 65104 | 65150 | 65184 | 71164 | 71232 | 72210 | 72211 | 72348 | 72349 | 92159 | 92160 | 93106 | 93107 | 93108 | 93111 | 93112 | 93113 | 93216 | 93217 | 93218 | 107576 | 107577 | 107582 | 107584 | 107585 | 107586 | 108256 | 108274 | 108473 | 108474 | 108497 | 108499 | 108500 | 108936 | 108937 | 109061 | 109222 | 109640 | 109641 | 109642 | 109643 | 109644 | 109645