From 383768d25321d2d74683e4f006b869b4167c2879 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 2 May 2016 13:50:15 +0100 Subject: [PATCH] Bug 16413: Change prototype 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 | 31 ++++++++++++------------------- circ/renew.pl | 7 +++++-- t/db_dependent/Circulation.t | 13 +++++++------ 3 files changed, 24 insertions(+), 27 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 32eaeba..7ac097d 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3224,14 +3224,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 @@ -3242,30 +3243,22 @@ or item cannot be found. =cut sub GetLatestAutoRenewDate { - my ( $borrowernumber, $itemnumber ) = @_; - - my $dbh = C4::Context->dbh; - - my $item = GetItem($itemnumber) or return; - my $itemissue = GetItemIssue($itemnumber) or return; + my ( $params ) = @_; - $borrowernumber ||= $itemissue->{borrowernumber}; - my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ) - or return; + my $patron = $params->{patron} or return; + my $item = $params->{item} or return; + my $issue = $params->{issue} or return; - my $branchcode = _GetCircControlBranch( $item, $borrower ); + my $branchcode = _GetCircControlBranch( $item, $patron ); my $issuingrule = - GetIssuingRule( $borrower->{categorycode}, $item->{itype}, $branchcode ); - - my $now = dt_from_string; + GetIssuingRule( $patron->{categorycode}, $item->{itype}, $branchcode ); return if ( not $issuingrule->{no_auto_renewal_after} or $issuingrule->{no_auto_renewal_after} eq '' ) and ( not $issuingrule->{no_auto_renewal_after_hard_limit} or $issuingrule->{no_auto_renewal_after_hard_limit} eq '' ); - my $maximum_renewal_date; if ( $issuingrule->{no_auto_renewal_after} ) { - $maximum_renewal_date = dt_from_string($itemissue->{issuedate}); + $maximum_renewal_date = dt_from_string($issue->{issuedate}); $maximum_renewal_date->add( $issuingrule->{lengthunit} => $issuingrule->{no_auto_renewal_after} ); diff --git a/circ/renew.pl b/circ/renew.pl index 2b4114e..42b7c3c 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(), + { + borrower => $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 eefae63..38402f3 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -605,33 +605,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.7.0