Bug 21683: Remove accountlines.accountno
authorJosef Moravec <josef.moravec@gmail.com>
Fri, 22 Feb 2019 14:27:21 +0000 (14:27 +0000)
committerNick Clemens <nick@bywatersolutions.com>
Thu, 21 Mar 2019 18:19:22 +0000 (18:19 +0000)
Test plan:
1) Play with fines, should work OK
2) Try to print receipts on fines - prinfeercpt.pl, printinvoice.pl
3) git grep getnextacctno -> no occurences
4) git grep accountno should return only:
  installer/data/mysql/atomicupdate/bug_21683_remove_column_accountno.perl
  installer/data/mysql/update22to30.pl
  misc/release_notes/release_notes_3_10_0.txt
  misc/release_notes/release_notes_3_22_0.txt
5) prove
  t/db_dependent/Accounts.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Stats.t
  t/db_dependent/Koha/Account.t

Rescued-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>

14 files changed:
C4/Accounts.pm
C4/Circulation.pm
C4/Stats.pm
Koha/Account.pm
Koha/REST/V1/Patrons/Account.pm
members/printfeercpt.pl
members/printinvoice.pl
misc/cronjobs/staticfines.pl
misc/maintenance/fix_accountlines_date.pl
misc/maintenance/fix_accountlines_rmdupfines_bug8253.pl
t/db_dependent/Accounts.t
t/db_dependent/ILSDI_Services.t
t/db_dependent/Stats.t
t/lib/TestBuilder.pm

