Attachment 86231 Details for Bug 21683 – Bug 21683: Remove accountlines.accountno

[patch] Bug 21683: Remove accountlines.accountno

Bug-21683-Remove-accountlinesaccountno.patch (text/plain), 20.96 KB, created by Martin Renvoize (ashimema) on 2019-03-07 08:16:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2019-03-07 08:16:23 UTC

Size: 20.96 KB

patch

obsolete

>From d868324c39a0c5dff8dd60ce3f483aec66f22750 Mon Sep 17 00:00:00 2001
>From: Josef Moravec <josef.moravec@gmail.com>
>Date: Fri, 22 Feb 2019 14:27:21 +0000
>Subject: [PATCH] Bug 21683: Remove accountlines.accountno
>
>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>
>---
> C4/Accounts.pm                                | 27 -------------------
> C4/Circulation.pm                             |  2 +-
> C4/Stats.pm                                   | 10 +++----
> Koha/Account.pm                               | 24 -----------------
> Koha/REST/V1/Patrons/Account.pm               |  1 -
> members/printfeercpt.pl                       |  1 -
> members/printinvoice.pl                       |  1 -
> misc/cronjobs/staticfines.pl                  |  9 +++----
> misc/maintenance/fix_accountlines_date.pl     |  6 ++---
> .../fix_accountlines_rmdupfines_bug8253.pl    | 14 ++++------
> t/db_dependent/Accounts.t                     |  4 ---
> t/db_dependent/ILSDI_Services.t               |  1 -
> t/db_dependent/Stats.t                        |  7 +----
> t/lib/TestBuilder.pm                          |  3 ---
> 14 files changed, 18 insertions(+), 92 deletions(-)
>
>diff --git a/C4/Accounts.pm b/C4/Accounts.pm
>index 0339e68f9b..3fdf2d71b3 100644
>--- a/C4/Accounts.pm
>+++ b/C4/Accounts.pm
>@@ -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,
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 3bee144e17..c2c685b08a 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -2429,7 +2429,7 @@ sub _FixAccountForLostAndReturned {
>             accounttype => { -in => [ 'L', 'Rep', 'W' ] },
>         },
>         {
>-            order_by => { -desc => [ 'date', 'accountno' ] }
>+            order_by => { -desc => [ 'date' ] }
>         }
>     );
> 
>diff --git a/C4/Stats.pm b/C4/Stats.pm
>index 04378184b3..8a8835c346 100644
>--- a/C4/Stats.pm
>+++ b/C4/Stats.pm
>@@ -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
>     );
> }
> 
>diff --git a/Koha/Account.pm b/Koha/Account.pm
>index ef8a20f4eb..ddd60127ef 100644
>--- a/Koha/Account.pm
>+++ b/Koha/Account.pm
>@@ -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,
>diff --git a/Koha/REST/V1/Patrons/Account.pm b/Koha/REST/V1/Patrons/Account.pm
>index deb4d3d58c..b0d623b719 100644
>--- a/Koha/REST/V1/Patrons/Account.pm
>+++ b/Koha/REST/V1/Patrons/Account.pm
>@@ -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',
>diff --git a/members/printfeercpt.pl b/members/printfeercpt.pl
>index 6333330f70..b8fed81551 100755
>--- a/members/printfeercpt.pl
>+++ b/members/printfeercpt.pl
>@@ -82,7 +82,6 @@ my %row = (
>     'description'             => $accountline->{'description'},
>     'amount'                  => $accountline->{'amount'},
>     'amountoutstanding'       => $accountline->{'amountoutstanding'},
>-    'accountno' => $accountline->{'accountno'},
>     accounttype => $accountline->{accounttype},
>     'note'      => $accountline->{'note'},
> );
>diff --git a/members/printinvoice.pl b/members/printinvoice.pl
>index a6d92f7e3a..81b5b9f54e 100755
>--- a/members/printinvoice.pl
>+++ b/members/printinvoice.pl
>@@ -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'},
> );
>diff --git a/misc/cronjobs/staticfines.pl b/misc/cronjobs/staticfines.pl
>index bee1172482..5803c89a2a 100755
>--- a/misc/cronjobs/staticfines.pl
>+++ b/misc/cronjobs/staticfines.pl
>@@ -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 );
> 
>         }
>     }
>diff --git a/misc/maintenance/fix_accountlines_date.pl b/misc/maintenance/fix_accountlines_date.pl
>index 470b84b1ae..34ee55a544 100755
>--- a/misc/maintenance/fix_accountlines_date.pl
>+++ b/misc/maintenance/fix_accountlines_date.pl
>@@ -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++;
> 
>diff --git a/misc/maintenance/fix_accountlines_rmdupfines_bug8253.pl b/misc/maintenance/fix_accountlines_rmdupfines_bug8253.pl
>index a22c36dd27..b637547ef4 100755
>--- a/misc/maintenance/fix_accountlines_rmdupfines_bug8253.pl
>+++ b/misc/maintenance/fix_accountlines_rmdupfines_bug8253.pl
>@@ -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'}
>         );
>     }
> }
>diff --git a/t/db_dependent/Accounts.t b/t/db_dependent/Accounts.t
>index 2dfe97b1e7..27fef79596 100644
>--- a/t/db_dependent/Accounts.t
>+++ b/t/db_dependent/Accounts.t
>@@ -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',
>diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t
>index 4ec7f81c4b..95243e9e7e 100644
>--- a/t/db_dependent/ILSDI_Services.t
>+++ b/t/db_dependent/ILSDI_Services.t
>@@ -197,7 +197,6 @@ subtest 'GetPatronInfo/GetBorrowerAttributes test for extended patron attributes
>             source => 'Accountline',
>             value  => {
>                 borrowernumber    => $brwr->{borrowernumber},
>-                accountno         => 1,
>                 accounttype       => 'xxx',
>                 amountoutstanding => 10
>             }
>diff --git a/t/db_dependent/Stats.t b/t/db_dependent/Stats.t
>index 76e973527f..7e2e6f7b6c 100644
>--- a/t/db_dependent/Stats.t
>+++ b/t/db_dependent/Stats.t
>@@ -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"
> };
>diff --git a/t/lib/TestBuilder.pm b/t/lib/TestBuilder.pm
>index f452d66375..5d7a0c1adc 100644
>--- a/t/lib/TestBuilder.pm
>+++ b/t/lib/TestBuilder.pm
>@@ -558,9 +558,6 @@ sub _gen_default_values {
>         AuthHeader => {
>             marcxml => '',
>         },
>-        Accountline => {
>-            accountno => 0,
>-        },
>     };
> }
> 
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 21683: 81281 | 81282 | 81283 | 82183 | 82184 | 82185 | 82188 | 85531 | 85532 | 85533 | 85534 | 85535 | 85536 | 85537 | 85538 | 86221 | 86222 | 86223 | 86224 | 86225 | 86226 | 86227 | 86228 | 86229 | 86230 | 86231 | 86232 | 86233 | 86234 | 86336 | 86470 | 86471 | 86472 | 86473 | 86474 | 86475