From 91eac2afce3f4ceb14bcaaa723990cd58ce7d4f1 Mon Sep 17 00:00:00 2001 From: Arthur Suzuki Date: Wed, 26 Jun 2019 12:36:07 +0200 Subject: [PATCH] Bug 11999: QA follow-up [18.11] --- C4/Reserves.pm | 5 ++-- reserve/request.pl | 4 +-- t/db_dependent/Holds.t | 72 ++++++++++++++++++-------------------------------- 3 files changed, 29 insertions(+), 52 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index a6b8ef4201..24e49f4cf8 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -328,7 +328,7 @@ sub CanBookBeReserved{ { status => libraryNotFound }, if given branchcode is not an existing library { status => libraryNotPickupLocation }, if given branchcode is not configured to be a pickup location { status => cannotBeTransferred }, if branch transfer limit applies on given item and branchcode - { status => alreadyReserved }, if the borrower has already reserved this item. + { status => alreadyReserved }, if the borrower has already reserved this item. =cut @@ -410,8 +410,7 @@ sub CanItemBeReserved { $ruleitemtype = '*'; } - $item = Koha::Items->find( $itemnumber ); - my $holds = Koha::Holds->search( + $holds = Koha::Holds->search( { borrowernumber => $borrowernumber, biblionumber => $item->biblionumber, diff --git a/reserve/request.pl b/reserve/request.pl index 8d7c733b36..791173cd5b 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -237,8 +237,8 @@ foreach my $biblionumber (@biblionumbers) { } elsif ( grep { $canReserve->{status} eq $_ } (qw(ageRestricted alreadyReserved none_available)) ) - { - $template->param( $canReserve->{status} => 1 ); + { + $template->param( $canReserve->{status} => 1 ); $biblioloopiter{ $canReserve->{status} } = 1; } else { diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 230deb6286..6d1ed3ec0e 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -7,7 +7,7 @@ use t::lib::TestBuilder; use C4::Context; -use Test::More tests => 58; +use Test::More tests => 63; use MARC::Record; use C4::Biblio; @@ -282,27 +282,26 @@ ok( '... unless canreservefromotherbranches is ON (bug 2394)' ); -{ - # Regression test for bug 11336 # Test if ModReserve correctly recalculate the priorities - $biblio = $builder->build_sample_biblio({ itemtype => 'DUMMY' }); - ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $biblio->biblionumber); - my $reserveid1 = AddReserve($branch_1, $borrowernumbers[0], $biblio->biblionumber, '', 1); - ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $biblio->biblionumber); - my $reserveid2 = AddReserve($branch_1, $borrowernumbers[1], $biblio->biblionumber, '', 2); - ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $biblio->biblionumber); - my $reserveid3 = AddReserve($branch_1, $borrowernumbers[2], $biblio->biblionumber, '', 3); - my $hhh = Koha::Holds->search({ biblionumber => $biblio->biblionumber }); - my $hold3 = Koha::Holds->find( $reserveid3 ); - is( $hold3->priority, 3, "The 3rd hold should have a priority set to 3" ); - ModReserve({ reserve_id => $reserveid1, rank => 'del' }); - ModReserve({ reserve_id => $reserveid2, rank => 'del' }); - is( $hold3->discard_changes->priority, 1, "After ModReserve, the 3rd reserve becomes the first on the waiting list" ); -} +# Regression test for bug 11336 # Test if ModReserve correctly recalculate the priorities +$biblio = $builder->build_sample_biblio({ itemtype => 'DUMMY' }); +($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $biblio->biblionumber); +my $reserveid1 = AddReserve($branch_1, $borrowernumbers[0], $biblio->biblionumber, '', 1); +($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $biblio->biblionumber); +my $reserveid2 = AddReserve($branch_1, $borrowernumbers[1], $biblio->biblionumber, '', 2); +($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $biblio->biblionumber); +my $reserveid3 = AddReserve($branch_1, $borrowernumbers[2], $biblio->biblionumber, '', 3); +my $hhh = Koha::Holds->search({ biblionumber => $biblio->biblionumber }); +my $hold3 = Koha::Holds->find( $reserveid3 ); +is( $hold3->priority, 3, "The 3rd hold should have a priority set to 3" ); +ModReserve({ reserve_id => $reserveid1, rank => 'del' }); +ModReserve({ reserve_id => $reserveid2, rank => 'del' }); +is( $hold3->discard_changes->priority, 1, "After ModReserve, the 3rd reserve becomes the first on the waiting list" ); ok( defined( ( CheckReserves($itemnumber) )[1] ), "Hold can be trapped for damaged item with AllowHoldsOnDamagedItems enabled" ); ModItem({ damaged => 1 }, $item_bibnum, $itemnumber); -CancelReserve({reserve_id => $reserveid3}); +$hold = Koha::Holds->find( $reserveid3 ); +$hold->cancel; t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 1 ); is( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'OK', "Patron can reserve damaged item with AllowHoldsOnDamagedItems enabled" ); ok( defined( ( CheckReserves($itemnumber) )[1] ), "Hold can be trapped for damaged item with AllowHoldsOnDamagedItems enabled" ); @@ -336,45 +335,31 @@ AddReserve( my ($bibnum2, $title2, $bibitemnum2) = create_helper_biblio('CANNOT'); my ($item_bibnum2, $item_bibitemnum2, $itemnumber2) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CANNOT' } , $bibnum2); is( -<<<<<<< HEAD - CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'tooManyReserves', -======= - CanItemBeReserved( $borrowernumbers[0], $itemnumber2), 'tooManyReserves', ->>>>>>> Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved + CanItemBeReserved( $borrowernumbers[0], $itemnumber2)->{status}, 'tooManyReserves', "cannot request item if policy that matches on item-level item type forbids it" ); ModItem({ itype => 'CAN' }, $item_bibnum2, $itemnumber2); ok( -<<<<<<< HEAD - CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'OK', -======= - CanItemBeReserved( $borrowernumbers[0], $itemnumber2) eq 'OK', ->>>>>>> Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved + CanItemBeReserved( $borrowernumbers[0], $itemnumber2)->{status} eq 'OK', "can request item if policy that matches on item type allows it" ); t::lib::Mocks::mock_preference('item-level_itypes', 0); -<<<<<<< HEAD -ModItem({ itype => undef }, $item_bibnum, $itemnumber); -ok( - CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'tooManyReserves', -======= -ModItem({ itype => undef }, $item_bibnum2, $itemnumber2); +ModItem({ itype => undef }, $item_bibnum, $itemnumber2); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber2), 'tooManyReserves', ->>>>>>> Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved + CanItemBeReserved( $borrowernumbers[0], $itemnumber2)->{status} eq 'tooManyReserves', "cannot request item if policy that matches on bib-level item type forbids it (bug 9532)" ); -is(CanBookBeReserved($borrowernumbers[0], $bibnum), 'OK'); +is(CanBookBeReserved($borrowernumbers[0], $biblio->biblionumber)->{status}, 'OK'); C4::Context->set_preference('maxreserves', 1); -is(CanBookBeReserved($borrowernumbers[0], $biblionumber), 'tooManyReserves'); +is(CanBookBeReserved($borrowernumbers[0], $biblio->biblionumber)->{status}, 'tooManyReserves'); C4::Context->set_preference('maxreserves', 0); t::lib::Mocks::mock_preference('IndependentBranches', 1); t::lib::Mocks::mock_preference('canreservefromotherbranches', 0); -ok(CanBookBeReserved($borrowernumbers[0], $foreign_bibnum) eq 'none_available'); +ok(CanBookBeReserved($borrowernumbers[0], $foreign_biblio->biblionumber)->{status} eq 'none_available'); # Test branch item rules @@ -438,13 +423,8 @@ is( CanItemBeReserved( $borrowernumbers[0], $itemnumber )->{status}, my $res_id = AddReserve( $branch_1, $borrowernumbers[0], $biblio->biblionumber, '', 1, ); -<<<<<<< HEAD is( CanItemBeReserved( $borrowernumbers[0], $itemnumber )->{status}, - 'tooManyReserves', 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' ); -======= -is( CanItemBeReserved( $borrowernumbers[0], $itemnumber ), 'tooManyHoldsForThisRecord', 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' ); ->>>>>>> Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved #results should be the same for both ReservesControlBranch settings t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'ItemHomeLibrary' ); @@ -453,7 +433,6 @@ is( CanItemBeReserved( $borrowernumbers[0], $itemnumber )->{status}, #reset for further tests t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' ); -<<<<<<< HEAD subtest 'Test max_holds per library/patron category' => sub { plan tests => 6; @@ -704,7 +683,7 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { $schema->storage->txn_rollback; }; -======= + # Helper method to set up a Biblio. sub create_helper_biblio { my $itemtype = shift; @@ -726,4 +705,3 @@ sub create_helper_biblio { return AddBiblio($bib, ''); } ->>>>>>> Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved -- 2.11.0