Bug 14591: book drop / drop box mode incorrectly decrements accrued overdue fines

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index ef750cd..587cf08 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -1836,7 +1836,7 @@ patron who last borrowed the book.
 =cut
 
 sub AddReturn {
-    my ( $barcode, $branch, $exemptfine, $dropbox, $return_date, $dropboxdate ) = @_;
+    my ( $barcode, $branch, $exemptfine, $return_date ) = @_;
 
     if ($branch and not Koha::Libraries->find($branch)) {
         warn "AddReturn error: branch '$branch' not found.  Reverting to " . C4::Context->userenv->{'branch'};
@@ -1943,25 +1943,14 @@ sub AddReturn {
         my $is_overdue;
         die "The item is not issed and cannot be returned" unless $issue; # Just in case...
         $patron or warn "AddReturn without current borrower";
-        if ($dropbox) {
-            $is_overdue = $issue->is_overdue( $dropboxdate );
-        } else {
-            $is_overdue = $issue->is_overdue;
-        }
+        $is_overdue = $issue->is_overdue( $return_date );
 
         if ($patron) {
             eval {
-                if ( $dropbox ) {
-                    MarkIssueReturned( $borrowernumber, $item->itemnumber,
-                        $dropboxdate, $patron->privacy );
-                }
-                else {
-                    MarkIssueReturned( $borrowernumber, $item->itemnumber,
-                        $return_date, $patron->privacy );
-                }
+                MarkIssueReturned( $borrowernumber, $item->itemnumber, $return_date, $patron->privacy );
             };
             unless ( $@ ) {
-                if ( ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue ) || $return_date ) {
+                if ( C4::Context->preference('CalculateFinesOnReturn') && $is_overdue ) {
                     _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed, return_date => $return_date } );
                 }
             } else {
@@ -2035,13 +2024,12 @@ sub AddReturn {
 
     # fix up the overdues in accounts...
     if ($borrowernumber) {
-        my $fix = _FixOverduesOnReturn($borrowernumber, $item->itemnumber, $exemptfine, $dropbox);
+        my $fix = _FixOverduesOnReturn( $borrowernumber, $item->itemnumber, $exemptfine );
         defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $item->itemnumber...) failed!";  # zero is OK, check defined
 
         if ( $issue and $issue->is_overdue ) {
         # fix fine days
-            $today = dt_from_string($return_date) if $return_date;
-            $today = $dropboxdate if $dropbox;
+            $today = $return_date if $return_date;
             my ($debardate,$reminder) = _debar_user_on_return( $patron_unblessed, $item_unblessed, dt_from_string($issue->date_due), $today );
             if ($reminder){
                 $messages->{'PrevDebarred'} = $debardate;
@@ -2334,7 +2322,7 @@ Internal function
 =cut
 
 sub _FixOverduesOnReturn {
-    my ($borrowernumber, $item, $exemptfine, $dropbox ) = @_;
+    my ($borrowernumber, $item, $exemptfine ) = @_;
     unless( $borrowernumber ) {
         warn "_FixOverduesOnReturn() not supplied valid borrowernumber";
         return;
@@ -2374,30 +2362,6 @@ sub _FixOverduesOnReturn {
         if (C4::Context->preference("FinesLog")) {
             &logaction("FINES", 'MODIFY',$borrowernumber,"Overdue forgiven: item $item");
         }
-    } elsif ($dropbox && $accountline->lastincrement) {
-        my $outstanding = $accountline->amountoutstanding - $accountline->lastincrement;
-        my $amt = $accountline->amount - $accountline->lastincrement;
-
-        Koha::Account::Offset->new(
-            {
-                debit_id => $accountline->id,
-                type => 'Dropbox',
-                amount => $accountline->lastincrement * -1,
-            }
-        )->store();
-
-        if ( C4::Context->preference("FinesLog") ) {
-            &logaction( "FINES", 'MODIFY', $borrowernumber,
-                "Dropbox adjustment $amt, item $item" );
-        }
-
-        $accountline->accounttype('F');
-
-        if ( $outstanding >= 0 && $amt >= 0 ) {
-            $accountline->amount($amt);
-            $accountline->amountoutstanding($outstanding);
-        }
-
     } else {
         $accountline->accounttype('F');
     }
@@ -4124,7 +4088,7 @@ sub _CalculateAndUpdateFine {
       : ( $control eq 'PatronLibrary' )   ? $borrower->{branchcode}
       :                                     $issue->branchcode;
 
-    my $date_returned = $return_date ? dt_from_string($return_date) : dt_from_string();
+    my $date_returned = $return_date ? $return_date : dt_from_string();
 
     my ( $amount, $unitcounttotal, $unitcount  ) =
       C4::Overdues::CalcFine( $item, $borrower->{categorycode}, $control_branchcode, $datedue, $date_returned );

diff --git a/Koha/Checkouts.pm b/Koha/Checkouts.pm
index 374cfb1..5726699 100644 (file)
--- a/Koha/Checkouts.pm
+++ b/Koha/Checkouts.pm
@@ -21,9 +21,9 @@ use Modern::Perl;
 
 use Carp;
 
-use Koha::Database;
-
+use C4::Context;
 use Koha::Checkout;
+use Koha::Database;
 
 use base qw(Koha::Objects);
 
@@ -37,6 +37,23 @@ Koha::Checkouts - Koha Checkout object set class
 
 =cut
 
+=head3 calculate_dropbox_date
+
+my $dt = Koha::Checkouts::calculate_dropbox_date();
+
+=cut
+
+sub calculate_dropbox_date {
+    my $userenv    = C4::Context->userenv;
+    my $branchcode = $userenv->{branch} // q{};
+
+    my $calendar = Koha::Calendar->new( branchcode => $branchcode );
+    my $today        = DateTime->now( time_zone => C4::Context->tz() );
+    my $dropbox_date = $calendar->addDate( $today, -1 );
+
+    return $dropbox_date;
+}
+
 =head3 type
 
 =cut

diff --git a/circ/returns.pl b/circ/returns.pl
index 63cf75b..528907a 100755 (executable)
--- a/circ/returns.pl
+++ b/circ/returns.pl
@@ -33,22 +33,24 @@ use Modern::Perl;
 
 use CGI qw ( -utf8 );
 use DateTime;
-use C4::Context;
+
 use C4::Auth qw/:DEFAULT get_session/;
-use C4::Output;
-use C4::Circulation;
-use C4::Print;
-use C4::Reserves;
 use C4::Biblio;
+use C4::Circulation;
+use C4::Context;
 use C4::Items;
-use C4::Members;
-use C4::Members::Messaging;
 use C4::Koha;   # FIXME : is it still useful ?
+use C4::Members::Messaging;
+use C4::Members;
+use C4::Output;
+use C4::Print;
+use C4::Reserves;
 use C4::RotatingCollections;
 use Koha::AuthorisedValues;
-use Koha::DateUtils;
-use Koha::Calendar;
 use Koha::BiblioFrameworks;
+use Koha::Calendar;
+use Koha::Checkouts;
+use Koha::DateUtils;
 use Koha::Holds;
 use Koha::Items;
 use Koha::Patrons;
@@ -203,17 +205,16 @@ my $dropboxmode = $query->param('dropboxmode');
 my $dotransfer  = $query->param('dotransfer');
 my $canceltransfer = $query->param('canceltransfer');
 my $dest = $query->param('dest');
-my $calendar    = Koha::Calendar->new( branchcode => $userenv_branch );
 #dropbox: get last open day (today - 1)
-my $today       = DateTime->now( time_zone => C4::Context->tz());
-my $dropboxdate = $calendar->addDate($today, -1);
+my $dropboxdate = Koha::Checkouts::calculate_dropbox_date();
 
 my $return_date_override = $query->param('return_date_override');
+my $return_date_override_dt;
 my $return_date_override_remember =
   $query->param('return_date_override_remember');
 if ($return_date_override) {
     if ( C4::Context->preference('SpecifyReturnDate') ) {
-        my $return_date_override_dt = eval {dt_from_string( $return_date_override ) };
+        $return_date_override_dt = eval {dt_from_string( $return_date_override ) };
         if ( $return_date_override_dt ) {
             # note that we've overriden the return date
             $template->param( return_date_was_overriden => 1);
@@ -301,10 +302,11 @@ if ($barcode) {
         barcode => $barcode,
     );
 
+    my $return_date = $dropboxmode ? $dropboxdate : $return_date_override_dt;
 
     # do the return
     ( $returned, $messages, $issue, $borrower ) =
-      AddReturn( $barcode, $userenv_branch, $exemptfine, $dropboxmode, $return_date_override, $dropboxdate );
+      AddReturn( $barcode, $userenv_branch, $exemptfine, $return_date );
 
     if ($returned) {
         my $time_now = DateTime->now( time_zone => C4::Context->tz )->truncate( to => 'minute');
@@ -616,7 +618,7 @@ $template->param(
     errmsgloop     => \@errmsgloop,
     exemptfine     => $exemptfine,
     dropboxmode    => $dropboxmode,
-    dropboxdate    => output_pref($dropboxdate),
+    dropboxdate    => $dropboxdate,
     forgivemanualholdsexpire => $forgivemanualholdsexpire,
     overduecharges => $overduecharges,
     AudioAlerts        => C4::Context->preference("AudioAlerts"),

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
index 2a06845..0ccd192 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
@@ -605,7 +605,7 @@
         <p>Fines are not charged for manually cancelled holds.</p>
     </div>
     <div id="dropboxmode" class="dialog message" style="display:none;">
-        <p>Book drop mode.  (Effective checkin date is [% dropboxdate | html %] ).</p>
+        <p>Book drop mode.  (Effective checkin date is [% dropboxdate | $KohaDates %] ).</p>
     </div>
 
 <div class="row">