From b9ecf361a82511a09b5487af0efd7f0d50a2b43e Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 5 Feb 2020 11:54:20 +0000 Subject: [PATCH] Bug 24592: Reword LOST_RETURN to LOST_FOUND MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This patch updates the wording in the 'lost and found' process to more closely reflect what the process is achieving by replacing 'RETURNED' with 'FOUND' Test plan: 1) Grep codebase for _FixAccountForLostAndReturned and note there are no longer any instanced of it. 2) Run t/db_dependent/Circulation.t and note it passes 3) Test returning/renewing an item that has been marked as lost and note the updated values in the accountlines now use LOST_FOUND as credit_type_code and 'FOUND' as the status for the 'LOST' fee (debit_type_code 'LOST') Signed-off-by: David Nind Signed-off-by: Joonas Kylmälä Signed-off-by: Martin Renvoize --- C4/Circulation.pm | 24 +++++---- Koha/Account.pm | 4 +- .../definitions/patron_account_credit.json | 2 +- .../prog/en/includes/accounts.inc | 2 +- .../bootstrap/en/includes/account-table.inc | 2 +- t/db_dependent/Circulation.t | 50 +++++++++---------- 6 files changed, 43 insertions(+), 41 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 379f1f46b5..dc12cd3352 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1455,7 +1455,7 @@ sub AddIssue { ) ) { - _FixAccountForLostAndReturned( $item_object->itemnumber, undef, + _FixAccountForLostAndFound( $item_object->itemnumber, undef, $item_object->barcode ); } } @@ -2037,7 +2037,7 @@ sub AddReturn { ) ) { - _FixAccountForLostAndReturned( $item->itemnumber, + _FixAccountForLostAndFound( $item->itemnumber, $borrowernumber, $barcode ); $messages->{'LostItemFeeRefunded'} = 1; } @@ -2443,9 +2443,9 @@ sub _FixOverduesOnReturn { return $result; } -=head2 _FixAccountForLostAndReturned +=head2 _FixAccountForLostAndFound - &_FixAccountForLostAndReturned($itemnumber, [$borrowernumber, $barcode]); + &_FixAccountForLostAndFound($itemnumber, [$borrowernumber, $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 @@ -2455,7 +2455,7 @@ Internal function, not exported, called only by AddReturn. =cut -sub _FixAccountForLostAndReturned { +sub _FixAccountForLostAndFound { my $itemnumber = shift or return; my $borrowernumber = @_ ? shift : undef; my $item_id = @_ ? shift : $itemnumber; # Send the barcode if you want that logged in the description @@ -2467,7 +2467,7 @@ sub _FixAccountForLostAndReturned { { itemnumber => $itemnumber, debit_type_code => 'LOST', - status => [ undef, { '<>' => 'RETURNED' } ] + status => [ undef, { '<>' => 'FOUND' } ] }, { order_by => { -desc => [ 'date', 'accountlines_id' ] } @@ -2506,11 +2506,13 @@ sub _FixAccountForLostAndReturned { if ( $credit_total > 0 ) { my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; $credit = $account->add_credit( - { amount => $credit_total, - description => 'Item Returned ' . $item_id, - type => 'LOST_RETURN', + { + amount => $credit_total, + description => 'Item found ' . $item_id, + type => 'LOST_FOUND', interface => C4::Context->interface, - library_id => $branchcode + library_id => $branchcode, + item_id => $itemnumber } ); @@ -2518,7 +2520,7 @@ sub _FixAccountForLostAndReturned { } # Update the account status - $accountline->discard_changes->status('RETURNED'); + $accountline->discard_changes->status('FOUND'); $accountline->store; if ( defined $account and C4::Context->preference('AccountAutoReconcile') ) { diff --git a/Koha/Account.pm b/Koha/Account.pm index ae6bebbb7a..6fc464178f 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -326,7 +326,7 @@ $credit_type can be any of: - 'CREDIT' - 'PAYMENT' - 'FORGIVEN' - - 'LOST_RETURN' + - 'LOST_FOUND' - 'WRITEOFF' =cut @@ -741,7 +741,7 @@ sub reconcile_balance { our $offset_type = { 'CREDIT' => 'Manual Credit', 'FORGIVEN' => 'Writeoff', - 'LOST_RETURN' => 'Lost Item', + 'LOST_FOUND' => 'Lost Item Found', 'PAYMENT' => 'Payment', 'WRITEOFF' => 'Writeoff', 'ACCOUNT' => 'Account Fee', diff --git a/api/v1/swagger/definitions/patron_account_credit.json b/api/v1/swagger/definitions/patron_account_credit.json index a231471c21..ba156fe9ff 100644 --- a/api/v1/swagger/definitions/patron_account_credit.json +++ b/api/v1/swagger/definitions/patron_account_credit.json @@ -3,7 +3,7 @@ "properties": { "credit_type": { "type": "string", - "description": "Type of credit ('CREDIT', 'FORGIVEN', 'LOST_RETURN', 'PAYMENT', 'WRITEOFF' )" + "description": "Type of credit ('CREDIT', 'FORGIVEN', 'LOST_FOUND', 'PAYMENT', 'WRITEOFF' )" }, "amount": { "type": "number", diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/accounts.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/accounts.inc index 162a641e57..172acc6347 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/accounts.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/accounts.inc @@ -6,7 +6,7 @@ [%- CASE 'WRITEOFF' -%]Writeoff [%- CASE 'FORGIVEN' -%]Forgiven [%- CASE 'CREDIT' -%]Credit - [%- CASE 'LOST_RETURN' -%]Lost item fee refund + [%- CASE 'LOST_FOUND' -%]Lost item fee refund [%- CASE 'Refund' -%]Refund [%- CASE -%][% account.credit_type.description | html %] [%- END -%] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/account-table.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/account-table.inc index 8f38a70d7d..a193b11a4c 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/account-table.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/account-table.inc @@ -175,7 +175,7 @@ [%- CASE 'WRITEOFF' -%]Writeoff [%- CASE 'FORGIVEN' -%]Forgiven [%- CASE 'CREDIT' -%]Credit - [%- CASE 'LOST_RETURN' -%]Lost item fee refund + [%- CASE 'LOST_FOUND' -%]Lost item fee refund [%- CASE -%][% account.credit_type.description | html %] [%- END -%] [%- ELSIF account.debit_type_code -%] diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 38502941dd..08822d765c 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -2409,7 +2409,7 @@ subtest 'AddReturn | is_overdue' => sub { } }; -subtest '_FixAccountForLostAndReturned' => sub { +subtest '_FixAccountForLostAndFound' => sub { plan tests => 5; @@ -2487,14 +2487,14 @@ subtest '_FixAccountForLostAndReturned' => sub { ); $credit->apply( { debits => [ $debts->as_list ], offset_type => 'Writeoff' } ); - my $credit_return_id = C4::Circulation::_FixAccountForLostAndReturned( $item->itemnumber, $patron->id ); - is( $credit_return_id, undef, 'No LOST_RETURN account line added' ); + my $credit_return_id = C4::Circulation::_FixAccountForLostAndFound( $item->itemnumber, $patron->id ); + is( $credit_return_id, undef, 'No LOST_FOUND account line added' ); $lost_fee_line->discard_changes; # reload from DB is( $lost_fee_line->amountoutstanding + 0, 0, 'Lost fee has no outstanding amount' ); is( $lost_fee_line->debit_type_code, 'LOST', 'Lost fee now still has account type of LOST' ); - is( $lost_fee_line->status, 'RETURNED', "Lost fee now has account status of RETURNED"); + is( $lost_fee_line->status, 'FOUND', "Lost fee now has account status of FOUND"); is( $patron->account->balance, -0, 'The patron balance is 0, everything was written off' ); }; @@ -2549,20 +2549,20 @@ subtest '_FixAccountForLostAndReturned' => sub { ); $credit->apply( { debits => [ $debts->as_list ], offset_type => 'Payment' } ); - my $credit_return_id = C4::Circulation::_FixAccountForLostAndReturned( $item->itemnumber, $patron->id ); + my $credit_return_id = C4::Circulation::_FixAccountForLostAndFound( $item->itemnumber, $patron->id ); my $credit_return = Koha::Account::Lines->find($credit_return_id); - is( $credit_return->credit_type_code, 'LOST_RETURN', 'An account line of type LOST_RETURN is added' ); + is( $credit_return->credit_type_code, 'LOST_FOUND', 'An account line of type LOST_FOUND is added' ); is( $credit_return->amount + 0, - -99.00, 'The account line of type LOST_RETURN has an amount of -99' ); + -99.00, 'The account line of type LOST_FOUND has an amount of -99' ); is( $credit_return->amountoutstanding + 0, - -99.00, 'The account line of type LOST_RETURN has an amountoutstanding of -99' ); + -99.00, 'The account line of type LOST_FOUND has an amountoutstanding of -99' ); $lost_fee_line->discard_changes; is( $lost_fee_line->amountoutstanding + 0, 0, 'Lost fee has no outstanding amount' ); is( $lost_fee_line->debit_type_code, 'LOST', 'Lost fee now still has account type of LOST' ); - is( $lost_fee_line->status, 'RETURNED', "Lost fee now has account status of RETURNED"); + is( $lost_fee_line->status, 'FOUND', "Lost fee now has account status of FOUND"); is( $patron->account->balance, -99, 'The patron balance is -99, a credit that equals the lost fee payment' ); @@ -2607,18 +2607,18 @@ subtest '_FixAccountForLostAndReturned' => sub { is( $lost_fee_line->amountoutstanding + 0, $replacement_amount, 'The right LOST amountountstanding is generated' ); - my $credit_return_id = C4::Circulation::_FixAccountForLostAndReturned( $item->itemnumber, $patron->id ); + my $credit_return_id = C4::Circulation::_FixAccountForLostAndFound( $item->itemnumber, $patron->id ); my $credit_return = Koha::Account::Lines->find($credit_return_id); - is( $credit_return->credit_type_code, 'LOST_RETURN', 'An account line of type LOST_RETURN is added' ); - is( $credit_return->amount + 0, -99.00, 'The account line of type LOST_RETURN has an amount of -99' ); - is( $credit_return->amountoutstanding + 0, 0, 'The account line of type LOST_RETURN has an amountoutstanding of 0' ); + is( $credit_return->credit_type_code, 'LOST_FOUND', 'An account line of type LOST_FOUND is added' ); + is( $credit_return->amount + 0, -99.00, 'The account line of type LOST_FOUND has an amount of -99' ); + is( $credit_return->amountoutstanding + 0, 0, 'The account line of type LOST_FOUND has an amountoutstanding of 0' ); $lost_fee_line->discard_changes; is( $lost_fee_line->amountoutstanding + 0, 0, 'Lost fee has no outstanding amount' ); is( $lost_fee_line->debit_type_code, 'LOST', 'Lost fee now still has account type of LOST' ); - is( $lost_fee_line->status, 'RETURNED', "Lost fee now has account status of RETURNED"); + is( $lost_fee_line->status, 'FOUND', "Lost fee now has account status of FOUND"); is( $patron->account->balance, 20, 'The patron balance is 20, still owes the processing fee' ); }; @@ -2693,25 +2693,25 @@ subtest '_FixAccountForLostAndReturned' => sub { $lost_fee_line->discard_changes; my $outstanding = $lost_fee_line->amountoutstanding; - my $credit_return_id = C4::Circulation::_FixAccountForLostAndReturned( $item->itemnumber, $patron->id ); + my $credit_return_id = C4::Circulation::_FixAccountForLostAndFound( $item->itemnumber, $patron->id ); my $credit_return = Koha::Account::Lines->find($credit_return_id); - is( $account->balance, $processfee_amount - $payment_amount, 'Balance is PROCESSING - PAYMENT (LOST_RETURN)' ); + is( $account->balance, $processfee_amount - $payment_amount, 'Balance is PROCESSING - PAYMENT (LOST_FOUND)' ); $lost_fee_line->discard_changes; is( $lost_fee_line->amountoutstanding + 0, 0, 'Lost fee has no outstanding amount' ); is( $lost_fee_line->debit_type_code, 'LOST', 'Lost fee now still has account type of LOST' ); - is( $lost_fee_line->status, 'RETURNED', "Lost fee now has account status of RETURNED"); + is( $lost_fee_line->status, 'FOUND', "Lost fee now has account status of FOUND"); - is( $credit_return->credit_type_code, 'LOST_RETURN', 'An account line of type LOST_RETURN is added' ); + is( $credit_return->credit_type_code, 'LOST_FOUND', 'An account line of type LOST_FOUND is added' ); is( $credit_return->amount + 0, ($payment_amount + $outstanding ) * -1, - 'The account line of type LOST_RETURN has an amount equal to the payment + outstanding' + 'The account line of type LOST_FOUND has an amount equal to the payment + outstanding' ); is( $credit_return->amountoutstanding + 0, $payment_amount * -1, - 'The account line of type LOST_RETURN has an amountoutstanding equal to the payment' + 'The account line of type LOST_FOUND has an amountoutstanding equal to the payment' ); is( $account->balance, @@ -2789,10 +2789,10 @@ subtest '_FixAccountForLostAndReturned' => sub { t::lib::Mocks::mock_preference( 'AccountAutoReconcile', 1 ); - my $credit_return_id = C4::Circulation::_FixAccountForLostAndReturned( $item_id, $patron->id ); + my $credit_return_id = C4::Circulation::_FixAccountForLostAndFound( $item_id, $patron->id ); my $credit_return = Koha::Account::Lines->find($credit_return_id); - is( $account->balance, $manual_debit_amount - $payment_amount, 'Balance is PROCESSING - payment (LOST_RETURN)' ); + is( $account->balance, $manual_debit_amount - $payment_amount, 'Balance is PROCESSING - payment (LOST_FOUND)' ); my $manual_debit = Koha::Account::Lines->search({ borrowernumber => $patron->id, debit_type_code => 'OVERDUE', status => 'UNRETURNED' })->next; is( $manual_debit->amountoutstanding + 0, $manual_debit_amount - $payment_amount, 'reconcile_balance was called' ); @@ -2866,7 +2866,7 @@ subtest '_FixOverduesOnReturn' => sub { is( $credit->amountoutstanding + 0, 0, "Credit amountoutstanding is correctly set to 0" ); }; -subtest '_FixAccountForLostAndReturned returns undef if patron is deleted' => sub { +subtest '_FixAccountForLostAndFound returns undef if patron is deleted' => sub { plan tests => 1; my $manager = $builder->build_object({ class => "Koha::Patrons" }); @@ -2902,9 +2902,9 @@ subtest '_FixAccountForLostAndReturned returns undef if patron is deleted' => su $patron->delete(); - my $return_value = C4::Circulation::_FixAccountForLostAndReturned( $patron->id, $item->itemnumber ); + my $return_value = C4::Circulation::_FixAccountForLostAndFound( $patron->id, $item->itemnumber ); - is( $return_value, undef, "_FixAccountForLostAndReturned returns undef if patron is deleted" ); + is( $return_value, undef, "_FixAccountForLostAndFound returns undef if patron is deleted" ); }; -- 2.39.5