Bugzilla – Attachment 150611 Details for
Bug 31735
Avoid re-fetching objects from database by passing them down instead of object ids
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines
Bug-31735-Avoid-re-fetcing-objects-from-database-b.patch (text/plain), 142.15 KB, created by
David Gustafsson
on 2023-05-03 18:08:14 UTC
(
hide
)
Description:
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines
Filename:
MIME Type:
Creator:
David Gustafsson
Created:
2023-05-03 18:08:14 UTC
Size:
142.15 KB
patch
obsolete
>From 7308bd5b5ceca05ea1db738600e8e2f059404411 Mon Sep 17 00:00:00 2001 >From: David Gustafsson <david.gustafsson@ub.gu.se> >Date: Tue, 27 Sep 2022 18:23:27 +0200 >Subject: [PATCH] Bug 31735: Avoid re-fetcing objects from database by passing > them directly instead of ids to various subroutines > >To test: > >1) Run the following test and make sure all pass: > t/db_dependent/api/v1/biblios.t > t/db_dependent/api/v1/checkouts.t > t/db_dependent/api/v1/return_claims.t > t/db_dependent/Circulation/CalcDateDue.t > t/db_dependent/Circulation/CheckIfIssuedToPatron.t > t/db_dependent/Circulation/dateexpiry.t > t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t > t/db_dependent/Circulation/GetTopIssues.t > t/db_dependent/Circulation_holdsqueue.t > t/db_dependent/Circulation/IsItemIssued.t > t/db_dependent/Circulation/issue.t > t/db_dependent/Circulation/MarkIssueReturned.t > t/db_dependent/Circulation/maxsuspensiondays.t > t/db_dependent/Circulation/ReturnClaims.t > t/db_dependent/Circulation/Returns.t > t/db_dependent/Circulation/SwitchOnSiteCheckouts.t > t/db_dependent/Circulation.t > t/db_dependent/Circulation/TooMany.t > t/db_dependent/Circulation/transferbook.t > t/db_dependent/DecreaseLoanHighHolds.t > t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t > t/db_dependent/HoldsQueue.t > t/db_dependent/Holds/RevertWaitingStatus.t > t/db_dependent/Illrequests.t > t/db_dependent/ILSDI_Services.t > t/db_dependent/Items.t > t/db_dependent/Koha/Account/Line.t > t/db_dependent/Koha/Acquisition/Order.t > t/db_dependent/Koha/Biblio.t > t/db_dependent/Koha/Holds.t > t/db_dependent/Koha/Items.t > t/db_dependent/Koha/Item.t > t/db_dependent/Koha/Object.t > t/db_dependent/Koha/Patrons.t > t/db_dependent/Koha/Plugins/Circulation_hooks.t > t/db_dependent/Koha/Pseudonymization.t > t/db_dependent/Koha/Recalls.t > t/db_dependent/Koha/Recall.t > t/db_dependent/Koha/Template/Plugin/CirculationRules.t > t/db_dependent/Letters/TemplateToolkit.t > t/db_dependent/Members/GetAllIssues.t > t/db_dependent/Members/IssueSlip.t > t/db_dependent/Patron/Borrower_Discharge.t > t/db_dependent/Patron/Borrower_PrevCheckout.t > t/db_dependent/Reserves/GetReserveFee.t > t/db_dependent/Reserves.t > t/db_dependent/rollingloans.t > t/db_dependent/selenium/regressions.t > t/db_dependent/SIP/ILS.t > t/db_dependent/Holds.t > t/db_dependent/Holds/LocalHoldsPriority.t > t/db_dependent/Holds/HoldFulfillmentPolicy.t > t/db_dependent/Holds/HoldItemtypeLimit.t > t/db_dependent/Circulation/transferbook.t >2) Performe one or more checkouts for a patron, making sure > that the circulation rules allows for renewals (for example by > setting an earlier due-date). >3) Log in as this patron in OPAC and make sure the list of > checkouts is displayed correctly, and that renewing an issue > still works. > >Sponsored-by: Gothenburg University Library >Signed-off-by: Nick Clemens <nick@bywatersolutions.com> >--- > C4/Circulation.pm | 149 ++++---- > C4/ILSDI/Services.pm | 12 +- > C4/Reserves.pm | 96 ++--- > C4/RotatingCollections.pm | 1 - > C4/SIP/ILS/Transaction/Checkin.pm | 2 +- > C4/SIP/ILS/Transaction/Renew.pm | 10 +- > C4/SIP/ILS/Transaction/RenewAll.pm | 7 +- > C4/SIP/Sip/MsgType.pm | 3 +- > Koha/Account/Line.pm | 7 +- > Koha/Hold.pm | 2 +- > Koha/REST/V1/Checkouts.pm | 15 +- > circ/renew.pl | 27 +- > .../prog/en/includes/renew_strings.inc | 2 + > .../bootstrap/en/modules/opac-user.tt | 2 + > misc/cronjobs/automatic_renewals.pl | 25 +- > opac/opac-renew.pl | 3 +- > opac/opac-user.pl | 13 +- > opac/sco/sco-main.pl | 7 +- > svc/checkouts | 4 +- > svc/renew | 5 +- > t/db_dependent/Circulation.t | 349 +++++++++--------- > t/db_dependent/Circulation/Returns.t | 1 + > t/db_dependent/Circulation/transferbook.t | 7 +- > t/db_dependent/Holds.t | 26 +- > t/db_dependent/Holds/HoldFulfillmentPolicy.t | 25 +- > t/db_dependent/Holds/HoldItemtypeLimit.t | 10 +- > t/db_dependent/Holds/LocalHoldsPriority.t | 20 +- > t/db_dependent/Koha/Account/Line.t | 1 + > t/db_dependent/Koha/Object.t | 3 + > t/db_dependent/Letters/TemplateToolkit.t | 9 +- > t/db_dependent/Reserves.t | 57 ++- > 31 files changed, 438 insertions(+), 462 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 6bba3691141..d327aba7583 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -65,7 +65,7 @@ use Koha::Plugins; > use Koha::Recalls; > use Carp qw( carp ); > use List::MoreUtils qw( any ); >-use Scalar::Util qw( looks_like_number ); >+use Scalar::Util qw( looks_like_number blessed ); > use Date::Calc qw( Date_to_Days ); > our (@ISA, @EXPORT_OK); > BEGIN { >@@ -369,7 +369,7 @@ sub transferbook { > } > > # check if it is still issued to someone, return it... >- my $issue = Koha::Checkouts->find({ itemnumber => $itemnumber }); >+ my $issue = $item->checkout; > if ( $issue ) { > AddReturn( $barcode, $fbr ); > $messages->{'WasReturned'} = $issue->borrowernumber; >@@ -378,7 +378,7 @@ sub transferbook { > # find reserves..... > # That'll save a database query. > my ( $resfound, $resrec, undef ) = >- CheckReserves( $itemnumber ); >+ CheckReserves( $item ); > if ( $resfound ) { > $resrec->{'ResFound'} = $resfound; > $messages->{'ResFound'} = $resrec; >@@ -958,10 +958,7 @@ sub CanBookBeIssued { > and C4::Context->preference('SwitchOnSiteCheckouts') ) { > $messages{ONSITE_CHECKOUT_WILL_BE_SWITCHED} = 1; > } else { >- my ($CanBookBeRenewed,$renewerror) = CanBookBeRenewed( >- $patron->borrowernumber, >- $item_object->itemnumber, >- ); >+ my ($CanBookBeRenewed,$renewerror) = CanBookBeRenewed($patron, $issue); > if ( $CanBookBeRenewed == 0 ) { # no more renewals allowed > if ( $renewerror eq 'onsite_checkout' ) { > $issuingimpossible{NO_RENEWAL_FOR_ONSITE_CHECKOUTS} = 1; >@@ -983,18 +980,19 @@ sub CanBookBeIssued { > > my ( $can_be_returned, $message ) = CanBookBeReturned( $item_unblessed, C4::Context->userenv->{branch} ); > >- unless ( $can_be_returned ) { >+ if ( !$can_be_returned ) { > $issuingimpossible{RETURN_IMPOSSIBLE} = 1; > $issuingimpossible{branch_to_return} = $message; > } else { > if ( C4::Context->preference('AutoReturnCheckedOutItems') ) { > $alerts{RETURNED_FROM_ANOTHER} = { patron => $patron }; >- } else { >- $needsconfirmation{ISSUED_TO_ANOTHER} = 1; >- $needsconfirmation{issued_firstname} = $patron->firstname; >- $needsconfirmation{issued_surname} = $patron->surname; >- $needsconfirmation{issued_cardnumber} = $patron->cardnumber; >- $needsconfirmation{issued_borrowernumber} = $patron->borrowernumber; >+ } >+ else { >+ $needsconfirmation{ISSUED_TO_ANOTHER} = 1; >+ $needsconfirmation{issued_firstname} = $patron->firstname; >+ $needsconfirmation{issued_surname} = $patron->surname; >+ $needsconfirmation{issued_cardnumber} = $patron->cardnumber; >+ $needsconfirmation{issued_borrowernumber} = $patron->borrowernumber; > } > } > } >@@ -1027,9 +1025,9 @@ sub CanBookBeIssued { > # CHECKPREVCHECKOUT: CHECK IF ITEM HAS EVER BEEN LENT TO PATRON > # > $patron = Koha::Patrons->find( $patron->borrowernumber ); # FIXME Refetch just in case, to avoid regressions. But must not be needed >- my $wants_check = $patron->wants_check_for_previous_checkout; >- $needsconfirmation{PREVISSUE} = 1 >- if ($wants_check and $patron->do_check_for_previous_checkout($item_unblessed)); >+ if ( $patron->wants_check_for_previous_checkout && $patron->do_check_for_previous_checkout($item_unblessed) ) { >+ $needsconfirmation{PREVISSUE} = 1; >+ } > > # > # ITEM CHECKING >@@ -1167,7 +1165,7 @@ sub CanBookBeIssued { > > unless ( $ignore_reserves and defined $recall ) { > # See if the item is on reserve. >- my ( $restype, $res ) = C4::Reserves::CheckReserves( $item_object->itemnumber ); >+ my ( $restype, $res ) = CheckReserves( $item_object ); > if ($restype) { > my $resbor = $res->{'borrowernumber'}; > if ( $resbor ne $patron->borrowernumber ) { >@@ -2384,7 +2382,7 @@ sub AddReturn { > # launch the Checkreserves routine to find any holds > my ($resfound, $resrec); > my $lookahead= C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds >- ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->itemnumber, undef, $lookahead ) unless ( $item->withdrawn ); >+ ($resfound, $resrec, undef) = CheckReserves( $item, $lookahead ) unless ( $item->withdrawn ); > # if a hold is found and is waiting at another branch, change the priority back to 1 and trigger the hold (this will trigger a transfer and update the hold status properly) > if ( $resfound and $resfound eq "Waiting" and $branch ne $resrec->{branchcode} ) { > my $hold = C4::Reserves::RevertWaitingStatus( { itemnumber => $item->itemnumber } ); >@@ -2880,14 +2878,13 @@ sub GetUpcomingDueIssues { > > =head2 CanBookBeRenewed > >- ($ok,$error,$info) = &CanBookBeRenewed($borrowernumber, $itemnumber[, $override_limit]); >+ ($ok,$error,$info) = &CanBookBeRenewed($patron, $issue, $override_limit); > > Find out whether a borrowed item may be renewed. > >-C<$borrowernumber> is the borrower number of the patron who currently >-has the item on loan. >+C<$patron> is the patron who currently has the issue. > >-C<$itemnumber> is the number of the item to renew. >+C<$issue> is the checkout to renew. > > C<$override_limit>, if supplied with a true value, causes > the limit on the number of times that the loan can be renewed >@@ -2906,19 +2903,19 @@ already renewed the loan. > =cut > > sub CanBookBeRenewed { >- my ( $borrowernumber, $itemnumber, $override_limit, $cron ) = @_; >+ my ( $patron, $issue, $override_limit, $cron ) = @_; > > my $auto_renew = "no"; > my $soonest; >+ my $item = $issue->item; > >- my $item = Koha::Items->find($itemnumber) or return ( 0, 'no_item' ); >- my $issue = $item->checkout or return ( 0, 'no_checkout' ); >+ return ( 0, 'no_item' ) unless $item; >+ return ( 0, 'no_checkout' ) unless $issue; > return ( 0, 'onsite_checkout' ) if $issue->onsite_checkout; >+ return ( 0, 'item_issued_to_other_patron') if $issue->borrowernumber != $patron->borrowernumber; > return ( 0, 'item_denied_renewal') if $item->is_denied_renewal; > >- my $patron = $issue->patron or return; >- >- # override_limit will override anything else except on_reserve >+ # override_limit will override anything else except on_reserve > unless ( $override_limit ){ > my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ); > my $issuing_rule = Koha::CirculationRules->get_effective_rules( >@@ -2944,7 +2941,6 @@ sub CanBookBeRenewed { > > my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing'); > my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing'); >- $patron = Koha::Patrons->find($borrowernumber); # FIXME Is this really useful? > my $restricted = $patron->is_debarred; > my $hasoverdues = $patron->has_overdues; > >@@ -2992,9 +2988,8 @@ sub CanBookBeRenewed { > non_priority => 0, > found => undef, > reservedate => { '<=' => \'NOW()' }, >- suspend => 0, >- }, >- { prefetch => 'patron' } >+ suspend => 0 >+ } > ); > if ( $fillable_holds->count ) { > if ( C4::Context->preference('AllowRenewalIfOtherItemsAvailable') ) { >@@ -3006,7 +3001,7 @@ sub CanBookBeRenewed { > biblionumber => $item->biblionumber, > onloan => undef, > notforloan => 0, >- -not => { itemnumber => $itemnumber } })->as_list; >+ -not => { itemnumber => $item->itemnumber } })->as_list; > > return ( 0, "on_reserve" ) if @possible_holds && (scalar @other_items < scalar @possible_holds); > >@@ -3014,7 +3009,10 @@ sub CanBookBeRenewed { > foreach my $possible_hold (@possible_holds) { > my $fillable = 0; > my $patron_with_reserve = Koha::Patrons->find($possible_hold->borrowernumber); >- my $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron_with_reserve }); >+ my $items_any_available = ItemsAnyAvailableAndNotRestricted({ >+ biblionumber => $item->biblionumber, >+ patron => $patron_with_reserve >+ }); > > # FIXME: We are not checking whether the item we are renewing can fill the hold > >@@ -3032,15 +3030,15 @@ sub CanBookBeRenewed { > } > return ( 0, "on_reserve" ) unless $fillable; > } >- >- } else { >- my ($status, $matched_reserve, $possible_reserves) = CheckReserves($itemnumber); >+ } >+ else { >+ my ($status, $matched_reserve, $possible_reserves) = CheckReserves($item); > return ( 0, "on_reserve" ) if $status; > } > } > > return ( 0, $auto_renew, { soonest_renew_date => $soonest } ) if $auto_renew =~ 'too_soon';#$auto_renew ne "no" && $auto_renew ne "ok"; >- $soonest = GetSoonestRenewDate($issue); >+ $soonest = GetSoonestRenewDate($patron, $issue); > if ( $soonest > dt_from_string() ){ > return (0, "too_soon", { soonest_renew_date => $soonest } ) unless $override_limit; > } >@@ -3290,7 +3288,8 @@ sub AddRenewal { > > sub GetRenewCount { > # check renewal status >- my ( $bornum, $itemno ) = @_; >+ my ( $borrowernumber_or_patron, $itemnumber_or_item ) = @_; >+ > my $dbh = C4::Context->dbh; > my $renewcount = 0; > my $unseencount = 0; >@@ -3298,9 +3297,10 @@ sub GetRenewCount { > my $unseenallowed = 0; > my $renewsleft = 0; > my $unseenleft = 0; >- >- my $patron = Koha::Patrons->find( $bornum ); >- my $item = Koha::Items->find($itemno); >+ my $patron = blessed $borrowernumber_or_patron ? >+ $borrowernumber_or_patron : Koha::Patrons->find($borrowernumber_or_patron); >+ my $item = blessed $itemnumber_or_item ? >+ $itemnumber_or_item : Koha::Items->find($itemnumber_or_item); > > return (0, 0, 0, 0, 0, 0) unless $patron or $item; # Wrong call, no renewal allowed > >@@ -3308,12 +3308,11 @@ sub GetRenewCount { > # and not yet returned. > > # FIXME - I think this function could be redone to use only one SQL call. >- my $sth = $dbh->prepare( >- "select * from issues >- where (borrowernumber = ?) >- and (itemnumber = ?)" >- ); >- $sth->execute( $bornum, $itemno ); >+ my $sth = $dbh->prepare(q{ >+ SELECT * FROM issues >+ WHERE (borrowernumber = ?) AND (itemnumber = ?) >+ }); >+ $sth->execute( $patron->borrowernumber, $item->itemnumber ); > my $data = $sth->fetchrow_hashref; > $renewcount = $data->{'renewals_count'} if $data->{'renewals_count'}; > $unseencount = $data->{'unseen_renewals'} if $data->{'unseen_renewals'}; >@@ -3348,11 +3347,13 @@ sub GetRenewCount { > > =head2 GetSoonestRenewDate > >- $NoRenewalBeforeThisDate = &GetSoonestRenewDate($checkout); >+ $NoRenewalBeforeThisDate = &GetSoonestRenewDate($patron, $issue); > > Find out the soonest possible renew date of a borrowed item. > >-C<$checkout> is the checkout object to renew. >+C<$patron> is the patron who currently has the item on loan. >+ >+C<$issue> is the the item issue. > > C<$GetSoonestRenewDate> returns the DateTime of the soonest possible > renew date, based on the value "No renewal before" of the applicable >@@ -3363,10 +3364,14 @@ cannot be found. > =cut > > sub GetSoonestRenewDate { >- my ( $checkout ) = @_; >+ my ( $patron, $issue ) = @_; >+ return unless $issue; >+ return unless $patron; > >- my $item = $checkout->item or return; >- my $patron = $checkout->patron or return; >+ my $item = $issue->item; >+ return unless $item; >+ >+ my $dbh = C4::Context->dbh; > > my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ); > my $issuing_rule = Koha::CirculationRules->get_effective_rules( >@@ -3386,7 +3391,7 @@ sub GetSoonestRenewDate { > and $issuing_rule->{norenewalbefore} ne "" ) > { > my $soonestrenewal = >- dt_from_string( $checkout->date_due )->subtract( >+ dt_from_string( $issue->date_due )->subtract( > $issuing_rule->{lengthunit} => $issuing_rule->{norenewalbefore} ); > > if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date' >@@ -3395,9 +3400,9 @@ sub GetSoonestRenewDate { > $soonestrenewal->truncate( to => 'day' ); > } > return $soonestrenewal if $now < $soonestrenewal; >- } elsif ( $checkout->auto_renew && $patron->autorenew_checkouts ) { >+ } elsif ( $issue->auto_renew && $patron->autorenew_checkouts ) { > # Checkouts with auto-renewing fall back to due date >- my $soonestrenewal = dt_from_string( $checkout->date_due ); >+ my $soonestrenewal = dt_from_string( $issue->date_due ); > if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date' > and $issuing_rule->{lengthunit} eq 'days' ) > { >@@ -3410,14 +3415,13 @@ sub GetSoonestRenewDate { > > =head2 GetLatestAutoRenewDate > >- $NoAutoRenewalAfterThisDate = &GetLatestAutoRenewDate($borrowernumber, $itemnumber); >+ $NoAutoRenewalAfterThisDate = &GetLatestAutoRenewDate($patron, $issue); > > Find out the latest possible auto renew date of a borrowed item. > >-C<$borrowernumber> is the borrower number of the patron who currently >-has the item on loan. >+C<$patron> is the patron who currently has the item on loan. > >-C<$itemnumber> is the number of the item to renew. >+C<$issue> is the item issue. > > C<$GetLatestAutoRenewDate> returns the DateTime of the latest possible > auto renew date, based on the value "No auto renewal after" and the "No auto >@@ -3428,16 +3432,14 @@ or item cannot be found. > =cut > > sub GetLatestAutoRenewDate { >- my ( $borrowernumber, $itemnumber ) = @_; >- >- my $dbh = C4::Context->dbh; >+ my ( $patron, $issue ) = @_; >+ return unless $issue; >+ return unless $patron; > >- my $item = Koha::Items->find($itemnumber) or return; >- my $itemissue = $item->checkout or return; >+ my $item = $issue->item; >+ return unless $item; > >- $borrowernumber ||= $itemissue->borrowernumber; >- my $patron = Koha::Patrons->find( $borrowernumber ) >- or return; >+ my $dbh = C4::Context->dbh; > > my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ); > my $circulation_rules = Koha::CirculationRules->get_effective_rules( >@@ -3462,7 +3464,7 @@ sub GetLatestAutoRenewDate { > > my $maximum_renewal_date; > if ( $circulation_rules->{no_auto_renewal_after} ) { >- $maximum_renewal_date = dt_from_string($itemissue->issuedate); >+ $maximum_renewal_date = dt_from_string($issue->issuedate); > $maximum_renewal_date->add( > $circulation_rules->{lengthunit} => $circulation_rules->{no_auto_renewal_after} > ); >@@ -3512,13 +3514,14 @@ sub GetIssuingCharges { > > my $sth = $dbh->prepare($charge_query); > $sth->execute($itemnumber); >+ my $patron; > if ( my $item_data = $sth->fetchrow_hashref ) { > $item_type = $item_data->{itemtype}; > $charge = $item_data->{rentalcharge}; > if ($charge) { > # FIXME This should follow CircControl > my $branch = C4::Context::mybranch(); >- my $patron = Koha::Patrons->find( $borrowernumber ); >+ $patron //= Koha::Patrons->find( $borrowernumber ); > my $discount = Koha::CirculationRules->get_effective_rule({ > categorycode => $patron->categorycode, > branchcode => $branch, >@@ -4456,7 +4459,7 @@ sub _CanBookBeAutoRenewed { > } > ); > >- if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) { >+ if ( $patron->is_expired && $patron->category->effective_BlockExpiredPatronOpacActions ) { > return 'auto_account_expired'; > } > >@@ -4492,7 +4495,7 @@ sub _CanBookBeAutoRenewed { > } > } > >- my $soonest = GetSoonestRenewDate($issue); >+ my $soonest = GetSoonestRenewDate($patron, $issue); > if ( $soonest > dt_from_string() ) > { > return ( "auto_too_soon", $soonest ); >diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm >index f6bf6bcc140..9327e3bafdc 100644 >--- a/C4/ILSDI/Services.pm >+++ b/C4/ILSDI/Services.pm >@@ -637,7 +637,7 @@ sub GetServices { > } > > # Renewal management >- my @renewal = CanBookBeRenewed( $borrowernumber, $itemnumber ); >+ my @renewal = CanBookBeRenewed( $patron, $item->checkout ); # TODO: Error if issue not found? > if ( $renewal[0] ) { > push @availablefor, 'loan renewal'; > } >@@ -681,16 +681,18 @@ sub RenewLoan { > return { code => 'PatronNotFound' } unless $patron; > > # Get the item, or return an error code >- my $itemnumber = $cgi->param('item_id'); >+ my $itemnumber = $cgi->param('item_id'); # TODO: Refactor and send issue_id instead? > my $item = Koha::Items->find($itemnumber); >+ > return { code => 'RecordNotFound' } unless $item; > >+ my $issue = $item->checkout; >+ return unless $issue; # FIXME should be handled >+ > # Add renewal if possible >- my @renewal = CanBookBeRenewed( $borrowernumber, $itemnumber ); >+ my @renewal = CanBookBeRenewed( $patron, $issue ); > if ( $renewal[0] ) { AddRenewal( $borrowernumber, $itemnumber, undef, undef, undef, undef, 0 ); } > >- my $issue = $item->checkout; >- return unless $issue; # FIXME should be handled > > # Hashref building > my $out; >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 84aa00e8648..8e64605a459 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -676,9 +676,9 @@ sub GetOtherReserves { > my ($itemnumber) = @_; > my $messages; > my $nextreservinfo; >- my ( undef, $checkreserves, undef ) = CheckReserves($itemnumber); >+ my $item = Koha::Items->find($itemnumber); >+ my ( undef, $checkreserves, undef ) = CheckReserves($item); > if ($checkreserves) { >- my $item = Koha::Items->find($itemnumber); > if ( $item->holdingbranch ne $checkreserves->{'branchcode'} ) { > $messages->{'transfert'} = $checkreserves->{'branchcode'}; > #minus priorities of others reservs >@@ -823,13 +823,12 @@ sub GetReserveStatus { > > =head2 CheckReserves > >- ($status, $matched_reserve, $possible_reserves) = &CheckReserves($itemnumber); >- ($status, $matched_reserve, $possible_reserves) = &CheckReserves(undef, $barcode); >- ($status, $matched_reserve, $possible_reserves) = &CheckReserves($itemnumber,undef,$lookahead); >+ ($status, $matched_reserve, $possible_reserves) = &CheckReserves($item); >+ ($status, $matched_reserve, $possible_reserves) = &CheckReserves($item, $lookahead); > > Find a book in the reserves. > >-C<$itemnumber> is the book's item number. >+C<$item> is the book's item. > C<$lookahead> is the number of days to look in advance for future reserves. > > As I understand it, C<&CheckReserves> looks for the given item in the >@@ -851,65 +850,32 @@ table in the Koha database. > =cut > > sub CheckReserves { >- my ( $item, $barcode, $lookahead_days, $ignore_borrowers) = @_; >- my $dbh = C4::Context->dbh; >- my $sth; >- my $select; >- if (C4::Context->preference('item-level_itypes')){ >- $select = " >- SELECT items.biblionumber, >- items.biblioitemnumber, >- itemtypes.notforloan, >- items.notforloan AS itemnotforloan, >- items.itemnumber, >- items.damaged, >- items.homebranch, >- items.holdingbranch >- FROM items >- LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber >- LEFT JOIN itemtypes ON items.itype = itemtypes.itemtype >- "; >- } >- else { >- $select = " >- SELECT items.biblionumber, >- items.biblioitemnumber, >- itemtypes.notforloan, >- items.notforloan AS itemnotforloan, >- items.itemnumber, >- items.damaged, >- items.homebranch, >- items.holdingbranch >- FROM items >- LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber >- LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype >- "; >- } >- >- if ($item) { >- $sth = $dbh->prepare("$select WHERE itemnumber = ?"); >- $sth->execute($item); >- } >- else { >- $sth = $dbh->prepare("$select WHERE barcode = ?"); >- $sth->execute($barcode); >- } >+ my ( $item, $lookahead_days, $ignore_borrowers ) = @_; > # note: we get the itemnumber because we might have started w/ just the barcode. Now we know for sure we have it. >- my ( $biblio, $bibitem, $notforloan_per_itemtype, $notforloan_per_item, $itemnumber, $damaged, $item_homebranch, $item_holdingbranch ) = $sth->fetchrow_array; >- return if ( $damaged && !C4::Context->preference('AllowHoldsOnDamagedItems') ); > >- return unless $itemnumber; # bail if we got nothing. >+ return unless $item; # bail if we got nothing. >+ >+ return if ( $item->damaged && !C4::Context->preference('AllowHoldsOnDamagedItems') ); >+ > # if item is not for loan it cannot be reserved either..... > # except where items.notforloan < 0 : This indicates the item is holdable. > > my @SkipHoldTrapOnNotForLoanValue = split( '\|', C4::Context->preference('SkipHoldTrapOnNotForLoanValue') ); >- return if grep { $_ eq $notforloan_per_item } @SkipHoldTrapOnNotForLoanValue; >+ return if grep { $_ eq $item->notforloan } @SkipHoldTrapOnNotForLoanValue; > >- my $dont_trap = C4::Context->preference('TrapHoldsOnOrder') ? ($notforloan_per_item > 0) : ($notforloan_per_item && 1 ); >- return if $dont_trap or $notforloan_per_itemtype; >+ my $dont_trap = C4::Context->preference('TrapHoldsOnOrder') ? $item->notforloan > 0 : $item->notforloan; >+ if ( !$dont_trap ) { >+ my $item_type = $item->effective_itemtype; >+ if ( $item_type ) { >+ return if Koha::ItemTypes->find( $item_type )->notforloan; >+ } >+ } >+ else { >+ return; >+ } > > # Find this item in the reserves >- my @reserves = _Findgroupreserve( $bibitem, $biblio, $itemnumber, $lookahead_days, $ignore_borrowers); >+ my @reserves = _Findgroupreserve( $item->biblionumber, $item->itemnumber, $lookahead_days, $ignore_borrowers); > > # $priority and $highest are used to find the most important item > # in the list returned by &_Findgroupreserve. (The lower $priority, >@@ -921,8 +887,8 @@ sub CheckReserves { > my $LocalHoldsPriority = C4::Context->preference('LocalHoldsPriority'); > my $LocalHoldsPriorityPatronControl = C4::Context->preference('LocalHoldsPriorityPatronControl'); > my $LocalHoldsPriorityItemControl = C4::Context->preference('LocalHoldsPriorityItemControl'); >- > my $priority = 10000000; >+ > foreach my $res (@reserves) { > if ($res->{'found'} && $res->{'found'} eq 'W') { > return ( "Waiting", $res, \@reserves ); # Found it, it is waiting >@@ -932,12 +898,10 @@ sub CheckReserves { > return ( "Transferred", $res, \@reserves ); # Found determinated hold, e. g. the transferred one > } else { > my $patron; >- my $item; > my $local_hold_match; > > if ($LocalHoldsPriority) { > $patron = Koha::Patrons->find( $res->{borrowernumber} ); >- $item = Koha::Items->find($itemnumber); > > unless ($item->exclude_from_local_holds_priority || $patron->category->exclude_from_local_holds_priority) { > my $local_holds_priority_item_branchcode = >@@ -956,10 +920,9 @@ sub CheckReserves { > > # See if this item is more important than what we've got so far > if ( ( $res->{'priority'} && $res->{'priority'} < $priority ) || $local_hold_match ) { >- $item ||= Koha::Items->find($itemnumber); > next if $res->{item_group_id} && ( !$item->item_group || $item->item_group->id != $res->{item_group_id} ); > next if $res->{itemtype} && $res->{itemtype} ne $item->effective_itemtype; >- $patron ||= Koha::Patrons->find( $res->{borrowernumber} ); >+ $patron //= Koha::Patrons->find( $res->{borrowernumber} ); > my $branch = GetReservesControlBranch( $item->unblessed, $patron->unblessed ); > my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$item->effective_itemtype); > next if ($branchitemrule->{'holdallowed'} eq 'not_allowed'); >@@ -982,7 +945,7 @@ sub CheckReserves { > # If we get this far, then no exact match was found. > # We return the most important (i.e. next) reservation. > if ($highest) { >- $highest->{'itemnumber'} = $item; >+ $highest->{'itemnumber'} = $item->itemnumber; > return ( "Reserved", $highest, \@reserves ); > } > >@@ -1710,7 +1673,7 @@ sub _FixPriority { > > =head2 _Findgroupreserve > >- @results = &_Findgroupreserve($biblioitemnumber, $biblionumber, $itemnumber, $lookahead, $ignore_borrowers); >+ @results = &_Findgroupreserve($biblionumber, $itemnumber, $lookahead, $ignore_borrowers); > > Looks for a holds-queue based item-specific match first, then for a holds-queue title-level match, returning the > first match found. If neither, then we look for non-holds-queue based holds. >@@ -1731,7 +1694,7 @@ All return values will respect any borrowernumbers passed as arrayref in $ignore > =cut > > sub _Findgroupreserve { >- my ( $bibitem, $biblio, $itemnumber, $lookahead, $ignore_borrowers) = @_; >+ my ( $biblionumber, $itemnumber, $lookahead, $ignore_borrowers) = @_; > my $dbh = C4::Context->dbh; > > # TODO: consolidate at least the SELECT portion of the first 2 queries to a common $select var. >@@ -1835,7 +1798,7 @@ sub _Findgroupreserve { > ORDER BY priority > }; > $sth = $dbh->prepare($query); >- $sth->execute( $biblio, $itemnumber, $lookahead||0); >+ $sth->execute( $biblionumber, $itemnumber, $lookahead||0); > @results = (); > while ( my $data = $sth->fetchrow_hashref ) { > push( @results, $data ) >@@ -2051,7 +2014,8 @@ sub MoveReserve { > $cancelreserve //= 0; > > my $lookahead = C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds >- my ( $restype, $res, undef ) = CheckReserves( $itemnumber, undef, $lookahead ); >+ my $item = Koha::Items->find($itemnumber); >+ my ( $restype, $res, undef ) = CheckReserves( $item, $lookahead ); > return unless $res; > > my $biblionumber = $res->{biblionumber}; >diff --git a/C4/RotatingCollections.pm b/C4/RotatingCollections.pm >index a1f8df740d6..18ba40a5c21 100644 >--- a/C4/RotatingCollections.pm >+++ b/C4/RotatingCollections.pm >@@ -25,7 +25,6 @@ package C4::RotatingCollections; > use Modern::Perl; > > use C4::Context; >-use C4::Reserves qw(CheckReserves); > use Koha::Database; > > use Try::Tiny qw( catch try ); >diff --git a/C4/SIP/ILS/Transaction/Checkin.pm b/C4/SIP/ILS/Transaction/Checkin.pm >index 004aa05efae..e512350be88 100644 >--- a/C4/SIP/ILS/Transaction/Checkin.pm >+++ b/C4/SIP/ILS/Transaction/Checkin.pm >@@ -90,7 +90,7 @@ sub do_checkin { > > my $reserved; > my $lookahead = C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds >- my ($resfound) = $item->withdrawn ? q{} : C4::Reserves::CheckReserves( $item->itemnumber, undef, $lookahead ); >+ my ($resfound) = $item->withdrawn ? q{} : CheckReserves( $item, $lookahead ); > if ( $resfound eq "Reserved") { > $reserved = 1; > } >diff --git a/C4/SIP/ILS/Transaction/Renew.pm b/C4/SIP/ILS/Transaction/Renew.pm >index 9e5579ee874..f48bee4cfe1 100644 >--- a/C4/SIP/ILS/Transaction/Renew.pm >+++ b/C4/SIP/ILS/Transaction/Renew.pm >@@ -31,8 +31,9 @@ sub new { > > sub do_renew_for { > my $self = shift; >- my $borrower = shift; >- my ($renewokay,$renewerror) = CanBookBeRenewed($borrower->{borrowernumber},$self->{item}->{itemnumber}); >+ my $patron = shift; >+ my $checkout = Koha::Checkouts->find({ itemnumber => $self->{item}->{itemnumber} }); >+ my ($renewokay,$renewerror) = CanBookBeRenewed($patron, $checkout); > if ($renewokay) { # ok so far check charges > my ($fee, undef) = GetIssuingCharges($self->{item}->{itemnumber}, $self->{patron}->{borrowernumber}); > if ($fee > 0) { >@@ -45,7 +46,7 @@ sub do_renew_for { > > } > if ($renewokay){ >- my $issue = AddIssue( $borrower, $self->{item}->id, undef, 0 ); >+ my $issue = AddIssue( $patron, $self->{item}->id, undef, 0 ); > $self->{due} = $self->duedatefromissue($issue, $self->{item}->{itemnumber}); > $self->renewal_ok(1); > } else { >@@ -53,6 +54,7 @@ sub do_renew_for { > $renewerror=~s/too_many/Item has reached maximum renewals/; > $renewerror=~s/too_unseen/Item has reached maximum consecutive renewals without being seen/; > $renewerror=~s/item_denied_renewal/Item renewal is not allowed/; >+ $renewerror=~s/item_issued_to_other_patron/Item already issued to other borrower/; > $self->screen_msg($renewerror); > $self->renewal_ok(0); > } >@@ -64,7 +66,7 @@ sub do_renew { > my $self = shift; > my $patron = Koha::Patrons->find( $self->{patron}->borrowernumber ); > $patron or return; # FIXME we should log that >- return $self->do_renew_for($patron->unblessed); >+ return $self->do_renew_for($patron); > } > > 1; >diff --git a/C4/SIP/ILS/Transaction/RenewAll.pm b/C4/SIP/ILS/Transaction/RenewAll.pm >index 5acd53efb7b..12cefea25c1 100644 >--- a/C4/SIP/ILS/Transaction/RenewAll.pm >+++ b/C4/SIP/ILS/Transaction/RenewAll.pm >@@ -33,12 +33,11 @@ sub new { > > sub do_renew_all { > my $self = shift; >- my $patron = $self->{patron}; # SIP's patron >- my $borrower = Koha::Patrons->find( { cardnumber => $patron->id } )->unblessed; # Koha's patron >+ my $patron = Koha::Patrons->find( $self->{patron}->{borrowernumber} ); > my $all_ok = 1; > $self->{renewed} = []; > $self->{unrenewed} = []; >- foreach my $itemx ( @{ $patron->{items} } ) { >+ foreach my $itemx ( @{ $self->{patron}->{items} } ) { > my $item_id = $itemx->{barcode}; > my $item = C4::SIP::ILS::Item->new($item_id); > if ( !defined($item) ) { >@@ -54,7 +53,7 @@ sub do_renew_all { > next; > } > $self->{item} = $item; >- $self->do_renew_for($borrower); >+ $self->do_renew_for($patron); > if ( $self->renewal_ok ) { > $item->{due_date} = $self->{due}; > push @{ $self->{renewed} }, $item_id; >diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm >index 6cfabeb9b6c..49fe0736adc 100644 >--- a/C4/SIP/Sip/MsgType.pm >+++ b/C4/SIP/Sip/MsgType.pm >@@ -1166,7 +1166,8 @@ sub handle_fee_paid { > "on_reserve" => "On hold for another patron", > "patron_restricted" => "Patron is currently restricted", > "item_denied_renewal" => "Item is not allowed renewal", >- "onsite_checkout" => "Item is an onsite checkout" >+ "onsite_checkout" => "Item is an onsite checkout", >+ "item_issued_to_other_patron" => "Item already issued to other borrower" > }; > my @success = (); > my @fail = (); >diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm >index 6ab6120a1a7..0313b1377c1 100644 >--- a/Koha/Account/Line.pm >+++ b/Koha/Account/Line.pm >@@ -20,7 +20,6 @@ use Modern::Perl; > use Data::Dumper qw( Dumper ); > > use C4::Log qw( logaction ); >-use C4::Overdues qw( UpdateFine ); > > use Koha::Account::CreditType; > use Koha::Account::DebitType; >@@ -1023,12 +1022,12 @@ sub renew_item { > } > > my $itemnumber = $self->item->itemnumber; >- my $borrowernumber = $self->patron->borrowernumber; > my ( $can_renew, $error ) = C4::Circulation::CanBookBeRenewed( >- $borrowernumber, >- $itemnumber >+ $self->patron, >+ $self->item->checkout > ); > if ( $can_renew ) { >+ my $borrowernumber = $self->patron->borrowernumber; > my $due_date = C4::Circulation::AddRenewal( > $borrowernumber, > $itemnumber, >diff --git a/Koha/Hold.pm b/Koha/Hold.pm >index a5a9d03003f..6d846a5026a 100644 >--- a/Koha/Hold.pm >+++ b/Koha/Hold.pm >@@ -762,7 +762,7 @@ sub cancel { > ); > > if ($autofill_next) { >- my ( undef, $next_hold ) = C4::Reserves::CheckReserves( $self->itemnumber ); >+ my ( undef, $next_hold ) = C4::Reserves::CheckReserves( $self->item ); > if ($next_hold) { > my $is_transfer = $self->branchcode ne $next_hold->{branchcode}; > >diff --git a/Koha/REST/V1/Checkouts.pm b/Koha/REST/V1/Checkouts.pm >index e4a52b4cadb..7eea31d03bc 100644 >--- a/Koha/REST/V1/Checkouts.pm >+++ b/Koha/REST/V1/Checkouts.pm >@@ -22,7 +22,7 @@ use Mojo::JSON; > > use C4::Auth qw( haspermission ); > use C4::Context; >-use C4::Circulation qw( AddRenewal ); >+use C4::Circulation qw( AddRenewal CanBookBeRenewed ); > use Koha::Checkouts; > use Koha::Old::Checkouts; > >@@ -156,11 +156,7 @@ sub renew { > } > > return try { >- my $borrowernumber = $checkout->borrowernumber; >- my $itemnumber = $checkout->itemnumber; >- >- my ($can_renew, $error) = C4::Circulation::CanBookBeRenewed( >- $borrowernumber, $itemnumber); >+ my ($can_renew, $error) = CanBookBeRenewed($checkout->patron, $checkout); > > if (!$can_renew) { > return $c->render( >@@ -170,8 +166,8 @@ sub renew { > } > > AddRenewal( >- $borrowernumber, >- $itemnumber, >+ $checkout->borrowernumber, >+ $checkout->itemnumber, > $checkout->branchcode, > undef, > undef, >@@ -210,8 +206,7 @@ sub allows_renewal { > } > > return try { >- my ($can_renew, $error) = C4::Circulation::CanBookBeRenewed( >- $checkout->borrowernumber, $checkout->itemnumber); >+ my ($can_renew, $error) = CanBookBeRenewed($checkout->patron, $checkout); > > my $renewable = Mojo::JSON->false; > $renewable = Mojo::JSON->true if $can_renew; >diff --git a/circ/renew.pl b/circ/renew.pl >index f495a4061f2..f9434a127b1 100755 >--- a/circ/renew.pl >+++ b/circ/renew.pl >@@ -48,28 +48,27 @@ my $override_limit = $cgi->param('override_limit'); > my $override_holds = $cgi->param('override_holds'); > my $hard_due_date = $cgi->param('hard_due_date'); > >-my ( $item, $issue, $borrower ); >+my ( $item, $checkout, $patron ); > my $error = q{}; > my ( $soonest_renew_date, $latest_auto_renew_date ); > > if ($barcode) { > $barcode = barcodedecode($barcode) if $barcode; >- $item = $schema->resultset("Item")->single( { barcode => $barcode } ); >+ $item = Koha::Items->find({ barcode => $barcode }); > > if ($item) { > >- $issue = $item->issue(); >+ $checkout = $item->checkout; > >- if ($issue) { >+ if ($checkout) { > >- $borrower = $issue->patron(); >- >- if ( ( $borrower->debarred() || q{} ) lt dt_from_string()->ymd() ) { >+ $patron = $checkout->patron; >+ >+ if ( ( $patron->is_debarred || q{} ) lt dt_from_string()->ymd() ) { > my $can_renew; > my $info; > ( $can_renew, $error, $info ) = >- CanBookBeRenewed( $borrower->borrowernumber(), >- $item->itemnumber(), $override_limit ); >+ CanBookBeRenewed( $patron, $checkout, $override_limit ); > > if ( $error && ($error eq 'on_reserve') ) { > if ($override_holds) { >@@ -85,9 +84,9 @@ if ($barcode) { > $soonest_renew_date = $info->{soonest_renew_date}; > } > if ( $error && ( $error eq 'auto_too_late' ) ) { >- $latest_auto_renew_date = C4::Circulation::GetLatestAutoRenewDate( >- $borrower->borrowernumber(), >- $item->itemnumber(), >+ $latest_auto_renew_date = GetLatestAutoRenewDate( >+ $patron, >+ $checkout, > ); > } > if ($can_renew) { >@@ -124,8 +123,8 @@ if ($barcode) { > > $template->param( > item => $item, >- issue => $issue, >- borrower => $borrower, >+ issue => $checkout, >+ borrower => $patron, > error => $error, > soonestrenewdate => $soonest_renew_date, > latestautorenewdate => $latest_auto_renew_date, >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/renew_strings.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/renew_strings.inc >index fd81f1240a1..9fb85b437c7 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/renew_strings.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/renew_strings.inc >@@ -27,6 +27,8 @@ > <span>Item is an onsite checkout</span> > [% CASE 'has_fine' %] > <span>Item has an outstanding fine</span> >+[% CASE 'item_issued_to_other_patron'%] >+ <span>Item already issued to other borrower</span> > [% CASE %] > <span>Unknown error</span> > [% END %] >diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt >index ebaecc5d3dc..d19377b6d32 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt >@@ -108,6 +108,8 @@ > <li>This item is on hold for another borrower.</li> > [% ELSIF error == 'item_denied_renewal' %] > <li>Item renewal is not allowed.</li> >+ [% ELSIF error == 'item_issued_to_other_patron'%] >+ <li>Item already issued to other borrower.</li> > [% ELSIF error == 'auto_too_soon' %] > <li>This item is scheduled for auto renewal.</li> > [% END %] >diff --git a/misc/cronjobs/automatic_renewals.pl b/misc/cronjobs/automatic_renewals.pl >index bf048c09d01..04488bf6e9e 100755 >--- a/misc/cronjobs/automatic_renewals.pl >+++ b/misc/cronjobs/automatic_renewals.pl >@@ -172,7 +172,7 @@ while ( my $auto_renew = $auto_renews->next ) { > } > > # CanBookBeRenewed returns 'auto_renew' when the renewal should be done by this script >- my ( $ok, $error ) = CanBookBeRenewed( $auto_renew->borrowernumber, $auto_renew->itemnumber, undef, 1 ); >+ my ( $ok, $error ) = CanBookBeRenewed( $auto_renew->patron, $auto_renew, undef, 1 ); > my $updated; > if ( $error eq 'auto_renew' ) { > $updated = 1; >@@ -187,16 +187,19 @@ while ( my $auto_renew = $auto_renews->next ) { > } > push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew > if ( $wants_messages ) && !$wants_digest; >- } elsif ( $error eq 'too_many' >- or $error eq 'on_reserve' >- or $error eq 'restriction' >- or $error eq 'overdue' >- or $error eq 'too_unseen' >- or $error eq 'auto_account_expired' >- or $error eq 'auto_too_late' >- or $error eq 'auto_too_much_oweing' >- or $error eq 'auto_too_soon' >- or $error eq 'item_denied_renewal' ) { >+ } elsif ( >+ $error eq 'too_many' || >+ $error eq 'on_reserve' || >+ $error eq 'restriction' || >+ $error eq 'overdue' || >+ $error eq 'too_unseen' || >+ $error eq 'auto_account_expired' || >+ $error eq 'auto_too_late' || >+ $error eq 'auto_too_much_oweing' || >+ $error eq 'auto_too_soon' || >+ $error eq 'item_denied_renewal' || >+ $error eq 'item_issued_to_other_patron' >+ ) { > if ( $verbose ) { > say sprintf "Issue id: %s for borrower: %s and item: %s %s not be renewed. (%s)", > $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber, $confirm ? 'will' : 'would', $error; >diff --git a/opac/opac-renew.pl b/opac/opac-renew.pl >index 46a205029e8..9e8efa0c5e9 100755 >--- a/opac/opac-renew.pl >+++ b/opac/opac-renew.pl >@@ -56,8 +56,9 @@ if ( $patron->category->effective_BlockExpiredPatronOpacActions > else { > my @renewed; > for my $itemnumber (@items) { >+ my $item = Koha::Items->find($itemnumber); > my ( $status, $error ) = >- CanBookBeRenewed( $borrowernumber, $itemnumber ); >+ CanBookBeRenewed( $patron, $item->checkout ); #TODO: Pass issue numbers instead > if ( $status == 1 && $opacrenew == 1 ) { > AddRenewal( $borrowernumber, $itemnumber, undef, undef, undef, undef, 0 ); > push( @renewed, $itemnumber ); >diff --git a/opac/opac-user.pl b/opac/opac-user.pl >index 494f481224d..b39b7493c0f 100755 >--- a/opac/opac-user.pl >+++ b/opac/opac-user.pl >@@ -193,7 +193,13 @@ my $overdues_count = 0; > my @overdues; > my @issuedat; > my $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search_with_localization->unblessed } }; >-my $pending_checkouts = $patron->pending_checkouts->search({}, { order_by => [ { -desc => 'date_due' }, { -asc => 'issue_id' } ] }); >+my $pending_checkouts = $patron->pending_checkouts->search( >+ {}, >+ { >+ order_by => [ { -desc => 'date_due' }, { -asc => 'issue_id' } ], >+ prefetch => 'item' >+ } >+); > my $are_renewable_items = 0; > if ( $pending_checkouts->count ) { # Useless test > while ( my $c = $pending_checkouts->next ) { >@@ -226,7 +232,7 @@ if ( $pending_checkouts->count ) { # Useless test > $issue->{rentalfines} = $rental_fines->total_outstanding; > > # check if item is renewable >- my ($status,$renewerror,$info) = CanBookBeRenewed( $borrowernumber, $issue->{'itemnumber'} ); >+ my ($status, $renewerror, $info) = CanBookBeRenewed( $patron, $c ); > ( > $issue->{'renewcount'}, > $issue->{'renewsallowed'}, >@@ -236,7 +242,7 @@ if ( $pending_checkouts->count ) { # Useless test > $issue->{'unseenleft'} > ) = GetRenewCount($borrowernumber, $issue->{'itemnumber'}); > ( $issue->{'renewalfee'}, $issue->{'renewalitemtype'} ) = GetIssuingCharges( $issue->{'itemnumber'}, $borrowernumber ); >- $issue->{itemtype_object} = Koha::ItemTypes->find( Koha::Items->find( $issue->{itemnumber} )->effective_itemtype ); >+ $issue->{itemtype_object} = Koha::ItemTypes->find( $c->item->effective_itemtype ); > if($status && C4::Context->preference("OpacRenewalAllowed")){ > $are_renewable_items = 1; > $issue->{'status'} = $status; >@@ -254,6 +260,7 @@ if ( $pending_checkouts->count ) { # Useless test > $issue->{'auto_too_late'} = 1 if $renewerror eq 'auto_too_late'; > $issue->{'auto_too_much_oweing'} = 1 if $renewerror eq 'auto_too_much_oweing'; > $issue->{'item_denied_renewal'} = 1 if $renewerror eq 'item_denied_renewal'; >+ $issue->{'item_issued_to_other_patron'} = 1 if $renewerror eq 'item_issued_to_other_patron'; > > if ( $renewerror eq 'too_soon' ) { > $issue->{'too_soon'} = 1; >diff --git a/opac/sco/sco-main.pl b/opac/sco/sco-main.pl >index ac89db749cc..e36db3d68b2 100755 >--- a/opac/sco/sco-main.pl >+++ b/opac/sco/sco-main.pl >@@ -279,7 +279,7 @@ if ( $patron && ( $op eq 'renew' ) ) { > my $item = Koha::Items->find({ barcode => $barcode }); > > if ( $patron->checkouts->find( { itemnumber => $item->itemnumber } ) ) { >- my ($status,$renewerror) = CanBookBeRenewed( $patron->borrowernumber, $item->itemnumber ); >+ my ($status,$renewerror) = CanBookBeRenewed( $patron, $item->checkout ); > if ($status) { > AddRenewal( $patron->borrowernumber, $item->itemnumber, undef, undef, undef, undef, 1 ); > push @newissueslist, $barcode; >@@ -296,10 +296,7 @@ if ( $patron) { > my @checkouts; > while ( my $c = $pending_checkouts->next ) { > my $checkout = $c->unblessed_all_relateds; >- my ($can_be_renewed, $renew_error) = CanBookBeRenewed( >- $patron->borrowernumber, >- $checkout->{itemnumber}, >- ); >+ my ($can_be_renewed, $renew_error) = CanBookBeRenewed( $patron, $c ); > $checkout->{can_be_renewed} = $can_be_renewed; # In the future this will be $checkout->can_be_renewed > $checkout->{renew_error} = $renew_error; > $checkout->{overdue} = $c->is_overdue; >diff --git a/svc/checkouts b/svc/checkouts >index b8e84f4704c..bf5399aea20 100755 >--- a/svc/checkouts >+++ b/svc/checkouts >@@ -67,6 +67,7 @@ print $input->header( -type => 'text/plain', -charset => 'UTF-8' ); > my @parameters; > my $sql = ' > SELECT >+ issues.issue_id, > issues.issuedate, > issues.date_due, > issues.date_due < now() as date_due_overdue, >@@ -154,8 +155,9 @@ while ( my $c = $sth->fetchrow_hashref() ) { > my ($charge) = GetIssuingCharges( $c->{itemnumber}, $c->{borrowernumber} ); > my $fine = GetFine( $c->{itemnumber}, $c->{borrowernumber} ); > >+ my $checkout_obj = Koha::Checkouts->find( $c->{issue_id} ); > my ( $can_renew, $can_renew_error, $info ) = >- CanBookBeRenewed( $c->{borrowernumber}, $c->{itemnumber} ); >+ CanBookBeRenewed( $checkout_obj->patron, $checkout_obj ); > my $can_renew_date = > $can_renew_error && $can_renew_error eq 'too_soon' > ? output_pref( >diff --git a/svc/renew b/svc/renew >index 470c02e7969..fb09023643c 100755 >--- a/svc/renew >+++ b/svc/renew >@@ -58,8 +58,11 @@ $data->{itemnumber} = $itemnumber; > $data->{borrowernumber} = $borrowernumber; > $data->{branchcode} = $branchcode; > >+my $patron = Koha::Patrons->find($borrowernumber); >+my $item = Koha::Items->find($itemnumber); >+ > ( $data->{renew_okay}, $data->{error} ) = >- CanBookBeRenewed( $borrowernumber, $itemnumber, $override_limit ); >+ CanBookBeRenewed( $patron, $item->checkout, $override_limit ); > > # If we're allowing reserved items to be renewed... > if ( $data->{error} && $data->{error} eq 'on_reserve' && C4::Context->preference('AllowRenewalOnHoldOverride')) { >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 1bbefe78a4e..3cd4ded868c 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -310,7 +310,7 @@ subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers a > ); > > # Patrons from three different branches >- my $patron_borrower = $builder->build_object({ class => 'Koha::Patrons' }); >+ my $patron = $builder->build_object({ class => 'Koha::Patrons' }); > my $patron_hold_1 = $builder->build_object({ class => 'Koha::Patrons' }); > my $patron_hold_2 = $builder->build_object({ class => 'Koha::Patrons' }); > my $biblio = $builder->build_sample_biblio(); >@@ -318,7 +318,7 @@ subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers a > # Item at each patron branch > my $item_1 = $builder->build_sample_item({ > biblionumber => $biblio->biblionumber, >- homebranch => $patron_borrower->branchcode >+ homebranch => $patron->branchcode > }); > my $item_2 = $builder->build_sample_item({ > biblionumber => $biblio->biblionumber, >@@ -329,7 +329,7 @@ subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers a > homebranch => $patron_hold_1->branchcode > }); > >- my $issue = AddIssue( $patron_borrower->unblessed, $item_1->barcode); >+ my $issue = AddIssue( $patron->unblessed, $item_1->barcode); > my $datedue = dt_from_string( $issue->date_due() ); > is (defined $issue->date_due(), 1, "Item 1 checked out, due date: " . $issue->date_due() ); > >@@ -360,13 +360,13 @@ subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers a > ); > t::lib::Mocks::mock_preference('AllowRenewalIfOtherItemsAvailable', 0 ); > >- my ( $renewokay, $error ) = CanBookBeRenewed($patron_borrower->borrowernumber, $item_1->itemnumber); >+ my ( $renewokay, $error ) = CanBookBeRenewed($patron, $issue); > is( $renewokay, 0, 'Cannot renew, reserved'); > is( $error, 'on_reserve', 'Cannot renew, reserved (returned error is on_reserve)'); > > t::lib::Mocks::mock_preference('AllowRenewalIfOtherItemsAvailable', 1 ); > >- ( $renewokay, $error ) = CanBookBeRenewed($patron_borrower->borrowernumber, $item_1->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($patron, $issue); > is( $renewokay, 1, 'Can renew, two items available for two holds'); > is( $error, undef, 'Can renew, each reserve has an item'); > >@@ -374,10 +374,9 @@ subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers a > my $hold = Koha::Holds->find( $reserve_1 ); > $hold->itemnumber( $item_1->itemnumber )->store; > >- ( $renewokay, $error ) = CanBookBeRenewed($patron_borrower->borrowernumber, $item_1->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($patron, $issue); > is( $renewokay, 0, 'Cannot renew when there is an item specific hold'); > is( $error, 'on_reserve', 'Cannot renew, only this item can fill the reserve'); >- > }; > > subtest "GetIssuingCharges tests" => sub { >@@ -470,6 +469,7 @@ subtest "CanBookBeRenewed tests" => sub { > surname => 'Renewal', > categorycode => $patron_category->{categorycode}, > branchcode => $branch, >+ autorenew_checkouts => 1, > ); > > my %reserving_borrower_data = ( >@@ -500,18 +500,16 @@ subtest "CanBookBeRenewed tests" => sub { > categorycode => $patron_category->{categorycode}, > branchcode => $branch, > dateexpiry => dt_from_string->subtract( months => 1 ), >+ autorenew_checkouts => 1, > ); > >- my $renewing_borrowernumber = Koha::Patron->new(\%renewing_borrower_data)->store->borrowernumber; >+ my $renewing_borrower_obj = Koha::Patron->new(\%renewing_borrower_data)->store; >+ my $renewing_borrowernumber = $renewing_borrower_obj->borrowernumber; > my $reserving_borrowernumber = Koha::Patron->new(\%reserving_borrower_data)->store->borrowernumber; > my $hold_waiting_borrowernumber = Koha::Patron->new(\%hold_waiting_borrower_data)->store->borrowernumber; >- my $restricted_borrowernumber = Koha::Patron->new(\%restricted_borrower_data)->store->borrowernumber; >- my $expired_borrowernumber = Koha::Patron->new(\%expired_borrower_data)->store->borrowernumber; >+ my $restricted_borrower_obj = Koha::Patron->new(\%restricted_borrower_data)->store; > >- my $renewing_borrower_obj = Koha::Patrons->find( $renewing_borrowernumber ); >- my $renewing_borrower = $renewing_borrower_obj->unblessed; >- my $restricted_borrower = Koha::Patrons->find( $restricted_borrowernumber )->unblessed; >- my $expired_borrower = Koha::Patrons->find( $expired_borrowernumber )->unblessed; >+ my $expired_borrower_obj = Koha::Patron->new(\%expired_borrower_data)->store; > > my $bibitems = ''; > my $priority = '1'; >@@ -521,19 +519,17 @@ subtest "CanBookBeRenewed tests" => sub { > my $checkitem = undef; > my $found = undef; > >- my $issue = AddIssue( $renewing_borrower, $item_1->barcode); >- my $datedue = dt_from_string( $issue->date_due() ); >- is (defined $issue->date_due(), 1, "Item 1 checked out, due date: " . $issue->date_due() ); >- >- my $issue2 = AddIssue( $renewing_borrower, $item_2->barcode); >- $datedue = dt_from_string( $issue->date_due() ); >- is (defined $issue2, 1, "Item 2 checked out, due date: " . $issue2->date_due()); >+ my $issue_1 = AddIssue( $renewing_borrower_obj->unblessed, $item_1->barcode); >+ my $datedue = dt_from_string( $issue_1->date_due() ); >+ is (defined $issue_1->date_due(), 1, "Item 1 checked out, due date: " . $issue_1->date_due() ); > >+ my $issue_2 = AddIssue( $renewing_borrower_obj->unblessed, $item_2->barcode); >+ is (defined $issue_2, 1, "Item 2 checked out, due date: " . $issue_2->date_due()); > > my $borrowing_borrowernumber = Koha::Checkouts->find( { itemnumber => $item_1->itemnumber } )->borrowernumber; >- is ($borrowing_borrowernumber, $renewing_borrowernumber, "Item checked out to $renewing_borrower->{firstname} $renewing_borrower->{surname}"); >+ is ($borrowing_borrowernumber, $renewing_borrowernumber, "Item checked out to ".$renewing_borrower_obj->firstname." ".$renewing_borrower_obj->surname); > >- my ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1); >+ my ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1, 1); > is( $renewokay, 1, 'Can renew, no holds for this title or item'); > > >@@ -572,9 +568,9 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > t::lib::Mocks::mock_preference('AllowRenewalIfOtherItemsAvailable', 1 ); >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); > is( $renewokay, 1, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_2->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_2); > is( $renewokay, 1, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); > > >@@ -592,7 +588,7 @@ subtest "CanBookBeRenewed tests" => sub { > found => $found, > } > ); >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); > is( $renewokay, 0, 'Renewal not possible when single patron\'s holds exceed the number of available items'); > Koha::Holds->find($reserve_id)->delete; > >@@ -607,7 +603,7 @@ subtest "CanBookBeRenewed tests" => sub { > reservedate => '1999-01-01', > } > ); >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); > is( $renewokay, 0, 'Bug 13919 - Renewal possible with item level hold on item'); > $hold->delete(); > >@@ -623,17 +619,17 @@ subtest "CanBookBeRenewed tests" => sub { > found => 'W' > } > ); >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); > is( $renewokay, 0, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_2->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_2); > is( $renewokay, 0, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); > t::lib::Mocks::mock_preference('AllowRenewalIfOtherItemsAvailable', 0 ); > >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); > is( $renewokay, 0, '(Bug 10663) Cannot renew, reserved'); > is( $error, 'on_reserve', '(Bug 10663) Cannot renew, reserved (returned error is on_reserve)'); > >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_2->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_2); > is( $renewokay, 0, '(Bug 10663) Cannot renew, reserved'); > is( $error, 'on_reserve', '(Bug 10663) Cannot renew, reserved (returned error is on_reserve)'); > >@@ -662,16 +658,16 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1, 1); > is( $renewokay, 0, '(Bug 10663) Cannot renew, item reserved'); > is( $error, 'on_reserve', '(Bug 10663) Cannot renew, item reserved (returned error is on_reserve)'); > >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_2->itemnumber, 1); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_2, 1); > is( $renewokay, 1, 'Can renew item 2, item-level hold is on item 1'); > > # Items can't fill hold for reasons >- $item_1->notforloan(1)->store; >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1); >+ $issue_1->item->notforloan(1)->store; >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1, 1); > is( $renewokay, 0, 'Cannot renew, item is marked not for loan, but an item specific hold always blocks'); > $item_1->set({notforloan => 0, itype => $itemtype })->store; > >@@ -686,13 +682,13 @@ subtest "CanBookBeRenewed tests" => sub { > itype => $itemtype, > } > ); >- my $datedue5 = AddIssue($restricted_borrower, $item_5->barcode); >- is (defined $datedue5, 1, "Item with date due checked out, due date: $datedue5"); >+ my $issue_5 = AddIssue($restricted_borrower_obj->unblessed, $item_5->barcode); >+ is (defined $issue_5, 1, "Item with date due checked out, due date: ". $issue_5->date_due); > > t::lib::Mocks::mock_preference('RestrictionBlockRenewing','1'); >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_2->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_2); > is( $renewokay, 1, '(Bug 8236), Can renew, user is not restricted'); >- ( $renewokay, $error ) = CanBookBeRenewed($restricted_borrowernumber, $item_5->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($restricted_borrower_obj, $issue_5); > is( $renewokay, 0, '(Bug 8236), Cannot renew, user is restricted'); > is( $error, 'restriction', "Correct error returned"); > >@@ -715,44 +711,44 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > >- my $datedue6 = AddIssue( $renewing_borrower, $item_6->barcode); >- is (defined $datedue6, 1, "Item 2 checked out, due date: ".$datedue6->date_due); >+ my $issue_6 = AddIssue( $renewing_borrower_obj->unblessed, $item_6->barcode); >+ is (defined $issue_6, 1, "Item 2 checked out, due date: ".$issue_6->date_due); > > my $now = dt_from_string(); > my $five_weeks = DateTime::Duration->new(weeks => 5); > my $five_weeks_ago = $now - $five_weeks; > t::lib::Mocks::mock_preference('finesMode', 'production'); > >- my $passeddatedue1 = AddIssue($renewing_borrower, $item_7->barcode, $five_weeks_ago); >- is (defined $passeddatedue1, 1, "Item with passed date due checked out, due date: " . $passeddatedue1->date_due); >+ my $issue_7 = AddIssue($renewing_borrower_obj->unblessed, $item_7->barcode, $five_weeks_ago); >+ is (defined $issue_7, 1, "Item with passed date due checked out, due date: " . $issue_7->date_due); > > t::lib::Mocks::mock_preference('OverduesBlockRenewing','allow'); >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_6); > is( $renewokay, 1, '(Bug 8236), Can renew, this item is not overdue'); >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_7); > is( $renewokay, 1, '(Bug 8236), Can renew, this item is overdue but not pref does not block'); > > t::lib::Mocks::mock_preference('OverduesBlockRenewing','block'); >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_6); > is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is not overdue but patron has overdues'); > is( $error, 'overdue', "Correct error returned"); >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_7); > is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue so patron has overdues'); > is( $error, 'overdue', "Correct error returned"); > > t::lib::Mocks::mock_preference('OverduesBlockRenewing','blockitem'); >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_6); > is( $renewokay, 1, '(Bug 8236), Can renew, this item is not overdue'); >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_7); > is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue'); > is( $error, 'overdue', "Correct error returned"); > >- my ( $fine ) = CalcFine( $item_7->unblessed, $renewing_borrower->{categorycode}, $branch, $five_weeks_ago, $now ); >+ my ( $fine ) = CalcFine( $item_7->unblessed, $renewing_borrower_obj->categorycode, $branch, $five_weeks_ago, $now ); > C4::Overdues::UpdateFine( > { >- issue_id => $passeddatedue1->id(), >+ issue_id => $issue_7->id(), > itemnumber => $item_7->itemnumber, >- borrowernumber => $renewing_borrower->{borrowernumber}, >+ borrowernumber => $renewing_borrower_obj->borrowernumber, > amount => $fine, > due => Koha::DateUtils::output_pref($five_weeks_ago) > } >@@ -783,7 +779,7 @@ subtest "CanBookBeRenewed tests" => sub { > my $old_log_size = Koha::ActionLogs->count( \%params_renewal ); > my $dt = dt_from_string(); > Time::Fake->offset( $dt->epoch ); >- my $datedue1 = AddRenewal( $renewing_borrower->{borrowernumber}, $item_7->itemnumber, $branch ); >+ my $datedue1 = AddRenewal( $renewing_borrower_obj->borrowernumber, $item_7->itemnumber, $branch ); > my $new_log_size = Koha::ActionLogs->count( \%params_renewal ); > is ($new_log_size, $old_log_size, 'renew log not added because of the syspref RenewalLog'); > isnt (DateTime->compare($datedue1, $dt), 0, "AddRenewal returned a good duedate"); >@@ -792,18 +788,26 @@ subtest "CanBookBeRenewed tests" => sub { > t::lib::Mocks::mock_preference('RenewalLog', 1); > $date = output_pref( { dt => dt_from_string(), dateonly => 1, dateformat => 'iso' } ); > $old_log_size = Koha::ActionLogs->count( \%params_renewal ); >- AddRenewal( $renewing_borrower->{borrowernumber}, $item_7->itemnumber, $branch ); >+ AddRenewal( $renewing_borrower_obj->borrowernumber, $item_7->itemnumber, $branch ); > $new_log_size = Koha::ActionLogs->count( \%params_renewal ); > is ($new_log_size, $old_log_size + 1, 'renew log successfully added'); > >- my $fines = Koha::Account::Lines->search( { borrowernumber => $renewing_borrower->{borrowernumber}, itemnumber => $item_7->itemnumber } ); >+ my $fines = Koha::Account::Lines->search( { borrowernumber => $renewing_borrower_obj->borrowernumber, itemnumber => $item_7->itemnumber } ); > is( $fines->count, 1, 'AddRenewal left fine' ); > is( $fines->next->status, 'RENEWED', 'Fine on renewed item is closed out properly' ); > $fines->delete(); > > my $old_issue_log_size = Koha::ActionLogs->count( \%params_issue ); > my $old_renew_log_size = Koha::ActionLogs->count( \%params_renewal ); >- AddIssue( $renewing_borrower,$item_7->barcode,Koha::DateUtils::output_pref({str=>$datedue6->date_due, dateformat =>'iso'}),0,$date, 0, undef ); >+ AddIssue( >+ $renewing_borrower_obj->unblessed, >+ $item_7->barcode, >+ Koha::DateUtils::output_pref({str=>$issue_6->date_due, dateformat =>'iso'}), >+ 0, >+ $date, >+ 0, >+ undef >+ ); # TODO: Already issued??? > $new_log_size = Koha::ActionLogs->count( \%params_renewal ); > is ($new_log_size, $old_renew_log_size + 1, 'renew log successfully added when renewed via issuing'); > $new_log_size = Koha::ActionLogs->count( \%params_issue ); >@@ -824,14 +828,14 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > >- $issue = AddIssue( $renewing_borrower, $item_4->barcode, undef, undef, undef, undef, { auto_renew => 1 } ); >+ my $issue_4 = AddIssue( $renewing_borrower_obj->unblessed, $item_4->barcode, undef, undef, undef, undef, { auto_renew => 1 } ); > my $info; > ( $renewokay, $error, $info ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); > is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic and premature' ); > is( $error, 'auto_too_soon', > 'Bug 14101: Cannot renew, renewal is automatic and premature, "No renewal before" = undef (returned code is auto_too_soon)' ); >- is( $info->{soonest_renew_date} , dt_from_string($issue->date_due), "Due date is returned as earliest renewal date when error is 'auto_too_soon'" ); >+ is( $info->{soonest_renew_date} , dt_from_string($issue_4->date_due), "Due date is returned as earliest renewal date when error is 'auto_too_soon'" ); > AddReserve( > { > branchcode => $branch, >@@ -847,30 +851,29 @@ subtest "CanBookBeRenewed tests" => sub { > found => $found > } > ); >- ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); > is( $renewokay, 0, 'Still should not be able to renew' ); > is( $error, 'on_reserve', 'returned code is on_reserve, reserve checked when not checking for cron' ); >- ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, undef, 1 ); >+ ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4, undef, 1 ); > is( $renewokay, 0, 'Still should not be able to renew' ); > is( $error, 'auto_too_soon', 'returned code is auto_too_soon, reserve not checked when checking for cron' ); >- is( $info->{soonest_renew_date}, dt_from_string($issue->date_due), "Due date is returned as earliest renewal date when error is 'auto_too_soon'" ); >- ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1 ); >+ is( $info->{soonest_renew_date}, dt_from_string($issue_4->date_due), "Due date is returned as earliest renewal date when error is 'auto_too_soon'" ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4, 1 ); > is( $renewokay, 0, 'Still should not be able to renew' ); > is( $error, 'on_reserve', 'returned code is on_reserve, auto_too_soon limit is overridden' ); >- ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1, 1 ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4, 1, 1 ); > is( $renewokay, 0, 'Still should not be able to renew' ); > is( $error, 'on_reserve', 'returned code is on_reserve, auto_too_soon limit is overridden' ); > $dbh->do('UPDATE circulation_rules SET rule_value = 0 where rule_name = "norenewalbefore"'); > Koha::Cache::Memory::Lite->flush(); >- ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1 ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4, 1 ); > is( $renewokay, 0, 'Still should not be able to renew' ); > is( $error, 'on_reserve', 'returned code is on_reserve, auto_renew only happens if not on reserve' ); > ModReserveCancelAll($item_4->itemnumber, $reserving_borrowernumber); > >- >- >+ $renewing_borrower_obj = Koha::Patrons->find($renewing_borrower_obj->borrowernumber); > $renewing_borrower_obj->autorenew_checkouts(0)->store; >- ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); > is( $renewokay, 1, 'No renewal before is undef, but patron opted out of auto_renewal' ); > $renewing_borrower_obj->autorenew_checkouts(1)->store; > >@@ -887,26 +890,26 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > >- ( $renewokay, $error, $info ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); >+ ( $renewokay, $error, $info ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); > is( $renewokay, 0, 'Bug 7413: Cannot renew, renewal is premature'); > is( $error, 'too_soon', 'Bug 7413: Cannot renew, renewal is premature (returned code is too_soon)'); >- is( $info->{soonest_renew_date}, dt_from_string($issue->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'too_soon'"); >+ is( $info->{soonest_renew_date}, dt_from_string($issue_1->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'too_soon'"); > > # Bug 14101 > # Test premature automatic renewal > ( $renewokay, $error, $info ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); > is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic and premature' ); > is( $error, 'auto_too_soon', > 'Bug 14101: Cannot renew, renewal is automatic and premature (returned code is auto_too_soon)' > ); >- is( $info->{soonest_renew_date}, dt_from_string($issue->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'auto_too_soon'"); >+ is( $info->{soonest_renew_date}, dt_from_string($issue_4->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'auto_too_soon'"); > > $renewing_borrower_obj->autorenew_checkouts(0)->store; >- ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); >+ ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); > is( $renewokay, 0, 'No renewal before is 7, patron opted out of auto_renewal still cannot renew early' ); > is( $error, 'too_soon', 'Error is too_soon, no auto' ); >- is( $info->{soonest_renew_date}, dt_from_string($issue->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'too_soon'"); >+ is( $info->{soonest_renew_date}, dt_from_string($issue_4->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'too_soon'"); > $renewing_borrower_obj->autorenew_checkouts(1)->store; > > # Change policy so that loans can only be renewed exactly on due date (0 days prior to due date) >@@ -914,18 +917,18 @@ subtest "CanBookBeRenewed tests" => sub { > $dbh->do(q{UPDATE circulation_rules SET rule_value = '0' WHERE rule_name = 'norenewalbefore'}); > Koha::Cache::Memory::Lite->flush(); > ( $renewokay, $error, $info ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); > is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic and premature' ); > is( $error, 'auto_too_soon', > 'Bug 14101: Cannot renew, renewal is automatic and premature, "No renewal before" = 0 (returned code is auto_too_soon)' > ); >- is( $info->{soonest_renew_date}, dt_from_string($issue->date_due), "Soonest renew date returned when error is 'auto_too_soon'"); >+ is( $info->{soonest_renew_date}, dt_from_string($issue_4->date_due), "Soonest renew date returned when error is 'auto_too_soon'"); > > $renewing_borrower_obj->autorenew_checkouts(0)->store; >- ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); >+ ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); > is( $renewokay, 0, 'No renewal before is 0, patron opted out of auto_renewal still cannot renew early' ); > is( $error, 'too_soon', 'Error is too_soon, no auto' ); >- is( $info->{soonest_renew_date}, dt_from_string($issue->date_due), "Soonest renew date returned when error is 'auto_too_soon'"); >+ is( $info->{soonest_renew_date}, dt_from_string($issue_4->date_due), "Soonest renew date returned when error is 'auto_too_soon'"); > $renewing_borrower_obj->autorenew_checkouts(1)->store; > > # Change policy so that loans can be renewed 99 days prior to the due date >@@ -933,14 +936,14 @@ subtest "CanBookBeRenewed tests" => sub { > $dbh->do(q{UPDATE circulation_rules SET rule_value = '99' WHERE rule_name = 'norenewalbefore'}); > Koha::Cache::Memory::Lite->flush(); > ( $renewokay, $error ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); > is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic' ); > is( $error, 'auto_renew', > 'Bug 14101: Cannot renew, renewal is automatic (returned code is auto_renew)' > ); > > $renewing_borrower_obj->autorenew_checkouts(0)->store; >- ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); > is( $renewokay, 1, 'No renewal before is 99, patron opted out of auto_renewal so can renew' ); > $renewing_borrower_obj->autorenew_checkouts(1)->store; > >@@ -955,7 +958,7 @@ subtest "CanBookBeRenewed tests" => sub { > > my $ten_days_before = dt_from_string->add( days => -10 ); > my $ten_days_ahead = dt_from_string->add( days => 10 ); >- AddIssue( $renewing_borrower, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); >+ my $issue = AddIssue( $renewing_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); > > Koha::CirculationRules->set_rules( > { >@@ -969,7 +972,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > ( $renewokay, $error ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' ); > >@@ -985,7 +988,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > ( $renewokay, $error ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_too_late', 'Cannot auto renew, too late - no_auto_renewal_after is inclusive(returned code is auto_too_late)' ); > >@@ -1001,7 +1004,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > ( $renewokay, $error ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_too_soon', 'Cannot auto renew, too soon - no_auto_renewal_after is defined(returned code is auto_too_soon)' ); > >@@ -1017,7 +1020,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > ( $renewokay, $error ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_renew', 'Cannot renew, renew is automatic' ); > >@@ -1034,7 +1037,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > ( $renewokay, $error ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' ); > >@@ -1051,7 +1054,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > ( $renewokay, $error ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' ); > >@@ -1068,7 +1071,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > ( $renewokay, $error ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_renew', 'Cannot renew, renew is automatic' ); > }; >@@ -1084,7 +1087,7 @@ subtest "CanBookBeRenewed tests" => sub { > > my $ten_days_before = dt_from_string->add( days => -10 ); > my $ten_days_ahead = dt_from_string->add( days => 10 ); >- AddIssue( $renewing_borrower, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); >+ my $issue = AddIssue( $renewing_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); > > Koha::CirculationRules->set_rules( > { >@@ -1112,7 +1115,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > )->status('RETURNED')->store; > ( $renewokay, $error ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, patron has 5' ); > >@@ -1126,7 +1129,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > )->status('RETURNED')->store; > ( $renewokay, $error ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, patron has 10' ); > >@@ -1140,7 +1143,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > )->status('RETURNED')->store; > ( $renewokay, $error ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_too_much_oweing', 'Cannot auto renew, OPACFineNoRenewals=10, patron has 15' ); > >@@ -1153,13 +1156,13 @@ subtest "CanBookBeRenewed tests" => sub { > } > )->store; > ( $renewokay, $error ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, OPACFineNoRenewalsIncludeCredit=1, patron has 15 debt, 5 credit' ); > > C4::Context->set_preference('OPACFineNoRenewalsIncludeCredit','0'); > ( $renewokay, $error ) = >- CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_too_much_oweing', 'Cannot auto renew, OPACFineNoRenewals=10, OPACFineNoRenewalsIncludeCredit=1, patron has 15 debt, 5 credit' ); > >@@ -1194,37 +1197,33 @@ subtest "CanBookBeRenewed tests" => sub { > # Patron is expired and BlockExpiredPatronOpacActions=0 > # => auto renew is allowed > t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 0); >- my $patron = $expired_borrower; >- my $checkout = AddIssue( $patron, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); >+ my $issue = AddIssue( $expired_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); > ( $renewokay, $error ) = >- CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $expired_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_renew', 'Can auto renew, patron is expired but BlockExpiredPatronOpacActions=0' ); >- Koha::Checkouts->find( $checkout->issue_id )->delete; >+ Koha::Checkouts->find( $issue->issue_id )->delete; > > > # Patron is expired and BlockExpiredPatronOpacActions=1 > # => auto renew is not allowed > t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 1); >- $patron = $expired_borrower; >- $checkout = AddIssue( $patron, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); >+ $issue = AddIssue( $expired_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); > ( $renewokay, $error ) = >- CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $expired_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_account_expired', 'Can not auto renew, lockExpiredPatronOpacActions=1 and patron is expired' ); >- Koha::Checkouts->find( $checkout->issue_id )->delete; >- >+ $issue->delete; > > # Patron is not expired and BlockExpiredPatronOpacActions=1 > # => auto renew is allowed > t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 1); >- $patron = $renewing_borrower; >- $checkout = AddIssue( $patron, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); >+ $issue = AddIssue( $renewing_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); > ( $renewokay, $error ) = >- CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->itemnumber ); >+ CanBookBeRenewed( $renewing_borrower_obj, $issue ); > is( $renewokay, 0, 'Do not renew, renewal is automatic' ); > is( $error, 'auto_renew', 'Can auto renew, BlockExpiredPatronOpacActions=1 but patron is not expired' ); >- Koha::Checkouts->find( $checkout->issue_id )->delete; >+ $issue->delete; > }; > > subtest "GetLatestAutoRenewDate" => sub { >@@ -1238,7 +1237,7 @@ subtest "CanBookBeRenewed tests" => sub { > > my $ten_days_before = dt_from_string->add( days => -10 ); > my $ten_days_ahead = dt_from_string->add( days => 10 ); >- AddIssue( $renewing_borrower, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); >+ my $issue = AddIssue( $renewing_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); > Koha::CirculationRules->set_rules( > { > categorycode => undef, >@@ -1251,7 +1250,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > } > ); >- my $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ my $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrower_obj, $issue ); > is( $latest_auto_renew_date, undef, 'GetLatestAutoRenewDate should return undef if no_auto_renewal_after or no_auto_renewal_after_hard_limit are not defined' ); > my $five_days_before = dt_from_string->add( days => -5 ); > Koha::CirculationRules->set_rules( >@@ -1266,7 +1265,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > } > ); >- $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrower_obj,, $issue ); > is( $latest_auto_renew_date->truncate( to => 'minute' ), > $five_days_before->truncate( to => 'minute' ), > 'GetLatestAutoRenewDate should return -5 days if no_auto_renewal_after = 5 and date_due is 10 days before' >@@ -1288,7 +1287,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > } > ); >- $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrower_obj, $issue ); > is( $latest_auto_renew_date->truncate( to => 'minute' ), > $five_days_ahead->truncate( to => 'minute' ), > 'GetLatestAutoRenewDate should return +5 days if no_auto_renewal_after = 15 and date_due is 10 days before' >@@ -1306,7 +1305,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > } > ); >- $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrower_obj, $issue ); > is( $latest_auto_renew_date->truncate( to => 'day' ), > $two_days_ahead->truncate( to => 'day' ), > 'GetLatestAutoRenewDate should return +2 days if no_auto_renewal_after_hard_limit is defined and not no_auto_renewal_after' >@@ -1323,7 +1322,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > } > ); >- $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); >+ $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrower_obj, $issue ); > is( $latest_auto_renew_date->truncate( to => 'day' ), > $two_days_ahead->truncate( to => 'day' ), > 'GetLatestAutoRenewDate should return +2 days if no_auto_renewal_after_hard_limit is < no_auto_renewal_after' >@@ -1345,7 +1344,7 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); > is( $renewokay, 0, 'Cannot renew, 0 renewals allowed'); > is( $error, 'too_many', 'Cannot renew, 0 renewals allowed (returned code is too_many)'); > >@@ -1362,8 +1361,9 @@ subtest "CanBookBeRenewed tests" => sub { > } > ); > t::lib::Mocks::mock_preference('UnseenRenewals', 1); >- $dbh->do('UPDATE issues SET unseen_renewals = 2 where borrowernumber = ? AND itemnumber = ?', undef, ($renewing_borrowernumber, $item_1->itemnumber)); >- ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); >+ $issue_1->unseen_renewals(2)->store; >+ >+ ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); > is( $renewokay, 0, 'Cannot renew, 0 unseen renewals allowed'); > is( $error, 'too_unseen', 'Cannot renew, returned code is too_unseen'); > Koha::CirculationRules->set_rules( >@@ -1385,21 +1385,21 @@ subtest "CanBookBeRenewed tests" => sub { > > C4::Overdues::UpdateFine( > { >- issue_id => $issue->id(), >+ issue_id => $issue_1->id(), > itemnumber => $item_1->itemnumber, >- borrowernumber => $renewing_borrower->{borrowernumber}, >+ borrowernumber => $renewing_borrower_obj->borrowernumber, > amount => 15.00, > type => q{}, > due => Koha::DateUtils::output_pref($datedue) > } > ); > >- my $line = Koha::Account::Lines->search({ borrowernumber => $renewing_borrower->{borrowernumber} })->next(); >+ my $line = Koha::Account::Lines->search({ borrowernumber => $renewing_borrower_obj->borrowernumber })->next(); > is( $line->debit_type_code, 'OVERDUE', 'Account line type is OVERDUE' ); > is( $line->status, 'UNRETURNED', 'Account line status is UNRETURNED' ); > is( $line->amountoutstanding+0, 15, 'Account line amount outstanding is 15.00' ); > is( $line->amount+0, 15, 'Account line amount is 15.00' ); >- is( $line->issue_id, $issue->id, 'Account line issue id matches' ); >+ is( $line->issue_id, $issue_1->id, 'Account line issue id matches' ); > > my $offset = Koha::Account::Offsets->search({ debit_id => $line->id })->next(); > is( $offset->type, 'CREATE', 'Account offset type is CREATE' ); >@@ -1421,7 +1421,7 @@ subtest "CanBookBeRenewed tests" => sub { > > my $total_due = $dbh->selectrow_array( > 'SELECT SUM( amountoutstanding ) FROM accountlines WHERE borrowernumber = ?', >- undef, $renewing_borrower->{borrowernumber} >+ undef, $renewing_borrower_obj->borrowernumber > ); > > is( $total_due+0, 15, 'Borrower only charged replacement fee with both WhenLostForgiveFine and WhenLostChargeReplacementFee enabled' ); >@@ -1430,9 +1430,9 @@ subtest "CanBookBeRenewed tests" => sub { > > C4::Overdues::UpdateFine( > { >- issue_id => $issue2->id(), >+ issue_id => $issue_2->id(), > itemnumber => $item_2->itemnumber, >- borrowernumber => $renewing_borrower->{borrowernumber}, >+ borrowernumber => $renewing_borrower_obj->borrowernumber, > amount => 15.00, > type => q{}, > due => Koha::DateUtils::output_pref($datedue) >@@ -1447,7 +1447,7 @@ subtest "CanBookBeRenewed tests" => sub { > > $total_due = $dbh->selectrow_array( > 'SELECT SUM( amountoutstanding ) FROM accountlines WHERE borrowernumber = ?', >- undef, $renewing_borrower->{borrowernumber} >+ undef, $renewing_borrower_obj->borrowernumber > ); > > ok( $total_due == 15, 'Borrower only charged fine with both WhenLostForgiveFine and WhenLostChargeReplacementFee disabled' ); >@@ -1485,11 +1485,11 @@ subtest "CanBookBeRenewed tests" => sub { > }, > }); > my $recall_borrower = $builder->build_object({ class => 'Koha::Patrons' }); >- my $recall_biblio = $builder->build_object({ class => 'Koha::Biblios' }); >- my $recall_item1 = $builder->build_object({ class => 'Koha::Items' }, { value => { biblionumber => $recall_biblio->biblionumber } }); >- my $recall_item2 = $builder->build_object({ class => 'Koha::Items' }, { value => { biblionumber => $recall_biblio->biblionumber } }); >+ my $recall_biblio = $builder->build_sample_biblio(); >+ my $recall_item1 = $builder->build_sample_item({ biblionumber => $recall_biblio->biblionumber }); >+ my $recall_item2 = $builder->build_sample_item({ biblionumber => $recall_biblio->biblionumber }); > >- AddIssue( $renewing_borrower, $recall_item1->barcode ); >+ my $recall_issue = AddIssue( $renewing_borrower_obj->unblessed, $recall_item1->barcode ); > > # item-level and this item: renewal not allowed > my $recall = Koha::Recall->new({ >@@ -1499,7 +1499,7 @@ subtest "CanBookBeRenewed tests" => sub { > pickup_library_id => $recall_borrower->branchcode, > item_level => 1, > })->store; >- ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $recall_item1->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $recall_issue ); > is( $error, 'recalled', 'Cannot renew item that has been recalled' ); > $recall->set_cancelled; > >@@ -1511,7 +1511,7 @@ subtest "CanBookBeRenewed tests" => sub { > pickup_library_id => $recall_borrower->branchcode, > item_level => 0, > })->store; >- ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $recall_item1->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $recall_issue ); > is( $error, 'recalled', 'Cannot renew item if biblio is recalled and has no item allocated' ); > $recall->set_cancelled; > >@@ -1523,7 +1523,7 @@ subtest "CanBookBeRenewed tests" => sub { > pickup_library_id => $recall_borrower->branchcode, > item_level => 1, > })->store; >- ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $recall_item1->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $recall_issue ); > is( $renewokay, 1, 'Can renew item if item-level recall on biblio is not on this item' ); > $recall->set_cancelled; > >@@ -1536,7 +1536,7 @@ subtest "CanBookBeRenewed tests" => sub { > item_level => 0, > })->store; > $recall->set_waiting({ item => $recall_item1 }); >- ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $recall_item1->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $recall_issue ); > is( $renewokay, 1, 'Can renew item if biblio-level recall has already been allocated an item' ); > $recall->set_cancelled; > }; >@@ -1578,7 +1578,7 @@ subtest "GetUpcomingDueIssues" => sub { > > my $issue = AddIssue( $a_borrower, $item_1->barcode, $yesterday ); > my $datedue = dt_from_string( $issue->date_due() ); >- my $issue2 = AddIssue( $a_borrower, $item_2->barcode, $two_days_ahead ); >+ my $issue_2 = AddIssue( $a_borrower, $item_2->barcode, $two_days_ahead ); > my $datedue2 = dt_from_string( $issue->date_due() ); > > my $upcoming_dues; >@@ -1704,12 +1704,12 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { > ); > > >- my $borrowernumber1 = Koha::Patron->new({ >+ my $borrower1 = Koha::Patron->new({ > firstname => 'Kyle', > surname => 'Hall', > categorycode => $patron_category->{categorycode}, > branchcode => $library2->{branchcode}, >- })->store->borrowernumber; >+ })->store; > my $borrowernumber2 = Koha::Patron->new({ > firstname => 'Chelsea', > surname => 'Hall', >@@ -1733,12 +1733,9 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { > branchcode => $library2->{branchcode}, > })->store->borrowernumber; > >- my $borrower1 = Koha::Patrons->find( $borrowernumber1 )->unblessed; >- my $borrower2 = Koha::Patrons->find( $borrowernumber2 )->unblessed; >+ my $issue = AddIssue( $borrower1->unblessed, $item_1->barcode ); > >- my $issue = AddIssue( $borrower1, $item_1->barcode ); >- >- my ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); >+ my ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); > is( $renewokay, 1, 'Bug 14337 - Verify the borrower can renew with no hold on the record' ); > > AddReserve( >@@ -1761,11 +1758,11 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { > } > ); > t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', 0 ); >- ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); > is( $renewokay, 0, 'Bug 14337 - Verify the borrower cannot renew with a hold on the record if AllowRenewalIfOtherItemsAvailable and onshelfholds are disabled' ); > > t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', 1 ); >- ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); > is( $renewokay, 0, 'Bug 14337 - Verify the borrower cannot renew with a hold on the record if AllowRenewalIfOtherItemsAvailable is enabled and onshelfholds is disabled' ); > > Koha::CirculationRules->set_rules( >@@ -1779,11 +1776,11 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { > } > ); > t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', 0 ); >- ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); > is( $renewokay, 0, 'Bug 14337 - Verify the borrower cannot renew with a hold on the record if AllowRenewalIfOtherItemsAvailable is disabled and onshelfhold is enabled' ); > > t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', 1 ); >- ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); > is( $renewokay, 1, 'Bug 14337 - Verify the borrower can renew with a hold on the record if AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled' ); > > AddReserve( >@@ -1795,7 +1792,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { > } > ); > >- ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); > is( $renewokay, 0, 'Verify the borrower cannot renew with 2 holds on the record if AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled and one other item on record' ); > > my $item_3= $builder->build_sample_item( >@@ -1806,7 +1803,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { > } > ); > >- ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); > is( $renewokay, 1, 'Verify the borrower cannot renew with 2 holds on the record if AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled and two other items on record' ); > > Koha::CirculationRules->set_rules( >@@ -1820,7 +1817,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { > } > ); > >- ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); > is( $renewokay, 0, 'Verify the borrower cannot renew with 2 holds on the record, but only one of those holds can be filled when AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled and two other items on record' ); > > Koha::CirculationRules->set_rules( >@@ -1837,7 +1834,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { > # Setting item not checked out to be not for loan but holdable > $item_2->notforloan(-1)->store; > >- ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); > is( $renewokay, 0, 'Bug 14337 - Verify the borrower can not renew with a hold on the record if AllowRenewalIfOtherItemsAvailable is enabled but the only available item is notforloan' ); > > my $mock_circ = Test::MockModule->new("C4::Circulation"); >@@ -1851,7 +1848,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { > # Two items total, one item available, one issued, two holds on record > > warnings_are{ >- ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); > } [], "CanItemBeReserved not called when there are more possible holds than available items"; > is( $renewokay, 0, 'Borrower cannot renew when there are more holds than available items' ); > >@@ -1875,7 +1872,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { > ); > > warnings_are{ >- ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); >+ ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); > } ["Checked","Checked"], "CanItemBeReserved only called once per available item if it returns a negative result for all items for a borrower"; > is( $renewokay, 0, 'Borrower cannot renew when there are more holds than available items' ); > >@@ -1896,17 +1893,15 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { > } > ); > >- my $borrowernumber = Koha::Patron->new({ >+ my $borrower = Koha::Patron->new({ > firstname => 'fn', > surname => 'dn', > categorycode => $patron_category->{categorycode}, > branchcode => $branch, >- })->store->borrowernumber; >- >- my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; >+ })->store; > >- my $issue = AddIssue( $borrower, $item->barcode, undef, undef, undef, undef, { onsite_checkout => 1 } ); >- my ( $renewed, $error ) = CanBookBeRenewed( $borrowernumber, $item->itemnumber ); >+ my $issue = AddIssue( $borrower->unblessed, $item->barcode, undef, undef, undef, undef, { onsite_checkout => 1 } ); >+ my ( $renewed, $error ) = CanBookBeRenewed( $borrower, $issue ); > is( $renewed, 0, 'CanBookBeRenewed should not allow to renew on-site checkout' ); > is( $error, 'onsite_checkout', 'A correct error code should be returned by CanBookBeRenewed for on-site checkout' ); > } >@@ -1923,24 +1918,23 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { > itype => $itemtype, > } > ); >+ my $patron = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library->{branchcode}, categorycode => $patron_category->{categorycode} } } ); > >- my $patron = $builder->build({ source => 'Borrower', value => { branchcode => $library->{branchcode}, categorycode => $patron_category->{categorycode} } } ); >- >- my $issue = AddIssue( $patron, $item->barcode ); >+ my $issue = AddIssue( $patron->unblessed, $item->barcode ); > UpdateFine( > { >- issue_id => $issue->id(), >+ issue_id => $issue->id, > itemnumber => $item->itemnumber, >- borrowernumber => $patron->{borrowernumber}, >+ borrowernumber => $patron->borrowernumber, > amount => 1, > type => q{} > } > ); > UpdateFine( > { >- issue_id => $issue->id(), >+ issue_id => $issue->id, > itemnumber => $item->itemnumber, >- borrowernumber => $patron->{borrowernumber}, >+ borrowernumber => $patron->borrowernumber, > amount => 2, > type => q{} > } >@@ -2365,9 +2359,8 @@ subtest 'MultipleReserves' => sub { > categorycode => $patron_category->{categorycode}, > branchcode => $branch, > ); >- my $renewing_borrowernumber = Koha::Patron->new(\%renewing_borrower_data)->store->borrowernumber; >- my $renewing_borrower = Koha::Patrons->find( $renewing_borrowernumber )->unblessed; >- my $issue = AddIssue( $renewing_borrower, $item_1->barcode); >+ my $patron = Koha::Patron->new(\%renewing_borrower_data)->store; >+ my $issue = AddIssue( $patron->unblessed, $item_1->barcode); > my $datedue = dt_from_string( $issue->date_due() ); > is (defined $issue->date_due(), 1, "item 1 checked out"); > my $borrowing_borrowernumber = Koha::Checkouts->find({ itemnumber => $item_1->itemnumber })->borrowernumber; >@@ -2415,7 +2408,7 @@ subtest 'MultipleReserves' => sub { > ); > > { >- my ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1); >+ my ( $renewokay, $error ) = CanBookBeRenewed($patron, $issue, 1); > is($renewokay, 0, 'Bug 17941 - should cover the case where 2 books are both reserved, so failing'); > } > >@@ -2429,7 +2422,7 @@ subtest 'MultipleReserves' => sub { > ); > > { >- my ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1); >+ my ( $renewokay, $error ) = CanBookBeRenewed($patron, $issue, 1); > is($renewokay, 1, 'Bug 17941 - should cover the case where 2 books are reserved, but a third one is available'); > } > }; >@@ -4031,7 +4024,7 @@ subtest 'Set waiting flag' => sub { > my $hold = Koha::Holds->find( $reserve_id ); > is( $hold->found, 'T', 'Hold is in transit' ); > >- my ( $status ) = CheckReserves($item->itemnumber); >+ my ( $status ) = CheckReserves($item); > is( $status, 'Transferred', 'Hold is not waiting yet'); > > set_userenv( $library_2 ); >@@ -4040,7 +4033,7 @@ subtest 'Set waiting flag' => sub { > ModReserveAffect( $item->itemnumber, undef, $do_transfer, $reserve_id ); > $hold = Koha::Holds->find( $reserve_id ); > is( $hold->found, 'W', 'Hold is waiting' ); >- ( $status ) = CheckReserves($item->itemnumber); >+ ( $status ) = CheckReserves($item); > is( $status, 'Waiting', 'Now the hold is waiting'); > > #Bug 21944 - Waiting transfer checked in at branch other than pickup location >@@ -4225,14 +4218,14 @@ subtest 'ItemsDeniedRenewal rules are checked' => sub { > my $mock_item_class = Test::MockModule->new("Koha::Item"); > $mock_item_class->mock( 'is_denied_renewal', sub { return 1; } ); > >- my ( $mayrenew, $error ) = CanBookBeRenewed( $idr_borrower->borrowernumber, $issue->itemnumber ); >+ my ( $mayrenew, $error ) = CanBookBeRenewed( $idr_borrower, $issue ); > is( $mayrenew, 0, 'Renewal blocked when $item->is_denied_renewal returns true' ); > is( $error, 'item_denied_renewal', 'Renewal blocked when $item->is_denied_renewal returns true' ); > > $mock_item_class->unmock( 'is_denied_renewal' ); > $mock_item_class->mock( 'is_denied_renewal', sub { return 0; } ); > >- ( $mayrenew, $error ) = CanBookBeRenewed( $idr_borrower->borrowernumber, $issue->itemnumber ); >+ ( $mayrenew, $error ) = CanBookBeRenewed( $idr_borrower, $issue ); > is( $mayrenew, 1, 'Renewal allowed when $item->is_denied_renewal returns false' ); > is( $error, undef, 'Renewal allowed when $item->is_denied_renewal returns false' ); > >@@ -5720,7 +5713,7 @@ subtest "GetSoonestRenewDate tests" => sub { > # Test 'exact time' setting for syspref NoRenewalBeforePrecision > t::lib::Mocks::mock_preference( 'NoRenewalBeforePrecision', 'exact_time' ); > is( >- GetSoonestRenewDate( $issue ), >+ GetSoonestRenewDate( $patron, $issue ), > $datedue->clone->add( days => -7 ), > 'Bug 14395: Renewals permitted 7 days before due date, as expected' > ); >@@ -5729,7 +5722,7 @@ subtest "GetSoonestRenewDate tests" => sub { > # Test 'date' setting for syspref NoRenewalBeforePrecision > t::lib::Mocks::mock_preference( 'NoRenewalBeforePrecision', 'date' ); > is( >- GetSoonestRenewDate( $issue ), >+ GetSoonestRenewDate( $patron, $issue ), > $datedue->clone->add( days => -7 )->truncate( to => 'day' ), > 'Bug 14395: Renewals permitted 7 days before due date, as expected' > ); >@@ -5746,7 +5739,7 @@ subtest "GetSoonestRenewDate tests" => sub { > ); > > is( >- GetSoonestRenewDate( $issue ), >+ GetSoonestRenewDate( $patron, $issue ), > dt_from_string, > 'Checkouts without auto-renewal can be renewed immediately if no norenewalbefore' > ); >@@ -5754,13 +5747,13 @@ subtest "GetSoonestRenewDate tests" => sub { > t::lib::Mocks::mock_preference( 'NoRenewalBeforePrecision', 'date' ); > $issue->auto_renew(1)->store; > is( >- GetSoonestRenewDate( $issue ), >+ GetSoonestRenewDate( $patron, $issue ), > $datedue->clone->truncate( to => 'day' ), > 'Checkouts with auto-renewal can be renewed earliest on due date if no renewalbefore' > ); > t::lib::Mocks::mock_preference( 'NoRenewalBeforePrecision', 'exact' ); > is( >- GetSoonestRenewDate( $issue ), >+ GetSoonestRenewDate( $patron, $issue ), > $datedue, > 'Checkouts with auto-renewal can be renewed earliest on due date if no renewalbefore' > ); >@@ -5773,7 +5766,7 @@ subtest "CanBookBeIssued + needsconfirmation message" => sub { > my $library = $builder->build_object( { class => 'Koha::Libraries' } ); > my $biblio = $builder->build_object({ class => 'Koha::Biblios' }); > my $biblioitem = $builder->build_object({ class => 'Koha::Biblioitems', value => { biblionumber => $biblio->biblionumber }}); >- my $item = $builder->build_object({ class => 'Koha::Items' , value => { biblionumber => $biblio->biblionumber }}); >+ my $item = $builder->build_object({ class => 'Koha::Items' , value => { itype => $itemtype, biblionumber => $biblio->biblionumber }}); > > my $hold = $builder->build_object({ class => 'Koha::Holds', value => { > biblionumber => $item->biblionumber, >diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t >index f2dc1949814..7c20b11bac9 100755 >--- a/t/db_dependent/Circulation/Returns.t >+++ b/t/db_dependent/Circulation/Returns.t >@@ -132,6 +132,7 @@ subtest "AddReturn logging on statistics table (item-level_itypes=1)" => sub { > "item-level itype recorded on statistics for return"); > warning_like { AddIssue( $borrower, $item_without_itemtype->barcode ) } > [qr/^item-level_itypes set but no itemtype set for item/, >+ qr/^item-level_itypes set but no itemtype set for item/, > qr/^item-level_itypes set but no itemtype set for item/], > 'Item without itemtype set raises warning on AddIssue'; > AddReturn( $item_without_itemtype->barcode, $branch ); >diff --git a/t/db_dependent/Circulation/transferbook.t b/t/db_dependent/Circulation/transferbook.t >index f6ee6b43230..c94e32dc4d5 100755 >--- a/t/db_dependent/Circulation/transferbook.t >+++ b/t/db_dependent/Circulation/transferbook.t >@@ -116,9 +116,11 @@ subtest 'transfer already at destination' => sub { > } > ); > >+ my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' })->store->itemtype; > my $item = $builder->build_sample_item( > { > library => $library->branchcode, >+ itype => $itemtype > } > ); > >@@ -169,7 +171,8 @@ subtest 'transfer already at destination' => sub { > is( $dotransfer, 0, 'Do not transfer recalled item, it has already arrived' ); > is( $messages->{RecallPlacedAtHoldingBranch}, 1, "We found the recall"); > >- my $item2 = $builder->build_object({ class => 'Koha::Items' }); # this item will have a different holding branch to the pickup branch >+ $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' })->store->itemtype; >+ my $item2 = $builder->build_object({ class => 'Koha::Items', value => { itype => $itemtype } }); # this item will have a different holding branch to the pickup branch > $recall = Koha::Recall->new( > { biblio_id => $item2->biblionumber, > item_id => $item2->itemnumber, >@@ -196,9 +199,11 @@ subtest 'transfer an issued item' => sub { > } > ); > >+ my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' })->store->itemtype; > my $item = $builder->build_sample_item( > { > library => $library->branchcode, >+ itype => $itemtype > } > ); > >diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t >index c5c79b1d768..2f2b0d57823 100755 >--- a/t/db_dependent/Holds.t >+++ b/t/db_dependent/Holds.t >@@ -64,7 +64,8 @@ $insert_sth->execute('ONLY1'); > my $biblio = $builder->build_sample_biblio({ itemtype => 'DUMMY' }); > > # Create item instance for testing. >-my $itemnumber = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber })->itemnumber; >+my $item = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber }); >+my $itemnumber = $item->itemnumber; > > # Create some borrowers > my @borrowernumbers; >@@ -101,7 +102,6 @@ is( $holds->next->priority, 3, "Reserve 3 has a priority of 3" ); > is( $holds->next->priority, 4, "Reserve 4 has a priority of 4" ); > is( $holds->next->priority, 5, "Reserve 5 has a priority of 5" ); > >-my $item = Koha::Items->find( $itemnumber ); > $holds = $item->current_holds; > my $first_hold = $holds->next; > my $reservedate = $first_hold->reservedate; >@@ -342,7 +342,7 @@ ok( > 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( $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" ); >+ok( defined( ( CheckReserves($damaged_item) )[1] ), "Hold can be trapped for damaged item with AllowHoldsOnDamagedItems enabled" ); > > $hold = Koha::Hold->new( > { >@@ -359,7 +359,7 @@ $hold->delete(); > > t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 0 ); > 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" ); >+ok( !defined( ( CheckReserves($damaged_item) )[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 ); >@@ -377,19 +377,19 @@ $hold = Koha::Hold->new( > branchcode => $branch_1, > } > )->store(); >-ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item that is not for loan but holdable ( notforloan < 0 )" ); >+ok( !defined( ( CheckReserves($nfl_item) )[1] ), "Hold cannot be trapped for item that is not for loan but holdable ( notforloan < 0 )" ); > t::lib::Mocks::mock_preference( 'TrapHoldsOnOrder', 1 ); >-ok( defined( ( CheckReserves($itemnumber) )[1] ), "Hold is trapped for item that is not for loan but holdable ( notforloan < 0 )" ); >+ok( defined( ( CheckReserves($nfl_item) )[1] ), "Hold is trapped for item that is not for loan but holdable ( notforloan < 0 )" ); > t::lib::Mocks::mock_preference( 'SkipHoldTrapOnNotForLoanValue', '-1' ); >-ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item with notforloan value matching SkipHoldTrapOnNotForLoanValue" ); >+ok( !defined( ( CheckReserves($nfl_item) )[1] ), "Hold cannot be trapped for item with notforloan value matching SkipHoldTrapOnNotForLoanValue" ); > t::lib::Mocks::mock_preference( 'SkipHoldTrapOnNotForLoanValue', '-1|1' ); >-ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item with notforloan value matching SkipHoldTrapOnNotForLoanValue" ); >+ok( !defined( ( CheckReserves($nfl_item) )[1] ), "Hold cannot be trapped for item with notforloan value matching SkipHoldTrapOnNotForLoanValue" ); > t::lib::Mocks::mock_preference( 'SkipHoldTrapOnNotForLoanValue', '' ); > my $item_group_1 = Koha::Biblio::ItemGroup->new( { biblio_id => $biblio->id } )->store(); > my $item_group_2 = Koha::Biblio::ItemGroup->new( { biblio_id => $biblio->id } )->store(); > $item_group_1->add_item({ item_id => $itemnumber }); > $hold->item_group_id( $item_group_2->id )->update; >-ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item with non-matching item group" ); >+ok( !defined( ( CheckReserves($nfl_item) )[1] ), "Hold cannot be trapped for item with non-matching item group" ); > is( > CanItemBeReserved( $patrons[0], $nfl_item)->{status}, 'itemAlreadyOnHold', > "cannot request item that you have already reservedd" >@@ -1501,7 +1501,7 @@ subtest 'non priority holds' => sub { > } > ); > >- Koha::Checkout->new( >+ my $issue = Koha::Checkout->new( > { > borrowernumber => $patron1->borrowernumber, > itemnumber => $item->itemnumber, >@@ -1520,7 +1520,7 @@ subtest 'non priority holds' => sub { > ); > > my ( $ok, $err ) = >- CanBookBeRenewed( $patron1->borrowernumber, $item->itemnumber ); >+ CanBookBeRenewed( $patron1, $issue ); > > ok( !$ok, 'Cannot renew' ); > is( $err, 'on_reserve', 'Item is on hold' ); >@@ -1529,7 +1529,7 @@ subtest 'non priority holds' => sub { > $hold->non_priority(1)->store; > > ( $ok, $err ) = >- CanBookBeRenewed( $patron1->borrowernumber, $item->itemnumber ); >+ CanBookBeRenewed( $patron1, $issue ); > > ok( $ok, 'Can renew' ); > is( $err, undef, 'Item is on non priority hold' ); >@@ -1553,7 +1553,7 @@ subtest 'non priority holds' => sub { > ); > > ( $ok, $err ) = >- CanBookBeRenewed( $patron1->borrowernumber, $item->itemnumber ); >+ CanBookBeRenewed( $patron1, $issue ); > > ok( !$ok, 'Cannot renew' ); > is( $err, 'on_reserve', 'Item is on hold' ); >diff --git a/t/db_dependent/Holds/HoldFulfillmentPolicy.t b/t/db_dependent/Holds/HoldFulfillmentPolicy.t >index 7e21336c106..5fadfb0cfe5 100755 >--- a/t/db_dependent/Holds/HoldFulfillmentPolicy.t >+++ b/t/db_dependent/Holds/HoldFulfillmentPolicy.t >@@ -67,9 +67,7 @@ $dbh->do(" > VALUES ($biblionumber, $biblioitemnumber, '$library_A', '$library_B', 0, 0, 0, 0, NULL, '$itemtype') > "); > >-my $itemnumber = >- $dbh->selectrow_array("SELECT itemnumber FROM items WHERE biblionumber = $biblionumber") >- or BAIL_OUT("Cannot find newly created item"); >+my $item = Koha::Items->find({ biblionumber => $biblionumber }); > > # With hold_fulfillment_policy = homebranch, hold should only be picked up if pickup branch = homebranch > $dbh->do("DELETE FROM circulation_rules"); >@@ -93,7 +91,7 @@ my $reserve_id = AddReserve( > priority => 1, > } > ); >-my ( $status ) = CheckReserves($itemnumber); >+my ( $status ) = CheckReserves($item); > is( $status, 'Reserved', "Hold where pickup branch matches home branch targeted" ); > Koha::Holds->find( $reserve_id )->cancel; > >@@ -106,7 +104,7 @@ $reserve_id = AddReserve( > priority => 1, > } > ); >-( $status ) = CheckReserves($itemnumber); >+( $status ) = CheckReserves($item); > is($status, q{}, "Hold where pickup ne home, pickup eq home not targeted" ); > Koha::Holds->find( $reserve_id )->cancel; > >@@ -119,7 +117,7 @@ $reserve_id = AddReserve( > priority => 1, > } > ); >-( $status ) = CheckReserves($itemnumber); >+( $status ) = CheckReserves($item); > is( $status, q{}, "Hold where pickup ne home, pickup ne holding not targeted" ); > Koha::Holds->find( $reserve_id )->cancel; > >@@ -145,7 +143,7 @@ $reserve_id = AddReserve( > priority => 1, > } > ); >-( $status ) = CheckReserves($itemnumber); >+( $status ) = CheckReserves($item); > is( $status, q{}, "Hold where pickup eq home, pickup ne holding not targeted" ); > Koha::Holds->find( $reserve_id )->cancel; > >@@ -158,7 +156,7 @@ $reserve_id = AddReserve( > priority => 1, > } > ); >-( $status ) = CheckReserves($itemnumber); >+( $status ) = CheckReserves($item); > is( $status, 'Reserved', "Hold where pickup ne home, pickup eq holding targeted" ); > Koha::Holds->find( $reserve_id )->cancel; > >@@ -171,7 +169,7 @@ $reserve_id = AddReserve( > priority => 1, > } > ); >-( $status ) = CheckReserves($itemnumber); >+( $status ) = CheckReserves($item); > is( $status, q{}, "Hold where pickup ne home, pickup ne holding not targeted" ); > Koha::Holds->find( $reserve_id )->cancel; > >@@ -197,7 +195,7 @@ $reserve_id = AddReserve( > priority => 1, > } > ); >-( $status ) = CheckReserves($itemnumber); >+( $status ) = CheckReserves($item); > is( $status, 'Reserved', "Hold where pickup eq home, pickup ne holding targeted" ); > Koha::Holds->find( $reserve_id )->cancel; > >@@ -210,7 +208,7 @@ $reserve_id = AddReserve( > priority => 1, > } > ); >-( $status ) = CheckReserves($itemnumber); >+( $status ) = CheckReserves($item); > is( $status, 'Reserved', "Hold where pickup ne home, pickup eq holding targeted" ); > Koha::Holds->find( $reserve_id )->cancel; > >@@ -223,7 +221,7 @@ $reserve_id = AddReserve( > priority => 1, > } > ); >-( $status ) = CheckReserves($itemnumber); >+( $status ) = CheckReserves($item); > is( $status, 'Reserved', "Hold where pickup ne home, pickup ne holding targeted" ); > Koha::Holds->find( $reserve_id )->cancel; > >@@ -231,7 +229,6 @@ Koha::Holds->find( $reserve_id )->cancel; > t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); > t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); > Koha::Holds->search()->delete(); >-my ($item) = Koha::Biblios->find($biblionumber)->items->as_list; > my $limit = Koha::Item::Transfer::Limit->new( > { > toBranch => $library_C, >@@ -247,6 +244,6 @@ $reserve_id = AddReserve( > priority => 1 > } > ); >-($status) = CheckReserves($itemnumber); >+($status) = CheckReserves($item); > is( $status, '', "No hold where branch transfer is not allowed" ); > Koha::Holds->find($reserve_id)->cancel; >diff --git a/t/db_dependent/Holds/HoldItemtypeLimit.t b/t/db_dependent/Holds/HoldItemtypeLimit.t >index 91573206948..2b5bdc141a8 100755 >--- a/t/db_dependent/Holds/HoldItemtypeLimit.t >+++ b/t/db_dependent/Holds/HoldItemtypeLimit.t >@@ -82,9 +82,7 @@ $dbh->do(" > VALUES ($biblionumber, $biblioitemnumber, '$branchcode', '$branchcode', 0, 0, 0, 0, NULL, '$right_itemtype') > "); > >-my $itemnumber = >- $dbh->selectrow_array("SELECT itemnumber FROM items WHERE biblionumber = $biblionumber") >- or BAIL_OUT("Cannot find newly created item"); >+my $item = Koha::Items->find({ biblionumber => $biblionumber }); > > $dbh->do("DELETE FROM circulation_rules"); > Koha::CirculationRules->set_rules( >@@ -108,7 +106,7 @@ my $reserve_id = AddReserve( > itemtype => $right_itemtype, > } > ); >-my ( $status ) = CheckReserves($itemnumber); >+my ( $status ) = CheckReserves($item); > is( $status, 'Reserved', "Hold where itemtype matches item's itemtype targed" ); > Koha::Holds->find( $reserve_id )->cancel; > >@@ -123,7 +121,7 @@ $reserve_id = AddReserve( > } > ); > >-( $status ) = CheckReserves($itemnumber); >+( $status ) = CheckReserves($item); > is($status, q{}, "Hold where itemtype does not match item's itemtype not targeted" ); > Koha::Holds->find( $reserve_id )->cancel; > >@@ -136,7 +134,7 @@ $reserve_id = AddReserve( > priority => 1, > } > ); >-( $status ) = CheckReserves($itemnumber); >+( $status ) = CheckReserves($item); > is( $status, 'Reserved', "Item targeted with no hold itemtype set" ); > Koha::Holds->find( $reserve_id )->cancel; > >diff --git a/t/db_dependent/Holds/LocalHoldsPriority.t b/t/db_dependent/Holds/LocalHoldsPriority.t >index a125d3eab19..85833ca8d50 100755 >--- a/t/db_dependent/Holds/LocalHoldsPriority.t >+++ b/t/db_dependent/Holds/LocalHoldsPriority.t >@@ -42,7 +42,7 @@ my $itemtype = $builder->build( > my $borrowers_count = 5; > > my $biblio = $builder->build_sample_biblio(); >-my $itemnumber = Koha::Item->new( >+my $item = Koha::Item->new( > { > biblionumber => $biblio->biblionumber, > homebranch => $library4->{branchcode}, >@@ -50,7 +50,7 @@ my $itemnumber = Koha::Item->new( > itype => $itemtype, > exclude_from_local_holds_priority => 0, > }, >-)->store->itemnumber; >+)->store; > > my @branchcodes = ( $library1->{branchcode}, $library2->{branchcode}, $library3->{branchcode}, $library4->{branchcode}, $library3->{branchcode}, $library4->{branchcode} ); > my $patron_category = $builder->build({ source => 'Category', value => {exclude_from_local_holds_priority => 0} }); >@@ -84,29 +84,29 @@ foreach my $borrowernumber (@borrowernumbers) { > my ($status, $reserve, $all_reserves); > > t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 0 ); >-($status, $reserve, $all_reserves) = CheckReserves($itemnumber); >+($status, $reserve, $all_reserves) = CheckReserves($item); > ok( $reserve->{borrowernumber} eq $borrowernumbers[0], "Received expected results with LocalHoldsPriority disabled" ); > > t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 1 ); > > t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'PickupLibrary' ); > t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' ); >-($status, $reserve, $all_reserves) = CheckReserves($itemnumber); >+($status, $reserve, $all_reserves) = CheckReserves($item); > ok( $reserve->{borrowernumber} eq $borrowernumbers[2], "Received expected results with PickupLibrary/homebranch" ); > > t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'PickupLibrary' ); > t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'holdingbranch' ); >-($status, $reserve, $all_reserves) = CheckReserves($itemnumber); >+($status, $reserve, $all_reserves) = CheckReserves($item); > ok( $reserve->{borrowernumber} eq $borrowernumbers[1], "Received expected results with PickupLibrary/holdingbranch" ); > > t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'HomeLibrary' ); > t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'holdingbranch' ); >-($status, $reserve, $all_reserves) = CheckReserves($itemnumber); >+($status, $reserve, $all_reserves) = CheckReserves($item); > ok( $reserve->{borrowernumber} eq $borrowernumbers[2], "Received expected results with HomeLibrary/holdingbranch" ); > > t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'HomeLibrary' ); > t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' ); >-($status, $reserve, $all_reserves) = CheckReserves($itemnumber); >+($status, $reserve, $all_reserves) = CheckReserves($item); > ok( $reserve->{borrowernumber} eq $borrowernumbers[3], "Received expected results with HomeLibrary/homebranch" ); > > $schema->storage->txn_rollback; >@@ -160,7 +160,7 @@ subtest "exclude from local holds" => sub { > ); > > my ($status, $reserve, $all_reserves); >- ($status, $reserve, $all_reserves) = CheckReserves($item1->itemnumber); >+ ($status, $reserve, $all_reserves) = CheckReserves($item1); > is($reserve->{borrowernumber}, $patron_nex_l1->borrowernumber, "Patron not excluded with local holds priorities is next checkout"); > > Koha::Holds->delete; >@@ -182,7 +182,7 @@ subtest "exclude from local holds" => sub { > } > ); > >- ($status, $reserve, $all_reserves) = CheckReserves($item1->itemnumber); >+ ($status, $reserve, $all_reserves) = CheckReserves($item1); > is($reserve->{borrowernumber}, $patron_nex_l2->borrowernumber, "Local patron is excluded from priority"); > > Koha::Holds->delete; >@@ -204,7 +204,7 @@ subtest "exclude from local holds" => sub { > } > ); > >- ($status, $reserve, $all_reserves) = CheckReserves($item2->itemnumber); >+ ($status, $reserve, $all_reserves) = CheckReserves($item2); > is($reserve->{borrowernumber}, $patron_nex_l2->borrowernumber, "Patron from other library is next checkout because item is excluded"); > > $schema->storage->txn_rollback; >diff --git a/t/db_dependent/Koha/Account/Line.t b/t/db_dependent/Koha/Account/Line.t >index b5926b4f70d..abd314e4198 100755 >--- a/t/db_dependent/Koha/Account/Line.t >+++ b/t/db_dependent/Koha/Account/Line.t >@@ -511,6 +511,7 @@ subtest 'Renewal related tests' => sub { > class => 'Koha::Checkouts', > value => { > itemnumber => $item->itemnumber, >+ borrowernumber => $patron->borrowernumber, > onsite_checkout => 0, > renewals_count => 99, > auto_renew => 0 >diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t >index a49a3784c2b..eb874d31f70 100755 >--- a/t/db_dependent/Koha/Object.t >+++ b/t/db_dependent/Koha/Object.t >@@ -973,6 +973,8 @@ subtest 'unblessed_all_relateds' => sub { > my $patron = Koha::Patron->new($patron_data)->store; > my ($biblionumber) = AddBiblio( MARC::Record->new, '' ); > my $biblio = Koha::Biblios->find( $biblionumber ); >+ my $itemtype = $builder->build({ source => 'Itemtype' })->{itemtype}; >+ > my $item = $builder->build_object( > { > class => 'Koha::Items', >@@ -982,6 +984,7 @@ subtest 'unblessed_all_relateds' => sub { > biblionumber => $biblio->biblionumber, > itemlost => 0, > withdrawn => 0, >+ itype => $itemtype > } > } > ); >diff --git a/t/db_dependent/Letters/TemplateToolkit.t b/t/db_dependent/Letters/TemplateToolkit.t >index edd91ac091e..7d01777f29f 100755 >--- a/t/db_dependent/Letters/TemplateToolkit.t >+++ b/t/db_dependent/Letters/TemplateToolkit.t >@@ -311,12 +311,13 @@ subtest 'regression tests' => sub { > plan tests => 8; > > my $library = $builder->build( { source => 'Branch' } ); >- my $patron = $builder->build( { source => 'Borrower' } ); >+ my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' })->store->itemtype; >+ > my $item1 = $builder->build_sample_item( > { > barcode => 'a_t_barcode', > library => $library->{branchcode}, >- itype => 'BK', >+ itype => $itemtype, > itemcallnumber => 'itemcallnumber1', > } > ); >@@ -326,7 +327,7 @@ subtest 'regression tests' => sub { > { > barcode => 'another_t_barcode', > library => $library->{branchcode}, >- itype => 'BK', >+ itype => $itemtype, > itemcallnumber => 'itemcallnumber2', > } > ); >@@ -336,7 +337,7 @@ subtest 'regression tests' => sub { > { > barcode => 'another_t_barcode_3', > library => $library->{branchcode}, >- itype => 'BK', >+ itype => $itemtype, > itemcallnumber => 'itemcallnumber3', > } > ); >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index 3371c9539d7..a533ca97fb3 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 77; >+use Test::More tests => 76; > use Test::MockModule; > use Test::Warn; > >@@ -94,6 +94,7 @@ my $biblio_with_no_item = $builder->build_sample_biblio; > my $testbarcode = '97531'; > $item->barcode($testbarcode)->store; # FIXME We should not hardcode a barcode! Also, what's the purpose of this? > >+ > # Create a borrower > my %data = ( > firstname => 'my firstname', >@@ -106,7 +107,6 @@ my $borrowernumber = Koha::Patron->new(\%data)->store->borrowernumber; > my $patron = Koha::Patrons->find( $borrowernumber ); > my $borrower = $patron->unblessed; > my $biblionumber = $bibnum; >-my $barcode = $testbarcode; > > my $branchcode = Koha::Libraries->search->next->branchcode; > >@@ -119,18 +119,15 @@ AddReserve( > } > ); > >-my ($status, $reserve, $all_reserves) = CheckReserves($item->itemnumber, $barcode); >+my ($status, $reserve, $all_reserves) = CheckReserves( $item ); > > is($status, "Reserved", "CheckReserves Test 1"); > > ok(exists($reserve->{reserve_id}), 'CheckReserves() include reserve_id in its response'); > >-($status, $reserve, $all_reserves) = CheckReserves($item->itemnumber); >+($status, $reserve, $all_reserves) = CheckReserves( $item ); > is($status, "Reserved", "CheckReserves Test 2"); > >-($status, $reserve, $all_reserves) = CheckReserves(undef, $barcode); >-is($status, "Reserved", "CheckReserves Test 3"); >- > my $ReservesControlBranch = C4::Context->preference('ReservesControlBranch'); > t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'ItemHomeLibrary' ); > ok( >@@ -345,9 +342,9 @@ AddReserve( > priority => 1, > } > ); >-($status)=CheckReserves($item->itemnumber,undef,undef); >+($status)=CheckReserves( $item ); > is( $status, 'Reserved', 'CheckReserves returns reserve without lookahead'); >-($status)=CheckReserves($item->itemnumber,undef,7); >+($status)=CheckReserves( $item, 7 ); > is( $status, 'Reserved', 'CheckReserves also returns reserve with lookahead'); > > # Test 9761b: Add a reserve with future date, CheckReserve should not return it >@@ -364,26 +361,26 @@ my $reserve_id = AddReserve( > reservation_date => $resdate, > } > ); >-($status)=CheckReserves($item->itemnumber,undef,undef); >+($status)=CheckReserves( $item ); > is( $status, '', 'CheckReserves returns no future reserve without lookahead'); > > # Test 9761c: Add a reserve with future date, CheckReserve should return it if lookahead is high enough >-($status)=CheckReserves($item->itemnumber,undef,3); >+($status)=CheckReserves( $item, 3 ); > is( $status, '', 'CheckReserves returns no future reserve with insufficient lookahead'); >-($status)=CheckReserves($item->itemnumber,undef,4); >+($status)=CheckReserves( $item, 4 ); > is( $status, 'Reserved', 'CheckReserves returns future reserve with sufficient lookahead'); > > # Test 9761d: Check ResFound message of AddReturn for future hold > # Note that AddReturn is in Circulation.pm, but this test really pertains to reserves; AddReturn uses the ConfirmFutureHolds pref when calling CheckReserves > # In this test we do not need an issued item; it is just a 'checkin' > t::lib::Mocks::mock_preference('ConfirmFutureHolds', 0); >-(my $doreturn, $messages)= AddReturn('97531',$branch_1); >+(my $doreturn, $messages)= AddReturn($testbarcode,$branch_1); > is($messages->{ResFound}//'', '', 'AddReturn does not care about future reserve when ConfirmFutureHolds is off'); > t::lib::Mocks::mock_preference('ConfirmFutureHolds', 3); >-($doreturn, $messages)= AddReturn('97531',$branch_1); >+($doreturn, $messages)= AddReturn($testbarcode,$branch_1); > is(exists $messages->{ResFound}?1:0, 0, 'AddReturn ignores future reserve beyond ConfirmFutureHolds days'); > t::lib::Mocks::mock_preference('ConfirmFutureHolds', 7); >-($doreturn, $messages)= AddReturn('97531',$branch_1); >+($doreturn, $messages)= AddReturn($testbarcode,$branch_1); > is(exists $messages->{ResFound}?1:0, 1, 'AddReturn considers future reserve within ConfirmFutureHolds days'); > > my $now_holder = $builder->build_object({ class => 'Koha::Patrons', value => { >@@ -399,9 +396,9 @@ my $now_reserve_id = AddReserve( > } > ); > my $which_highest; >-($status,$which_highest)=CheckReserves($item->itemnumber,undef,3); >+($status,$which_highest)=CheckReserves( $item, 3 ); > is( $which_highest->{reserve_id}, $now_reserve_id, 'CheckReserves returns lower priority current reserve with insufficient lookahead'); >-($status, $which_highest)=CheckReserves($item->itemnumber,undef,4); >+($status, $which_highest)=CheckReserves( $item, 4 ); > is( $which_highest->{reserve_id}, $reserve_id, 'CheckReserves returns higher priority future reserve with sufficient lookahead'); > ModReserve({ reserve_id => $now_reserve_id, rank => 'del', cancellation_reason => 'test reserve' }); > >@@ -586,7 +583,7 @@ AddReserve( > priority => 1, > } > ); >-my (undef, $canres, undef) = CheckReserves($item->itemnumber); >+my (undef, $canres, undef) = CheckReserves( $item ); > > is( CanReserveBeCanceledFromOpac(), undef, > 'CanReserveBeCanceledFromOpac should return undef if called without any parameter' >@@ -624,7 +621,7 @@ AddReserve( > priority => 1, > } > ); >-(undef, $canres, undef) = CheckReserves($item->itemnumber); >+(undef, $canres, undef) = CheckReserves( $item ); > > ModReserveAffect($item->itemnumber, $requesters{$branch_1}, 0); > $cancancel = CanReserveBeCanceledFromOpac($canres->{reserve_id}, $requesters{$branch_1}); >@@ -708,7 +705,7 @@ AddReserve( > } > ); > MoveReserve( $item->itemnumber, $borrowernumber ); >-($status)=CheckReserves( $item->itemnumber ); >+($status)=CheckReserves( $item ); > is( $status, '', 'MoveReserve filled hold'); > # hold from A waiting, today, no fut holds: MoveReserve should fill it > AddReserve( >@@ -721,7 +718,7 @@ AddReserve( > } > ); > MoveReserve( $item->itemnumber, $borrowernumber ); >-($status)=CheckReserves( $item->itemnumber ); >+($status)=CheckReserves( $item ); > is( $status, '', 'MoveReserve filled waiting hold'); > # hold from A pos 1, tomorrow, no fut holds: not filled > $resdate= dt_from_string(); >@@ -736,7 +733,7 @@ AddReserve( > } > ); > MoveReserve( $item->itemnumber, $borrowernumber ); >-($status)=CheckReserves( $item->itemnumber, undef, 1 ); >+($status)=CheckReserves( $item, 1 ); > is( $status, 'Reserved', 'MoveReserve did not fill future hold'); > $dbh->do('DELETE FROM reserves', undef, ($bibnum)); > # hold from A pos 1, tomorrow, fut holds=2: MoveReserve should fill it >@@ -751,7 +748,7 @@ AddReserve( > } > ); > MoveReserve( $item->itemnumber, $borrowernumber ); >-($status)=CheckReserves( $item->itemnumber, undef, 2 ); >+($status)=CheckReserves( $item, undef, 2 ); > is( $status, '', 'MoveReserve filled future hold now'); > # hold from A waiting, tomorrow, fut holds=2: MoveReserve should fill it > AddReserve( >@@ -764,7 +761,7 @@ AddReserve( > } > ); > MoveReserve( $item->itemnumber, $borrowernumber ); >-($status)=CheckReserves( $item->itemnumber, undef, 2 ); >+($status)=CheckReserves( $item, undef, 2 ); > is( $status, '', 'MoveReserve filled future waiting hold now'); > # hold from A pos 1, today+3, fut holds=2: MoveReserve should not fill it > $resdate= dt_from_string(); >@@ -779,7 +776,7 @@ AddReserve( > } > ); > MoveReserve( $item->itemnumber, $borrowernumber ); >-($status)=CheckReserves( $item->itemnumber, undef, 3 ); >+($status)=CheckReserves( $item, 3 ); > is( $status, 'Reserved', 'MoveReserve did not fill future hold of 3 days'); > $dbh->do('DELETE FROM reserves', undef, ($bibnum)); > >@@ -1241,7 +1238,7 @@ subtest 'CheckReserves additional tests' => sub { > ModReserveAffect( $item->itemnumber, $reserve1->borrowernumber, 1, > $reserve1->reserve_id ); > my ( $status, $matched_reserve, $possible_reserves ) = >- CheckReserves( $item->itemnumber ); >+ CheckReserves( $item ); > > is( $status, 'Transferred', "We found a reserve" ); > is( $matched_reserve->{reserve_id}, >@@ -1284,7 +1281,7 @@ subtest 'CheckReserves additional tests' => sub { > > ok( $reserve_id, "We can place a record level hold because one item is owned by patron's home library"); > t::lib::Mocks::mock_preference('ReservesControlBranch', 'ItemHomeLibrary'); >- ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A->itemnumber ); >+ ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A ); > is( $status, "", "We do not fill the hold with item A because it is not from the patron's homebranch"); > Koha::CirculationRules->set_rule({ > branchcode => $item_A->homebranch, >@@ -1292,13 +1289,13 @@ subtest 'CheckReserves additional tests' => sub { > rule_name => 'holdallowed', > rule_value => 'from_any_library' > }); >- ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A->itemnumber ); >+ ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A ); > is( $status, "Reserved", "We fill the hold with item A because item's branch rule says allow any"); > > > # Changing the control branch should change only the rule we get > t::lib::Mocks::mock_preference('ReservesControlBranch', 'PatronLibrary'); >- ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A->itemnumber ); >+ ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A ); > is( $status, "", "We do not fill the hold with item A because it is not from the patron's homebranch"); > Koha::CirculationRules->set_rule({ > branchcode => $patron_B->branchcode, >@@ -1306,7 +1303,7 @@ subtest 'CheckReserves additional tests' => sub { > rule_name => 'holdallowed', > rule_value => 'from_any_library' > }); >- ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A->itemnumber ); >+ ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A ); > is( $status, "Reserved", "We fill the hold with item A because patron's branch rule says allow any"); > > }; >-- >2.40.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 31735
:
141603
|
141604
|
141605
|
141606
|
141607
|
141608
|
141609
|
141610
|
141795
|
141796
|
141797
|
141798
|
141799
|
141800
|
141819
|
141820
|
141821
|
142865
|
142866
|
142867
|
142868
|
142869
|
142870
|
144628
|
144629
|
144630
|
144631
|
144632
|
144635
|
144721
|
144724
|
144784
|
144790
|
144791
|
144792
|
144798
|
144799
|
144800
|
144801
|
144802
|
144803
|
145028
|
145029
|
145030
|
145032
|
145033
|
145034
|
145035
|
146735
|
146736
|
146737
|
147124
|
147125
|
147126
|
147127
|
147128
|
147164
|
147165
|
147166
|
150611
|
150612
|
150613
|
150676
|
150677
|
150678
|
150699
|
150700
|
150701