From 9ab25a29b61598b74285123bd01571b54502e285 Mon Sep 17 00:00:00 2001 From: Josef Moravec Date: Fri, 26 Oct 2018 11:04:05 +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 --- C4/Accounts.pm | 32 ---------------------- C4/Circulation.pm | 5 +--- C4/Overdues.pm | 3 -- C4/Stats.pm | 10 +++---- Koha/Account.pm | 23 ---------------- Koha/REST/V1/Patrons/Account.pm | 1 - members/printfeercpt.pl | 5 ---- members/printinvoice.pl | 1 - misc/cronjobs/staticfines.pl | 7 ++--- 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 -- 15 files changed, 17 insertions(+), 105 deletions(-) diff --git a/C4/Accounts.pm b/C4/Accounts.pm index 96db2f3..e4d1455 100644 --- a/C4/Accounts.pm +++ b/C4/Accounts.pm @@ -38,7 +38,6 @@ BEGIN { @ISA = qw(Exporter); @EXPORT = qw( &manualinvoice - &getnextacctno &chargelostitem &ReversePayment &purge_zero_balance_fees @@ -61,29 +60,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 @@ -129,7 +105,6 @@ sub chargelostitem{ my $accountline = Koha::Account::Line->new( { borrowernumber => $borrowernumber, - accountno => getnextacctno($borrowernumber), date => \'NOW()', amount => $processfee, description => $description, @@ -154,7 +129,6 @@ sub chargelostitem{ logaction("FINES", 'CREATE',$borrowernumber,Dumper({ action => 'create_fee', borrowernumber => $accountline->borrowernumber,, - accountno => $accountline->accountno, amount => $accountline->amount, description => $accountline->description, accounttype => $accountline->accounttype, @@ -170,7 +144,6 @@ sub chargelostitem{ my $accountline = Koha::Account::Line->new( { borrowernumber => $borrowernumber, - accountno => getnextacctno($borrowernumber), date => \'NOW()', amount => $replacementprice, description => $description, @@ -194,7 +167,6 @@ sub chargelostitem{ logaction("FINES", 'CREATE',$borrowernumber,Dumper({ action => 'create_fee', borrowernumber => $accountline->borrowernumber,, - accountno => $accountline->accountno, amount => $accountline->amount, description => $accountline->description, accounttype => $accountline->accounttype, @@ -240,7 +212,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; @@ -248,7 +219,6 @@ sub manualinvoice { my $accountline = Koha::Account::Line->new( { borrowernumber => $borrowernumber, - accountno => $accountno, date => \'NOW()', amount => $amount, description => $desc, @@ -273,7 +243,6 @@ sub manualinvoice { logaction("FINES", 'CREATE',$borrowernumber,Dumper({ action => 'create_fee', borrowernumber => $borrowernumber, - accountno => $accountno, amount => $amount, description => $desc, accounttype => $type, @@ -325,7 +294,6 @@ sub ReversePayment { new_amountoutstanding => $new_amountoutstanding, , accountlines_id => $accountline->id, - accountno => $accountline->accountno, manager_id => $manager_id, } ) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 479990e..53aa710 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2400,7 +2400,7 @@ sub _FixAccountForLostAndReturned { accounttype => { -in => [ 'L', 'Rep', 'W' ] }, }, { - order_by => { -desc => [ 'date', 'accountno' ] } + order_by => { -desc => [ 'date' ] } } )->next(); @@ -3187,8 +3187,6 @@ sub AddIssuingCharge { # FIXME What if checkout does not exist? - my $nextaccntno = getnextacctno($checkout->borrowernumber); - my $manager_id = 0; $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv; @@ -3199,7 +3197,6 @@ sub AddIssuingCharge { borrowernumber => $checkout->borrowernumber, itemnumber => $checkout->itemnumber, issue_id => $checkout->issue_id, - accountno => $nextaccntno, amount => $charge, amountoutstanding => $charge, manager_id => $manager_id, diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 1e67b7f..1706e1d 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -608,8 +608,6 @@ sub UpdateFine { $sth4->execute($itemnum); my $title = $sth4->fetchrow; - my $nextaccntno = C4::Accounts::getnextacctno($borrowernumber); - my $desc = ( $type ? "$type " : '' ) . "$title $due"; # FIXEDME, avoid whitespace prefix on empty $type my $accountline = Koha::Account::Line->new( @@ -622,7 +620,6 @@ sub UpdateFine { accounttype => 'FU', amountoutstanding => $amount, lastincrement => $amount, - accountno => $nextaccntno, issue_id => $issue_id, } )->store(); diff --git a/C4/Stats.pm b/C4/Stats.pm index 0437818..8a8835c 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 c7efd21..15fc870 100644 --- a/Koha/Account.pm +++ b/Koha/Account.pm @@ -85,17 +85,6 @@ sub pay { my $patron = Koha::Patrons->find( $self->{patron_id} ); - # 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; - my $manager_id = $userenv ? $userenv->{number} : 0; my @fines_paid; # List of account lines paid on with this payment @@ -143,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, } @@ -195,7 +183,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, } @@ -218,7 +205,6 @@ sub pay { my $payment = Koha::Account::Line->new( { borrowernumber => $self->{patron_id}, - accountno => $accountno, date => dt_from_string(), amount => 0 - $amount, description => $description, @@ -242,7 +228,6 @@ sub pay { type => $type, amount => $amount, borrowernumber => $self->{patron_id}, - accountno => $accountno, } ); @@ -254,7 +239,6 @@ sub pay { { action => "create_$type", borrowernumber => $self->{patron_id}, - accountno => $accountno, amount => 0 - $amount, amountoutstanding => 0 - $balance_remaining, accounttype => $account_type, @@ -349,11 +333,6 @@ sub add_credit { $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( { borrowernumber => $self->{patron_id}, @@ -382,7 +361,6 @@ sub add_credit { type => $type, amount => $amount, borrowernumber => $self->{patron_id}, - accountno => $accountno, } ) if grep { $type eq $_ } ('payment', 'writeoff') ; @@ -393,7 +371,6 @@ sub add_credit { 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 68f0788..6885653 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 4aa8bd0..b8fed81 100755 --- a/members/printfeercpt.pl +++ b/members/printfeercpt.pl @@ -54,10 +54,6 @@ my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in" my $patron = Koha::Patrons->find( $borrowernumber ); output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); -if ( $action eq 'print' ) { -# ReversePayment( $borrowernumber, $input->param('accountno') ); -} - #get account details my $total = $patron->account->balance; @@ -86,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 a6d92f7..81b5b9f 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 c57e99e..636d1c2 100755 --- a/misc/cronjobs/staticfines.pl +++ b/misc/cronjobs/staticfines.pl @@ -229,14 +229,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) + (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 470b84b..34ee55a 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 a22c36d..b637547 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 e4ba29d..b268ea4 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 ReversePayment @@ -688,7 +687,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', @@ -698,7 +696,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', @@ -708,7 +705,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 9b6908f..eee0690 100644 --- a/t/db_dependent/ILSDI_Services.t +++ b/t/db_dependent/ILSDI_Services.t @@ -196,7 +196,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 4ec1bce..c97efaf 100644 --- a/t/db_dependent/Stats.t +++ b/t/db_dependent/Stats.t @@ -3,7 +3,7 @@ use Modern::Perl; use C4::Stats; -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 c44e889..4542da6 100644 --- a/t/lib/TestBuilder.pm +++ b/t/lib/TestBuilder.pm @@ -493,9 +493,6 @@ sub _gen_default_values { AuthHeader => { marcxml => '', }, - Accountline => { - accountno => 0, - }, }; } -- 2.1.4