Bug 13098: Make _FixAccountForLostAndReturned refund the right thing
[koha.git] / C4 / Circulation.pm
index 3e711cd..23d0b5f 100644 (file)
@@ -22,6 +22,7 @@ package C4::Circulation;
 use strict;
 #use warnings; FIXME - Bug 2505
 use DateTime;
+use POSIX qw( floor );
 use Koha::DateUtils;
 use C4::Context;
 use C4::Stats;
@@ -668,6 +669,12 @@ sub CanBookBeIssued {
     my $override_high_holds = $params->{override_high_holds} || 0;
 
     my $item = GetItem(undef, $barcode );
+    # MANDATORY CHECKS - unless item exists, nothing else matters
+    unless ( $item ) {
+        $issuingimpossible{UNKNOWN_BARCODE} = 1;
+    }
+    return ( \%issuingimpossible, \%needsconfirmation ) if %issuingimpossible;
+
     my $issue = Koha::Checkouts->find( { itemnumber => $item->{itemnumber} } );
     my $biblio = Koha::Biblios->find( $item->{biblionumber} );
     my $biblioitem = $biblio->biblioitem;
@@ -675,12 +682,6 @@ sub CanBookBeIssued {
     my $dbh             = C4::Context->dbh;
     my $patron_unblessed = $patron->unblessed;
 
-    # MANDATORY CHECKS - unless item exists, nothing else matters
-    unless ( $item->{barcode} ) {
-        $issuingimpossible{UNKNOWN_BARCODE} = 1;
-    }
-       return ( \%issuingimpossible, \%needsconfirmation ) if %issuingimpossible;
-
     #
     # DUE DATE is OK ? -- should already have checked.
     #
@@ -916,7 +917,7 @@ sub CanBookBeIssued {
         if (C4::Context->preference('item-level_itypes')){
             # this should probably be a subroutine
             my $sth = $dbh->prepare("SELECT notforloan FROM itemtypes WHERE itemtype = ?");
-            $sth->execute($item->{'itemtype'});
+            $sth->execute($effective_itemtype);
             my $notforloan=$sth->fetchrow_hashref();
             if ($notforloan->{'notforloan'}) {
                 if (!C4::Context->preference("AllowNotForLoanOverride")) {
@@ -928,13 +929,16 @@ sub CanBookBeIssued {
                 }
             }
         }
-        elsif ($biblioitem->notforloan == 1){
-            if (!C4::Context->preference("AllowNotForLoanOverride")) {
-                $issuingimpossible{NOT_FOR_LOAN} = 1;
-                $issuingimpossible{itemtype_notforloan} = $effective_itemtype;
-            } else {
-                $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1;
-                $needsconfirmation{itemtype_notforloan} = $effective_itemtype;
+        else {
+            my $itemtype = Koha::ItemTypes->find($biblioitem->itemtype);
+            if ( $itemtype and $itemtype->notforloan == 1){
+                if (!C4::Context->preference("AllowNotForLoanOverride")) {
+                    $issuingimpossible{NOT_FOR_LOAN} = 1;
+                    $issuingimpossible{itemtype_notforloan} = $effective_itemtype;
+                } else {
+                    $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1;
+                    $needsconfirmation{itemtype_notforloan} = $effective_itemtype;
+                }
             }
         }
     }
@@ -1030,14 +1034,14 @@ sub CanBookBeIssued {
                 $alerts{HIGHHOLDS} = {
                     num_holds  => $check->{outstanding},
                     duration   => $check->{duration},
-                    returndate => output_pref( $check->{due_date} ),
+                    returndate => output_pref( { dt => dt_from_string($check->{due_date}), dateformat => 'iso', timeformat => '24hr' }),
                 };
             }
             else {
                 $needsconfirmation{HIGHHOLDS} = {
                     num_holds  => $check->{outstanding},
                     duration   => $check->{duration},
-                    returndate => output_pref( $check->{due_date} ),
+                    returndate => output_pref( { dt => dt_from_string($check->{due_date}), dateformat => 'iso', timeformat => '24hr' }),
                 };
             }
         }
@@ -1179,7 +1183,7 @@ sub checkHighHolds {
             }
 
             # Remove any items that are not holdable for this patron
-            @items = grep { CanItemBeReserved( $borrower->{borrowernumber}, $_->itemnumber ) eq 'OK' } @items;
+            @items = grep { CanItemBeReserved( $borrower->{borrowernumber}, $_->itemnumber )->{status} eq 'OK' } @items;
 
             my $items_count = scalar @items;
 
@@ -1313,7 +1317,7 @@ sub AddIssue {
                 AddReturn( $item->{'barcode'}, C4::Context->userenv->{'branch'} );
             }
 
-            MoveReserve( $item->{'itemnumber'}, $borrower->{'borrowernumber'}, $cancelreserve );
+            C4::Reserves::MoveReserve( $item->{'itemnumber'}, $borrower->{'borrowernumber'}, $cancelreserve );
 
             # Starting process for transfer job (checking transfert and validate it if we have one)
             my ($datesent) = GetTransfers( $item->{'itemnumber'} );
@@ -1397,14 +1401,15 @@ sub AddIssue {
                     datelastborrowed => DateTime->now( time_zone => C4::Context->tz() )->ymd(),
                 },
                 $item->{'biblionumber'},
-                $item->{'itemnumber'}
+                $item->{'itemnumber'},
+                { log_action => 0 }
             );
             ModDateLastSeen( $item->{'itemnumber'} );
 
            # If it costs to borrow this book, charge it to the patron's account.
             my ( $charge, $itemtype ) = GetIssuingCharges( $item->{'itemnumber'}, $borrower->{'borrowernumber'} );
             if ( $charge > 0 ) {
-                AddIssuingCharge( $item->{'itemnumber'}, $borrower->{'borrowernumber'}, $charge );
+                AddIssuingCharge( $issue, $charge );
                 $item->{'charge'} = $charge;
             }
 
@@ -1812,6 +1817,7 @@ sub AddReturn {
                 . Dumper($issue->unblessed) . "\n";
     } else {
         $messages->{'NotIssued'} = $barcode;
+        ModItem({ onloan => undef }, $item->{biblionumber}, $item->{itemnumber}) if defined $item->{onloan};
         # even though item is not on loan, it may still be transferred;  therefore, get current branch info
         $doreturn = 0;
         # No issue, no borrowernumber.  ONLY if $doreturn, *might* you have a $borrower later.
@@ -1830,7 +1836,7 @@ sub AddReturn {
             $item->{location} = $item->{permanent_location};
         }
 
-        ModItem( $item, $item->{'biblionumber'}, $item->{'itemnumber'} );
+        ModItem( $item, $item->{'biblionumber'}, $item->{'itemnumber'}, { log_action => 0 } );
     }
 
         # full item data, but no borrowernumber or checkout info (no issue)
@@ -1854,7 +1860,7 @@ sub AddReturn {
             foreach my $key ( keys %$rules ) {
                 if ( $item->{notforloan} eq $key ) {
                     $messages->{'NotForLoanStatusUpdated'} = { from => $item->{notforloan}, to => $rules->{$key} };
-                    ModItem( { notforloan => $rules->{$key} }, undef, $itemnumber );
+                    ModItem( { notforloan => $rules->{$key} }, undef, $itemnumber, { log_action => 0 } );
                     last;
                 }
             }
@@ -1920,7 +1926,7 @@ sub AddReturn {
 
         }
 
-        ModItem({ onloan => undef }, $item->{biblionumber}, $item->{'itemnumber'});
+        ModItem( { onloan => undef }, $item->{biblionumber}, $item->{itemnumber}, { log_action => 0 } );
     }
 
     # the holdingbranch is updated if the document is returned to another location.
@@ -1930,7 +1936,9 @@ sub AddReturn {
         UpdateHoldingbranch($branch, $item->{'itemnumber'});
         $item->{'holdingbranch'} = $branch; # update item data holdingbranch too
     }
-    ModDateLastSeen( $item->{'itemnumber'} );
+
+    my $leave_item_lost = C4::Context->preference("BlockReturnOfLostItems") ? 1 : 0;
+    ModDateLastSeen( $item->{itemnumber}, $leave_item_lost );
 
     # check if we have a transfer for this document
     my ($datesent,$frombranch,$tobranch) = GetTransfers( $item->{'itemnumber'} );
@@ -1983,6 +1991,7 @@ sub AddReturn {
 
         if ( $issue and $issue->is_overdue ) {
         # fix fine days
+            $today = dt_from_string($return_date) if $return_date;
             $today = $dropboxdate if $dropbox;
             my ($debardate,$reminder) = _debar_user_on_return( $patron_unblessed, $item, dt_from_string($issue->date_due), $today );
             if ($reminder){
@@ -2156,7 +2165,7 @@ sub MarkIssueReturned {
         # And finally delete the issue
         $issue->delete;
 
-        ModItem( { 'onloan' => undef }, undef, $itemnumber );
+        ModItem( { 'onloan' => undef }, undef, $itemnumber, { log_action => 0 } );
 
         if ( C4::Context->preference('StoreLastBorrower') ) {
             my $item = Koha::Items->find( $itemnumber );
@@ -2237,8 +2246,25 @@ sub _debar_user_on_return {
                 }
             }
 
-            my $new_debar_dt =
-              $return_date->clone()->add_duration( $suspension_days );
+            if ( $issuing_rule->suspension_chargeperiod > 1 ) {
+                # No need to / 1 and do not consider / 0
+                $suspension_days = DateTime::Duration->new(
+                    days => floor( $suspension_days->in_units('days') / $issuing_rule->suspension_chargeperiod )
+                );
+            }
+
+            my $new_debar_dt;
+            # Use the calendar or not to calculate the debarment date
+            if ( C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed' ) {
+                my $calendar = Koha::Calendar->new(
+                    branchcode => $branchcode,
+                    days_mode  => 'Calendar'
+                );
+                $new_debar_dt = $calendar->addDate( $return_date, $suspension_days );
+            }
+            else {
+                $new_debar_dt = $return_date->clone()->add_duration($suspension_days);
+            }
 
             Koha::Patron::Debarments::AddUniqueDebarment({
                 borrowernumber => $borrower->{borrowernumber},
@@ -2366,8 +2392,10 @@ sub _FixAccountForLostAndReturned {
     my $borrowernumber = @_ ? shift : undef;
     my $item_id        = @_ ? shift : $itemnumber;  # Send the barcode if you want that logged in the description
 
+    my $credit;
+
     # check for charge made for lost book
-    my $accountline = Koha::Account::Lines->search(
+    my $accountlines = Koha::Account::Lines->search(
         {
             itemnumber  => $itemnumber,
             accounttype => { -in => [ 'L', 'Rep', 'W' ] },
@@ -2375,29 +2403,49 @@ sub _FixAccountForLostAndReturned {
         {
             order_by => { -desc => [ 'date', 'accountno' ] }
         }
-    )->next();
+    );
 
-    return unless $accountline;
-    return if $accountline->accounttype eq 'W';    # Written off
+    return unless $accountlines->count > 0;
+    my $accountline = $accountlines->next;
+
+    # Use cases
+    if ( $accountline->amount > $accountline->amountoutstanding ) {
+        # some amount has been cancelled. collect the offsets that are not writeoffs
+        # this works because the only way to subtract from a debt is
+        # using the UI buttons 'Pay' and 'Write off'
+        my $credits_offsets = Koha::Account::Offsets->search({
+            debit_id  => $accountline->id,
+            credit_id => { '!=' => undef }, # it is not the debit itself
+            type      => { '!=' => 'Writeoff' },
+            amount    => { '<'  => 0 } # credits are negative on the DB
+        });
+
+        my $total_to_refund = ( $credits_offsets->count > 0 )
+                                ? $credits_offsets->total * -1 # credits are negative on the DB
+                                : 0;
+
+        if ( $total_to_refund > 0 ) {
+            my $account = Koha::Patrons->find( $accountline->borrowernumber )->account;
+            $credit = $account->add_credit(
+                {
+                    amount      => $total_to_refund,
+                    description => 'Item Returned ' . $item_id,
+                    type        => 'lost_item_return'
+                }
+            );
+        }
+
+        ModItem( { paidfor => '' }, undef, $itemnumber, { log_action => 0 } );
+    }
+    # else {
+        # $accountline->amount == $accountline->amountoutstanding
+    #}
 
     $accountline->accounttype('LR');
+    $accountline->amountoutstanding(0);
     $accountline->store();
 
-    my $account = Koha::Account->new( { patron_id => $accountline->borrowernumber } );
-    my $credit_id = $account->pay(
-        {
-            amount       => $accountline->amount,
-            description  => "Item Returned " . $item_id,
-            account_type => 'CR',
-            offset_type  => 'Lost Item Return',
-            accounlines  => [$accountline],
-
-        }
-    );
-
-    ModItem( { paidfor => '' }, undef, $itemnumber );
-
-    return $credit_id;
+    return ($credit) ? $credit->id : undef;
 }
 
 =head2 _GetCircControlBranch
@@ -2820,12 +2868,12 @@ sub AddRenewal {
 
     # Update the renewal count on the item, and tell zebra to reindex
     $renews = $item->{renewals} + 1;
-    ModItem({ renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item->{biblionumber}, $itemnumber);
+    ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item->{biblionumber}, $itemnumber, { log_action => 0 } );
 
     # Charge a new rental fee, if applicable?
     my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
     if ( $charge > 0 ) {
-        my $accountno = getnextacctno( $borrowernumber );
+        my $accountno = C4::Accounts::getnextacctno( $borrowernumber );
         my $manager_id = 0;
         $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv; 
         $sth = $dbh->prepare(
@@ -3156,22 +3204,25 @@ sub _get_discount_from_rule {
 
 =head2 AddIssuingCharge
 
-  &AddIssuingCharge( $itemno, $borrowernumber, $charge )
+  &AddIssuingCharge( $checkout, $charge )
 
 =cut
 
 sub AddIssuingCharge {
-    my ( $itemnumber, $borrowernumber, $charge ) = @_;
+    my ( $checkout, $charge ) = @_;
 
-    my $nextaccntno = getnextacctno($borrowernumber);
+    # FIXME What if checkout does not exist?
+
+    my $nextaccntno = C4::Accounts::getnextacctno( $checkout->borrowernumber );
 
     my $manager_id  = 0;
     $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv;
 
     my $accountline = Koha::Account::Line->new(
         {
-            borrowernumber    => $borrowernumber,
-            itemnumber        => $itemnumber,
+            borrowernumber    => $checkout->borrowernumber,
+            itemnumber        => $checkout->itemnumber,
+            issue_id          => $checkout->issue_id,
             accountno         => $nextaccntno,
             amount            => $charge,
             amountoutstanding => $charge,
@@ -3527,6 +3578,9 @@ return $exist;
 
 Code is either an itemtype or collection doe depending on the pref BranchTransferLimitsType
 
+Deprecated in favor of Koha::Item::Transfer::Limits->find/search and
+Koha::Item->can_be_transferred.
+
 =cut
 
 sub IsBranchTransferAllowed {
@@ -3555,6 +3609,8 @@ sub IsBranchTransferAllowed {
 
 $code is either itemtype or collection code depending on what the pref BranchTransferLimitsType is set to.
 
+Deprecated in favor of Koha::Item::Transfer::Limit->new.
+
 =cut
 
 sub CreateBranchTransferLimit {
@@ -3576,6 +3632,10 @@ Deletes all the library transfer limits for one library.  Returns the
 number of limits deleted, 0e0 if no limits were deleted, or undef if
 no arguments are supplied.
 
+Deprecated in favor of Koha::Item::Transfer::Limits->search({
+    fromBranch => $fromBranch
+    })->delete.
+
 =cut
 
 sub DeleteBranchTransferLimits {
@@ -3601,9 +3661,20 @@ sub ReturnLostItem{
 
 
 sub LostItem{
-    my ($itemnumber, $mark_returned) = @_;
+    my ($itemnumber, $mark_lost_from, $force_mark_returned) = @_;
+
+    unless ( $mark_lost_from ) {
+        # Temporary check to avoid regressions
+        die q|LostItem called without $mark_lost_from, check the API.|;
+    }
 
-    $mark_returned //= C4::Context->preference('MarkLostItemsAsReturned');
+    my $mark_returned;
+    if ( $force_mark_returned ) {
+        $mark_returned = 1;
+    } else {
+        my $pref = C4::Context->preference('MarkLostItemsAsReturned') // q{};
+        $mark_returned = ( $pref =~ m|$mark_lost_from| );
+    }
 
     my $dbh = C4::Context->dbh();
     my $sth=$dbh->prepare("SELECT issues.*,items.*,biblio.title 
@@ -3699,7 +3770,8 @@ sub ProcessOfflineReturn {
             ModItem(
                 { renewals => 0, onloan => undef },
                 $issue->{'biblionumber'},
-                $itemnumber
+                $itemnumber,
+                { log_action => 0 }
             );
             return "Success.";
         } else {