From 842d4482766c0fa9f1153fe2f91fc5e1860b95ab Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Tue, 23 Nov 2021 19:56:52 +0000 Subject: [PATCH] Bug 29562: Adjust CanItemBeReserved and checkHighHolds to take objects Most of the changes here are simple, this can be read to view the changes Testing that holds can be placed via staff client, and opac, and are disallowed when expected is the best test plan, beyond running the unit tests Signed-off-by: Tomas Cohen Arazi Signed-off-by: Jonathan Druart Signed-off-by: Fridolin Somers --- C4/Circulation.pm | 23 +++-- C4/ILSDI/Services.pm | 2 +- C4/Reserves.pm | 48 +++++---- Koha/Club/Hold.pm | 12 ++- Koha/REST/V1/Holds.pm | 2 +- opac/opac-reserve.pl | 12 +-- reserve/placerequest.pl | 17 ++-- reserve/request.pl | 2 +- t/db_dependent/Holds.t | 201 +++++++++++++++++++------------------- t/db_dependent/Reserves.t | 6 +- 10 files changed, 165 insertions(+), 160 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 69bf445aea..d59fbf6380 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1166,7 +1166,7 @@ sub CanBookBeIssued { ## check for high holds decreasing loan period if ( C4::Context->preference('decreaseLoanHighHolds') ) { - my $check = checkHighHolds( $item_unblessed, $patron_unblessed ); + my $check = checkHighHolds( $item_object, $patron ); if ( $check->{exceeded} ) { if ($override_high_holds) { @@ -1278,9 +1278,8 @@ sub CanBookBeReturned { =cut sub checkHighHolds { - my ( $item, $borrower ) = @_; - my $branchcode = _GetCircControlBranch( $item, $borrower ); - my $item_object = Koha::Items->find( $item->{itemnumber} ); + my ( $item, $patron ) = @_; + my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ); my $return_data = { exceeded => 0, @@ -1289,7 +1288,7 @@ sub checkHighHolds { due_date => undef, }; - my $holds = Koha::Holds->search( { biblionumber => $item->{'biblionumber'} } ); + my $holds = Koha::Holds->search( { biblionumber => $item->biblionumber } ); if ( $holds->count() ) { $return_data->{outstanding} = $holds->count(); @@ -1322,7 +1321,7 @@ sub checkHighHolds { } # Remove any items that are not holdable for this patron - @items = grep { CanItemBeReserved( $borrower->{borrowernumber}, $_->itemnumber, undef, { ignore_found_holds => 1 } )->{status} eq 'OK' } @items; + @items = grep { CanItemBeReserved( $patron , $_, undef, { ignore_found_holds => 1 } )->{status} eq 'OK' } @items; my $items_count = scalar @items; @@ -1337,22 +1336,22 @@ sub checkHighHolds { my $issuedate = dt_from_string(); - my $itype = $item_object->effective_itemtype; + my $itype = $item->effective_itemtype; my $daysmode = Koha::CirculationRules->get_effective_daysmode( { - categorycode => $borrower->{categorycode}, + categorycode => $patron->categorycode, itemtype => $itype, branchcode => $branchcode, } ); my $calendar = Koha::Calendar->new( branchcode => $branchcode, days_mode => $daysmode ); - my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branchcode, $borrower ); + my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branchcode, $patron->unblessed ); my $rule = Koha::CirculationRules->get_effective_rule( { - categorycode => $borrower->{categorycode}, - itemtype => $item_object->effective_itemtype, + categorycode => $patron->categorycode, + itemtype => $item->effective_itemtype, branchcode => $branchcode, rule_name => 'decreaseloanholds', } @@ -2849,7 +2848,7 @@ sub CanBookBeRenewed { next if IsItemOnHoldAndFound( $item->itemnumber ); while ( my $patron = $patrons->next ) { next unless IsAvailableForItemLevelRequest($item, $patron); - next unless CanItemBeReserved($patron->borrowernumber,$item->itemnumber,undef,{ignore_hold_counts=>1})->{status} eq 'OK'; + next unless CanItemBeReserved($patron,$item,undef,{ignore_hold_counts=>1})->{status} eq 'OK'; push @reservable, $item->itemnumber; if (@reservable >= @borrowernumbers) { $resfound = 0; diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index c7714dcd52..9c2fc9494a 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -874,7 +874,7 @@ sub HoldItem { } # Check for item disponibility - my $canitembereserved = C4::Reserves::CanItemBeReserved( $borrowernumber, $itemnumber, $branch )->{status}; + my $canitembereserved = C4::Reserves::CanItemBeReserved( $patron, $item, $branch )->{status}; return { code => $canitembereserved } unless $canitembereserved eq 'OK'; my $resdate; diff --git a/C4/Reserves.pm b/C4/Reserves.pm index b95f0a61b8..11a6088f1c 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -330,16 +330,24 @@ sub CanBookBeReserved{ return { status =>'alreadypossession' }; } - my @itemnumbers = Koha::Items->search({ biblionumber => $biblionumber})->get_column("itemnumber"); + my $items; #get items linked via host records - my @hostitems = get_hostitemnumbers_of($biblionumber); - if (@hostitems){ - push (@itemnumbers, @hostitems); + my @hostitemnumbers = get_hostitemnumbers_of($biblionumber); + if (@hostitemnumbers){ + $items = Koha::Items->search({ + -or => [ + biblionumber => $biblionumber, + itemnumber => { -in => @hostitemnumbers } + ] + }); + } else { + $items = Koha::Items->search({ biblionumber => $biblionumber}); } my $canReserve = { status => '' }; - foreach my $itemnumber (@itemnumbers) { - $canReserve = CanItemBeReserved( $borrowernumber, $itemnumber, $pickup_branchcode, $params ); + my $patron = Koha::Patrons->find( $borrowernumber ); + while ( my $item = $items->next ) { + $canReserve = CanItemBeReserved( $patron, $item, $pickup_branchcode, $params ); return { status => 'OK' } if $canReserve->{status} eq 'OK'; } return $canReserve; @@ -347,7 +355,7 @@ sub CanBookBeReserved{ =head2 CanItemBeReserved - $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber, $branchcode, $params) + $canReserve = &CanItemBeReserved($patron, $item, $branchcode, $params) if ($canReserve->{status} eq 'OK') { #We can reserve this Item! } current params are: @@ -372,7 +380,7 @@ sub CanBookBeReserved{ =cut sub CanItemBeReserved { - my ( $borrowernumber, $itemnumber, $pickup_branchcode, $params ) = @_; + my ( $patron, $item, $pickup_branchcode, $params ) = @_; my $dbh = C4::Context->dbh; my $ruleitemtype; # itemtype of the matching issuing rule @@ -380,9 +388,7 @@ sub CanItemBeReserved { # 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 ); my $borrower = $patron->unblessed; # If an item is damaged and we don't allow holds on damaged items, we can stop right here @@ -397,7 +403,7 @@ sub CanItemBeReserved { # Check that the patron doesn't have an item level hold on this item already return { status =>'itemAlreadyOnHold' } - if ( !$params->{ignore_hold_counts} && Koha::Holds->search( { borrowernumber => $borrowernumber, itemnumber => $itemnumber } )->count() ); + if ( !$params->{ignore_hold_counts} && Koha::Holds->search( { borrowernumber => $patron->borrowernumber, itemnumber => $item->itemnumber } )->count() ); # Check that patron have not checked out this biblio (if AllowHoldsOnPatronsPossessions set) if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions') @@ -454,7 +460,7 @@ sub CanItemBeReserved { my $holds_per_day = $rights->{holds_per_day}; my $search_params = { - borrowernumber => $borrowernumber, + borrowernumber => $patron->borrowernumber, biblionumber => $item->biblionumber, }; $search_params->{found} = undef if $params->{ignore_found_holds}; @@ -470,7 +476,7 @@ sub CanItemBeReserved { } my $today_holds = Koha::Holds->search({ - borrowernumber => $borrowernumber, + borrowernumber => $patron->borrowernumber, reservedate => dt_from_string->date }); @@ -495,10 +501,10 @@ sub CanItemBeReserved { my $sthcount = $dbh->prepare($querycount); if ( defined $ruleitemtype ) { - $sthcount->execute( $borrowernumber, $branchcode, $ruleitemtype ); + $sthcount->execute( $patron->borrowernumber, $branchcode, $ruleitemtype ); } else { - $sthcount->execute( $borrowernumber, $branchcode ); + $sthcount->execute( $patron->borrowernumber, $branchcode ); } my $reservecount = "0"; @@ -519,7 +525,7 @@ sub CanItemBeReserved { # Now we need to check hold limits by patron category my $rule = Koha::CirculationRules->get_effective_rule( { - categorycode => $borrower->{categorycode}, + categorycode => $patron->categorycode, branchcode => $branchcode, rule_name => 'max_holds', } @@ -527,7 +533,7 @@ sub CanItemBeReserved { if (!$params->{ignore_hold_counts} && $rule && defined( $rule->rule_value ) && $rule->rule_value ne '' ) { my $total_holds_count = Koha::Holds->search( { - borrowernumber => $borrower->{borrowernumber} + borrowernumber => $patron->borrowernumber } )->count(); @@ -535,7 +541,7 @@ sub CanItemBeReserved { } my $reserves_control_branch = - GetReservesControlBranch( $item->unblessed(), $borrower ); + GetReservesControlBranch( $item->unblessed(), $patron->unblessed ); my $branchitemrule = C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->effective_itemtype ); @@ -551,7 +557,7 @@ sub CanItemBeReserved { my $item_library = Koha::Libraries->find( {branchcode => $item->homebranch} ); if ( $branchitemrule->{holdallowed} eq 'from_local_hold_group') { - if($borrower->{branchcode} ne $item->homebranch && !$item_library->validate_hold_sibling( {branchcode => $borrower->{branchcode}} )) { + if($patron->branchcode ne $item->homebranch && !$item_library->validate_hold_sibling( {branchcode => $patron->branchcode} )) { return { status => 'branchNotInHoldGroup' }; } } @@ -561,7 +567,7 @@ sub CanItemBeReserved { if ( C4::Context->preference('IndependentBranches') and !C4::Context->preference('canreservefromotherbranches') ) { - if ( $item->homebranch ne $borrower->{branchcode} ) { + if ( $item->homebranch ne $patron->branchcode ) { return { status => 'cannotReserveFromOtherBranches' }; } } @@ -1429,7 +1435,7 @@ sub ItemsAnyAvailableAndNotRestricted { || Koha::ItemTypes->find( $i->effective_itemtype() )->notforloan || $branchitemrule->{holdallowed} eq 'from_home_library' && $param->{patron}->branchcode ne $i->homebranch || $branchitemrule->{holdallowed} eq 'from_local_hold_group' && ! $item_library->validate_hold_sibling( { branchcode => $param->{patron}->branchcode } ) - || CanItemBeReserved( $param->{patron}->borrowernumber, $i->id )->{status} ne 'OK'; + || CanItemBeReserved( $param->{patron}, $i )->{status} ne 'OK'; } return 0; diff --git a/Koha/Club/Hold.pm b/Koha/Club/Hold.pm index fbf35f4314..9c1bbc7a70 100644 --- a/Koha/Club/Hold.pm +++ b/Koha/Club/Hold.pm @@ -53,6 +53,7 @@ Class (static) method that returns a new Koha::Club::Hold instance sub add { my ( $params ) = @_; + my $itemnumber = $params->{item_id}; # check for mandatory params my @mandatory = ( 'biblio_id', 'club_id' ); @@ -87,11 +88,12 @@ sub add { my $pickup_id = $params->{pickup_library_id}; my $can_place_hold; + my $patron = Koha::Patrons->find($patron_id); + my $item = $itemnumber ? Koha::Items->find( $itemnumber ) : undef; if($params->{default_patron_home}) { - my $patron = Koha::Patrons->find($patron_id); my $patron_home = $patron->branchcode; - $can_place_hold = $params->{item_id} - ? C4::Reserves::CanItemBeReserved( $patron_id, $params->{item_id}, $patron_home ) + $can_place_hold = $itemnumber + ? C4::Reserves::CanItemBeReserved( $patron, $item, $patron_home ) : C4::Reserves::CanBookBeReserved( $patron_id, $params->{biblio_id}, $patron_home ); $pickup_id = $patron_home if $can_place_hold->{status} eq 'OK'; unless ( $can_place_hold->{status} eq 'OK' ) { @@ -100,8 +102,8 @@ sub add { } unless ( defined $can_place_hold && $can_place_hold->{status} eq 'OK' ) { - $can_place_hold = $params->{item_id} - ? C4::Reserves::CanItemBeReserved( $patron_id, $params->{item_id}, $pickup_id ) + $can_place_hold = $itemnumber + ? C4::Reserves::CanItemBeReserved( $patron, $item, $pickup_id ) : C4::Reserves::CanBookBeReserved( $patron_id, $params->{biblio_id}, $pickup_id ); } diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index 92e6e4774e..d4f6dbcf3e 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -166,7 +166,7 @@ sub add { my $can_place_hold = $item_id - ? C4::Reserves::CanItemBeReserved( $patron_id, $item_id ) + ? C4::Reserves::CanItemBeReserved( $patron, $item ) : C4::Reserves::CanBookBeReserved( $patron_id, $biblio_id ); if ( $patron->holds->count + 1 > C4::Context->preference('maxreserves') ) { diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 47f91c0d03..62c7ad79db 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -239,9 +239,9 @@ if ( $query->param('place_reserve') ) { $branch = $patron->branchcode; } + my $item = $itemNum ? Koha::Items->find( $itemNum ) : undef; # When choosing a specific item, the default pickup library should be dictated by the default hold policy if ( ! C4::Context->preference("OPACAllowUserToChooseBranch") && $itemNum ) { - my $item = Koha::Items->find( $itemNum ); my $type = $item->effective_itemtype; my $rule = GetBranchItemRule( $patron->branchcode, $type ); @@ -256,8 +256,7 @@ if ( $query->param('place_reserve') ) { } #item may belong to a host biblio, if yes change biblioNum to hosts bilbionumber - if ( $itemNum ne '' ) { - my $item = Koha::Items->find( $itemNum ); + if ( $itemNum ) { my $hostbiblioNum = $item->biblio->biblionumber; if ( $hostbiblioNum ne $biblioNum ) { $biblioNum = $hostbiblioNum; @@ -278,8 +277,9 @@ if ( $query->param('place_reserve') ) { my $patron_expiration_date = $query->param("expiration_date_$biblioNum"); my $rank = $biblioData->{rank}; - if ( $itemNum ne '' ) { - $canreserve = 1 if CanItemBeReserved( $borrowernumber, $itemNum, $branch )->{status} eq 'OK'; + my $patron = Koha::Patrons->find( $borrowernumber ); + if ( $itemNum ) { + $canreserve = 1 if CanItemBeReserved( $patron, $item, $branch )->{status} eq 'OK'; } else { $canreserve = 1 if CanBookBeReserved( $borrowernumber, $biblioNum, $branch )->{status} eq 'OK'; @@ -569,7 +569,7 @@ foreach my $biblioNum (@biblionumbers) { # items_any_available defined outside of the current loop, # so we avoiding loop inside IsAvailableForItemLevelRequest: $policy_holdallowed &&= - CanItemBeReserved( $borrowernumber, $itemNum )->{status} eq 'OK' && + CanItemBeReserved( $patron, $tem )->{status} eq 'OK' && IsAvailableForItemLevelRequest($item, $patron, undef, $items_any_available); if ($policy_holdallowed) { diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 5ee8dce2f6..9f6d82dc4f 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -49,8 +49,7 @@ my $expirationdate = $input->param('expiration_date'); my $itemtype = $input->param('itemtype') || undef; my $non_priority = $input->param('non_priority'); -my $borrower = Koha::Patrons->find( $borrowernumber ); -$borrower = $borrower->unblessed if $borrower; +my $patron = Koha::Patrons->find( $borrowernumber ); my $biblionumbers = $input->param('biblionumbers'); $biblionumbers ||= $biblionumber . '/'; @@ -70,7 +69,7 @@ foreach my $bibnum (@biblionumbers) { my $found; -if ( $type eq 'str8' && $borrower ) { +if ( $type eq 'str8' && $patron ) { foreach my $biblionumber ( keys %bibinfos ) { my $count = @bibitems; @@ -97,13 +96,13 @@ if ( $type eq 'str8' && $borrower ) { $biblionumber = $item->biblionumber; } - my $can_item_be_reserved = CanItemBeReserved($borrower->{'borrowernumber'}, $item->itemnumber, $item_pickup_location)->{status}; + my $can_item_be_reserved = CanItemBeReserved($patron, $item, $item_pickup_location)->{status}; if ( $can_item_be_reserved eq 'OK' || ( $can_item_be_reserved ne 'itemAlreadyOnHold' && $can_override ) ) { AddReserve( { branchcode => $item_pickup_location, - borrowernumber => $borrower->{'borrowernumber'}, + borrowernumber => $patron->borrowernumber, biblionumber => $biblionumber, priority => $rank[0], reservation_date => $startdate, @@ -121,11 +120,11 @@ if ( $type eq 'str8' && $borrower ) { } elsif (@biblionumbers > 1) { my $bibinfo = $bibinfos{$biblionumber}; - if ( $can_override || CanBookBeReserved($borrower->{'borrowernumber'}, $biblionumber)->{status} eq 'OK' ) { + if ( $can_override || CanBookBeReserved($patron->borrowernumber, $biblionumber)->{status} eq 'OK' ) { AddReserve( { branchcode => $bibinfo->{pickup}, - borrowernumber => $borrower->{'borrowernumber'}, + borrowernumber => $patron->borrowernumber, biblionumber => $biblionumber, priority => $bibinfo->{rank}, reservation_date => $startdate, @@ -142,11 +141,11 @@ if ( $type eq 'str8' && $borrower ) { } else { # place a request on 1st available for ( my $i = 0 ; $i < $holds_to_place_count ; $i++ ) { - if ( $can_override || CanBookBeReserved($borrower->{'borrowernumber'}, $biblionumber)->{status} eq 'OK' ) { + if ( $can_override || CanBookBeReserved($patron->borrowernumber, $biblionumber)->{status} eq 'OK' ) { AddReserve( { branchcode => $branch, - borrowernumber => $borrower->{'borrowernumber'}, + borrowernumber => $patron->borrowernumber, biblionumber => $biblionumber, priority => $rank[0], reservation_date => $startdate, diff --git a/reserve/request.pl b/reserve/request.pl index 9bad2e8faa..792eb978a6 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -578,7 +578,7 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) $item->{'holdallowed'} = $branchitemrule->{'holdallowed'}; - my $can_item_be_reserved = CanItemBeReserved( $patron->borrowernumber, $itemnumber )->{status}; + my $can_item_be_reserved = CanItemBeReserved( $patron, $item_object )->{status}; $item->{not_holdable} = $can_item_be_reserved unless ( $can_item_be_reserved eq 'OK' ); $item->{item_level_holds} = Koha::CirculationRules->get_opacitemholds_policy( { item => $item_object, patron => $patron } ); diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 69e9b2bc10..03bcc53f9f 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -64,14 +64,16 @@ my $itemnumber = $builder->build_sample_item({ library => $branch_1, biblionumbe # Create some borrowers my @borrowernumbers; +my @patrons; foreach (1..$borrowers_count) { - my $borrowernumber = Koha::Patron->new({ + my $patron = Koha::Patron->new({ firstname => 'my firstname', surname => 'my surname ' . $_, categorycode => $category->{categorycode}, branchcode => $branch_1, - })->store->borrowernumber; - push @borrowernumbers, $borrowernumber; + })->store; + push @patrons, $patron; + push @borrowernumbers, $patron->borrowernumber; } # Create five item level holds @@ -233,7 +235,7 @@ is( $hold->priority, '6', "Test AlterPriority(), move to bottom" ); # IndependentBranches is OFF. my $foreign_biblio = $builder->build_sample_biblio({ itemtype => 'DUMMY' }); -my $foreign_itemnumber = $builder->build_sample_item({ library => $branch_2, biblionumber => $foreign_biblio->biblionumber })->itemnumber; +my $foreign_item = $builder->build_sample_item({ library => $branch_2, biblionumber => $foreign_biblio->biblionumber }); Koha::CirculationRules->set_rules( { categorycode => undef, @@ -265,7 +267,7 @@ t::lib::Mocks::mock_preference('item-level_itypes', 1); t::lib::Mocks::mock_preference('IndependentBranches', 0); is( - CanItemBeReserved($borrowernumbers[0], $foreign_itemnumber)->{status}, 'OK', + CanItemBeReserved($patrons[0], $foreign_item)->{status}, 'OK', '$branch_1 patron allowed to reserve $branch_2 item with IndependentBranches OFF (bug 2394)' ); @@ -273,14 +275,14 @@ is( t::lib::Mocks::mock_preference('IndependentBranches', 1); t::lib::Mocks::mock_preference('canreservefromotherbranches', 0); ok( - CanItemBeReserved($borrowernumbers[0], $foreign_itemnumber)->{status} eq 'cannotReserveFromOtherBranches', + CanItemBeReserved($patrons[0], $foreign_item)->{status} eq 'cannotReserveFromOtherBranches', '$branch_1 patron NOT allowed to reserve $branch_2 item with IndependentBranches ON ... (bug 2394)' ); # ... unless canreservefromotherbranches is ON t::lib::Mocks::mock_preference('canreservefromotherbranches', 1); ok( - CanItemBeReserved($borrowernumbers[0], $foreign_itemnumber)->{status} eq 'OK', + CanItemBeReserved($patrons[0], $foreign_item)->{status} eq 'OK', '... unless canreservefromotherbranches is ON (bug 2394)' ); @@ -325,9 +327,9 @@ ok( is( $hold3->discard_changes->priority, 1, "After ModReserve, the 3rd reserve becomes the first on the waiting list" ); } -Koha::Items->find($itemnumber)->damaged(1)->store; # FIXME The $itemnumber is a bit confusing here +my $damaged_item = Koha::Items->find($itemnumber)->damaged(1)->store; # FIXME The $itemnumber is a bit confusing here t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 1 ); -is( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'OK', "Patron can reserve damaged item with AllowHoldsOnDamagedItems enabled" ); +is( CanItemBeReserved( $patrons[0], $damaged_item)->{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( @@ -337,20 +339,20 @@ $hold = Koha::Hold->new( biblionumber => $biblio->biblionumber, } )->store(); -is( CanItemBeReserved( $borrowernumbers[0], $itemnumber )->{status}, +is( CanItemBeReserved( $patrons[0], $damaged_item )->{status}, 'itemAlreadyOnHold', "Patron cannot place a second item level hold for a given item" ); $hold->delete(); t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 0 ); -ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'damaged', "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" ); +ok( CanItemBeReserved( $patrons[0], $damaged_item)->{status} eq 'damaged', "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" ); ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for damaged item with AllowHoldsOnDamagedItems disabled" ); # Items that are not for loan, but holdable should not be trapped until they are available for loan t::lib::Mocks::mock_preference( 'TrapHoldsOnOrder', 0 ); -Koha::Items->find($itemnumber)->damaged(0)->notforloan(-1)->store; +my $nfl_item = Koha::Items->find($itemnumber)->damaged(0)->notforloan(-1)->store; Koha::Holds->search({ biblionumber => $biblio->id })->delete(); -is( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'OK', "Patron can place hold on item that is not for loan but holdable ( notforloan < 0 )" ); +is( CanItemBeReserved( $patrons[0], $nfl_item)->{status}, 'OK', "Patron can place hold on item that is not for loan but holdable ( notforloan < 0 )" ); $hold = Koha::Hold->new( { borrowernumber => $borrowernumbers[0], @@ -370,11 +372,11 @@ ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for i t::lib::Mocks::mock_preference( 'SkipHoldTrapOnNotForLoanValue', '-1|1' ); ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item with notforloan value matching SkipHoldTrapOnNotForLoanValue" ); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'itemAlreadyOnHold', + CanItemBeReserved( $patrons[0], $nfl_item)->{status}, 'itemAlreadyOnHold', "cannot request item that you have already reservedd" ); is( - CanItemBeReserved( $borrowernumbers[0], $item->itemnumber, undef, { ignore_hold_counts => 1 })->{status}, 'OK', + CanItemBeReserved( $patrons[0], $item, undef, { ignore_hold_counts => 1 })->{status}, 'OK', "can request item if we are not checking holds counts, but only if policy allows or forbids it" ); $hold->delete(); @@ -391,11 +393,11 @@ AddReserve( } ); is( - CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status}, 'noReservesAllowed', + CanItemBeReserved( $patrons[0], $item)->{status}, 'noReservesAllowed', "cannot request item if policy that matches on item-level item type forbids it" ); is( - CanItemBeReserved( $borrowernumbers[0], $item->itemnumber, undef, { ignore_hold_counts => 1 })->{status}, 'noReservesAllowed', + CanItemBeReserved( $patrons[0], $item, undef, { ignore_hold_counts => 1 })->{status}, 'noReservesAllowed', "cannot request item if policy that matches on item-level item type forbids it even if ignoring counts" ); @@ -468,34 +470,34 @@ subtest 'CanItemBeReserved' => sub { my $biblionumber_can = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber; my $biblionumber_record_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant_record })->biblionumber; - my $itemnumber_1_can = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_cannot })->itemnumber; - my $itemnumber_1_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant, biblionumber => $biblionumber_cannot })->itemnumber; - my $itemnumber_2_can = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_can })->itemnumber; - my $itemnumber_2_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant, biblionumber => $biblionumber_can })->itemnumber; - my $itemnumber_3_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant_record, biblionumber => $biblionumber_record_cannot })->itemnumber; + my $item_1_can = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_cannot }); + my $item_1_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant, biblionumber => $biblionumber_cannot }); + my $item_2_can = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_can }); + my $item_2_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant, biblionumber => $biblionumber_can }); + my $item_3_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant_record, biblionumber => $biblionumber_record_cannot }); Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete; t::lib::Mocks::mock_preference('item-level_itypes', 1); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'noReservesAllowed', + CanItemBeReserved( $patrons[0], $item_2_cannot)->{status}, 'noReservesAllowed', "With item level set, rule from item must be picked (CANNOT)" ); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'OK', + CanItemBeReserved( $patrons[0], $item_1_can)->{status}, 'OK', "With item level set, rule from item must be picked (CAN)" ); t::lib::Mocks::mock_preference('item-level_itypes', 0); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'noReservesAllowed', + CanItemBeReserved( $patrons[0], $item_1_can)->{status}, 'noReservesAllowed', "With biblio level set, rule from biblio must be picked (CANNOT)" ); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'OK', + CanItemBeReserved( $patrons[0], $item_2_cannot)->{status}, 'OK', "With biblio level set, rule from biblio must be picked (CAN)" ); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_3_cannot)->{status}, 'noReservesAllowed', + CanItemBeReserved( $patrons[0], $item_3_cannot)->{status}, 'noReservesAllowed', "When no holds allowed and no holds per record allowed should return noReservesAllowed" ); }; @@ -504,11 +506,11 @@ subtest 'CanItemBeReserved' => sub { plan tests => 7; my $biblionumber_1 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber; - my $itemnumber_11 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_1 })->itemnumber; - my $itemnumber_12 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_1 })->itemnumber; + my $item_11 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_1 }); + my $item_12 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_1 }); my $biblionumber_2 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber; - my $itemnumber_21 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_2 })->itemnumber; - my $itemnumber_22 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_2 })->itemnumber; + my $item_21 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_2 }); + my $item_22 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_2 }); Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete; @@ -522,7 +524,7 @@ subtest 'CanItemBeReserved' => sub { t::lib::Mocks::mock_preference('item-level_itypes', $item_level); is( # FIXME This is not really correct, but CanItemBeReserved does not check if biblio-level holds already exist - CanItemBeReserved( $borrowernumbers[0], $itemnumber_11)->{status}, 'OK', + CanItemBeReserved( $patrons[0], $item_11)->{status}, 'OK', "A biblio-level hold already exists - another hold can be placed on a specific item item" ); } @@ -533,7 +535,7 @@ subtest 'CanItemBeReserved' => sub { branch => $branch_1, borrowernumber => $borrowernumbers[0], biblionumber => $biblionumber_1, - itemnumber => $itemnumber_11, + itemnumber => $item_11->itemnumber, }); $dbh->do('DELETE FROM circulation_rules'); @@ -549,7 +551,7 @@ subtest 'CanItemBeReserved' => sub { } ); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord', + CanItemBeReserved( $patrons[0], $item_12)->{status}, 'tooManyHoldsForThisRecord', "A item-level hold already exists and holds_per_record=1, another hold cannot be placed on this record" ); Koha::CirculationRules->set_rules( @@ -564,7 +566,7 @@ subtest 'CanItemBeReserved' => sub { } ); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord', + CanItemBeReserved( $patrons[0], $item_12)->{status}, 'tooManyHoldsForThisRecord', "A item-level hold already exists and holds_per_record=1 - tooManyHoldsForThisRecord has priority over tooManyReserves" ); Koha::CirculationRules->set_rules( @@ -579,7 +581,7 @@ subtest 'CanItemBeReserved' => sub { } ); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'OK', + CanItemBeReserved( $patrons[0], $item_12)->{status}, 'OK', "A item-level hold already exists but holds_per_record=2- another item-level hold can be placed on this record" ); @@ -587,7 +589,7 @@ subtest 'CanItemBeReserved' => sub { branch => $branch_1, borrowernumber => $borrowernumbers[0], biblionumber => $biblionumber_2, - itemnumber => $itemnumber_21 + itemnumber => $item_21->itemnumber }); Koha::CirculationRules->set_rules( { @@ -601,11 +603,11 @@ subtest 'CanItemBeReserved' => sub { } ); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_21)->{status}, 'itemAlreadyOnHold', + CanItemBeReserved( $patrons[0], $item_21)->{status}, 'itemAlreadyOnHold', "A item-level holds already exists on this item, itemAlreadyOnHold should be raised" ); is( - CanItemBeReserved( $borrowernumbers[0], $itemnumber_22)->{status}, 'tooManyReserves', + CanItemBeReserved( $patrons[0], $item_22)->{status}, 'tooManyReserves', "This patron has already placed reservesallowed holds, tooManyReserves should be raised" ); }; @@ -647,22 +649,22 @@ Koha::CirculationRules->set_rules( } ); $biblio = $builder->build_sample_biblio({ itemtype => 'CANNOT' }); -$itemnumber = $builder->build_sample_item({ library => $branch_1, itype => 'CANNOT', biblionumber => $biblio->biblionumber})->itemnumber; -is(CanItemBeReserved($borrowernumbers[0], $itemnumber)->{status}, 'notReservable', +my $branch_rule_item = $builder->build_sample_item({ library => $branch_1, itype => 'CANNOT', biblionumber => $biblio->biblionumber}); +is(CanItemBeReserved($patrons[0], $branch_rule_item)->{status}, 'notReservable', "CanItemBeReserved should return 'notReservable'"); t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' ); -$itemnumber = $builder->build_sample_item({ library => $branch_2, itype => 'CAN', biblionumber => $biblio->biblionumber})->itemnumber; -is(CanItemBeReserved($borrowernumbers[0], $itemnumber)->{status}, +$branch_rule_item = $builder->build_sample_item({ library => $branch_2, itype => 'CAN', biblionumber => $biblio->biblionumber}); +is(CanItemBeReserved($patrons[0], $branch_rule_item)->{status}, 'cannotReserveFromOtherBranches', "CanItemBeReserved should use PatronLibrary rule when ReservesControlBranch set to 'PatronLibrary'"); t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'ItemHomeLibrary' ); -is(CanItemBeReserved($borrowernumbers[0], $itemnumber)->{status}, +is(CanItemBeReserved($patrons[0], $branch_rule_item)->{status}, 'OK', "CanItemBeReserved should use item home library rule when ReservesControlBranch set to 'ItemsHomeLibrary'"); -$itemnumber = $builder->build_sample_item({ library => $branch_1, itype => 'CAN', biblionumber => $biblio->biblionumber})->itemnumber; -is(CanItemBeReserved($borrowernumbers[0], $itemnumber)->{status}, 'OK', +$branch_rule_item = $builder->build_sample_item({ library => $branch_1, itype => 'CAN', biblionumber => $biblio->biblionumber}); +is(CanItemBeReserved($patrons[0], $branch_rule_item)->{status}, 'OK', "CanItemBeReserved should return 'OK'"); # Bug 12632 @@ -675,7 +677,7 @@ $dbh->do('DELETE FROM items'); $dbh->do('DELETE FROM biblio'); $biblio = $builder->build_sample_biblio({ itemtype => 'ONLY1' }); -$itemnumber = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber})->itemnumber; +my $limit_count_item = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber}); Koha::CirculationRules->set_rules( { @@ -688,7 +690,7 @@ Koha::CirculationRules->set_rules( } } ); -is( CanItemBeReserved( $borrowernumbers[0], $itemnumber )->{status}, +is( CanItemBeReserved( $patrons[0], $limit_count_item )->{status}, 'OK', 'Patron can reserve item with hold limit of 1, no holds placed' ); my $res_id = AddReserve( @@ -700,14 +702,14 @@ my $res_id = AddReserve( } ); -is( CanItemBeReserved( $borrowernumbers[0], $itemnumber )->{status}, +is( CanItemBeReserved( $patrons[0], $limit_count_item )->{status}, 'tooManyReserves', 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' ); -is( CanItemBeReserved( $borrowernumbers[0], $itemnumber, undef, { ignore_hold_counts => 1 } )->{status}, +is( CanItemBeReserved( $patrons[0], $limit_count_item, undef, { ignore_hold_counts => 1 } )->{status}, 'OK', 'Patron can reserve item if checking policy but not counts' ); #results should be the same for both ReservesControlBranch settings t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'ItemHomeLibrary' ); -is( CanItemBeReserved( $borrowernumbers[0], $itemnumber )->{status}, +is( CanItemBeReserved( $patrons[0], $limit_count_item )->{status}, 'tooManyReserves', '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' ); @@ -718,7 +720,7 @@ subtest 'Test max_holds per library/patron category' => sub { $dbh->do('DELETE FROM reserves'); $biblio = $builder->build_sample_biblio; - $itemnumber = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber})->itemnumber; + my $max_holds_item = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber}); Koha::CirculationRules->set_rules( { categorycode => undef, @@ -746,7 +748,7 @@ subtest 'Test max_holds per library/patron category' => sub { Koha::Holds->search( { borrowernumber => $borrowernumbers[0] } )->count(); is( $count, 3, 'Patron now has 3 holds' ); - my $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); + my $ret = CanItemBeReserved( $patrons[0], $max_holds_item ); is( $ret->{status}, 'OK', 'Patron can place hold with no borrower circ rules' ); my $rule_all = Koha::CirculationRules->set_rule( @@ -767,19 +769,19 @@ subtest 'Test max_holds per library/patron category' => sub { } ); - $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); + $ret = CanItemBeReserved( $patrons[0], $max_holds_item ); is( $ret->{status}, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 3' ); $rule_branch->delete(); - $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); + $ret = CanItemBeReserved( $patrons[0], $max_holds_item ); is( $ret->{status}, 'tooManyReserves', 'Patron cannot place hold with only a category rule of 3' ); $rule_all->delete(); $rule_branch->rule_value(3); $rule_branch->store(); - $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); + $ret = CanItemBeReserved( $patrons[0], $max_holds_item ); is( $ret->{status}, 'tooManyReserves', 'Patron cannot place hold with only a branch/category rule of 3' ); $rule_branch->rule_value(5); @@ -787,7 +789,7 @@ subtest 'Test max_holds per library/patron category' => sub { $rule_branch->rule_value(5); $rule_branch->store(); - $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); + $ret = CanItemBeReserved( $patrons[0], $max_holds_item ); is( $ret->{status}, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 5' ); }; @@ -795,7 +797,7 @@ subtest 'Pickup location availability tests' => sub { plan tests => 4; $biblio = $builder->build_sample_biblio({ itemtype => 'ONLY1' }); - $itemnumber = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber})->itemnumber; + my $pickup_item = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber}); #Add a default rule to allow some holds Koha::CirculationRules->set_rules( @@ -809,32 +811,31 @@ subtest 'Pickup location availability tests' => sub { } } ); - my $item = Koha::Items->find($itemnumber); my $branch_to = $builder->build({ source => 'Branch' })->{ branchcode }; my $library = Koha::Libraries->find($branch_to); $library->pickup_location('1')->store; - my $patron = $builder->build({ source => 'Borrower' })->{ borrowernumber }; + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1); t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype'); $library->pickup_location('1')->store; - is(CanItemBeReserved($patron, $item->itemnumber, $branch_to)->{status}, + is(CanItemBeReserved($patron, $pickup_item, $branch_to)->{status}, 'OK', 'Library is a pickup location'); my $limit = Koha::Item::Transfer::Limit->new({ - fromBranch => $item->holdingbranch, + fromBranch => $pickup_item->holdingbranch, toBranch => $branch_to, - itemtype => $item->effective_itemtype, + itemtype => $pickup_item->effective_itemtype, })->store; - is(CanItemBeReserved($patron, $item->itemnumber, $branch_to)->{status}, + is(CanItemBeReserved($patron, $pickup_item, $branch_to)->{status}, 'cannotBeTransferred', 'Item cannot be transferred'); $limit->delete; $library->pickup_location('0')->store; - is(CanItemBeReserved($patron, $item->itemnumber, $branch_to)->{status}, + is(CanItemBeReserved($patron, $pickup_item, $branch_to)->{status}, 'libraryNotPickupLocation', 'Library is not a pickup location'); - is(CanItemBeReserved($patron, $item->itemnumber, 'nonexistent')->{status}, + is(CanItemBeReserved($patron, $pickup_item, 'nonexistent')->{status}, 'libraryNotFound', 'Cannot set unknown library as pickup location'); }; @@ -852,11 +853,11 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { # Create 3 biblios with items my $biblio_1 = $builder->build_sample_biblio({ itemtype => $itemtype->itemtype }); - my $itemnumber_1 = $builder->build_sample_item({ library => $library->branchcode, biblionumber => $biblio_1->biblionumber})->itemnumber; + my $item_1 = $builder->build_sample_item({ library => $library->branchcode, biblionumber => $biblio_1->biblionumber}); my $biblio_2 = $builder->build_sample_biblio({ itemtype => $itemtype->itemtype }); - my $itemnumber_2 = $builder->build_sample_item({ library => $library->branchcode, biblionumber => $biblio_2->biblionumber})->itemnumber; + my $item_2 = $builder->build_sample_item({ library => $library->branchcode, biblionumber => $biblio_2->biblionumber}); my $biblio_3 = $builder->build_sample_biblio({ itemtype => $itemtype->itemtype }); - my $itemnumber_3 = $builder->build_sample_item({ library => $library->branchcode, biblionumber => $biblio_3->biblionumber})->itemnumber; + my $item_3 = $builder->build_sample_item({ library => $library->branchcode, biblionumber => $biblio_3->biblionumber}); Koha::CirculationRules->set_rules( { @@ -872,7 +873,7 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { ); is_deeply( - CanItemBeReserved( $patron->borrowernumber, $itemnumber_1 ), + CanItemBeReserved( $patron, $item_1 ), { status => 'OK' }, 'Patron can reserve item with hold limit of 1, no holds placed' ); @@ -887,7 +888,7 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { ); is_deeply( - CanItemBeReserved( $patron->borrowernumber, $itemnumber_1 ), + CanItemBeReserved( $patron, $item_1 ), { status => 'tooManyReserves', limit => 1 }, 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' ); @@ -905,7 +906,7 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { ); is_deeply( - CanItemBeReserved( $patron->borrowernumber, $itemnumber_2 ), + CanItemBeReserved( $patron, $item_2 ), { status => 'OK' }, 'Patron can reserve item with 2 reserves daily cap' ); @@ -920,7 +921,7 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { } ); is_deeply( - CanItemBeReserved( $patron->borrowernumber, $itemnumber_2 ), + CanItemBeReserved( $patron, $item_2 ), { status => 'tooManyReservesToday', limit => 2 }, 'Patron cannot a third item with 2 reserves daily cap' ); @@ -931,7 +932,7 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { $hold->reservedate($yesterday)->store; is_deeply( - CanItemBeReserved( $patron->borrowernumber, $itemnumber_2 ), + CanItemBeReserved( $patron, $item_2 ), { status => 'OK' }, 'Patron can reserve item with 2 bib level hold placed on different days, 2 reserves daily cap' ); @@ -952,7 +953,7 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { # Delete existing holds Koha::Holds->search->delete; is_deeply( - CanItemBeReserved( $patron->borrowernumber, $itemnumber_2 ), + CanItemBeReserved( $patron, $item_2 ), { status => 'tooManyReservesToday', limit => 0 }, 'Patron cannot reserve if holds_per_day is 0 (i.e. 0 is 0)' ); @@ -970,7 +971,7 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { Koha::Holds->search->delete; is_deeply( - CanItemBeReserved( $patron->borrowernumber, $itemnumber_2 ), + CanItemBeReserved( $patron, $item_2 ), { status => 'OK' }, 'Patron can reserve if holds_per_day is undef (i.e. undef is unlimited daily cap)' ); @@ -992,7 +993,7 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { ); is_deeply( - CanItemBeReserved( $patron->borrowernumber, $itemnumber_3 ), + CanItemBeReserved( $patron, $item_3 ), { status => 'OK' }, 'Patron can reserve if holds_per_day is undef (i.e. undef is unlimited daily cap)' ); @@ -1005,14 +1006,14 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { } ); is_deeply( - CanItemBeReserved( $patron->borrowernumber, $itemnumber_3 ), + CanItemBeReserved( $patron, $item_3 ), { status => 'tooManyReserves', limit => 3 }, 'Unlimited daily holds, but reached reservesallowed' ); #results should be the same for both ReservesControlBranch settings t::lib::Mocks::mock_preference('ReservesControlBranch', 'ItemHomeLibrary'); is_deeply( - CanItemBeReserved( $patron->borrowernumber, $itemnumber_3 ), + CanItemBeReserved( $patron, $item_3 ), { status => 'tooManyReserves', limit => 3 }, 'Unlimited daily holds, but reached reservesallowed' ); @@ -1079,7 +1080,7 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub { # Test 1: Patron 3 can place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ), + CanItemBeReserved( $patron3, $item_2 ), { status => 'OK' }, 'Patron can place hold if no circ_rules where defined' ); @@ -1099,14 +1100,14 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub { # Test 2: Patron 1 can place hold is_deeply( - CanItemBeReserved( $patron1->borrowernumber, $itemnumber_2 ), + CanItemBeReserved( $patron1, $item_2 ), { status => 'OK' }, 'Patron can place hold because patron\'s home library is part of hold group' ); # Test 3: Patron 3 cannot place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ), + CanItemBeReserved( $patron3, $item_2 ), { status => 'branchNotInHoldGroup' }, 'Patron cannot place hold because patron\'s home library is not part of hold group' ); @@ -1126,7 +1127,7 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub { # Test 4: Patron 3 can place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ), + CanItemBeReserved( $patron3, $item_2 ), { status => 'OK' }, 'Patron can place hold if holdallowed is set to "any" for library 2' ); @@ -1146,7 +1147,7 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub { # Test 5: Patron 3 cannot place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ), + CanItemBeReserved( $patron3, $item_2 ), { status => 'branchNotInHoldGroup' }, 'Patron cannot place hold if holdallowed is set to "hold group" for library 2' ); @@ -1166,7 +1167,7 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub { # Test 6: Patron 3 can place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ), + CanItemBeReserved( $patron3, $item_2 ), { status => 'OK' }, 'Patron can place hold if holdallowed is set to "any" for itemtype 2' ); @@ -1186,7 +1187,7 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub { # Test 7: Patron 3 cannot place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ), + CanItemBeReserved( $patron3, $item_2 ), { status => 'branchNotInHoldGroup' }, 'Patron cannot place hold if holdallowed is set to "hold group" for itemtype 2' ); @@ -1206,7 +1207,7 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub { # Test 8: Patron 3 can place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ), + CanItemBeReserved( $patron3, $item_2 ), { status => 'OK' }, 'Patron can place hold if holdallowed is set to "any" for itemtype 2 and library 2' ); @@ -1226,7 +1227,7 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub { # Test 9: Patron 3 cannot place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2 ), + CanItemBeReserved( $patron3, $item_2 ), { status => 'branchNotInHoldGroup' }, 'Patron cannot place hold if holdallowed is set to "hold group" for itemtype 2 and library 2' ); @@ -1293,7 +1294,7 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub { # Test 1: Patron 3 can place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ), + CanItemBeReserved( $patron3, $item_2, $library3->branchcode ), { status => 'OK' }, 'Patron can place hold if no circ_rules where defined' ); @@ -1313,14 +1314,14 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub { # Test 2: Patron 1 can place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library1->branchcode ), + CanItemBeReserved( $patron3, $item_2, $library1->branchcode ), { status => 'OK' }, 'Patron can place hold because pickup location is part of hold group' ); # Test 3: Patron 3 cannot place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ), + CanItemBeReserved( $patron3, $item_2, $library3->branchcode ), { status => 'pickupNotInHoldGroup' }, 'Patron cannot place hold because pickup location is not part of hold group' ); @@ -1340,7 +1341,7 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub { # Test 4: Patron 3 can place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ), + CanItemBeReserved( $patron3, $item_2, $library3->branchcode ), { status => 'OK' }, 'Patron can place hold if default_branch_circ_rules is set to "any" for library 2' ); @@ -1360,7 +1361,7 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub { # Test 5: Patron 3 cannot place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ), + CanItemBeReserved( $patron3, $item_2, $library3->branchcode ), { status => 'pickupNotInHoldGroup' }, 'Patron cannot place hold if hold_fulfillment_policy is set to "hold group" for library 2' ); @@ -1380,7 +1381,7 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub { # Test 6: Patron 3 can place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ), + CanItemBeReserved( $patron3, $item_2, $library3->branchcode ), { status => 'OK' }, 'Patron can place hold if hold_fulfillment_policy is set to "any" for itemtype 2' ); @@ -1400,7 +1401,7 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub { # Test 7: Patron 3 cannot place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ), + CanItemBeReserved( $patron3, $item_2, $library3->branchcode ), { status => 'pickupNotInHoldGroup' }, 'Patron cannot place hold if hold_fulfillment_policy is set to "hold group" for itemtype 2' ); @@ -1420,7 +1421,7 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub { # Test 8: Patron 3 can place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ), + CanItemBeReserved( $patron3, $item_2, $library3->branchcode ), { status => 'OK' }, 'Patron can place hold if hold_fulfillment_policy is set to "any" for itemtype 2 and library 2' ); @@ -1440,7 +1441,7 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub { # Test 9: Patron 3 cannot place hold is_deeply( - CanItemBeReserved( $patron3->borrowernumber, $itemnumber_2, $library3->branchcode ), + CanItemBeReserved( $patron3, $item_2, $library3->branchcode ), { status => 'pickupNotInHoldGroup' }, 'Patron cannot place hold if hold_fulfillment_policy is set to "hold group" for itemtype 2 and library 2' ); @@ -1574,7 +1575,7 @@ subtest 'CanItemBeReserved rule precedence tests' => sub { } ); is_deeply( - CanItemBeReserved( $patron->borrowernumber, $item->itemnumber, $library->branchcode ), + CanItemBeReserved( $patron, $item, $library->branchcode ), { status => 'OK' }, 'Patron of specified category can place 1 hold on specified itemtype' ); @@ -1587,7 +1588,7 @@ subtest 'CanItemBeReserved rule precedence tests' => sub { borrowernumber => $patron->borrowernumber, }}); is_deeply( - CanItemBeReserved( $patron->borrowernumber, $item->itemnumber, $library->branchcode ), + CanItemBeReserved( $patron, $item, $library->branchcode ), { status => 'tooManyReserves', limit => 1 }, 'Patron of specified category can place 1 hold on specified itemtype, cannot place a second' ); @@ -1602,7 +1603,7 @@ subtest 'CanItemBeReserved rule precedence tests' => sub { } ); is_deeply( - CanItemBeReserved( $patron->borrowernumber, $item->itemnumber, $library->branchcode ), + CanItemBeReserved( $patron, $item, $library->branchcode ), { status => 'OK' }, 'Patron of specified category can place 1 hold on specified itemtype if library rule for all types and categories set to 2' ); diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index c416476caa..11f18959e9 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -1239,8 +1239,7 @@ subtest 'AllowHoldOnPatronPossession test' => sub { 'alreadypossession', 'Patron cannot place hold on a book loaned to itself'); - is(C4::Reserves::CanItemBeReserved($patron->borrowernumber, - $item->itemnumber)->{status}, + is(C4::Reserves::CanItemBeReserved( $patron, $item )->{status}, 'alreadypossession', 'Patron cannot place hold on an item loaned to itself'); @@ -1251,8 +1250,7 @@ subtest 'AllowHoldOnPatronPossession test' => sub { 'OK', 'Patron can place hold on a book loaned to itself'); - is(C4::Reserves::CanItemBeReserved($patron->borrowernumber, - $item->itemnumber)->{status}, + is(C4::Reserves::CanItemBeReserved( $patron, $item )->{status}, 'OK', 'Patron can place hold on an item loaned to itself'); }; -- 2.39.5