Bugzilla – Attachment 84811 Details for
Bug 21747
Update C4::Overdues::UpdateFine to use Koha::Account->add_debit and Koha::Account::Line->adjust
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 21747: (follow-up) Intelligent rename of offsets
Bug-21747-follow-up-Intelligent-rename-of-offsets.patch (text/plain), 10.19 KB, created by
Martin Renvoize (ashimema)
on 2019-02-06 11:58:51 UTC
(
hide
)
Description:
Bug 21747: (follow-up) Intelligent rename of offsets
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2019-02-06 11:58:51 UTC
Size:
10.19 KB
patch
obsolete
>From d4d99892f27bbe621611a5af45f912f6e06a8a4c Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >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 <martin.renvoize@ptfs-europe.com> >--- > 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..3bfc2012f0 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 ) ? "_increment" : "_decrement"; >+ > # 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..1a92d5b5a5 >--- /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_increment' ), ( 'fine_decrement' ); >+ }); >+ >+ $dbh->do(q{ >+ UPDATE account_offsets SET type = 'fine_increment' WHERE type = 'Fine Update' AND amount > 0; >+ }); >+ >+ $dbh->do(q{ >+ UPDATE account_offsets SET type = 'fine_decrement' 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_increment' and 'fine_decrement')\n"; >+} >diff --git a/t/db_dependent/Koha/Account/Lines.t b/t/db_dependent/Koha/Account/Lines.t >index f7f0c16554..f3a44880eb 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_increment', '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_increment', '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_decrement', '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_decrement', 'Adjust type stored correctly' ); > > my $overpayment_refund = $account->lines->last; > is( $overpayment_refund->amount * 1, -10, 'A new credit has been added' ); >-- >2.20.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=84811">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 21747
:
81890
|
83130
|
83234
|
83252
|
84725
|
84726
|
84807
|
84808
|
84810
|
84811
|
84816
|
84817
|
85179
|
85180
|
85795