From f7265585109d28750eedddb1ba3d44d4c61382b7 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 t/db_dependent/Holds.t t/db_dependent/Holds/LocalHoldsPriority.t t/db_dependent/Holds/HoldFulfillmentPolicy.t t/db_dependent/Holds/HoldItemtypeLimit.t t/db_dependent/Reserves/GetReserveFee.t t/db_dependent/api/v1/return_claims.t t/db_dependent/api/v1/biblios.t t/db_dependent/api/v1/checkouts.t t/db_dependent/Reserves.t t/db_dependent/HoldsQueue.t t/db_dependent/selenium/regressions.t t/db_dependent/Koha/Plugins/Circulation_hooks.t t/db_dependent/Koha/Plugins/Recall_hooks.t t/db_dependent/Koha/Recalls.t t/db_dependent/Koha/Recall.t t/db_dependent/Circulation/_CalculateAndUpdateFine.t Sponsored-by: Gothenburg University Library Signed-off-by: Jonathan Druart Signed-off-by: Marcel de Rooy Signed-off-by: Tomas Cohen Arazi --- 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 | 224 ++++++++++-------- t/db_dependent/Circulation/CalcDateDue.t | 50 ++-- .../Circulation/CheckIfIssuedToPatron.t | 30 ++- .../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 +- .../Circulation/_CalculateAndUpdateFine.t | 12 +- t/db_dependent/Circulation/dateexpiry.t | 20 +- t/db_dependent/Circulation/issue.t | 24 +- .../Circulation/maxsuspensiondays.t | 18 +- 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/HoldsQueue.t | 2 +- 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 | 24 +- t/db_dependent/Koha/Object.t | 5 +- t/db_dependent/Koha/Patrons.t | 7 +- .../Koha/Plugins/Circulation_hooks.t | 4 +- t/db_dependent/Koha/Plugins/Recall_hooks.t | 2 +- t/db_dependent/Koha/Pseudonymization.t | 4 +- t/db_dependent/Koha/Recall.t | 4 +- t/db_dependent/Koha/Recalls.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 | 68 +++--- t/db_dependent/Patron/Borrower_PrevCheckout.t | 6 +- t/db_dependent/Reserves.t | 4 +- t/db_dependent/Reserves/GetReserveFee.t | 104 ++++---- t/db_dependent/SIP/ILS.t | 10 +- t/db_dependent/api/v1/biblios.t | 4 +- t/db_dependent/api/v1/checkouts.t | 8 +- t/db_dependent/api/v1/return_claims.t | 8 +- t/db_dependent/rollingloans.t | 10 +- t/db_dependent/selenium/regressions.t | 2 +- 58 files changed, 600 insertions(+), 570 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 98c66828f5..531b99f1f1 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 @@ -795,10 +791,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; } @@ -1007,7 +1003,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 ) { @@ -1347,7 +1343,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, @@ -1421,7 +1417,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( { @@ -1456,13 +1452,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. @@ -1497,7 +1493,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}; @@ -1525,23 +1521,23 @@ 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( { - borrowernumber => $borrower->{'borrowernumber'}, + borrowernumber => $patron->borrowernumber, itemnumber => $item_object->itemnumber, branch => $branchcode, datedue => $datedue, @@ -1555,7 +1551,7 @@ sub AddIssue { unless ($datedue) { my $itype = $item_object->effective_itemtype; - $datedue = CalcDateDue( $issuedate, $itype, $branchcode, $borrower ); + $datedue = CalcDateDue( $issuedate, $itype, $branchcode, $patron ); } @@ -1564,7 +1560,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; @@ -1579,7 +1575,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( { @@ -1607,12 +1602,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 ) { @@ -1637,7 +1632,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' @@ -1648,7 +1643,7 @@ sub AddIssue { } my $issue_attributes = { - borrowernumber => $borrower->{'borrowernumber'}, + borrowernumber => $patron->borrowernumber, issuedate => $issuedate, date_due => $datedue, branchcode => C4::Context->userenv->{'branch'}, @@ -1682,7 +1677,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. @@ -1708,31 +1703,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' ); } @@ -1776,10 +1770,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'}, - interface => C4::Context->interface, + categorycode => $patron->categorycode } ); @@ -1787,7 +1780,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', ); @@ -1796,14 +1789,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"); @@ -2377,7 +2370,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 ($debardate and $debardate ne "9999-12-31") { if ($reminder){ $messages->{'PrevDebarred'} = $debardate; @@ -2647,11 +2640,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 @@ -2668,13 +2661,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', @@ -2723,8 +2716,7 @@ sub _calculate_new_debar_dt { } my ( $has_been_extended ); - if ( C4::Context->preference('CumulativeRestrictionPeriods') and $borrower->{debarred} ) { - my $patron = Koha::Patrons->find($borrower->{borrowernumber}); + if ( C4::Context->preference('CumulativeRestrictionPeriods') and $patron->is_debarred ) { my $debarment = $patron->restrictions->search({type => 'SUSPENSION' },{rows => 1})->single; if ( $debarment ) { $return_date = dt_from_string( $debarment->expiration, 'sql' ); @@ -2750,25 +2742,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(); } @@ -2859,36 +2850,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; @@ -2964,7 +2955,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, @@ -3177,7 +3168,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{ @@ -3196,7 +3187,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( @@ -3375,7 +3366,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( { @@ -3431,7 +3422,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, @@ -3498,8 +3489,8 @@ sub GetLatestAutoRenewDate { return unless $item; my $dbh = C4::Context->dbh; + my $branchcode = _GetCircControlBranch( $item, $patron ); - my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed ); my $circulation_rules = Koha::CirculationRules->get_effective_rules( { categorycode => $patron->categorycode, @@ -3796,19 +3787,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} @@ -3828,7 +3819,7 @@ sub CalcDateDue { my $daysmode = Koha::CirculationRules->get_effective_daysmode( { - categorycode => $borrower->{categorycode}, + categorycode => $patron->categorycode, itemtype => $itemtype, branchcode => $branch, } @@ -3862,7 +3853,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); @@ -3882,7 +3873,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'); @@ -4165,7 +4156,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 3f7e34d13f..87b1179cba 100644 --- a/Koha/Illrequest.pm +++ b/Koha/Illrequest.pm @@ -1369,8 +1369,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 dba357e680..8be3082f4d 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1877,7 +1877,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 ab390962b2..c70acb89dd 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 9b925e29ad..99cee0c87b 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -399,7 +399,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 36f9f34920..2630e677cc 100755 --- a/offline_circ/process_koc.pl +++ b/offline_circ/process_koc.pl @@ -190,7 +190,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; @@ -198,7 +197,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( @@ -214,11 +213,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 { @@ -229,17 +228,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. @@ -249,17 +248,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 9f70a415cc..bffa06f1c7 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 f9f18aa62f..36af34399c 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -476,8 +476,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 6a722ac8c2..babbc0384a 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -90,7 +90,7 @@ sub test_debarment_on_checkout { ); my @caller = caller; my $line_number = $caller[2]; - AddIssue( $patron->unblessed, $item->barcode, $due_date ); + AddIssue( $patron, $item->barcode, $due_date ); my ( undef, $message ) = AddReturn( $item->barcode, $library->{branchcode}, undef, $return_date ); is( $message->{WasReturned} && exists $message->{Debarred}, 1, 'AddReturn must have debarred the patron' ) @@ -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() ); @@ -519,11 +523,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; @@ -634,7 +638,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 = ?', @@ -682,7 +686,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'); @@ -711,7 +715,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(); @@ -719,7 +723,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'); @@ -812,7 +816,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, @@ -840,7 +844,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 ); @@ -970,7 +974,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( { @@ -1099,7 +1103,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( { @@ -1209,7 +1213,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' ); @@ -1220,7 +1224,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' ); @@ -1230,7 +1234,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' ); @@ -1249,7 +1253,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, @@ -1501,7 +1505,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({ @@ -1582,7 +1586,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 ); @@ -1659,7 +1663,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( { @@ -1745,7 +1749,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { branchcode => $library2->{branchcode}, })->store->borrowernumber; - my $issue = AddIssue( $borrower1->unblessed, $item_1->barcode ); + my $issue = AddIssue( $borrower1, $item_1->barcode ); my ( $renewokay, $error ) = CanBookBeRenewed( $borrower1, $issue ); is( $renewokay, 1, 'Bug 14337 - Verify the borrower can renew with no hold on the record' ); @@ -1912,7 +1916,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' ); @@ -1932,7 +1936,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, @@ -1991,7 +1995,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 ); @@ -2066,8 +2070,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( { @@ -2152,7 +2165,7 @@ subtest 'AddIssue | recalls' => sub { pickup_library_id => $patron1->branchcode, } )->store; - AddIssue( $patron1->unblessed, $item->barcode, undef, undef, undef, undef, { recall_id => $recall1->id } ); + AddIssue( $patron1, $item->barcode, undef, undef, undef, undef, { recall_id => $recall1->id } ); $recall1 = Koha::Recalls->find( $recall1->id ); is( $recall1->fulfilled, 1, 'Recall was fulfilled when patron checked out item' ); AddReturn( $item->barcode, $item->homebranch ); @@ -2166,7 +2179,7 @@ subtest 'AddIssue | recalls' => sub { pickup_library_id => $patron2->branchcode, } )->store; - AddIssue( $patron1->unblessed, $item->barcode, undef, undef, undef, undef, { recall_id => $recall2->id, cancel_recall => 'cancel' } ); + AddIssue( $patron1, $item->barcode, undef, undef, undef, undef, { recall_id => $recall2->id, cancel_recall => 'cancel' } ); $recall2 = Koha::Recalls->find( $recall2->id ); is( $recall2->cancelled, 1, 'Recall was cancelled when patron checked out item' ); AddReturn( $item->barcode, $item->homebranch ); @@ -2181,7 +2194,7 @@ subtest 'AddIssue | recalls' => sub { } )->store; $recall3->set_waiting; - AddIssue( $patron1->unblessed, $item->barcode, undef, undef, undef, undef, { recall_id => $recall3->id, cancel_recall => 'revert' } ); + AddIssue( $patron1, $item->barcode, undef, undef, undef, undef, { recall_id => $recall3->id, cancel_recall => 'revert' } ); $recall3 = Koha::Recalls->find( $recall3->id ); is( $recall3->requested, 1, 'Recall was reverted from waiting when patron checked out item' ); AddReturn( $item->barcode, $item->homebranch ); @@ -2206,7 +2219,7 @@ subtest 'AddIssue & illrequests.due_date' => sub { due_date => $custom_date_due, })->store; - my $issue = AddIssue( $patron->unblessed, $item->barcode ); + my $issue = AddIssue( $patron, $item->barcode ); is( $issue->date_due, $expected_date_due, 'Custom illrequest date due has been set for this issue'); $patron = $builder->build_object( { class => 'Koha::Patrons' } ); @@ -2220,7 +2233,7 @@ subtest 'AddIssue & illrequests.due_date' => sub { due_date => $custom_date_due, })->store; - $issue = AddIssue( $patron->unblessed, $item->barcode ); + $issue = AddIssue( $patron, $item->barcode ); is( $issue->date_due, $expected_date_due, 'Custom illrequest date due has been set for this issue'); }; @@ -2263,7 +2276,7 @@ subtest 'CanBookBeIssued + Koha::Patron->is_debarred|has_overdues' => sub { # Patron cannot issue item_1, they have overdues my $yesterday = DateTime->today( time_zone => C4::Context->tz() )->add( days => -1 ); - my $issue = AddIssue( $patron->unblessed, $item_1->barcode, $yesterday ); # Add an overdue + my $issue = AddIssue( $patron, $item_1->barcode, $yesterday ); # Add an overdue t::lib::Mocks::mock_preference( 'OverduesBlockCirc', 'confirmation' ); ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->barcode ); @@ -2372,7 +2385,7 @@ subtest 'MultipleReserves' => sub { branchcode => $branch, ); my $patron = Koha::Patron->new(\%renewing_borrower_data)->store; - 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"); my $borrowing_borrowernumber = Koha::Checkouts->find({ itemnumber => $item_1->itemnumber })->borrowernumber; @@ -2485,7 +2498,7 @@ subtest 'CanBookBeIssued + AllowMultipleIssuesOnABiblio' => sub { ); my ( $error, $question, $alerts ); - my $issue = AddIssue( $patron->unblessed, $item_1->barcode, dt_from_string->add( days => 1 ) ); + my $issue = AddIssue( $patron, $item_1->barcode, dt_from_string->add( days => 1 ) ); t::lib::Mocks::mock_preference('AllowMultipleIssuesOnABiblio', 0); ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->barcode ); @@ -2569,8 +2582,8 @@ subtest 'AddReturn + CumulativeRestrictionPeriods' => sub { my $now = dt_from_string; my $five_days_ago = $now->clone->subtract( days => 5 ); my $ten_days_ago = $now->clone->subtract( days => 10 ); - AddIssue( $patron->unblessed, $item_1->barcode, $five_days_ago ); # Add an overdue - AddIssue( $patron->unblessed, $item_2->barcode, $ten_days_ago ) + AddIssue( $patron, $item_1->barcode, $five_days_ago ); # Add an overdue + AddIssue( $patron, $item_2->barcode, $ten_days_ago ) ; # Add another overdue t::lib::Mocks::mock_preference( 'CumulativeRestrictionPeriods', '0' ); @@ -2608,8 +2621,8 @@ subtest 'AddReturn + CumulativeRestrictionPeriods' => sub { { borrowernumber => $patron->borrowernumber, type => 'SUSPENSION' } ); t::lib::Mocks::mock_preference( 'CumulativeRestrictionPeriods', '1' ); - AddIssue( $patron->unblessed, $item_1->barcode, $five_days_ago ); # Add an overdue - AddIssue( $patron->unblessed, $item_2->barcode, $ten_days_ago ) + AddIssue( $patron, $item_1->barcode, $five_days_ago ); # Add an overdue + AddIssue( $patron, $item_2->barcode, $ten_days_ago ) ; # Add another overdue AddReturn( $item_1->barcode, $library->{branchcode}, undef, $now ); $suspensions = $patron->restrictions->search( { type => 'SUSPENSION' } ); @@ -2892,7 +2905,7 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { } ); - AddIssue( $patron->unblessed, $item_1->barcode, $now->clone->subtract( days => 1 ) ); + AddIssue( $patron, $item_1->barcode, $now->clone->subtract( days => 1 ) ); my ( undef, $message ) = AddReturn( $item_1->barcode, $library->{branchcode}, undef, $now ); is( $message->{WasReturned} && exists $message->{ForeverDebarred}, 1, 'Forever debarred message for Addreturn when overdue'); @@ -2910,7 +2923,7 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { } ); - AddIssue( $patron->unblessed, $item_1->barcode, $now->clone->subtract( days => 1 ) ); + AddIssue( $patron, $item_1->barcode, $now->clone->subtract( days => 1 ) ); (undef, $message) = AddReturn( $item_1->barcode, $library->{branchcode}, undef, $now ); is( $message->{WasReturned} && exists $message->{PrevDebarred}, 1, 'Previously debarred message for Addreturn when overdue'); }; @@ -2947,7 +2960,7 @@ subtest 'CanBookBeIssued + AutoReturnCheckedOutItems' => sub { ); my ( $error, $question, $alerts ); - my $issue = AddIssue( $patron1->unblessed, $item->barcode ); + my $issue = AddIssue( $patron1, $item->barcode ); t::lib::Mocks::mock_preference('AutoReturnCheckedOutItems', 0); ( $error, $question, $alerts ) = CanBookBeIssued( $patron2, $item->barcode ); @@ -3008,25 +3021,25 @@ subtest 'AddReturn | is_overdue' => sub { my $ten_days_ago = $now->clone->subtract( days => 10 ); # No return date specified, today will be used => 10 days overdue charged - AddIssue( $patron->unblessed, $item->barcode, $ten_days_ago ); # date due was 10d ago + AddIssue( $patron, $item->barcode, $ten_days_ago ); # date due was 10d ago AddReturn( $item->barcode, $library->{branchcode} ); is( int($patron->account->balance()), 10, 'Patron should have a charge of 10 (10 days x 1)' ); Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; # specify return date 5 days before => no overdue charged - AddIssue( $patron->unblessed, $item->barcode, $five_days_ago ); # date due was 5d ago + AddIssue( $patron, $item->barcode, $five_days_ago ); # date due was 5d ago AddReturn( $item->barcode, $library->{branchcode}, undef, $ten_days_ago ); is( int($patron->account->balance()), 0, 'AddReturn: pass return_date => no overdue' ); Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; # specify return date 5 days later => 5 days overdue charged - AddIssue( $patron->unblessed, $item->barcode, $ten_days_ago ); # date due was 10d ago + AddIssue( $patron, $item->barcode, $ten_days_ago ); # date due was 10d ago AddReturn( $item->barcode, $library->{branchcode}, undef, $five_days_ago ); is( int($patron->account->balance()), 5, 'AddReturn: pass return_date => overdue' ); Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; # specify return date 5 days later, specify exemptfine => no overdue charge - AddIssue( $patron->unblessed, $item->barcode, $ten_days_ago ); # date due was 10d ago + AddIssue( $patron, $item->barcode, $ten_days_ago ); # date due was 10d ago AddReturn( $item->barcode, $library->{branchcode}, 1, $five_days_ago ); is( int($patron->account->balance()), 0, 'AddReturn: pass return_date => no overdue' ); Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; @@ -3035,7 +3048,7 @@ subtest 'AddReturn | is_overdue' => sub { plan tests => 3; - my $issue = AddIssue( $patron->unblessed, $item->barcode, $ten_days_ago ); # date due was 10d ago + my $issue = AddIssue( $patron, $item->barcode, $ten_days_ago ); # date due was 10d ago # Fake fines cronjob on this checkout my ($fine) = @@ -3073,7 +3086,7 @@ subtest 'AddReturn | is_overdue' => sub { t::lib::Mocks::mock_preference('CalculateFinesOnBackdate', 1); - my $issue = AddIssue( $patron->unblessed, $item->barcode, $one_day_ago ); # date due was 1d ago + my $issue = AddIssue( $patron, $item->barcode, $one_day_ago ); # date due was 1d ago # Fake fines cronjob on this checkout my ($fine) = @@ -3098,7 +3111,7 @@ subtest 'AddReturn | is_overdue' => sub { my $lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber }); is( $lines->count, 0, "Overdue fine accountline has been removed"); - $issue = AddIssue( $patron->unblessed, $item->barcode, $two_days_ago ); # date due was 2d ago + $issue = AddIssue( $patron, $item->barcode, $two_days_ago ); # date due was 2d ago # Fake fines cronjob on this checkout ($fine) = @@ -3164,7 +3177,7 @@ subtest 'AddReturn | is_overdue' => sub { t::lib::Mocks::mock_preference('CalculateFinesOnBackdate', 1); - my $issue = AddIssue( $patron->unblessed, $item->barcode, $one_day_ago ); # date due was 1d ago + my $issue = AddIssue( $patron, $item->barcode, $one_day_ago ); # date due was 1d ago # Fake fines cronjob on this checkout my ($fine) = @@ -3197,7 +3210,7 @@ subtest 'AddReturn | is_overdue' => sub { t::lib::Mocks::mock_preference( 'CalculateFinesOnBackdate', 1 ); my $due_date = dt_from_string; - my $issue = AddIssue( $patron->unblessed, $item->barcode, $due_date ); + my $issue = AddIssue( $patron, $item->barcode, $due_date ); # Add fine UpdateFine( @@ -3222,7 +3235,7 @@ subtest 'AddReturn | is_overdue' => sub { ok( !$accountline, 'accountline removed as expected' ); # Re-issue - $issue = AddIssue( $patron->unblessed, $item->barcode, $due_date ); + $issue = AddIssue( $patron, $item->barcode, $due_date ); # Add fine UpdateFine( @@ -3378,7 +3391,7 @@ subtest 'AddReturn | is_overdue' => sub { ); # Issue the item - my $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + my $issue = C4::Circulation::AddIssue( $patron, $item->barcode ); # Mark item as lost $item->itemlost(3)->store; @@ -3428,7 +3441,7 @@ subtest 'AddReturn | is_overdue' => sub { ); # Issue the item - $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + $issue = C4::Circulation::AddIssue( $patron, $item->barcode ); # Mark item as lost $item->itemlost(3)->store; @@ -3488,7 +3501,7 @@ subtest 'AddReturn | is_overdue' => sub { ); # Issue the item - $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode ); + $issue = C4::Circulation::AddIssue( $patron, $item->barcode ); # Mark item as lost $item->itemlost(3)->store; @@ -3553,7 +3566,7 @@ subtest 'AddReturn | is_overdue' => sub { ); # Issue the item - my $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode, $ten_days_ago ); + my $issue = C4::Circulation::AddIssue( $patron, $item->barcode, $ten_days_ago ); # Fake fines cronjob on this checkout my ($fine) = @@ -3636,7 +3649,7 @@ subtest 'AddReturn | is_overdue' => sub { ); # Issue the item - my $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode, $ten_days_ago ); + my $issue = C4::Circulation::AddIssue( $patron, $item->barcode, $ten_days_ago ); # Fake fines cronjob on this checkout my ($fine) = @@ -3718,7 +3731,7 @@ subtest 'AddReturn | is_overdue' => sub { ); # Issue the item - my $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode , $ten_days_ago); + my $issue = C4::Circulation::AddIssue( $patron, $item->barcode , $ten_days_ago); # Fake fines cronjob on this checkout my ($fine) = @@ -3815,7 +3828,7 @@ subtest 'AddReturn | is_overdue' => sub { ); # Issue the item - my $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode, $ten_days_ago ); + my $issue = C4::Circulation::AddIssue( $patron, $item->barcode, $ten_days_ago ); # Fake fines cronjob on this checkout my ($fine) = @@ -4168,7 +4181,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); @@ -4460,7 +4473,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, @@ -4474,7 +4487,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, @@ -4488,7 +4501,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, @@ -4566,7 +4579,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( @@ -4643,7 +4656,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( { borrowernumber => $patron->id, @@ -4674,7 +4687,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 @@ -4766,7 +4779,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, @@ -4795,7 +4808,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, @@ -4834,7 +4847,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, @@ -4863,7 +4876,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" ); @@ -4903,7 +4916,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, @@ -4931,7 +4944,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, @@ -4959,7 +4972,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, @@ -5081,7 +5094,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( { @@ -5096,7 +5109,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' @@ -5202,7 +5215,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 ); @@ -5237,7 +5250,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 ); @@ -5272,7 +5285,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 ); @@ -5307,7 +5320,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 ); @@ -5395,7 +5408,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 ); @@ -5409,7 +5422,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; @@ -5431,7 +5444,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 ); @@ -5445,7 +5458,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; @@ -5466,7 +5479,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 ); @@ -5480,7 +5493,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; @@ -5501,7 +5514,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 ); @@ -5516,7 +5529,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; @@ -5602,7 +5615,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; @@ -5645,14 +5658,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" ); }; @@ -5680,9 +5693,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" ); }; @@ -5759,7 +5772,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', @@ -5773,7 +5786,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', @@ -5807,7 +5820,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 @@ -5895,6 +5908,7 @@ subtest "CanBookBeIssued + needsconfirmation message" => sub { priority => 1, found => undef, suspend => 0, + item_group_id => undef }}); my ( $error, $needsconfirmation, $alerts, $messages ); 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..9add304139 100755 --- a/t/db_dependent/Circulation/CheckIfIssuedToPatron.t +++ b/t/db_dependent/Circulation/CheckIfIssuedToPatron.t @@ -66,11 +66,27 @@ 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 +107,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 +123,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 fc939a2486..ee66ff558a 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/_CalculateAndUpdateFine.t b/t/db_dependent/Circulation/_CalculateAndUpdateFine.t index 7e9b98bc14..701e3bfb56 100755 --- a/t/db_dependent/Circulation/_CalculateAndUpdateFine.t +++ b/t/db_dependent/Circulation/_CalculateAndUpdateFine.t @@ -48,8 +48,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}, @@ -110,26 +110,26 @@ subtest 'HomeOrHoldingBranch is used' => sub { my $return_date = dt_from_string()->add( days => 1 )->set( hour => 23, minute => 59, second => 0 ); C4::Circulation::_CalculateAndUpdateFine( { - borrower => $patron, + borrower => $patron->unblessed, issue => $issue, item => $item->unblessed, return_date => $return_date, } ); - my $fines = Koha::Account::Lines->search({ borrowernumber => $patron->{borrowernumber} }); + my $fines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber }); is($fines->total_outstanding, 0, "Fine is not accrued for holdingbranch"); t::lib::Mocks::mock_preference('HomeOrHoldingBranch', 'homebranch'); C4::Circulation::_CalculateAndUpdateFine( { - borrower => $patron, + borrower => $patron->unblessed, issue => $issue, item => $item->unblessed, return_date => $return_date, } ); - $fines = Koha::Account::Lines->search({ borrowernumber => $patron->{borrowernumber} }); + $fines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber }); is($fines->total_outstanding, 1, "Fine is accrued for homebranch"); }; 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 8315a11866..e222f33736 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 @@ -271,7 +269,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( { @@ -296,7 +294,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( { @@ -320,7 +318,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( @@ -401,7 +399,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" ); @@ -487,7 +485,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} ); @@ -536,7 +534,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' ); @@ -559,7 +557,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 94311487d1..614a32303a 100755 --- a/t/db_dependent/Circulation/maxsuspensiondays.t +++ b/t/db_dependent/Circulation/maxsuspensiondays.t @@ -42,14 +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 $patron_object = Koha::Patrons->find( $borrowernumber ); -my $borrower = $patron_object->unblessed; +})->store; my $record = MARC::Record->new(); $record->append_fields( @@ -75,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 = $patron_object->restrictions; +my $debarments = $patron->restrictions; my $THE_debarment = $debarments->next; is( $THE_debarment->expiration, @@ -99,9 +97,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 = $patron_object->restrictions; +$debarments = $patron->restrictions; $THE_debarment = $debarments->next; is( $THE_debarment->expiration, @@ -130,7 +128,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' ) ); @@ -156,7 +154,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 0fb342d3db..26486b156a 100755 --- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t +++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t @@ -113,7 +113,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" ); @@ -121,7 +121,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 ); $cache->flush(); $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblionumber, patron => $patron1 } ); @@ -464,7 +464,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 ); $cache->flush(); $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblio2->biblionumber, patron => $patron1 } ); 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/HoldsQueue.t b/t/db_dependent/HoldsQueue.t index 0286a33d8d..7875c69eed 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -1840,7 +1840,7 @@ subtest 'Remove holds on check-in match' => sub { t::lib::Mocks::mock_userenv( { branchcode => $lib->branchcode } ); - AddIssue( $patron1->unblessed, $item->barcode, dt_from_string ); + AddIssue( $patron1, $item->barcode, dt_from_string ); my $hold_id = AddReserve( { 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 c2e54ec987..ad2245ec4e 100755 --- a/t/db_dependent/Illrequests.t +++ b/t/db_dependent/Illrequests.t @@ -1610,7 +1610,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 abd314e419..53e596bc09 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, @@ -703,7 +704,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, @@ -816,14 +817,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 6895ea5bfa..a7f681847d 100755 --- a/t/db_dependent/Koha/Biblio.t +++ b/t/db_dependent/Koha/Biblio.t @@ -904,8 +904,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 }); @@ -1044,7 +1044,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" ); @@ -1063,8 +1063,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 8464ab2ce5..5a51592e17 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; @@ -1485,12 +1485,12 @@ subtest 'checkout' => sub { is( $checkout, undef, 'Koha::Item->checkout should return undef if there is no current checkout on this item' ); # Add a checkout - my $patron = $builder->build({ source => 'Borrower' }); + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); C4::Circulation::AddIssue( $patron, $item->barcode ); $checkout = $retrieved_item_1->checkout; is( ref( $checkout ), 'Koha::Checkout', 'Koha::Item->checkout should return a Koha::Checkout' ); is( $checkout->itemnumber, $item->itemnumber, 'Koha::Item->checkout should return the correct checkout' ); - is( $checkout->borrowernumber, $patron->{borrowernumber}, 'Koha::Item->checkout should return the correct checkout' ); + is( $checkout->borrowernumber, $patron->borrowernumber, 'Koha::Item->checkout should return the correct checkout' ); # Do the return C4::Circulation::AddReturn( $item->barcode ); diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t index 1624cd43e7..b0ab60447b 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 89ce426187..880ec2c5a8 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' ); @@ -1495,11 +1491,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 9fa7f7c22f..89587f5de7 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/Plugins/Recall_hooks.t b/t/db_dependent/Koha/Plugins/Recall_hooks.t index 19624f1211..680aaa1362 100755 --- a/t/db_dependent/Koha/Plugins/Recall_hooks.t +++ b/t/db_dependent/Koha/Plugins/Recall_hooks.t @@ -80,7 +80,7 @@ subtest 'after_recall_action hook' => sub { } }); - C4::Circulation::AddIssue( $patron2->unblessed, $item->barcode ); + C4::Circulation::AddIssue( $patron2, $item->barcode ); warning_like { Koha::Recalls->add_recall({ 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/Recall.t b/t/db_dependent/Koha/Recall.t index f36e328ce5..1b4f8f70eb 100755 --- a/t/db_dependent/Koha/Recall.t +++ b/t/db_dependent/Koha/Recall.t @@ -65,7 +65,7 @@ Koha::CirculationRules->set_rule({ }); my $overdue_date = dt_from_string->subtract( days => 4 ); -C4::Circulation::AddIssue( $patron2->unblessed, $item1->barcode, $overdue_date ); +C4::Circulation::AddIssue( $patron2, $item1->barcode, $overdue_date ); my $recall1 = Koha::Recall->new({ patron_id => $patron1->borrowernumber, @@ -160,7 +160,7 @@ is( $recall3->item_id, $item1->itemnumber, "Item persists for item-level recall" $recall3->set_fulfilled; ok( $recall3->fulfilled, "Recall has been fulfilled" ); -C4::Circulation::AddIssue( $patron2->unblessed, $item1->barcode ); +C4::Circulation::AddIssue( $patron2, $item1->barcode ); my $recall4 = Koha::Recall->new({ patron_id => $patron1->borrowernumber, created_date => dt_from_string, diff --git a/t/db_dependent/Koha/Recalls.t b/t/db_dependent/Koha/Recalls.t index a32ed768cb..546bbf21c4 100755 --- a/t/db_dependent/Koha/Recalls.t +++ b/t/db_dependent/Koha/Recalls.t @@ -65,8 +65,8 @@ Koha::CirculationRules->set_rules({ } }); -C4::Circulation::AddIssue( $patron3->unblessed, $item1->barcode ); -C4::Circulation::AddIssue( $patron3->unblessed, $item2->barcode ); +C4::Circulation::AddIssue( $patron3, $item1->barcode ); +C4::Circulation::AddIssue( $patron3, $item2->barcode ); my ( $recall, $due_interval, $due_date ) = Koha::Recalls->add_recall({ patron => undef, 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 8c7f38c979..7c38aef3d2 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, ''); @@ -86,37 +84,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)'); SKIP: { @@ -124,7 +122,7 @@ SKIP: { 5 unless can_run('weasyprint'); isnt( - Koha::Patron::Discharge::generate_as_pdf( { borrowernumber => $patron->{borrowernumber} } ), + Koha::Patron::Discharge::generate_as_pdf( { borrowernumber => $patron->borrowernumber } ), undef, "Temporary PDF generated." ); @@ -135,7 +133,7 @@ SKIP: { my $result; warning_is - { $result = Koha::Patron::Discharge::generate_as_pdf( { borrowernumber => $patron->{borrowernumber} } ); } + { $result = Koha::Patron::Discharge::generate_as_pdf( { borrowernumber => $patron->borrowernumber } ); } 'Some error', 'Failed call to run() prints the generated error'; @@ -144,7 +142,7 @@ SKIP: { $mocked_ipc->mock( 'can_run', undef ); warning_is - { $result = Koha::Patron::Discharge::generate_as_pdf( { borrowernumber => $patron->{borrowernumber} } ); } + { $result = Koha::Patron::Discharge::generate_as_pdf( { borrowernumber => $patron->borrowernumber } ); } 'weasyprint not found!', 'Expected failure because of missing weasyprint'; @@ -152,7 +150,7 @@ SKIP: { } # 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; @@ -161,17 +159,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/Reserves.t b/t/db_dependent/Reserves.t index 2a6ed48120..ce5c661ea0 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -1156,7 +1156,7 @@ subtest 'RevertWaitingStatus' => sub { my $hold_4 = place_item_hold( $patron_4, $item_1, $library, $priority ); $hold_1->set_waiting; - AddIssue( $patron_3->unblessed, $item_1->barcode, undef, 'revert' ); + AddIssue( $patron_3, $item_1->barcode, undef, 'revert' ); my $holds = $biblio->holds; is( $holds->count, 3, 'One hold has been deleted' ); @@ -1320,7 +1320,7 @@ subtest 'AllowHoldOnPatronPossession test' => sub { my $patron = $builder->build_object({ class => "Koha::Patrons", value => { branchcode => $item->homebranch }}); - C4::Circulation::AddIssue($patron->unblessed, + C4::Circulation::AddIssue($patron, $item->barcode); t::lib::Mocks::mock_preference('AllowHoldsOnPatronsPossessions', 0); diff --git a/t/db_dependent/Reserves/GetReserveFee.t b/t/db_dependent/Reserves/GetReserveFee.t index ce00290c0b..03029d8333 100755 --- a/t/db_dependent/Reserves/GetReserveFee.t +++ b/t/db_dependent/Reserves/GetReserveFee.t @@ -59,23 +59,23 @@ $builder->build({ reservefee => 0, }, }); -my $patron1 = $builder->build({ - source => 'Borrower', +my $patron1 = $builder->build_object({ + class => 'Koha::Patrons', value => { categorycode => 'XYZ1', }, }); -my $patron2 = $builder->build({ - source => 'Borrower', +my $patron2 = $builder->build_object({ + class => 'Koha::Patrons', value => { categorycode => 'XYZ1', }, }); -my $patron3 = $builder->build({ - source => 'Borrower', +my $patron3 = $builder->build_object({ + class => 'Koha::Patrons', }); -my $patron4 = $builder->build({ - source => 'Borrower', +my $patron4 = $builder->build_object({ + class => 'Koha::Patrons', value => { categorycode => 'XYZ2', }, @@ -101,30 +101,30 @@ subtest 'GetReserveFee' => sub { C4::Circulation::AddIssue( $patron1, $item1->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter C4::Circulation::AddIssue( $patron3, $item2->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter - my $acc2 = acctlines( $patron2->{borrowernumber} ); - my $res1 = addreserve( $patron1->{borrowernumber} ); + my $acc2 = acctlines( $patron2->borrowernumber ); + my $res1 = addreserve( $patron1->borrowernumber ); t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always'); - my $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber ); + my $fee = C4::Reserves::GetReserveFee( $patron2->borrowernumber, $biblio->biblionumber ); is( $fee > 0, 1, 'Patron 2 should be charged cf GetReserveFee' ); - C4::Reserves::ChargeReserveFee( $patron2->{borrowernumber}, $fee, $biblio->title ); - is( acctlines( $patron2->{borrowernumber} ), $acc2 + 1, 'Patron 2 has been charged by ChargeReserveFee' ); + C4::Reserves::ChargeReserveFee( $patron2->borrowernumber, $fee, $biblio->title ); + is( acctlines( $patron2->borrowernumber ), $acc2 + 1, 'Patron 2 has been charged by ChargeReserveFee' ); # If we delete the reserve, there should be no charge - $dbh->do( "DELETE FROM reserves WHERE borrowernumber = ?", undef, ( $patron1->{borrowernumber}) ); - $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber ); + $dbh->do( "DELETE FROM reserves WHERE borrowernumber = ?", undef, ( $patron1->borrowernumber) ); + $fee = C4::Reserves::GetReserveFee( $patron2->borrowernumber, $biblio->biblionumber ); is( $fee, 0, 'HoldFeeMode=not_always, Patron 2 should not be charged' ); t::lib::Mocks::mock_preference('HoldFeeMode', 'any_time_is_placed'); - $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber ); + $fee = C4::Reserves::GetReserveFee( $patron2->borrowernumber, $biblio->biblionumber ); is( int($fee), 2, 'HoldFeeMode=any_time_is_placed, Patron 2 should be charged' ); t::lib::Mocks::mock_preference('HoldFeeMode', 'any_time_is_collected'); - $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->biblionumber ); + $fee = C4::Reserves::GetReserveFee( $patron2->borrowernumber, $biblio->biblionumber ); is( int($fee), 2, 'HoldFeeMode=any_time_is_collected, Patron 2 should be charged' ); t::lib::Mocks::mock_preference('HoldFeeMode', 'any_time_is_placed'); - $fee = C4::Reserves::GetReserveFee( $patron4->{borrowernumber}, $biblio->biblionumber ); + $fee = C4::Reserves::GetReserveFee( $patron4->borrowernumber, $biblio->biblionumber ); is( $fee, 0, 'HoldFeeMode=any_time_is_placed ; fee == 0, Patron 4 should not be charged' ); }; @@ -138,21 +138,21 @@ subtest 'Integration with AddReserve' => sub { t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always'); $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); - $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); - addreserve( $patron1->{borrowernumber} ); - is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - No fee charged for patron 1 if not issued' ); + $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->borrowernumber ); + addreserve( $patron1->borrowernumber ); + is( acctlines( $patron1->borrowernumber ), 0, 'not_always - No fee charged for patron 1 if not issued' ); t::lib::Mocks::mock_preference('HoldFeeMode', 'any_time_is_placed'); $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); - $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); - addreserve( $patron1->{borrowernumber} ); - is( acctlines( $patron1->{borrowernumber} ), 1, 'any_time_is_placed - Patron should be always charged' ); + $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->borrowernumber ); + addreserve( $patron1->borrowernumber ); + is( acctlines( $patron1->borrowernumber ), 1, 'any_time_is_placed - Patron should be always charged' ); t::lib::Mocks::mock_preference('HoldFeeMode', 'any_time_is_collected'); $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); - $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); - addreserve( $patron1->{borrowernumber} ); - is( acctlines( $patron1->{borrowernumber} ), 0, 'any_time_is_collected - Patron should not be charged when placing a hold' ); + $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->borrowernumber ); + addreserve( $patron1->borrowernumber ); + is( acctlines( $patron1->borrowernumber ), 0, 'any_time_is_collected - Patron should not be charged when placing a hold' ); }; subtest 'Items are issued' => sub { @@ -164,56 +164,56 @@ subtest 'Integration with AddReserve' => sub { t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always'); $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); - $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); - addreserve( $patron1->{borrowernumber} ); - is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if items are not all checked out' ); + $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->borrowernumber ); + addreserve( $patron1->borrowernumber ); + is( acctlines( $patron1->borrowernumber ), 0, 'not_always - Patron should not be charged if items are not all checked out' ); $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); - $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); - addreserve( $patron3->{borrowernumber} ); - addreserve( $patron1->{borrowernumber} ); - is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if all the items are not checked out, even if 1 hold is already placed' ); + $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->borrowernumber ); + addreserve( $patron3->borrowernumber ); + addreserve( $patron1->borrowernumber ); + is( acctlines( $patron1->borrowernumber ), 0, 'not_always - Patron should not be charged if all the items are not checked out, even if 1 hold is already placed' ); C4::Circulation::AddIssue( $patron3, $item2->barcode, '2015-12-31', 0, undef, 0, {} ); $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); - $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); - addreserve( $patron1->{borrowernumber} ); - is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if all items are checked out but no holds are placed' ); + $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->borrowernumber ); + addreserve( $patron1->borrowernumber ); + is( acctlines( $patron1->borrowernumber ), 0, 'not_always - Patron should not be charged if all items are checked out but no holds are placed' ); $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); - $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); - addreserve( $patron3->{borrowernumber} ); - addreserve( $patron1->{borrowernumber} ); - is( acctlines( $patron1->{borrowernumber} ), 1, 'not_always - Patron should only be charged if all items are checked out and at least 1 hold is already placed' ); + $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->borrowernumber ); + addreserve( $patron3->borrowernumber ); + addreserve( $patron1->borrowernumber ); + is( acctlines( $patron1->borrowernumber ), 1, 'not_always - Patron should only be charged if all items are checked out and at least 1 hold is already placed' ); }; }; subtest 'Integration with AddIssue' => sub { plan tests => 5; - $dbh->do( "DELETE FROM issues WHERE borrowernumber = ?", undef, $patron1->{borrowernumber} ); + $dbh->do( "DELETE FROM issues WHERE borrowernumber = ?", undef, $patron1->borrowernumber ); $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); - $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); + $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->borrowernumber ); t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always'); C4::Circulation::AddIssue( $patron1, $item1->barcode, '2015-12-31', 0, undef, 0, {} ); - is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged' ); + is( acctlines( $patron1->borrowernumber ), 0, 'not_always - Patron should not be charged' ); t::lib::Mocks::mock_preference('HoldFeeMode', 'any_time_is_placed'); - $dbh->do( "DELETE FROM issues WHERE borrowernumber = ?", undef, $patron1->{borrowernumber} ); + $dbh->do( "DELETE FROM issues WHERE borrowernumber = ?", undef, $patron1->borrowernumber ); C4::Circulation::AddIssue( $patron1, $item1->barcode, '2015-12-31', 0, undef, 0, {} ); - is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged' ); + is( acctlines( $patron1->borrowernumber ), 0, 'not_always - Patron should not be charged' ); t::lib::Mocks::mock_preference('HoldFeeMode', 'any_time_is_collected'); - $dbh->do( "DELETE FROM issues WHERE borrowernumber = ?", undef, $patron1->{borrowernumber} ); + $dbh->do( "DELETE FROM issues WHERE borrowernumber = ?", undef, $patron1->borrowernumber ); C4::Circulation::AddIssue( $patron1, $item1->barcode, '2015-12-31', 0, undef, 0, {} ); - is( acctlines( $patron1->{borrowernumber} ), 0, 'any_time_is_collected - Patron should not be charged when checking out an item which was not placed hold for him' ); + is( acctlines( $patron1->borrowernumber ), 0, 'any_time_is_collected - Patron should not be charged when checking out an item which was not placed hold for him' ); - $dbh->do( "DELETE FROM issues WHERE borrowernumber = ?", undef, $patron1->{borrowernumber} ); - my $id = addreserve( $patron1->{borrowernumber} ); - is( acctlines( $patron1->{borrowernumber} ), 0, 'any_time_is_collected - Patron should not be charged yet (just checking to make sure)'); + $dbh->do( "DELETE FROM issues WHERE borrowernumber = ?", undef, $patron1->borrowernumber ); + my $id = addreserve( $patron1->borrowernumber ); + is( acctlines( $patron1->borrowernumber ), 0, 'any_time_is_collected - Patron should not be charged yet (just checking to make sure)'); C4::Circulation::AddIssue( $patron1, $item1->barcode, '2015-12-31', 0, undef, 0, {} ); - is( acctlines( $patron1->{borrowernumber} ), 1, 'any_time_is_collected - Patron should not be charged when checking out an item which was not placed hold for him' ); + is( acctlines( $patron1->borrowernumber ), 1, 'any_time_is_collected - Patron should not be charged when checking out an item which was not placed hold for him' ); }; sub acctlines { #calculate number of accountlines for a patron diff --git a/t/db_dependent/SIP/ILS.t b/t/db_dependent/SIP/ILS.t index 3bc5f63146..4b62dc051a 100755 --- a/t/db_dependent/SIP/ILS.t +++ b/t/db_dependent/SIP/ILS.t @@ -199,7 +199,7 @@ subtest cancel_waiting_hold => 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( @@ -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/api/v1/biblios.t b/t/db_dependent/api/v1/biblios.t index 53aeaaaa94..cb4f02bb1e 100755 --- a/t/db_dependent/api/v1/biblios.t +++ b/t/db_dependent/api/v1/biblios.t @@ -686,8 +686,8 @@ subtest 'get_checkouts() tests' => sub { my $item_1 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber }); my $item_2 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber }); - AddIssue( $patron->unblessed, $item_1->barcode ); - AddIssue( $patron->unblessed, $item_2->barcode ); + AddIssue( $patron, $item_1->barcode ); + AddIssue( $patron, $item_2->barcode ); my $ret = $t->get_ok( "//$userid:$password@/api/v1/biblios/" . $biblio->biblionumber . "/checkouts") ->status_is(200) diff --git a/t/db_dependent/api/v1/checkouts.t b/t/db_dependent/api/v1/checkouts.t index 1d83f8916b..cd62c00fde 100755 --- a/t/db_dependent/api/v1/checkouts.t +++ b/t/db_dependent/api/v1/checkouts.t @@ -89,13 +89,13 @@ my $item3 = $builder->build_sample_item; my $item4 = $builder->build_sample_item; my $date_due = DateTime->now->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($librarian->unblessed, $item3->barcode, $date_due); +my $issue3 = C4::Circulation::AddIssue($librarian, $item3->barcode, $date_due); my $date_due3 = Koha::DateUtils::dt_from_string( $issue3->date_due ); -my $issue4 = C4::Circulation::AddIssue($patron->unblessed, $item4->barcode); +my $issue4 = C4::Circulation::AddIssue($patron, $item4->barcode); C4::Circulation::AddReturn($item4->barcode, $branchcode); $t->get_ok( "//$userid:$password@/api/v1/checkouts?patron_id=$patron_id" ) diff --git a/t/db_dependent/api/v1/return_claims.t b/t/db_dependent/api/v1/return_claims.t index a9877f7946..a79dd9e73f 100755 --- a/t/db_dependent/api/v1/return_claims.t +++ b/t/db_dependent/api/v1/return_claims.t @@ -62,7 +62,7 @@ subtest 'claim_returned() tests' => sub { t::lib::Mocks::mock_userenv({ branchcode => $librarian->branchcode }); my $item = $builder->build_sample_item; - my $issue = AddIssue( $patron->unblessed, $item->barcode, dt_from_string->add( weeks => 2 ) ); + my $issue = AddIssue( $patron, $item->barcode, dt_from_string->add( weeks => 2 ) ); t::lib::Mocks::mock_preference( 'ClaimReturnedChargeFee', 'ask' ); t::lib::Mocks::mock_preference( 'ClaimReturnedLostValue', '99' ); @@ -131,7 +131,7 @@ subtest 'update_notes() tests' => sub { t::lib::Mocks::mock_userenv( { branchcode => $item->homebranch } ) ; # needed by AddIssue - my $issue = AddIssue( $librarian->unblessed, $item->barcode, + my $issue = AddIssue( $librarian, $item->barcode, dt_from_string->add( weeks => 2 ) ); my $claim = $issue->claim_returned( @@ -197,7 +197,7 @@ subtest 'resolve_claim() tests' => sub { my $ClaimReturnedLostValue = 1; t::lib::Mocks::mock_preference('ClaimReturnedLostValue', $ClaimReturnedLostValue); - my $issue = AddIssue( $librarian->unblessed, $item->barcode, dt_from_string->add( weeks => 2 ) ); + my $issue = AddIssue( $librarian, $item->barcode, dt_from_string->add( weeks => 2 ) ); my $claim = $issue->claim_returned( { @@ -278,7 +278,7 @@ subtest 'delete() tests' => sub { t::lib::Mocks::mock_userenv({ branchcode => $item->homebranch }); - my $issue = C4::Circulation::AddIssue( $librarian->unblessed, + my $issue = C4::Circulation::AddIssue( $librarian, $item->barcode, dt_from_string->add( weeks => 2 ) ); my $claim = $issue->claim_returned( 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 66d169c1eb..04aed37e29 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.39.5