https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21683 --- Comment #4 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 81282 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=81282 Bug 21683: Remove accountlines.accountno Review of attachment 81282: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=21683&attachment=81282) ----------------------------------------------------------------- Some of these are issues. Some of them are reminders, at least for me, to look more closely in places that might have side-effects. ::: C4/Circulation.pm @@ +2400,4 @@
accounttype => { -in => [ 'L', 'Rep', 'W' ] }, }, { + order_by => { -desc => [ 'date' ] }
accountno was being used to keep them in the same order they were added. I would have substituted accountlines_id for the same effect. I noticed the other patched files were. ::: C4/Stats.pm @@ +130,5 @@
(datetime, branch, type, value, other, itemnumber, itemtype, location, + borrowernumber, ccode) + VALUES (now(),?,?,?,?,?,?,?,?,?)"
Strangely enough, I think the defaulting to null is better for the statistics for fine/charges. accountno was NOT really a "type of procedure used when making payments". But I'm going to have to read the code as a whole after application to make sure this didn't mess things up for other statistics. ::: Koha/Account.pm @@ -93,5 @@
- { - order_by => 'accountno' - } - )->next(); - my $accountno = $last ? $last->accountno + 1 : 1;
This was what I was thinking should be it's only function in the Koha namespace, before notions of removing it came out. @@ -351,5 @@
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;
See how it was put in multiple places? That's why. ::: members/printfeercpt.pl @@ -56,5 @@
-if ( $action eq 'print' ) { -# ReversePayment( $borrowernumber, $input->param('accountno') ); -} -
Scope creep! ;) -- but totally fine. ::: misc/maintenance/fix_accountlines_date.pl @@ -127,4 @@
our $dbh = C4::Context->dbh; $dbh->{AutoCommit} = 0; my $sth = $dbh->prepare(" -SELECT borrowernumber, itemnumber, accountno, description
Will need to read applied patch to see if removing itemnumber from query has impact. ::: misc/maintenance/fix_accountlines_rmdupfines_bug8253.pl @@ +98,4 @@
}
my $sql = + "DELETE FROM accountlines WHERE accountlines_id = ? LIMIT 1";
accountlines_id is auto increment and thus unique. There is no need for LIMIT 1. @@ +103,5 @@
}
if ($has_changed) { my $sql = + "UPDATE accountlines SET amountoutstanding = ? WHERE accountlines_id = ? LIMIT 1";
accountlines_id is auto increment and thus unique. There is no need for LIMIT 1. ::: t/db_dependent/Stats.t @@ -120,4 @@
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");
Removed test, could have changed test to confirm it is null/undef. -- You are receiving this mail because: You are watching all bug changes.