From 1a385cae8d8e84cdf48824c8897545ff5e25471c Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 4 Feb 2019 14:29:08 +0000 Subject: [PATCH] Bug 21747: (follow-up) Intelligent rename of offsets This patch intelligently renames the account_offset types for updateing fines from `Fine Update` to `fine_increment` and `fine_decrement` depending on the sign of the calculated difference of the adjustment. Signed-off-by: Martin Renvoize Signed-off-by: Tomas Cohen Arazi Signed-off-by: Nick Clemens --- C4/Overdues.pm | 5 +--- Koha/Account/Line.pm | 23 ++++++++----------- .../data/mysql/atomicupdate/bug_21747.perl | 21 +++++++++++++++++ t/db_dependent/Koha/Account/Lines.t | 18 +++++++-------- 4 files changed, 41 insertions(+), 26 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_21747.perl diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 7f0394f720..de23dd906d 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -573,12 +573,9 @@ sub UpdateFine { } if ( $data ) { - # we're updating an existing fine. Only modify if amount changed - # Note that in the current implementation, you cannot pay against an accruing fine - # (i.e. , of accounttype 'FU'). Doing so will break accrual. if ( $data->{'amount'} != $amount ) { my $accountline = Koha::Account::Lines->find( $data->{accountlines_id} ); - $accountline->adjust({ amount => $amount, type => 'fine_increment' }); + $accountline->adjust({ amount => $amount, type => 'fine_update' }); } } else { if ( $amount ) { # Don't add new fines with an amount of 0 diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm index df5d98b016..a93cb81d2c 100644 --- a/Koha/Account/Line.pm +++ b/Koha/Account/Line.pm @@ -220,7 +220,7 @@ This method allows updating a debit or credit on a patron's account ); $update_type can be any of: - - fine_increment + - fine_update Authors Note: The intention here is that this method is only used to adjust accountlines where the final amount is not yet known/fixed. @@ -236,7 +236,7 @@ sub adjust { my $amount = $params->{amount}; my $update_type = $params->{type}; - unless ( exists($Koha::Account::Line::offset_type->{$update_type}) ) { + unless ( exists($Koha::Account::Line::allowed_update->{$update_type}) ) { Koha::Exceptions::Account::UnrecognisedType->throw( error => 'Update type not recognised' ); @@ -259,6 +259,9 @@ sub adjust { my $difference = $amount - $amount_before; my $new_outstanding = $amount_outstanding_before + $difference; + my $offset_type = substr( $update_type, 0, index( $update_type, '_' ) ); + $offset_type .= ( $difference > 0 ) ? "_increase" : "_decrease"; + # Catch cases that require patron refunds if ( $new_outstanding < 0 ) { my $account = @@ -268,7 +271,7 @@ sub adjust { amount => $new_outstanding * -1, description => 'Overpayment refund', type => 'credit', - ( $update_type eq 'fine_increment' ? ( item_id => $self->itemnumber ) : ()), + ( $update_type eq 'fine_update' ? ( item_id => $self->itemnumber ) : ()), } ); $new_outstanding = 0; @@ -280,7 +283,7 @@ sub adjust { date => \'NOW()', amount => $amount, amountoutstanding => $new_outstanding, - ( $update_type eq 'fine_increment' ? ( lastincrement => $difference ) : ()), + ( $update_type eq 'fine_update' ? ( lastincrement => $difference ) : ()), } )->store(); @@ -288,7 +291,7 @@ sub adjust { my $account_offset = Koha::Account::Offset->new( { debit_id => $self->id, - type => $Koha::Account::Line::offset_type->{$update_type}, + type => $offset_type, amount => $difference } )->store(); @@ -310,7 +313,7 @@ sub adjust { manager_id => undef, } ) - ) if ( $update_type eq 'fine_increment' ); + ) if ( $update_type eq 'fine_update' ); } } ); @@ -358,17 +361,11 @@ sub _type { =head2 Name mappings -=head3 $offset_type - -=cut - -our $offset_type = { 'fine_increment' => 'Fine Update', }; - =head3 $allowed_update =cut -our $allowed_update = { 'fine_increment' => 'FU', }; +our $allowed_update = { 'fine_update' => 'FU', }; =head1 AUTHORS diff --git a/installer/data/mysql/atomicupdate/bug_21747.perl b/installer/data/mysql/atomicupdate/bug_21747.perl new file mode 100644 index 0000000000..3cdaf8a958 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_21747.perl @@ -0,0 +1,21 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + $dbh->do(q{ + INSERT IGNORE INTO account_offset_types ( type ) VALUES ( 'fine_increase' ), ( 'fine_decrease' ); + }); + + $dbh->do(q{ + UPDATE account_offsets SET type = 'fine_increase' WHERE type = 'Fine Update' AND amount > 0; + }); + + $dbh->do(q{ + UPDATE account_offsets SET type = 'fine_decrease' WHERE type = 'Fine Update' AND amount < 0; + }); + + $dbh->do(q{ + DELETE FROM account_offset_types WHERE type = 'Fine Update'; + }); + + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug 21747 - Update account_offset_types to include 'fine_increase' and 'fine_decrease')\n"; +} diff --git a/t/db_dependent/Koha/Account/Lines.t b/t/db_dependent/Koha/Account/Lines.t index f7f0c16554..ec0743f3e7 100755 --- a/t/db_dependent/Koha/Account/Lines.t +++ b/t/db_dependent/Koha/Account/Lines.t @@ -336,12 +336,12 @@ subtest 'adjust() tests' => sub { throws_ok { $debit_1->adjust( { amount => 50, type => 'bad' } ) } qr/Update type not recognised/, 'Exception thrown for unrecognised type'; - throws_ok { $debit_1->adjust( { amount => 50, type => 'fine_increment' } ) } + throws_ok { $debit_1->adjust( { amount => 50, type => 'fine_update' } ) } qr/Update type not allowed on this accounttype/, 'Exception thrown for type conflict'; # Increment an unpaid fine - $debit_2->adjust( { amount => 150, type => 'fine_increment' } )->discard_changes; + $debit_2->adjust( { amount => 150, type => 'fine_update' } )->discard_changes; is( $debit_2->amount * 1, 150, 'Fine amount was updated in full' ); is( $debit_2->amountoutstanding * 1, 150, 'Fine amountoutstanding was update in full' ); @@ -352,7 +352,7 @@ subtest 'adjust() tests' => sub { is( $offsets->count, 1, 'An offset is generated for the increment' ); my $THIS_offset = $offsets->next; is( $THIS_offset->amount * 1, 50, 'Amount was calculated correctly (increment by 50)' ); - is( $THIS_offset->type, 'Fine Update', 'Adjust type stored correctly' ); + is( $THIS_offset->type, 'fine_increase', 'Adjust type stored correctly' ); is( $schema->resultset('ActionLog')->count(), $action_logs + 0, 'No log was added' ); @@ -368,7 +368,7 @@ subtest 'adjust() tests' => sub { t::lib::Mocks::mock_preference( 'FinesLog', 1 ); # Increment the partially paid fine - $debit_2->adjust( { amount => 160, type => 'fine_increment' } )->discard_changes; + $debit_2->adjust( { amount => 160, type => 'fine_update' } )->discard_changes; is( $debit_2->amount * 1, 160, 'Fine amount was updated in full' ); is( $debit_2->amountoutstanding * 1, 120, 'Fine amountoutstanding was updated by difference' ); @@ -378,12 +378,12 @@ subtest 'adjust() tests' => sub { is( $offsets->count, 3, 'An offset is generated for the increment' ); $THIS_offset = $offsets->last; is( $THIS_offset->amount * 1, 10, 'Amount was calculated correctly (increment by 10)' ); - is( $THIS_offset->type, 'Fine Update', 'Adjust type stored correctly' ); + is( $THIS_offset->type, 'fine_increase', 'Adjust type stored correctly' ); is( $schema->resultset('ActionLog')->count(), $action_logs + 1, 'Log was added' ); # Decrement the partially paid fine, less than what was paid - $debit_2->adjust( { amount => 50, type => 'fine_increment' } )->discard_changes; + $debit_2->adjust( { amount => 50, type => 'fine_update' } )->discard_changes; is( $debit_2->amount * 1, 50, 'Fine amount was updated in full' ); is( $debit_2->amountoutstanding * 1, 10, 'Fine amountoutstanding was updated by difference' ); @@ -393,10 +393,10 @@ subtest 'adjust() tests' => sub { is( $offsets->count, 4, 'An offset is generated for the decrement' ); $THIS_offset = $offsets->last; is( $THIS_offset->amount * 1, -110, 'Amount was calculated correctly (decrement by 110)' ); - is( $THIS_offset->type, 'Fine Update', 'Adjust type stored correctly' ); + is( $THIS_offset->type, 'fine_decrease', 'Adjust type stored correctly' ); # Decrement the partially paid fine, more than what was paid - $debit_2->adjust( { amount => 30, type => 'fine_increment' } )->discard_changes; + $debit_2->adjust( { amount => 30, type => 'fine_update' } )->discard_changes; is( $debit_2->amount * 1, 30, 'Fine amount was updated in full' ); is( $debit_2->amountoutstanding * 1, 0, 'Fine amountoutstanding was zeroed (payment was 40)' ); is( $debit_2->lastincrement * 1, -20, 'lastincrement is the to the right value' ); @@ -405,7 +405,7 @@ subtest 'adjust() tests' => sub { is( $offsets->count, 5, 'An offset is generated for the decrement' ); $THIS_offset = $offsets->last; is( $THIS_offset->amount * 1, -20, 'Amount was calculated correctly (decrement by 20)' ); - is( $THIS_offset->type, 'Fine Update', 'Adjust type stored correctly' ); + is( $THIS_offset->type, 'fine_decrease', 'Adjust type stored correctly' ); my $overpayment_refund = $account->lines->last; is( $overpayment_refund->amount * 1, -10, 'A new credit has been added' ); -- 2.39.5