Bug 32413 - JSON reports shows inaccurate results with repeated parameters
Summary: JSON reports shows inaccurate results with repeated parameters
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Johanna Räisä
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-12-06 23:27 UTC by Christopher Brannon
Modified: 2024-11-22 21:41 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
When creating a reports with runtime parameters that use the same description, the form in Koha would present only one input field for them, but the JSON API required to send the value multiple times for each occurrence of the runtime parameter. This makes the behavior in Koha and the JSON API match in that the parameter needs to be only sent once. **Important:** Scripts using the JSON feature with repeatable parameters before this change will need to be adapted.
Version(s) released in:
24.11.00
Circulation function:

Attachments
Bug 32413: Fix repeated param names for JSON report (1.36 KB, patch)
2024-10-09 05:37 UTC, Johanna Räisä 		Details | Diff | Splinter Review
Bug 32413: Fix repeated param names for JSON report (2.71 KB, patch)
2024-10-09 09:20 UTC, Johanna Räisä 		Details | Diff | Splinter Review
Bug 32413: Fix repeated param names for JSON report (2.78 KB, patch)
2024-10-28 08:55 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Brannon 2022-12-06 23:27:43 UTC 
If I create a report that uses a repeating parameter, for example <<library>>, more than once in a report, and then run a JSON call on that report, I get inaccurate results (Like every other result).

Currently, the only workaround is to make each parameter unique, and then repeat the parameter over as many times as it is needed in the report.  For example,

https://mydomain.com/cgi-bin/koha/svc/report?id=938&sql_params=blah would only return half the results if the report because I use the same parameters 7 times int he sql.  However, if I rename each parameter something different, like <<library1>>, <<library2>>,etc, and call https://mydomain.com/cgi-bin/koha/svc/report?id=938&sql_params=blah&sql_params=blah&sql_params=blah&sql_params=blah&sql_params=blah&sql_params=blah&sql_params=blah, it works.

The JSON call needs to be updated to accept repeated parameters.
Comment 1 Katrin Fischer 2022-12-07 00:11:23 UTC 
Repeated params don't work for JSON, you need to actually repeat them in the URL right now. You don't need to name them differently, they can still be 'collapsed' for the SQL report when run in Koha.
Comment 2 Fridolin Somers 2024-03-07 15:16:05 UTC 
Same issue with command-line runreport.pl, it dies because of repeating parameter
Comment 3 Fridolin Somers 2024-03-07 15:44:27 UTC 
(In reply to Fridolin Somers from comment #2)
> Same issue with command-line runreport.pl, it dies because of repeating
> parameter

Ahhh forget that.
Runreport simply replaces <<>> with ? so it works.
But param is not repeated, you need as meany as there are <<>>
Comment 4 Johanna Räisä 2024-10-09 05:37:29 UTC 
Created attachment 172555 [details] [review]
Bug 32413: Fix repeated param names for JSON report

This patch fixes repeated param names for JSON report.

To test:

1) Create a report
    SELECT count(*) from items where homebranch = <<Branchcode>>
    UNION ALL
    SELECT count(*) from deleteditems where homebranch = <<Branchcode>>
2) Run the report as JSON, cgi-bin/koha/svc/report?id=<id>&sql_params=<Branchcode>
3) Check that the second value is 0
4) Apply the patch
5) Run the report as JSON again
6) Check that the second value has the correct value

Sponsored-by: Koha-Suomi Oy
Comment 5 Johanna Räisä 2024-10-09 09:20:22 UTC 
Created attachment 172565 [details] [review]
Bug 32413: Fix repeated param names for JSON report

This patch fixes repeated param names for JSON report.

To test:

1) Create a report
    SELECT count(*) from items where homebranch = <<Branchcode>>
    UNION ALL
    SELECT count(*) from deleteditems where homebranch = <<Branchcode>>
2) Run the report as JSON, cgi-bin/koha/svc/report?id=<id>&sql_params=<Branchcode>
3) Check that the second value is 0
4) Apply the patch
5) Run the report as JSON again
6) Check that the second value has the correct value
7) prove t/db_dependent/Koha/Reports.t

Sponsored-by: Koha-Suomi Oy
Comment 6 Martin Renvoize (ashimema) 2024-10-28 08:55:05 UTC 
Created attachment 173437 [details] [review]
Bug 32413: Fix repeated param names for JSON report

This patch fixes repeated param names for JSON report.

To test:

1) Create a report
    SELECT count(*) from items where homebranch = <<Branchcode>>
    UNION ALL
    SELECT count(*) from deleteditems where homebranch = <<Branchcode>>
2) Run the report as JSON, cgi-bin/koha/svc/report?id=<id>&sql_params=<Branchcode>
3) Check that the second value is 0
4) Apply the patch
5) Run the report as JSON again
6) Check that the second value has the correct value
7) prove t/db_dependent/Koha/Reports.t

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize (ashimema) 2024-10-28 08:55:36 UTC 
Nice clear fix... going straight for QA here.
Comment 8 Katrin Fischer 2024-10-30 06:31:26 UTC 
While this is a nice fix, it an break people scripts. Please add nice clear release notes to make this change more visible.

It's also why I think we might want to hold back on backporting this. I know I have scripts I will need to adapt that use repeated params that are now sent multiple times to get around this.
Comment 9 Katrin Fischer 2024-10-30 06:48:31 UTC 
Pushed for 24.11!

Well done everyone, thank you!
Comment 10 Katrin Fischer 2024-11-22 10:16:08 UTC 
(In reply to Katrin Fischer from comment #8)
> While this is a nice fix, it an break people scripts. Please add nice clear
> release notes to make this change more visible.
> 
> It's also why I think we might want to hold back on backporting this. I know
> I have scripts I will need to adapt that use repeated params that are now
> sent multiple times to get around this.

Johanna, I am going to try and write the notes now, but I really need people to react to requests for additional work on their bugs or I can't keep pushing them.
Comment 11 Lucas Gass (lukeg) 2024-11-22 21:41:52 UTC 
(In reply to Katrin Fischer from comment #8)
> While this is a nice fix, it an break people scripts. Please add nice clear
> release notes to make this change more visible.
> 
> It's also why I think we might want to hold back on backporting this. I know
> I have scripts I will need to adapt that use repeated params that are now
> sent multiple times to get around this.

I agree, I don't want to break scripts in a stable version. Declining to backport.