Bug 18501: Add _set_found_trigger
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Thu, 13 Aug 2020 16:42:01 +0000 (18:42 +0200)
committerJonathan Druart <jonathan.druart@bugs.koha-community.org>
Thu, 20 Aug 2020 10:31:59 +0000 (12:31 +0200)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

C4/Circulation.pm
Koha/Item.pm
catalogue/updateitem.pl
cataloguing/additem.pl

index d9b3790..bb235b9 100644 (file)
@@ -1508,11 +1508,6 @@ sub AddIssue {
                 UpdateTotalIssues( $item_object->biblionumber, 1 );
             }
 
-            ## If item was lost, it has now been found, reverse any list item charges if necessary.
-            if ( $item_object->itemlost ) {
-                $item_object->set_found;
-            }
-
             $item_object->issues( ( $item_object->issues || 0 ) + 1);
             $item_object->holdingbranch(C4::Context->userenv->{'branch'});
             $item_object->itemlost(0);
@@ -2487,94 +2482,6 @@ sub _FixOverduesOnReturn {
     return $result;
 }
 
-=head2 _FixAccountForLostAndFound
-
-  &_FixAccountForLostAndFound($itemnumber, [$barcode]);
-
-Finds the most recent lost item charge for this item and refunds the borrower
-appropriatly, taking into account any payments or writeoffs already applied
-against the charge.
-
-Internal function, not exported, called only by AddReturn.
-
-=cut
-
-sub _FixAccountForLostAndFound {
-    my $itemnumber     = shift or return;
-    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 $accountlines = Koha::Account::Lines->search(
-        {
-            itemnumber      => $itemnumber,
-            debit_type_code => 'LOST',
-            status          => [ undef, { '<>' => 'FOUND' } ]
-        },
-        {
-            order_by => { -desc => [ 'date', 'accountlines_id' ] }
-        }
-    );
-
-    return unless $accountlines->count > 0;
-    my $accountline     = $accountlines->next;
-    my $total_to_refund = 0;
-
-    return unless $accountline->borrowernumber;
-    my $patron = Koha::Patrons->find( $accountline->borrowernumber );
-    return unless $patron; # Patron has been deleted, nobody to credit the return to
-
-    my $account = $patron->account;
-
-    # 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 this kind of 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
-        });
-
-        $total_to_refund = ( $credits_offsets->count > 0 )
-                            ? $credits_offsets->total * -1 # credits are negative on the DB
-                            : 0;
-    }
-
-    my $credit_total = $accountline->amountoutstanding + $total_to_refund;
-
-    if ( $credit_total > 0 ) {
-        my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;
-        $credit = $account->add_credit(
-            {
-                amount      => $credit_total,
-                description => 'Item found ' . $item_id,
-                type        => 'LOST_FOUND',
-                interface   => C4::Context->interface,
-                library_id  => $branchcode,
-                item_id     => $itemnumber
-            }
-        );
-
-        $credit->apply( { debits => [ $accountline ] } );
-    }
-
-    # Update the account status
-    $accountline->discard_changes->status('FOUND');
-    $accountline->store;
-
-    $accountline->item->paidfor('')->store({ log_action => 0 });
-
-    if ( defined $account and C4::Context->preference('AccountAutoReconcile') ) {
-        $account->reconcile_balance;
-    }
-
-    return ($credit) ? $credit->id : undef;
-}
-
 =head2 _GetCircControlBranch
 
    my $circ_control_branch = _GetCircControlBranch($iteminfos, $borrower);
index a752ca8..5e03e51 100644 (file)
@@ -169,6 +169,14 @@ sub store {
             $self->permanent_location( $self->location );
         }
 
+        # If item was lost, it has now been found, reverse any list item charges if necessary.
+        if ( exists $updated_columns{itemlost}
+                and $self->itemlost != $updated_columns{itemlost}
+                and $updated_columns{itemlost} >= 1 ) {
+            $self->_set_found_trigger;
+            $self->paidfor('');
+        }
+
         C4::Biblio::ModZebra( $self->biblionumber, "specialUpdate", "biblioserver" )
             unless $params->{skip_modzebra_update};
 
@@ -763,10 +771,20 @@ sub renewal_branchcode {
     return $branchcode;
 }
 
