[Bug 32413] New: JSON reports shows inaccurate results with repeated parameters
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 Bug ID: 32413 Summary: JSON reports shows inaccurate results with repeated parameters Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Reports Assignee: koha-bugs@lists.koha-community.org Reporter: cbrannon@cdalibrary.org QA Contact: testopia@bugs.koha-community.org 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. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 --- Comment #1 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 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. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #2 from Fridolin Somers <fridolin.somers@biblibre.com> --- Same issue with command-line runreport.pl, it dies because of repeating parameter -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 --- Comment #3 from Fridolin Somers <fridolin.somers@biblibre.com> --- (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 <<>> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 Johanna Räisä <johanna.raisa@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |johanna.raisa@koha-suomi.fi Assignee|koha-bugs@lists.koha-commun |johanna.raisa@koha-suomi.fi |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 --- Comment #4 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- Created attachment 172555 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=172555&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 Johanna Räisä <johanna.raisa@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 Johanna Räisä <johanna.raisa@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #172555|0 |1 is obsolete| | --- Comment #5 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- Created attachment 172565 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=172565&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #172565|0 |1 is obsolete| | --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 173437 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173437&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com CC| |martin.renvoize@ptfs-europe | |.com Status|Needs Signoff |Signed Off --- Comment #7 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Nice clear fix... going straight for QA here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.11! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | Text to go in the| |When creating a reports release notes| |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. --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32413 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Pushed to main |Needs documenting --- Comment #11 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- (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. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org