From 728de37bc3ff6726443c66254081b147de089712 Mon Sep 17 00:00:00 2001 From: David Gustafsson 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 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 --- C4/Circulation.pm | 149 ++++---- C4/ILSDI/Services.pm | 12 +- C4/Reserves.pm | 101 ++--- C4/RotatingCollections.pm | 1 - C4/SIP/ILS/Transaction/Checkin.pm | 2 +- C4/SIP/ILS/Transaction/Renew.pm | 8 +- C4/SIP/Sip/MsgType.pm | 3 +- Koha/Account/Line.pm | 13 +- Koha/CirculationRules.pm | 2 +- 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 | 345 +++++++++--------- t/db_dependent/Holds.t | 8 +- 22 files changed, 361 insertions(+), 388 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 336ce9490a..2ae750b055 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; @@ -957,10 +957,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; @@ -982,18 +979,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; } } } @@ -1026,9 +1024,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->do_check_for_previous_checkout($item_unblessed) && $patron->wants_check_for_previous_checkout) { + $needsconfirmation{PREVISSUE} = 1; + } # # ITEM CHECKING @@ -1166,7 +1164,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 ) { @@ -2338,7 +2336,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 } ); @@ -2832,14 +2830,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 @@ -2858,19 +2855,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( @@ -2896,7 +2893,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; @@ -2940,9 +2936,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') ) { @@ -2954,7 +2949,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); @@ -2962,7 +2957,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 @@ -2980,15 +2978,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; } @@ -3220,7 +3218,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; @@ -3228,9 +3227,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 @@ -3238,12 +3238,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'}; @@ -3278,11 +3277,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 @@ -3293,10 +3294,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( @@ -3316,7 +3321,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' @@ -3325,9 +3330,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' ) { @@ -3340,14 +3345,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 @@ -3358,16 +3362,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( @@ -3392,7 +3394,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} ); @@ -3442,13 +3444,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, @@ -4385,7 +4388,7 @@ sub _CanBookBeAutoRenewed { } ); - if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) { + if ( $patron->is_expired && $patron->category->effective_BlockExpiredPatronOpacActions ) { return 'auto_account_expired'; } @@ -4421,7 +4424,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 f6bf6bcc14..9327e3bafd 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 6ff7149edc..7a4b21ae81 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 @@ -822,13 +822,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 @@ -850,65 +849,34 @@ 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; - - my $dont_trap = C4::Context->preference('TrapHoldsOnOrder') ? ($notforloan_per_item > 0) : ($notforloan_per_item && 1 ); - return if $dont_trap or $notforloan_per_itemtype; + return if grep { $_ eq $item->notforloan } @SkipHoldTrapOnNotForLoanValue; + + # TODO: Double check 'TrapHoldsOnOrder' should not also be performed on itemtype? + # or perhaps perform one check depeding on 'item-level_itypes'? + 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, @@ -920,8 +888,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 @@ -931,12 +899,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 = @@ -955,10 +921,8 @@ 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'); @@ -981,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 ); } @@ -1709,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. @@ -1730,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. @@ -1834,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 ) @@ -2050,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 a1f8df740d..18ba40a5c2 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 004aa05efa..e512350be8 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 9e5579ee87..e419258550 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) { @@ -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/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm index cad82e0b35..822be91cb0 100644 --- a/C4/SIP/Sip/MsgType.pm +++ b/C4/SIP/Sip/MsgType.pm @@ -1163,7 +1163,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 b940d6acef..adceb5cb5d 100644 --- a/Koha/Account/Line.pm +++ b/Koha/Account/Line.pm @@ -21,6 +21,7 @@ use Data::Dumper qw( Dumper ); use C4::Log qw( logaction ); use C4::Overdues qw( UpdateFine ); +use C4::Circulation qw( CanBookBeRenewed AddRenewal ReturnLostItem ); use Koha::Account::CreditType; use Koha::Account::DebitType; @@ -693,8 +694,7 @@ sub apply { ) ) { - C4::Circulation::ReturnLostItem( $self->borrowernumber, - $debit->itemnumber ); + ReturnLostItem( $self->borrowernumber, $debit->itemnumber ); } last if $available_credit == 0; @@ -1023,13 +1023,10 @@ sub renew_item { } my $itemnumber = $self->item->itemnumber; - my $borrowernumber = $self->patron->borrowernumber; - my ( $can_renew, $error ) = C4::Circulation::CanBookBeRenewed( - $borrowernumber, - $itemnumber - ); + my ( $can_renew, $error ) = CanBookBeRenewed( $self->patron, $self->item->checkout ); if ( $can_renew ) { - my $due_date = C4::Circulation::AddRenewal( + my $borrowernumber = $self->patron->borrowernumber; + my $due_date = AddRenewal( $borrowernumber, $itemnumber, $self->{branchcode}, diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index f96df496fb..96a1a23458 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -301,7 +301,7 @@ sub get_effective_rule_value { $categorycode // q{}, $branchcode // q{}, $itemtype // q{}; my $cached = $memory_cache->get_from_cache($cache_key); - return $cached if $cached; + return $cached if defined($cached); my $rule = $self->get_effective_rule($params); diff --git a/Koha/Hold.pm b/Koha/Hold.pm index a5a9d03003..80cafe86ff 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 ) = 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 e4a52b4cad..7eea31d03b 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 f495a4061f..f9434a127b 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 fd81f1240a..9fb85b437c 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 @@ Item is an onsite checkout [% CASE 'has_fine' %] Item has an outstanding fine +[% CASE 'item_issued_to_other_patron'%] + Item already issued to other borrower [% CASE %] Unknown error [% 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 9ab67912e9..ca9395d8d9 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 @@
  • This item is on hold for another borrower.
  • [% ELSIF error == 'item_denied_renewal' %]
  • Item renewal is not allowed.
  • + [% ELSIF error == 'item_issued_to_other_patron'%] +
  • Item already issued to other borrower.
  • [% ELSIF error == 'auto_too_soon' %]
  • This item is scheduled for auto renewal.
  • [% END %] diff --git a/misc/cronjobs/automatic_renewals.pl b/misc/cronjobs/automatic_renewals.pl index 4884eee2f3..d74edb6ea4 100755 --- a/misc/cronjobs/automatic_renewals.pl +++ b/misc/cronjobs/automatic_renewals.pl @@ -171,7 +171,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; @@ -185,16 +185,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 46a205029e..9e8efa0c5e 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 ba45ddd237..2b079957b9 100755 --- a/opac/opac-user.pl +++ b/opac/opac-user.pl @@ -187,7 +187,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 ) { @@ -220,7 +226,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'}, @@ -230,7 +236,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; @@ -248,6 +254,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 ac89db749c..e36db3d68b 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 b8e84f4704..bf5399aea2 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 470c02e796..fb09023643 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 0ba8d69742..e8fd7a68b6 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,17 +360,16 @@ 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'); - }; subtest "GetIssuingCharges tests" => sub { @@ -463,6 +462,7 @@ subtest "CanBookBeRenewed tests" => sub { surname => 'Renewal', categorycode => $patron_category->{categorycode}, branchcode => $branch, + autorenew_checkouts => 1, ); my %reserving_borrower_data = ( @@ -493,18 +493,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'; @@ -514,19 +512,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'); @@ -565,9 +561,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'); @@ -585,7 +581,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; @@ -600,7 +596,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(); @@ -616,17 +612,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)'); @@ -655,16 +651,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, 1, 'Can renew, item is marked not for loan, hold does not block'); $item_1->set({notforloan => 0, itype => $itemtype })->store; @@ -679,13 +675,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"); @@ -708,44 +704,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) } @@ -776,7 +772,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"); @@ -785,18 +781,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 ); @@ -817,14 +821,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, @@ -840,30 +844,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; @@ -880,26 +883,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) @@ -907,18 +910,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 @@ -926,14 +929,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; @@ -948,7 +951,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( { @@ -962,7 +965,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)' ); @@ -978,7 +981,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)' ); @@ -994,7 +997,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)' ); @@ -1010,7 +1013,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' ); @@ -1027,7 +1030,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)' ); @@ -1044,7 +1047,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)' ); @@ -1061,7 +1064,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' ); }; @@ -1077,7 +1080,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( { @@ -1105,7 +1108,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' ); @@ -1119,7 +1122,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' ); @@ -1133,7 +1136,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' ); @@ -1146,13 +1149,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' ); @@ -1187,37 +1190,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 { @@ -1231,7 +1230,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, @@ -1244,7 +1243,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( @@ -1259,7 +1258,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' @@ -1281,7 +1280,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' @@ -1299,7 +1298,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' @@ -1316,7 +1315,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' @@ -1338,7 +1337,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)'); @@ -1355,8 +1354,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( @@ -1378,21 +1378,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' ); @@ -1414,7 +1414,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' ); @@ -1423,9 +1423,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) @@ -1440,7 +1440,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' ); @@ -1478,11 +1478,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({ @@ -1492,7 +1492,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; @@ -1504,7 +1504,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; @@ -1516,7 +1516,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; @@ -1529,7 +1529,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; }; @@ -1571,7 +1571,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; @@ -1697,12 +1697,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', @@ -1726,12 +1726,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, $item_1->barcode ); + my $issue = AddIssue( $borrower1->unblessed, $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( @@ -1754,11 +1751,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( @@ -1772,11 +1769,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( @@ -1788,7 +1785,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( @@ -1799,7 +1796,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( @@ -1813,7 +1810,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( @@ -1830,7 +1827,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"); @@ -1844,7 +1841,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' ); @@ -1868,7 +1865,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' ); @@ -1889,17 +1886,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' ); } @@ -1916,24 +1911,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{} } @@ -2349,9 +2343,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; @@ -2399,7 +2392,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'); } @@ -2413,7 +2406,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'); } }; @@ -3966,7 +3959,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 ); @@ -3975,7 +3968,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 @@ -4160,14 +4153,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' ); @@ -5654,7 +5647,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' ); @@ -5663,7 +5656,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' ); @@ -5680,7 +5673,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' ); @@ -5688,13 +5681,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' ); diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 29e773e399..4e29bdd2ad 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -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' ); -- 2.35.1