https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21683
--- Comment #4 from M. Tompsett <[email protected]> --- 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. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
