From 51da69cd4a13c5789c1e022b278787c942901c38 Mon Sep 17 00:00:00 2001 From: David Gustafsson Date: Thu, 29 Sep 2022 17:15:32 +0200 Subject: [PATCH] Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm Refactor the most performance critical subroutines in Circulation.pm to take objects instead of unblessed ones to reduce unnecessary unblessings and generally clean up the code. To test: 1) Ensure the following tests all pass: t/db_dependent/Circulation.t t/db_dependent/Circulation/CalcDateDue.t t/db_dependent/Circulation/CheckIfIssuedToPatron.t t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t t/db_dependent/Circulation/GetTopIssues.t t/db_dependent/Circulation/IsItemIssued.t t/db_dependent/Circulation/MarkIssueReturned.t t/db_dependent/Circulation/ReturnClaims.t t/db_dependent/Circulation/Returns.t t/db_dependent/Circulation/SwitchOnSiteCheckouts.t t/db_dependent/Circulation/TooMany.t t/db_dependent/Circulation/dateexpiry.t t/db_dependent/Circulation/issue.t t/db_dependent/Circulation/maxsuspensiondays.t t/db_dependent/Circulation/transferbook.t t/db_dependent/Circulation_holdsqueue.t t/db_dependent/DecreaseLoanHighHolds.t t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t t/db_dependent/Holds/RevertWaitingStatus.t t/db_dependent/ILSDI_Services.t t/db_dependent/Illrequests.t t/db_dependent/Koha/Account/Line.t t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Items.t t/db_dependent/Koha/Object.t t/db_dependent/Koha/Patrons.t t/db_dependent/Koha/Pseudonymization.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/SIP/ILS.t Sponsored-by: Gothenburg University Library --- C4/Circulation.pm | 149 ++++++++--------- C4/SIP/ILS/Transaction/Checkout.pm | 2 +- Koha/CurbsidePickup.pm | 2 +- Koha/Illrequest.pm | 2 - Koha/Item.pm | 2 +- Koha/Recall.pm | 2 +- circ/circulation.pl | 2 +- offline_circ/process_koc.pl | 37 ++--- opac/sco/sco-main.pl | 2 +- reserve/request.pl | 3 +- svc/letters/preview | 7 +- t/db_dependent/Circulation.t | 155 ++++++++++-------- t/db_dependent/Circulation/CalcDateDue.t | 50 ++++-- .../Circulation/CheckIfIssuedToPatron.t | 34 +++- .../Circulation/GetPendingOnSiteCheckouts.t | 14 +- t/db_dependent/Circulation/GetTopIssues.t | 2 +- t/db_dependent/Circulation/IsItemIssued.t | 6 +- .../Circulation/MarkIssueReturned.t | 16 +- t/db_dependent/Circulation/ReturnClaims.t | 6 +- t/db_dependent/Circulation/Returns.t | 42 +++-- .../Circulation/SwitchOnSiteCheckouts.t | 7 +- t/db_dependent/Circulation/TooMany.t | 17 +- t/db_dependent/Circulation/dateexpiry.t | 20 ++- t/db_dependent/Circulation/issue.t | 24 ++- .../Circulation/maxsuspensiondays.t | 17 +- t/db_dependent/Circulation/transferbook.t | 4 +- t/db_dependent/Circulation_holdsqueue.t | 4 +- t/db_dependent/DecreaseLoanHighHolds.t | 2 +- .../Holds/DisallowHoldIfItemsAvailable.t | 6 +- t/db_dependent/Holds/RevertWaitingStatus.t | 5 +- t/db_dependent/ILSDI_Services.t | 50 +++--- t/db_dependent/Illrequests.t | 2 +- t/db_dependent/Koha/Account/Line.t | 26 ++- t/db_dependent/Koha/Biblio.t | 10 +- t/db_dependent/Koha/Items.t | 20 +-- t/db_dependent/Koha/Object.t | 5 +- t/db_dependent/Koha/Patrons.t | 7 +- .../Koha/Plugins/Circulation_hooks.t | 4 +- t/db_dependent/Koha/Pseudonymization.t | 4 +- .../Koha/Template/Plugin/CirculationRules.t | 2 +- t/db_dependent/Letters/TemplateToolkit.t | 71 ++++---- t/db_dependent/Members/GetAllIssues.t | 4 +- t/db_dependent/Members/IssueSlip.t | 6 +- t/db_dependent/Patron/Borrower_Discharge.t | 66 ++++---- t/db_dependent/Patron/Borrower_PrevCheckout.t | 6 +- t/db_dependent/SIP/ILS.t | 8 +- t/db_dependent/rollingloans.t | 10 +- t/db_dependent/selenium/regressions.t | 2 +- 48 files changed, 490 insertions(+), 454 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index d556b263d6..e3b2be8c87 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -414,16 +414,14 @@ sub transferbook { sub TooMany { - my $borrower = shift; - my $item_object = shift; - my $params = shift; + my ($patron, $item, $params) = @_; my $onsite_checkout = $params->{onsite_checkout} || 0; my $switch_onsite_checkout = $params->{switch_onsite_checkout} || 0; - my $cat_borrower = $borrower->{'categorycode'}; + my $cat_borrower = $patron->categorycode; my $dbh = C4::Context->dbh; # Get which branchcode we need - my $branch = _GetCircControlBranch($item_object->unblessed,$borrower); - my $type = $item_object->effective_itemtype; + my $branch = _GetCircControlBranch($item, $patron); + my $type = $item->effective_itemtype; my ($type_object, $parent_type, $parent_maxissueqty_rule); $type_object = Koha::ItemTypes->find( $type ); @@ -463,8 +461,6 @@ sub TooMany { } ); - - my $patron = Koha::Patrons->find($borrower->{borrowernumber}); # if a rule is found and has a loan limit set, count # how many loans the patron already has that meet that # rule @@ -793,10 +789,10 @@ sub CanBookBeIssued { my $dbh = C4::Context->dbh; my $patron_unblessed = $patron->unblessed; - my $circ_library = Koha::Libraries->find( _GetCircControlBranch($item_unblessed, $patron_unblessed) ); + my $circ_library = Koha::Libraries->find( _GetCircControlBranch($item_object, $patron) ); my $now = dt_from_string(); - $duedate ||= CalcDateDue( $now, $effective_itemtype, $circ_library->branchcode, $patron_unblessed ); + $duedate ||= CalcDateDue( $now, $effective_itemtype, $circ_library->branchcode, $patron ); if (DateTime->compare($duedate,$now) == -1 ) { # duedate cannot be before now $needsconfirmation{INVALID_DATE} = $duedate; } @@ -1003,7 +999,7 @@ sub CanBookBeIssued { and $issue and $issue->onsite_checkout and $issue->borrowernumber == $patron->borrowernumber ? 1 : 0 ); - my $toomany = TooMany( $patron_unblessed, $item_object, { onsite_checkout => $onsite_checkout, switch_onsite_checkout => $switch_onsite_checkout, } ); + my $toomany = TooMany( $patron, $item_object, { onsite_checkout => $onsite_checkout, switch_onsite_checkout => $switch_onsite_checkout, } ); # if TooMany max_allowed returns 0 the user doesn't have permission to check out this book if ( $toomany && not exists $needsconfirmation{RENEW_ISSUE} ) { if ( $toomany->{max_allowed} == 0 ) { @@ -1343,7 +1339,7 @@ sub CanBookBeReturned { sub checkHighHolds { my ( $item, $patron ) = @_; - my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ); + my $branchcode = _GetCircControlBranch( $item, $patron ); my $return_data = { exceeded => 0, @@ -1417,7 +1413,7 @@ sub checkHighHolds { ); my $calendar = Koha::Calendar->new( branchcode => $branchcode, days_mode => $daysmode ); - my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branchcode, $patron->unblessed ); + my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branchcode, $patron ); my $rule = Koha::CirculationRules->get_effective_rule_value( { @@ -1452,13 +1448,13 @@ sub checkHighHolds { =head2 AddIssue - &AddIssue($borrower, $barcode, [$datedue], [$cancelreserve], [$issuedate]) + &AddIssue($patron, $barcode, [$datedue], [$cancelreserve], [$issuedate]) Issue a book. Does no check, they are done in CanBookBeIssued. If we reach this sub, it means the user confirmed if needed. =over 4 -=item C<$borrower> is a hash with borrower informations (from Koha::Patron->unblessed). +=item C<$patron> is a patron object. =item C<$barcode> is the barcode of the item being issued. @@ -1493,7 +1489,7 @@ AddIssue does the following things : =cut sub AddIssue { - my ( $borrower, $barcode, $datedue, $cancelreserve, $issuedate, $sipmode, $params ) = @_; + my ( $patron, $barcode, $datedue, $cancelreserve, $issuedate, $sipmode, $params ) = @_; my $onsite_checkout = $params && $params->{onsite_checkout} ? 1 : 0; my $switch_onsite_checkout = $params && $params->{switch_onsite_checkout}; @@ -1521,22 +1517,22 @@ sub AddIssue { } # Stop here if the patron or barcode doesn't exist - if ( $borrower && $barcode && $barcodecheck ) { + if ( $patron && $barcode && $barcodecheck ) { # find which item we issue my $item_object = Koha::Items->find({ barcode => $barcode }) or return; # if we don't get an Item, abort. my $item_unblessed = $item_object->unblessed; - my $branchcode = _GetCircControlBranch( $item_unblessed, $borrower ); + my $branchcode = _GetCircControlBranch( $item_object, $patron ); # get actual issuing if there is one my $actualissue = $item_object->checkout; # check if we just renew the issue. - if ( $actualissue and $actualissue->borrowernumber eq $borrower->{'borrowernumber'} + if ( $actualissue and $actualissue->borrowernumber eq $patron->borrowernumber and not $switch_onsite_checkout ) { $datedue = AddRenewal( - $borrower->{'borrowernumber'}, + $patron->borrowernumber, $item_object->itemnumber, $branchcode, $datedue, @@ -1547,7 +1543,7 @@ sub AddIssue { unless ($datedue) { my $itype = $item_object->effective_itemtype; - $datedue = CalcDateDue( $issuedate, $itype, $branchcode, $borrower ); + $datedue = CalcDateDue( $issuedate, $itype, $branchcode, $patron ); } @@ -1556,7 +1552,7 @@ sub AddIssue { # Check if there is an ILL connected with the biblio of the item we are issuing my $ill_request = Koha::Illrequests->search({ biblio_id => $item_object->biblionumber, - borrowernumber => $borrower->{'borrowernumber'}, + borrowernumber => $patron->borrowernumber, completed => undef, due_date => { '!=', undef }, })->next; @@ -1571,7 +1567,6 @@ sub AddIssue { $datedue->truncate( to => 'minute' ); - my $patron = Koha::Patrons->find( $borrower ); my $library = Koha::Libraries->find( $branchcode ); my $fees = Koha::Charges::Fees->new( { @@ -1599,12 +1594,12 @@ sub AddIssue { action => $cancel_recall, recall_id => $recall_id, item => $item_object, - borrowernumber => $borrower->{borrowernumber}, + borrowernumber => $patron->borrowernumber, } ); } - C4::Reserves::MoveReserve( $item_object->itemnumber, $borrower->{'borrowernumber'}, $cancelreserve ); + C4::Reserves::MoveReserve( $item_object->itemnumber, $patron->borrowernumber, $cancelreserve ); # Starting process for transfer job (checking transfert and validate it if we have one) if ( my $transfer = $item_object->get_transfer ) { @@ -1629,7 +1624,7 @@ sub AddIssue { unless ($auto_renew) { my $rule = Koha::CirculationRules->get_effective_rule_value( { - categorycode => $borrower->{categorycode}, + categorycode => $patron->categorycode, itemtype => $item_object->effective_itemtype, branchcode => $branchcode, rule_name => 'auto_renew' @@ -1640,7 +1635,7 @@ sub AddIssue { } my $issue_attributes = { - borrowernumber => $borrower->{'borrowernumber'}, + borrowernumber => $patron->borrowernumber, issuedate => $issuedate, date_due => $datedue, branchcode => C4::Context->userenv->{'branch'}, @@ -1674,7 +1669,7 @@ sub AddIssue { )->store; } $issue->discard_changes; - C4::Auth::track_login_daily( $borrower->{userid} ); + C4::Auth::track_login_daily( $patron->userid ); if ( $item_object->location && $item_object->location eq 'CART' && ( !$item_object->permanent_location || $item_object->permanent_location ne 'CART' ) ) { ## Item was moved to cart via UpdateItemLocationOnCheckin, anything issued should be taken off the cart. @@ -1700,31 +1695,30 @@ sub AddIssue { if ($was_lost) { if ( $item_object->{_charge} ) { $actualissue //= Koha::Old::Checkouts->search( - { itemnumber => $item_unblessed->{itemnumber} }, + { itemnumber => $item_object->itemnumber }, { order_by => { '-desc' => 'returndate' }, rows => 1 } )->single; - unless ( exists( $borrower->{branchcode} ) ) { - my $patron = $actualissue->patron; - $borrower = $patron->unblessed; + unless ( $patron->branchcode ) { + $patron = $actualissue->patron; } _CalculateAndUpdateFine( { issue => $actualissue, item => $item_unblessed, - borrower => $borrower, + borrower => $patron->unblessed, return_date => $issuedate } ); - _FixOverduesOnReturn( $borrower->{borrowernumber}, + _FixOverduesOnReturn( $patron->borrowernumber, $item_object->itemnumber, undef, 'RENEWED' ); } } # If it costs to borrow this book, charge it to the patron's account. - my ( $charge, $itemtype ) = GetIssuingCharges( $item_object->itemnumber, $borrower->{'borrowernumber'} ); + my ( $charge, $itemtype ) = GetIssuingCharges( $item_object->itemnumber, $patron->borrowernumber ); if ( $charge && $charge > 0 ) { AddIssuingCharge( $issue, $charge, 'RENT' ); } @@ -1749,9 +1743,9 @@ sub AddIssue { itemnumber => $item_object->itemnumber, itemtype => $item_object->effective_itemtype, location => $item_object->location, - borrowernumber => $borrower->{'borrowernumber'}, + borrowernumber => $patron->borrowernumber, ccode => $item_object->ccode, - categorycode => $borrower->{'categorycode'} + categorycode => $patron->categorycode } ); @@ -1759,7 +1753,7 @@ sub AddIssue { my $circulation_alert = 'C4::ItemCirculationAlertPreference'; my %conditions = ( branchcode => $branchcode, - categorycode => $borrower->{categorycode}, + categorycode => $patron->categorycode, item_type => $item_object->effective_itemtype, notification => 'CHECKOUT', ); @@ -1768,14 +1762,14 @@ sub AddIssue { { type => 'CHECKOUT', item => $item_object->unblessed, - borrower => $borrower, + borrower => $patron->unblessed, branch => $branchcode, } ); } logaction( "CIRCULATION", "ISSUE", - $borrower->{'borrowernumber'}, + $patron->borrowernumber, $item_object->itemnumber, ) if C4::Context->preference("IssueLog"); @@ -2298,7 +2292,7 @@ sub AddReturn { if ( $issue and $issue->is_overdue($return_date) ) { # fix fine days - my ($debardate,$reminder) = _debar_user_on_return( $patron_unblessed, $item->unblessed, dt_from_string($issue->date_due), $return_date ); + my ( $debardate, $reminder ) = _debar_user_on_return( $patron, $item, dt_from_string($issue->date_due), $return_date ); if ($reminder){ $messages->{'PrevDebarred'} = $debardate; } else { @@ -2553,11 +2547,11 @@ sub MarkIssueReturned { =head2 _debar_user_on_return - _debar_user_on_return($borrower, $item, $datedue, $returndate); + _debar_user_on_return($patron, $item, $datedue, $returndate); -C<$borrower> borrower hashref +C<$patron> patron object -C<$item> item hashref +C<$item> patron object C<$datedue> date due DateTime object @@ -2574,13 +2568,13 @@ to ease testing. =cut sub _calculate_new_debar_dt { - my ( $borrower, $item, $dt_due, $return_date ) = @_; + my ( $patron, $item, $dt_due, $return_date ) = @_; - my $branchcode = _GetCircControlBranch( $item, $borrower ); + my $branchcode = _GetCircControlBranch( $item, $patron ); my $circcontrol = C4::Context->preference('CircControl'); my $issuing_rule = Koha::CirculationRules->get_effective_rules( - { categorycode => $borrower->{categorycode}, - itemtype => $item->{itype}, + { categorycode => $patron->categorycode, + itemtype => $item->effective_itemtype, branchcode => $branchcode, rules => [ 'finedays', @@ -2629,8 +2623,8 @@ sub _calculate_new_debar_dt { } my ( $has_been_extended ); - if ( C4::Context->preference('CumulativeRestrictionPeriods') and $borrower->{debarred} ) { - my $debarment = @{ GetDebarments( { borrowernumber => $borrower->{borrowernumber}, type => 'SUSPENSION' } ) }[0]; + if ( C4::Context->preference('CumulativeRestrictionPeriods') and $patron->is_debarred ) { + my $debarment = @{ GetDebarments( { borrowernumber => $patron->borrowernumber, type => 'SUSPENSION' } ) }[0]; if ( $debarment ) { $return_date = dt_from_string( $debarment->{expiration}, 'sql' ); $has_been_extended = 1; @@ -2655,25 +2649,24 @@ sub _calculate_new_debar_dt { } sub _debar_user_on_return { - my ( $borrower, $item, $dt_due, $return_date ) = @_; + my ( $patron, $item, $dt_due, $return_date ) = @_; $return_date //= dt_from_string(); - my $new_debar_dt = _calculate_new_debar_dt ($borrower, $item, $dt_due, $return_date); + my $new_debar_dt = _calculate_new_debar_dt($patron, $item, $dt_due, $return_date); return unless $new_debar_dt; Koha::Patron::Debarments::AddUniqueDebarment({ - borrowernumber => $borrower->{borrowernumber}, + borrowernumber => $patron->borrowernumber, expiration => $new_debar_dt->ymd(), type => 'SUSPENSION', }); # if borrower was already debarred but does not get an extra debarment - my $patron = Koha::Patrons->find( $borrower->{borrowernumber} ); my ($new_debarment_str, $is_a_reminder); - if ( $borrower->{debarred} && $borrower->{debarred} eq $patron->is_debarred ) { + if ( $patron->is_debarred ) { $is_a_reminder = 1; - $new_debarment_str = $borrower->{debarred}; + $new_debarment_str = $patron->debarred; } else { $new_debarment_str = $new_debar_dt->ymd(); } @@ -2764,36 +2757,36 @@ sub _FixOverduesOnReturn { =head2 _GetCircControlBranch - my $circ_control_branch = _GetCircControlBranch($iteminfos, $borrower); + my $circ_control_branch = _GetCircControlBranch($item, $patron); -Internal function : +Internal function : Return the library code to be used to determine which circulation policy applies to a transaction. Looks up the CircControl and HomeOrHoldingBranch system preferences. -C<$iteminfos> is a hashref to iteminfo. Only {homebranch or holdingbranch} is used. +C<$item> is an item object. -C<$borrower> is a hashref to borrower. Only {branchcode} is used. +C<$patron> is a patron object. =cut sub _GetCircControlBranch { - my ($item, $borrower) = @_; + my ($item, $patron) = @_; my $circcontrol = C4::Context->preference('CircControl'); my $branch; if ($circcontrol eq 'PickupLibrary' and (C4::Context->userenv and C4::Context->userenv->{'branch'}) ) { - $branch= C4::Context->userenv->{'branch'}; + $branch = C4::Context->userenv->{'branch'}; } elsif ($circcontrol eq 'PatronLibrary') { - $branch=$borrower->{branchcode}; + $branch = $patron->branchcode; } else { my $branchfield = C4::Context->preference('HomeOrHoldingBranch') || 'homebranch'; - $branch = $item->{$branchfield}; + $branch = $item->get_column($branchfield); # default to item home branch if holdingbranch is used # and is not defined if (!defined($branch) && $branchfield eq 'holdingbranch') { - $branch = $item->{homebranch}; + $branch = $item->homebranch; } } return $branch; @@ -2869,7 +2862,7 @@ sub CanBookBeRenewed { # override_limit will override anything else except on_reserve unless ( $override_limit ){ - my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ); + my $branchcode = _GetCircControlBranch( $item, $patron ); my $issuing_rule = Koha::CirculationRules->get_effective_rules( { categorycode => $patron->categorycode, @@ -3059,7 +3052,7 @@ sub AddRenewal { my $patron = Koha::Patrons->find( $borrowernumber ) or return; # FIXME Should do more than just return my $patron_unblessed = $patron->unblessed; - my $circ_library = Koha::Libraries->find( _GetCircControlBranch($item_unblessed, $patron_unblessed) ); + my $circ_library = Koha::Libraries->find( _GetCircControlBranch($item_object, $patron) ); my $schema = Koha::Database->schema; $schema->txn_do(sub{ @@ -3078,7 +3071,7 @@ sub AddRenewal { $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ? dt_from_string( $issue->date_due, 'sql' ) : dt_from_string(); - $datedue = CalcDateDue($datedue, $itemtype, $circ_library->branchcode, $patron_unblessed, 'is a renewal'); + $datedue = CalcDateDue($datedue, $itemtype, $circ_library->branchcode, $patron, 'is a renewal'); } my $fees = Koha::Charges::Fees->new( @@ -3247,7 +3240,7 @@ sub GetRenewCount { $renewcount = $data->{'renewals_count'} if $data->{'renewals_count'}; $unseencount = $data->{'unseen_renewals'} if $data->{'unseen_renewals'}; # $item and $borrower should be calculated - my $branchcode = _GetCircControlBranch($item->unblessed, $patron->unblessed); + my $branchcode = _GetCircControlBranch($item, $patron); my $rules = Koha::CirculationRules->get_effective_rules( { @@ -3303,7 +3296,7 @@ sub GetSoonestRenewDate { my $dbh = C4::Context->dbh; - my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ); + my $branchcode = _GetCircControlBranch( $item, $patron ); my $issuing_rule = Koha::CirculationRules->get_effective_rules( { categorycode => $patron->categorycode, itemtype => $item->effective_itemtype, @@ -3370,8 +3363,8 @@ sub GetLatestAutoRenewDate { return unless $item; my $dbh = C4::Context->dbh; - my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ); + my $circulation_rules = Koha::CirculationRules->get_effective_rules( { categorycode => $patron->categorycode, @@ -3667,19 +3660,19 @@ checking against the holidays calendar as per the daysmode circulation rule. C<$startdate> = DateTime object representing start date of loan period (assumed to be today) C<$itemtype> = itemtype code of item in question C<$branch> = location whose calendar to use -C<$borrower> = Borrower object +C<$patron> = Patron object C<$isrenewal> = Boolean: is true if we want to calculate the date due for a renewal. Else is false. =cut sub CalcDateDue { - my ( $startdate, $itemtype, $branch, $borrower, $isrenewal ) = @_; + my ( $startdate, $itemtype, $branch, $patron, $isrenewal ) = @_; $isrenewal ||= 0; # loanlength now a href my $loanlength = - GetLoanLength( $borrower->{'categorycode'}, $itemtype, $branch ); + GetLoanLength( $patron->categorycode, $itemtype, $branch ); my $length_key = ( $isrenewal and defined $loanlength->{renewalperiod} and $loanlength->{renewalperiod} ne q{} ) ? qq{renewalperiod} @@ -3699,7 +3692,7 @@ sub CalcDateDue { my $daysmode = Koha::CirculationRules->get_effective_daysmode( { - categorycode => $borrower->{categorycode}, + categorycode => $patron->categorycode, itemtype => $itemtype, branchcode => $branch, } @@ -3733,7 +3726,7 @@ sub CalcDateDue { # if Hard Due Dates are used, retrieve them and apply as necessary my ( $hardduedate, $hardduedatecompare ) = - GetHardDueDate( $borrower->{'categorycode'}, $itemtype, $branch ); + GetHardDueDate( $patron->categorycode, $itemtype, $branch ); if ($hardduedate) { # hardduedates are currently dates $hardduedate->truncate( to => 'minute' ); $hardduedate->set_hour(23); @@ -3753,7 +3746,7 @@ sub CalcDateDue { # if ReturnBeforeExpiry ON the datedue can't be after borrower expirydate if ( C4::Context->preference('ReturnBeforeExpiry') ) { - my $expiry_dt = dt_from_string( $borrower->{dateexpiry}, 'iso', 'floating'); + my $expiry_dt = dt_from_string( $patron->dateexpiry, 'iso', 'floating'); if( $expiry_dt ) { #skip empty expiry date.. $expiry_dt->set( hour => 23, minute => 59); my $d1= $datedue->clone->set_time_zone('floating'); @@ -4037,7 +4030,7 @@ sub ProcessOfflineIssue { ); } AddIssue( - $patron->unblessed, + $patron, $operation->{'barcode'}, undef, undef, diff --git a/C4/SIP/ILS/Transaction/Checkout.pm b/C4/SIP/ILS/Transaction/Checkout.pm index d9056f1c82..990545a02e 100644 --- a/C4/SIP/ILS/Transaction/Checkout.pm +++ b/C4/SIP/ILS/Transaction/Checkout.pm @@ -141,7 +141,7 @@ sub do_checkout { }); } else { # can issue - my $issue = AddIssue( $patron->unblessed, $barcode, $overridden_duedate, 0 ); + my $issue = AddIssue( $patron, $barcode, $overridden_duedate, 0 ); $self->{due} = $self->duedatefromissue($issue, $itemnumber); } diff --git a/Koha/CurbsidePickup.pm b/Koha/CurbsidePickup.pm index cbbd1632d3..06526961a1 100644 --- a/Koha/CurbsidePickup.pm +++ b/Koha/CurbsidePickup.pm @@ -250,7 +250,7 @@ sub mark_as_delivered { unless ( keys %$issuingimpossible ) { my $issue = - C4::Circulation::AddIssue( $patron->unblessed, $hold->item->barcode ); + C4::Circulation::AddIssue( $patron, $hold->item->barcode ); if ($issue) { Koha::CurbsidePickupIssue->new( { diff --git a/Koha/Illrequest.pm b/Koha/Illrequest.pm index 723b11843e..f5423e0b56 100644 --- a/Koha/Illrequest.pm +++ b/Koha/Illrequest.pm @@ -1320,8 +1320,6 @@ sub check_out { } # We can allegedly check out, so make it so - # For some reason, AddIssue requires an unblessed Patron - $issue_args[0] = $patron->unblessed; my $issue = C4::Circulation::AddIssue(@issue_args); if ($issue) { diff --git a/Koha/Item.pm b/Koha/Item.pm index e508e7eb55..3a5b1af6b2 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1869,7 +1869,7 @@ sub can_be_recalled { my $branchcode = C4::Context->userenv->{'branch'}; if ( $patron ) { - $branchcode = C4::Circulation::_GetCircControlBranch( $self->unblessed, $patron->unblessed ); + $branchcode = C4::Circulation::_GetCircControlBranch( $self, $patron ); } # Check the circulation rule for each relevant itemtype for this item diff --git a/Koha/Recall.pm b/Koha/Recall.pm index c993bb7fda..a365b9816d 100644 --- a/Koha/Recall.pm +++ b/Koha/Recall.pm @@ -258,7 +258,7 @@ sub calc_expirationdate { my $branchcode = $self->patron->branchcode; if ( $item ) { - $branchcode = C4::Circulation::_GetCircControlBranch( $item->unblessed, $self->patron->unblessed ); + $branchcode = C4::Circulation::_GetCircControlBranch( $item, $self->patron ); } my $rule = Koha::CirculationRules->get_effective_rule({ diff --git a/circ/circulation.pl b/circ/circulation.pl index b7270bfe56..f9acb6215d 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -400,7 +400,7 @@ if (@$barcodes) { ); $recall_id = ( $recall and $recall->id ) ? $recall->id : undef; } - my $issue = AddIssue( $patron->unblessed, $barcode, $datedue, $cancelreserve, undef, undef, { onsite_checkout => $onsite_checkout, auto_renew => $session->param('auto_renew'), switch_onsite_checkout => $switch_onsite_checkout, cancel_recall => $cancel_recall, recall_id => $recall_id, } ); + my $issue = AddIssue( $patron, $barcode, $datedue, $cancelreserve, undef, undef, { onsite_checkout => $onsite_checkout, auto_renew => $session->param('auto_renew'), switch_onsite_checkout => $switch_onsite_checkout, cancel_recall => $cancel_recall, recall_id => $recall_id, } ); $template_params->{issue} = $issue; $session->clear('auto_renew'); $inprocess = 1; diff --git a/offline_circ/process_koc.pl b/offline_circ/process_koc.pl index b99f5b29ee..d8e38e2026 100755 --- a/offline_circ/process_koc.pl +++ b/offline_circ/process_koc.pl @@ -246,7 +246,6 @@ sub kocIssueItem { my $branchcode = C4::Context->userenv->{branch}; my $patron = Koha::Patrons->find( { cardnumber => $circ->{cardnumber} } ); - my $borrower = $patron->unblessed; my $item = Koha::Items->find({ barcode => $circ->{barcode} }); my $issue = Koha::Checkouts->find( { itemnumber => $item->itemnumber } ); my $biblio = $item->biblio; @@ -254,7 +253,7 @@ sub kocIssueItem { if ( $issue ) { ## Item is currently checked out to another person. #warn "Item Currently Issued."; - if ( $issue->borrowernumber eq $borrower->{'borrowernumber'} ) { ## Issued to this person already, renew it. + if ( $issue->borrowernumber eq $patron->borrowernumber ) { ## Issued to this person already, renew it. #warn "Item issued to this member already, renewing."; C4::Circulation::AddRenewal( @@ -270,11 +269,11 @@ sub kocIssueItem { title => $biblio->title, biblionumber => $biblio->biblionumber, barcode => $item->barcode, - firstname => $borrower->{ 'firstname' }, - surname => $borrower->{ 'surname' }, - borrowernumber => $borrower->{'borrowernumber'}, - cardnumber => $borrower->{'cardnumber'}, - datetime => $circ->{ 'datetime' } + firstname => $patron->firstname, + surname => $patron->surname, + borrowernumber => $patron->borrowernumber, + cardnumber => $patron->cardnumber, + datetime => $circ->datetime }; } else { @@ -285,17 +284,17 @@ sub kocIssueItem { my ( $c_y, $c_m, $c_d ) = split( /-/, $circ->{'date'} ); if ( Date_to_Days( $i_y, $i_m, $i_d ) < Date_to_Days( $c_y, $c_m, $c_d ) ) { ## Current issue to a different persion is older than this issue, return and issue. - C4::Circulation::AddIssue( $borrower, $circ->{'barcode'}, undef, undef, $circ->{'date'} ) unless ( DEBUG ); + C4::Circulation::AddIssue( $patron, $circ->{'barcode'}, undef, undef, $circ->{'date'} ) unless ( DEBUG ); push @output, { issue => 1, title => $biblio->title, biblionumber => $biblio->biblionumber, barcode => $item->barcode, - firstname => $borrower->{ 'firstname' }, - surname => $borrower->{ 'surname' }, - borrowernumber => $borrower->{'borrowernumber'}, - cardnumber => $borrower->{'cardnumber'}, - datetime => $circ->{ 'datetime' } + firstname => $patron->firstname, + surname => $patron->surname, + borrowernumber => $patron->borrowernumber, + cardnumber => $patron->cardnumber, + datetime => $circ->datetime }; } else { ## Current issue is *newer* than this issue, write a 'returned' issue, as the item is most likely in the hands of someone else now. @@ -305,17 +304,17 @@ sub kocIssueItem { } } } else { ## Item is not checked out to anyone at the moment, go ahead and issue it - C4::Circulation::AddIssue( $borrower, $circ->{'barcode'}, undef, undef, $circ->{'date'} ) unless ( DEBUG ); + C4::Circulation::AddIssue( $patron, $circ->{'barcode'}, undef, undef, $circ->{'date'} ) unless ( DEBUG ); push @output, { issue => 1, title => $biblio->title, biblionumber => $biblio->biblionumber, barcode => $item->barcode, - firstname => $borrower->{ 'firstname' }, - surname => $borrower->{ 'surname' }, - borrowernumber => $borrower->{'borrowernumber'}, - cardnumber => $borrower->{'cardnumber'}, - datetime =>$circ->{ 'datetime' } + firstname => $patron->firstname, + surname => $patron->surname, + borrowernumber => $patron->borrowernumber, + cardnumber => $patron->cardnumber, + datetime =>$circ->datetime }; } } diff --git a/opac/sco/sco-main.pl b/opac/sco/sco-main.pl index e36db3d68b..d5e252cc00 100755 --- a/opac/sco/sco-main.pl +++ b/opac/sco/sco-main.pl @@ -245,7 +245,7 @@ elsif ( $patron && ( $op eq 'checkout' ) ) { )->count; } - AddIssue( $patron->unblessed, $barcode ); + AddIssue( $patron, $barcode ); $template->param( issued => 1 ); push @newissueslist, $barcode; diff --git a/reserve/request.pl b/reserve/request.pl index e5faf1426a..f3e8770b70 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -479,8 +479,7 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) } if ( $patron ) { - my $patron_unblessed = $patron->unblessed; - my $branch = C4::Circulation::_GetCircControlBranch($item, $patron_unblessed); + my $branch = _GetCircControlBranch($item_object, $patron); my $branchitemrule = GetBranchItemRule( $branch, $item->{'itype'} ); diff --git a/svc/letters/preview b/svc/letters/preview index 843372ad08..8b7f5ac1a4 100755 --- a/svc/letters/preview +++ b/svc/letters/preview @@ -151,9 +151,7 @@ if ( $code eq 'CHECKIN' ) { my $checkout = Koha::Checkouts->find( { itemnumber => $item->itemnumber } ); if ($checkout) { my $patron = Koha::Patrons->find( $checkout->borrowernumber ); - my $branchcode = - C4::Circulation::_GetCircControlBranch( $item->unblessed, - $patron->unblessed ); + my $branchcode = C4::Circulation::_GetCircControlBranch( $item, $patron ); $letter_params = { tables => { issues => $item->itemnumber, @@ -177,8 +175,7 @@ elsif ( $code eq 'CHECKOUT' ) { my $patron = Koha::Patrons->find( $borrowernumber ); if ($item and $patron) { my $branchcode = - C4::Circulation::_GetCircControlBranch( $item->unblessed, - $patron->unblessed ); + C4::Circulation::_GetCircControlBranch( $item, $patron ); $letter_params = { tables => { issues => $item->itemnumber, diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 322a7ea585..f4f0edaeb2 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -164,14 +164,18 @@ my $patron_category = $builder->build( my $CircControl = C4::Context->preference('CircControl'); my $HomeOrHoldingBranch = C4::Context->preference('HomeOrHoldingBranch'); -my $item = { - homebranch => $library2->{branchcode}, - holdingbranch => $library2->{branchcode} -}; +my $item = $builder->build_object({ + class => 'Koha::Items', + value => { + homebranch => $library2->{branchcode}, + holdingbranch => $library2->{branchcode} + } +}); -my $borrower = { - branchcode => $library2->{branchcode} -}; +my $borrower = $builder->build_object({ + class => 'Koha::Patrons', + value => { branchcode => $library2->{branchcode} } +}); t::lib::Mocks::mock_preference('AutoReturnCheckedOutItems', 0); @@ -185,7 +189,7 @@ is( ); is( C4::Circulation::_GetCircControlBranch($item, $borrower), - $item->{$HomeOrHoldingBranch}, + $item->get_column($HomeOrHoldingBranch), '_GetCircControlBranch returned item branch (no userenv defined)' ); @@ -198,7 +202,7 @@ is( ); is( C4::Circulation::_GetCircControlBranch($item, $borrower), - $borrower->{branchcode}, + $borrower->branchcode, '_GetCircControlBranch returned borrower branch' ); @@ -210,7 +214,7 @@ is( 'CircControl changed to ItemHomeLibrary' ); is( - $item->{$HomeOrHoldingBranch}, + $item->get_column($HomeOrHoldingBranch), C4::Circulation::_GetCircControlBranch($item, $borrower), '_GetCircControlBranch returned item branch' ); @@ -241,7 +245,7 @@ is( ); is( C4::Circulation::_GetCircControlBranch($item, $borrower), - $borrower->{branchcode}, + $borrower->branchcode, '_GetCircControlBranch returned borrower branch' ); @@ -254,7 +258,7 @@ is( ); is( C4::Circulation::_GetCircControlBranch($item, $borrower), - $item->{$HomeOrHoldingBranch}, + $item->get_column($HomeOrHoldingBranch), '_GetCircControlBranch returned item branch' ); @@ -329,7 +333,7 @@ subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers a homebranch => $patron_hold_1->branchcode }); - my $issue = AddIssue( $patron->unblessed, $item_1->barcode); + my $issue = AddIssue( $patron, $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() ); @@ -512,11 +516,11 @@ subtest "CanBookBeRenewed tests" => sub { my $checkitem = undef; my $found = undef; - my $issue_1 = AddIssue( $renewing_borrower_obj->unblessed, $item_1->barcode); + my $issue_1 = AddIssue( $renewing_borrower_obj, $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); + my $issue_2 = AddIssue( $renewing_borrower_obj, $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; @@ -627,7 +631,7 @@ subtest "CanBookBeRenewed tests" => sub { is( $error, 'on_reserve', '(Bug 10663) Cannot renew, reserved (returned error is on_reserve)'); my $reserveid = Koha::Holds->search({ biblionumber => $biblio->biblionumber, borrowernumber => $reserving_borrowernumber })->next->reserve_id; - my $reserving_borrower = Koha::Patrons->find( $reserving_borrowernumber )->unblessed; + my $reserving_borrower = Koha::Patrons->find( $reserving_borrowernumber ); AddIssue($reserving_borrower, $item_3->barcode); my $reserve = $dbh->selectrow_hashref( 'SELECT * FROM old_reserves WHERE reserve_id = ?', @@ -675,7 +679,7 @@ subtest "CanBookBeRenewed tests" => sub { itype => $itemtype, } ); - my $issue_5 = AddIssue($restricted_borrower_obj->unblessed, $item_5->barcode); + my $issue_5 = AddIssue($restricted_borrower_obj, $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'); @@ -704,7 +708,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); - my $issue_6 = AddIssue( $renewing_borrower_obj->unblessed, $item_6->barcode); + my $issue_6 = AddIssue( $renewing_borrower_obj, $item_6->barcode); is (defined $issue_6, 1, "Item 2 checked out, due date: ".$issue_6->date_due); my $now = dt_from_string(); @@ -712,7 +716,7 @@ subtest "CanBookBeRenewed tests" => sub { my $five_weeks_ago = $now - $five_weeks; t::lib::Mocks::mock_preference('finesMode', 'production'); - my $issue_7 = AddIssue($renewing_borrower_obj->unblessed, $item_7->barcode, $five_weeks_ago); + my $issue_7 = AddIssue($renewing_borrower_obj, $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'); @@ -793,7 +797,7 @@ subtest "CanBookBeRenewed tests" => sub { my $old_issue_log_size = Koha::ActionLogs->count( \%params_issue ); my $old_renew_log_size = Koha::ActionLogs->count( \%params_renewal ); AddIssue( - $renewing_borrower_obj->unblessed, + $renewing_borrower_obj, $item_7->barcode, Koha::DateUtils::output_pref({str=>$issue_6->date_due, dateformat =>'iso'}), 0, @@ -821,7 +825,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); - my $issue_4 = AddIssue( $renewing_borrower_obj->unblessed, $item_4->barcode, undef, undef, undef, undef, { auto_renew => 1 } ); + my $issue_4 = AddIssue( $renewing_borrower_obj, $item_4->barcode, undef, undef, undef, undef, { auto_renew => 1 } ); my $info; ( $renewokay, $error, $info ) = CanBookBeRenewed( $renewing_borrower_obj, $issue_4 ); @@ -951,7 +955,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 ); - my $issue = AddIssue( $renewing_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + my $issue = AddIssue( $renewing_borrower_obj, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); Koha::CirculationRules->set_rules( { @@ -1080,7 +1084,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 ); - my $issue = AddIssue( $renewing_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + my $issue = AddIssue( $renewing_borrower_obj, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); Koha::CirculationRules->set_rules( { @@ -1190,7 +1194,7 @@ subtest "CanBookBeRenewed tests" => sub { # Patron is expired and BlockExpiredPatronOpacActions=0 # => auto renew is allowed t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 0); - my $issue = AddIssue( $expired_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + my $issue = AddIssue( $expired_borrower_obj, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); ( $renewokay, $error ) = CanBookBeRenewed( $expired_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); @@ -1201,7 +1205,7 @@ subtest "CanBookBeRenewed tests" => sub { # Patron is expired and BlockExpiredPatronOpacActions=1 # => auto renew is not allowed t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 1); - $issue = AddIssue( $expired_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + $issue = AddIssue( $expired_borrower_obj, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); ( $renewokay, $error ) = CanBookBeRenewed( $expired_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); @@ -1211,7 +1215,7 @@ subtest "CanBookBeRenewed tests" => sub { # Patron is not expired and BlockExpiredPatronOpacActions=1 # => auto renew is allowed t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 1); - $issue = AddIssue( $renewing_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + $issue = AddIssue( $renewing_borrower_obj, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $issue ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); @@ -1230,7 +1234,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 ); - my $issue = AddIssue( $renewing_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + my $issue = AddIssue( $renewing_borrower_obj, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); Koha::CirculationRules->set_rules( { categorycode => undef, @@ -1482,7 +1486,7 @@ subtest "CanBookBeRenewed tests" => sub { my $recall_item1 = $builder->build_sample_item({ biblionumber => $recall_biblio->biblionumber }); my $recall_item2 = $builder->build_sample_item({ biblionumber => $recall_biblio->biblionumber }); - my $recall_issue = AddIssue( $renewing_borrower_obj->unblessed, $recall_item1->barcode ); + my $recall_issue = AddIssue( $renewing_borrower_obj, $recall_item1->barcode ); # item-level and this item: renewal not allowed my $recall = Koha::Recall->new({ @@ -1563,7 +1567,7 @@ subtest "GetUpcomingDueIssues" => sub { ); my $a_borrower_borrowernumber = Koha::Patron->new(\%a_borrower_data)->store->borrowernumber; - my $a_borrower = Koha::Patrons->find( $a_borrower_borrowernumber )->unblessed; + my $a_borrower = Koha::Patrons->find( $a_borrower_borrowernumber ); my $yesterday = DateTime->today(time_zone => C4::Context->tz())->add( days => -1 ); my $two_days_ahead = DateTime->today(time_zone => C4::Context->tz())->add( days => 2 ); @@ -1640,7 +1644,7 @@ subtest "Bug 13841 - Do not create new 0 amount fines" => sub { my $borrowernumber = Koha::Patron->new(\%a_borrower_data)->store->borrowernumber; - my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; + my $borrower = Koha::Patrons->find( $borrowernumber ); my $issue = AddIssue( $borrower, $item->barcode ); UpdateFine( { @@ -1893,7 +1897,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { branchcode => $branch, })->store; - my $issue = AddIssue( $borrower->unblessed, $item->barcode, undef, undef, undef, undef, { onsite_checkout => 1 } ); + my $issue = AddIssue( $borrower, $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' ); @@ -1913,7 +1917,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { ); my $patron = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library->{branchcode}, categorycode => $patron_category->{categorycode} } } ); - my $issue = AddIssue( $patron->unblessed, $item->barcode ); + my $issue = AddIssue( $patron, $item->barcode ); UpdateFine( { issue_id => $issue->id, @@ -1972,7 +1976,7 @@ subtest 'CanBookBeIssued & AllowReturnToBranch' => sub { set_userenv($holdingbranch); - my $issue = AddIssue( $patron_1->unblessed, $item->barcode ); + my $issue = AddIssue( $patron_1, $item->barcode ); is( ref($issue), 'Koha::Checkout', 'AddIssue should return a Koha::Checkout object' ); my ( $error, $question, $alerts ); @@ -2047,8 +2051,17 @@ subtest 'AddIssue & AllowReturnToBranch' => sub { my $homebranch = $builder->build( { source => 'Branch' } ); my $holdingbranch = $builder->build( { source => 'Branch' } ); my $otherbranch = $builder->build( { source => 'Branch' } ); - my $patron_1 = $builder->build( { source => 'Borrower', value => { categorycode => $patron_category->{categorycode} } } ); - my $patron_2 = $builder->build( { source => 'Borrower', value => { categorycode => $patron_category->{categorycode} } } ); + + my $patron_1 = $builder->build_object({ + class => 'Koha::Patrons', + value => { categorycode => $patron_category->{categorycode} } + }); + + my $patron_2 = $builder->build_object({ + class => 'Koha::Patrons', + value => { categorycode => $patron_category->{categorycode} } + }); + my $item = $builder->build_sample_item( { @@ -4091,7 +4104,7 @@ subtest 'CanBookBeIssued | is_overdue' => sub { } ); - my $issue = AddIssue( $patron->unblessed, $item->barcode, $five_days_go ); # date due was 10d ago + my $issue = AddIssue( $patron, $item->barcode, $five_days_go ); # date due was 10d ago my $actualissue = Koha::Checkouts->find( { itemnumber => $item->itemnumber } ); is( output_pref({ str => $actualissue->date_due, dateonly => 1}), output_pref({ str => $five_days_go, dateonly => 1}), "First issue works"); my ($issuingimpossible, $needsconfirmation) = CanBookBeIssued($patron, $item->barcode, $ten_days_go, undef, undef, undef); @@ -4383,7 +4396,7 @@ subtest 'AddReturn | recalls' => sub { }); # this item can fill a recall with pickup at this branch - AddIssue( $patron1->unblessed, $item1->barcode ); + AddIssue( $patron1, $item1->barcode ); my $recall1 = Koha::Recall->new( { patron_id => $patron2->borrowernumber, biblio_id => $item1->biblionumber, @@ -4397,7 +4410,7 @@ subtest 'AddReturn | recalls' => sub { $recall1->set_cancelled; # this item can fill a recall but needs transfer - AddIssue( $patron1->unblessed, $item1->barcode ); + AddIssue( $patron1, $item1->barcode ); $recall1 = Koha::Recall->new( { patron_id => $patron2->borrowernumber, biblio_id => $item1->biblionumber, @@ -4411,7 +4424,7 @@ subtest 'AddReturn | recalls' => sub { $recall1->set_cancelled; # this item is already in transit, do not ask to transfer - AddIssue( $patron1->unblessed, $item1->barcode ); + AddIssue( $patron1, $item1->barcode ); $recall1 = Koha::Recall->new( { patron_id => $patron2->borrowernumber, biblio_id => $item1->biblionumber, @@ -4489,7 +4502,7 @@ subtest 'AddRenewal and AddIssuingCharge tests' => sub { $context->mock( userenv => { branch => $library->id } ); # Check the item out - AddIssue( $patron->unblessed, $item->barcode ); + AddIssue( $patron, $item->barcode ); throws_ok { AddRenewal( $patron->borrowernumber, $item->itemnumber, $library->id, undef, {break=>"the_renewal"} ); @@ -4547,7 +4560,7 @@ subtest 'AddRenewal and AddIssuingCharge tests' => sub { is( $new_stats_size, $old_stats_size + 1, 'renew statistic successfully added with passed branch' ); AddReturn( $item->id, $library->id, undef, $date ); - AddIssue( $patron->unblessed, $item->barcode, $now ); + AddIssue( $patron, $item->barcode, $now ); AddRenewal( $patron->id, $item->id, $library->id, undef, undef, 1 ); my $lines_skipped = Koha::Account::Lines->search({ borrowernumber => $patron->id, @@ -4571,7 +4584,7 @@ subtest 'AddRenewal() adds to renewals' => sub { set_userenv( $library->unblessed ); # Check the item out - my $issue = AddIssue( $patron->unblessed, $item->barcode ); + my $issue = AddIssue( $patron, $item->barcode ); is(ref($issue), 'Koha::Checkout', 'Issue added'); # Renew item @@ -4655,7 +4668,7 @@ subtest 'Incremented fee tests' => sub { # Daily Tests my $issue = - AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from ); + AddIssue( $patron, $item->barcode, $dt_to, undef, $dt_from ); my $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } ); is( $accountline->amount + 0, @@ -4676,7 +4689,7 @@ subtest 'Incremented fee tests' => sub { t::lib::Mocks::mock_preference( 'finesCalendar', 'noFinesWhenClosed' ); $itemtype->rentalcharge_daily_calendar(1)->store(); $issue = - AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from ); + AddIssue( $patron, $item->barcode, $dt_to, undef, $dt_from ); $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } ); is( $accountline->amount + 0, @@ -4707,7 +4720,7 @@ subtest 'Incremented fee tests' => sub { description => 'Test holiday' ); $issue = - AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from ); + AddIssue( $patron, $item->barcode, $dt_to, undef, $dt_from ); $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } ); is( $accountline->amount + 0, @@ -4728,7 +4741,7 @@ subtest 'Incremented fee tests' => sub { $itemtype->rentalcharge(2)->store; is( $itemtype->rentalcharge + 0, 2, 'Rental charge updated and retreived correctly' ); $issue = - AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from ); + AddIssue( $patron, $item->barcode, $dt_to, undef, $dt_from ); my $accountlines = Koha::Account::Lines->search( { itemnumber => $item->id } ); is( $accountlines->count, '2', "Fixed charge and accrued charge recorded distinctly" ); @@ -4760,7 +4773,7 @@ subtest 'Incremented fee tests' => sub { $itemtype->rentalcharge_hourly_calendar(0)->store(); $issue = - AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from ); + AddIssue( $patron, $item->barcode, $dt_to, undef, $dt_from ); $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } ); is( $accountline->amount + 0, @@ -4780,7 +4793,7 @@ subtest 'Incremented fee tests' => sub { $itemtype->rentalcharge_hourly_calendar(1)->store(); $issue = - AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from ); + AddIssue( $patron, $item->barcode, $dt_to, undef, $dt_from ); $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } ); is( $accountline->amount + 0, @@ -4800,7 +4813,7 @@ subtest 'Incremented fee tests' => sub { $calendar->delete_holiday( weekday => $closed_day ); $issue = - AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from ); + AddIssue( $patron, $item->barcode, $dt_to, undef, $dt_from ); $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } ); is( $accountline->amount + 0, @@ -4914,7 +4927,7 @@ subtest 'Do not return on renewal (LOST charge)' => sub { ); my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); - AddIssue( $patron->unblessed, $item->barcode ); + AddIssue( $patron, $item->barcode ); my $accountline = Koha::Account::Line->new( { @@ -4929,7 +4942,7 @@ subtest 'Do not return on renewal (LOST charge)' => sub { )->store(); # AddRenewal doesn't call _FixAccountForLostAndFound - AddIssue( $patron->unblessed, $item->barcode ); + AddIssue( $patron, $item->barcode ); is( $patron->checkouts->count, 1, 'Renewal should not return the item even if a LOST payment has been made earlier' @@ -5035,7 +5048,7 @@ subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddReturn' => sub { replacementprice => '42', } ); - my $issue = AddIssue( $patron->unblessed, $item->barcode ); + my $issue = AddIssue( $patron, $item->barcode ); LostItem( $item->itemnumber, 'cli', 0 ); $item->_result->itemlost(1); $item->_result->itemlost_on( $lost_on ); @@ -5070,7 +5083,7 @@ subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddReturn' => sub { replacementprice => '42', } ); - my $issue = AddIssue( $patron->unblessed, $item->barcode ); + my $issue = AddIssue( $patron, $item->barcode ); LostItem( $item->itemnumber, 'cli', 0 ); $item->_result->itemlost(1); $item->_result->itemlost_on( $lost_on ); @@ -5105,7 +5118,7 @@ subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddReturn' => sub { replacementprice => '42', } ); - my $issue = AddIssue( $patron->unblessed, $item->barcode ); + my $issue = AddIssue( $patron, $item->barcode ); LostItem( $item->itemnumber, 'cli', 0 ); $item->_result->itemlost(1); $item->_result->itemlost_on( $lost_on ); @@ -5140,7 +5153,7 @@ subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddReturn' => sub { replacementprice => '42', } ); - my $issue = AddIssue( $patron->unblessed, $item->barcode ); + my $issue = AddIssue( $patron, $item->barcode ); LostItem( $item->itemnumber, 'cli', 0 ); $item->_result->itemlost(1); $item->_result->itemlost_on( $lost_on ); @@ -5228,7 +5241,7 @@ subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddIssue' => sub { replacementprice => '42', } ); - my $issue = AddIssue( $patron->unblessed, $item->barcode ); + my $issue = AddIssue( $patron, $item->barcode ); LostItem( $item->itemnumber, 'cli', 0 ); $item->_result->itemlost(1); $item->_result->itemlost_on( $lost_on ); @@ -5242,7 +5255,7 @@ subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddIssue' => sub { )->next; ok( $a, "Found accountline for lost fee" ); is( $a->amountoutstanding + 0, 42, "Lost fee charged correctly" ); - $issue = AddIssue( $patron2->unblessed, $item->barcode ); + $issue = AddIssue( $patron2, $item->barcode ); $a = $a->get_from_storage; is( $a->amountoutstanding + 0, 0, "Lost fee was refunded" ); $a->delete; @@ -5264,7 +5277,7 @@ subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddIssue' => sub { replacementprice => '42', } ); - my $issue = AddIssue( $patron->unblessed, $item->barcode ); + my $issue = AddIssue( $patron, $item->barcode ); LostItem( $item->itemnumber, 'cli', 0 ); $item->_result->itemlost(1); $item->_result->itemlost_on( $lost_on ); @@ -5278,7 +5291,7 @@ subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddIssue' => sub { )->next; ok( $a, "Found accountline for lost fee" ); is( $a->amountoutstanding + 0, 42, "Lost fee charged correctly" ); - $issue = AddIssue( $patron2->unblessed, $item->barcode ); + $issue = AddIssue( $patron2, $item->barcode ); $a = $a->get_from_storage; is( $a->amountoutstanding + 0, 0, "Lost fee was refunded" ); $a->delete; @@ -5299,7 +5312,7 @@ subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddIssue' => sub { replacementprice => '42', } ); - my $issue = AddIssue( $patron->unblessed, $item->barcode ); + my $issue = AddIssue( $patron, $item->barcode ); LostItem( $item->itemnumber, 'cli', 0 ); $item->_result->itemlost(1); $item->_result->itemlost_on( $lost_on ); @@ -5313,7 +5326,7 @@ subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddIssue' => sub { )->next; ok( $a, "Found accountline for lost fee" ); is( $a->amountoutstanding + 0, 42, "Lost fee charged correctly" ); - $issue = AddIssue( $patron2->unblessed, $item->barcode ); + $issue = AddIssue( $patron2, $item->barcode ); $a = $a->get_from_storage; is( $a->amountoutstanding + 0, 42, "Lost fee was not refunded" ); $a->delete; @@ -5334,7 +5347,7 @@ subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddIssue' => sub { replacementprice => '42', } ); - my $issue = AddIssue( $patron->unblessed, $item->barcode ); + my $issue = AddIssue( $patron, $item->barcode ); LostItem( $item->itemnumber, 'cli', 0 ); $item->_result->itemlost(1); $item->_result->itemlost_on( $lost_on ); @@ -5349,7 +5362,7 @@ subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddIssue' => sub { $a = $a->next; ok( $a, "Found accountline for lost fee" ); is( $a->amountoutstanding + 0, 42, "Lost fee charged correctly" ); - $issue = AddIssue( $patron2->unblessed, $item->barcode ); + $issue = AddIssue( $patron2, $item->barcode ); $a = $a->get_from_storage; is( $a->amountoutstanding + 0, 42, "Lost fee was not refunded" ); $a->delete; @@ -5435,7 +5448,7 @@ subtest 'Checkout should correctly terminate a transfer' => sub { is( $transfer->reason, 'Reserve' ); t::lib::Mocks::mock_userenv( { branchcode => $library_2->branchcode } ); - AddIssue( $patron_1->unblessed, $item->barcode ); + AddIssue( $patron_1, $item->barcode ); $transfer = $transfer->get_from_storage; isnt( $transfer->datearrived, undef ); $hold = $hold->get_from_storage; @@ -5478,14 +5491,14 @@ subtest 'AddIssue records staff who checked out item if appropriate' => sub { my $dt_from = dt_from_string(); my $dt_to = dt_from_string()->add( days => 7 ); - my $issue_1 = AddIssue( $patron->unblessed, $item_1->barcode, $dt_to, undef, $dt_from ); + my $issue_1 = AddIssue( $patron, $item_1->barcode, $dt_to, undef, $dt_from ); is( $issue_1->issuer, undef, "Staff who checked out the item not recorded when RecordStaffUserOnCheckout turned off" ); t::lib::Mocks::mock_preference('RecordStaffUserOnCheckout', 1); my $issue_2 = - AddIssue( $patron->unblessed, $item_2->barcode, $dt_to, undef, $dt_from ); + AddIssue( $patron, $item_2->barcode, $dt_to, undef, $dt_from ); is( $issue_2->issuer->borrowernumber, $issuer->borrowernumber, "Staff who checked out the item recorded when RecordStaffUserOnCheckout turned on" ); }; @@ -5513,9 +5526,9 @@ subtest "Item's onloan value should be set if checked out item is checked out to } ); - AddIssue( $patron_1->unblessed, $item->barcode ); + AddIssue( $patron_1, $item->barcode ); ok( $item->get_from_storage->onloan, "Item's onloan column is set after initial checkout" ); - AddIssue( $patron_2->unblessed, $item->barcode ); + AddIssue( $patron_2, $item->barcode ); ok( $item->get_from_storage->onloan, "Item's onloan column is set after second checkout" ); }; @@ -5592,7 +5605,7 @@ subtest "SendCirculationAlert" => sub { })->store; # Checkout an item, mark it returned, generate a notice - my $issue_1 = AddIssue( $patron->unblessed, $item->barcode); + my $issue_1 = AddIssue( $patron, $item->barcode); MarkIssueReturned( $patron->borrowernumber, $item->itemnumber, undef, 0, { skip_record_index => 1} ); C4::Circulation::SendCirculationAlert({ type => 'CHECKIN', @@ -5606,7 +5619,7 @@ subtest "SendCirculationAlert" => sub { is($notice->to_address, $patron->smsalertnumber, "Letter has the correct to_address set to smsalertnumber for SMS type notices"); # Checkout an item, mark it returned, generate a notice - my $issue_2 = AddIssue( $patron->unblessed, $item->barcode); + my $issue_2 = AddIssue( $patron, $item->barcode); MarkIssueReturned( $patron->borrowernumber, $item->itemnumber, undef, 0, { skip_record_index => 1} ); C4::Circulation::SendCirculationAlert({ type => 'CHECKIN', @@ -5640,7 +5653,7 @@ subtest "GetSoonestRenewDate tests" => sub { } ); my $item = $builder->build_sample_item(); - my $issue = AddIssue( $patron->unblessed, $item->barcode); + my $issue = AddIssue( $patron, $item->barcode); my $datedue = dt_from_string( $issue->date_due() ); # Bug 14395 diff --git a/t/db_dependent/Circulation/CalcDateDue.t b/t/db_dependent/Circulation/CalcDateDue.t index cdcd800870..215e2e7e88 100755 --- a/t/db_dependent/Circulation/CalcDateDue.t +++ b/t/db_dependent/Circulation/CalcDateDue.t @@ -18,9 +18,21 @@ my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; my $builder = t::lib::TestBuilder->new; -my $categorycode = 'B'; -my $itemtype = 'MX'; -my $branchcode = 'FPL'; + +my $library = $builder->build_object({ class => 'Koha::Libraries' })->store; +my $dateexpiry = '2013-01-01'; +my $patron_category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { category_type => 'B' } })->store; +my $borrower = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $patron_category->categorycode, + dateexpiry => $dateexpiry, + } + } +)->store; + +my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' })->store->itemtype; my $issuelength = 10; my $renewalperiod = 5; my $lengthunit = 'days'; @@ -28,9 +40,9 @@ my $lengthunit = 'days'; Koha::CirculationRules->search()->delete(); Koha::CirculationRules->set_rules( { - categorycode => $categorycode, + categorycode => $patron_category->categorycode, itemtype => $itemtype, - branchcode => $branchcode, + branchcode => $library->branchcode, rules => { issuelength => $issuelength, renewalperiod => $renewalperiod, @@ -43,13 +55,12 @@ Koha::CirculationRules->set_rules( t::lib::Mocks::mock_preference('ReturnBeforeExpiry', 1); t::lib::Mocks::mock_preference('useDaysMode', 'Days'); +my $branchcode = $library->branchcode; + my $cache = Koha::Caches->get_instance(); my $key = $branchcode . "_holidays"; $cache->clear_from_cache($key); -my $dateexpiry = '2013-01-01'; - -my $borrower = {categorycode => 'B', dateexpiry => $dateexpiry}; my $start_date = DateTime->new({year => 2013, month => 2, day => 9}); my $date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower ); is($date, $dateexpiry . 'T23:59:00', 'date expiry'); @@ -60,7 +71,6 @@ $date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borr t::lib::Mocks::mock_preference('ReturnBeforeExpiry', 1); t::lib::Mocks::mock_preference('useDaysMode', 'noDays'); -$borrower = {categorycode => 'B', dateexpiry => $dateexpiry}; $start_date = DateTime->new({year => 2013, month => 2, day => 9}); $date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower ); is($date, $dateexpiry . 'T23:59:00', 'date expiry with useDaysMode to noDays'); @@ -95,7 +105,6 @@ $date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borr t::lib::Mocks::mock_preference('ReturnBeforeExpiry', 0); t::lib::Mocks::mock_preference('useDaysMode', 'Days'); -$borrower = {categorycode => 'B', dateexpiry => $dateexpiry}; $start_date = DateTime->new({year => 2013, month => 2, day => 9}); $date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower ); is($date, '2013-02-' . (9 + $issuelength) . 'T23:59:00', "date expiry ( 9 + $issuelength )"); @@ -158,18 +167,17 @@ $calendar->delete_holiday( # Now we test it does the right thing if the loan and renewal periods # are a multiple of 7 days -my $dayweek_categorycode = 'K'; -my $dayweek_itemtype = 'MX'; -my $dayweek_branchcode = 'FPL'; my $dayweek_issuelength = 14; my $dayweek_renewalperiod = 7; my $dayweek_lengthunit = 'days'; +$patron_category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { category_type => 'K' } })->store; + Koha::CirculationRules->set_rules( { - categorycode => $dayweek_categorycode, - itemtype => $dayweek_itemtype, - branchcode => $dayweek_branchcode, + categorycode => $patron_category->categorycode, + itemtype => $itemtype, + branchcode => $branchcode, rules => { issuelength => $dayweek_issuelength, renewalperiod => $dayweek_renewalperiod, @@ -178,7 +186,15 @@ Koha::CirculationRules->set_rules( } ); -my $dayweek_borrower = {categorycode => 'K', dateexpiry => $dateexpiry}; +my $dayweek_borrower = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $patron_category->categorycode, + dateexpiry => $dateexpiry, + } + } +); # For issues... $start_date = DateTime->new({year => 2013, month => 2, day => 9}); diff --git a/t/db_dependent/Circulation/CheckIfIssuedToPatron.t b/t/db_dependent/Circulation/CheckIfIssuedToPatron.t index 447d9545ad..b4a428ffe9 100755 --- a/t/db_dependent/Circulation/CheckIfIssuedToPatron.t +++ b/t/db_dependent/Circulation/CheckIfIssuedToPatron.t @@ -66,11 +66,31 @@ my ($biblionumber2) = AddBiblio(MARC::Record->new, ''); my $barcode2 = '0202'; Koha::Item->new({ barcode => $barcode2, %item_info, biblionumber => $biblionumber2 })->store; -my $borrowernumber1 = Koha::Patron->new({categorycode => $categorycode, branchcode => $branchcode})->store->borrowernumber; -my $borrowernumber2 = Koha::Patron->new({categorycode => $categorycode, branchcode => $branchcode})->store->borrowernumber; -# FIXME following code must be simplified to use the Koha::Patron objects -my $borrower1 = Koha::Patrons->find( $borrowernumber1 )->unblessed; -my $borrower2 = Koha::Patrons->find( $borrowernumber2 )->unblessed; +my $patron1 = $builder->build_object( + { + class => 'Koha::Patrons' + value => { + { + categorycode => $categorycode, + branchcode => $branchcode + } + } + } +); +my $patron2 = $builder->build_object( + { + class => 'Koha::Patrons' + value => { + { + categorycode => $categorycode, + branchcode => $branchcode + } + } + } +); + +my $borrowernumber1 = $patron1->borrowernumber; +my $borrowernumber2 = $patron2->borrowernumber; my $module = Test::MockModule->new('C4::Context'); $module->mock('userenv', sub { { branch => $branchcode } }); @@ -91,7 +111,7 @@ is( $check_if_issued, undef, 'CheckIfIssuedToPatron returns undef' ); $check_if_issued = C4::Circulation::CheckIfIssuedToPatron($borrowernumber2, $biblionumber2); is( $check_if_issued, undef, 'CheckIfIssuedToPatron returns undef' ); -AddIssue($borrower1, '0101'); +AddIssue($patron1, '0101'); $check_if_issued = C4::Circulation::CheckIfIssuedToPatron(); is( $check_if_issued, undef, 'CheckIfIssuedToPatron without argument returns undef' ); $check_if_issued = C4::Circulation::CheckIfIssuedToPatron(undef, $biblionumber1); @@ -107,7 +127,7 @@ is( $check_if_issued, undef, 'CheckIfIssuedToPatron returns undef' ); $check_if_issued = C4::Circulation::CheckIfIssuedToPatron($borrowernumber2, $biblionumber2); is( $check_if_issued, undef, 'CheckIfIssuedToPatron returns undef' ); -AddIssue($borrower2, '0202'); +AddIssue($patron2, '0202'); $check_if_issued = C4::Circulation::CheckIfIssuedToPatron(); is( $check_if_issued, undef, 'CheckIfIssuedToPatron without argument returns undef' ); $check_if_issued = C4::Circulation::CheckIfIssuedToPatron(undef, $biblionumber1); diff --git a/t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t b/t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t index 15d59521d9..01d60d9790 100755 --- a/t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t +++ b/t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t @@ -57,26 +57,24 @@ my ($biblionumber2) = AddBiblio(MARC::Record->new, ''); my $itemnumber3 = Koha::Item->new({ barcode => '0203', %item_infos, biblionumber => $biblionumber2})->store->itemnumber; my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; -my $borrowernumber = $builder->build( - { source => 'Borrower', +my $patron = $builder->build_object( + { class => 'Koha::Patrons', value => { categorycode => $categorycode, branchcode => $branchcode } } -)->{borrowernumber}; - -my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; +); # Need to mock userenv for AddIssue my $module = Test::MockModule->new('C4::Context'); $module->mock('userenv', sub { { branch => $branchcode } }); -AddIssue($borrower, '0101'); -AddIssue($borrower, '0203'); +AddIssue($patron, '0101'); +AddIssue($patron, '0203'); # Begin tests... my $onsite_checkouts = GetPendingOnSiteCheckouts; is( scalar @$onsite_checkouts, 0, "No pending on-site checkouts" ); my $itemnumber4 = Koha::Item->new({ barcode => '0104', %item_infos, biblionumber => $biblionumber1})->store->itemnumber; -AddIssue( $borrower, '0104', undef, undef, undef, undef, { onsite_checkout => 1 } ); +AddIssue( $patron, '0104', undef, undef, undef, undef, { onsite_checkout => 1 } ); $onsite_checkouts = GetPendingOnSiteCheckouts; is( scalar @$onsite_checkouts, 1, "There is 1 pending on-site checkout" ); diff --git a/t/db_dependent/Circulation/GetTopIssues.t b/t/db_dependent/Circulation/GetTopIssues.t index a5dc375e74..3130faeff0 100755 --- a/t/db_dependent/Circulation/GetTopIssues.t +++ b/t/db_dependent/Circulation/GetTopIssues.t @@ -68,7 +68,7 @@ my $borrowernumber = Koha::Patron->new({ categorycode => $category, branchcode => $branch_1->{ branchcode } })->store->borrowernumber; -my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; +my $borrower = Koha::Patrons->find( $borrowernumber ); AddIssue($borrower, 'GTI_BARCODE_001'); AddIssue($borrower, 'GTI_BARCODE_002'); diff --git a/t/db_dependent/Circulation/IsItemIssued.t b/t/db_dependent/Circulation/IsItemIssued.t index 8238969566..5cfae54e70 100755 --- a/t/db_dependent/Circulation/IsItemIssued.t +++ b/t/db_dependent/Circulation/IsItemIssued.t @@ -42,14 +42,14 @@ my $patron_category = $builder->build({ source => 'Category' }); t::lib::Mocks::mock_userenv({ branchcode => $library->{branchcode} }); -my $borrowernumber = Koha::Patron->new({ +my $borrower = Koha::Patron->new({ firstname => 'my firstname', surname => 'my surname', categorycode => $patron_category->{categorycode}, branchcode => $library->{branchcode}, -})->store->borrowernumber; +})->store; + -my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; my $record = MARC::Record->new(); my ( $biblionumber, $biblioitemnumber ) = AddBiblio( $record, '' ); diff --git a/t/db_dependent/Circulation/MarkIssueReturned.t b/t/db_dependent/Circulation/MarkIssueReturned.t index 654ac37451..76bc8fa7e1 100755 --- a/t/db_dependent/Circulation/MarkIssueReturned.t +++ b/t/db_dependent/Circulation/MarkIssueReturned.t @@ -63,7 +63,7 @@ subtest 'Failure tests' => sub { is( $issue_id, undef, 'No issue_id returned' ); # In the next call we return the item and try it another time - $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + $issue = C4::Circulation::AddIssue( $patron, $item->barcode ); eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item->itemnumber, undef, 0 ) }; is( $issue_id, $issue->issue_id, "Item has been returned (issue $issue_id)" ); eval { $issue_id = C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item->itemnumber, undef, 0 ) }; @@ -98,7 +98,7 @@ subtest 'Anonymous patron tests' => sub { # Anonymous patron not set t::lib::Mocks::mock_preference( 'AnonymousPatron', '' ); - my $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + my $issue = C4::Circulation::AddIssue( $patron, $item->barcode ); throws_ok { C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item->itemnumber, undef, 2 ); } @@ -117,7 +117,7 @@ subtest 'Anonymous patron tests' => sub { } }); t::lib::Mocks::mock_preference('AnonymousPatron', $anonymous->borrowernumber); - $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + $issue = C4::Circulation::AddIssue( $patron, $item->barcode ); eval { C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item->itemnumber, undef, 2 ) }; is ( $@, q||, 'AnonymousPatron is set correctly - no error expected'); @@ -148,14 +148,14 @@ subtest 'Manually pass a return date' => sub { my ( $issue, $issue_id ); - $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + $issue = C4::Circulation::AddIssue( $patron, $item->barcode ); $issue_id = C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item->itemnumber, '2018-12-25', 0 ); is( $issue_id, $issue->issue_id, "Item has been returned" ); my $old_checkout = Koha::Old::Checkouts->find( $issue_id ); is( $old_checkout->returndate, '2018-12-25 00:00:00', 'Manually passed date stored correctly' ); - $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + $issue = C4::Circulation::AddIssue( $patron, $item->barcode ); { # Hiding the expected warning displayed by DBI @@ -185,9 +185,9 @@ subtest 'AutoRemoveOverduesRestrictions' => sub { my $item_2 = $builder->build_sample_item; my $item_3 = $builder->build_sample_item; my $five_days_ago = dt_from_string->subtract( days => 5 ); - my $checkout_1 = AddIssue( $patron->unblessed, $item_1->barcode, $five_days_ago ); # overdue - my $checkout_2 = AddIssue( $patron->unblessed, $item_2->barcode, $five_days_ago ); # overdue - my $checkout_3 = AddIssue( $patron->unblessed, $item_3->barcode ); # not overdue + my $checkout_1 = AddIssue( $patron, $item_1->barcode, $five_days_ago ); # overdue + my $checkout_2 = AddIssue( $patron, $item_2->barcode, $five_days_ago ); # overdue + my $checkout_3 = AddIssue( $patron, $item_3->barcode ); # not overdue Koha::Patron::Debarments::AddUniqueDebarment( { diff --git a/t/db_dependent/Circulation/ReturnClaims.t b/t/db_dependent/Circulation/ReturnClaims.t index ee7b8c54e6..6fb1c8d176 100755 --- a/t/db_dependent/Circulation/ReturnClaims.t +++ b/t/db_dependent/Circulation/ReturnClaims.t @@ -65,7 +65,7 @@ subtest 'Test Koha::Checkout::claim_returned, do not mark as returned' => sub { t::lib::Mocks::mock_preference( 'MarkLostItemsAsReturned', q{} ); my $item = $builder->build_sample_item; my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); - my $checkout = AddIssue( $patron->unblessed, $item->barcode ); + my $checkout = AddIssue( $patron, $item->barcode ); my $claim = $checkout->claim_returned( { @@ -91,7 +91,7 @@ subtest 'Test Koha::Patronn::return_claims' => sub { t::lib::Mocks::mock_preference( 'ClaimReturnedLostValue', 1 ); my $item = $builder->build_sample_item; my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); - my $checkout = AddIssue( $patron->unblessed, $item->barcode ); + my $checkout = AddIssue( $patron, $item->barcode ); $checkout->claim_returned( { @@ -121,7 +121,7 @@ subtest 'Test Koha::Checkout::claim_returned, mark as returned' => sub { t::lib::Mocks::mock_preference( 'MarkLostItemsAsReturned', q{claim_returned} ); my $item = $builder->build_sample_item; my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); - my $checkout = AddIssue( $patron->unblessed, $item->barcode ); + my $checkout = AddIssue( $patron, $item->barcode ); my $claim = $checkout->claim_returned( { diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t index 7c20b11bac..db25fda549 100755 --- a/t/db_dependent/Circulation/Returns.t +++ b/t/db_dependent/Circulation/Returns.t @@ -86,10 +86,10 @@ subtest "AddReturn logging on statistics table (item-level_itypes=1)" => sub { # Create a branch $branch = $builder->build({ source => 'Branch' })->{ branchcode }; # Create a borrower - my $borrowernumber = $builder->build({ - source => 'Borrower', + my $patron = $builder->build_object({ + class => 'Koha::Patrons', value => { branchcode => $branch } - })->{ borrowernumber }; + }); # Look for the defined MARC field for biblio-level itemtype my $rs = $schema->resultset('MarcSubfieldStructure')->search({ frameworkcode => '', @@ -118,8 +118,7 @@ subtest "AddReturn logging on statistics table (item-level_itypes=1)" => sub { } )->_result->update({ itype => undef }); - my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; - AddIssue( $borrower, $item_with_itemtype->barcode ); + AddIssue( $patron, $item_with_itemtype->barcode ); AddReturn( $item_with_itemtype->barcode, $branch ); # Test item-level itemtype was recorded on the 'statistics' table my $stat = $schema->resultset('Statistic')->search({ @@ -130,7 +129,7 @@ subtest "AddReturn logging on statistics table (item-level_itypes=1)" => sub { is( $stat->itemtype, $ilevel_itemtype, "item-level itype recorded on statistics for return"); - warning_like { AddIssue( $borrower, $item_without_itemtype->barcode ) } + warning_like { AddIssue( $patron, $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/], @@ -166,8 +165,8 @@ subtest "AddReturn logging on statistics table (item-level_itypes=0)" => sub { # Create a branch $branch = $builder->build({ source => 'Branch' })->{ branchcode }; # Create a borrower - my $borrowernumber = $builder->build({ - source => 'Borrower', + my $patron = $builder->build_object({ + class => 'Koha::Patrons', value => { branchcode => $branch } })->{ borrowernumber }; # Look for the defined MARC field for biblio-level itemtype @@ -199,9 +198,7 @@ subtest "AddReturn logging on statistics table (item-level_itypes=0)" => sub { } ); - my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; - - AddIssue( $borrower, $item_with_itemtype->barcode ); + AddIssue( $patron, $item_with_itemtype->barcode ); AddReturn( $item_with_itemtype->barcode, $branch ); # Test item-level itemtype was recorded on the 'statistics' table my $stat = $schema->resultset('Statistic')->search({ @@ -213,7 +210,7 @@ subtest "AddReturn logging on statistics table (item-level_itypes=0)" => sub { is( $stat->itemtype, $blevel_itemtype, "biblio-level itype recorded on statistics for return"); - AddIssue( $borrower, $item_without_itemtype->barcode ); + AddIssue( $patron, $item_without_itemtype->barcode ); AddReturn( $item_without_itemtype->barcode, $branch ); # Test biblio-level itemtype was recorded on the 'statistics' table $stat = $schema->resultset('Statistic')->search({ @@ -251,10 +248,11 @@ subtest 'Handle ids duplication' => sub { itype => $itemtype->{itemtype}, } ); - my $patron = $builder->build({source => 'Borrower'}); - $patron = Koha::Patrons->find( $patron->{borrowernumber} ); + my $patron = $builder->build_object( + { class => 'Koha::Patrons'} + ); - my $original_checkout = AddIssue( $patron->unblessed, $item->barcode, dt_from_string->subtract( days => 50 ) ); + my $original_checkout = AddIssue( $patron, $item->barcode, dt_from_string->subtract( days => 50 ) ); my $issue_id = $original_checkout->issue_id; my $account_lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber, issue_id => $issue_id }); is( $account_lines->count, 1, '1 account line should exist for this issue_id' ); @@ -281,7 +279,7 @@ subtest 'Handle ids duplication' => sub { is( $messages->{WasReturned}, 0, 'messages should have the WasReturned flag set to 0' ); is( $messages->{DataCorrupted}, 1, 'messages should have the DataCorrupted flag set to 1' ); - $account_lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber, issue_id => $issue_id }); + $account_lines = Koha::Account::Lines->search({ borrowernumber => $borrower->{borrowernumber}, issue_id => $issue_id }); is( $account_lines->count, 0, 'No account lines should exist for this issue_id, patron should not have been charged' ); is( Koha::Checkouts->find( $issue_id )->issue_id, $issue_id, 'The issues entry should not have been removed' ); @@ -290,8 +288,8 @@ subtest 'Handle ids duplication' => sub { subtest 'BlockReturnOfLostItems' => sub { plan tests => 4; my $item = $builder->build_sample_item; - my $patron = $builder->build_object({class => 'Koha::Patrons'}); - my $checkout = AddIssue( $patron->unblessed, $item->barcode ); + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + my $checkout = AddIssue( $patron, $item->barcode ); # Mark the item as lost $item->itemlost(1)->store; @@ -315,7 +313,7 @@ subtest 'Checkin of an item claimed as returned should generate a message' => su t::lib::Mocks::mock_preference('ClaimReturnedLostValue', 1); my $item = $builder->build_sample_item; my $patron = $builder->build_object({class => 'Koha::Patrons'}); - my $checkout = AddIssue( $patron->unblessed, $item->barcode ); + my $checkout = AddIssue( $patron, $item->barcode ); $checkout->claim_returned({ created_by => $patron->id }); @@ -332,12 +330,12 @@ subtest 'BranchTransferLimitsType' => sub { my $item = $builder->build_sample_item; my $patron = $builder->build_object({class => 'Koha::Patrons'}); - my $checkout = AddIssue( $patron->unblessed, $item->barcode ); + my $checkout = AddIssue( $patron, $item->barcode ); my ( $doreturn, $messages, $issue ) = AddReturn($item->barcode); is( $doreturn, 1, 'AddReturn should have checkin the item if BranchTransferLimitsType=ccode'); t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype'); - $checkout = AddIssue( $patron->unblessed, $item->barcode ); + $checkout = AddIssue( $patron, $item->barcode ); ( $doreturn, $messages, $issue ) = AddReturn($item->barcode); is( $doreturn, 1, 'AddReturn should have checkin the item if BranchTransferLimitsType=itemtype'); }; @@ -351,7 +349,7 @@ subtest 'Backdated returns should reduce fine if needed' => sub { my $biblio = $builder->build_object( { class => 'Koha::Biblios' } ); my $item = $builder->build_sample_item; my $patron = $builder->build_object({class => 'Koha::Patrons'}); - my $checkout = AddIssue( $patron->unblessed, $item->barcode ); + my $checkout = AddIssue( $patron, $item->barcode ); my $fine = Koha::Account::Line->new({ issue_id => $checkout->id, borrowernumber => $patron->id, diff --git a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t index 1c8ea0f7d0..e0aafb3868 100755 --- a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t +++ b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t @@ -55,7 +55,6 @@ my $patron = $builder->build_object({ categorycode => $patron_category->{categorycode}, }, }); -my $patron_unblessed = $patron->unblessed; my $item = $builder->build_sample_item( { @@ -85,7 +84,7 @@ t::lib::Mocks::mock_userenv({ patron => $patron }); t::lib::Mocks::mock_preference('AllowTooManyOverride', 0); # Add onsite checkout -C4::Circulation::AddIssue( $patron_unblessed, $item->barcode, dt_from_string, undef, dt_from_string, undef, { onsite_checkout => 1 } ); +C4::Circulation::AddIssue( $patron, $item->barcode, dt_from_string, undef, dt_from_string, undef, { onsite_checkout => 1 } ); my ( $impossible, $messages ); t::lib::Mocks::mock_preference('SwitchOnSiteCheckouts', 0); @@ -96,7 +95,7 @@ t::lib::Mocks::mock_preference('SwitchOnSiteCheckouts', 1); ( $impossible, undef, undef, $messages ) = C4::Circulation::CanBookBeIssued( $patron, $item->barcode ); is( $messages->{ONSITE_CHECKOUT_WILL_BE_SWITCHED}, 1, 'If SwitchOnSiteCheckouts, switch the on-site checkout' ); is( exists $impossible->{TOO_MANY}, '', 'If SwitchOnSiteCheckouts, switch the on-site checkout' ); -C4::Circulation::AddIssue( $patron_unblessed, $item->barcode, undef, undef, undef, undef, { switch_onsite_checkout => 1 } ); +C4::Circulation::AddIssue( $patron, $item->barcode, undef, undef, undef, undef, { switch_onsite_checkout => 1 } ); my $issue = Koha::Checkouts->find( { itemnumber => $item->itemnumber } ); is( $issue->onsite_checkout, 0, 'The issue should have been switched to a regular checkout' ); my $five_days_after = dt_from_string->add( days => 5 )->set( hour => 23, minute => 59, second => 0 ); @@ -111,7 +110,7 @@ my $another_item = $builder->build_sample_item( } ); -C4::Circulation::AddIssue( $patron_unblessed, $another_item->barcode, dt_from_string, undef, dt_from_string, undef, { onsite_checkout => 1 } ); +C4::Circulation::AddIssue( $patron, $another_item->barcode, dt_from_string, undef, dt_from_string, undef, { onsite_checkout => 1 } ); ( $impossible, undef, undef, $messages ) = C4::Circulation::CanBookBeIssued( $patron, $another_item->barcode ); is( $messages->{ONSITE_CHECKOUT_WILL_BE_SWITCHED}, 1, 'Specific case 1 - Switch is allowed' ); is( exists $impossible->{TOO_MANY}, '', 'Specific case 1 - Switch is allowed' ); diff --git a/t/db_dependent/Circulation/TooMany.t b/t/db_dependent/Circulation/TooMany.t index c151411e51..46c6bd8b1a 100755 --- a/t/db_dependent/Circulation/TooMany.t +++ b/t/db_dependent/Circulation/TooMany.t @@ -39,7 +39,7 @@ our $dbh = C4::Context->dbh; $dbh->do(q|DELETE FROM issues|); $dbh->do(q|DELETE FROM items|); $dbh->do(q|DELETE FROM borrowers|); -$dbh->do(q|DELETE FROM branches|); +#$dbh->do(q|DELETE FROM branches|); $dbh->do(q|DELETE FROM categories|); $dbh->do(q|DELETE FROM accountlines|); $dbh->do(q|DELETE FROM itemtypes|); @@ -60,8 +60,8 @@ my $category = $builder->build({ source => 'Category', }); -my $patron = $builder->build({ - source => 'Borrower', +my $patron = $builder->build_object({ + class => 'Koha::Patrons', value => { categorycode => $category->{categorycode}, branchcode => $branch->{branchcode}, @@ -75,8 +75,7 @@ my $item = $builder->build_sample_item({ holdingbranch => $branch->{branchcode}, }); -my $patron_object = Koha::Patrons->find( $patron->{borrowernumber} ); -t::lib::Mocks::mock_userenv( { patron => $patron_object }); +t::lib::Mocks::mock_userenv( { patron => $patron }); # TooMany return ($current_loan_count, $max_loans_allowed) or undef # CO = Checkout @@ -486,8 +485,8 @@ subtest 'General vs specific rules limit quantity correctly' => sub { notforloan => 0, } }); - my $patron = $builder->build({ - source => 'Borrower', + my $patron = $builder->build_object({ + class => 'Koha::Patrons', value => { categorycode => $category->{categorycode}, branchcode => $branch->{branchcode}, @@ -777,9 +776,9 @@ subtest 'itemtype group tests' => sub { my $branch = $builder->build( { source => 'Branch', } ); my $category = $builder->build( { source => 'Category', } ); - my $patron = $builder->build( + my $patron = $builder->build_object( { - source => 'Borrower', + class => 'Koha::Patrons', value => { categorycode => $category->{categorycode}, branchcode => $branch->{branchcode}, diff --git a/t/db_dependent/Circulation/dateexpiry.t b/t/db_dependent/Circulation/dateexpiry.t index 8533794141..79ba9e0a02 100755 --- a/t/db_dependent/Circulation/dateexpiry.t +++ b/t/db_dependent/Circulation/dateexpiry.t @@ -91,25 +91,27 @@ sub calc_date_due { # this triggers the compare between expiry and due date - my $patron = $builder->build({ - source => 'Borrower', - value => { - categorycode => $patron_category->{categorycode}, + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $patron_category->{categorycode}, + } } - }); + ); my $item = $builder->build_sample_item; my $branch = $builder->build( { source => 'Branch' } ); my $today = dt_from_string(); # first test with empty expiry date # note that this expiry date will never lead to an issue btw !! - $patron->{dateexpiry} = undef; + $patron->dateexpiry(undef)->store; my $d = C4::Circulation::CalcDateDue( $today, $item->effective_itemtype, $branch->{branchcode}, $patron ); is( ref $d eq "DateTime" && $d->mdy() =~ /^\d+/, 1, "CalcDateDue with expiry undef" ); # second test expiry date==today my $d2 = output_pref( { dt => $today, dateonly => 1, dateformat => 'sql' } ); - $patron->{dateexpiry} = $d2; + $patron->dateexpiry($d2)->store; $d = C4::Circulation::CalcDateDue( $today, $item->effective_itemtype, $branch->{branchcode}, $patron ); is( ref $d eq "DateTime" && DateTime->compare( $d->truncate( to => 'day' ), $today->truncate( to => 'day' ) ) == 0, 1, "CalcDateDue with expiry today" ); @@ -117,12 +119,12 @@ sub calc_date_due { my $dur = DateTime::Duration->new( days => 1 ); my $tomorrow = $today->clone->add_duration($dur); $d2 = output_pref( { dt => $tomorrow, dateonly => 1, dateformat => 'sql' } ); - $patron->{dateexpiry} = $d2; + $patron->dateexpiry($d2)->store; $d = C4::Circulation::CalcDateDue( $today, $item->effective_itemtype, $branch->{branchcode}, $patron ); is( ref $d eq "DateTime" && $d->mdy() =~ /^\d+/, 1, "CalcDateDue with expiry tomorrow" ); # fourth test far future - $patron->{dateexpiry} = '9876-12-31'; + $patron->dateexpiry('9876-12-31')->store; my $t1 = time; $d = C4::Circulation::CalcDateDue( $today, $item->effective_itemtype, $branch->{branchcode}, $patron ); my $t2 = time; diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t index f388dab007..f9e36da40a 100755 --- a/t/db_dependent/Circulation/issue.t +++ b/t/db_dependent/Circulation/issue.t @@ -160,7 +160,6 @@ my $borrower_id1 = Koha::Patron->new({ branchcode => $branchcode_1 })->store->borrowernumber; my $patron_1 = Koha::Patrons->find( $borrower_id1 ); -my $borrower_1 = $patron_1->unblessed; my $borrower_id2 = Koha::Patron->new({ firstname => 'firstname2', surname => 'surname2 ', @@ -168,7 +167,6 @@ my $borrower_id2 = Koha::Patron->new({ branchcode => $branchcode_2, })->store->borrowernumber; my $patron_2 = Koha::Patrons->find( $borrower_id2 ); -my $borrower_2 = $patron_2->unblessed; t::lib::Mocks::mock_userenv({ patron => $patron_1 }); @@ -178,9 +176,9 @@ t::lib::Mocks::mock_userenv({ patron => $patron_1 }); my $query = " SELECT count(*) FROM issues"; my $sth = $dbh->prepare($query); $sth->execute; -my $countissue = $sth -> fetchrow_array; +my $countissue = $sth->fetchrow_array; is ($countissue ,0, "there is no issue"); -my $issue1 = C4::Circulation::AddIssue( $borrower_1, $barcode_1, $daysago10,0, $today, '' ); +my $issue1 = C4::Circulation::AddIssue( $patron_1, $barcode_1, $daysago10, 0, $today, '' ); is( ref $issue1, 'Koha::Checkout', 'AddIssue returns a Koha::Checkout object' ); my $datedue1 = dt_from_string( $issue1->date_due() ); @@ -191,11 +189,11 @@ like( ); my $issue_id1 = $issue1->issue_id; -my $issue2 = C4::Circulation::AddIssue( $borrower_1, 'nonexistent_barcode' ); +my $issue2 = C4::Circulation::AddIssue( $patron_1, 'nonexistent_barcode' ); is( $issue2, undef, "AddIssue returns undef if no datedue is specified" ); $sth->execute; -$countissue = $sth -> fetchrow_array; +$countissue = $sth->fetchrow_array; is ($countissue,1,"1 issues have been added"); #Test AddIssuingCharge @@ -263,7 +261,7 @@ subtest 'Show that AddRenewal respects OpacRenewalBranch and interface' => sub { my $item = $builder->build_sample_item( { library => $item_library->branchcode, itype => $itemtype } ); my $opac_renew_issue = - C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + C4::Circulation::AddIssue( $patron, $item->barcode ); AddRenewal( $patron->borrowernumber, $item->itemnumber, "Stavromula", $datedue1, $daysago10 ); @@ -281,7 +279,7 @@ subtest 'Show that AddRenewal respects OpacRenewalBranch and interface' => sub { my $item = $builder->build_sample_item( { library => $item_library->branchcode, itype => $itemtype } ); my $opac_renew_issue = - C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + C4::Circulation::AddIssue( $patron, $item->barcode ); AddRenewal( $patron->borrowernumber, $item->itemnumber, "Stavromula", $datedue1, $daysago10 ); @@ -298,7 +296,7 @@ subtest 'Show that AddRenewal respects OpacRenewalBranch and interface' => sub { my @renewcount; #Test GetRenewCount -my $issue3 = C4::Circulation::AddIssue( $borrower_1, $barcode_1 ); +my $issue3 = C4::Circulation::AddIssue( $patron_1, $barcode_1 ); #Without anything in DB @renewcount = C4::Circulation::GetRenewCount(); is_deeply( @@ -372,7 +370,7 @@ my $return = $dbh->selectrow_hashref("SELECT DATE(returndate) AS return_date, CU ok( $return->{return_date} eq $return->{today}, "Item returned with no return date specified has todays date" ); $dbh->do("DELETE FROM old_issues"); -C4::Circulation::AddIssue( $borrower_1, $barcode_1, $daysago10, 0, $today ); +C4::Circulation::AddIssue( $patron_1, $barcode_1, $daysago10, 0, $today ); AddReturn($barcode_1, undef, undef, dt_from_string('2014-04-01 23:42')); $return = $dbh->selectrow_hashref("SELECT * FROM old_issues LIMIT 1" ); ok( $return->{returndate} eq '2014-04-01 23:42:00', "Item returned with a return date of '2014-04-01 23:42' has that return date" ); @@ -458,7 +456,7 @@ $item2 = Koha::Items->find( $itemnumber2 ); ok( $item2->location eq 'CART', q{UpdateItemLocationOnCheckin updates location value from 'GEN' with setting "_ALL_: CART" when transfer filled} ); ok( $item2->permanent_location eq 'GEN', q{UpdateItemLocationOnCheckin does not update permanent_location value from 'GEN' with setting "_ALL_: CART"} ); -AddIssue( $borrower_1, 'barcode_4', $daysago10,0, $today, '' ); +AddIssue( $patron_1, 'barcode_4', $daysago10,0, $today, '' ); $item2 = Koha::Items->find( $itemnumber2 ); ok( $item2->location eq 'GEN', q{Location updates from 'CART' to permanent location on issue} ); @@ -507,7 +505,7 @@ my $reserve_id = AddReserve( } ); ok( $reserve_id, 'The reserve should have been inserted' ); -AddIssue( $borrower_2, $barcode_1, dt_from_string, 'cancel' ); +AddIssue( $patron_2, $barcode_1, dt_from_string, 'cancel' ); my $hold = Koha::Holds->find( $reserve_id ); is( $hold, undef, 'The reserve should have been correctly cancelled' ); @@ -530,7 +528,7 @@ my $unseen_library = $builder->build_object( { class => 'Koha::Libraries' } ); my $unseen_patron = $builder->build_object( { class => 'Koha::Patrons' } ); my $unseen_item = $builder->build_sample_item( { library => $unseen_library->branchcode, itype => $itemtype } ); -my $unseen_issue = C4::Circulation::AddIssue( $unseen_patron->unblessed, $unseen_item->barcode ); +my $unseen_issue = C4::Circulation::AddIssue( $unseen_patron, $unseen_item->barcode ); # Does an unseen renewal increment the issue's count my ( $unseen_before ) = ( C4::Circulation::GetRenewCount( $unseen_patron->borrowernumber, $unseen_item->itemnumber ) )[3]; diff --git a/t/db_dependent/Circulation/maxsuspensiondays.t b/t/db_dependent/Circulation/maxsuspensiondays.t index 0660b27415..57a03f13d2 100755 --- a/t/db_dependent/Circulation/maxsuspensiondays.t +++ b/t/db_dependent/Circulation/maxsuspensiondays.t @@ -42,13 +42,12 @@ Koha::CirculationRules->set_rules( } ); -my $borrowernumber = Koha::Patron->new({ +my $patron = Koha::Patron->new({ firstname => 'my firstname', surname => 'my surname', categorycode => $patron_category->{categorycode}, branchcode => $branchcode, -})->store->borrowernumber; -my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; +})->store; my $record = MARC::Record->new(); $record->append_fields( @@ -74,9 +73,9 @@ $dbh->do('DELETE FROM repeatable_holidays'); my $daysago20 = dt_from_string->add_duration(DateTime::Duration->new(days => -20)); my $daysafter40 = dt_from_string->add_duration(DateTime::Duration->new(days => 40)); -AddIssue( $borrower, $barcode, $daysago20 ); +AddIssue( $patron, $barcode, $daysago20 ); AddReturn( $barcode, $branchcode ); -my $debarments = GetDebarments({borrowernumber => $borrower->{borrowernumber}}); +my $debarments = GetDebarments({borrowernumber => $patron->borrowernumber}); is( $debarments->[0]->{expiration}, output_pref({ dt => $daysafter40, dateformat => 'iso', dateonly => 1 }), @@ -97,9 +96,9 @@ Koha::CirculationRules->set_rules( ); my $daysafter10 = dt_from_string->add_duration(DateTime::Duration->new(days => 10)); -AddIssue( $borrower, $barcode, $daysago20 ); +AddIssue( $patron, $barcode, $daysago20 ); AddReturn( $barcode, $branchcode ); -$debarments = GetDebarments({borrowernumber => $borrower->{borrowernumber}}); +$debarments = GetDebarments({borrowernumber => $patron->borrowernumber}); is( $debarments->[0]->{expiration}, output_pref({ dt => $daysafter10, dateformat => 'iso', dateonly => 1 }), @@ -127,7 +126,7 @@ subtest "suspension_chargeperiod" => sub { my $last_year = dt_from_string->clone->subtract( years => 1 ); my $today = dt_from_string; - my $new_debar_dt = C4::Circulation::_calculate_new_debar_dt( $patron->unblessed, $item->unblessed, $last_year, $today ); + my $new_debar_dt = C4::Circulation::_calculate_new_debar_dt( $patron, $item, $last_year, $today ); is( $new_debar_dt->truncate( to => 'day' ), $today->clone->add( days => 365 / 15 * 7 )->truncate( to => 'day' ) ); @@ -153,7 +152,7 @@ subtest "maxsuspensiondays" => sub { my $last_year = dt_from_string->clone->subtract( years => 1 ); my $today = dt_from_string; - my $new_debar_dt = C4::Circulation::_calculate_new_debar_dt( $patron->unblessed, $item->unblessed, $last_year, $today ); + my $new_debar_dt = C4::Circulation::_calculate_new_debar_dt( $patron, $item, $last_year, $today ); is( $new_debar_dt->truncate( to => 'day' ), $today->clone->add( days => 333 )->truncate( to => 'day' ) ); }; diff --git a/t/db_dependent/Circulation/transferbook.t b/t/db_dependent/Circulation/transferbook.t index c94e32dc4d..78ca0b34c8 100755 --- a/t/db_dependent/Circulation/transferbook.t +++ b/t/db_dependent/Circulation/transferbook.t @@ -208,7 +208,7 @@ subtest 'transfer an issued item' => sub { ); my $dt_to = dt_from_string(); - my $issue = AddIssue( $patron->unblessed, $item->barcode, $dt_to ); + my $issue = AddIssue( $patron, $item->barcode, $dt_to ); # We are making sure there is no regression, feel free to change the behavior if needed. # * WasReturned does not seem like a variable that should contain a borrowernumber @@ -222,7 +222,7 @@ subtest 'transfer an issued item' => sub { is( $messages->{WasReturned}, $patron->borrowernumber, 'transferbook should have return a WasReturned flag is the item was issued before the transferbook call'); # Reset issue - $issue = AddIssue( $patron->unblessed, $item->barcode, $dt_to ); + $issue = AddIssue( $patron, $item->barcode, $dt_to ); # We are making sure there is no regression, feel free to change the behavior if needed. # * Contrary to the POD, if ignore_reserves is not passed (or is false), any item reserve diff --git a/t/db_dependent/Circulation_holdsqueue.t b/t/db_dependent/Circulation_holdsqueue.t index 61d8294076..44dcc00d5a 100755 --- a/t/db_dependent/Circulation_holdsqueue.t +++ b/t/db_dependent/Circulation_holdsqueue.t @@ -58,7 +58,7 @@ subtest 'AddIssue() and AddReturn() real-time holds queue tests' => sub { } ); $action = 'AddIssue'; - AddIssue( $patron->unblessed, $item->barcode, ); + AddIssue( $patron, $item->barcode, ); $action = 'AddReturn'; AddReturn( $item->barcode ); @@ -66,7 +66,7 @@ subtest 'AddIssue() and AddReturn() real-time holds queue tests' => sub { t::lib::Mocks::mock_preference( 'RealTimeHoldsQueue', 0 ); $action = 'AddIssue'; - AddIssue( $patron->unblessed, $item->barcode, ); + AddIssue( $patron, $item->barcode, ); $action = 'AddReturn'; AddReturn( $item->barcode ); diff --git a/t/db_dependent/DecreaseLoanHighHolds.t b/t/db_dependent/DecreaseLoanHighHolds.t index ed93a21c33..2d42d28b9b 100755 --- a/t/db_dependent/DecreaseLoanHighHolds.t +++ b/t/db_dependent/DecreaseLoanHighHolds.t @@ -118,7 +118,7 @@ my $orig_due = C4::Circulation::CalcDateDue( dt_from_string(), $item->effective_itemtype, $patron->branchcode, - $patron->unblessed + $patron ); t::lib::Mocks::mock_preference( 'decreaseLoanHighHolds', 1 ); diff --git a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t index 18031c15d0..b579f7d50b 100755 --- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t +++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t @@ -104,7 +104,7 @@ is( $is, 1, "Items availability: both of 2 items are available" ); $is = IsAvailableForItemLevelRequest( $item1, $patron1 ); is( $is, 0, "Item cannot be held, 2 items available" ); -my $issue1 = AddIssue( $patron2->unblessed, $item1->barcode ); +my $issue1 = AddIssue( $patron2, $item1->barcode ); $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblionumber, patron => $patron1 } ); is( $is, 1, "Items availability: one item is available" ); @@ -112,7 +112,7 @@ is( $is, 1, "Items availability: one item is available" ); $is = IsAvailableForItemLevelRequest( $item1, $patron1 ); is( $is, 0, "Item cannot be held, 1 item available" ); -AddIssue( $patron2->unblessed, $item2->barcode ); +AddIssue( $patron2, $item2->barcode ); $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblionumber, patron => $patron1 } ); is( $is, 0, "Items availability: none of items are available" ); @@ -379,7 +379,7 @@ subtest 'Check holds availability with different item types' => sub { $is = IsAvailableForItemLevelRequest( $item5, $patron1 ); is( $is, 0, "Item5 cannot be requested to hold: 2 items, Item4 available, Item5 restricted" ); - AddIssue( $patron2->unblessed, $item4->barcode ); + AddIssue( $patron2, $item4->barcode ); $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblio2->biblionumber, patron => $patron1 } ); is( $is, 0, "Items availability: 2 items, one allowed by smart rule and checked out, another one not allowed to be held by smart rule" ); diff --git a/t/db_dependent/Holds/RevertWaitingStatus.t b/t/db_dependent/Holds/RevertWaitingStatus.t index 4dab0f3b70..75b1290d34 100755 --- a/t/db_dependent/Holds/RevertWaitingStatus.t +++ b/t/db_dependent/Holds/RevertWaitingStatus.t @@ -82,9 +82,8 @@ foreach my $borrowernumber (@borrowernumbers) { } ModReserveAffect( $itemnumber, $borrowernumbers[0] ); -my $patron = Koha::Patrons->find( $borrowernumbers[1] )->unblessed; -C4::Circulation::AddIssue( $patron, - $item_barcode, my $datedue, my $cancelreserve = 'revert' ); +my $patron = Koha::Patrons->find( $borrowernumbers[1] ); +C4::Circulation::AddIssue( $patron, $item_barcode, undef, 'revert' ); my $priorities = $dbh->selectall_arrayref( "SELECT priority FROM reserves ORDER BY priority ASC"); diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t index 65120df27a..2ecda5500c 100755 --- a/t/db_dependent/ILSDI_Services.t +++ b/t/db_dependent/ILSDI_Services.t @@ -325,9 +325,7 @@ subtest 'Holds test' => sub { t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 0 ); - my $patron = $builder->build({ - source => 'Borrower', - }); + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); my $item = $builder->build_sample_item( { @@ -336,7 +334,7 @@ subtest 'Holds test' => sub { ); my $query = CGI->new; - $query->param( 'patron_id', $patron->{borrowernumber}); + $query->param( 'patron_id', $patron->borrowernumber); $query->param( 'bib_id', $item->biblionumber); my $reply = C4::ILSDI::Services::HoldTitle( $query ); @@ -347,7 +345,7 @@ subtest 'Holds test' => sub { my $hold = $builder->build({ source => 'Reserve', value => { - borrowernumber => $patron->{borrowernumber}, + borrowernumber => $patron->borrowernumber, biblionumber => $item->biblionumber, itemnumber => $item->itemnumber } @@ -359,7 +357,7 @@ subtest 'Holds test' => sub { my $biblio_with_no_item = $builder->build_sample_biblio; $query = CGI->new; - $query->param( 'patron_id', $patron->{borrowernumber}); + $query->param( 'patron_id', $patron->borrowernumber); $query->param( 'bib_id', $biblio_with_no_item->biblionumber); $reply = C4::ILSDI::Services::HoldTitle( $query ); @@ -374,16 +372,16 @@ subtest 'Holds test' => sub { t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' ); Koha::CirculationRules->set_rule( { - categorycode => $patron->{categorycode}, + categorycode => $patron->categorycode, itemtype => $item2->{itype}, - branchcode => $patron->{branchcode}, + branchcode => $patron->branchcode, rule_name => 'reservesallowed', rule_value => 1, } ); $query = CGI->new; - $query->param( 'patron_id', $patron->{borrowernumber}); + $query->param( 'patron_id', $patron->borrowernumber); $query->param( 'bib_id', $item2->biblionumber); $query->param( 'item_id', $item2->itemnumber); @@ -392,16 +390,16 @@ subtest 'Holds test' => sub { Koha::CirculationRules->set_rule( { - categorycode => $patron->{categorycode}, + categorycode => $patron->categorycode, itemtype => $item2->{itype}, - branchcode => $patron->{branchcode}, + branchcode => $patron->branchcode, rule_name => 'reservesallowed', rule_value => 0, } ); $query = CGI->new; - $query->param( 'patron_id', $patron->{borrowernumber}); + $query->param( 'patron_id', $patron->borrowernumber); $query->param( 'bib_id', $item2->biblionumber); $query->param( 'item_id', $item2->itemnumber); @@ -433,16 +431,16 @@ subtest 'Holds test' => sub { Koha::CirculationRules->set_rule( { - categorycode => $patron->{categorycode}, + categorycode => $patron->categorycode, itemtype => $item3->{itype}, - branchcode => $patron->{branchcode}, + branchcode => $patron->branchcode, rule_name => 'reservesallowed', rule_value => 10, } ); $query = CGI->new; - $query->param( 'patron_id', $patron->{borrowernumber}); + $query->param( 'patron_id', $patron->borrowernumber); $query->param( 'bib_id', $item4->biblionumber); $query->param( 'item_id', $item4->itemnumber); @@ -450,12 +448,12 @@ subtest 'Holds test' => sub { is( $reply->{code}, 'damaged', "Item is damaged" ); my $module = Test::MockModule->new('C4::Context'); - $module->mock('userenv', sub { { patron => $patron } }); + $module->mock('userenv', sub { { patron => $patron->unblessed } }); my $issue = C4::Circulation::AddIssue($patron, $item3->barcode); t::lib::Mocks::mock_preference( 'AllowHoldsOnPatronsPossessions', '0' ); $query = CGI->new; - $query->param( 'patron_id', $patron->{borrowernumber}); + $query->param( 'patron_id', $patron->borrowernumber); $query->param( 'bib_id', $item3->biblionumber); $query->param( 'item_id', $item3->itemnumber); $query->param( 'pickup_location', $origin_branch->{branchcode}); @@ -511,8 +509,8 @@ subtest 'Holds test for branch transfer limits' => sub { t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); - my $patron = $builder->build({ - source => 'Borrower', + my $patron = $builder->build_object({ + class => 'Koha::Patrons', }); my $origin_branch = $builder->build( @@ -556,7 +554,7 @@ subtest 'Holds test for branch transfer limits' => sub { my $query = CGI->new; $query->param( 'pickup_location', $pickup_branch->{branchcode} ); - $query->param( 'patron_id', $patron->{borrowernumber}); + $query->param( 'patron_id', $patron->borrowernumber); $query->param( 'bib_id', $item->biblionumber); $query->param( 'item_id', $item->itemnumber); @@ -570,14 +568,14 @@ subtest 'Holds test for branch transfer limits' => sub { $reply = C4::ILSDI::Services::HoldItem( $query ); is( $reply->{code}, undef, "Item hold, Item can be transferred" ); - my $hold = Koha::Holds->search({ itemnumber => $item->itemnumber, borrowernumber => $patron->{borrowernumber} })->next; + my $hold = Koha::Holds->search({ itemnumber => $item->itemnumber, borrowernumber => $patron->borrowernumber })->next; is( $hold->branchcode, $pickup_branch->{branchcode}, 'The library id is correctly set' ); Koha::Holds->search()->delete(); $reply = C4::ILSDI::Services::HoldTitle( $query ); is( $reply->{code}, undef, "Record hold, Item con be transferred" ); - $hold = Koha::Holds->search({ biblionumber => $item->biblionumber, borrowernumber => $patron->{borrowernumber} })->next; + $hold = Koha::Holds->search({ biblionumber => $item->biblionumber, borrowernumber => $patron->borrowernumber })->next; is( $hold->branchcode, $pickup_branch->{branchcode}, 'The library id is correctly set' ); $schema->storage->txn_rollback; @@ -723,7 +721,7 @@ subtest 'RenewHold' => sub { $cgi->param( item_id => $item->itemnumber ); t::lib::Mocks::mock_userenv( { patron => $patron } ); # For AddIssue - my $checkout = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + my $checkout = C4::Circulation::AddIssue( $patron, $item->barcode ); # Everything is ok my $reply = C4::ILSDI::Services::RenewLoan($cgi); @@ -826,11 +824,11 @@ subtest 'GetPatronInfo paginated loans' => sub { my $module = Test::MockModule->new('C4::Context'); $module->mock('userenv', sub { { branch => $library->branchcode } }); my $date_due = Koha::DateUtils::dt_from_string()->add(weeks => 2); - my $issue1 = C4::Circulation::AddIssue($patron->unblessed, $item1->barcode, $date_due); + my $issue1 = C4::Circulation::AddIssue($patron, $item1->barcode, $date_due); my $date_due1 = Koha::DateUtils::dt_from_string( $issue1->date_due ); - my $issue2 = C4::Circulation::AddIssue($patron->unblessed, $item2->barcode, $date_due); + my $issue2 = C4::Circulation::AddIssue($patron, $item2->barcode, $date_due); my $date_due2 = Koha::DateUtils::dt_from_string( $issue2->date_due ); - my $issue3 = C4::Circulation::AddIssue($patron->unblessed, $item3->barcode, $date_due); + my $issue3 = C4::Circulation::AddIssue($patron, $item3->barcode, $date_due); my $date_due3 = Koha::DateUtils::dt_from_string( $issue3->date_due ); my $cgi = CGI->new; diff --git a/t/db_dependent/Illrequests.t b/t/db_dependent/Illrequests.t index f33181ce1a..428e345f3d 100755 --- a/t/db_dependent/Illrequests.t +++ b/t/db_dependent/Illrequests.t @@ -1602,7 +1602,7 @@ subtest 'Checking in hook' => sub { t::lib::Mocks::mock_preference('CirculateILL', 1); # Add an issue - AddIssue( $patron->unblessed, $item->barcode ); + AddIssue( $patron, $item->barcode ); # Make the item withdrawn so checking-in is rejected t::lib::Mocks::mock_preference('BlockReturnOfWithdrawnItems', 1); $item->set({ withdrawn => 1 })->store; diff --git a/t/db_dependent/Koha/Account/Line.t b/t/db_dependent/Koha/Account/Line.t index 436a22d988..8b70e46604 100755 --- a/t/db_dependent/Koha/Account/Line.t +++ b/t/db_dependent/Koha/Account/Line.t @@ -518,6 +518,7 @@ subtest 'Renewal related tests' => sub { } } ); + my $line = Koha::Account::Line->new( { borrowernumber => $patron->borrowernumber, @@ -702,7 +703,7 @@ subtest 'checkout() tests' => sub { my $account = $patron->account; t::lib::Mocks::mock_userenv({ branchcode => $library->branchcode }); - my $checkout = AddIssue( $patron->unblessed, $item->barcode ); + my $checkout = AddIssue( $patron, $item->barcode ); my $line = $account->add_debit({ amount => 10, @@ -815,14 +816,21 @@ subtest "void() tests" => sub { my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; - my $borrower = Koha::Patron->new( { - cardnumber => 'dariahall', - surname => 'Hall', - firstname => 'Daria', - } ); - $borrower->categorycode( $categorycode ); - $borrower->branchcode( $branchcode ); - $borrower->store; + my $staff = $builder->build_object({ class => 'Koha::Patrons' }); + t::lib::Mocks::mock_userenv({ patron => $staff }); + + my $borrower = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + cardnumber => 'dariahall', + surname => 'Hall', + firstname => 'Daria', + categorycode => $categorycode, + branchcode => $branchcode, + } + } + ); my $account = Koha::Account->new({ patron_id => $borrower->id }); diff --git a/t/db_dependent/Koha/Biblio.t b/t/db_dependent/Koha/Biblio.t index ac9fcd0466..b27cb56a32 100755 --- a/t/db_dependent/Koha/Biblio.t +++ b/t/db_dependent/Koha/Biblio.t @@ -857,8 +857,8 @@ subtest 'current_checkouts() and old_checkouts() tests' => sub { my $library = $builder->build_object({ class => 'Koha::Libraries' }); - my $patron_1 = $builder->build_object({ class => 'Koha::Patrons' })->unblessed; - my $patron_2 = $builder->build_object({ class => 'Koha::Patrons' })->unblessed; + my $patron_1 = $builder->build_object({ class => 'Koha::Patrons' }); + my $patron_2 = $builder->build_object({ class => 'Koha::Patrons' }); my $item_1 = $builder->build_sample_item; my $item_2 = $builder->build_sample_item({ biblionumber => $item_1->biblionumber }); @@ -997,7 +997,7 @@ subtest 'Recalls tests' => sub { is( $biblio->can_be_recalled({ patron => $patron1 }), 0, "Can't recall if patron has more existing recall(s) than recalls_per_record" ); $recall1->set_cancelled; - C4::Circulation::AddIssue( $patron1->unblessed, $item2->barcode ); + C4::Circulation::AddIssue( $patron1, $item2->barcode ); is( $biblio->can_be_recalled({ patron => $patron1 }), 0, "Can't recall if patron has already checked out an item attached to this biblio" ); is( $biblio->can_be_recalled({ patron => $patron1 }), 0, "Can't recall if on_shelf_recalls = all and items are still available" ); @@ -1016,8 +1016,8 @@ subtest 'Recalls tests' => sub { is( $biblio->can_be_recalled({ patron => $patron1 }), 0, "Can't recall if no items are checked out" ); $recall2->set_cancelled; - C4::Circulation::AddIssue( $patron2->unblessed, $item2->barcode ); - C4::Circulation::AddIssue( $patron2->unblessed, $item1->barcode ); + C4::Circulation::AddIssue( $patron2, $item2->barcode ); + C4::Circulation::AddIssue( $patron2, $item1->barcode ); is( $biblio->can_be_recalled({ patron => $patron1 }), 2, "Can recall two items" ); $item1->update({ withdrawn => 1 }); diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t index 0c424f98db..234fa8db96 100755 --- a/t/db_dependent/Koha/Items.t +++ b/t/db_dependent/Koha/Items.t @@ -298,7 +298,7 @@ subtest 'store' => sub { } ); - C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + C4::Circulation::AddIssue( $patron, $item->barcode ); # Simulate item marked as lost $item->itemlost(3)->store; @@ -382,7 +382,7 @@ subtest 'store' => sub { ); my $issue = - C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + C4::Circulation::AddIssue( $patron, $item->barcode ); # Simulate item marked as lost $item->itemlost(1)->store; @@ -497,7 +497,7 @@ subtest 'store' => sub { ); my $issue = - C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + C4::Circulation::AddIssue( $patron, $item->barcode ); # Simulate item marked as lost $item->itemlost(3)->store; @@ -603,7 +603,7 @@ subtest 'store' => sub { ); my $issue = - C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + C4::Circulation::AddIssue( $patron, $item->barcode ); # Simulate item marked as lost $item->itemlost(1)->store; @@ -810,7 +810,7 @@ subtest 'store' => sub { )->store; my $issue = - C4::Circulation::AddIssue( $patron->unblessed, $barcode ); + C4::Circulation::AddIssue( $patron, $barcode ); # Simulate item marked as lost $item->itemlost(1)->store; @@ -934,7 +934,7 @@ subtest 'store' => sub { )->store; my $issue = - C4::Circulation::AddIssue( $patron->unblessed, $barcode ); + C4::Circulation::AddIssue( $patron, $barcode ); # Simulate item marked as lost $item->itemlost(1)->store; @@ -984,7 +984,7 @@ subtest 'store' => sub { ); my $issue = - C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + C4::Circulation::AddIssue( $patron, $item->barcode ); # Simulate item marked as lost $item->itemlost(1)->store; @@ -1075,7 +1075,7 @@ subtest 'store' => sub { ); my $issue = - C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + C4::Circulation::AddIssue( $patron, $item->barcode ); # Simulate item marked as lost $item->itemlost(1)->store; @@ -1185,7 +1185,7 @@ subtest 'store' => sub { ); my $issue = - C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + C4::Circulation::AddIssue( $patron, $item->barcode ); # Simulate item marked as lost $item->itemlost(1)->store; @@ -1309,7 +1309,7 @@ subtest 'store' => sub { ); my $issue = - C4::Circulation::AddIssue( $patron->unblessed, $barcode ); + C4::Circulation::AddIssue( $patron, $barcode ); # Simulate item marked as lost $item->itemlost(1)->store; diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t index 64a48c192a..2e20df37db 100755 --- a/t/db_dependent/Koha/Object.t +++ b/t/db_dependent/Koha/Object.t @@ -974,7 +974,6 @@ subtest 'unblessed_all_relateds' => sub { 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', @@ -984,12 +983,12 @@ subtest 'unblessed_all_relateds' => sub { biblionumber => $biblio->biblionumber, itemlost => 0, withdrawn => 0, - itype => $itemtype + itype => $itemtype } } ); - my $issue = AddIssue( $patron->unblessed, $item->barcode, DateTime->now->subtract( days => 1 ) ); + my $issue = AddIssue( $patron, $item->barcode, DateTime->now->subtract( days => 1 ) ); my $overdues = Koha::Patrons->find( $patron->id )->overdues; # Koha::Patron->overdues prefetches my $overdue = $overdues->next->unblessed_all_relateds; is( $overdue->{issue_id}, $issue->issue_id, 'unblessed_all_relateds has field from the original table (issues)' ); diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index b1adccde3d..01dd8a3872 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -658,16 +658,12 @@ subtest 'checkouts + pending_checkouts + overdues + old_checkouts' => sub { is( $old_checkouts->count, 0, 'old_checkouts should not return any issues for that patron' ); is( ref($old_checkouts), 'Koha::Old::Checkouts', 'old_checkouts should return a Koha::Old::Checkouts object' ); - # Not sure how this is useful, but AddIssue pass this variable to different other subroutines - $patron = Koha::Patrons->find( $patron->borrowernumber )->unblessed; - t::lib::Mocks::mock_userenv({ branchcode => $library->{branchcode} }); AddIssue( $patron, $item_1->barcode, DateTime->now->subtract( days => 1 ) ); AddIssue( $patron, $item_2->barcode, DateTime->now->subtract( days => 5 ) ); AddIssue( $patron, $item_3->barcode ); - $patron = Koha::Patrons->find( $patron->{borrowernumber} ); $checkouts = $patron->checkouts; is( $checkouts->count, 3, 'checkouts should return 3 issues for that patron' ); is( ref($checkouts), 'Koha::Checkouts', 'checkouts should return a Koha::Checkouts object' ); @@ -1509,11 +1505,12 @@ subtest 'overdues' => sub { t::lib::Mocks::mock_preference({ branchcode => $library->{branchcode} }); + $patron = Koha::Patrons->find( $patron->{borrowernumber} ); + AddIssue( $patron, $item_1->barcode, DateTime->now->subtract( days => 1 ) ); AddIssue( $patron, $item_2->barcode, DateTime->now->subtract( days => 5 ) ); AddIssue( $patron, $item_3->barcode ); - $patron = Koha::Patrons->find( $patron->{borrowernumber} ); my $overdues = $patron->overdues; is( $overdues->count, 2, 'Patron should have 2 overdues'); is( $overdues->next->itemnumber, $item_1->itemnumber, 'The issue should be returned in the same order as they have been done, first is correct' ); diff --git a/t/db_dependent/Koha/Plugins/Circulation_hooks.t b/t/db_dependent/Koha/Plugins/Circulation_hooks.t index e667f17971..65c60b9cf5 100755 --- a/t/db_dependent/Koha/Plugins/Circulation_hooks.t +++ b/t/db_dependent/Koha/Plugins/Circulation_hooks.t @@ -75,11 +75,11 @@ subtest 'after_circ_action() hook tests' => sub { subtest 'AddIssue' => sub { plan tests => 2; - warning_like { AddIssue( $patron->unblessed, $item_1->barcode ); } + warning_like { AddIssue( $patron, $item_1->barcode ); } qr/after_circ_action called with action: checkout, ref: Koha::Checkout type: issue/, 'AddIssue calls the after_circ_action hook'; - warning_like { AddIssue( $patron->unblessed, $item_2->barcode, undef, undef, undef, undef, { onsite_checkout => 1 } ); } + warning_like { AddIssue( $patron, $item_2->barcode, undef, undef, undef, undef, { onsite_checkout => 1 } ); } qr/after_circ_action called with action: checkout, ref: Koha::Checkout type: onsite_checkout/, 'AddIssue calls the after_circ_action hook (onsite_checkout case)'; }; diff --git a/t/db_dependent/Koha/Pseudonymization.t b/t/db_dependent/Koha/Pseudonymization.t index 9ee23ceaa3..93cea32f21 100755 --- a/t/db_dependent/Koha/Pseudonymization.t +++ b/t/db_dependent/Koha/Pseudonymization.t @@ -84,7 +84,7 @@ subtest 'Koha::Anonymized::Transactions tests' => sub { t::lib::Mocks::mock_preference( 'Pseudonymization', 0 ); my $item = $builder->build_sample_item; t::lib::Mocks::mock_userenv({ branchcode => $item->homebranch }); - AddIssue( $patron->unblessed, $item->barcode, dt_from_string ); + AddIssue( $patron, $item->barcode, dt_from_string ); AddReturn( $item->barcode, $item->homebranch, undef, dt_from_string ); my $pseudonymized= Koha::PseudonymizedTransactions->search( { itemnumber => $item->itemnumber } )->next; @@ -96,7 +96,7 @@ subtest 'Koha::Anonymized::Transactions tests' => sub { ); $item = $builder->build_sample_item; t::lib::Mocks::mock_userenv({ branchcode => $item->homebranch }); - AddIssue( $patron->unblessed, $item->barcode, dt_from_string ); + AddIssue( $patron, $item->barcode, dt_from_string ); AddReturn( $item->barcode, $item->homebranch, undef, dt_from_string ); my $statistic = Koha::Statistics->search( { itemnumber => $item->itemnumber } )->next; $pseudonymized = Koha::PseudonymizedTransactions->search( { itemnumber => $item->itemnumber } )->next; diff --git a/t/db_dependent/Koha/Template/Plugin/CirculationRules.t b/t/db_dependent/Koha/Template/Plugin/CirculationRules.t index 458920d4d4..a358da3d3f 100755 --- a/t/db_dependent/Koha/Template/Plugin/CirculationRules.t +++ b/t/db_dependent/Koha/Template/Plugin/CirculationRules.t @@ -74,7 +74,7 @@ my $item = $builder->build_sample_item( } ); -my $issue = AddIssue( $patron->unblessed, $item->barcode ); +my $issue = AddIssue( $patron, $item->barcode ); my $rules = $plugin->Renewals( $patron->id, $item->id ); diff --git a/t/db_dependent/Letters/TemplateToolkit.t b/t/db_dependent/Letters/TemplateToolkit.t index d9213feb4e..caf4ae9c03 100755 --- a/t/db_dependent/Letters/TemplateToolkit.t +++ b/t/db_dependent/Letters/TemplateToolkit.t @@ -63,7 +63,8 @@ my $mocked_datetime = Test::MockModule->new('DateTime'); $mocked_datetime->mock( 'now', sub { return $now_value->clone; } ); my $library = $builder->build( { source => 'Branch' } ); -my $patron = $builder->build( { source => 'Borrower' } ); +my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $patron2 = $builder->build( { source => 'Borrower' } ); my $item = $builder->build_sample_item(); @@ -71,7 +72,7 @@ my $hold = $builder->build_object( { class => 'Koha::Holds', value => { - borrowernumber => $patron->{borrowernumber}, + borrowernumber => $patron->borrowernumber, biblionumber => $item->biblionumber } } @@ -109,47 +110,47 @@ $prepared_letter = GetPreparedLetter( module => 'test', letter_code => 'TEST_PATRON', tables => { - borrowers => $patron->{borrowernumber}, + borrowers => $patron->borrowernumber, }, ) ); -is( $prepared_letter->{content}, $patron->{borrowernumber}, 'Patron object used correctly with scalar for content' ); -is( $prepared_letter->{title}, $patron->{firstname}, 'Patron object used correctly with scalar for title' ); +is( $prepared_letter->{content}, $patron->borrowernumber, 'Patron object used correctly with scalar for content' ); +is( $prepared_letter->{title}, $patron->firstname, 'Patron object used correctly with scalar for title' ); $prepared_letter = GetPreparedLetter( ( module => 'test', letter_code => 'TEST_PATRON', tables => { - borrowers => $patron, + borrowers => $patron->unblessed, }, ) ); -is( $prepared_letter->{content}, $patron->{borrowernumber}, 'Patron object used correctly with hashref for content' ); -is( $prepared_letter->{title}, $patron->{firstname}, 'Patron object used correctly with hashref for title' ); +is( $prepared_letter->{content}, $patron->borrowernumber, 'Patron object used correctly with hashref for content' ); +is( $prepared_letter->{title}, $patron->firstname, 'Patron object used correctly with hashref for title' ); $prepared_letter = GetPreparedLetter( ( module => 'test', letter_code => 'TEST_PATRON', tables => { - borrowers => [ $patron->{borrowernumber} ], + borrowers => [ $patron->borrowernumber ], }, ) ); -is( $prepared_letter->{content}, $patron->{borrowernumber}, 'Patron object used correctly with arrayref for content' ); -is( $prepared_letter->{title}, $patron->{firstname}, 'Patron object used correctly with arrayref for title' ); +is( $prepared_letter->{content}, $patron->borrowernumber, 'Patron object used correctly with arrayref for content' ); +is( $prepared_letter->{title}, $patron->firstname, 'Patron object used correctly with arrayref for title' ); $prepared_letter = GetPreparedLetter( ( module => 'test', letter_code => 'TEST_PATRON', objects => { - borrower => scalar Koha::Patrons->find( $patron->{borrowernumber} ), + borrower => scalar Koha::Patrons->find( $patron->borrowernumber ), }, ) ); -is( $prepared_letter->{content}, $patron->{borrowernumber}, 'Patron object used correctly as object' ); +is( $prepared_letter->{content}, $patron->borrowernumber, 'Patron object used correctly as object' ); $sth->execute( "TEST_BIBLIO", "[% biblio.title %]", "[% biblio.id %]" ); $prepared_letter = GetPreparedLetter( @@ -209,7 +210,7 @@ $prepared_letter = GetPreparedLetter( module => 'test', letter_code => 'TEST_HOLD', tables => { - reserves => { borrowernumber => $patron->{borrowernumber}, biblionumber => $item->biblionumber }, + reserves => { borrowernumber => $patron->borrowernumber, biblionumber => $item->biblionumber }, }, ) ); @@ -222,7 +223,7 @@ eval { module => 'test', letter_code => 'TEST_HOLD', tables => { - reserves => [ $patron->{borrowernumber}, $item->biblionumber ], + reserves => [ $patron->borrowernumber, $item->biblionumber ], }, ) ) @@ -237,7 +238,7 @@ $prepared_letter = GetPreparedLetter( letter_code => 'TEST_HOLD', tables => { 'branches' => $library, - 'borrowers' => $patron, + 'borrowers' => $patron->unblessed, 'biblio' => $item->biblionumber, 'biblioitems' => $item->biblioitemnumber, 'reserves' => $hold->unblessed, @@ -357,7 +358,7 @@ Dear <> <>, The order <> (<>) has been received. Your library. |; - my $params = { code => $code, branchcode => $branchcode, tables => { branches => $library, borrowers => $patron, biblio => $biblio1, aqorders => $order } }; + my $params = { code => $code, branchcode => $branchcode, tables => { branches => $library, borrowers => $patron->unblessed, biblio => $biblio1, aqorders => $order } }; my $letter = process_letter( { template => $template, %$params }); my $tt_template = q| Dear [% borrower.firstname %] [% borrower.surname %], @@ -433,14 +434,18 @@ Notes: [% article_request.patron_notes %] my $dbh = C4::Context->dbh; # Enable notification for CHECKOUT - Things are hardcoded here but should work with default data - $dbh->do(q|INSERT INTO borrower_message_preferences( borrowernumber, message_attribute_id ) VALUES ( ?, ? )|, undef, $patron->{borrowernumber}, 6 ); + $dbh->do(q|INSERT INTO borrower_message_preferences( borrowernumber, message_attribute_id ) VALUES ( ?, ? )|, + undef, $patron->borrowernumber, 6 ); my $borrower_message_preference_id = $dbh->last_insert_id(undef, undef, "borrower_message_preferences", undef); $dbh->do(q|INSERT INTO borrower_message_transport_preferences( borrower_message_preference_id, message_transport_type) VALUES ( ?, ? )|, undef, $borrower_message_preference_id, 'email' ); + # Enable notification for CHECKIN - Things are hardcoded here but should work with default data - $dbh->do(q|INSERT INTO borrower_message_preferences( borrowernumber, message_attribute_id ) VALUES ( ?, ? )|, undef, $patron->{borrowernumber}, 5 ); + $dbh->do(q|INSERT INTO borrower_message_preferences( borrowernumber, message_attribute_id ) VALUES ( ?, ? )|, + undef, $patron->borrowernumber, 5); $borrower_message_preference_id = $dbh->last_insert_id(undef, undef, "borrower_message_preferences", undef); $dbh->do(q|INSERT INTO borrower_message_transport_preferences( borrower_message_preference_id, message_transport_type) VALUES ( ?, ? )|, undef, $borrower_message_preference_id, 'email' ); + # historic syntax my $checkout_template = q| The following items have been checked out: @@ -461,11 +466,13 @@ Thank you for visiting <>. C4::Circulation::AddIssue( $patron, $item1->{barcode} ); my $first_checkout_letter = Koha::Notice::Messages->search( {}, { order_by => { -desc => 'message_id' } } )->next; + C4::Circulation::AddIssue( $patron, $item2->{barcode} ); my $second_checkout_letter = Koha::Notice::Messages->search( {}, { order_by => { -desc => 'message_id' } } )->next; AddReturn( $item1->{barcode} ); my $first_checkin_letter = Koha::Notice::Messages->search( {}, { order_by => { -desc => 'message_id' } } )->next; + AddReturn( $item2->{barcode} ); my $second_checkin_letter = Koha::Notice::Messages->search( {}, { order_by => { -desc => 'message_id' } } )->next; @@ -491,11 +498,13 @@ Thank you for visiting [% branch.branchname %]. C4::Circulation::AddIssue( $patron, $item1->{barcode} ); my $first_checkout_tt_letter = Koha::Notice::Messages->search( {}, { order_by => { -desc => 'message_id' } } )->next; + C4::Circulation::AddIssue( $patron, $item2->{barcode} ); my $second_checkout_tt_letter = Koha::Notice::Messages->search( {}, { order_by => { -desc => 'message_id' } } )->next; AddReturn( $item1->{barcode} ); my $first_checkin_tt_letter = Koha::Notice::Messages->search( {}, { order_by => { -desc => 'message_id' } } )->next; + AddReturn( $item2->{barcode} ); my $second_checkin_tt_letter = Koha::Notice::Messages->search( {}, { order_by => { -desc => 'message_id' } } )->next; @@ -513,7 +522,7 @@ Thank you for visiting [% branch.branchname %]. my $dbh = C4::Context->dbh; # Enable notification for DUEDGST - Things are hardcoded here but should work with default data - $dbh->do(q|INSERT INTO borrower_message_preferences( borrowernumber, message_attribute_id ) VALUES ( ?, ? )|, undef, $patron->{borrowernumber}, 1 ); + $dbh->do(q|INSERT INTO borrower_message_preferences( borrowernumber, message_attribute_id ) VALUES ( ?, ? )|, undef, $patron->borrowernumber, 1 ); my $borrower_message_preference_id = $dbh->last_insert_id(undef, undef, "borrower_message_preferences", undef); $dbh->do(q|INSERT INTO borrower_message_transport_preferences( borrower_message_preference_id, message_transport_type) VALUES ( ?, ? )|, undef, $borrower_message_preference_id, 'email' ); @@ -542,7 +551,7 @@ You have [% count %] items due my $reserve_id1 = C4::Reserves::AddReserve( { branchcode => $library->{branchcode}, - borrowernumber => $patron->{borrowernumber}, + borrowernumber => $patron->borrowernumber, biblionumber => $biblio1->{biblionumber}, notes => "a note", itemnumber => $item1->{itemnumber}, @@ -551,7 +560,7 @@ You have [% count %] items due my $reserve_id2 = C4::Reserves::AddReserve( { branchcode => $library->{branchcode}, - borrowernumber => $patron->{borrowernumber}, + borrowernumber => $patron->borrowernumber, biblionumber => $biblio1->{biblionumber}, notes => "a note", itemnumber => $item1->{itemnumber}, @@ -560,7 +569,7 @@ You have [% count %] items due my $reserve_id3 = C4::Reserves::AddReserve( { branchcode => $library->{branchcode}, - borrowernumber => $patron->{borrowernumber}, + borrowernumber => $patron->borrowernumber, biblionumber => $biblio2->{biblionumber}, notes => "another note", itemnumber => $item2->{itemnumber}, @@ -708,13 +717,13 @@ EOF my $checkout = C4::Circulation::AddIssue( $patron, $item1->{barcode} ); # Add a first checkout $checkout->set( { timestamp => $now, issuedate => $one_minute_ago } )->store; - my $first_slip = C4::Members::IssueSlip( $branchcode, $patron->{borrowernumber} ); + my $first_slip = C4::Members::IssueSlip( $branchcode, $patron->borrowernumber ); $checkout = C4::Circulation::AddIssue( $patron, $item2->{barcode} ); # Add a second checkout $checkout->set( { timestamp => $now, issuedate => $now } )->store; my $yesterday = dt_from_string->subtract( days => 1 ); C4::Circulation::AddIssue( $patron, $item3->{barcode}, $yesterday ); # Add an overdue - my $second_slip = C4::Members::IssueSlip( $branchcode, $patron->{borrowernumber} ); + my $second_slip = C4::Members::IssueSlip( $branchcode, $patron->borrowernumber ); # Cleanup AddReturn( $item1->{barcode} ); @@ -768,12 +777,12 @@ EOF $checkout = C4::Circulation::AddIssue( $patron, $item1->{barcode} ); # Add a first checkout $checkout->set( { timestamp => $now, issuedate => $one_minute_ago } )->store; - my $first_tt_slip = C4::Members::IssueSlip( $branchcode, $patron->{borrowernumber} ); + my $first_tt_slip = C4::Members::IssueSlip( $branchcode, $patron->borrowernumber ); $checkout = C4::Circulation::AddIssue( $patron, $item2->{barcode} ); # Add a second checkout $checkout->set( { timestamp => $now, issuedate => $now } )->store; C4::Circulation::AddIssue( $patron, $item3->{barcode}, $yesterday ); # Add an overdue - my $second_tt_slip = C4::Members::IssueSlip( $branchcode, $patron->{borrowernumber} ); + my $second_tt_slip = C4::Members::IssueSlip( $branchcode, $patron->borrowernumber ); # There is too many line breaks generated by the historic syntax $second_slip->{content} =~ s|

\n\n\n

|

\n\n

|s; @@ -846,7 +855,7 @@ EOF my $letter = C4::Overdues::parse_overdues_letter( { letter_code => $code, - borrowernumber => $patron->{borrowernumber}, + borrowernumber => $patron->borrowernumber, branchcode => $library->{branchcode}, items => \@items, substitute => { @@ -904,7 +913,7 @@ EOF my $tt_letter = C4::Overdues::parse_overdues_letter( { letter_code => $code, - borrowernumber => $patron->{borrowernumber}, + borrowernumber => $patron->borrowernumber, branchcode => $library->{branchcode}, items => \@items, substitute => { @@ -931,11 +940,11 @@ EOF $dbh->do("DELETE FROM message_queue"); # Enable notification for CHECKOUT - Things are hardcoded here but should work with default data - $dbh->do(q|INSERT INTO borrower_message_preferences( borrowernumber, message_attribute_id ) VALUES ( ?, ? )|, undef, $patron->{borrowernumber}, 6 ); + $dbh->do(q|INSERT INTO borrower_message_preferences( borrowernumber, message_attribute_id ) VALUES ( ?, ? )|, undef, $patron->borrowernumber, 6 ); my $borrower_message_preference_id = $dbh->last_insert_id(undef, undef, "borrower_message_preferences", undef); $dbh->do(q|INSERT INTO borrower_message_transport_preferences( borrower_message_preference_id, message_transport_type) VALUES ( ?, ? )|, undef, $borrower_message_preference_id, 'email' ); # Enable notification for CHECKIN - Things are hardcoded here but should work with default data - $dbh->do(q|INSERT INTO borrower_message_preferences( borrowernumber, message_attribute_id ) VALUES ( ?, ? )|, undef, $patron->{borrowernumber}, 5 ); + $dbh->do(q|INSERT INTO borrower_message_preferences( borrowernumber, message_attribute_id ) VALUES ( ?, ? )|, undef, $patron->borrowernumber, 5 ); $borrower_message_preference_id = $dbh->last_insert_id(undef, undef, "borrower_message_preferences", undef); $dbh->do(q|INSERT INTO borrower_message_transport_preferences( borrower_message_preference_id, message_transport_type) VALUES ( ?, ? )|, undef, $borrower_message_preference_id, 'email' ); diff --git a/t/db_dependent/Members/GetAllIssues.t b/t/db_dependent/Members/GetAllIssues.t index b327c8fdc5..275faa509a 100755 --- a/t/db_dependent/Members/GetAllIssues.t +++ b/t/db_dependent/Members/GetAllIssues.t @@ -66,8 +66,8 @@ my $borrowernumber1 = Koha::Patron->new({ categorycode => $categorycode, branchcode => $branchcode })->store->borrowernumber; my $borrowernumber2 = Koha::Patron->new({ categorycode => $categorycode, branchcode => $branchcode })->store->borrowernumber; -my $borrower1 = Koha::Patrons->find( $borrowernumber1 )->unblessed; -my $borrower2 = Koha::Patrons->find( $borrowernumber2 )->unblessed; +my $borrower1 = Koha::Patrons->find( $borrowernumber1 ); +my $borrower2 = Koha::Patrons->find( $borrowernumber2 ); my $module = Test::MockModule->new('C4::Context'); $module->mock( 'userenv', sub { { branch => $branchcode } } ); diff --git a/t/db_dependent/Members/IssueSlip.t b/t/db_dependent/Members/IssueSlip.t index dced42384d..bbf4bb3a7d 100755 --- a/t/db_dependent/Members/IssueSlip.t +++ b/t/db_dependent/Members/IssueSlip.t @@ -118,7 +118,7 @@ my $itemnumber2 = $builder->build_sample_item({ biblionumber => $biblionumber2, my $borrowernumber = Koha::Patron->new({ categorycode => $categorycode, branchcode => $branchcode })->store->borrowernumber; -my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; +my $borrower = Koha::Patrons->find( $borrowernumber ); my $module = Test::MockModule->new('C4::Context'); $module->mock( 'userenv', sub { { branch => $branchcode } } ); @@ -164,7 +164,7 @@ EOS # Set timestamps to the same value to avoid a different order Koha::Checkouts->search( - { borrowernumber => $borrower->{borrowernumber} } + { borrowernumber => $borrower->borrowernumber } )->update( { timestamp => dt_from_string } ); $expected_slip = <search( - { borrowernumber => $borrower->{borrowernumber} } + { borrowernumber => $borrower->borrowernumber } )->update( { timestamp => dt_from_string } ); $expected_slip = <build({ source => 'Branch' }); my $itemtype = $builder->build({ source => 'Itemtype' })->{itemtype}; C4::Context->_new_userenv('xxx'); -my $patron = $builder->build({ - source => 'Borrower', +my $patron = $builder->build_object({ + class => 'Koha::Patrons', value => { branchcode => $library->{branchcode}, flags => 1, # superlibrarian } }); -my $p = Koha::Patrons->find( $patron->{borrowernumber} ); -t::lib::Mocks::mock_userenv({ patron => $p }); +t::lib::Mocks::mock_userenv({ patron => $patron }); -my $patron2 = $builder->build({ - source => 'Borrower', +my $patron2 = $builder->build_object({ + class => 'Koha::Patrons', value => { branchcode => $library->{branchcode}, } }); -my $patron3 = $builder->build({ - source => 'Borrower', +my $patron3 = $builder->build_object({ + class => 'Koha::Patrons', value => { branchcode => $another_library->{branchcode}, flags => undef, } }); -my $p3 = Koha::Patrons->find( $patron3->{borrowernumber} ); # Discharge not possible with issues my ( $biblionumber ) = AddBiblio( MARC::Record->new, ''); @@ -81,37 +79,37 @@ $builder->build_sample_item( ); AddIssue( $patron, $barcode ); -is( Koha::Patron::Discharge::can_be_discharged({ borrowernumber => $patron->{borrowernumber} }), 0, 'A patron with issues cannot be discharged' ); +is( Koha::Patron::Discharge::can_be_discharged({ borrowernumber => $patron->borrowernumber }), 0, 'A patron with issues cannot be discharged' ); -is( Koha::Patron::Discharge::request({ borrowernumber => $patron->{borrowernumber} }), undef, 'No request done if patron has issues' ); -is( Koha::Patron::Discharge::discharge({ borrowernumber => $patron->{borrowernumber} }), undef, 'No discharge done if patron has issues' ); +is( Koha::Patron::Discharge::request({ borrowernumber => $patron->borrowernumber }), undef, 'No request done if patron has issues' ); +is( Koha::Patron::Discharge::discharge({ borrowernumber => $patron->borrowernumber }), undef, 'No discharge done if patron has issues' ); is_deeply( [ Koha::Patron::Discharge::get_pendings ], [], 'There is no pending discharge request' ); is_deeply( [ Koha::Patron::Discharge::get_validated ], [], 'There is no validated discharge' ); AddReturn( $barcode ); # Discharge possible without issue -is( Koha::Patron::Discharge::can_be_discharged({ borrowernumber => $patron->{borrowernumber} }), 1, 'A patron without issues can be discharged' ); +is( Koha::Patron::Discharge::can_be_discharged({ borrowernumber => $patron->borrowernumber }), 1, 'A patron without issues can be discharged' ); is(Koha::Patron::Discharge::generate_as_pdf,undef,"Confirm failure when lacking borrower number"); # Verify that the user is not discharged anymore if the restriction has been lifted -Koha::Patron::Discharge::discharge( { borrowernumber => $patron->{borrowernumber} } ); -Koha::Patron::Discharge::discharge( { borrowernumber => $patron2->{borrowernumber} } ); -Koha::Patron::Discharge::discharge( { borrowernumber => $patron3->{borrowernumber} } ); -is( Koha::Patron::Discharge::is_discharged( { borrowernumber => $patron->{borrowernumber} } ), 1, 'The patron has been discharged' ); -is( Koha::Patrons->find( $patron->{borrowernumber} )->is_debarred, '9999-12-31', 'The patron has been debarred after discharge' ); +Koha::Patron::Discharge::discharge( { borrowernumber => $patron->borrowernumber } ); +Koha::Patron::Discharge::discharge( { borrowernumber => $patron2->borrowernumber } ); +Koha::Patron::Discharge::discharge( { borrowernumber => $patron3->borrowernumber } ); +is( Koha::Patron::Discharge::is_discharged( { borrowernumber => $patron->borrowernumber } ), 1, 'The patron has been discharged' ); +is( Koha::Patrons->find( $patron->borrowernumber )->is_debarred, '9999-12-31', 'The patron has been debarred after discharge' ); is( scalar( Koha::Patron::Discharge::get_validated ), 3, 'There are 3 validated discharges' ); -is( scalar( Koha::Patron::Discharge::get_validated( { borrowernumber => $patron->{borrowernumber} } ) ), 1, 'There is 1 validated discharge for a given patron' ); +is( scalar( Koha::Patron::Discharge::get_validated( { borrowernumber => $patron->borrowernumber } ) ), 1, 'There is 1 validated discharge for a given patron' ); is( scalar( Koha::Patron::Discharge::get_validated( { branchcode => $library->{branchcode} } ) ), 2, 'There is 2 validated discharges for a given branchcode' ); # This is not used in the code yet -Koha::Patron::Debarments::DelUniqueDebarment( { 'borrowernumber' => $patron->{borrowernumber}, 'type' => 'DISCHARGE' } ); -ok( !Koha::Patrons->find( $patron->{borrowernumber} )->is_debarred, 'The debarment has been lifted' ); -ok( !Koha::Patron::Discharge::is_discharged( { borrowernumber => $patron->{borrowernumber} } ), 'The patron is not discharged after the restriction has been lifted' ); +Koha::Patron::Debarments::DelUniqueDebarment( { 'borrowernumber' => $patron->borrowernumber, 'type' => 'DISCHARGE' } ); +ok( !Koha::Patrons->find( $patron->borrowernumber )->is_debarred, 'The debarment has been lifted' ); +ok( !Koha::Patron::Discharge::is_discharged( { borrowernumber => $patron->borrowernumber } ), 'The patron is not discharged after the restriction has been lifted' ); # Verify that the discharge works multiple times -Koha::Patron::Discharge::request({ borrowernumber => $patron->{borrowernumber} }); +Koha::Patron::Discharge::request({ borrowernumber => $patron->borrowernumber }); is(scalar( Koha::Patron::Discharge::get_pendings ), 1, 'There is a pending discharge request (second time)'); -Koha::Patron::Discharge::discharge( { borrowernumber => $patron->{borrowernumber} } ); +Koha::Patron::Discharge::discharge( { borrowernumber => $patron->borrowernumber } ); is_deeply( [ Koha::Patron::Discharge::get_pendings ], [], 'There is no pending discharge request (second time)'); # Check if PDF::FromHTML is installed. @@ -119,17 +117,17 @@ my $check = eval { require PDF::FromHTML; }; # Tests for if PDF::FromHTML is installed if ($check) { - isnt( Koha::Patron::Discharge::generate_as_pdf({ borrowernumber => $patron->{borrowernumber} }), undef, "Temporary PDF generated." ); + isnt( Koha::Patron::Discharge::generate_as_pdf({ borrowernumber => $patron->borrowernumber }), undef, "Temporary PDF generated." ); } # Tests for if PDF::FromHTML is not installed else { - warning_like { Koha::Patron::Discharge::generate_as_pdf({ borrowernumber => $patron->{borrowernumber}, testing => 1 }) } + warning_like { Koha::Patron::Discharge::generate_as_pdf({ borrowernumber => $patron->borrowernumber, testing => 1 }) } [ qr/Can't locate PDF\/FromHTML.pm in \@INC/ ], "Expected failure because of missing PDF::FromHTML."; } # FIXME Should be a Koha::Object object -is( ref(Koha::Patron::Discharge::request({ borrowernumber => $patron->{borrowernumber} })), 'Koha::Schema::Result::Discharge', 'Discharge request sent' ); +is( ref(Koha::Patron::Discharge::request({ borrowernumber => $patron->borrowernumber })), 'Koha::Schema::Result::Discharge', 'Discharge request sent' ); subtest 'search_limited' => sub { plan tests => 4; @@ -138,17 +136,17 @@ subtest 'search_limited' => sub { my $group_2 = Koha::Library::Group->new( { title => 'TEST Group 2' } )->store; # $patron and $patron2 are from the same library, $patron3 from another one # Logged in user is $patron, superlibrarian - t::lib::Mocks::mock_userenv({ patron => $p }); - Koha::Library::Group->new({ parent_id => $group_1->id, branchcode => $patron->{branchcode} })->store(); - Koha::Library::Group->new({ parent_id => $group_2->id, branchcode => $patron3->{branchcode} })->store(); - Koha::Patron::Discharge::request({ borrowernumber => $patron->{borrowernumber} }); - Koha::Patron::Discharge::request({ borrowernumber => $patron2->{borrowernumber} }); - Koha::Patron::Discharge::request({ borrowernumber => $patron3->{borrowernumber} }); + t::lib::Mocks::mock_userenv({ patron => $patron }); + Koha::Library::Group->new({ parent_id => $group_1->id, branchcode => $patron->branchcode })->store(); + Koha::Library::Group->new({ parent_id => $group_2->id, branchcode => $patron3->branchcode })->store(); + Koha::Patron::Discharge::request({ borrowernumber => $patron->borrowernumber }); + Koha::Patron::Discharge::request({ borrowernumber => $patron2->borrowernumber }); + Koha::Patron::Discharge::request({ borrowernumber => $patron3->borrowernumber }); is( scalar( Koha::Patron::Discharge::get_pendings), 3, 'With permission, all discharges are visible' ); is( Koha::Patron::Discharge::count({pending => 1}), 3, 'With permission, all discharges are visible' ); # With patron 3 logged in, only discharges from their group are visible - t::lib::Mocks::mock_userenv({ patron => $p3 }); + t::lib::Mocks::mock_userenv({ patron => $patron3 }); is( scalar( Koha::Patron::Discharge::get_pendings), 1, 'Without permission, only discharge from our group are visible' ); is( Koha::Patron::Discharge::count({pending => 1}), 1, 'Without permission, only discharge from our group are visible' ); }; diff --git a/t/db_dependent/Patron/Borrower_PrevCheckout.t b/t/db_dependent/Patron/Borrower_PrevCheckout.t index ca10733a60..310458d119 100755 --- a/t/db_dependent/Patron/Borrower_PrevCheckout.t +++ b/t/db_dependent/Patron/Borrower_PrevCheckout.t @@ -297,7 +297,7 @@ my $cpvmappings = [ test_it($cpvmappings, "PreIssue"); # Issue item_1 to $patron: -my $patron_get_mem = Koha::Patrons->find( $patron->{borrowernumber} )->unblessed; +my $patron_get_mem = Koha::Patrons->find( $patron->{borrowernumber} ); BAIL_OUT("Issue failed") unless AddIssue($patron_get_mem, $item_1->{barcode}); @@ -382,7 +382,7 @@ my $new_item = $builder->build_sample_item->unblessed; my $prev_item = $builder->build_sample_item->unblessed; # Second is Checked Out BAIL_OUT("CanBookBeIssued Issue failed") - unless AddIssue($patron->unblessed, $prev_item->{barcode}); + unless AddIssue($patron, $prev_item->{barcode}); # Mappings my $CBBI_mappings = [ @@ -445,7 +445,7 @@ subtest 'Check previous checkouts for serial' => sub { my $item1 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber }); my $item2 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber }); - AddIssue($patron->unblessed, $item1->barcode); + AddIssue($patron, $item1->barcode); is($patron->do_check_for_previous_checkout($item1->unblessed), 1, 'Check only one item if bibliographic record is serial'); is($patron->do_check_for_previous_checkout($item2->unblessed), 0, 'Check only one item if bibliographic record is serial'); diff --git a/t/db_dependent/SIP/ILS.t b/t/db_dependent/SIP/ILS.t index e96dd4ce55..79aa8ddca2 100755 --- a/t/db_dependent/SIP/ILS.t +++ b/t/db_dependent/SIP/ILS.t @@ -260,7 +260,7 @@ subtest checkout => sub { t::lib::Mocks::mock_userenv({ branchcode => $library->branchcode, flags => 1 }); my $item = $builder->build_sample_item({ - library => $library->branchcode, + library => $library->branchcode, }); Koha::CirculationRules->set_rules( @@ -279,16 +279,16 @@ subtest checkout => sub { } ); - AddIssue( $patron->unblessed, $item->barcode, undef, 0 ); + AddIssue( $patron, $item->barcode, undef, 0 ); my $checkout = $item->checkout; ok( defined($checkout), "Checkout added"); is( $checkout->renewals_count, 0, "Correct renewals"); my $ils = C4::SIP::ILS->new({ id => $library->branchcode }); my $sip_patron = C4::SIP::ILS::Patron->new( $patron->cardnumber ); - my $transaction = $ils->checkout($patron->cardnumber,$item->barcode,undef,undef); + my $transaction = $ils->checkout($patron->cardnumber, $item->barcode); - is( $transaction->{screen_msg},"Item already checked out to you: renewing item.","We get a success message when issue is renewed"); + is( $transaction->{screen_msg},"Item already checked out to you: renewing item.", "We get a success message when issue is renewed"); $checkout->discard_changes(); is( $checkout->renewals_count, 1, "Renewals has been reduced"); diff --git a/t/db_dependent/rollingloans.t b/t/db_dependent/rollingloans.t index c997cc90fa..60a4d6522c 100755 --- a/t/db_dependent/rollingloans.t +++ b/t/db_dependent/rollingloans.t @@ -62,16 +62,16 @@ SKIP: { } sub try_issue { - my ($cardnumber, $item ) = @_; + my ($cardnumber, $item_barcode ) = @_; my $issuedate = '2011-05-16'; my $patron = Koha::Patrons->find( { cardnumber => $cardnumber } ); - my ($issuingimpossible,$needsconfirmation) = CanBookBeIssued( $patron, $item ); - my $issue = AddIssue($patron->unblessed, $item, undef, 0, $issuedate); + my ($issuingimpossible,$needsconfirmation) = CanBookBeIssued( $patron, $item_barcode ); + my $issue = AddIssue($patron, $item_barcode, undef, 0, $issuedate); return dt_from_string( $issue->date_due ); } sub try_return { - my $barcode = shift; - my ($ret, $messages, $iteminformation, $borrower) = AddReturn($barcode); + my $item_barcode = shift; + my ($ret, $messages, $iteminformation, $borrower) = AddReturn($item_barcode); return $ret; } diff --git a/t/db_dependent/selenium/regressions.t b/t/db_dependent/selenium/regressions.t index 8b3e34646b..304c3ab5b9 100755 --- a/t/db_dependent/selenium/regressions.t +++ b/t/db_dependent/selenium/regressions.t @@ -176,7 +176,7 @@ subtest 'Display circulation table correctly' => sub { } ); - C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + C4::Circulation::AddIssue( $patron, $item->barcode ); my $mainpage = $s->base_url . q|mainpage.pl|; $driver->get($mainpage . q|?logout.x=1|); -- 2.35.1