Bug 5952 changed the interface to C4::Members::GetPendingIssues however in circulation.pl it is now being called with an empty array in many cases so that a simple checkout is resulting in two calls the first (for the borrower succeeds) the second for the relatives fails to getting an error return from the database This is not good
Created attachment 4014 [details] [review] Proposed Patch
Is an error this patch addresses? circulation.pl: DBD::mysql::st execute failed: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'ORDER BY issues.issuedate' at line 25 at /kohaclone/C4/Members.pm line 1070
Yes its executing bad syntax like WHERE ORDER BY and the second (failing) query is unnecessary
Created attachment 4438 [details] [review] Signed-off patch Although I'm not qualified to evaluate how it does it, applying this patch quiets the errors I was seeing in the log from circulation.pl so I'm signing off.
*** Bug 6568 has been marked as a duplicate of this bug. ***
Created attachment 4935 [details] [review] proposed patch (passed QA) QA comment * works as announced, usefull patch, logs will thank you forever ;-) * Just a minor comment/suggestion. You write: +if ( @borrowernumbers ) { + $relissue = GetPendingIssues(@borrowernumbers); +} It's correct although it's more perlish to have : $relissue = GetPendingIssues(@borrowernumbers) if @borrowernumbers; I agree with the change - ($_->{date_due} lt $today) and $_->{overdue} = 1; to + if ($_->{date_due} and $_->{date_due} lt $today) { + $_->{overdue} = 1; + } (we must be Perlish, but sometimes that result in really cryptic things...) passed QA
Paul it is considered bad practice in many perl circles these days to use postfix if except for flow of control statements e.g. " LAST if $x;". They cause problems because people often miss them when rereading code. The default policies in perlcritic will warn against them see Perl Best Practices for a rationale
I agree with Colin, we should follow Perl best practices where ever we can. And if you look at bug 6679, you will see we are extending the Perl::Critic tests slowly to cover everything.
Pushed, please test
*** Bug 6805 has been marked as a duplicate of this bug. ***