-sub set_found {
-    my ($self, $params) = @_;
+=head3 _set_found_trigger
+
+    $self->_set_found_trigger
+
+Finds the most recent lost item charge for this item and refunds the patron
+appropriatly, taking into account any payments or writeoffs already applied
+against the charge.
 
-    my $holdingbranch = $params->{holdingbranch} || $self->holdingbranch;
+Internal function, not exported, called only by Koha::Item->store.
+
+=cut
+
+sub _set_found_trigger {
+    my ( $self, $params ) = @_;
 
     ## If item was lost, it has now been found, reverse any list item charges if necessary.
     my $refund = 1;
@@ -778,25 +796,93 @@ sub set_found {
           dt_from_string( $self->itemlost_on )->delta_days($today)
           ->in_units('days');
 
-        $refund = 0 unless ( $lost_age_in_days < $no_refund_after_days );
+        return $self unless $lost_age_in_days < $no_refund_after_days;
+    }
+
+    return $self
+      unless Koha::CirculationRules->get_lostreturn_policy(
+        {
+            current_branch => C4::Context->userenv->{branch},
+            item           => $self,
+        }
+      );
+
+    # check for charge made for lost book
+    my $accountlines = Koha::Account::Lines->search(
+        {
+            itemnumber      => $self->itemnumber,
+            debit_type_code => 'LOST',
+            status          => [ undef, { '<>' => 'FOUND' } ]
+        },
+        {
+            order_by => { -desc => [ 'date', 'accountlines_id' ] }
+        }
+    );
+
+    return $self unless $accountlines->count > 0;
+
+    my $accountline     = $accountlines->next;
+    my $total_to_refund = 0;
+
+    return $self unless $accountline->borrowernumber;
+
+    my $patron = Koha::Patrons->find( $accountline->borrowernumber );
+    return $self
+      unless $patron;  # Patron has been deleted, nobody to credit the return to
+                       # FIXME Should not we notify this somehwere
+
+    my $account = $patron->account;
+
+    # 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 this kind of 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
+            }
+        );
+
+        $total_to_refund = ( $credits_offsets->count > 0 )
+          ? $credits_offsets->total * -1    # credits are negative on the DB
+          : 0;
     }
 
-    my $refunded;
-    if (
-        $refund
-        && Koha::CirculationRules->get_lostreturn_policy(
+    my $credit_total = $accountline->amountoutstanding + $total_to_refund;
+
+    my $credit;
+    if ( $credit_total > 0 ) {
+        my $branchcode =
+          C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;
+        $credit = $account->add_credit(
             {
-                current_branch => C4::Context->userenv->{branch},
-                item           => $self,
+                amount      => $credit_total,
+                description => 'Item found ' . $item_id,
+                type        => 'LOST_FOUND',
+                interface   => C4::Context->interface,
+                library_id  => $branchcode,
+                item_id     => $itemnumber
             }
-        )
-      )
-    {
-        C4::Circulation::_FixAccountForLostAndFound( $self->itemnumber, $self->barcode );
-        $refunded = 1;
+        );
+
+        $credit->apply( { debits => [$accountline] } );
     }
 
-    return $refunded;
+    # Update the account status
+    $accountline->discard_changes->status('FOUND')
+      ; # FIXME JD Why discard_changes? $accountline has not been modified since last fetch
+    $accountline->store;
+
+    if ( defined $account and C4::Context->preference('AccountAutoReconcile') ) {
+        $account->reconcile_balance;
+    }
+
+    return $self;
 }
 
 =head3 to_api_mapping
index 43f076c..f8295a0 100755 (executable)
@@ -68,9 +68,6 @@ elsif ( $op eq "set_public_note" ) { # i.e., itemnotes parameter passed from for
         $item->itemnotes($itemnotes);
     }
 } elsif ( $op eq "set_lost" && $itemlost ne $item_data_hashref->{'itemlost'}) {
-    $item->set_found
-        if !$itemlost && $item->itemlost && $item->itemlost ge '1';
-
     $item->itemlost($itemlost);
 } elsif ( $op eq "set_withdrawn" && $withdrawn ne $item_data_hashref->{'withdrawn'}) {
     $item->withdrawn($withdrawn);
index 9e15c43..a971089 100755 (executable)
@@ -751,8 +751,6 @@ if ($op eq "additem") {
         my $newitemlost = $newitem->{itemlost};
         if ( $newitemlost && $newitemlost ge '1' && !$olditemlost ) {
             LostItem( $item->itemnumber, 'additem' )
-        } elsif ( !$newitemlost && $olditemlost && $olditemlost ge '1' ) {
-            $item->set_found;
         }
     }
     $nextop="additem";