Bugzilla – Attachment 108614 Details for
Bug 18501
Automatic refunds need protection from failure
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18501: Add _set_found_trigger
Bug-18501-Add-setfoundtrigger.patch (text/plain), 10.53 KB, created by
Jonathan Druart
on 2020-08-19 12:42:22 UTC
(
hide
)
Description:
Bug 18501: Add _set_found_trigger
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2020-08-19 12:42:22 UTC
Size:
10.53 KB
patch
obsolete
>From c5f9dbe0cf70edfccd4a9ecfdc12d12660b8b07a Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Thu, 13 Aug 2020 18:42:01 +0200 >Subject: [PATCH] Bug 18501: Add _set_found_trigger > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> > >Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >--- > 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.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18501
:
108004
|
108005
|
108006
|
108121
|
108122
|
108123
|
108167
|
108168
|
108169
|
108170
|
108193
|
108194
|
108195
|
108243
|
108248
|
108249
|
108291
|
108292
|
108293
|
108294
|
108295
|
108296
|
108297
|
108298
|
108299
|
108300
|
108301
|
108482
|
108483
|
108484
|
108485
|
108486
|
108487
|
108488
|
108489
|
108490
|
108491
|
108492
|
108493
|
108494
|
108575
|
108576
|
108577
|
108578
|
108579
|
108580
|
108581
|
108582
|
108583
|
108584
|
108585
|
108586
|
108596
|
108597
|
108598
|
108599
|
108600
|
108601
|
108602
|
108603
|
108604
|
108605
|
108606
|
108607
|
108608
|
108610
|
108611
|
108612
|
108613
| 108614 |
108615
|
108616
|
108617
|
108618
|
108619
|
108620
|
108621
|
108717
|
108732
|
108738
|
108739
|
108740
|
108741
|
108784
|
108785
|
108786
|
108787
|
108788
|
108795
|
111295