I tested the patch following your test plan but json output remains empty (just []) when i call report requiring parameters.
By cons, if i call a report without parameters, i get a json response.

I tested with the following sql query :

SELECT COUNT(s.datetime) as "Total" FROM statistics AS s JOIN borrowers AS b ON s.borrowernumber=b.borrowernumber WHERE (year(s.datetime)=<<Année>> AND s.type = "issue") AND s.branch = <<Bibliothèque|branches>>

Tested on master (3.11.00.201)

Did you specify the parameters in the URL when calling the report? Maybe you can share the URL?

Yes i specified paramesters in URL like below (my report ID is 49) :
cgi-bin/koha/svc/report?id=49&sql_params=2012&sql_params=BU

I can't share url because i tested from a local install. I will try to test it from a live install.

I tested on live install, same behavior :-(
Below are the URLs :

with parameters :
http://pro-kohapreprod.bu.uhb.fr/cgi-bin/koha/svc/report?id=108&sql_params=2012&sql_params=BU

without :
http://pro-kohapreprod.bu.uhb.fr/cgi-bin/koha/svc/report?id=109

I precise that reports are public.

And it works in the report interface with those values?

yes, normally the response of report #108 should be the same as report #109.

Hm I tested it too and it does look like the params are not replaced in the SQL:

[Sun Apr 07 13:21:18 2013] [error] [client 127.0.0.1] [Sun Apr  7 13:21:18 2013] report: 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 '<<holding|branches>> and homebranch = <<home|branches>> LIMIT 0, 10' at line 1 at /home/katrin/kohaclone/C4/Reports/Guided.pm line 507.
[Sun Apr 07 13:21:18 2013] [error] [client 127.0.0.1] [Sun Apr  7 13:21:18 2013] report: DBD::mysql::st fetchall_arrayref failed: fetch() without execute() at /home/katrin/kohaclone/svc/report line 60.


My report is:
select * from items where holdingbranch = <<holding|branches>> and homebranch = <<home|branches>>

Used URL:
http://localhost:8080/cgi-bin/koha/svc/report?id=2&sql_params=MPL&sql_params=MPL

Chris, could you maybe take another look at this? I think it would be a really nice and helpful feature.

(In reply to Katrin Fischer from comment #9)
> Chris, could you maybe take another look at this? I think it would be a
> really nice and helpful feature.

Maybe, it's just something I was doing on my own time, and I don't have much of that at the moment.

Try it without the <<something|something>> construct.
Because that is for making a dropdown, which we obviously can't do

Merge conflict in opac/svc/report

From a QA perspective, should this be using placeholders rather than quote? I know I've had a finger wagged at be once or twice for using quote and/or quote_identifier.

(In reply to Kyle M Hall from comment #15)
> From a QA perspective, should this be using placeholders rather than quote?
> I know I've had a finger wagged at be once or twice for using quote and/or
> quote_identifier.

Perhaps, but it is using the exact same code as the reports engine on the staff side, so that it gives the same behaviour. I don't like to mix refactoring and new features on the same bug.

I'm afraid I can't get this to work for more than one parameter?

I have the below report to grab 'New Items added between two dates', running through the normal report interface I can varify I do get responses for the query, but running through svc interface I get zero results (i.e [] is the response)

URLs and Report detailed below:

svc URL used:
http://koha-staff.koha-ptfs.co.uk/cgi-bin/koha/svc/report?id=69&sql_params=2010-03-19&sql_params=2010-11-18

report URL used:
http://koha-staff.koha-ptfs.co.uk/cgi-bin/koha/reports/guided_reports.pl?reports=69&phase=Run+this+report&sql_params=2010-03-19&sql_params=2010-11-18

report used:
SELECT  biblio.title,biblio.author,biblio.datecreated,biblioitems.isbn,items.barcode,items.location FROM items LEFT JOIN biblioitems on (items.biblioitemnumber=biblioitems.biblioitemnumber) LEFT JOIN biblio on (biblioitems.biblionumber=biblio.biblionumber)   WHERE biblio.datecreated >= <<Enter Start Date|date>>  AND biblio.datecreated <= <<Enter End Date|date>> ORDER BY biblio.author asc

If I remove the second parameter from the report and hard code it, I can use both the reports and svc interface without issue.

Could you look into this Chris?

(In reply to Martin Renvoize from comment #17)
> I'm afraid I can't get this to work for more than one parameter?
> 
> I have the below report to grab 'New Items added between two dates', running
> through the normal report interface I can varify I do get responses for the
> query, but running through svc interface I get zero results (i.e [] is the
> response)
> 
> URLs and Report detailed below:
> 
> svc URL used:
> http://koha-staff.koha-ptfs.co.uk/cgi-bin/koha/svc/
> report?id=69&sql_params=2010-03-19&sql_params=2010-11-18
> 
> report URL used:
> http://koha-staff.koha-ptfs.co.uk/cgi-bin/koha/reports/guided_reports.
> pl?reports=69&phase=Run+this+report&sql_params=2010-03-19&sql_params=2010-11-
> 18
> 
> report used:
> SELECT 
> biblio.title,biblio.author,biblio.datecreated,biblioitems.isbn,items.barcode,
> items.location FROM items LEFT JOIN biblioitems on
> (items.biblioitemnumber=biblioitems.biblioitemnumber) LEFT JOIN biblio on
> (biblioitems.biblionumber=biblio.biblionumber)   WHERE biblio.datecreated >=
> <<Enter Start Date|date>>  AND biblio.datecreated <= <<Enter End Date|date>>
> ORDER BY biblio.author asc
> 
> If I remove the second parameter from the report and hard code it, I can use
> both the reports and svc interface without issue.
> 
> Could you look into this Chris?

I cant replicate it failing, I did one with 3 parameters and it worked fine
http://rorohiko/cgi-bin/koha/svc/report?id=196&sql_params=2014-01-01&sql_params=AS&sql_params=ABS

I think I found the problem - staff URLs (non-public reports) don't accept the params, but it works nicely for public reports. It would be nice if the same feature could be made available for non-public reports as well, but this is still a very nice addition as it is.

Created attachment 27327 [details] [review]
Bug 9915: Allow sql_params to be passed to a report using the OPAC svc/report interface

Test plan:

1/ Make a public report that contains SQL parameters
2/ Test it from the staff side
3/ Hit the url like
cgi-bin/koha/svc/report?id=1&sql_params=chris&sql_params=chris2
4/ Test you get JSON

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.

Tested with a public report:
select * from items
  - cgi-bin/koha/svc/report?id=6
  - URL shows JSON and all items
select * from items where itype = <<itype>>
  - cgi-bin/koha/svc/report?id=6&sql_params=BK
  - URL shows JSON and only BK items
select * from items where itype = <<itype>> and homebranch = <<homebranch>>
  - cgi-bin/koha/svc/report?id=6&sql_params=BK&sql_params=MPL
  - URL shows JSON and only BK items in MPL
select * from items where itype = <<itype|itemtypes>> and homebranch = <<homebranch|branches>>
  - same as above
  - URL shows JSON and only BK items in MPL
select * from items where dateaccessioned = 2013-12-15
  - cgi-bin/koha/svc/report?id=6&sql_params=BK&sql_params=MPL&sql_params=2013-12-15
  - URL shows JSON and correct items

Note: This will currently only work for public reports!

Signed-off-by: Galen Charlton <gmc@esilibrary.com>

I've added a follow-up that replaces the use of ->quote() with placeholders, and am kicking this back for QA review of the follow-up.

Created attachment 27330 [details] [review]
[PASSED QA] Bug 9915: (follow-up) use SQL placeholders

This patch teaches C4::Reports::Guided::execute_query()
how to accept a list of query parameter values.  It then
follows-up on the main patch by simplifying how it converts
report parameters to a complete SQL query, and removes the
use of DBI->quote() and complicated regexes.

To test:

[1] Verify that using the OPAC svc/report service with
    sql_params continues to work.
[2] Verify that there are no regressions with running
    reports from the staff interface, both via the web
    service and the reports interface.
[3] Verify that prove -v /db_dependent/Reports_Guided.t passes.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
No regressions found.

Pushed to master.  Thanks, Chris!