index 0339e68..3fdf2d7 100644 (file)
@@ -38,7 +38,6 @@ BEGIN {
     require Exporter;
     @ISA    = qw(Exporter);
     @EXPORT = qw(
-      &getnextacctno
       &chargelostitem
       &purge_zero_balance_fees
     );
@@ -60,29 +59,6 @@ patron.
 
 =head1 FUNCTIONS
 
-=head2 getnextacctno
-
-  $nextacct = &getnextacctno($borrowernumber);
-
-Returns the next unused account number for the patron with the given
-borrower number.
-
-=cut
-
-#'
-# FIXME - Okay, so what does the above actually _mean_?
-sub getnextacctno {
-    my ($borrowernumber) = shift or return;
-    my $sth = C4::Context->dbh->prepare(
-        "SELECT accountno+1 FROM accountlines
-            WHERE    (borrowernumber = ?)
-            ORDER BY accountno DESC
-            LIMIT 1"
-    );
-    $sth->execute($borrowernumber);
-    return ($sth->fetchrow || 1);
-}
-
 =head2 chargelostitem
 
 In a default install of Koha the following lost values are set
@@ -174,7 +150,6 @@ sub manualinvoice {
     $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv;
     my $dbh      = C4::Context->dbh;
     my $insert;
-    my $accountno  = getnextacctno($borrowernumber);
     my $amountleft = $amount;
 
     my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;
@@ -182,7 +157,6 @@ sub manualinvoice {
     my $accountline = Koha::Account::Line->new(
         {
             borrowernumber    => $borrowernumber,
-            accountno         => $accountno,
             date              => \'NOW()',
             amount            => $amount,
             description       => $desc,
@@ -207,7 +181,6 @@ sub manualinvoice {
         logaction("FINES", 'CREATE',$borrowernumber,Dumper({
             action            => 'create_fee',
             borrowernumber    => $borrowernumber,
-            accountno         => $accountno,
             amount            => $amount,
             description       => $desc,
             accounttype       => $type,
index b74fdff..e01ddd9 100644 (file)
@@ -2385,7 +2385,7 @@ sub _FixAccountForLostAndReturned {
             accounttype => { -in => [ 'L', 'Rep', 'W' ] },
         },
         {
-            order_by => { -desc => [ 'date', 'accountno' ] }
+            order_by => { -desc => [ 'date' ] }
         }
     );
 
index 0437818..8a8835c 100644 (file)
@@ -64,7 +64,6 @@ C<$params> is an hashref whose expected keys are:
     amount             : the amount of the transaction
     other              : sipmode
     itemtype           : the type of the item
-    accountno          : the count
     ccode              : the collection code of the item
 
 type key is mandatory.
@@ -83,7 +82,7 @@ sub UpdateStats {
 # make some controls
     return () if ! defined $params;
 # change these arrays if new types of transaction or new parameters are allowed
-    my @allowed_keys = qw (type branch amount other itemnumber itemtype borrowernumber accountno ccode location);
+    my @allowed_keys = qw (type branch amount other itemnumber itemtype borrowernumber ccode location);
     my @allowed_circulation_types = qw (renew issue localuse return onsite_checkout);
     my @allowed_accounts_types = qw (writeoff payment);
     my @circulation_mandatory_keys = qw (type branch borrowernumber itemnumber ccode itemtype);
@@ -123,7 +122,6 @@ sub UpdateStats {
     my $other             = exists $params->{other}          ? $params->{other}          : '';
     my $itemtype          = exists $params->{itemtype}       ? $params->{itemtype}       : '';
     my $location          = exists $params->{location}       ? $params->{location}       : undef;
-    my $accountno         = exists $params->{accountno}      ? $params->{accountno}      : '';
     my $ccode             = exists $params->{ccode}          ? $params->{ccode}          : '';
 
     my $dbh = C4::Context->dbh;
@@ -132,13 +130,13 @@ sub UpdateStats {
         (datetime,
          branch,          type,        value,
          other,           itemnumber,  itemtype, location,
-         borrowernumber,  proccode,    ccode)
-         VALUES (now(),?,?,?,?,?,?,?,?,?,?)"
+         borrowernumber,  ccode)
+         VALUES (now(),?,?,?,?,?,?,?,?,?)"
     );
     $sth->execute(
         $branch,     $type,     $amount,   $other,
         $itemnumber, $itemtype, $location, $borrowernumber,
-        $accountno,  $ccode
+        $ccode
     );
 }
 
index ef8a20f..ddd6012 100644 (file)
@@ -85,12 +85,6 @@ sub pay {
 
     my $patron = Koha::Patrons->find( $self->{patron_id} );
 
-    # We should remove accountno, it is no longer needed
-    my $last = $self->lines->search(
-        {},
-        { order_by => 'accountno' } )->next();
-    my $accountno = $last ? $last->accountno + 1 : 1;
-
     my $manager_id = $userenv ? $userenv->{number} : 0;
 
     my @fines_paid; # List of account lines paid on with this payment
@@ -138,7 +132,6 @@ sub pay {
                         new_amountoutstanding => 0,
                         amount_paid           => $old_amountoutstanding,
                         accountlines_id       => $fine->id,
-                        accountno             => $fine->accountno,
                         manager_id            => $manager_id,
                         note                  => $note,
                     }
@@ -189,7 +182,6 @@ sub pay {
                         new_amountoutstanding => $fine->amountoutstanding,
                         amount_paid           => $amount_to_pay,
                         accountlines_id       => $fine->id,
-                        accountno             => $fine->accountno,
                         manager_id            => $manager_id,
                         note                  => $note,
                     }
@@ -212,7 +204,6 @@ sub pay {
     my $payment = Koha::Account::Line->new(
         {
             borrowernumber    => $self->{patron_id},
-            accountno         => $accountno,
             date              => dt_from_string(),
             amount            => 0 - $amount,
             description       => $description,
@@ -236,7 +227,6 @@ sub pay {
             type           => $type,
             amount         => $amount,
             borrowernumber => $self->{patron_id},
-            accountno      => $accountno,
         }
     );
 
@@ -248,7 +238,6 @@ sub pay {
                 {
                     action            => "create_$type",
                     borrowernumber    => $self->{patron_id},
-                    accountno         => $accountno,
                     amount            => 0 - $amount,
                     amountoutstanding => 0 - $balance_remaining,
                     accounttype       => $account_type,
@@ -343,16 +332,10 @@ sub add_credit {
 
     $schema->txn_do(
         sub {
-            # We should remove accountno, it is no longer needed
-            my $last = $self->lines->search(
-                {},
-                { order_by => 'accountno' } )->next();
-            my $accountno = $last ? $last->accountno + 1 : 1;
 
             # Insert the account line
             $line = Koha::Account::Line->new(
                 {   borrowernumber    => $self->{patron_id},
-                    accountno         => $accountno,
                     date              => \'NOW()',
                     amount            => $amount,
                     description       => $description,
@@ -380,7 +363,6 @@ sub add_credit {
                     type           => $type,
                     amount         => $amount,
                     borrowernumber => $self->{patron_id},
-                    accountno      => $accountno,
                 }
             ) if grep { $type eq $_ } ('payment', 'writeoff') ;
 
@@ -391,7 +373,6 @@ sub add_credit {
                     Dumper(
                         {   action            => "create_$type",
                             borrowernumber    => $self->{patron_id},
-                            accountno         => $accountno,
                             amount            => $amount,
                             description       => $description,
                             amountoutstanding => $amount,
@@ -476,10 +457,6 @@ sub add_debit {
 
     $schema->txn_do(
         sub {
-            # We should remove accountno, it is no longer needed
-            my $last = Koha::Account::Lines->search( { borrowernumber => $self->{patron_id} },
-                { order_by => 'accountno' } )->next();
-            my $accountno = $last ? $last->accountno + 1 : 1;
 
             # Insert the account line
             $line = Koha::Account::Line->new(
@@ -514,7 +491,6 @@ sub add_debit {
                     Dumper(
                         {   action            => "create_$type",
                             borrowernumber    => $self->{patron_id},
-                            accountno         => $accountno,
                             amount            => $amount,
                             description       => $description,
                             amountoutstanding => $amount,
index deb4d3d..b0d623b 100644 (file)
@@ -226,7 +226,6 @@ sub _to_model {
 
 our $to_api_mapping = {
     accountlines_id   => 'account_line_id',
-    accountno         => undef,                  # removed
     accounttype       => 'account_type',
     amountoutstanding => 'amount_outstanding',
     borrowernumber    => 'patron_id',
index 6333330..b8fed81 100755 (executable)
@@ -82,7 +82,6 @@ my %row = (
     'description'             => $accountline->{'description'},
     'amount'                  => $accountline->{'amount'},
     'amountoutstanding'       => $accountline->{'amountoutstanding'},
-    'accountno' => $accountline->{'accountno'},
     accounttype => $accountline->{accounttype},
     'note'      => $accountline->{'note'},
 );
index a6d92f7..81b5b9f 100755 (executable)
@@ -81,7 +81,6 @@ my %row = (
     'amount'                  => sprintf( "%.2f", $accountline->{'amount'} ),
     'amountoutstanding' =>
       sprintf( "%.2f", $accountline->{'amountoutstanding'} ),
-    'accountno' => $accountline->{'accountno'},
     accounttype => $accountline->{accounttype},
     'note'      => $accountline->{'note'},
 );
index bee1172..5803c89 100755 (executable)
@@ -226,14 +226,13 @@ for ( my $i = 0 ; $i < scalar(@$data) ; $i++ ) {
             $sth4->execute($itemnumber);
             my $title = $sth4->fetchrow;
 
-            my $nextaccntno = C4::Accounts::getnextacctno($borrowernumber);
             my $desc        = "staticfine";
             my $query       = "INSERT INTO accountlines
-                        (borrowernumber,itemnumber,date,amount,description,accounttype,amountoutstanding,lastincrement,accountno)
-                                VALUES (?,?,now(),?,?,'F',?,?,?)";
+                        (borrowernumber,itemnumber,date,amount,description,accounttype,amountoutstanding,lastincrement)
+                                VALUES (?,?,now(),?,?,'F',?,?)";
             my $sth2 = $dbh->prepare($query);
-            $bigdebug and warn "query: $query\nw/ args: $borrowernumber, $itemnumber, $amount, $desc, $amount, $amount, $nextaccntno\n";
-            $sth2->execute( $borrowernumber, $itemnumber, $amount, $desc, $amount, $amount, $nextaccntno );
+            $bigdebug and warn "query: $query\nw/ args: $borrowernumber, $itemnumber, $amount, $desc, $amount, $amount\n";
+            $sth2->execute( $borrowernumber, $itemnumber, $amount, $desc, $amount, $amount );
 
         }
     }
index 470b84b..34ee55a 100755 (executable)
@@ -127,7 +127,7 @@ if (not $result or $want_help or ($mode ne 'us' and $mode ne 'metric')) {
 our $dbh = C4::Context->dbh;
 $dbh->{AutoCommit} = 0;
 my $sth = $dbh->prepare("
-SELECT borrowernumber, itemnumber, accountno, description
+SELECT accountlines_id, description
   FROM accountlines
   WHERE accounttype in ('FU', 'F', 'O', 'M')
 ;");
@@ -136,7 +136,7 @@ $sth->execute();
 my $update_sth = $dbh->prepare('
 UPDATE accountlines
   SET description = ?
-  WHERE borrowernumber = ? AND itemnumber = ? AND accountno = ?
+  WHERE accountlines_id = ?
 ;');
 
 
@@ -161,7 +161,7 @@ while (my $accountline = $sth->fetchrow_hashref) {
     }
 
     print "Changing description from '" . $accountline->{'description'} . "' to '" . $description . "'\n" if $DEBUG;
-    $update_sth->execute($description, $accountline->{'borrowernumber'}, $accountline->{'itemnumber'}, $accountline->{'accountno'});
+    $update_sth->execute($description, $accountline->{'accountlines_id'});
 
     $done++;
 
index a22c36d..b637547 100755 (executable)
@@ -65,7 +65,7 @@ my $query = "
     SELECT * FROM accountlines
     WHERE ( accounttype =  'FU' OR accounttype =  'F' )
     AND description like '%23:59%'
-    ORDER BY borrowernumber, itemnumber, accountno, description
+    ORDER BY borrowernumber, itemnumber, accountlines_id, description
 ";
 my $sth = $dbh->prepare($query);
 $sth->execute();
@@ -98,20 +98,16 @@ foreach my $keeper (@$results) {
         }
 
         my $sql =
-            "DELETE FROM accountlines WHERE borrowernumber = ? AND accountno = ? AND itemnumber = ? AND date = ? AND description = ? LIMIT 1";
-        $dbh->do( $sql, undef, $f->{'borrowernumber'},
-            $f->{'accountno'}, $f->{'itemnumber'}, $f->{'date'},
-            $f->{'description'} );
+            "DELETE FROM accountlines WHERE accountlines_id = ? LIMIT 1";
+        $dbh->do( $sql, undef, $f->{'accountlines_id'} );
     }
 
     if ($has_changed) {
         my $sql =
-            "UPDATE accountlines SET amountoutstanding = ? WHERE borrowernumber = ? AND accountno = ? AND itemnumber = ? AND date = ? AND description = ? LIMIT 1";
+            "UPDATE accountlines SET amountoutstanding = ? WHERE accountlines_id = ? LIMIT 1";
         $dbh->do(
             $sql,                           undef,
-            $keeper->{'amountoutstanding'}, $keeper->{'borrowernumber'},
-            $keeper->{'accountno'},         $keeper->{'itemnumber'},
-            $keeper->{'date'},              $keeper->{'description'}
+            $keeper->{'amountoutstanding'}, $keeper->{'accountlines_id'}
         );
     }
 }
index f9c99bb..6af3a3d 100644 (file)
@@ -41,7 +41,6 @@ BEGIN {
 
 can_ok( 'C4::Accounts',
     qw(
-        getnextacctno
         chargelostitem
         manualinvoice
         purge_zero_balance_fees )
@@ -756,7 +755,6 @@ subtest "Koha::Account::non_issues_charges tests" => sub {
     Koha::Account::Line->new(
         {
             borrowernumber    => $patron->borrowernumber,
-            accountno         => 1,
             date              => $today,
             description       => 'a Res fee',
             accounttype       => 'Res',
@@ -766,7 +764,6 @@ subtest "Koha::Account::non_issues_charges tests" => sub {
     Koha::Account::Line->new(
         {
             borrowernumber    => $patron->borrowernumber,
-            accountno         => 2,
             date              => $today,
             description       => 'a Rental fee',
             accounttype       => 'Rent',
@@ -776,7 +773,6 @@ subtest "Koha::Account::non_issues_charges tests" => sub {
     Koha::Account::Line->new(
         {
             borrowernumber    => $patron->borrowernumber,
-            accountno         => 3,
             date              => $today,
             description       => 'a Manual invoice fee',
             accounttype       => 'Copie',
index 8d8db44..1c93ba9 100644 (file)
@@ -197,7 +197,6 @@ subtest 'GetPatronInfo/GetBorrowerAttributes test for extended patron attributes
             source => 'Accountline',
             value  => {
                 borrowernumber    => $brwr->{borrowernumber},
-                accountno         => 1,
                 accounttype       => 'xxx',
                 amountoutstanding => 10
             }
index 76e9735..7e2e6f7 100644 (file)
@@ -4,7 +4,7 @@ use Modern::Perl;
 use C4::Stats;
 use Koha::Database;
 
-use Test::More tests => 19;
+use Test::More tests => 18;
 
 BEGIN {
     use_ok('C4::Stats');
@@ -32,7 +32,6 @@ my $params = {
               other => "bla",
               itemtype => "BK",
               location => "LOC",
-              accountno => 51,
               ccode => "CODE",
 };
 my $return_error;
@@ -105,7 +104,6 @@ $params = {
               other => "bla",
               itemtype => "BK",
               location => "LOC",
-              accountno => 51,
               ccode => "CODE",
               type => "return"
 };
@@ -120,7 +118,6 @@ cmp_ok($params->{amount},'==', $line->{value},          "UpdateStats save amount
 is ($params->{other},          $line->{other},          "UpdateStats save other param in other field of statistics table");
 is ($params->{itemtype},       $line->{itemtype},       "UpdateStats save itemtype param in itemtype field of statistics table");
 is ($params->{location},       $line->{location},       "UpdateStats save location param in location field of statistics table");
-is ($params->{accountno},      $line->{proccode},       "UpdateStats save accountno param in proccode field of statistics table");
 is ($params->{ccode},          $line->{ccode},          "UpdateStats save ccode param in ccode field of statistics table");
 
 $dbh->do(q|DELETE FROM statistics|);
@@ -131,7 +128,6 @@ $params = {
     amount         => 5.1,
     other          => "bla",
     itemtype       => "BK",
-    accountno      => 51,
     ccode          => "CODE",
     type           => "return"
 };
@@ -151,7 +147,6 @@ $params = {
     other          => "bla",
     itemtype       => "BK",
     location       => undef,
-    accountno      => 51,
     ccode          => "CODE",
     type           => "return"
 };
index 286fde7..260da2b 100644 (file)
@@ -561,9 +561,6 @@ sub _gen_default_values {
         AuthHeader => {
             marcxml => '',
         },
-        Accountline => {
-            accountno => 0,
-        },
     };
 }