From 79ea151c21572c89881e854b3bf4ed9e6c832e5b Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 16 Feb 2024 14:57:33 +0000 Subject: [PATCH] Bug 23415: (QA follow-up) Move to Circulation::CanBookBeRenewed This moves the basic calculation for too_much_oweing out of the controller and into the class method. Signed-off-by: Martin Renvoize --- C4/Circulation.pm | 10 +++++++--- circ/renew.pl | 15 +++++++-------- .../intranet-tmpl/prog/en/modules/circ/renew.tt | 2 +- t/db_dependent/Circulation.t | 11 ++++++++++- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 25a3b8c3aa3..fd6c48e095f 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3048,15 +3048,19 @@ sub CanBookBeRenewed { $final_unseen_renewal = ( C4::Context->preference('UnseenRenewals') && $issuing_rule->{unseen_renewals_allowed} == ( $issue->unseen_renewals + 1 ) ) ? 1 : 0; - my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing'); + my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing'); + my $finesblockrenewing = C4::Context->preference("FineNoRenewals"); my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing'); - my $restricted = $patron->is_debarred; - my $hasoverdues = $patron->has_overdues; + my $restricted = $patron->is_debarred; + my $hasoverdues = $patron->has_overdues; + my $balance = $patron->account->balance; if ( $restricted and $restrictionblockrenewing ) { return ( 0, 'restriction'); } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($issue->is_overdue and $overduesblockrenewing eq 'blockitem') ) { return ( 0, 'overdue'); + } elsif ( $finesblockrenewing && $balance > $finesblockrenewing ) { + return ( 0, 'too_much_oweing'); } } diff --git a/circ/renew.pl b/circ/renew.pl index 28701e78368..170381a52a9 100755 --- a/circ/renew.pl +++ b/circ/renew.pl @@ -67,9 +67,6 @@ if ($op eq 'cud-renew' && $barcode) { $patron = $checkout->patron; - $balance = $patron->account->balance; - my $amountlimit = C4::Context->preference("FineNoRenewals"); - if ( ( $patron->is_debarred || q{} ) lt dt_from_string()->ymd() ) { my $can_renew; my $info; @@ -96,12 +93,15 @@ if ($op eq 'cud-renew' && $barcode) { ); } - if ( $balance > $amountlimit ) { - $error = "too_much_debt"; - $can_renew = 0; + if ( $error && ($error eq 'too_much_oweing' or $error eq 'auto_too_much_oweing') ) { if ($override_debt) { $can_renew = 1; - $error = undef; + $error = undef; + } + else { + $balance = $patron->account->balance; + $template->param( balance => $balance ); + $can_renew = 0; } } @@ -143,7 +143,6 @@ if ($op eq 'cud-renew' && $barcode) { error => $error, soonestrenewdate => $soonest_renew_date, latestautorenewdate => $latest_auto_renew_date, - balance => $balance, ); } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt index 4a3cfba85c1..a76e79c8dec 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/renew.tt @@ -187,7 +187,7 @@ [% ELSIF error == 'recalled' %]

This item has been recalled.

- [% ELSIF error == "too_much_debt" %] + [% ELSIF error == "too_much_oweing" %]
  • The patron has a debt of [% balance | $Price %].
  • diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index e8e33ee52a7..d0f4e946d2c 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -475,7 +475,7 @@ subtest "GetIssuingCharges tests" => sub { my ( $reused_itemnumber_1, $reused_itemnumber_2 ); subtest "CanBookBeRenewed tests" => sub { - plan tests => 114; + plan tests => 116; C4::Context->set_preference('ItemsDeniedRenewal',''); # Generate test biblio @@ -1727,6 +1727,8 @@ subtest "CanBookBeRenewed tests" => sub { "Account line description must not contain 'Lost Items ', but be title, barcode, itemcallnumber" ); + t::lib::Mocks::mock_preference('FineNoRenewals', 0); + # Recalls t::lib::Mocks::mock_preference('UseRecalls', 1); Koha::CirculationRules->set_rules({ @@ -1793,6 +1795,13 @@ subtest "CanBookBeRenewed tests" => sub { ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrower_obj, $recall_issue ); is( $renewokay, 1, 'Can renew item if biblio-level recall has already been allocated an item' ); $recall->set_cancelled; + + # Too much debt + t::lib::Mocks::mock_preference('FineNoRenewals', 1); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrower_obj, $issue_1); + is( $renewokay, 0, 'Cannot renew, too much debt and FineNoRenewals=1' ); + is( $error, 'too_much_oweing', 'Cannot renew, debt not allowed (returned code is too_much_oweing)'); + C4::Context->dbh->do("DELETE FROM accountlines"); }; subtest "CanBookBeRenewed | bookings" => sub { -- 2.47.0