From dd1eec2715b465c74339c9282f97a3be3102cc2b Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 13 Aug 2020 18:42:01 +0200 Subject: [PATCH] Bug 18501: Add _set_found_trigger Signed-off-by: Martin Renvoize Signed-off-by: Jonathan Druart Signed-off-by: Jonathan Druart --- C4/Circulation.pm | 93 ------------------------------- Koha/Item.pm | 118 ++++++++++++++++++++++++++++++++++------ catalogue/updateitem.pl | 3 - cataloguing/additem.pl | 2 - 4 files changed, 102 insertions(+), 114 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index d9b3790dc2..bb235b9df6 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -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); diff --git a/Koha/Item.pm b/Koha/Item.pm index a752ca8676..5e03e51515 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -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 diff --git a/catalogue/updateitem.pl b/catalogue/updateitem.pl index 43f076c3d2..f8295a0314 100755 --- a/catalogue/updateitem.pl +++ b/catalogue/updateitem.pl @@ -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); diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl index 9e15c436be..a971089168 100755 --- a/cataloguing/additem.pl +++ b/cataloguing/additem.pl @@ -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"; -- 2.39.5