From e1a6e8e6d615eb2bbd41c5c4828e92c0a2f39759 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 9 Jan 2017 09:00:10 +0100 Subject: [PATCH] Bug 16413: Change prototype of GetLatestAutoRenewDate Currently the 2 GetLatestAutoRenewDate and GetSoonestRenewDate subroutines takes $borrowernumber and $itemnumber in parameter. They refetch the patron, item and issue information but they already are available from where they are called. It would make sense to change the prototype of these 2 subroutines to accept the patron, item and issue information directly to avoid unnecessary refetches. Test plan: Make sure this change does not introduce regressions on bug 15581 and bug 16344. --- C4/Circulation.pm | 28 ++++++++++++---------------- circ/renew.pl | 7 +++++-- t/db_dependent/Circulation.t | 13 +++++++------ 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 6e990aa..aa3dba3 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3028,14 +3028,15 @@ sub GetSoonestRenewDate { =head2 GetLatestAutoRenewDate - $NoAutoRenewalAfterThisDate = &GetLatestAutoRenewDate($borrowernumber, $itemnumber); + $NoAutoRenewalAfterThisDate = &GetLatestAutoRenewDate({ patron => $patron, issue => $issue, item => $item }); Find out the latest possible auto renew date of a borrowed item. -C<$borrowernumber> is the borrower number of the patron who currently -has the item on loan. +C<$borrower> is the patron who currencly has the item on loan. -C<$itemnumber> is the number of the item to renew. +C<$item> is the item to renew. + +C<$issue> is the issue to renew. C<$GetLatestAutoRenewDate> returns the DateTime of the latest possible auto renew date, based on the value "No auto renewal after" and the "No auto @@ -3046,20 +3047,15 @@ or item cannot be found. =cut sub GetLatestAutoRenewDate { - my ( $borrowernumber, $itemnumber ) = @_; + my ( $params ) = @_; - my $dbh = C4::Context->dbh; + my $patron = $params->{patron} or return; + my $item = $params->{item} or return; + my $issue = $params->{issue} or return; - my $item = GetItem($itemnumber) or return; - my $itemissue = GetItemIssue($itemnumber) or return; - - $borrowernumber ||= $itemissue->{borrowernumber}; - my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ) - or return; - - my $branchcode = _GetCircControlBranch( $item, $borrower ); + my $branchcode = _GetCircControlBranch( $item, $patron ); my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule( - { categorycode => $borrower->{categorycode}, + { categorycode => $patron->{categorycode}, itemtype => $item->{itype}, branchcode => $branchcode } @@ -3072,7 +3068,7 @@ sub GetLatestAutoRenewDate { my $maximum_renewal_date; if ( $issuing_rule->no_auto_renewal_after ) { - $maximum_renewal_date = dt_from_string($itemissue->{issuedate}); + $maximum_renewal_date = dt_from_string($issue->{issuedate}); $maximum_renewal_date->add( $issuing_rule->lengthunit => $issuing_rule->no_auto_renewal_after ); diff --git a/circ/renew.pl b/circ/renew.pl index 2b4114e..1afb6d2a 100755 --- a/circ/renew.pl +++ b/circ/renew.pl @@ -84,8 +84,11 @@ if ($barcode) { } if ( $error && ( $error eq 'auto_too_late' ) ) { $latest_auto_renew_date = C4::Circulation::GetLatestAutoRenewDate( - $borrower->borrowernumber(), - $item->itemnumber(), + { + patron => $borrower->unblessed, + item => $item->unblessed, + issue => $issue->unblessed, + } ); } if ($can_renew) { diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index b33489f..faa8fcc 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -638,33 +638,34 @@ C4::Context->dbh->do("DELETE FROM accountlines"); my $ten_days_before = dt_from_string->add( days => -10 ); my $ten_days_ahead = dt_from_string->add( days => 10 ); - AddIssue( $renewing_borrower, $item_to_auto_renew->{barcode}, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + my $issue = AddIssue( $renewing_borrower, $item_to_auto_renew->{barcode}, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + $issue->result_class('DBIx::Class::ResultClass::HashRefInflator'); $dbh->do('UPDATE issuingrules SET norenewalbefore = 7, no_auto_renewal_after = "", no_auto_renewal_after_hard_limit = NULL'); - my $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + my $latest_auto_renew_date = GetLatestAutoRenewDate( { patron => $renewing_borrower, item => $item_to_auto_renew, issue => { issuedate => $issue->issuedate }, } ); is( $latest_auto_renew_date, undef, 'GetLatestAutoRenewDate should return undef if no_auto_renewal_after or no_auto_renewal_after_hard_limit are not defined' ); my $five_days_before = dt_from_string->add( days => -5 ); $dbh->do('UPDATE issuingrules SET norenewalbefore = 10, no_auto_renewal_after = 5, no_auto_renewal_after_hard_limit = NULL'); - $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + $latest_auto_renew_date = GetLatestAutoRenewDate( { patron => $renewing_borrower, item => $item_to_auto_renew, issue => { issuedate => $issue->issuedate }, } ); is( $latest_auto_renew_date->truncate( to => 'minute' ), $five_days_before->truncate( to => 'minute' ), 'GetLatestAutoRenewDate should return -5 days if no_auto_renewal_after = 5 and date_due is 10 days before' ); my $five_days_ahead = dt_from_string->add( days => 5 ); $dbh->do('UPDATE issuingrules SET norenewalbefore = 10, no_auto_renewal_after = 15, no_auto_renewal_after_hard_limit = NULL'); - $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + $latest_auto_renew_date = GetLatestAutoRenewDate( { patron => $renewing_borrower, item => $item_to_auto_renew, issue => { issuedate => $issue->issuedate }, } ); is( $latest_auto_renew_date->truncate( to => 'minute' ), $five_days_ahead->truncate( to => 'minute' ), 'GetLatestAutoRenewDate should return +5 days if no_auto_renewal_after = 15 and date_due is 10 days before' ); my $two_days_ahead = dt_from_string->add( days => 2 ); $dbh->do('UPDATE issuingrules SET norenewalbefore = 10, no_auto_renewal_after = "", no_auto_renewal_after_hard_limit = ?', undef, dt_from_string->add( days => 2 ) ); - $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + $latest_auto_renew_date = GetLatestAutoRenewDate( { patron => $renewing_borrower, item => $item_to_auto_renew, issue => { issuedate => $issue->issuedate }, } ); is( $latest_auto_renew_date->truncate( to => 'day' ), $two_days_ahead->truncate( to => 'day' ), 'GetLatestAutoRenewDate should return +2 days if no_auto_renewal_after_hard_limit is defined and not no_auto_renewal_after' ); $dbh->do('UPDATE issuingrules SET norenewalbefore = 10, no_auto_renewal_after = 15, no_auto_renewal_after_hard_limit = ?', undef, dt_from_string->add( days => 2 ) ); - $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + $latest_auto_renew_date = GetLatestAutoRenewDate( { patron => $renewing_borrower, item => $item_to_auto_renew, issue => { issuedate => $issue->issuedate }, } ); is( $latest_auto_renew_date->truncate( to => 'day' ), $two_days_ahead->truncate( to => 'day' ), 'GetLatestAutoRenewDate should return +2 days if no_auto_renewal_after_hard_limit is < no_auto_renewal_after' -- 2.1.4