Bugzilla – Attachment 129261 Details for
Bug 29562
Pass objects to CanItemBeReserved and checkHighHolds
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 29562: Adjust CanItemBeReserved and checkHighHolds to take objects
Bug-29562-Adjust-CanItemBeReserved-and-checkHighHo.patch (text/plain), 58.79 KB, created by
Jonathan Druart
on 2022-01-10 13:20:39 UTC
(
hide
)
Description:
Bug 29562: Adjust CanItemBeReserved and checkHighHolds to take objects
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2022-01-10 13:20:39 UTC
Size:
58.79 KB
patch
obsolete
>From 3c5c0f32f52b2e96438dc6b56d63dbfcec5622c3 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >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 <tomascohen@theke.io> > >Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >--- > 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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=129261">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 29562
:
127966
|
129245
|
129246
|
129252
|
129253
|
129254
|
129255
|
129257
|
129261
|
129262
|
129263
|
129264
|
129926
|
129927
|
129928
|
129929
|
130033