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>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

diff --git a/C4/Overdues.pm b/C4/Overdues.pm
index 7f0394f..de23dd9 100644 (file)
--- 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 df5d98b..a93cb81 100644 (file)
--- 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 (file)
index 0000000..3cdaf8a
--- /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 f7f0c16..ec0743f 100755 (executable)
--- 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' );