From 12b90bb02f50af547c8214e2e89620ef2ea50385 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 10 Jun 2022 14:47:44 +0000 Subject: [PATCH] Bug 12630: Rebase tests and cover CheckReserves It turns out we do honor reservedate in CheckReserves, so a hold with a lower priority will fill before a hold in the future. I add tests to cover this and fix the old tests to pass again Signed-off-by: Tomas Cohen Arazi (cherry picked from commit a1739ea43bfb7110059446989483f3282784527c) Signed-off-by: Lucas Gass --- C4/Reserves.pm | 2 +- t/db_dependent/Reserves.t | 57 +++++++++++++++++++++++++++++++-------- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index f05e43bdd1..9a1a7fc758 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -225,7 +225,7 @@ sub AddReserve { } } if ( C4::Context->preference( 'AllowHoldDateInFuture' ) ) { - # Make room in reserves for this before those of a later reserve date + # Make room in reserves for this if passed a priority $priority = _ShiftPriority( $biblionumber, $priority ); } diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index a001bc8d39..91ebbf3f3b 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 75; +use Test::More tests => 77; use Test::MockModule; use Test::Warn; @@ -387,8 +387,29 @@ t::lib::Mocks::mock_preference('ConfirmFutureHolds', 7); ($doreturn, $messages)= AddReturn('97531',$branch_1); is(exists $messages->{ResFound}?1:0, 1, 'AddReturn considers future reserve within ConfirmFutureHolds days'); +my $now_holder = $builder->build_object({ class => 'Koha::Patrons', value => { + branchcode => $branch_1, +}}); +my $now_reserve_id = AddReserve( + { + branchcode => $branch_1, + borrowernumber => $requesters{$branch_1}, + biblionumber => $bibnum, + priority => 2, + reservation_date => output_pref(dt_from_string()), + } +); +my $which_highest; +($status,$which_highest)=CheckReserves($item->itemnumber,undef,3); +is( $which_highest->{reserve_id}, $now_reserve_id, 'CheckReserves returns lower priority current reserve with insufficient lookahead'); +($status, $which_highest)=CheckReserves($item->itemnumber,undef,4); +is( $which_highest->{reserve_id}, $reserve_id, 'CheckReserves returns higher priority future reserve with sufficient lookahead'); +ModReserve({ reserve_id => $now_reserve_id, rank => 'del', cancellation_reason => 'test reserve' }); + + # End of tests for bug 9761 (ConfirmFutureHolds) + # test marking a hold as captured my $hold_notice_count = count_hold_print_messages(); ModReserveAffect($item->itemnumber, $requesters{$branch_1}, 0); @@ -521,26 +542,40 @@ is($p, 3, 'CalculatePriority should now return priority 3'); # regression test for bug 12630 # Now there are 2 reserves on $bibnum t::lib::Mocks::mock_preference('AllowHoldDateInFuture', 1); -my $borrowernumber_tmp_1 = AddMember( +my $bor_tmp_1 = $builder->build_object({ class => 'Koha::Patrons',value =>{ firstname => 'my firstname tmp 1', surname => 'my surname tmp 1', categorycode => 'S', branchcode => 'CPL', -); -my $borrowernumber_tmp_2 = AddMember( +}}); +my $bor_tmp_2 = $builder->build_object({ class => 'Koha::Patrons',value =>{ firstname => 'my firstname tmp 2', surname => 'my surname tmp 2', categorycode => 'S', branchcode => 'CPL', -); +}}); +my $borrowernumber_tmp_1 = $bor_tmp_1->borrowernumber; +my $borrowernumber_tmp_2 = $bor_tmp_2->borrowernumber; my $date_in_future = dt_from_string(); $date_in_future = $date_in_future->add_duration(DateTime::Duration->new(days => 1)); -AddReserve('CPL', $borrowernumber_tmp_1, $bibnum, undef, undef, 3, output_pref($date_in_future)); -AddReserve('CPL', $borrowernumber_tmp_2, $bibnum, undef, undef, 4); -my @r1 = GetReservesFromBorrowernumber( $borrowernumber_tmp_1 ); -my @r2 = GetReservesFromBorrowernumber( $borrowernumber_tmp_2 ); -is( $r1[0]->{priority}, 3, 'priority for hold in future should be correct'); -is( $r2[0]->{priority}, 4, 'priority for hold not in future should be correct'); +AddReserve({ + branch => 'CPL', + borrowernumber => $borrowernumber_tmp_1, + biblionumber => $bibnum, + priority => 3, + reservation_date => output_pref($date_in_future) +}); +AddReserve({ + branch => 'CPL', + borrowernumber => $borrowernumber_tmp_2, + biblionumber => $bibnum, + priority => 4, + reservation_date => output_pref($date_in_future) +}); +my @r1 = Koha::Holds->search({ borrowernumber => $borrowernumber_tmp_1 })->as_list; +my @r2 = Koha::Holds->search({ borrowernumber => $borrowernumber_tmp_2 })->as_list; +is( $r1[0]->priority, 3, 'priority for hold in future should be correct'); +is( $r2[0]->priority, 4, 'priority for hold not in future should be correct'); # end of tests for bug 12630 # Tests for cancel reserves by users from OPAC. -- 2.39.5