From 25e1fcc43ee3b0b4030163415f483a6a1baf7699 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 5 Jul 2021 14:16:30 +0100 Subject: [PATCH] Bug 22435: (follow-up) Set CREATE offset for credits to positive The history view of an accountline shows increases and decreases in the amount outstanding from the perspective of the credit/debit line you are inspecting. Apply creates a negative offset amount which acts to 'reduce' both the outstanding 'debt' for the debit line and the outstanding 'credit' for the credit line. Starting with a negative offset for the credit thus makes this nonsenical. Void creates positive offset amounts which act to 'restore' both the outstanding 'debt' for the debit line and the outstanding 'credit' for the credit line. Signed-off-by: Martin Renvoize Signed-off-by: Tomas Cohen Arazi --- Koha/Account.pm | 2 +- installer/data/mysql/atomicupdate/bug_22435.perl | 1 + t/db_dependent/Accounts.t | 4 ++-- t/db_dependent/Koha/Account.t | 16 +++++++++++----- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/Koha/Account.pm b/Koha/Account.pm index 962f6e14fa..fe2465cae1 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -242,7 +242,7 @@ sub add_credit { { credit_id => $line->id, type => 'CREATE', - amount => $amount + amount => $amount * -1 } )->store(); diff --git a/installer/data/mysql/atomicupdate/bug_22435.perl b/installer/data/mysql/atomicupdate/bug_22435.perl index 9564445b38..2862574b71 100644 --- a/installer/data/mysql/atomicupdate/bug_22435.perl +++ b/installer/data/mysql/atomicupdate/bug_22435.perl @@ -10,6 +10,7 @@ if( CheckVersion( $DBversion ) ) { # Update offset_types to 'CREATE' where appropriate $dbh->do( "UPDATE account_offsets SET type = 'CREATE' WHERE type != 'OVERDUE_INCREASE' AND type != 'OVERDUE_DECREASE' AND ( debit_id IS NULL OR credit_id IS NULL)" ); + $dbh->do( "UPDATE account_offsets SET amount = ABS(amount) WHERE type = 'CREATE'" ); # Update offset_types to 'APPLY' where appropriate $dbh->do( "UPDATE account_offsets SET type = 'APPLY' WHERE type != 'OVERDUE_INCREASE' AND type != 'OVERDUE_DECREASE' AND type != 'CREATE' AND type != 'VOID'" ); diff --git a/t/db_dependent/Accounts.t b/t/db_dependent/Accounts.t index c1ad715168..f468cd98e0 100755 --- a/t/db_dependent/Accounts.t +++ b/t/db_dependent/Accounts.t @@ -376,7 +376,7 @@ subtest "More Koha::Account::pay tests" => sub { is( $offsets->count, 2, 'Two offsets recorded'); my $offset = $offsets->next; is( $offset->type(), 'CREATE', 'First offset type is CREATE' ); - is( $offset->amount+0, -100, 'First offset amount is -100.00' ); + is( $offset->amount+0, 100, 'First offset amount is 100.00' ); $offset = $offsets->next; is( $offset->type(), 'APPLY', 'Second offset type is APPLY' ); is( $offset->amount+0, -100, 'Second offset amount is -100.00' ); @@ -449,7 +449,7 @@ subtest "Even more Koha::Account::pay tests" => sub { is( $offsets->count, 2, 'Two offsets recorded'); my $offset = $offsets->next; is( $offset->type(), 'CREATE', 'First offset type is CREATE' ); - is( $offset->amount+0, -60, 'First offset amount is -60.00' ); + is( $offset->amount+0, 60, 'First offset amount is 60.00' ); $offset = $offsets->next; is( $offset->type(), 'APPLY', 'Second offset type is APPLY' ); is( $offset->amount+0, -60, 'Second offset amount is -60.00' ); diff --git a/t/db_dependent/Koha/Account.t b/t/db_dependent/Koha/Account.t index a66af97723..fa36c4b835 100755 --- a/t/db_dependent/Koha/Account.t +++ b/t/db_dependent/Koha/Account.t @@ -201,7 +201,7 @@ subtest 'outstanding_credits() tests' => sub { subtest 'add_credit() tests' => sub { - plan tests => 17; + plan tests => 21; $schema->storage->txn_begin; @@ -219,7 +219,8 @@ subtest 'add_credit() tests' => sub { throws_ok { $account->add_credit( - { amount => 25, + { + amount => 25, description => 'Payment of 25', library_id => $patron->branchcode, note => 'not really important', @@ -231,7 +232,8 @@ subtest 'add_credit() tests' => sub { 'Koha::Exceptions::MissingParameter', 'Exception thrown if interface parameter missing'; my $line_1 = $account->add_credit( - { amount => 25, + { + amount => 25, description => 'Payment of 25', library_id => $patron->branchcode, note => 'not really important', @@ -245,6 +247,7 @@ subtest 'add_credit() tests' => sub { is( $schema->resultset('ActionLog')->count(), $action_logs + 0, 'No log was added' ); is( $schema->resultset('Statistic')->count(), $statistics + 1, 'Action added to statistics' ); is( $line_1->credit_type_code, 'PAYMENT', 'Account type is correctly set' ); + ok( $line_1->amount < 0, 'Credit amount is stored as a negative' ); # Enable logs t::lib::Mocks::mock_preference( 'FinesLog', 1 ); @@ -263,6 +266,7 @@ subtest 'add_credit() tests' => sub { is( $schema->resultset('ActionLog')->count(), $action_logs + 1, 'Log was added' ); is( $schema->resultset('Statistic')->count(), $statistics + 2, 'Action added to statistics' ); is( $line_2->credit_type_code, 'PAYMENT', 'Account type is correctly set' ); + ok( $line_1->amount < 0, 'Credit amount is stored as a negative' ); # offsets have the credit_id set to accountlines_id, and debit_id is undef my $offset_1 = Koha::Account::Offsets->search({ credit_id => $line_1->id })->next; @@ -270,8 +274,10 @@ subtest 'add_credit() tests' => sub { is( $offset_1->credit_id, $line_1->id, 'No debit_id is set for credits' ); is( $offset_1->debit_id, undef, 'No debit_id is set for credits' ); + ok( $offset_1->amount > 0, 'Credit creation offset is a positive' ); is( $offset_2->credit_id, $line_2->id, 'No debit_id is set for credits' ); is( $offset_2->debit_id, undef, 'No debit_id is set for credits' ); + ok( $offset_2->amount > 0, 'Credit creation offset is a positive' ); my $line_3 = $account->add_credit( { @@ -1392,7 +1398,7 @@ subtest 'Koha::Account::payin_amount() tests' => sub { is( $offsets->count, 4, 'Four offsets generated' ); my $offset = $offsets->next; is( $offset->type, 'CREATE', 'CREATE offset added for payin line' ); - is( $offset->amount * 1, -10, 'Correct offset amount recorded' ); + is( $offset->amount * 1, 10, 'Correct offset amount recorded' ); $offset = $offsets->next; is( $offset->debit_id, $debit_1->id, "Offset added against debit_1"); is( $offset->type, 'APPLY', "APPLY used for offset_type" ); @@ -1425,7 +1431,7 @@ subtest 'Koha::Account::payin_amount() tests' => sub { is( $offsets->count, 2, 'Two offsets generated' ); $offset = $offsets->next; is( $offset->type, 'CREATE', 'CREATE offset added for payin line' ); - is( $offset->amount * 1, -2.50, 'Correct offset amount recorded' ); + is( $offset->amount * 1, 2.50, 'Correct offset amount recorded' ); $offset = $offsets->next; is( $offset->debit_id, $debit_5->id, "Offset added against debit_5"); is( $offset->type, 'APPLY', "APPLY used for offset_type" ); -- 2.20.1