Bugzilla – Attachment 91494 Details for
Bug 11999
Add two checks in CanBookBeReserved and CanItemBeReserved
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 11999: QA follow-up
Bug-11999-QA-follow-up.patch (text/plain), 10.27 KB, created by
Arthur Suzuki
on 2019-07-12 14:42:40 UTC
(
hide
)
Description:
Bug 11999: QA follow-up
Filename:
MIME Type:
Creator:
Arthur Suzuki
Created:
2019-07-12 14:42:40 UTC
Size:
10.27 KB
patch
obsolete
>From 642bbe19a6ada82a6ec566ffc38d9b6aad33af76 Mon Sep 17 00:00:00 2001 >From: Arthur Suzuki <arthur.suzuki@biblibre.com> >Date: Wed, 26 Jun 2019 12:36:07 +0200 >Subject: [PATCH] Bug 11999: QA follow-up > >--- > C4/Reserves.pm | 6 ++-- > reserve/request.pl | 4 +-- > t/db_dependent/Holds.t | 77 ++++++++++++++++++-------------------------------- > 3 files changed, 33 insertions(+), 54 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 1d680b068b..dddda1b5a3 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -320,7 +320,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 > >@@ -365,7 +365,7 @@ sub CanItemBeReserved { > # Check if borrower has reached the maximum number of holds allowed > my $maxreserves = C4::Context->preference('maxreserves'); > if ($maxreserves && $holds->count >= $maxreserves) { >- return { status => 'tooManyReserves' }; >+ return { status => 'tooManyReserves', limit => $maxreserves }; > } > > my $controlbranch = C4::Context->preference('ReservesControlBranch'); >@@ -402,7 +402,7 @@ sub CanItemBeReserved { > $ruleitemtype = '*'; > } > >- 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 3d6a1e4706..5f775b3e73 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -242,8 +242,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 97e78054b8..c565cf52a2 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 => 59; >+use Test::More tests => 62; > use MARC::Record; > > use C4::Biblio; >@@ -282,30 +282,28 @@ 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" ); > > $hold = Koha::Hold->new( > { >@@ -336,45 +334,33 @@ 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}, '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'); >+t::lib::Mocks::mock_preference('IndependentBranches', 0); >+t::lib::Mocks::mock_preference('canreservefromotherbranches', 1); > > # Test branch item rules > >@@ -443,22 +429,16 @@ 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' ); > is( CanItemBeReserved( $borrowernumbers[0], $itemnumber )->{status}, >- 'tooManyReserves', 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' ); >+ 'tooManyHoldsForThisRecord', 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' ); > #reset for further tests > t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' ); > >-<<<<<<< HEAD > subtest 'Test max_holds per library/patron category' => sub { > plan tests => 6; > >@@ -709,7 +689,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; >@@ -731,4 +711,3 @@ sub create_helper_biblio { > > return AddBiblio($bib, ''); > } >->>>>>>> Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved >-- >2.11.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 11999
:
26589
|
28986
|
31524
|
35697
|
36358
|
36404
|
36630
|
38397
|
41951
|
41952
|
44631
|
44632
|
46262
|
46263
|
50224
|
50225
|
58741
|
58742
|
58743
|
63593
|
63594
|
63595
|
66267
|
66268
|
66269
|
66270
|
88300
|
88301
|
88302
|
88303
|
90142
|
90143
|
90144
|
90146
|
90147
|
91046
|
91402
|
91403
|
91479
|
91481
|
91494
|
91580
|
91581
|
91582
|
91590
|
91591
|
91592
|
91593
|
99927
|
99928
|
106481
|
106494
|
111687
|
111688
|
111689
|
112871
|
112872
|
112873
|
112874
|
120776
|
120777
|
120778
|
120779
|
120780
|
121531
|
121612