@@ -, +, @@ --- 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(-) --- a/C4/Circulation.pm +++ a/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', } @@ -2846,7 +2845,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; --- a/C4/ILSDI/Services.pm +++ a/C4/ILSDI/Services.pm @@ -872,7 +872,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; --- a/C4/Reserves.pm +++ a/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' }; } } @@ -1423,7 +1429,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; --- a/Koha/Club/Hold.pm +++ a/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 ); } --- a/Koha/REST/V1/Holds.pm +++ a/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') ) { --- a/opac/opac-reserve.pl +++ a/opac/opac-reserve.pl @@ -237,9 +237,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 ); @@ -254,8 +254,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; @@ -276,8 +275,9 @@ if ( $query->param('place_reserve') ) { my $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'; @@ -559,7 +559,7 @@ foreach my $biblioNum (@biblionumbers) { my $policy_holdallowed = !$itemLoopIter->{already_reserved}; $policy_holdallowed &&= IsAvailableForItemLevelRequest($item, $patron) && - CanItemBeReserved( $borrowernumber, $itemNum )->{status} eq 'OK'; + CanItemBeReserved( $patron, $item )->{status} eq 'OK'; if ($policy_holdallowed) { my $opac_hold_policy = Koha::CirculationRules->get_opacitemholds_policy( { item => $item, patron => $patron } ); --- a/reserve/placerequest.pl +++ a/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, --- a/reserve/request.pl +++ a/reserve/request.pl @@ -565,7 +565,7 @@ foreach my $biblionumber (@biblionumbers) { $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 } ); --- a/t/db_dependent/Holds.t +++ a/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' ); --- a/t/db_dependent/Reserves.t +++ a/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'); }; --