From 12587f5b0144603ded8b2f9965138094f6a92f5f Mon Sep 17 00:00:00 2001 From: Arthur Suzuki Date: Thu, 21 Mar 2019 21:52:06 +0100 Subject: [PATCH] Bug 20985: Add OnShelfHoldsAllowed checks to CanItemBeReserved Test plan : 1 / Check that default circulation have OnShelfHoldsAllowed to Yes 2 / Place a hold on a book with a single item, item being available for loan, verify that hold can be placed. 3 / Set OnShelfHoldsAllowed to any other value, verify that hold cannot be placed for reason "onShelfHoldsNotAllowed" (shown in the item table). --- C4/Reserves.pm | 31 ++++++++++++- t/db_dependent/Reserves.t | 92 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 1 deletion(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 8d7dcbc611..0ceb756c92 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -343,6 +343,7 @@ sub CanBookBeReserved{ current checkout against the high holds threshold @RETURNS { status => OK }, if the Item can be reserved. + { status => onShelfHoldsNotAllowed }, if onShelfHoldsAllowed parameter and item availability combination doesn't allow holds. { status => ageRestricted }, if the Item is age restricted for this borrower. { status => damaged }, if the Item is damaged. { status => cannotReserveFromOtherBranches }, if syspref 'canreservefromotherbranches' is OK. @@ -365,8 +366,15 @@ sub CanItemBeReserved { my $holds_per_record = 1; # Total number of holds allowed for this one given record my $holds_per_day; # Default to unlimited + my $opacitemholds = 'Y'; # Itemlevel holds default to allowed + my $on_shelf_holds = 0; # Default to "if any unavailable" + + # Get calling context + my $caller = (caller(1))[3]; + # we retrieve borrowers and items informations # # item->{itype} will come for biblioitems if necessery + my $item = Koha::Items->find($itemnumber); my $biblio = $item->biblio; my $patron = Koha::Patrons->find( $borrowernumber ); @@ -415,6 +423,8 @@ sub CanItemBeReserved { $allowedreserves = $rights->{reservesallowed} // $allowedreserves; $holds_per_record = $rights->{holds_per_record} // $holds_per_record; $holds_per_day = $rights->{holds_per_day}; + $opacitemholds = $rights->{opacitemholds}; + $on_shelf_holds = $rights->{onshelfholds}; } else { $ruleitemtype = undef; @@ -424,8 +434,25 @@ sub CanItemBeReserved { borrowernumber => $borrowernumber, biblionumber => $item->biblionumber, }; + $search_params->{found} = undef if $params->{ignore_found_holds}; + if ( $opacitemholds eq "N" and index($caller,"CanBookBeReserved") eq -1 ) { + return { status => "notReservable" }; + } + + # Check for item on shelves and OnShelfHoldsAllowed + return { status => 'onShelfHoldsNotAllowed' } + if ( 0 == ( + $item->{'notforloan'} or + $item->{'onloan'} or + $item->{'itemlost'} or + $item->{'withdrawn'} or + $item->{'damaged'} ) + && ($on_shelf_holds != "1")); + + $item = Koha::Items->find( $itemnumber ); + my $holds = Koha::Holds->search($search_params); if ( defined $holds_per_record && $holds_per_record ne '' && $holds->count() >= $holds_per_record ) { @@ -2264,7 +2291,7 @@ sub GetHoldRule { itemtype => $itemtype, categorycode => $categorycode, branchcode => $branchcode, - rules => ['holds_per_record', 'holds_per_day'], + rules => ['holds_per_record', 'holds_per_day', 'opacitemholds', 'onshelfholds'], order_by => { -desc => [ 'categorycode', 'itemtype', 'branchcode' ] } @@ -2272,6 +2299,8 @@ sub GetHoldRule { ); $rules->{holds_per_record} = $holds_per_x_rules->{holds_per_record}; $rules->{holds_per_day} = $holds_per_x_rules->{holds_per_day}; + $rules->{opacitemholds} = $holds_per_x_rules->{opacitemholds}; + $rules->{onshelfholds} = $holds_per_x_rules->{onshelfholds}; return $rules; } diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 26f1f4863e..5fe19b265f 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -187,6 +187,7 @@ Koha::CirculationRules->set_rules( rules => { reservesallowed => 25, holds_per_record => 1, + onshelfholds => 1, } } ); @@ -986,6 +987,97 @@ subtest 'reserves.item_level_hold' => sub { $hold->delete; }; + subtest 'test opacitemholds rules' => sub { + plan tests => 6; + + Koha::CirculationRules->set_rules( + { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rules => { + reservesallowed => 25, + opacitemholds => 'F', + onshelfholds => 1, + } + } + ); + + my $canreserve = C4::Reserves::CanBookBeReserved( + $patron->borrowernumber, + $item->biblionumber, + ); + + is( $canreserve->{status}, + 'RecordHoldNotAllowed', + 'record-level holds should not be possible with opacitemholds set to "Force"' ); + + $canreserve = C4::Reserves::CanItemBeReserved( + $patron->borrowernumber, + $item->itemnumber, + ); + + is( $canreserve->{status}, 'OK', + 'item-level holds should be possible with opacitemholds set to "Force"' ); + + Koha::CirculationRules->set_rules( + { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rules => { + reservesallowed => 25, + opacitemholds => 'N', + onshelfholds => 1, + } + } + ); + + $canreserve = C4::Reserves::CanBookBeReserved( + $patron->borrowernumber, + $item->biblionumber, + ); + + is( $canreserve->{status}, 'OK', + 'record-level holds should be possible with opacitemholds set to "No"' ); + + $canreserve = C4::Reserves::CanItemBeReserved( + $patron->borrowernumber, + $item->itemnumber, + ); + + is( $canreserve->{status}, 'notReservable', + 'item-level holds should not be possible with opacitemholds set to "No"' ); + + Koha::CirculationRules->set_rules( + { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rules => { + reservesallowed => 25, + opacitemholds => 'Y', + onshelfholds => 1, + } + } + ); + + $canreserve = C4::Reserves::CanBookBeReserved( + $patron->borrowernumber, + $item->biblionumber, + ); + + is( $canreserve->{status}, 'OK', + 'record-level holds should be possible with opacitemholds set to "Yes"' ); + + $canreserve = C4::Reserves::CanItemBeReserved( + $patron->borrowernumber, + $item->itemnumber, + ); + + is( $canreserve->{status}, 'OK', + 'item-level holds should be possible with opacitemholds set to "Yes"' ); + }; }; subtest 'MoveReserve additional test' => sub { -- 2.20.1