@@ -, +, @@ CanItemBeReserved - Check if borrower already has reserved the same biblio (or item). - Check if borrower has reached the maximum number of holds allowed (syspref maxreserves) ---------- 1 hold for a borrower ILS-DI and note that you can than 1 hold for a borrower ILS-DI. You shouldn't be able to do that. identical than before the patch. --- C4/Reserves.pm | 45 +++++++++++++++- .../prog/en/modules/reserve/request.tt | 2 +- reserve/request.pl | 6 ++- t/db_dependent/Holds.t | 61 +++++++++++++++++++++- 4 files changed, 108 insertions(+), 6 deletions(-) --- a/C4/Reserves.pm +++ a/C4/Reserves.pm @@ -274,7 +274,23 @@ See CanItemBeReserved() for possible return values. sub CanBookBeReserved{ my ($borrowernumber, $biblionumber, $pickup_branchcode) = @_; + # Check if borrower already has reserved the same biblio. + my $patron = Koha::Patrons->find($borrowernumber); + my $holds = $patron->holds; + while (my $hold = $holds->next) { + if ($hold->biblionumber == $biblionumber) { + return { status => 'alreadyReserved' }; + } + } + + # 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' }; + } + my @itemnumbers = Koha::Items->search({ biblionumber => $biblionumber})->get_column("itemnumber"); + #get items linked via host records my @hostitems = get_hostitemnumbers_of($biblionumber); if (@hostitems){ @@ -286,7 +302,8 @@ sub CanBookBeReserved{ $canReserve = CanItemBeReserved( $borrowernumber, $itemnumber, $pickup_branchcode ); return { status => 'OK' } if $canReserve->{status} eq 'OK'; } - return $canReserve; + + return { status => 'none_available' }; } =head2 CanItemBeReserved @@ -294,6 +311,7 @@ sub CanBookBeReserved{ $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber, $branchcode) if ($canReserve->{status} eq 'OK') { #We can reserve this Item! } +<<<<<<< HEAD @RETURNS { status => OK }, if the Item can be reserved. { status => ageRestricted }, if the Item is age restricted for this borrower. { status => damaged }, if the Item is damaged. @@ -303,6 +321,15 @@ 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 +======= +@RETURNS OK, if the Item can be reserved. + ageRestricted, if the Item is age restricted for this borrower. + damaged, if the Item is damaged. + cannotReserveFromOtherBranches, if syspref 'canreservefromotherbranches' is OK. + tooManyReserves, if the borrower has exceeded his maximum reserve amount. + notReservable, if holds on this item are not allowed + alreadyReserved, if the borrower has already reserved this item. +>>>>>>> Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved =cut @@ -336,6 +363,22 @@ sub CanItemBeReserved { return { status =>'itemAlreadyOnHold' } if Koha::Holds->search( { borrowernumber => $borrowernumber, itemnumber => $itemnumber } )->count(); + # Check if borrower already has reserved the same item or biblio. + my $holds = $patron->holds; + while (my $hold = $holds->next) { + if ( $hold->itemnumber == $itemnumber + or $hold->biblionumber == $item->{biblionumber} ) + { + return 'alreadyReserved'; + } + } + + # Check if borrower has reached the maximum number of holds allowed + my $maxreserves = C4::Context->preference('maxreserves'); + if ($maxreserves && $holds->count >= $maxreserves) { + return 'tooManyReserves'; + } + my $controlbranch = C4::Context->preference('ReservesControlBranch'); my $querycount = q{ --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -90,7 +90,7 @@ [% ELSIF NOT noitems %] -[% IF ( exceeded_maxreserves || exceeded_holds_per_record || alreadyreserved || none_available || alreadypossession || ageRestricted ) %] +[% IF ( exceeded_maxreserves || exceeded_holds_per_record || alreadyReserved || none_available || alreadypossession || ageRestricted ) %]
[% UNLESS ( multi_hold ) %] --- a/reserve/request.pl +++ a/reserve/request.pl @@ -235,8 +235,10 @@ foreach my $biblionumber (@biblionumbers) { $exceeded_holds_per_record = 1; $biblioloopiter{ $canReserve->{status} } = 1; } - elsif ( $canReserve->{status} eq 'ageRestricted' ) { - $template->param( $canReserve->{status} => 1 ); + elsif ( grep { $canReserve->{status} eq $_ } + (qw(ageRestricted alreadyReserved none_available)) ) + { + $template->param( $canReserve->{status} => 1 ); $biblioloopiter{ $canReserve->{status} } = 1; } else { --- a/t/db_dependent/Holds.t +++ a/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 => 61; use MARC::Record; use C4::Biblio; @@ -299,7 +299,10 @@ ok( 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}); 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" ); @@ -330,23 +333,48 @@ AddReserve( '', 1, ); +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 "cannot request item if policy that matches on item-level item type forbids it" ); -ModItem({ itype => 'CAN' }, $item_bibnum, $itemnumber); +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 "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); +is( + CanItemBeReserved( $borrowernumbers[0], $itemnumber2), 'tooManyReserves', +>>>>>>> Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved "cannot request item if policy that matches on bib-level item type forbids it (bug 9532)" ); +is(CanBookBeReserved($borrowernumbers[0], $bibnum), 'OK'); + +C4::Context->set_preference('maxreserves', 1); +ok(CanBookBeReserved($borrowernumbers[0], $biblionumber) eq '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'); # Test branch item rules @@ -410,8 +438,13 @@ 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' ); @@ -420,6 +453,7 @@ 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; @@ -670,3 +704,26 @@ 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; + my $bib = MARC::Record->new(); + my $title = 'Silence in the library'; + if (C4::Context->preference('marcflavour') eq 'UNIMARC') { + $bib->append_fields( + MARC::Field->new('700', ' ', '0', a => 'Moffat, Steven'), + MARC::Field->new('200', ' ', ' ', a => $title), + MARC::Field->new('099', ' ', ' ', t => $itemtype), + ); + } else { + $bib->append_fields( + MARC::Field->new('100', ' ', ' ', a => 'Moffat, Steven'), + MARC::Field->new('245', ' ', ' ', a => $title), + MARC::Field->new('942', ' ', ' ', c => $itemtype), + ); + } + + return AddBiblio($bib, ''); +} +>>>>>>> Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved --