Bug 7447

Summary: Allow to specify a date in overdue_notice.pl
Product: Koha Reporter: Paul Poulain <paul.poulain>
Component: Command-line UtilitiesAssignee: Paul Poulain <paul.poulain>
Status: CLOSED FIXED QA Contact: Ian Walls <koha.sekjal>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, kyle.m.hall
Version: 3.10   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 11120    
Attachments: Bug 7447 allow to specify a date in overdue_notice.pl
[SIGNED-OFF] Bug 7447 allow to specify a date in overdue_notice.pl

Description Paul Poulain 2012-01-16 16:15:59 UTC

    
Comment 1 Paul Poulain 2012-01-16 16:19:39 UTC Comment hidden (obsolete)
Comment 2 Ian Walls 2012-02-07 15:17:48 UTC
This patch doesn't do any kind of format-checking on the 'date' param, and then loads the variable directly into the SQL.  This could cause the query to fail (at best), return completely different values than intended, or destroy entire tables (at very worst).

The date variable should be parameterized for the sth->execute(), and should be rigourously checked for proper date formatting.  If the incoming value is not a correct date, either warn and use NOW(), or abort the script.
Comment 3 Paul Poulain 2012-02-13 14:01:37 UTC
(In reply to comment #2)
> This patch doesn't do any kind of format-checking on the 'date' param, and then
> loads the variable directly into the SQL.  This could cause the query to fail
> (at best), return completely different values than intended, or destroy entire
> tables (at very worst).

I disagree, the following line quote & make the query safe:
+    $date=$dbh->quote($date);
It's needed, as, if there is no $date passed, then the date must be compared to NOW(), which is mad on the else:
+    $date="NOW()";


> The date variable should be parameterized for the sth->execute(), and should be
> rigourously checked for proper date formatting.  If the incoming value is not a
> correct date, either warn and use NOW(), or abort the script.

We don't do any format checking in most of the scripts, this script does not differ from others.

If you think we must enforce our coding guidelines, and it's another topic (and I tend to agree with it), that should be discussed outside from this patch.

Switching back to "needs signoff"
Comment 4 Kyle M Hall 2012-06-09 10:03:15 UTC
Created attachment 9995 [details] [review]
[SIGNED-OFF] Bug 7447 allow to specify a date in overdue_notice.pl

This patch add a new parameter to overdue_notices.pl, that is a date.
If you add --date=YYYY-MM-DD when running overdue_notices, it will generate overdues as if you were on date provided

that's usefull if you want to relaunch an overdue calculation that has failed, of after changing your circ rules

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Ian Walls 2012-06-15 15:23:49 UTC
$dbh->quote takes care of the cleanliness, so my original objection is withdrawn.  Marking Passed QA.
Comment 6 Jonathan Druart 2012-08-09 13:42:31 UTC
This patch introduces a bug, see Bug 8607