@@ -, +, @@ 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 | 34 ++++++++++++++- .../prog/en/modules/reserve/request.tt | 4 +- reserve/request.pl | 4 +- t/db_dependent/Holds.t | 48 ++++++++++++++++------ 4 files changed, 73 insertions(+), 17 deletions(-) --- a/C4/Reserves.pm +++ a/C4/Reserves.pm @@ -325,6 +325,20 @@ See CanItemBeReserved() for possible return values. sub CanBookBeReserved{ my ($borrowernumber, $biblionumber) = @_; + # Check if borrower already has reserved the same biblio. + my @reserves = GetReservesFromBorrowernumber($borrowernumber); + foreach my $reserve (@reserves) { + if ($reserve->{biblionumber} == $biblionumber) { + return 'alreadyReserved'; + } + } + + # Check if borrower has reached the maximum number of holds allowed + my $maxreserves = C4::Context->preference('maxreserves'); + if ($maxreserves && scalar(@reserves) >= $maxreserves) { + return 'tooManyReserves'; + } + my $items = GetItemnumbersForBiblio($biblionumber); #get items linked via host records my @hostitems = get_hostitemnumbers_of($biblionumber); @@ -337,7 +351,8 @@ sub CanBookBeReserved{ $canReserve = CanItemBeReserved( $borrowernumber, $item ); return 'OK' if $canReserve eq 'OK'; } - return $canReserve; + + return 'none_available'; } =head2 CanItemBeReserved @@ -351,6 +366,7 @@ sub CanBookBeReserved{ 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. =cut @@ -382,6 +398,22 @@ sub CanItemBeReserved { return 'itemAlreadyOnHold' if Koha::Holds->search( { borrowernumber => $borrowernumber, itemnumber => $itemnumber } )->count(); + # Check if borrower already has reserved the same item or biblio. + my @reserves = GetReservesFromBorrowernumber($borrowernumber); + foreach my $reserve (@reserves) { + if ( $reserve->{itemnumber} == $itemnumber + or $reserve->{biblionumber} == $item->{biblionumber} ) + { + return 'alreadyReserved'; + } + } + + # Check if borrower has reached the maximum number of holds allowed + my $maxreserves = C4::Context->preference('maxreserves'); + if ($maxreserves && scalar(@reserves) >= $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 @@ -270,7 +270,7 @@ function checkMultiHold() { [% 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 ) %] @@ -282,7 +282,7 @@ function checkMultiHold() {
  • Too many holds for this record: [% borrowerfirstname %] [% borrowersurname %] can only place a maximum of [% max_holds_for_record %] hold(s) on this record.
  • [% ELSIF ( alreadypossession ) %]
  • [% borrowerfirstname %] [% borrowersurname %] is already in possession of one item.
  • - [% ELSIF ( alreadyreserved ) %] + [% ELSIF ( alreadyReserved ) %]
  • [% borrowerfirstname %] [% borrowersurname %] already has a hold on this item.
  • [% ELSIF ( ageRestricted ) %]
  • Age restricted
  • --- a/reserve/request.pl +++ a/reserve/request.pl @@ -228,7 +228,9 @@ foreach my $biblionumber (@biblionumbers) { $exceeded_holds_per_record = 1; $biblioloopiter{$canReserve} = 1; } - elsif ( $canReserve eq 'ageRestricted' ) { + elsif ( grep { $canReserve eq $_ } + (qw(ageRestricted alreadyReserved none_available)) ) + { $template->param( $canReserve => 1 ); $biblioloopiter{$canReserve} = 1; } --- 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; use C4::Items; @@ -345,10 +345,12 @@ ModReserve({ reserve_id => $reserveid2, rank => 'del' }); $reserve3 = GetReserve( $reserveid3 ); is( $reserve3->{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), '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( { @@ -376,23 +378,34 @@ 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( - CanItemBeReserved( $borrowernumbers[0], $itemnumber), 'tooManyReserves', + CanItemBeReserved( $borrowernumbers[0], $itemnumber2), 'tooManyReserves', "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( - CanItemBeReserved( $borrowernumbers[0], $itemnumber) eq 'OK', + CanItemBeReserved( $borrowernumbers[0], $itemnumber2) eq 'OK', "can request item if policy that matches on item type allows it" ); t::lib::Mocks::mock_preference('item-level_itypes', 0); -ModItem({ itype => undef }, $item_bibnum, $itemnumber); +ModItem({ itype => undef }, $item_bibnum2, $itemnumber2); ok( - CanItemBeReserved( $borrowernumbers[0], $itemnumber) eq 'tooManyReserves', + CanItemBeReserved( $borrowernumbers[0], $itemnumber2) eq 'tooManyReserves', "cannot request item if policy that matches on bib-level item type forbids it (bug 9532)" ); +is(CanBookBeReserved($borrowernumbers[0], $bibnum), 'alreadyReserved'); + +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 @@ -465,10 +478,19 @@ sub create_helper_biblio { my $itemtype = shift; my $bib = MARC::Record->new(); my $title = 'Silence in the library'; - $bib->append_fields( - MARC::Field->new('100', ' ', ' ', a => 'Moffat, Steven'), - MARC::Field->new('245', ' ', ' ', a => $title), - MARC::Field->new('942', ' ', ' ', c => $itemtype), - ); - return ($bibnum, $title, $bibitemnum) = AddBiblio($bib, ''); + 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, ''); } --