Bug 14390 - Fine not updated from 'FU' to 'F' on renewal

Test Plan:
1) Find an overdue checkout with a fine
2) Renew item, note fine is not closed out (Account type F)
3) Apply this patch
4) Find another overdue checkout with a fine
5) Renew item, note fine is now correctly closed out
6) Backdate a checkout to be already overdue ( but not have a fine since
    fines.pl hasn't run yet )
7) Renew item, note a closed out fine is created

Signed-off-by: Sean Minkel <sminkel@rcplib.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit fe71eb0811a6271fff568ca6b599514a57ff3206)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
(cherry picked from commit 895bdb8b40a30a76dcccdfe5ed116e84f89ef227)
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 8c3df27..11696b1 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -1932,39 +1932,7 @@ sub AddReturn {
 
         if ($borrowernumber) {
             if ( ( C4::Context->preference('CalculateFinesOnReturn') && $issue->{'overdue'} ) || $return_date ) {
-                # we only need to calculate and change the fines if we want to do that on return
-                # Should be on for hourly loans
-                my $control = C4::Context->preference('CircControl');
-                my $control_branchcode =
-                    ( $control eq 'ItemHomeLibrary' ) ? $item->{homebranch}
-                  : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
-                  :                                     $issue->{branchcode};
-
-                my $date_returned =
-                  $return_date ? dt_from_string($return_date) : $today;
-
-                my ( $amount, $type, $unitcounttotal ) =
-                  C4::Overdues::CalcFine( $item, $borrower->{categorycode},
-                    $control_branchcode, $datedue, $date_returned );
-
-                $type ||= q{};
-
-                if ( C4::Context->preference('finesMode') eq 'production' ) {
-                    if ( $amount > 0 ) {
-                        C4::Overdues::UpdateFine( $issue->{itemnumber},
-                            $issue->{borrowernumber},
-                            $amount, $type, output_pref($datedue) );
-                    }
-                    elsif ($return_date) {
-
-                       # Backdated returns may have fines that shouldn't exist,
-                       # so in this case, we need to drop those fines to 0
-
-                        C4::Overdues::UpdateFine( $issue->{itemnumber},
-                            $issue->{borrowernumber},
-                            0, $type, output_pref($datedue) );
-                    }
-                }
+                _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $borrower, return_date => $return_date } );
             }
 
             eval {
@@ -2908,10 +2876,7 @@ sub AddRenewal {
     my $dbh = C4::Context->dbh;
 
     # Find the issues record for this book
-    my $sth =
-      $dbh->prepare("SELECT * FROM issues WHERE itemnumber = ?");
-    $sth->execute( $itemnumber );
-    my $issuedata = $sth->fetchrow_hashref;
+    my $issuedata  = GetItemIssue($itemnumber);
 
     return unless ( $issuedata );
 
@@ -2922,12 +2887,18 @@ sub AddRenewal {
         return;
     }
 
+    my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ) or return;
+
+    if ( C4::Context->preference('CalculateFinesOnReturn') && $issuedata->{overdue} ) {
+        _CalculateAndUpdateFine( { issue => $issuedata, item => $item, borrower => $borrower } );
+    }
+    _FixOverduesOnReturn( $borrowernumber, $itemnumber );
+
     # If the due date wasn't specified, calculate it by adding the
     # book's loan length to today's date or the current due date
     # based on the value of the RenewalPeriodBase syspref.
     unless ($datedue) {
 
-        my $borrower = C4::Members::GetMember( borrowernumber => $borrowernumber ) or return;
         my $itemtype = (C4::Context->preference('item-level_itypes')) ? $biblio->{'itype'} : $biblio->{'itemtype'};
 
         $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
@@ -2939,7 +2910,7 @@ sub AddRenewal {
     # Update the issues record to have the new due date, and a new count
     # of how many times it has been renewed.
     my $renews = $issuedata->{'renewals'} + 1;
-    $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ?
+    my $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ?
                             WHERE borrowernumber=? 
                             AND itemnumber=?"
     );
@@ -2969,23 +2940,25 @@ sub AddRenewal {
     }
 
     # Send a renewal slip according to checkout alert preferencei
-    if ( C4::Context->preference('RenewalSendNotice') eq '1') {
-       my $borrower = C4::Members::GetMemberDetails( $borrowernumber, 0 );
-       my $circulation_alert = 'C4::ItemCirculationAlertPreference';
-       my %conditions = (
-               branchcode   => $branch,
-               categorycode => $borrower->{categorycode},
-               item_type    => $item->{itype},
-               notification => 'CHECKOUT',
-       );
-       if ($circulation_alert->is_enabled_for(\%conditions)) {
-               SendCirculationAlert({
-                       type     => 'RENEWAL',
-                       item     => $item,
-               borrower => $borrower,
-               branch   => $branch,
-               });
-       }
+    if ( C4::Context->preference('RenewalSendNotice') eq '1' ) {
+        $borrower = C4::Members::GetMemberDetails( $borrowernumber, 0 );
+        my $circulation_alert = 'C4::ItemCirculationAlertPreference';
+        my %conditions        = (
+            branchcode   => $branch,
+            categorycode => $borrower->{categorycode},
+            item_type    => $item->{itype},
+            notification => 'CHECKOUT',
+        );
+        if ( $circulation_alert->is_enabled_for( \%conditions ) ) {
+            SendCirculationAlert(
+                {
+                    type     => 'RENEWAL',
+                    item     => $item,
+                    borrower => $borrower,
+                    branch   => $branch,
+                }
+            );
+        }
     }
 
     # Remove any OVERDUES related debarment if the borrower has no overdues
@@ -4046,7 +4019,52 @@ sub GetTopIssues {
     return @$rows;
 }
 
+sub _CalculateAndUpdateFine {
+    my ($params) = @_;
+
+    my $borrower    = $params->{borrower};
+    my $item        = $params->{item};
+    my $issue       = $params->{issue};
+    my $return_date = $params->{return_date};
+
+    unless ($borrower) { carp "No borrower passed in!" && return; }
+    unless ($item)     { carp "No item passed in!"     && return; }
+    unless ($issue)    { carp "No issue passed in!"    && return; }
+
+    my $datedue = $issue->{date_due};
+
+    # we only need to calculate and change the fines if we want to do that on return
+    # Should be on for hourly loans
+    my $control = C4::Context->preference('CircControl');
+    my $control_branchcode =
+        ( $control eq 'ItemHomeLibrary' ) ? $item->{homebranch}
+      : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
+      :                                     $issue->{branchcode};
+
+    my $date_returned = $return_date ? dt_from_string($return_date) : dt_from_string();
+
+    my ( $amount, $type, $unitcounttotal ) =
+      C4::Overdues::CalcFine( $item, $borrower->{categorycode}, $control_branchcode, $datedue, $date_returned );
+
+    $type ||= q{};
+
+    if ( C4::Context->preference('finesMode') eq 'production' ) {
+        if ( $amount > 0 ) {
+            C4::Overdues::UpdateFine( $issue->{itemnumber}, $issue->{borrowernumber},
+                $amount, $type, output_pref($datedue) );
+        }
+        elsif ($return_date) {
+
+            # Backdated returns may have fines that shouldn't exist,
+            # so in this case, we need to drop those fines to 0
+
+            C4::Overdues::UpdateFine( $issue->{itemnumber}, $issue->{borrowernumber}, 0, $type, output_pref($datedue) );
+        }
+    }
+}
+
 1;
+
 __END__
 
 =head1 AUTHOR
@@ -4054,4 +4072,3 @@ __END__
 Koha Development Team <http://koha-community.org/>
 
 =cut
-