From ddc2906b7735fda81ec8e267303a81a921b472e6 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 t/db_dependent/Holds.t t/db_dependent/Holds/LocalHoldsPriority.t t/db_dependent/Holds/HoldFulfillmentPolicy.t t/db_dependent/Holds/HoldItemtypeLimit.t t/db_dependent/Circulation/transferbook.t 2) Performe one or more checkouts for a patron, making sure that the circulation rules allows for renewals (for example by setting an earlier due-date). 3) Log in as this patron in OPAC and make sure the list of checkouts is displayed correctly, and that renewing an issue still works. Sponsored-by: Gothenburg University Library Signed-off-by: Nick Clemens Signed-off-by: Kyle M Hall Signed-off-by: David Nind Signed-off-by: Tomas Cohen Arazi --- C4/Circulation.pm | 149 ++++---- C4/ILSDI/Services.pm | 12 +- C4/Reserves.pm | 96 ++--- C4/RotatingCollections.pm | 1 - C4/SIP/ILS/Transaction/Checkin.pm | 2 +- C4/SIP/ILS/Transaction/Renew.pm | 10 +- C4/SIP/ILS/Transaction/RenewAll.pm | 7 +- C4/SIP/Sip/MsgType.pm | 3 +- Koha/Account/Line.pm | 7 +- Koha/Hold.pm | 2 +- Koha/REST/V1/Checkouts.pm | 15 +- circ/renew.pl | 27 +- .../prog/en/includes/renew_strings.inc | 2 + .../bootstrap/en/modules/opac-user.tt | 2 + misc/cronjobs/automatic_renewals.pl | 25 +- opac/opac-renew.pl | 3 +- opac/opac-user.pl | 13 +- opac/sco/sco-main.pl | 7 +- svc/checkouts | 4 +- svc/renew | 5 +- t/db_dependent/Circulation.t | 349 +++++++++--------- t/db_dependent/Circulation/Returns.t | 1 + t/db_dependent/Circulation/transferbook.t | 7 +- t/db_dependent/Holds.t | 26 +- t/db_dependent/Holds/HoldFulfillmentPolicy.t | 25 +- t/db_dependent/Holds/HoldItemtypeLimit.t | 10 +- t/db_dependent/Holds/LocalHoldsPriority.t | 20 +- t/db_dependent/Koha/Account/Line.t | 1 + t/db_dependent/Koha/Object.t | 3 + t/db_dependent/Letters/TemplateToolkit.t | 9 +- t/db_dependent/Reserves.t | 57 ++- 31 files changed, 438 insertions(+), 462 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 080ee3bc15..d1c44b97ad 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; @@ -959,10 +959,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; @@ -984,18 +981,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; } } } @@ -1028,9 +1026,9 @@ sub CanBookBeIssued { # CHECKPREVCHECKOUT: CHECK IF ITEM HAS EVER BEEN LENT TO PATRON # $patron = Koha::Patrons->find( $patron->borrowernumber ); # FIXME Refetch just in case, to avoid regressions. But must not be needed - my $wants_check = $patron->wants_check_for_previous_checkout; - $needsconfirmation{PREVISSUE} = 1 - if ($wants_check and $patron->do_check_for_previous_checkout($item_unblessed)); + if ( $patron->wants_check_for_previous_checkout && $patron->do_check_for_previous_checkout($item_unblessed) ) { + $needsconfirmation{PREVISSUE} = 1; + } # # ITEM CHECKING @@ -1168,7 +1166,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 ) { @@ -2386,7 +2384,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 } ); @@ -2883,14 +2881,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 @@ -2909,19 +2906,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( @@ -2947,7 +2944,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; @@ -2995,9 +2991,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') ) { @@ -3009,7 +3004,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); @@ -3017,7 +3012,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 @@ -3035,15 +3033,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; } @@ -3294,7 +3292,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; @@ -3302,9 +3301,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 @@ -3312,12 +3312,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'}; @@ -3352,11 +3351,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 @@ -3367,10 +3368,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( @@ -3390,7 +3395,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' @@ -3399,9 +3404,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' ) { @@ -3414,14 +3419,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 @@ -3432,16 +3436,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( @@ -3466,7 +3468,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} ); @@ -3516,13 +3518,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, @@ -4460,7 +4463,7 @@ sub _CanBookBeAutoRenewed { } ); - if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) { + if ( $patron->is_expired && $patron->category->effective_BlockExpiredPatronOpacActions ) { return 'auto_account_expired'; } @@ -4496,7 +4499,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 84aa00e864..8e64605a45 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -676,9 +676,9 @@ sub GetOtherReserves { my ($itemnumber) = @_; my $messages; my $nextreservinfo; - my ( undef, $checkreserves, undef ) = CheckReserves($itemnumber); + my $item = Koha::Items->find($itemnumber); + my ( undef, $checkreserves, undef ) = CheckReserves($item); if ($checkreserves) { - my $item = Koha::Items->find($itemnumber); if ( $item->holdingbranch ne $checkreserves->{'branchcode'} ) { $messages->{'transfert'} = $checkreserves->{'branchcode'}; #minus priorities of others reservs @@ -823,13 +823,12 @@ sub GetReserveStatus { =head2 CheckReserves - ($status, $matched_reserve, $possible_reserves) = &CheckReserves($itemnumber); - ($status, $matched_reserve, $possible_reserves) = &CheckReserves(undef, $barcode); - ($status, $matched_reserve, $possible_reserves) = &CheckReserves($itemnumber,undef,$lookahead); + ($status, $matched_reserve, $possible_reserves) = &CheckReserves($item); + ($status, $matched_reserve, $possible_reserves) = &CheckReserves($item, $lookahead); Find a book in the reserves. -C<$itemnumber> is the book's item number. +C<$item> is the book's item. C<$lookahead> is the number of days to look in advance for future reserves. As I understand it, C<&CheckReserves> looks for the given item in the @@ -851,65 +850,32 @@ table in the Koha database. =cut sub CheckReserves { - my ( $item, $barcode, $lookahead_days, $ignore_borrowers) = @_; - my $dbh = C4::Context->dbh; - my $sth; - my $select; - if (C4::Context->preference('item-level_itypes')){ - $select = " - SELECT items.biblionumber, - items.biblioitemnumber, - itemtypes.notforloan, - items.notforloan AS itemnotforloan, - items.itemnumber, - items.damaged, - items.homebranch, - items.holdingbranch - FROM items - LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber - LEFT JOIN itemtypes ON items.itype = itemtypes.itemtype - "; - } - else { - $select = " - SELECT items.biblionumber, - items.biblioitemnumber, - itemtypes.notforloan, - items.notforloan AS itemnotforloan, - items.itemnumber, - items.damaged, - items.homebranch, - items.holdingbranch - FROM items - LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber - LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype - "; - } - - if ($item) { - $sth = $dbh->prepare("$select WHERE itemnumber = ?"); - $sth->execute($item); - } - else { - $sth = $dbh->prepare("$select WHERE barcode = ?"); - $sth->execute($barcode); - } + my ( $item, $lookahead_days, $ignore_borrowers ) = @_; # note: we get the itemnumber because we might have started w/ just the barcode. Now we know for sure we have it. - my ( $biblio, $bibitem, $notforloan_per_itemtype, $notforloan_per_item, $itemnumber, $damaged, $item_homebranch, $item_holdingbranch ) = $sth->fetchrow_array; - return if ( $damaged && !C4::Context->preference('AllowHoldsOnDamagedItems') ); - return unless $itemnumber; # bail if we got nothing. + return unless $item; # bail if we got nothing. + + return if ( $item->damaged && !C4::Context->preference('AllowHoldsOnDamagedItems') ); + # if item is not for loan it cannot be reserved either..... # except where items.notforloan < 0 : This indicates the item is holdable. my @SkipHoldTrapOnNotForLoanValue = split( '\|', C4::Context->preference('SkipHoldTrapOnNotForLoanValue') ); - return if grep { $_ eq $notforloan_per_item } @SkipHoldTrapOnNotForLoanValue; + return if grep { $_ eq $item->notforloan } @SkipHoldTrapOnNotForLoanValue; - my $dont_trap = C4::Context->preference('TrapHoldsOnOrder') ? ($notforloan_per_item > 0) : ($notforloan_per_item && 1 ); - return if $dont_trap or $notforloan_per_itemtype; + my $dont_trap = C4::Context->preference('TrapHoldsOnOrder') ? $item->notforloan > 0 : $item->notforloan; + if ( !$dont_trap ) { + my $item_type = $item->effective_itemtype; + if ( $item_type ) { + return if Koha::ItemTypes->find( $item_type )->notforloan; + } + } + else { + return; + } # Find this item in the reserves - my @reserves = _Findgroupreserve( $bibitem, $biblio, $itemnumber, $lookahead_days, $ignore_borrowers); + my @reserves = _Findgroupreserve( $item->biblionumber, $item->itemnumber, $lookahead_days, $ignore_borrowers); # $priority and $highest are used to find the most important item # in the list returned by &_Findgroupreserve. (The lower $priority, @@ -921,8 +887,8 @@ sub CheckReserves { my $LocalHoldsPriority = C4::Context->preference('LocalHoldsPriority'); my $LocalHoldsPriorityPatronControl = C4::Context->preference('LocalHoldsPriorityPatronControl'); my $LocalHoldsPriorityItemControl = C4::Context->preference('LocalHoldsPriorityItemControl'); - my $priority = 10000000; + foreach my $res (@reserves) { if ($res->{'found'} && $res->{'found'} eq 'W') { return ( "Waiting", $res, \@reserves ); # Found it, it is waiting @@ -932,12 +898,10 @@ sub CheckReserves { return ( "Transferred", $res, \@reserves ); # Found determinated hold, e. g. the transferred one } else { my $patron; - my $item; my $local_hold_match; if ($LocalHoldsPriority) { $patron = Koha::Patrons->find( $res->{borrowernumber} ); - $item = Koha::Items->find($itemnumber); unless ($item->exclude_from_local_holds_priority || $patron->category->exclude_from_local_holds_priority) { my $local_holds_priority_item_branchcode = @@ -956,10 +920,9 @@ sub CheckReserves { # See if this item is more important than what we've got so far if ( ( $res->{'priority'} && $res->{'priority'} < $priority ) || $local_hold_match ) { - $item ||= Koha::Items->find($itemnumber); next if $res->{item_group_id} && ( !$item->item_group || $item->item_group->id != $res->{item_group_id} ); next if $res->{itemtype} && $res->{itemtype} ne $item->effective_itemtype; - $patron ||= Koha::Patrons->find( $res->{borrowernumber} ); + $patron //= Koha::Patrons->find( $res->{borrowernumber} ); my $branch = GetReservesControlBranch( $item->unblessed, $patron->unblessed ); my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$item->effective_itemtype); next if ($branchitemrule->{'holdallowed'} eq 'not_allowed'); @@ -982,7 +945,7 @@ sub CheckReserves { # If we get this far, then no exact match was found. # We return the most important (i.e. next) reservation. if ($highest) { - $highest->{'itemnumber'} = $item; + $highest->{'itemnumber'} = $item->itemnumber; return ( "Reserved", $highest, \@reserves ); } @@ -1710,7 +1673,7 @@ sub _FixPriority { =head2 _Findgroupreserve - @results = &_Findgroupreserve($biblioitemnumber, $biblionumber, $itemnumber, $lookahead, $ignore_borrowers); + @results = &_Findgroupreserve($biblionumber, $itemnumber, $lookahead, $ignore_borrowers); Looks for a holds-queue based item-specific match first, then for a holds-queue title-level match, returning the first match found. If neither, then we look for non-holds-queue based holds. @@ -1731,7 +1694,7 @@ All return values will respect any borrowernumbers passed as arrayref in $ignore =cut sub _Findgroupreserve { - my ( $bibitem, $biblio, $itemnumber, $lookahead, $ignore_borrowers) = @_; + my ( $biblionumber, $itemnumber, $lookahead, $ignore_borrowers) = @_; my $dbh = C4::Context->dbh; # TODO: consolidate at least the SELECT portion of the first 2 queries to a common $select var. @@ -1835,7 +1798,7 @@ sub _Findgroupreserve { ORDER BY priority }; $sth = $dbh->prepare($query); - $sth->execute( $biblio, $itemnumber, $lookahead||0); + $sth->execute( $biblionumber, $itemnumber, $lookahead||0); @results = (); while ( my $data = $sth->fetchrow_hashref ) { push( @results, $data ) @@ -2051,7 +2014,8 @@ sub MoveReserve { $cancelreserve //= 0; my $lookahead = C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds - my ( $restype, $res, undef ) = CheckReserves( $itemnumber, undef, $lookahead ); + my $item = Koha::Items->find($itemnumber); + my ( $restype, $res, undef ) = CheckReserves( $item, $lookahead ); return unless $res; my $biblionumber = $res->{biblionumber}; diff --git a/C4/RotatingCollections.pm b/C4/RotatingCollections.pm index 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..f48bee4cfe 100644 --- a/C4/SIP/ILS/Transaction/Renew.pm +++ b/C4/SIP/ILS/Transaction/Renew.pm @@ -31,8 +31,9 @@ sub new { sub do_renew_for { my $self = shift; - my $borrower = shift; - my ($renewokay,$renewerror) = CanBookBeRenewed($borrower->{borrowernumber},$self->{item}->{itemnumber}); + my $patron = shift; + my $checkout = Koha::Checkouts->find({ itemnumber => $self->{item}->{itemnumber} }); + my ($renewokay,$renewerror) = CanBookBeRenewed($patron, $checkout); if ($renewokay) { # ok so far check charges my ($fee, undef) = GetIssuingCharges($self->{item}->{itemnumber}, $self->{patron}->{borrowernumber}); if ($fee > 0) { @@ -45,7 +46,7 @@ sub do_renew_for { } if ($renewokay){ - my $issue = AddIssue( $borrower, $self->{item}->id, undef, 0 ); + my $issue = AddIssue( $patron, $self->{item}->id, undef, 0 ); $self->{due} = $self->duedatefromissue($issue, $self->{item}->{itemnumber}); $self->renewal_ok(1); } else { @@ -53,6 +54,7 @@ sub do_renew_for { $renewerror=~s/too_many/Item has reached maximum renewals/; $renewerror=~s/too_unseen/Item has reached maximum consecutive renewals without being seen/; $renewerror=~s/item_denied_renewal/Item renewal is not allowed/; + $renewerror=~s/item_issued_to_other_patron/Item already issued to other borrower/; $self->screen_msg($renewerror); $self->renewal_ok(0); } @@ -64,7 +66,7 @@ sub do_renew { my $self = shift; my $patron = Koha::Patrons->find( $self->{patron}->borrowernumber ); $patron or return; # FIXME we should log that - return $self->do_renew_for($patron->unblessed); + return $self->do_renew_for($patron); } 1; diff --git a/C4/SIP/ILS/Transaction/RenewAll.pm b/C4/SIP/ILS/Transaction/RenewAll.pm index 5acd53efb7..12cefea25c 100644 --- a/C4/SIP/ILS/Transaction/RenewAll.pm +++ b/C4/SIP/ILS/Transaction/RenewAll.pm @@ -33,12 +33,11 @@ sub new { sub do_renew_all { my $self = shift; - my $patron = $self->{patron}; # SIP's patron - my $borrower = Koha::Patrons->find( { cardnumber => $patron->id } )->unblessed; # Koha's patron + my $patron = Koha::Patrons->find( $self->{patron}->{borrowernumber} ); my $all_ok = 1; $self->{renewed} = []; $self->{unrenewed} = []; - foreach my $itemx ( @{ $patron->{items} } ) { + foreach my $itemx ( @{ $self->{patron}->{items} } ) { my $item_id = $itemx->{barcode}; my $item = C4::SIP::ILS::Item->new($item_id); if ( !defined($item) ) { @@ -54,7 +53,7 @@ sub do_renew_all { next; } $self->{item} = $item; - $self->do_renew_for($borrower); + $self->do_renew_for($patron); if ( $self->renewal_ok ) { $item->{due_date} = $self->{due}; push @{ $self->{renewed} }, $item_id; diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm index 6e1e284976..09034a7f86 100644 --- a/C4/SIP/Sip/MsgType.pm +++ b/C4/SIP/Sip/MsgType.pm @@ -1167,7 +1167,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 cab0a3cae3..3a364ac8d0 100644 --- a/Koha/Account/Line.pm +++ b/Koha/Account/Line.pm @@ -20,7 +20,6 @@ use Modern::Perl; use Data::Dumper qw( Dumper ); use C4::Log qw( logaction ); -use C4::Overdues qw( UpdateFine ); use Koha::Account::CreditType; use Koha::Account::DebitType; @@ -1022,12 +1021,12 @@ sub renew_item { } my $itemnumber = $self->item->itemnumber; - my $borrowernumber = $self->patron->borrowernumber; my ( $can_renew, $error ) = C4::Circulation::CanBookBeRenewed( - $borrowernumber, - $itemnumber + $self->patron, + $self->item->checkout ); if ( $can_renew ) { + my $borrowernumber = $self->patron->borrowernumber; my $due_date = C4::Circulation::AddRenewal( $borrowernumber, $itemnumber, diff --git a/Koha/Hold.pm b/Koha/Hold.pm index d832c2097b..ad9560ff39 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -762,7 +762,7 @@ sub cancel { ); if ($autofill_next) { - my ( undef, $next_hold ) = C4::Reserves::CheckReserves( $self->itemnumber ); + my ( undef, $next_hold ) = C4::Reserves::CheckReserves( $self->item ); if ($next_hold) { my $is_transfer = $self->branchcode ne $next_hold->{branchcode}; diff --git a/Koha/REST/V1/Checkouts.pm b/Koha/REST/V1/Checkouts.pm index 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 ab56a84657..27e1b6b07b 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt @@ -152,6 +152,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 bf048c09d0..04488bf6e9 100755 --- a/misc/cronjobs/automatic_renewals.pl +++ b/misc/cronjobs/automatic_renewals.pl @@ -172,7 +172,7 @@ while ( my $auto_renew = $auto_renews->next ) { } # CanBookBeRenewed returns 'auto_renew' when the renewal should be done by this script - my ( $ok, $error ) = CanBookBeRenewed( $auto_renew->borrowernumber, $auto_renew->itemnumber, undef, 1 ); + my ( $ok, $error ) = CanBookBeRenewed( $auto_renew->patron, $auto_renew, undef, 1 ); my $updated; if ( $error eq 'auto_renew' ) { $updated = 1; @@ -187,16 +187,19 @@ while ( my $auto_renew = $auto_renews->next ) { } push @{ $report{ $auto_renew->borrowernumber } }, $auto_renew if ( $wants_messages ) && !$wants_digest; - } elsif ( $error eq 'too_many' - or $error eq 'on_reserve' - or $error eq 'restriction' - or $error eq 'overdue' - or $error eq 'too_unseen' - or $error eq 'auto_account_expired' - or $error eq 'auto_too_late' - or $error eq 'auto_too_much_oweing' - or $error eq 'auto_too_soon' - or $error eq 'item_denied_renewal' ) { + } elsif ( + $error eq 'too_many' || + $error eq 'on_reserve' || + $error eq 'restriction' || + $error eq 'overdue' || + $error eq 'too_unseen' || + $error eq 'auto_account_expired' || + $error eq 'auto_too_late' || + $error eq 'auto_too_much_oweing' || + $error eq 'auto_too_soon' || + $error eq 'item_denied_renewal' || + $error eq 'item_issued_to_other_patron' + ) { if ( $verbose ) { say sprintf "Issue id: %s for borrower: %s and item: %s %s not be renewed. (%s)", $auto_renew->issue_id, $auto_renew->borrowernumber, $auto_renew->itemnumber, $confirm ? 'will' : 'would', $error; diff --git a/opac/opac-renew.pl b/opac/opac-renew.pl index 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 494f481224..b39b7493c0 100755 --- a/opac/opac-user.pl +++ b/opac/opac-user.pl @@ -193,7 +193,13 @@ my $overdues_count = 0; my @overdues; my @issuedat; my $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search_with_localization->unblessed } }; -my $pending_checkouts = $patron->pending_checkouts->search({}, { order_by => [ { -desc => 'date_due' }, { -asc => 'issue_id' } ] }); +my $pending_checkouts = $patron->pending_checkouts->search( + {}, + { + order_by => [ { -desc => 'date_due' }, { -asc => 'issue_id' } ], + prefetch => 'item' + } +); my $are_renewable_items = 0; if ( $pending_checkouts->count ) { # Useless test while ( my $c = $pending_checkouts->next ) { @@ -226,7 +232,7 @@ if ( $pending_checkouts->count ) { # Useless test $issue->{rentalfines} = $rental_fines->total_outstanding; # check if item is renewable - my ($status,$renewerror,$info) = CanBookBeRenewed( $borrowernumber, $issue->{'itemnumber'} ); + my ($status, $renewerror, $info) = CanBookBeRenewed( $patron, $c ); ( $issue->{'renewcount'}, $issue->{'renewsallowed'}, @@ -236,7 +242,7 @@ if ( $pending_checkouts->count ) { # Useless test $issue->{'unseenleft'} ) = GetRenewCount($borrowernumber, $issue->{'itemnumber'}); ( $issue->{'renewalfee'}, $issue->{'renewalitemtype'} ) = GetIssuingCharges( $issue->{'itemnumber'}, $borrowernumber ); - $issue->{itemtype_object} = Koha::ItemTypes->find( Koha::Items->find( $issue->{itemnumber} )->effective_itemtype ); + $issue->{itemtype_object} = Koha::ItemTypes->find( $c->item->effective_itemtype ); if($status && C4::Context->preference("OpacRenewalAllowed")){ $are_renewable_items = 1; $issue->{'status'} = $status; @@ -254,6 +260,7 @@ if ( $pending_checkouts->count ) { # Useless test $issue->{'auto_too_late'} = 1 if $renewerror eq 'auto_too_late'; $issue->{'auto_too_much_oweing'} = 1 if $renewerror eq 'auto_too_much_oweing'; $issue->{'item_denied_renewal'} = 1 if $renewerror eq 'item_denied_renewal'; + $issue->{'item_issued_to_other_patron'} = 1 if $renewerror eq 'item_issued_to_other_patron'; if ( $renewerror eq 'too_soon' ) { $issue->{'too_soon'} = 1; diff --git a/opac/sco/sco-main.pl b/opac/sco/sco-main.pl index 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 1bbefe78a4..3cd4ded868 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -310,7 +310,7 @@ subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers a ); # Patrons from three different branches - my $patron_borrower = $builder->build_object({ class => 'Koha::Patrons' }); + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); my $patron_hold_1 = $builder->build_object({ class => 'Koha::Patrons' }); my $patron_hold_2 = $builder->build_object({ class => 'Koha::Patrons' }); my $biblio = $builder->build_sample_biblio(); @@ -318,7 +318,7 @@ subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers a # Item at each patron branch my $item_1 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber, - homebranch => $patron_borrower->branchcode + homebranch => $patron->branchcode }); my $item_2 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber, @@ -329,7 +329,7 @@ subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers a homebranch => $patron_hold_1->branchcode }); - my $issue = AddIssue( $patron_borrower->unblessed, $item_1->barcode); + my $issue = AddIssue( $patron->unblessed, $item_1->barcode); my $datedue = dt_from_string( $issue->date_due() ); is (defined $issue->date_due(), 1, "Item 1 checked out, due date: " . $issue->date_due() ); @@ -360,13 +360,13 @@ subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers a ); t::lib::Mocks::mock_preference('AllowRenewalIfOtherItemsAvailable', 0 ); - my ( $renewokay, $error ) = CanBookBeRenewed($patron_borrower->borrowernumber, $item_1->itemnumber); + my ( $renewokay, $error ) = CanBookBeRenewed($patron, $issue); is( $renewokay, 0, 'Cannot renew, reserved'); is( $error, 'on_reserve', 'Cannot renew, reserved (returned error is on_reserve)'); t::lib::Mocks::mock_preference('AllowRenewalIfOtherItemsAvailable', 1 ); - ( $renewokay, $error ) = CanBookBeRenewed($patron_borrower->borrowernumber, $item_1->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($patron, $issue); is( $renewokay, 1, 'Can renew, two items available for two holds'); is( $error, undef, 'Can renew, each reserve has an item'); @@ -374,10 +374,9 @@ subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers a my $hold = Koha::Holds->find( $reserve_1 ); $hold->itemnumber( $item_1->itemnumber )->store; - ( $renewokay, $error ) = CanBookBeRenewed($patron_borrower->borrowernumber, $item_1->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($patron, $issue); is( $renewokay, 0, 'Cannot renew when there is an item specific hold'); is( $error, 'on_reserve', 'Cannot renew, only this item can fill the reserve'); - }; subtest "GetIssuingCharges tests" => sub { @@ -470,6 +469,7 @@ subtest "CanBookBeRenewed tests" => sub { surname => 'Renewal', categorycode => $patron_category->{categorycode}, branchcode => $branch, + autorenew_checkouts => 1, ); my %reserving_borrower_data = ( @@ -500,18 +500,16 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => $patron_category->{categorycode}, branchcode => $branch, dateexpiry => dt_from_string->subtract( months => 1 ), + autorenew_checkouts => 1, ); - my $renewing_borrowernumber = Koha::Patron->new(\%renewing_borrower_data)->store->borrowernumber; + my $renewing_borrower_obj = Koha::Patron->new(\%renewing_borrower_data)->store; + my $renewing_borrowernumber = $renewing_borrower_obj->borrowernumber; my $reserving_borrowernumber = Koha::Patron->new(\%reserving_borrower_data)->store->borrowernumber; my $hold_waiting_borrowernumber = Koha::Patron->new(\%hold_waiting_borrower_data)->store->borrowernumber; - my $restricted_borrowernumber = Koha::Patron->new(\%restricted_borrower_data)->store->borrowernumber; - my $expired_borrowernumber = Koha::Patron->new(\%expired_borrower_data)->store->borrowernumber; + my $restricted_borrower_obj = Koha::Patron->new(\%restricted_borrower_data)->store; - my $renewing_borrower_obj = Koha::Patrons->find( $renewing_borrowernumber ); - my $renewing_borrower = $renewing_borrower_obj->unblessed; - my $restricted_borrower = Koha::Patrons->find( $restricted_borrowernumber )->unblessed; - my $expired_borrower = Koha::Patrons->find( $expired_borrowernumber )->unblessed; + my $expired_borrower_obj = Koha::Patron->new(\%expired_borrower_data)->store; my $bibitems = ''; my $priority = '1'; @@ -521,19 +519,17 @@ subtest "CanBookBeRenewed tests" => sub { my $checkitem = undef; my $found = undef; - my $issue = AddIssue( $renewing_borrower, $item_1->barcode); - my $datedue = dt_from_string( $issue->date_due() ); - is (defined $issue->date_due(), 1, "Item 1 checked out, due date: " . $issue->date_due() ); - - my $issue2 = AddIssue( $renewing_borrower, $item_2->barcode); - $datedue = dt_from_string( $issue->date_due() ); - is (defined $issue2, 1, "Item 2 checked out, due date: " . $issue2->date_due()); + my $issue_1 = AddIssue( $renewing_borrower_obj->unblessed, $item_1->barcode); + my $datedue = dt_from_string( $issue_1->date_due() ); + is (defined $issue_1->date_due(), 1, "Item 1 checked out, due date: " . $issue_1->date_due() ); + my $issue_2 = AddIssue( $renewing_borrower_obj->unblessed, $item_2->barcode); + is (defined $issue_2, 1, "Item 2 checked out, due date: " . $issue_2->date_due()); my $borrowing_borrowernumber = Koha::Checkouts->find( { itemnumber => $item_1->itemnumber } )->borrowernumber; - is ($borrowing_borrowernumber, $renewing_borrowernumber, "Item checked out to $renewing_borrower->{firstname} $renewing_borrower->{surname}"); + is ($borrowing_borrowernumber, $renewing_borrowernumber, "Item checked out to ".$renewing_borrower_obj->firstname." ".$renewing_borrower_obj->surname); - my ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1); + my ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1, 1); is( $renewokay, 1, 'Can renew, no holds for this title or item'); @@ -572,9 +568,9 @@ subtest "CanBookBeRenewed tests" => sub { } ); t::lib::Mocks::mock_preference('AllowRenewalIfOtherItemsAvailable', 1 ); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); is( $renewokay, 1, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_2->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_2); is( $renewokay, 1, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); @@ -592,7 +588,7 @@ subtest "CanBookBeRenewed tests" => sub { found => $found, } ); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); is( $renewokay, 0, 'Renewal not possible when single patron\'s holds exceed the number of available items'); Koha::Holds->find($reserve_id)->delete; @@ -607,7 +603,7 @@ subtest "CanBookBeRenewed tests" => sub { reservedate => '1999-01-01', } ); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); is( $renewokay, 0, 'Bug 13919 - Renewal possible with item level hold on item'); $hold->delete(); @@ -623,17 +619,17 @@ subtest "CanBookBeRenewed tests" => sub { found => 'W' } ); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); is( $renewokay, 0, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_2->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_2); is( $renewokay, 0, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); t::lib::Mocks::mock_preference('AllowRenewalIfOtherItemsAvailable', 0 ); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); is( $renewokay, 0, '(Bug 10663) Cannot renew, reserved'); is( $error, 'on_reserve', '(Bug 10663) Cannot renew, reserved (returned error is on_reserve)'); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_2->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_2); is( $renewokay, 0, '(Bug 10663) Cannot renew, reserved'); is( $error, 'on_reserve', '(Bug 10663) Cannot renew, reserved (returned error is on_reserve)'); @@ -662,16 +658,16 @@ subtest "CanBookBeRenewed tests" => sub { } ); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1, 1); is( $renewokay, 0, '(Bug 10663) Cannot renew, item reserved'); is( $error, 'on_reserve', '(Bug 10663) Cannot renew, item reserved (returned error is on_reserve)'); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_2->itemnumber, 1); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_2, 1); is( $renewokay, 1, 'Can renew item 2, item-level hold is on item 1'); # Items can't fill hold for reasons - $item_1->notforloan(1)->store; - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1); + $issue_1->item->notforloan(1)->store; + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1, 1); is( $renewokay, 0, 'Cannot renew, item is marked not for loan, but an item specific hold always blocks'); $item_1->set({notforloan => 0, itype => $itemtype })->store; @@ -686,13 +682,13 @@ subtest "CanBookBeRenewed tests" => sub { itype => $itemtype, } ); - my $datedue5 = AddIssue($restricted_borrower, $item_5->barcode); - is (defined $datedue5, 1, "Item with date due checked out, due date: $datedue5"); + my $issue_5 = AddIssue($restricted_borrower_obj->unblessed, $item_5->barcode); + is (defined $issue_5, 1, "Item with date due checked out, due date: ". $issue_5->date_due); t::lib::Mocks::mock_preference('RestrictionBlockRenewing','1'); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_2->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_2); is( $renewokay, 1, '(Bug 8236), Can renew, user is not restricted'); - ( $renewokay, $error ) = CanBookBeRenewed($restricted_borrowernumber, $item_5->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($restricted_borrower_obj, $issue_5); is( $renewokay, 0, '(Bug 8236), Cannot renew, user is restricted'); is( $error, 'restriction', "Correct error returned"); @@ -715,44 +711,44 @@ subtest "CanBookBeRenewed tests" => sub { } ); - my $datedue6 = AddIssue( $renewing_borrower, $item_6->barcode); - is (defined $datedue6, 1, "Item 2 checked out, due date: ".$datedue6->date_due); + my $issue_6 = AddIssue( $renewing_borrower_obj->unblessed, $item_6->barcode); + is (defined $issue_6, 1, "Item 2 checked out, due date: ".$issue_6->date_due); my $now = dt_from_string(); my $five_weeks = DateTime::Duration->new(weeks => 5); my $five_weeks_ago = $now - $five_weeks; t::lib::Mocks::mock_preference('finesMode', 'production'); - my $passeddatedue1 = AddIssue($renewing_borrower, $item_7->barcode, $five_weeks_ago); - is (defined $passeddatedue1, 1, "Item with passed date due checked out, due date: " . $passeddatedue1->date_due); + my $issue_7 = AddIssue($renewing_borrower_obj->unblessed, $item_7->barcode, $five_weeks_ago); + is (defined $issue_7, 1, "Item with passed date due checked out, due date: " . $issue_7->date_due); t::lib::Mocks::mock_preference('OverduesBlockRenewing','allow'); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_6); is( $renewokay, 1, '(Bug 8236), Can renew, this item is not overdue'); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_7); is( $renewokay, 1, '(Bug 8236), Can renew, this item is overdue but not pref does not block'); t::lib::Mocks::mock_preference('OverduesBlockRenewing','block'); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_6); is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is not overdue but patron has overdues'); is( $error, 'overdue', "Correct error returned"); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_7); is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue so patron has overdues'); is( $error, 'overdue', "Correct error returned"); t::lib::Mocks::mock_preference('OverduesBlockRenewing','blockitem'); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_6); is( $renewokay, 1, '(Bug 8236), Can renew, this item is not overdue'); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_7); is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue'); is( $error, 'overdue', "Correct error returned"); - my ( $fine ) = CalcFine( $item_7->unblessed, $renewing_borrower->{categorycode}, $branch, $five_weeks_ago, $now ); + my ( $fine ) = CalcFine( $item_7->unblessed, $renewing_borrower_obj->categorycode, $branch, $five_weeks_ago, $now ); C4::Overdues::UpdateFine( { - issue_id => $passeddatedue1->id(), + issue_id => $issue_7->id(), itemnumber => $item_7->itemnumber, - borrowernumber => $renewing_borrower->{borrowernumber}, + borrowernumber => $renewing_borrower_obj->borrowernumber, amount => $fine, due => Koha::DateUtils::output_pref($five_weeks_ago) } @@ -783,7 +779,7 @@ subtest "CanBookBeRenewed tests" => sub { my $old_log_size = Koha::ActionLogs->count( \%params_renewal ); my $dt = dt_from_string(); Time::Fake->offset( $dt->epoch ); - my $datedue1 = AddRenewal( $renewing_borrower->{borrowernumber}, $item_7->itemnumber, $branch ); + my $datedue1 = AddRenewal( $renewing_borrower_obj->borrowernumber, $item_7->itemnumber, $branch ); my $new_log_size = Koha::ActionLogs->count( \%params_renewal ); is ($new_log_size, $old_log_size, 'renew log not added because of the syspref RenewalLog'); isnt (DateTime->compare($datedue1, $dt), 0, "AddRenewal returned a good duedate"); @@ -792,18 +788,26 @@ subtest "CanBookBeRenewed tests" => sub { t::lib::Mocks::mock_preference('RenewalLog', 1); $date = output_pref( { dt => dt_from_string(), dateonly => 1, dateformat => 'iso' } ); $old_log_size = Koha::ActionLogs->count( \%params_renewal ); - AddRenewal( $renewing_borrower->{borrowernumber}, $item_7->itemnumber, $branch ); + AddRenewal( $renewing_borrower_obj->borrowernumber, $item_7->itemnumber, $branch ); $new_log_size = Koha::ActionLogs->count( \%params_renewal ); is ($new_log_size, $old_log_size + 1, 'renew log successfully added'); - my $fines = Koha::Account::Lines->search( { borrowernumber => $renewing_borrower->{borrowernumber}, itemnumber => $item_7->itemnumber } ); + my $fines = Koha::Account::Lines->search( { borrowernumber => $renewing_borrower_obj->borrowernumber, itemnumber => $item_7->itemnumber } ); is( $fines->count, 1, 'AddRenewal left fine' ); is( $fines->next->status, 'RENEWED', 'Fine on renewed item is closed out properly' ); $fines->delete(); my $old_issue_log_size = Koha::ActionLogs->count( \%params_issue ); my $old_renew_log_size = Koha::ActionLogs->count( \%params_renewal ); - AddIssue( $renewing_borrower,$item_7->barcode,Koha::DateUtils::output_pref({str=>$datedue6->date_due, dateformat =>'iso'}),0,$date, 0, undef ); + AddIssue( + $renewing_borrower_obj->unblessed, + $item_7->barcode, + Koha::DateUtils::output_pref({str=>$issue_6->date_due, dateformat =>'iso'}), + 0, + $date, + 0, + undef + ); # TODO: Already issued??? $new_log_size = Koha::ActionLogs->count( \%params_renewal ); is ($new_log_size, $old_renew_log_size + 1, 'renew log successfully added when renewed via issuing'); $new_log_size = Koha::ActionLogs->count( \%params_issue ); @@ -824,14 +828,14 @@ subtest "CanBookBeRenewed tests" => sub { } ); - $issue = AddIssue( $renewing_borrower, $item_4->barcode, undef, undef, undef, undef, { auto_renew => 1 } ); + my $issue_4 = AddIssue( $renewing_borrower_obj->unblessed, $item_4->barcode, undef, undef, undef, undef, { auto_renew => 1 } ); my $info; ( $renewokay, $error, $info ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic and premature' ); is( $error, 'auto_too_soon', 'Bug 14101: Cannot renew, renewal is automatic and premature, "No renewal before" = undef (returned code is auto_too_soon)' ); - is( $info->{soonest_renew_date} , dt_from_string($issue->date_due), "Due date is returned as earliest renewal date when error is 'auto_too_soon'" ); + is( $info->{soonest_renew_date} , dt_from_string($issue_4->date_due), "Due date is returned as earliest renewal date when error is 'auto_too_soon'" ); AddReserve( { branchcode => $branch, @@ -847,30 +851,29 @@ subtest "CanBookBeRenewed tests" => sub { found => $found } ); - ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); is( $renewokay, 0, 'Still should not be able to renew' ); is( $error, 'on_reserve', 'returned code is on_reserve, reserve checked when not checking for cron' ); - ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, undef, 1 ); + ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4, undef, 1 ); is( $renewokay, 0, 'Still should not be able to renew' ); is( $error, 'auto_too_soon', 'returned code is auto_too_soon, reserve not checked when checking for cron' ); - is( $info->{soonest_renew_date}, dt_from_string($issue->date_due), "Due date is returned as earliest renewal date when error is 'auto_too_soon'" ); - ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1 ); + is( $info->{soonest_renew_date}, dt_from_string($issue_4->date_due), "Due date is returned as earliest renewal date when error is 'auto_too_soon'" ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4, 1 ); is( $renewokay, 0, 'Still should not be able to renew' ); is( $error, 'on_reserve', 'returned code is on_reserve, auto_too_soon limit is overridden' ); - ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1, 1 ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4, 1, 1 ); is( $renewokay, 0, 'Still should not be able to renew' ); is( $error, 'on_reserve', 'returned code is on_reserve, auto_too_soon limit is overridden' ); $dbh->do('UPDATE circulation_rules SET rule_value = 0 where rule_name = "norenewalbefore"'); Koha::Cache::Memory::Lite->flush(); - ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1 ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4, 1 ); is( $renewokay, 0, 'Still should not be able to renew' ); is( $error, 'on_reserve', 'returned code is on_reserve, auto_renew only happens if not on reserve' ); ModReserveCancelAll($item_4->itemnumber, $reserving_borrowernumber); - - + $renewing_borrower_obj = Koha::Patrons->find($renewing_borrower_obj->borrowernumber); $renewing_borrower_obj->autorenew_checkouts(0)->store; - ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); is( $renewokay, 1, 'No renewal before is undef, but patron opted out of auto_renewal' ); $renewing_borrower_obj->autorenew_checkouts(1)->store; @@ -887,26 +890,26 @@ subtest "CanBookBeRenewed tests" => sub { } ); - ( $renewokay, $error, $info ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); + ( $renewokay, $error, $info ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); is( $renewokay, 0, 'Bug 7413: Cannot renew, renewal is premature'); is( $error, 'too_soon', 'Bug 7413: Cannot renew, renewal is premature (returned code is too_soon)'); - is( $info->{soonest_renew_date}, dt_from_string($issue->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'too_soon'"); + is( $info->{soonest_renew_date}, dt_from_string($issue_1->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'too_soon'"); # Bug 14101 # Test premature automatic renewal ( $renewokay, $error, $info ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic and premature' ); is( $error, 'auto_too_soon', 'Bug 14101: Cannot renew, renewal is automatic and premature (returned code is auto_too_soon)' ); - is( $info->{soonest_renew_date}, dt_from_string($issue->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'auto_too_soon'"); + is( $info->{soonest_renew_date}, dt_from_string($issue_4->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'auto_too_soon'"); $renewing_borrower_obj->autorenew_checkouts(0)->store; - ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); + ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); is( $renewokay, 0, 'No renewal before is 7, patron opted out of auto_renewal still cannot renew early' ); is( $error, 'too_soon', 'Error is too_soon, no auto' ); - is( $info->{soonest_renew_date}, dt_from_string($issue->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'too_soon'"); + is( $info->{soonest_renew_date}, dt_from_string($issue_4->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'too_soon'"); $renewing_borrower_obj->autorenew_checkouts(1)->store; # Change policy so that loans can only be renewed exactly on due date (0 days prior to due date) @@ -914,18 +917,18 @@ subtest "CanBookBeRenewed tests" => sub { $dbh->do(q{UPDATE circulation_rules SET rule_value = '0' WHERE rule_name = 'norenewalbefore'}); Koha::Cache::Memory::Lite->flush(); ( $renewokay, $error, $info ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic and premature' ); is( $error, 'auto_too_soon', 'Bug 14101: Cannot renew, renewal is automatic and premature, "No renewal before" = 0 (returned code is auto_too_soon)' ); - is( $info->{soonest_renew_date}, dt_from_string($issue->date_due), "Soonest renew date returned when error is 'auto_too_soon'"); + is( $info->{soonest_renew_date}, dt_from_string($issue_4->date_due), "Soonest renew date returned when error is 'auto_too_soon'"); $renewing_borrower_obj->autorenew_checkouts(0)->store; - ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); + ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); is( $renewokay, 0, 'No renewal before is 0, patron opted out of auto_renewal still cannot renew early' ); is( $error, 'too_soon', 'Error is too_soon, no auto' ); - is( $info->{soonest_renew_date}, dt_from_string($issue->date_due), "Soonest renew date returned when error is 'auto_too_soon'"); + is( $info->{soonest_renew_date}, dt_from_string($issue_4->date_due), "Soonest renew date returned when error is 'auto_too_soon'"); $renewing_borrower_obj->autorenew_checkouts(1)->store; # Change policy so that loans can be renewed 99 days prior to the due date @@ -933,14 +936,14 @@ subtest "CanBookBeRenewed tests" => sub { $dbh->do(q{UPDATE circulation_rules SET rule_value = '99' WHERE rule_name = 'norenewalbefore'}); Koha::Cache::Memory::Lite->flush(); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic' ); is( $error, 'auto_renew', 'Bug 14101: Cannot renew, renewal is automatic (returned code is auto_renew)' ); $renewing_borrower_obj->autorenew_checkouts(0)->store; - ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); is( $renewokay, 1, 'No renewal before is 99, patron opted out of auto_renewal so can renew' ); $renewing_borrower_obj->autorenew_checkouts(1)->store; @@ -955,7 +958,7 @@ subtest "CanBookBeRenewed tests" => sub { my $ten_days_before = dt_from_string->add( days => -10 ); my $ten_days_ahead = dt_from_string->add( days => 10 ); - AddIssue( $renewing_borrower, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + my $issue = AddIssue( $renewing_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); Koha::CirculationRules->set_rules( { @@ -969,7 +972,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' ); @@ -985,7 +988,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_too_late', 'Cannot auto renew, too late - no_auto_renewal_after is inclusive(returned code is auto_too_late)' ); @@ -1001,7 +1004,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_too_soon', 'Cannot auto renew, too soon - no_auto_renewal_after is defined(returned code is auto_too_soon)' ); @@ -1017,7 +1020,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Cannot renew, renew is automatic' ); @@ -1034,7 +1037,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' ); @@ -1051,7 +1054,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' ); @@ -1068,7 +1071,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Cannot renew, renew is automatic' ); }; @@ -1084,7 +1087,7 @@ subtest "CanBookBeRenewed tests" => sub { my $ten_days_before = dt_from_string->add( days => -10 ); my $ten_days_ahead = dt_from_string->add( days => 10 ); - AddIssue( $renewing_borrower, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + my $issue = AddIssue( $renewing_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); Koha::CirculationRules->set_rules( { @@ -1112,7 +1115,7 @@ subtest "CanBookBeRenewed tests" => sub { } )->status('RETURNED')->store; ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, patron has 5' ); @@ -1126,7 +1129,7 @@ subtest "CanBookBeRenewed tests" => sub { } )->status('RETURNED')->store; ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, patron has 10' ); @@ -1140,7 +1143,7 @@ subtest "CanBookBeRenewed tests" => sub { } )->status('RETURNED')->store; ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_too_much_oweing', 'Cannot auto renew, OPACFineNoRenewals=10, patron has 15' ); @@ -1153,13 +1156,13 @@ subtest "CanBookBeRenewed tests" => sub { } )->store; ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, OPACFineNoRenewalsIncludeCredit=1, patron has 15 debt, 5 credit' ); C4::Context->set_preference('OPACFineNoRenewalsIncludeCredit','0'); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_too_much_oweing', 'Cannot auto renew, OPACFineNoRenewals=10, OPACFineNoRenewalsIncludeCredit=1, patron has 15 debt, 5 credit' ); @@ -1194,37 +1197,33 @@ subtest "CanBookBeRenewed tests" => sub { # Patron is expired and BlockExpiredPatronOpacActions=0 # => auto renew is allowed t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 0); - my $patron = $expired_borrower; - my $checkout = AddIssue( $patron, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + my $issue = AddIssue( $expired_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); ( $renewokay, $error ) = - CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $expired_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Can auto renew, patron is expired but BlockExpiredPatronOpacActions=0' ); - Koha::Checkouts->find( $checkout->issue_id )->delete; + Koha::Checkouts->find( $issue->issue_id )->delete; # Patron is expired and BlockExpiredPatronOpacActions=1 # => auto renew is not allowed t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 1); - $patron = $expired_borrower; - $checkout = AddIssue( $patron, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + $issue = AddIssue( $expired_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); ( $renewokay, $error ) = - CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $expired_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_account_expired', 'Can not auto renew, lockExpiredPatronOpacActions=1 and patron is expired' ); - Koha::Checkouts->find( $checkout->issue_id )->delete; - + $issue->delete; # Patron is not expired and BlockExpiredPatronOpacActions=1 # => auto renew is allowed t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 1); - $patron = $renewing_borrower; - $checkout = AddIssue( $patron, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + $issue = AddIssue( $renewing_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); ( $renewokay, $error ) = - CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->itemnumber ); + CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Can auto renew, BlockExpiredPatronOpacActions=1 but patron is not expired' ); - Koha::Checkouts->find( $checkout->issue_id )->delete; + $issue->delete; }; subtest "GetLatestAutoRenewDate" => sub { @@ -1238,7 +1237,7 @@ subtest "CanBookBeRenewed tests" => sub { my $ten_days_before = dt_from_string->add( days => -10 ); my $ten_days_ahead = dt_from_string->add( days => 10 ); - AddIssue( $renewing_borrower, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + my $issue = AddIssue( $renewing_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); Koha::CirculationRules->set_rules( { categorycode => undef, @@ -1251,7 +1250,7 @@ subtest "CanBookBeRenewed tests" => sub { } } ); - my $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + my $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrower_obj, $issue ); is( $latest_auto_renew_date, undef, 'GetLatestAutoRenewDate should return undef if no_auto_renewal_after or no_auto_renewal_after_hard_limit are not defined' ); my $five_days_before = dt_from_string->add( days => -5 ); Koha::CirculationRules->set_rules( @@ -1266,7 +1265,7 @@ subtest "CanBookBeRenewed tests" => sub { } } ); - $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrower_obj,, $issue ); is( $latest_auto_renew_date->truncate( to => 'minute' ), $five_days_before->truncate( to => 'minute' ), 'GetLatestAutoRenewDate should return -5 days if no_auto_renewal_after = 5 and date_due is 10 days before' @@ -1288,7 +1287,7 @@ subtest "CanBookBeRenewed tests" => sub { } } ); - $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrower_obj, $issue ); is( $latest_auto_renew_date->truncate( to => 'minute' ), $five_days_ahead->truncate( to => 'minute' ), 'GetLatestAutoRenewDate should return +5 days if no_auto_renewal_after = 15 and date_due is 10 days before' @@ -1306,7 +1305,7 @@ subtest "CanBookBeRenewed tests" => sub { } } ); - $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrower_obj, $issue ); is( $latest_auto_renew_date->truncate( to => 'day' ), $two_days_ahead->truncate( to => 'day' ), 'GetLatestAutoRenewDate should return +2 days if no_auto_renewal_after_hard_limit is defined and not no_auto_renewal_after' @@ -1323,7 +1322,7 @@ subtest "CanBookBeRenewed tests" => sub { } } ); - $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); + $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrower_obj, $issue ); is( $latest_auto_renew_date->truncate( to => 'day' ), $two_days_ahead->truncate( to => 'day' ), 'GetLatestAutoRenewDate should return +2 days if no_auto_renewal_after_hard_limit is < no_auto_renewal_after' @@ -1345,7 +1344,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); is( $renewokay, 0, 'Cannot renew, 0 renewals allowed'); is( $error, 'too_many', 'Cannot renew, 0 renewals allowed (returned code is too_many)'); @@ -1362,8 +1361,9 @@ subtest "CanBookBeRenewed tests" => sub { } ); t::lib::Mocks::mock_preference('UnseenRenewals', 1); - $dbh->do('UPDATE issues SET unseen_renewals = 2 where borrowernumber = ? AND itemnumber = ?', undef, ($renewing_borrowernumber, $item_1->itemnumber)); - ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); + $issue_1->unseen_renewals(2)->store; + + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); is( $renewokay, 0, 'Cannot renew, 0 unseen renewals allowed'); is( $error, 'too_unseen', 'Cannot renew, returned code is too_unseen'); Koha::CirculationRules->set_rules( @@ -1385,21 +1385,21 @@ subtest "CanBookBeRenewed tests" => sub { C4::Overdues::UpdateFine( { - issue_id => $issue->id(), + issue_id => $issue_1->id(), itemnumber => $item_1->itemnumber, - borrowernumber => $renewing_borrower->{borrowernumber}, + borrowernumber => $renewing_borrower_obj->borrowernumber, amount => 15.00, type => q{}, due => Koha::DateUtils::output_pref($datedue) } ); - my $line = Koha::Account::Lines->search({ borrowernumber => $renewing_borrower->{borrowernumber} })->next(); + my $line = Koha::Account::Lines->search({ borrowernumber => $renewing_borrower_obj->borrowernumber })->next(); is( $line->debit_type_code, 'OVERDUE', 'Account line type is OVERDUE' ); is( $line->status, 'UNRETURNED', 'Account line status is UNRETURNED' ); is( $line->amountoutstanding+0, 15, 'Account line amount outstanding is 15.00' ); is( $line->amount+0, 15, 'Account line amount is 15.00' ); - is( $line->issue_id, $issue->id, 'Account line issue id matches' ); + is( $line->issue_id, $issue_1->id, 'Account line issue id matches' ); my $offset = Koha::Account::Offsets->search({ debit_id => $line->id })->next(); is( $offset->type, 'CREATE', 'Account offset type is CREATE' ); @@ -1421,7 +1421,7 @@ subtest "CanBookBeRenewed tests" => sub { my $total_due = $dbh->selectrow_array( 'SELECT SUM( amountoutstanding ) FROM accountlines WHERE borrowernumber = ?', - undef, $renewing_borrower->{borrowernumber} + undef, $renewing_borrower_obj->borrowernumber ); is( $total_due+0, 15, 'Borrower only charged replacement fee with both WhenLostForgiveFine and WhenLostChargeReplacementFee enabled' ); @@ -1430,9 +1430,9 @@ subtest "CanBookBeRenewed tests" => sub { C4::Overdues::UpdateFine( { - issue_id => $issue2->id(), + issue_id => $issue_2->id(), itemnumber => $item_2->itemnumber, - borrowernumber => $renewing_borrower->{borrowernumber}, + borrowernumber => $renewing_borrower_obj->borrowernumber, amount => 15.00, type => q{}, due => Koha::DateUtils::output_pref($datedue) @@ -1447,7 +1447,7 @@ subtest "CanBookBeRenewed tests" => sub { $total_due = $dbh->selectrow_array( 'SELECT SUM( amountoutstanding ) FROM accountlines WHERE borrowernumber = ?', - undef, $renewing_borrower->{borrowernumber} + undef, $renewing_borrower_obj->borrowernumber ); ok( $total_due == 15, 'Borrower only charged fine with both WhenLostForgiveFine and WhenLostChargeReplacementFee disabled' ); @@ -1485,11 +1485,11 @@ subtest "CanBookBeRenewed tests" => sub { }, }); my $recall_borrower = $builder->build_object({ class => 'Koha::Patrons' }); - my $recall_biblio = $builder->build_object({ class => 'Koha::Biblios' }); - my $recall_item1 = $builder->build_object({ class => 'Koha::Items' }, { value => { biblionumber => $recall_biblio->biblionumber } }); - my $recall_item2 = $builder->build_object({ class => 'Koha::Items' }, { value => { biblionumber => $recall_biblio->biblionumber } }); + my $recall_biblio = $builder->build_sample_biblio(); + my $recall_item1 = $builder->build_sample_item({ biblionumber => $recall_biblio->biblionumber }); + my $recall_item2 = $builder->build_sample_item({ biblionumber => $recall_biblio->biblionumber }); - AddIssue( $renewing_borrower, $recall_item1->barcode ); + my $recall_issue = AddIssue( $renewing_borrower_obj->unblessed, $recall_item1->barcode ); # item-level and this item: renewal not allowed my $recall = Koha::Recall->new({ @@ -1499,7 +1499,7 @@ subtest "CanBookBeRenewed tests" => sub { pickup_library_id => $recall_borrower->branchcode, item_level => 1, })->store; - ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $recall_item1->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $recall_issue ); is( $error, 'recalled', 'Cannot renew item that has been recalled' ); $recall->set_cancelled; @@ -1511,7 +1511,7 @@ subtest "CanBookBeRenewed tests" => sub { pickup_library_id => $recall_borrower->branchcode, item_level => 0, })->store; - ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $recall_item1->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $recall_issue ); is( $error, 'recalled', 'Cannot renew item if biblio is recalled and has no item allocated' ); $recall->set_cancelled; @@ -1523,7 +1523,7 @@ subtest "CanBookBeRenewed tests" => sub { pickup_library_id => $recall_borrower->branchcode, item_level => 1, })->store; - ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $recall_item1->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $recall_issue ); is( $renewokay, 1, 'Can renew item if item-level recall on biblio is not on this item' ); $recall->set_cancelled; @@ -1536,7 +1536,7 @@ subtest "CanBookBeRenewed tests" => sub { item_level => 0, })->store; $recall->set_waiting({ item => $recall_item1 }); - ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $recall_item1->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $recall_issue ); is( $renewokay, 1, 'Can renew item if biblio-level recall has already been allocated an item' ); $recall->set_cancelled; }; @@ -1578,7 +1578,7 @@ subtest "GetUpcomingDueIssues" => sub { my $issue = AddIssue( $a_borrower, $item_1->barcode, $yesterday ); my $datedue = dt_from_string( $issue->date_due() ); - my $issue2 = AddIssue( $a_borrower, $item_2->barcode, $two_days_ahead ); + my $issue_2 = AddIssue( $a_borrower, $item_2->barcode, $two_days_ahead ); my $datedue2 = dt_from_string( $issue->date_due() ); my $upcoming_dues; @@ -1704,12 +1704,12 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { ); - my $borrowernumber1 = Koha::Patron->new({ + my $borrower1 = Koha::Patron->new({ firstname => 'Kyle', surname => 'Hall', categorycode => $patron_category->{categorycode}, branchcode => $library2->{branchcode}, - })->store->borrowernumber; + })->store; my $borrowernumber2 = Koha::Patron->new({ firstname => 'Chelsea', surname => 'Hall', @@ -1733,12 +1733,9 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { branchcode => $library2->{branchcode}, })->store->borrowernumber; - my $borrower1 = Koha::Patrons->find( $borrowernumber1 )->unblessed; - my $borrower2 = Koha::Patrons->find( $borrowernumber2 )->unblessed; + my $issue = AddIssue( $borrower1->unblessed, $item_1->barcode ); - my $issue = AddIssue( $borrower1, $item_1->barcode ); - - my ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); + my ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); is( $renewokay, 1, 'Bug 14337 - Verify the borrower can renew with no hold on the record' ); AddReserve( @@ -1761,11 +1758,11 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { } ); t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', 0 ); - ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); is( $renewokay, 0, 'Bug 14337 - Verify the borrower cannot renew with a hold on the record if AllowRenewalIfOtherItemsAvailable and onshelfholds are disabled' ); t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', 1 ); - ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); is( $renewokay, 0, 'Bug 14337 - Verify the borrower cannot renew with a hold on the record if AllowRenewalIfOtherItemsAvailable is enabled and onshelfholds is disabled' ); Koha::CirculationRules->set_rules( @@ -1779,11 +1776,11 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { } ); t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', 0 ); - ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); is( $renewokay, 0, 'Bug 14337 - Verify the borrower cannot renew with a hold on the record if AllowRenewalIfOtherItemsAvailable is disabled and onshelfhold is enabled' ); t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', 1 ); - ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); is( $renewokay, 1, 'Bug 14337 - Verify the borrower can renew with a hold on the record if AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled' ); AddReserve( @@ -1795,7 +1792,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { } ); - ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); is( $renewokay, 0, 'Verify the borrower cannot renew with 2 holds on the record if AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled and one other item on record' ); my $item_3= $builder->build_sample_item( @@ -1806,7 +1803,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { } ); - ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); is( $renewokay, 1, 'Verify the borrower cannot renew with 2 holds on the record if AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled and two other items on record' ); Koha::CirculationRules->set_rules( @@ -1820,7 +1817,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { } ); - ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); is( $renewokay, 0, 'Verify the borrower cannot renew with 2 holds on the record, but only one of those holds can be filled when AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled and two other items on record' ); Koha::CirculationRules->set_rules( @@ -1837,7 +1834,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { # Setting item not checked out to be not for loan but holdable $item_2->notforloan(-1)->store; - ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); is( $renewokay, 0, 'Bug 14337 - Verify the borrower can not renew with a hold on the record if AllowRenewalIfOtherItemsAvailable is enabled but the only available item is notforloan' ); my $mock_circ = Test::MockModule->new("C4::Circulation"); @@ -1851,7 +1848,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { # Two items total, one item available, one issued, two holds on record warnings_are{ - ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); } [], "CanItemBeReserved not called when there are more possible holds than available items"; is( $renewokay, 0, 'Borrower cannot renew when there are more holds than available items' ); @@ -1875,7 +1872,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { ); warnings_are{ - ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); + ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); } ["Checked","Checked"], "CanItemBeReserved only called once per available item if it returns a negative result for all items for a borrower"; is( $renewokay, 0, 'Borrower cannot renew when there are more holds than available items' ); @@ -1896,17 +1893,15 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { } ); - my $borrowernumber = Koha::Patron->new({ + my $borrower = Koha::Patron->new({ firstname => 'fn', surname => 'dn', categorycode => $patron_category->{categorycode}, branchcode => $branch, - })->store->borrowernumber; - - my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; + })->store; - my $issue = AddIssue( $borrower, $item->barcode, undef, undef, undef, undef, { onsite_checkout => 1 } ); - my ( $renewed, $error ) = CanBookBeRenewed( $borrowernumber, $item->itemnumber ); + my $issue = AddIssue( $borrower->unblessed, $item->barcode, undef, undef, undef, undef, { onsite_checkout => 1 } ); + my ( $renewed, $error ) = CanBookBeRenewed( $borrower, $issue ); is( $renewed, 0, 'CanBookBeRenewed should not allow to renew on-site checkout' ); is( $error, 'onsite_checkout', 'A correct error code should be returned by CanBookBeRenewed for on-site checkout' ); } @@ -1923,24 +1918,23 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { itype => $itemtype, } ); + my $patron = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library->{branchcode}, categorycode => $patron_category->{categorycode} } } ); - my $patron = $builder->build({ source => 'Borrower', value => { branchcode => $library->{branchcode}, categorycode => $patron_category->{categorycode} } } ); - - my $issue = AddIssue( $patron, $item->barcode ); + my $issue = AddIssue( $patron->unblessed, $item->barcode ); UpdateFine( { - issue_id => $issue->id(), + issue_id => $issue->id, itemnumber => $item->itemnumber, - borrowernumber => $patron->{borrowernumber}, + borrowernumber => $patron->borrowernumber, amount => 1, type => q{} } ); UpdateFine( { - issue_id => $issue->id(), + issue_id => $issue->id, itemnumber => $item->itemnumber, - borrowernumber => $patron->{borrowernumber}, + borrowernumber => $patron->borrowernumber, amount => 2, type => q{} } @@ -2365,9 +2359,8 @@ subtest 'MultipleReserves' => sub { categorycode => $patron_category->{categorycode}, branchcode => $branch, ); - my $renewing_borrowernumber = Koha::Patron->new(\%renewing_borrower_data)->store->borrowernumber; - my $renewing_borrower = Koha::Patrons->find( $renewing_borrowernumber )->unblessed; - my $issue = AddIssue( $renewing_borrower, $item_1->barcode); + my $patron = Koha::Patron->new(\%renewing_borrower_data)->store; + my $issue = AddIssue( $patron->unblessed, $item_1->barcode); my $datedue = dt_from_string( $issue->date_due() ); is (defined $issue->date_due(), 1, "item 1 checked out"); my $borrowing_borrowernumber = Koha::Checkouts->find({ itemnumber => $item_1->itemnumber })->borrowernumber; @@ -2415,7 +2408,7 @@ subtest 'MultipleReserves' => sub { ); { - my ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1); + my ( $renewokay, $error ) = CanBookBeRenewed($patron, $issue, 1); is($renewokay, 0, 'Bug 17941 - should cover the case where 2 books are both reserved, so failing'); } @@ -2429,7 +2422,7 @@ subtest 'MultipleReserves' => sub { ); { - my ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1); + my ( $renewokay, $error ) = CanBookBeRenewed($patron, $issue, 1); is($renewokay, 1, 'Bug 17941 - should cover the case where 2 books are reserved, but a third one is available'); } }; @@ -4031,7 +4024,7 @@ subtest 'Set waiting flag' => sub { my $hold = Koha::Holds->find( $reserve_id ); is( $hold->found, 'T', 'Hold is in transit' ); - my ( $status ) = CheckReserves($item->itemnumber); + my ( $status ) = CheckReserves($item); is( $status, 'Transferred', 'Hold is not waiting yet'); set_userenv( $library_2 ); @@ -4040,7 +4033,7 @@ subtest 'Set waiting flag' => sub { ModReserveAffect( $item->itemnumber, undef, $do_transfer, $reserve_id ); $hold = Koha::Holds->find( $reserve_id ); is( $hold->found, 'W', 'Hold is waiting' ); - ( $status ) = CheckReserves($item->itemnumber); + ( $status ) = CheckReserves($item); is( $status, 'Waiting', 'Now the hold is waiting'); #Bug 21944 - Waiting transfer checked in at branch other than pickup location @@ -4225,14 +4218,14 @@ subtest 'ItemsDeniedRenewal rules are checked' => sub { my $mock_item_class = Test::MockModule->new("Koha::Item"); $mock_item_class->mock( 'is_denied_renewal', sub { return 1; } ); - my ( $mayrenew, $error ) = CanBookBeRenewed( $idr_borrower->borrowernumber, $issue->itemnumber ); + my ( $mayrenew, $error ) = CanBookBeRenewed( $idr_borrower, $issue ); is( $mayrenew, 0, 'Renewal blocked when $item->is_denied_renewal returns true' ); is( $error, 'item_denied_renewal', 'Renewal blocked when $item->is_denied_renewal returns true' ); $mock_item_class->unmock( 'is_denied_renewal' ); $mock_item_class->mock( 'is_denied_renewal', sub { return 0; } ); - ( $mayrenew, $error ) = CanBookBeRenewed( $idr_borrower->borrowernumber, $issue->itemnumber ); + ( $mayrenew, $error ) = CanBookBeRenewed( $idr_borrower, $issue ); is( $mayrenew, 1, 'Renewal allowed when $item->is_denied_renewal returns false' ); is( $error, undef, 'Renewal allowed when $item->is_denied_renewal returns false' ); @@ -5720,7 +5713,7 @@ subtest "GetSoonestRenewDate tests" => sub { # Test 'exact time' setting for syspref NoRenewalBeforePrecision t::lib::Mocks::mock_preference( 'NoRenewalBeforePrecision', 'exact_time' ); is( - GetSoonestRenewDate( $issue ), + GetSoonestRenewDate( $patron, $issue ), $datedue->clone->add( days => -7 ), 'Bug 14395: Renewals permitted 7 days before due date, as expected' ); @@ -5729,7 +5722,7 @@ subtest "GetSoonestRenewDate tests" => sub { # Test 'date' setting for syspref NoRenewalBeforePrecision t::lib::Mocks::mock_preference( 'NoRenewalBeforePrecision', 'date' ); is( - GetSoonestRenewDate( $issue ), + GetSoonestRenewDate( $patron, $issue ), $datedue->clone->add( days => -7 )->truncate( to => 'day' ), 'Bug 14395: Renewals permitted 7 days before due date, as expected' ); @@ -5746,7 +5739,7 @@ subtest "GetSoonestRenewDate tests" => sub { ); is( - GetSoonestRenewDate( $issue ), + GetSoonestRenewDate( $patron, $issue ), dt_from_string, 'Checkouts without auto-renewal can be renewed immediately if no norenewalbefore' ); @@ -5754,13 +5747,13 @@ subtest "GetSoonestRenewDate tests" => sub { t::lib::Mocks::mock_preference( 'NoRenewalBeforePrecision', 'date' ); $issue->auto_renew(1)->store; is( - GetSoonestRenewDate( $issue ), + GetSoonestRenewDate( $patron, $issue ), $datedue->clone->truncate( to => 'day' ), 'Checkouts with auto-renewal can be renewed earliest on due date if no renewalbefore' ); t::lib::Mocks::mock_preference( 'NoRenewalBeforePrecision', 'exact' ); is( - GetSoonestRenewDate( $issue ), + GetSoonestRenewDate( $patron, $issue ), $datedue, 'Checkouts with auto-renewal can be renewed earliest on due date if no renewalbefore' ); @@ -5773,7 +5766,7 @@ subtest "CanBookBeIssued + needsconfirmation message" => sub { my $library = $builder->build_object( { class => 'Koha::Libraries' } ); my $biblio = $builder->build_object({ class => 'Koha::Biblios' }); my $biblioitem = $builder->build_object({ class => 'Koha::Biblioitems', value => { biblionumber => $biblio->biblionumber }}); - my $item = $builder->build_object({ class => 'Koha::Items' , value => { biblionumber => $biblio->biblionumber }}); + my $item = $builder->build_object({ class => 'Koha::Items' , value => { itype => $itemtype, biblionumber => $biblio->biblionumber }}); my $hold = $builder->build_object({ class => 'Koha::Holds', value => { biblionumber => $item->biblionumber, diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t index f2dc194981..7c20b11bac 100755 --- a/t/db_dependent/Circulation/Returns.t +++ b/t/db_dependent/Circulation/Returns.t @@ -132,6 +132,7 @@ subtest "AddReturn logging on statistics table (item-level_itypes=1)" => sub { "item-level itype recorded on statistics for return"); warning_like { AddIssue( $borrower, $item_without_itemtype->barcode ) } [qr/^item-level_itypes set but no itemtype set for item/, + qr/^item-level_itypes set but no itemtype set for item/, qr/^item-level_itypes set but no itemtype set for item/], 'Item without itemtype set raises warning on AddIssue'; AddReturn( $item_without_itemtype->barcode, $branch ); diff --git a/t/db_dependent/Circulation/transferbook.t b/t/db_dependent/Circulation/transferbook.t index f6ee6b4323..c94e32dc4d 100755 --- a/t/db_dependent/Circulation/transferbook.t +++ b/t/db_dependent/Circulation/transferbook.t @@ -116,9 +116,11 @@ subtest 'transfer already at destination' => sub { } ); + my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' })->store->itemtype; my $item = $builder->build_sample_item( { library => $library->branchcode, + itype => $itemtype } ); @@ -169,7 +171,8 @@ subtest 'transfer already at destination' => sub { is( $dotransfer, 0, 'Do not transfer recalled item, it has already arrived' ); is( $messages->{RecallPlacedAtHoldingBranch}, 1, "We found the recall"); - my $item2 = $builder->build_object({ class => 'Koha::Items' }); # this item will have a different holding branch to the pickup branch + $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' })->store->itemtype; + my $item2 = $builder->build_object({ class => 'Koha::Items', value => { itype => $itemtype } }); # this item will have a different holding branch to the pickup branch $recall = Koha::Recall->new( { biblio_id => $item2->biblionumber, item_id => $item2->itemnumber, @@ -196,9 +199,11 @@ subtest 'transfer an issued item' => sub { } ); + my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' })->store->itemtype; my $item = $builder->build_sample_item( { library => $library->branchcode, + itype => $itemtype } ); diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index c5c79b1d76..2f2b0d5782 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -64,7 +64,8 @@ $insert_sth->execute('ONLY1'); my $biblio = $builder->build_sample_biblio({ itemtype => 'DUMMY' }); # Create item instance for testing. -my $itemnumber = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber })->itemnumber; +my $item = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber }); +my $itemnumber = $item->itemnumber; # Create some borrowers my @borrowernumbers; @@ -101,7 +102,6 @@ is( $holds->next->priority, 3, "Reserve 3 has a priority of 3" ); is( $holds->next->priority, 4, "Reserve 4 has a priority of 4" ); is( $holds->next->priority, 5, "Reserve 5 has a priority of 5" ); -my $item = Koha::Items->find( $itemnumber ); $holds = $item->current_holds; my $first_hold = $holds->next; my $reservedate = $first_hold->reservedate; @@ -342,7 +342,7 @@ ok( my $damaged_item = Koha::Items->find($itemnumber)->damaged(1)->store; # FIXME The $itemnumber is a bit confusing here t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 1 ); is( CanItemBeReserved( $patrons[0], $damaged_item)->{status}, 'OK', "Patron can reserve damaged item with AllowHoldsOnDamagedItems enabled" ); -ok( defined( ( CheckReserves($itemnumber) )[1] ), "Hold can be trapped for damaged item with AllowHoldsOnDamagedItems enabled" ); +ok( defined( ( CheckReserves($damaged_item) )[1] ), "Hold can be trapped for damaged item with AllowHoldsOnDamagedItems enabled" ); $hold = Koha::Hold->new( { @@ -359,7 +359,7 @@ $hold->delete(); t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 0 ); ok( CanItemBeReserved( $patrons[0], $damaged_item)->{status} eq 'damaged', "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" ); -ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for damaged item with AllowHoldsOnDamagedItems disabled" ); +ok( !defined( ( CheckReserves($damaged_item) )[1] ), "Hold cannot be trapped for damaged item with AllowHoldsOnDamagedItems disabled" ); # Items that are not for loan, but holdable should not be trapped until they are available for loan t::lib::Mocks::mock_preference( 'TrapHoldsOnOrder', 0 ); @@ -377,19 +377,19 @@ $hold = Koha::Hold->new( branchcode => $branch_1, } )->store(); -ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item that is not for loan but holdable ( notforloan < 0 )" ); +ok( !defined( ( CheckReserves($nfl_item) )[1] ), "Hold cannot be trapped for item that is not for loan but holdable ( notforloan < 0 )" ); t::lib::Mocks::mock_preference( 'TrapHoldsOnOrder', 1 ); -ok( defined( ( CheckReserves($itemnumber) )[1] ), "Hold is trapped for item that is not for loan but holdable ( notforloan < 0 )" ); +ok( defined( ( CheckReserves($nfl_item) )[1] ), "Hold is trapped for item that is not for loan but holdable ( notforloan < 0 )" ); t::lib::Mocks::mock_preference( 'SkipHoldTrapOnNotForLoanValue', '-1' ); -ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item with notforloan value matching SkipHoldTrapOnNotForLoanValue" ); +ok( !defined( ( CheckReserves($nfl_item) )[1] ), "Hold cannot be trapped for item with notforloan value matching SkipHoldTrapOnNotForLoanValue" ); t::lib::Mocks::mock_preference( 'SkipHoldTrapOnNotForLoanValue', '-1|1' ); -ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item with notforloan value matching SkipHoldTrapOnNotForLoanValue" ); +ok( !defined( ( CheckReserves($nfl_item) )[1] ), "Hold cannot be trapped for item with notforloan value matching SkipHoldTrapOnNotForLoanValue" ); t::lib::Mocks::mock_preference( 'SkipHoldTrapOnNotForLoanValue', '' ); my $item_group_1 = Koha::Biblio::ItemGroup->new( { biblio_id => $biblio->id } )->store(); my $item_group_2 = Koha::Biblio::ItemGroup->new( { biblio_id => $biblio->id } )->store(); $item_group_1->add_item({ item_id => $itemnumber }); $hold->item_group_id( $item_group_2->id )->update; -ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item with non-matching item group" ); +ok( !defined( ( CheckReserves($nfl_item) )[1] ), "Hold cannot be trapped for item with non-matching item group" ); is( CanItemBeReserved( $patrons[0], $nfl_item)->{status}, 'itemAlreadyOnHold', "cannot request item that you have already reservedd" @@ -1501,7 +1501,7 @@ subtest 'non priority holds' => sub { } ); - Koha::Checkout->new( + my $issue = Koha::Checkout->new( { borrowernumber => $patron1->borrowernumber, itemnumber => $item->itemnumber, @@ -1520,7 +1520,7 @@ subtest 'non priority holds' => sub { ); my ( $ok, $err ) = - CanBookBeRenewed( $patron1->borrowernumber, $item->itemnumber ); + CanBookBeRenewed( $patron1, $issue ); ok( !$ok, 'Cannot renew' ); is( $err, 'on_reserve', 'Item is on hold' ); @@ -1529,7 +1529,7 @@ subtest 'non priority holds' => sub { $hold->non_priority(1)->store; ( $ok, $err ) = - CanBookBeRenewed( $patron1->borrowernumber, $item->itemnumber ); + CanBookBeRenewed( $patron1, $issue ); ok( $ok, 'Can renew' ); is( $err, undef, 'Item is on non priority hold' ); @@ -1553,7 +1553,7 @@ subtest 'non priority holds' => sub { ); ( $ok, $err ) = - CanBookBeRenewed( $patron1->borrowernumber, $item->itemnumber ); + CanBookBeRenewed( $patron1, $issue ); ok( !$ok, 'Cannot renew' ); is( $err, 'on_reserve', 'Item is on hold' ); diff --git a/t/db_dependent/Holds/HoldFulfillmentPolicy.t b/t/db_dependent/Holds/HoldFulfillmentPolicy.t index 7e21336c10..5fadfb0cfe 100755 --- a/t/db_dependent/Holds/HoldFulfillmentPolicy.t +++ b/t/db_dependent/Holds/HoldFulfillmentPolicy.t @@ -67,9 +67,7 @@ $dbh->do(" VALUES ($biblionumber, $biblioitemnumber, '$library_A', '$library_B', 0, 0, 0, 0, NULL, '$itemtype') "); -my $itemnumber = - $dbh->selectrow_array("SELECT itemnumber FROM items WHERE biblionumber = $biblionumber") - or BAIL_OUT("Cannot find newly created item"); +my $item = Koha::Items->find({ biblionumber => $biblionumber }); # With hold_fulfillment_policy = homebranch, hold should only be picked up if pickup branch = homebranch $dbh->do("DELETE FROM circulation_rules"); @@ -93,7 +91,7 @@ my $reserve_id = AddReserve( priority => 1, } ); -my ( $status ) = CheckReserves($itemnumber); +my ( $status ) = CheckReserves($item); is( $status, 'Reserved', "Hold where pickup branch matches home branch targeted" ); Koha::Holds->find( $reserve_id )->cancel; @@ -106,7 +104,7 @@ $reserve_id = AddReserve( priority => 1, } ); -( $status ) = CheckReserves($itemnumber); +( $status ) = CheckReserves($item); is($status, q{}, "Hold where pickup ne home, pickup eq home not targeted" ); Koha::Holds->find( $reserve_id )->cancel; @@ -119,7 +117,7 @@ $reserve_id = AddReserve( priority => 1, } ); -( $status ) = CheckReserves($itemnumber); +( $status ) = CheckReserves($item); is( $status, q{}, "Hold where pickup ne home, pickup ne holding not targeted" ); Koha::Holds->find( $reserve_id )->cancel; @@ -145,7 +143,7 @@ $reserve_id = AddReserve( priority => 1, } ); -( $status ) = CheckReserves($itemnumber); +( $status ) = CheckReserves($item); is( $status, q{}, "Hold where pickup eq home, pickup ne holding not targeted" ); Koha::Holds->find( $reserve_id )->cancel; @@ -158,7 +156,7 @@ $reserve_id = AddReserve( priority => 1, } ); -( $status ) = CheckReserves($itemnumber); +( $status ) = CheckReserves($item); is( $status, 'Reserved', "Hold where pickup ne home, pickup eq holding targeted" ); Koha::Holds->find( $reserve_id )->cancel; @@ -171,7 +169,7 @@ $reserve_id = AddReserve( priority => 1, } ); -( $status ) = CheckReserves($itemnumber); +( $status ) = CheckReserves($item); is( $status, q{}, "Hold where pickup ne home, pickup ne holding not targeted" ); Koha::Holds->find( $reserve_id )->cancel; @@ -197,7 +195,7 @@ $reserve_id = AddReserve( priority => 1, } ); -( $status ) = CheckReserves($itemnumber); +( $status ) = CheckReserves($item); is( $status, 'Reserved', "Hold where pickup eq home, pickup ne holding targeted" ); Koha::Holds->find( $reserve_id )->cancel; @@ -210,7 +208,7 @@ $reserve_id = AddReserve( priority => 1, } ); -( $status ) = CheckReserves($itemnumber); +( $status ) = CheckReserves($item); is( $status, 'Reserved', "Hold where pickup ne home, pickup eq holding targeted" ); Koha::Holds->find( $reserve_id )->cancel; @@ -223,7 +221,7 @@ $reserve_id = AddReserve( priority => 1, } ); -( $status ) = CheckReserves($itemnumber); +( $status ) = CheckReserves($item); is( $status, 'Reserved', "Hold where pickup ne home, pickup ne holding targeted" ); Koha::Holds->find( $reserve_id )->cancel; @@ -231,7 +229,6 @@ Koha::Holds->find( $reserve_id )->cancel; t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); Koha::Holds->search()->delete(); -my ($item) = Koha::Biblios->find($biblionumber)->items->as_list; my $limit = Koha::Item::Transfer::Limit->new( { toBranch => $library_C, @@ -247,6 +244,6 @@ $reserve_id = AddReserve( priority => 1 } ); -($status) = CheckReserves($itemnumber); +($status) = CheckReserves($item); is( $status, '', "No hold where branch transfer is not allowed" ); Koha::Holds->find($reserve_id)->cancel; diff --git a/t/db_dependent/Holds/HoldItemtypeLimit.t b/t/db_dependent/Holds/HoldItemtypeLimit.t index 9157320694..2b5bdc141a 100755 --- a/t/db_dependent/Holds/HoldItemtypeLimit.t +++ b/t/db_dependent/Holds/HoldItemtypeLimit.t @@ -82,9 +82,7 @@ $dbh->do(" VALUES ($biblionumber, $biblioitemnumber, '$branchcode', '$branchcode', 0, 0, 0, 0, NULL, '$right_itemtype') "); -my $itemnumber = - $dbh->selectrow_array("SELECT itemnumber FROM items WHERE biblionumber = $biblionumber") - or BAIL_OUT("Cannot find newly created item"); +my $item = Koha::Items->find({ biblionumber => $biblionumber }); $dbh->do("DELETE FROM circulation_rules"); Koha::CirculationRules->set_rules( @@ -108,7 +106,7 @@ my $reserve_id = AddReserve( itemtype => $right_itemtype, } ); -my ( $status ) = CheckReserves($itemnumber); +my ( $status ) = CheckReserves($item); is( $status, 'Reserved', "Hold where itemtype matches item's itemtype targed" ); Koha::Holds->find( $reserve_id )->cancel; @@ -123,7 +121,7 @@ $reserve_id = AddReserve( } ); -( $status ) = CheckReserves($itemnumber); +( $status ) = CheckReserves($item); is($status, q{}, "Hold where itemtype does not match item's itemtype not targeted" ); Koha::Holds->find( $reserve_id )->cancel; @@ -136,7 +134,7 @@ $reserve_id = AddReserve( priority => 1, } ); -( $status ) = CheckReserves($itemnumber); +( $status ) = CheckReserves($item); is( $status, 'Reserved', "Item targeted with no hold itemtype set" ); Koha::Holds->find( $reserve_id )->cancel; diff --git a/t/db_dependent/Holds/LocalHoldsPriority.t b/t/db_dependent/Holds/LocalHoldsPriority.t index a125d3eab1..85833ca8d5 100755 --- a/t/db_dependent/Holds/LocalHoldsPriority.t +++ b/t/db_dependent/Holds/LocalHoldsPriority.t @@ -42,7 +42,7 @@ my $itemtype = $builder->build( my $borrowers_count = 5; my $biblio = $builder->build_sample_biblio(); -my $itemnumber = Koha::Item->new( +my $item = Koha::Item->new( { biblionumber => $biblio->biblionumber, homebranch => $library4->{branchcode}, @@ -50,7 +50,7 @@ my $itemnumber = Koha::Item->new( itype => $itemtype, exclude_from_local_holds_priority => 0, }, -)->store->itemnumber; +)->store; my @branchcodes = ( $library1->{branchcode}, $library2->{branchcode}, $library3->{branchcode}, $library4->{branchcode}, $library3->{branchcode}, $library4->{branchcode} ); my $patron_category = $builder->build({ source => 'Category', value => {exclude_from_local_holds_priority => 0} }); @@ -84,29 +84,29 @@ foreach my $borrowernumber (@borrowernumbers) { my ($status, $reserve, $all_reserves); t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 0 ); -($status, $reserve, $all_reserves) = CheckReserves($itemnumber); +($status, $reserve, $all_reserves) = CheckReserves($item); ok( $reserve->{borrowernumber} eq $borrowernumbers[0], "Received expected results with LocalHoldsPriority disabled" ); t::lib::Mocks::mock_preference( 'LocalHoldsPriority', 1 ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'PickupLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' ); -($status, $reserve, $all_reserves) = CheckReserves($itemnumber); +($status, $reserve, $all_reserves) = CheckReserves($item); ok( $reserve->{borrowernumber} eq $borrowernumbers[2], "Received expected results with PickupLibrary/homebranch" ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'PickupLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'holdingbranch' ); -($status, $reserve, $all_reserves) = CheckReserves($itemnumber); +($status, $reserve, $all_reserves) = CheckReserves($item); ok( $reserve->{borrowernumber} eq $borrowernumbers[1], "Received expected results with PickupLibrary/holdingbranch" ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'HomeLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'holdingbranch' ); -($status, $reserve, $all_reserves) = CheckReserves($itemnumber); +($status, $reserve, $all_reserves) = CheckReserves($item); ok( $reserve->{borrowernumber} eq $borrowernumbers[2], "Received expected results with HomeLibrary/holdingbranch" ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityPatronControl', 'HomeLibrary' ); t::lib::Mocks::mock_preference( 'LocalHoldsPriorityItemControl', 'homebranch' ); -($status, $reserve, $all_reserves) = CheckReserves($itemnumber); +($status, $reserve, $all_reserves) = CheckReserves($item); ok( $reserve->{borrowernumber} eq $borrowernumbers[3], "Received expected results with HomeLibrary/homebranch" ); $schema->storage->txn_rollback; @@ -160,7 +160,7 @@ subtest "exclude from local holds" => sub { ); my ($status, $reserve, $all_reserves); - ($status, $reserve, $all_reserves) = CheckReserves($item1->itemnumber); + ($status, $reserve, $all_reserves) = CheckReserves($item1); is($reserve->{borrowernumber}, $patron_nex_l1->borrowernumber, "Patron not excluded with local holds priorities is next checkout"); Koha::Holds->delete; @@ -182,7 +182,7 @@ subtest "exclude from local holds" => sub { } ); - ($status, $reserve, $all_reserves) = CheckReserves($item1->itemnumber); + ($status, $reserve, $all_reserves) = CheckReserves($item1); is($reserve->{borrowernumber}, $patron_nex_l2->borrowernumber, "Local patron is excluded from priority"); Koha::Holds->delete; @@ -204,7 +204,7 @@ subtest "exclude from local holds" => sub { } ); - ($status, $reserve, $all_reserves) = CheckReserves($item2->itemnumber); + ($status, $reserve, $all_reserves) = CheckReserves($item2); is($reserve->{borrowernumber}, $patron_nex_l2->borrowernumber, "Patron from other library is next checkout because item is excluded"); $schema->storage->txn_rollback; diff --git a/t/db_dependent/Koha/Account/Line.t b/t/db_dependent/Koha/Account/Line.t index b5926b4f70..abd314e419 100755 --- a/t/db_dependent/Koha/Account/Line.t +++ b/t/db_dependent/Koha/Account/Line.t @@ -511,6 +511,7 @@ subtest 'Renewal related tests' => sub { class => 'Koha::Checkouts', value => { itemnumber => $item->itemnumber, + borrowernumber => $patron->borrowernumber, onsite_checkout => 0, renewals_count => 99, auto_renew => 0 diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t index d71a21d735..1624cd43e7 100755 --- a/t/db_dependent/Koha/Object.t +++ b/t/db_dependent/Koha/Object.t @@ -973,6 +973,8 @@ subtest 'unblessed_all_relateds' => sub { my $patron = Koha::Patron->new($patron_data)->store; my ($biblionumber) = AddBiblio( MARC::Record->new, '' ); my $biblio = Koha::Biblios->find( $biblionumber ); + my $itemtype = $builder->build({ source => 'Itemtype' })->{itemtype}; + my $item = $builder->build_object( { class => 'Koha::Items', @@ -982,6 +984,7 @@ subtest 'unblessed_all_relateds' => sub { biblionumber => $biblio->biblionumber, itemlost => 0, withdrawn => 0, + itype => $itemtype } } ); diff --git a/t/db_dependent/Letters/TemplateToolkit.t b/t/db_dependent/Letters/TemplateToolkit.t index edd91ac091..7d01777f29 100755 --- a/t/db_dependent/Letters/TemplateToolkit.t +++ b/t/db_dependent/Letters/TemplateToolkit.t @@ -311,12 +311,13 @@ subtest 'regression tests' => sub { plan tests => 8; my $library = $builder->build( { source => 'Branch' } ); - my $patron = $builder->build( { source => 'Borrower' } ); + my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' })->store->itemtype; + my $item1 = $builder->build_sample_item( { barcode => 'a_t_barcode', library => $library->{branchcode}, - itype => 'BK', + itype => $itemtype, itemcallnumber => 'itemcallnumber1', } ); @@ -326,7 +327,7 @@ subtest 'regression tests' => sub { { barcode => 'another_t_barcode', library => $library->{branchcode}, - itype => 'BK', + itype => $itemtype, itemcallnumber => 'itemcallnumber2', } ); @@ -336,7 +337,7 @@ subtest 'regression tests' => sub { { barcode => 'another_t_barcode_3', library => $library->{branchcode}, - itype => 'BK', + itype => $itemtype, itemcallnumber => 'itemcallnumber3', } ); diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index e64393154e..2d97fa43a1 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 78; +use Test::More tests => 77; use Test::MockModule; use Test::Warn; @@ -94,6 +94,7 @@ my $biblio_with_no_item = $builder->build_sample_biblio; my $testbarcode = '97531'; $item->barcode($testbarcode)->store; # FIXME We should not hardcode a barcode! Also, what's the purpose of this? + # Create a borrower my %data = ( firstname => 'my firstname', @@ -106,7 +107,6 @@ my $borrowernumber = Koha::Patron->new(\%data)->store->borrowernumber; my $patron = Koha::Patrons->find( $borrowernumber ); my $borrower = $patron->unblessed; my $biblionumber = $bibnum; -my $barcode = $testbarcode; my $branchcode = Koha::Libraries->search->next->branchcode; @@ -119,18 +119,15 @@ AddReserve( } ); -my ($status, $reserve, $all_reserves) = CheckReserves($item->itemnumber, $barcode); +my ($status, $reserve, $all_reserves) = CheckReserves( $item ); is($status, "Reserved", "CheckReserves Test 1"); ok(exists($reserve->{reserve_id}), 'CheckReserves() include reserve_id in its response'); -($status, $reserve, $all_reserves) = CheckReserves($item->itemnumber); +($status, $reserve, $all_reserves) = CheckReserves( $item ); is($status, "Reserved", "CheckReserves Test 2"); -($status, $reserve, $all_reserves) = CheckReserves(undef, $barcode); -is($status, "Reserved", "CheckReserves Test 3"); - my $ReservesControlBranch = C4::Context->preference('ReservesControlBranch'); t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'ItemHomeLibrary' ); ok( @@ -345,9 +342,9 @@ AddReserve( priority => 1, } ); -($status)=CheckReserves($item->itemnumber,undef,undef); +($status)=CheckReserves( $item ); is( $status, 'Reserved', 'CheckReserves returns reserve without lookahead'); -($status)=CheckReserves($item->itemnumber,undef,7); +($status)=CheckReserves( $item, 7 ); is( $status, 'Reserved', 'CheckReserves also returns reserve with lookahead'); # Test 9761b: Add a reserve with future date, CheckReserve should not return it @@ -364,26 +361,26 @@ my $reserve_id = AddReserve( reservation_date => $resdate, } ); -($status)=CheckReserves($item->itemnumber,undef,undef); +($status)=CheckReserves( $item ); is( $status, '', 'CheckReserves returns no future reserve without lookahead'); # Test 9761c: Add a reserve with future date, CheckReserve should return it if lookahead is high enough -($status)=CheckReserves($item->itemnumber,undef,3); +($status)=CheckReserves( $item, 3 ); is( $status, '', 'CheckReserves returns no future reserve with insufficient lookahead'); -($status)=CheckReserves($item->itemnumber,undef,4); +($status)=CheckReserves( $item, 4 ); is( $status, 'Reserved', 'CheckReserves returns future reserve with sufficient lookahead'); # Test 9761d: Check ResFound message of AddReturn for future hold # Note that AddReturn is in Circulation.pm, but this test really pertains to reserves; AddReturn uses the ConfirmFutureHolds pref when calling CheckReserves # In this test we do not need an issued item; it is just a 'checkin' t::lib::Mocks::mock_preference('ConfirmFutureHolds', 0); -(my $doreturn, $messages)= AddReturn('97531',$branch_1); +(my $doreturn, $messages)= AddReturn($testbarcode,$branch_1); is($messages->{ResFound}//'', '', 'AddReturn does not care about future reserve when ConfirmFutureHolds is off'); t::lib::Mocks::mock_preference('ConfirmFutureHolds', 3); -($doreturn, $messages)= AddReturn('97531',$branch_1); +($doreturn, $messages)= AddReturn($testbarcode,$branch_1); is(exists $messages->{ResFound}?1:0, 0, 'AddReturn ignores future reserve beyond ConfirmFutureHolds days'); t::lib::Mocks::mock_preference('ConfirmFutureHolds', 7); -($doreturn, $messages)= AddReturn('97531',$branch_1); +($doreturn, $messages)= AddReturn($testbarcode,$branch_1); is(exists $messages->{ResFound}?1:0, 1, 'AddReturn considers future reserve within ConfirmFutureHolds days'); my $now_holder = $builder->build_object({ class => 'Koha::Patrons', value => { @@ -399,9 +396,9 @@ my $now_reserve_id = AddReserve( } ); my $which_highest; -($status,$which_highest)=CheckReserves($item->itemnumber,undef,3); +($status,$which_highest)=CheckReserves( $item, 3 ); is( $which_highest->{reserve_id}, $now_reserve_id, 'CheckReserves returns lower priority current reserve with insufficient lookahead'); -($status, $which_highest)=CheckReserves($item->itemnumber,undef,4); +($status, $which_highest)=CheckReserves( $item, 4 ); is( $which_highest->{reserve_id}, $reserve_id, 'CheckReserves returns higher priority future reserve with sufficient lookahead'); ModReserve({ reserve_id => $now_reserve_id, rank => 'del', cancellation_reason => 'test reserve' }); @@ -586,7 +583,7 @@ AddReserve( priority => 1, } ); -my (undef, $canres, undef) = CheckReserves($item->itemnumber); +my (undef, $canres, undef) = CheckReserves( $item ); is( CanReserveBeCanceledFromOpac(), undef, 'CanReserveBeCanceledFromOpac should return undef if called without any parameter' @@ -624,7 +621,7 @@ AddReserve( priority => 1, } ); -(undef, $canres, undef) = CheckReserves($item->itemnumber); +(undef, $canres, undef) = CheckReserves( $item ); ModReserveAffect($item->itemnumber, $requesters{$branch_1}, 0); $cancancel = CanReserveBeCanceledFromOpac($canres->{reserve_id}, $requesters{$branch_1}); @@ -708,7 +705,7 @@ AddReserve( } ); MoveReserve( $item->itemnumber, $borrowernumber ); -($status)=CheckReserves( $item->itemnumber ); +($status)=CheckReserves( $item ); is( $status, '', 'MoveReserve filled hold'); # hold from A waiting, today, no fut holds: MoveReserve should fill it AddReserve( @@ -721,7 +718,7 @@ AddReserve( } ); MoveReserve( $item->itemnumber, $borrowernumber ); -($status)=CheckReserves( $item->itemnumber ); +($status)=CheckReserves( $item ); is( $status, '', 'MoveReserve filled waiting hold'); # hold from A pos 1, tomorrow, no fut holds: not filled $resdate= dt_from_string(); @@ -736,7 +733,7 @@ AddReserve( } ); MoveReserve( $item->itemnumber, $borrowernumber ); -($status)=CheckReserves( $item->itemnumber, undef, 1 ); +($status)=CheckReserves( $item, 1 ); is( $status, 'Reserved', 'MoveReserve did not fill future hold'); $dbh->do('DELETE FROM reserves', undef, ($bibnum)); # hold from A pos 1, tomorrow, fut holds=2: MoveReserve should fill it @@ -751,7 +748,7 @@ AddReserve( } ); MoveReserve( $item->itemnumber, $borrowernumber ); -($status)=CheckReserves( $item->itemnumber, undef, 2 ); +($status)=CheckReserves( $item, undef, 2 ); is( $status, '', 'MoveReserve filled future hold now'); # hold from A waiting, tomorrow, fut holds=2: MoveReserve should fill it AddReserve( @@ -764,7 +761,7 @@ AddReserve( } ); MoveReserve( $item->itemnumber, $borrowernumber ); -($status)=CheckReserves( $item->itemnumber, undef, 2 ); +($status)=CheckReserves( $item, undef, 2 ); is( $status, '', 'MoveReserve filled future waiting hold now'); # hold from A pos 1, today+3, fut holds=2: MoveReserve should not fill it $resdate= dt_from_string(); @@ -779,7 +776,7 @@ AddReserve( } ); MoveReserve( $item->itemnumber, $borrowernumber ); -($status)=CheckReserves( $item->itemnumber, undef, 3 ); +($status)=CheckReserves( $item, 3 ); is( $status, 'Reserved', 'MoveReserve did not fill future hold of 3 days'); $dbh->do('DELETE FROM reserves', undef, ($bibnum)); @@ -1241,7 +1238,7 @@ subtest 'CheckReserves additional tests' => sub { ModReserveAffect( $item->itemnumber, $reserve1->borrowernumber, 1, $reserve1->reserve_id ); my ( $status, $matched_reserve, $possible_reserves ) = - CheckReserves( $item->itemnumber ); + CheckReserves( $item ); is( $status, 'Transferred', "We found a reserve" ); is( $matched_reserve->{reserve_id}, @@ -1284,7 +1281,7 @@ subtest 'CheckReserves additional tests' => sub { ok( $reserve_id, "We can place a record level hold because one item is owned by patron's home library"); t::lib::Mocks::mock_preference('ReservesControlBranch', 'ItemHomeLibrary'); - ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A->itemnumber ); + ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A ); is( $status, "", "We do not fill the hold with item A because it is not from the patron's homebranch"); Koha::CirculationRules->set_rule({ branchcode => $item_A->homebranch, @@ -1292,13 +1289,13 @@ subtest 'CheckReserves additional tests' => sub { rule_name => 'holdallowed', rule_value => 'from_any_library' }); - ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A->itemnumber ); + ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A ); is( $status, "Reserved", "We fill the hold with item A because item's branch rule says allow any"); # Changing the control branch should change only the rule we get t::lib::Mocks::mock_preference('ReservesControlBranch', 'PatronLibrary'); - ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A->itemnumber ); + ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A ); is( $status, "", "We do not fill the hold with item A because it is not from the patron's homebranch"); Koha::CirculationRules->set_rule({ branchcode => $patron_B->branchcode, @@ -1306,7 +1303,7 @@ subtest 'CheckReserves additional tests' => sub { rule_name => 'holdallowed', rule_value => 'from_any_library' }); - ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A->itemnumber ); + ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A ); is( $status, "Reserved", "We fill the hold with item A because patron's branch rule says allow any"); }; -- 2.39.5