Bugzilla – Attachment 94607 Details for
Bug 23051
Add ability to optionally renew fine accruing items when all fines on item are paid off
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 23051: (follow-up) Refactor renewal code
Bug-23051-follow-up-Refactor-renewal-code.patch (text/plain), 7.38 KB, created by
Andrew Isherwood
on 2019-10-23 10:59:42 UTC
(
hide
)
Description:
Bug 23051: (follow-up) Refactor renewal code
Filename:
MIME Type:
Creator:
Andrew Isherwood
Created:
2019-10-23 10:59:42 UTC
Size:
7.38 KB
patch
obsolete
>From d5960a55a292225261ed93876746d73dcdec9b99 Mon Sep 17 00:00:00 2001 >From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com> >Date: Wed, 9 Oct 2019 15:13:54 +0100 >Subject: [PATCH] Bug 23051: (follow-up) Refactor renewal code > >As per Nick's first point in comment #20, the code that tests for >renewability and renews items has been refactored into it's own >function. >--- > Koha/Account.pm | 106 ++++++++++++++++++++++++++++++++------------------- > Koha/Account/Line.pm | 46 +++++----------------- > 2 files changed, 76 insertions(+), 76 deletions(-) > >diff --git a/Koha/Account.pm b/Koha/Account.pm >index b576f0c5e3..63e0d67e67 100644 >--- a/Koha/Account.pm >+++ b/Koha/Account.pm >@@ -119,13 +119,7 @@ sub pay { > > # If we need to make a note of the item associated with this line, > # in order that we can potentially renew it, do so. >- if ( >- $new_amountoutstanding == 0 && >- $fine->accounttype && >- $fine->accounttype eq 'OVERDUE' && >- $fine->status && >- $fine->status eq 'UNRETURNED' >- ) { >+ if (_fine_paid_renewable($new_amountoutstanding, $fine)) { > $overdue_unreturned->{$fine->itemnumber} = $fine; > } > >@@ -191,13 +185,8 @@ sub pay { > > # If we need to make a note of the item associated with this line, > # in order that we can potentially renew it, do so. >- if ( >- $old_amountoutstanding - $amount_to_pay == 0 && >- $fine->accounttype && >- $fine->accounttype eq 'OVERDUE' && >- $fine->status && >- $fine->status eq 'UNRETURNED' >- ) { >+ my $amt = $old_amountoutstanding - $amount_to_pay; >+ if (_fine_paid_renewable($amt, $fine)) { > $overdue_unreturned->{$fine->itemnumber} = $fine; > } > >@@ -284,32 +273,7 @@ sub pay { > # If we have overdue unreturned items that have had payments made > # against them, check whether the balance on those items is now zero > # and, if the syspref is set, renew them >- # Same logic exists in Koha::Account::Line::apply >- if ( >- C4::Context->preference('RenewAccruingItemWhenPaid') && >- keys %{$overdue_unreturned} >- ) { >- foreach my $itemnumber (keys %{$overdue_unreturned}) { >- # Only do something if this item has no fines left on it >- my $fine = C4::Overdues::GetFine( $itemnumber, $self->{patron_id} ); >- next if $fine && $fine > 0; >- >- my ( $renew_ok, $error ) = >- C4::Circulation::CanBookBeRenewed( >- $self->{patron_id}, $itemnumber >- ); >- if ( $renew_ok ) { >- C4::Circulation::AddRenewal( >- $self->{patron_id}, >- $itemnumber, >- $library_id, >- undef, >- undef, >- 1 >- ); >- } >- } >- } >+ _maybe_renew($overdue_unreturned, $self->{patron_id}); > > if ( C4::Context->preference("FinesLog") ) { > logaction( >@@ -760,6 +724,68 @@ sub reconcile_balance { > return $self; > } > >+=head3 _fine_paid_renewable >+ >+my $bool = _fine_paid_renewable($amt, $fine); >+ >+Given an outstanding amount and a fine object, determine if this item >+is potentially renewable as a result of the fine being paid off >+ >+=cut >+ >+sub _fine_paid_renewable { >+ my ($amt, $fine) = @_; >+ >+ return ( >+ $amt == 0 && >+ $fine->accounttype && >+ $fine->accounttype eq 'OVERDUE' && >+ $fine->status && >+ $fine->status eq 'UNRETURNED' >+ ) ? 1 : 0; >+} >+ >+=head3 _maybe_renew >+ >+my $result = _maybe_renew($overdue_unreturned, $patron_id, $library_id); >+ >+If we have overdue unreturned items that have had payments made >+against them, check whether the balance on those items is now zero >+and, if the syspref is set, renew them >+ >+=cut >+ >+sub _maybe_renew { >+ my ($items, $patron_id) = @_; >+ >+ if ( >+ C4::Context->preference('RenewAccruingItemWhenPaid') && >+ keys %{$items} >+ ) { >+ foreach my $itemnumber (keys %{$items}) { >+ # Only do something if this item has no fines left on it >+ my $fine = C4::Overdues::GetFine( $itemnumber, $patron_id ); >+ next if $fine && $fine > 0; >+ >+ my ( $renew_ok, $error ) = >+ C4::Circulation::CanBookBeRenewed($patron_id, $itemnumber); >+ if ( $renew_ok ) { >+ my $due_date = C4::Circulation::AddRenewal( >+ $patron_id, >+ $itemnumber, >+ $items->{$itemnumber}->{branchcode}, >+ undef, >+ undef, >+ 1 >+ ); >+ return $due_date; >+ } else { >+ return $error; >+ } >+ } >+ } >+} >+ > 1; > > =head2 Name mappings >diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm >index 19351a0c19..3793eba733 100644 >--- a/Koha/Account/Line.pm >+++ b/Koha/Account/Line.pm >@@ -23,6 +23,7 @@ use Data::Dumper; > use C4::Log qw(logaction); > use C4::Overdues qw(GetFine); > >+use Koha::Account qw( _fine_paid_renewable _maybe_renew ); > use Koha::Account::Offsets; > use Koha::Database; > use Koha::Exceptions::Account; >@@ -243,16 +244,11 @@ sub apply { > > # If we need to make a note of the item associated with this line, > # in order that we can potentially renew it, do so. >- # Same logic existing in Koha::Account::pay >- if ( >- $debit->amountoutstanding == 0 && >- $debit->accounttype && >- $debit->accounttype eq 'OVERDUE' && >- $debit->status && >- $debit->status eq 'UNRETURNED' >- ) { >- $overdue_unreturned->{$debit->itemnumber} = $debit; >- } >+ my $renewable = Koha::Account::_fine_paid_renewable( >+ $debit->amountoutstanding, >+ $debit >+ ); >+ $overdue_unreturned->{$debit->itemnumber} = $debit if $renewable; > > # Same logic exists in Koha::Account::pay > if ( $debit->amountoutstanding == 0 >@@ -269,32 +265,10 @@ sub apply { > # If we have overdue unreturned items that have had payments made > # against them, check whether the balance on those items is now zero > # and, if the syspref is set, renew them >- # Same logic existing in Koha::Account::pay >- if ( >- C4::Context->preference('RenewAccruingItemWhenPaid') && >- keys %{$overdue_unreturned} >- ) { >- foreach my $itemnumber (keys %{$overdue_unreturned}) { >- # Only do something if this item has no fines left on it >- my $fine = C4::Overdues::GetFine( $itemnumber, $self->borrowernumber ); >- next if $fine && $fine > 0; >- >- my ( $renew_ok, $error ) = >- C4::Circulation::CanBookBeRenewed( >- $self->borrowernumber, $itemnumber >- ); >- if ( $renew_ok ) { >- C4::Circulation::AddRenewal( >- $self->borrowernumber, >- $itemnumber, >- $overdue_unreturned->{$itemnumber}->{branchcode}, >- undef, >- undef, >- 1 >- ); >- } >- } >- } >+ Koha::Account::_maybe_renew( >+ $overdue_unreturned, >+ $self->borrowernumber >+ ); > > return $available_credit; > } >-- >2.11.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 23051
:
90559
|
90560
|
90561
|
91058
|
91059
|
91060
|
91792
|
91793
|
91794
|
91795
|
92684
|
92685
|
92686
|
92687
|
92688
|
92689
|
93922
|
93923
|
93924
|
93925
|
94020
|
94021
|
94022
|
94023
|
94024
|
94402
|
94434
|
94453
|
94525
|
94604
|
94605
|
94606
|
94607
|
94608
|
94609
|
94610
|
94611
|
94629
|
94649
|
95557
|
95558
|
95559
|
95560
|
100127
|
100128
|
100129
|
100130
|
100131
|
100132
|
100133
|
100134
|
100135
|
100138
|
100172
|
100182
|
100183
|
100184
|
100185
|
100186
|
100187
|
100188
|
100271