From 3c5c0f32f52b2e96438dc6b56d63dbfcec5622c3 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 --- 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 c2edf81f949..f61065a8ab2 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', } @@ -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; diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index 34465a7caf6..e9af230f734 100644 --- a/C4/ILSDI/Services.pm +++ b/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; diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 1490d5aa88a..3a49f7cc1b9 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' }; } } @@ -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; diff --git a/Koha/Club/Hold.pm b/Koha/Club/Hold.pm index fbf35f43144..9c1bbc7a705 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 0ca60b14597..c4a942caca3 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 a7391e23273..c759e4fd362 100755 --- a/opac/opac-reserve.pl +++ b/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 } ); diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 5ee8dce2f6b..9f6d82dc4fb 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 1b6638307db..327421102c9 100755 --- a/reserve/request.pl +++ b/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 } ); diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 69e9b2bc104..03bcc53f9f5 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 a0e9df00164..11f49d4da82 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.25.1