From a031e0988a9be28ebd395771f0a832f9032e6c96 Mon Sep 17 00:00:00 2001 From: Josef Moravec 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 Signed-off-by: Martin Renvoize --- C4/Accounts.pm | 27 ------------------- C4/Circulation.pm | 2 +- C4/Stats.pm | 8 +++--- 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, 17 insertions(+), 91 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 1a78ddc208..6020c9278a 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2392,7 +2392,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..50dd7c24a5 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; @@ -133,12 +131,12 @@ sub UpdateStats { branch, type, value, other, itemnumber, itemtype, location, borrowernumber, proccode, ccode) - VALUES (now(),?,?,?,?,?,?,?,?,?,?)" + 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 c8f63807b0..ccc1265f19 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 3fd3eef398..c6c39ac5ab 100644 --- a/Koha/REST/V1/Patrons/Account.pm +++ b/Koha/REST/V1/Patrons/Account.pm @@ -224,7 +224,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 be2a83491e..c427efb3c8 100644 --- a/t/lib/TestBuilder.pm +++ b/t/lib/TestBuilder.pm @@ -556,9 +556,6 @@ sub _gen_default_values { AuthHeader => { marcxml => '', }, - Accountline => { - accountno => 0, - }, }; } -- 2.20.1