Bug 23091: Move _Restore to Koha::Item store trigger

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Martin Renvoize 2020-09-03 17:12:47 +01:00 committed by Jonathan Druart
parent 1008e21d43
commit d643bf75a0
4 changed files with 488 additions and 268 deletions

View File

@ -1541,51 +1541,31 @@ sub AddIssue {
$item_object->datelastseen( dt_from_string()->ymd() );
$item_object->store({log_action => 0});
# If the item was lost, it has now been found, restore/charge the overdue if necessary
# If the item was lost, it has now been found, charge the overdue if necessary
if ($was_lost) {
my $lostreturn_policy =
Koha::CirculationRules->get_lostreturn_policy(
{
return_branch => C4::Context->userenv->{branch},
item => $item_object
}
);
if ($lostreturn_policy) {
if ( $lostreturn_policy eq 'charge' ) {
$actualissue //= Koha::Old::Checkouts->search(
{ itemnumber => $item_unblessed->{itemnumber} },
{
order_by => { '-desc' => 'returndate' },
rows => 1
}
)->single;
unless ( exists( $borrower->{branchcode} ) ) {
my $patron = $actualissue->patron;
$borrower = $patron->unblessed;
if ( $item_object->{_charge} ) {
$actualissue //= Koha::Old::Checkouts->search(
{ itemnumber => $item_unblessed->{itemnumber} },
{
order_by => { '-desc' => 'returndate' },
rows => 1
}
_CalculateAndUpdateFine(
{
issue => $actualissue,
item => $item_unblessed,
borrower => $borrower,
return_date => $issuedate
}
);
_FixOverduesOnReturn( $borrower->{borrowernumber},
$item_object->itemnumber, undef, 'RENEWED' );
}
elsif ( $lostreturn_policy eq 'restore' ) {
_RestoreOverdueForLostAndFound(
$item_object->itemnumber );
}
if ( C4::Context->preference('AccountAutoReconcile') ) {
$account->reconcile_balance;
)->single;
unless ( exists( $borrower->{branchcode} ) ) {
my $patron = $actualissue->patron;
$borrower = $patron->unblessed;
}
_CalculateAndUpdateFine(
{
issue => $actualissue,
item => $item_unblessed,
borrower => $borrower,
return_date => $issuedate
}
);
_FixOverduesOnReturn( $borrower->{borrowernumber},
$item_object->itemnumber, undef, 'RENEWED' );
}
}
# If it costs to borrow this book, charge it to the patron's account.
@ -2094,46 +2074,32 @@ sub AddReturn {
my $updated_item = ModDateLastSeen( $item->itemnumber, $leave_item_lost, { skip_record_index => 1 } ); # will unset itemlost if needed
# fix up the accounts.....
if ( $item_was_lost ) {
if ($item_was_lost) {
$messages->{'WasLost'} = 1;
unless ( C4::Context->preference("BlockReturnOfLostItems") ) {
$messages->{'LostItemFeeRefunded'} = $updated_item->{_refunded};
$messages->{'LostItemFeeRestored'} = $updated_item->{_restored};
my $lostreturn_policy =
Koha::CirculationRules->get_lostreturn_policy(
{
return_branch => C4::Context->userenv->{branch},
item => $updated_item
if ( $updated_item->{_charge} ) {
$issue //= Koha::Old::Checkouts->search(
{ itemnumber => $item->itemnumber },
{ order_by => { '-desc' => 'returndate' }, rows => 1 } )
->single;
unless ( exists( $patron_unblessed->{branchcode} ) ) {
my $patron = $issue->patron;
$patron_unblessed = $patron->unblessed;
}
);
if ($lostreturn_policy) {
if ( $lostreturn_policy eq 'charge' ) {
$issue //= Koha::Old::Checkouts->search(
{ itemnumber => $item->itemnumber },
{ order_by => { '-desc' => 'returndate' }, rows => 1 }
)->single;
unless (exists($patron_unblessed->{branchcode})) {
my $patron = $issue->patron;
$patron_unblessed = $patron->unblessed;
_CalculateAndUpdateFine(
{
issue => $issue,
item => $item->unblessed,
borrower => $patron_unblessed,
return_date => $return_date
}
_CalculateAndUpdateFine(
{
issue => $issue,
item => $item->unblessed,
borrower => $patron_unblessed,
return_date => $return_date
}
);
_FixOverduesOnReturn( $patron_unblessed->{borrowernumber},
$item->itemnumber, undef, 'RETURNED' );
$messages->{'LostItemFeeCharged'} = 1;
}
elsif ( $lostreturn_policy eq 'restore' ) {
_RestoreOverdueForLostAndFound( $item->itemnumber );
$messages->{'LostItemFeeRestored'} = 1;
}
);
_FixOverduesOnReturn( $patron_unblessed->{borrowernumber},
$item->itemnumber, undef, 'RETURNED' );
$messages->{'LostItemFeeCharged'} = 1;
}
}
}
@ -2593,66 +2559,6 @@ sub _FixOverduesOnReturn {
return $result;
}
=head2 _RestoreOverdueForLostAndFound
&_RestoreOverdueForLostAndFound( $itemnumber );
C<$itemnumber> itemnumber
Internal function
=cut
sub _RestoreOverdueForLostAndFound {
my ( $item ) = @_;
unless( $item ) {
warn "_RestoreOverdueForLostAndFound() not supplied valid itemnumber";
return;
}
my $schema = Koha::Database->schema;
my $result = $schema->txn_do(
sub {
# check for lost overdue fine
my $accountlines = Koha::Account::Lines->search(
{
itemnumber => $item,
debit_type_code => 'OVERDUE',
status => 'LOST'
},
{
order_by => { '-desc' => 'date' },
rows => 1
}
);
return 0 unless $accountlines->count; # no warning, there's just nothing to fix
# Update status of fine
my $overdue = $accountlines->next;
$overdue->status('RETURNED')->store();
# Find related forgive credit
my $refunds = $overdue->credits(
{
credit_type_code => 'FORGIVEN',
itemnumber => $item,
status => [ { '!=' => 'VOID' }, undef ]
},
{ order_by => { '-desc' => 'date' }, rows => 1 }
);
return 0 unless $refunds->count; # no warning, there's just nothing to fix
# Revert the forgive credit
my $refund = $refunds->next;
return $refund->void();
}
);
return $result;
}
=head2 _GetCircControlBranch
my $circ_control_branch = _GetCircControlBranch($iteminfos, $borrower);

View File

@ -825,8 +825,7 @@ sub _set_found_trigger {
return $self unless $lost_age_in_days < $no_refund_after_days;
}
return $self
unless Koha::CirculationRules->get_lostreturn_policy(
my $lostreturn_policy = Koha::CirculationRules->get_lostreturn_policy(
{
item => $self,
return_branch => C4::Context->userenv
@ -835,80 +834,130 @@ sub _set_found_trigger {
}
);
# 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' ] }
if ( $lostreturn_policy ) {
# refund charge made for lost book
my $lost_charge = Koha::Account::Lines->search(
{
itemnumber => $self->itemnumber,
debit_type_code => 'LOST',
status => [ undef, { '<>' => 'FOUND' } ]
},
{
order_by => { -desc => [ 'date', 'accountlines_id' ] },
rows => 1
}
)->single;
if ( $lost_charge ) {
my $patron = $lost_charge->patron;
if ( $patron ) {
my $account = $patron->account;
my $total_to_refund = 0;
# Use cases
if ( $lost_charge->amount > $lost_charge->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 => $lost_charge->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 = $lost_charge->amountoutstanding + $total_to_refund;
my $credit;
if ( $credit_total > 0 ) {
my $branchcode =
C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;
$credit = $account->add_credit(
{
amount => $credit_total,
description => 'Item found ' . $self->itemnumber,
type => 'LOST_FOUND',
interface => C4::Context->interface,
library_id => $branchcode,
item_id => $self->itemnumber,
issue_id => $lost_charge->issue_id
}
);
$credit->apply( { debits => [$lost_charge] } );
$self->{_refunded} = 1;
}
# Update the account status
$lost_charge->status('FOUND');
$lost_charge->store();
# Reconcile balances if required
if ( C4::Context->preference('AccountAutoReconcile') ) {
$account->reconcile_balance;
}
}
}
);
return $self unless $accountlines->count > 0;
# restore fine for lost book
if ( $lostreturn_policy eq 'restore' ) {
my $lost_overdue = Koha::Account::Lines->search(
{
itemnumber => $self->itemnumber,
debit_type_code => 'OVERDUE',
status => 'LOST'
},
{
order_by => { '-desc' => 'date' },
rows => 1
}
)->single;
my $accountline = $accountlines->next;
my $total_to_refund = 0;
if ( $lost_overdue ) {
return $self unless $accountline->borrowernumber;
my $patron = $lost_overdue->patron;
if ($patron) {
my $account = $patron->account;
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 somewhere
# Update status of fine
$lost_overdue->status('FOUND')->store();
my $account = $patron->account;
# Find related forgive credit
my $refund = $lost_overdue->credits(
{
credit_type_code => 'FORGIVEN',
itemnumber => $self->itemnumber,
status => [ { '!=' => 'VOID' }, undef ]
},
{ order_by => { '-desc' => 'date' }, rows => 1 }
)->single;
# Use cases
if ( $accountline->amount > $accountline->amountoutstanding ) {
if ( $refund ) {
# Revert the forgive credit
$refund->void();
$self->{_restored} = 1;
}
# 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
# Reconcile balances if required
if ( C4::Context->preference('AccountAutoReconcile') ) {
$account->reconcile_balance;
}
}
}
);
$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;
my $credit;
if ( $credit_total > 0 ) {
my $branchcode =
C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;
$credit = $account->add_credit(
{
amount => $credit_total,
description => 'Item found ' . $self->itemnumber,
type => 'LOST_FOUND',
interface => C4::Context->interface,
library_id => $branchcode,
item_id => $self->itemnumber,
issue_id => $accountline->issue_id
}
);
$credit->apply( { debits => [$accountline] } );
$self->{_refunded} = 1;
}
# Update the account status
$accountline->status('FOUND');
$accountline->store();
if ( defined $account and C4::Context->preference('AccountAutoReconcile') ) {
$account->reconcile_balance;
} elsif ( $lostreturn_policy eq 'charge' ) {
$self->{_charge} = 1;
}
}
return $self;

View File

@ -2698,71 +2698,6 @@ subtest 'AddReturn | is_overdue' => sub {
};
};
subtest '_RestoreOverdueForLostAndFound' => sub {
plan tests => 7;
my $manager = $builder->build_object( { class => "Koha::Patrons" } );
t::lib::Mocks::mock_userenv(
{ patron => $manager, branchcode => $manager->branchcode } );
my $patron = $builder->build_object( { class => "Koha::Patrons" } );
my $item = $builder->build_sample_item();
# No fine found
my $result = C4::Circulation::_RestoreOverdueForLostAndFound( $item->itemnumber);
is($result, 0, "0 returned when no overdue found");
# Fine not forgiven
my $account = $patron->account;
my $overdue = $account->add_debit(
{
amount => 30.00,
user_id => $manager->borrowernumber,
library_id => $library2->{branchcode},
interface => 'test',
item_id => $item->itemnumber,
type => 'OVERDUE',
}
)->store();
$overdue->status('LOST')->store();
$result = C4::Circulation::_RestoreOverdueForLostAndFound( $item->itemnumber);
is($result, 0, "0 returned when overdue found without forgival");
$overdue->discard_changes;
is($overdue->status, 'RETURNED', 'Overdue status updated to RETURNED');
# Reset status
$overdue->status('LOST')->store();
# Fine forgiven
my $credit = $account->add_credit(
{
amount => 30.00,
user_id => $manager->borrowernumber,
library_id => $library2->{branchcode},
interface => 'test',
type => 'FORGIVEN',
item_id => $item->itemnumber
}
);
$credit->apply( { debits => [$overdue], offset_type => 'Forgiven' } );
$result = C4::Circulation::_RestoreOverdueForLostAndFound( $item->itemnumber);
is( ref($result), 'Koha::Account::Line', 'Return a Koha::Account::Line object on sucess');
$overdue->discard_changes;
is($overdue->status, 'RETURNED', 'Overdue status updated to RETURNED');
is($overdue->amountoutstanding, $overdue->amount, 'Overdue outstanding restored');
# Missing parameters
warning_like {
C4::Circulation::_RestoreOverdueForLostAndFound( undef )
}
qr/_RestoreOverdueForLostAndFound\(\) not supplied valid itemnumber/,
"parameter warning received for missing itemnumbernumber";
};
subtest '_FixOverduesOnReturn' => sub {
plan tests => 14;

View File

@ -138,7 +138,7 @@ subtest 'store' => sub {
};
subtest '_lost_found_trigger' => sub {
plan tests => 7;
plan tests => 10;
t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 );
t::lib::Mocks::mock_preference( 'WhenLostForgiveFine', 0 );
@ -761,6 +761,336 @@ subtest 'store' => sub {
};
subtest 'restore fine | no overdue' => sub {
plan tests => 8;
my $manager =
$builder->build_object( { class => "Koha::Patrons" } );
t::lib::Mocks::mock_userenv(
{ patron => $manager, branchcode => $manager->branchcode } );
# Set lostreturn_policy to 'restore' for tests
my $specific_rule_restore = $builder->build(
{
source => 'CirculationRule',
value => {
branchcode => $manager->branchcode,
categorycode => undef,
itemtype => undef,
rule_name => 'lostreturn',
rule_value => 'restore'
}
}
);
my $patron = $builder->build_object( { class => 'Koha::Patrons' } );
my $item = $builder->build_sample_item(
{
biblionumber => $biblio->biblionumber,
library => $library->branchcode,
replacementprice => $replacement_amount,
itype => $item_type->itemtype
}
);
my $issue =
C4::Circulation::AddIssue( $patron->unblessed, $item->barcode );
# Simulate item marked as lost
$item->itemlost(1)->store;
C4::Circulation::LostItem( $item->itemnumber, 1 );
my $processing_fee_lines = Koha::Account::Lines->search(
{
borrowernumber => $patron->id,
itemnumber => $item->itemnumber,
debit_type_code => 'PROCESSING'
}
);
is( $processing_fee_lines->count,
1, 'Only one processing fee produced' );
my $processing_fee_line = $processing_fee_lines->next;
is( $processing_fee_line->amount + 0,
$processfee_amount,
'The right PROCESSING amount is generated' );
is( $processing_fee_line->amountoutstanding + 0,
$processfee_amount,
'The right PROCESSING amountoutstanding is generated' );
my $lost_fee_lines = Koha::Account::Lines->search(
{
borrowernumber => $patron->id,
itemnumber => $item->itemnumber,
debit_type_code => 'LOST'
}
);
is( $lost_fee_lines->count, 1, 'Only one lost item fee produced' );
my $lost_fee_line = $lost_fee_lines->next;
is( $lost_fee_line->amount + 0,
$replacement_amount, 'The right LOST amount is generated' );
is( $lost_fee_line->amountoutstanding + 0,
$replacement_amount,
'The right LOST amountountstanding is generated' );
my $account = $patron->account;
my $debts = $account->outstanding_debits;
# Pay off the debt
my $credit = $account->add_credit(
{
amount => $account->balance,
type => 'PAYMENT',
interface => 'test',
}
);
$credit->apply(
{ debits => [ $debts->as_list ], offset_type => 'Payment' } );
# Simulate item marked as found
$item->itemlost(0)->store;
is( $item->{_refunded}, 1, 'Refund triggered' );
is( $item->{_restored}, undef, 'Restore not triggered when there is no overdue fine found' );
};
subtest 'restore fine | unforgiven overdue' => sub {
plan tests => 10;
# Set lostreturn_policy to 'restore' for tests
my $manager =
$builder->build_object( { class => "Koha::Patrons" } );
t::lib::Mocks::mock_userenv(
{ patron => $manager, branchcode => $manager->branchcode } );
my $specific_rule_restore = $builder->build(
{
source => 'CirculationRule',
value => {
branchcode => $manager->branchcode,
categorycode => undef,
itemtype => undef,
rule_name => 'lostreturn',
rule_value => 'restore'
}
}
);
my $patron = $builder->build_object( { class => 'Koha::Patrons' } );
my $item = $builder->build_sample_item(
{
biblionumber => $biblio->biblionumber,
library => $library->branchcode,
replacementprice => $replacement_amount,
itype => $item_type->itemtype
}
);
my $issue =
C4::Circulation::AddIssue( $patron->unblessed, $item->barcode );
# Simulate item marked as lost
$item->itemlost(1)->store;
C4::Circulation::LostItem( $item->itemnumber, 1 );
my $processing_fee_lines = Koha::Account::Lines->search(
{
borrowernumber => $patron->id,
itemnumber => $item->itemnumber,
debit_type_code => 'PROCESSING'
}
);
is( $processing_fee_lines->count,
1, 'Only one processing fee produced' );
my $processing_fee_line = $processing_fee_lines->next;
is( $processing_fee_line->amount + 0,
$processfee_amount,
'The right PROCESSING amount is generated' );
is( $processing_fee_line->amountoutstanding + 0,
$processfee_amount,
'The right PROCESSING amountoutstanding is generated' );
my $lost_fee_lines = Koha::Account::Lines->search(
{
borrowernumber => $patron->id,
itemnumber => $item->itemnumber,
debit_type_code => 'LOST'
}
);
is( $lost_fee_lines->count, 1, 'Only one lost item fee produced' );
my $lost_fee_line = $lost_fee_lines->next;
is( $lost_fee_line->amount + 0,
$replacement_amount, 'The right LOST amount is generated' );
is( $lost_fee_line->amountoutstanding + 0,
$replacement_amount,
'The right LOST amountountstanding is generated' );
my $account = $patron->account;
my $debts = $account->outstanding_debits;
# Pay off the debt
my $credit = $account->add_credit(
{
amount => $account->balance,
type => 'PAYMENT',
interface => 'test',
}
);
$credit->apply(
{ debits => [ $debts->as_list ], offset_type => 'Payment' } );
# Fine not forgiven
my $overdue = $account->add_debit(
{
amount => 30.00,
user_id => $manager->borrowernumber,
library_id => $library->branchcode,
interface => 'test',
item_id => $item->itemnumber,
type => 'OVERDUE',
}
)->store();
$overdue->status('LOST')->store();
$overdue->discard_changes;
is( $overdue->status, 'LOST',
'Overdue status set to LOST' );
# Simulate item marked as found
$item->itemlost(0)->store;
is( $item->{_refunded}, 1, 'Refund triggered' );
is( $item->{_restored}, undef, 'Restore not triggered when overdue was not forgiven' );
$overdue->discard_changes;
is( $overdue->status, 'FOUND',
'Overdue status updated to FOUND' );
};
subtest 'restore fine | forgiven overdue' => sub {
plan tests => 12;
# Set lostreturn_policy to 'restore' for tests
my $manager =
$builder->build_object( { class => "Koha::Patrons" } );
t::lib::Mocks::mock_userenv(
{ patron => $manager, branchcode => $manager->branchcode } );
my $specific_rule_restore = $builder->build(
{
source => 'CirculationRule',
value => {
branchcode => $manager->branchcode,
categorycode => undef,
itemtype => undef,
rule_name => 'lostreturn',
rule_value => 'restore'
}
}
);
my $patron = $builder->build_object( { class => 'Koha::Patrons' } );
my $item = $builder->build_sample_item(
{
biblionumber => $biblio->biblionumber,
library => $library->branchcode,
replacementprice => $replacement_amount,
itype => $item_type->itemtype
}
);
my $issue =
C4::Circulation::AddIssue( $patron->unblessed, $item->barcode );
# Simulate item marked as lost
$item->itemlost(1)->store;
C4::Circulation::LostItem( $item->itemnumber, 1 );
my $processing_fee_lines = Koha::Account::Lines->search(
{
borrowernumber => $patron->id,
itemnumber => $item->itemnumber,
debit_type_code => 'PROCESSING'
}
);
is( $processing_fee_lines->count,
1, 'Only one processing fee produced' );
my $processing_fee_line = $processing_fee_lines->next;
is( $processing_fee_line->amount + 0,
$processfee_amount,
'The right PROCESSING amount is generated' );
is( $processing_fee_line->amountoutstanding + 0,
$processfee_amount,
'The right PROCESSING amountoutstanding is generated' );
my $lost_fee_lines = Koha::Account::Lines->search(
{
borrowernumber => $patron->id,
itemnumber => $item->itemnumber,
debit_type_code => 'LOST'
}
);
is( $lost_fee_lines->count, 1, 'Only one lost item fee produced' );
my $lost_fee_line = $lost_fee_lines->next;
is( $lost_fee_line->amount + 0,
$replacement_amount, 'The right LOST amount is generated' );
is( $lost_fee_line->amountoutstanding + 0,
$replacement_amount,
'The right LOST amountountstanding is generated' );
my $account = $patron->account;
my $debts = $account->outstanding_debits;
# Pay off the debt
my $credit = $account->add_credit(
{
amount => $account->balance,
type => 'PAYMENT',
interface => 'test',
}
);
$credit->apply(
{ debits => [ $debts->as_list ], offset_type => 'Payment' } );
# Add overdue
my $overdue = $account->add_debit(
{
amount => 30.00,
user_id => $manager->borrowernumber,
library_id => $library->branchcode,
interface => 'test',
item_id => $item->itemnumber,
type => 'OVERDUE',
}
)->store();
$overdue->status('LOST')->store();
is( $overdue->status, 'LOST',
'Overdue status set to LOST' );
# Forgive fine
$credit = $account->add_credit(
{
amount => 30.00,
user_id => $manager->borrowernumber,
library_id => $library->branchcode,
interface => 'test',
type => 'FORGIVEN',
item_id => $item->itemnumber
}
);
$credit->apply(
{ debits => [$overdue], offset_type => 'Forgiven' } );
# Simulate item marked as found
$item->itemlost(0)->store;
is( $item->{_refunded}, 1, 'Refund triggered' );
is( $item->{_restored}, 1, 'Restore triggered when overdue was forgiven' );
$overdue->discard_changes;
is( $overdue->status, 'FOUND', 'Overdue status updated to FOUND' );
is( $overdue->amountoutstanding, $overdue->amount, 'Overdue outstanding has been restored' );
$credit->discard_changes;
is( $credit->status, 'VOID', 'Overdue Forgival has been marked as VOID');
};
subtest 'Continue when userenv is not set' => sub {
plan tests => 1;