Runreport.pl has a major limitation in that it is not capable of running reports that use sql parameters: Runreport will run this: SELECT p.surname, p.firstname, p.email, p.cardnumber, h.waitingdate AS 'hold date', i.barcode, b.title FROM reserves h LEFT JOIN borrowers p USING (borrowernumber) LEFT JOIN items i USING (itemnumber) LEFT JOIN biblio b ON (i.biblionumber=b.biblionumber) WHERE h.waitingdate IS NOT NULL AND h.branchcode='ABC' but if your report looks like this: SELECT p.surname, p.firstname, p.email, p.cardnumber, h.waitingdate AS 'hold date', i.barcode, b.title FROM reserves h LEFT JOIN borrowers p USING (borrowernumber) LEFT JOIN items i USING (itemnumber) LEFT JOIN biblio b ON (i.biblionumber=b.biblionumber) WHERE h.waitingdate IS NOT NULL AND h.branchcode=<<Waiting at|branches>> We should be able to supply a --sql_params argument one or more times to provide the sql parameters. runreport.pl --to="someone@someplace.com" --subject="Patrons with Holds Waiting at Library" --format=csv --sql_params="ABC" 1
Created attachment 78434 [details] [review] Bug 16219: Add 'params' parameter to runreport.pl To test: 1 - Apply patch 2 - Write a report that takes no variables 3 - Run this using runreport.pl perl misc/cronjobs/runreport.pl 1 4 - Confirm results as expected 5 - Write a report that takes one or more variables 6 - Run using runreport.pl, but pass no params 7 - You should get a SQL error 8 - Now run passing the correct number of params perl misc/cronjobs/runreport.pl --param first --param=2nd 2 9 - Verify results as expected 10 - call runreport with no parameters perl misc/cronjobs/runreport.pl 11 - Verify documentation is correct and helpful
Created attachment 90363 [details] [review] Bug 16219: Add 'params' parameter to runreport.pl To test: 1 - Apply patch 2 - Write a report that takes no variables 3 - Run this using runreport.pl perl misc/cronjobs/runreport.pl 1 4 - Confirm results as expected 5 - Write a report that takes one or more variables 6 - Run using runreport.pl, but pass no params 7 - You should get a SQL error 8 - Now run passing the correct number of params perl misc/cronjobs/runreport.pl --param first --param=2nd 2 9 - Verify results as expected 10 - call runreport with no parameters perl misc/cronjobs/runreport.pl 11 - Verify documentation is correct and helpful Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
This works, but feels a little hacky: - When the report expects params, the SQL message is output. We could possibly print a nicer error message instead with the number of params to expected. - When the report is built with reusable variables, you need to enter each of those on command line. If not easy to fix, hinting at it in the help might be good. My example query was: select * from items where holdingbranch = <<b|branches>> and homebranch = <<b|branches>> and itype= <<i>> So b will be shown as 1 input field, but will be required twice in the command: ./misc/cronjobs/runreport.pl 1 --param=CPL --param=BK DBD::mysql::st execute failed: called with 4 bind variables when 5 are needed [for Statement "select * from items where holdingbranch = ? and homebranch = ? and itype= ? LIMIT ?, ?" with ParamValues: 0=Null!, 1=Null!, 2=Null!, 3=Null!, 4=Null!] at /home/vagrant/kohaclone/C4/Reports/Guided.pm line 544. NO OUTPUT: 0 results from execute_query ./misc/cronjobs/runreport.pl 1 --param=CPL --param=CPL --param=BK Works. Pushing back to Nick for comment.
Created attachment 91668 [details] [review] Bug 16219: (follow-up) Nicer message if params missing and note that they are not combined
(In reply to Katrin Fischer from comment #3) > This works, but feels a little hacky: > - When the report expects params, the SQL message is output. We could > possibly print a nicer error message instead with the number of params to > expected. Ok, done > - When the report is built with reusable variables, you need to enter each > of those on command line. If not easy to fix, hinting at it in the help > might be good. Noted in the POD, really reports needs an update and move to objects, this is a helpful patch mostly for sysadmins (and to prevent libraries having to have hardcoded cron version reports and staff side version with params)
QA: Looking here
Created attachment 92113 [details] [review] Bug 16219: Add 'params' parameter to runreport.pl To test: 1 - Apply patch 2 - Write a report that takes no variables 3 - Run this using runreport.pl perl misc/cronjobs/runreport.pl 1 4 - Confirm results as expected 5 - Write a report that takes one or more variables 6 - Run using runreport.pl, but pass no params 7 - You should get a SQL error 8 - Now run passing the correct number of params perl misc/cronjobs/runreport.pl --param first --param=2nd 2 9 - Verify results as expected 10 - call runreport with no parameters perl misc/cronjobs/runreport.pl 11 - Verify documentation is correct and helpful Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 92114 [details] [review] Bug 16219: (follow-up) Nicer message if params missing and note that they are not combined Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 92115 [details] [review] Bug 16219: (QA follow-up) Tiny regex change The parameter <<>> does not work. There should be text inside. The construction .*? does not look good completely. Replacing it by the stricter [^>]+ So, no greater than's and at least one. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Marcel de Rooy from comment #9) > Created attachment 92115 [details] [review] [review] > Bug 16219: (QA follow-up) Tiny regex change > > The parameter <<>> does not work. There should be text inside. > The construction .*? does not look good completely. > Replacing it by the stricter [^>]+ So, no greater than's and at least one. > > Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Thanks Marcel!
Nice work! Pushed to master for 19.11.00
Small but powerfull enhancement, I choose to backport. Pushed to 19.05.x for 19.05.04
enhancement, not backporting to 18.11.x