Summary: | JSON reports shows inaccurate results with repeated parameters | ||
---|---|---|---|
Product: | Koha | Reporter: | Christopher Brannon <cbrannon> |
Component: | Reports | Assignee: | Johanna Räisä <johanna.raisa> |
Status: | Needs documenting --- | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | fridolin.somers, johanna.raisa, lucas, martin.renvoize |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
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
Bug 32413: Fix repeated param names for JSON report Bug 32413: Fix repeated param names for JSON report |
Description
Christopher Brannon
2022-12-06 23:27:43 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. Same issue with command-line runreport.pl, it dies because of repeating parameter (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 <<>> 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 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 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> Nice clear fix... going straight for QA here. 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. Pushed for 24.11! Well done everyone, thank you! (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. (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. |