Bug 27380

Summary: Add option for taking a list parameter in reports
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: ReportsAssignee: Nick Clemens (kidclamp) <nick>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: andrew, blawlor, fridolin.somers, jonathan.druart, kyle
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This enhancement adds a new option for taking input in reports. You can now specify the input as a 'list' which will allow inputting a newline separated list of values When requesting in put in the report you can use 'list' after the pipe symbol, similar to the way you can specify a 'date' input "Data to enter|list"
Version(s) released in:
21.05.00
Circulation function:
Bug Depends on:    
Bug Blocks: 27712, 29328    
Attachments: Bug 27380: Add 'list' option to Koha report parameters
Bug 27380: Add 'list' option to Koha report parameters
Bug 27380: Move get_prepped_report to object and use for svc/reports
Bug 27380: Add 'list' option to Koha report parameters
Bug 27380: Move get_prepped_report to object and use for svc/reports
Bug 27380: (follow-up) Remove earlier declaration and unused assignment
Bug 27380: Add 'list' option to Koha report parameters
Bug 27380: Move get_prepped_report to object and use for svc/reports
Bug 27380: (follow-up) Remove earlier declaration and unused assignment
Bug 27380: Add 'list' option to Koha report parameters
Bug 27380: Move get_prepped_report to object and use for svc/reports
Bug 27380: (follow-up) Remove earlier declaration and unused assignment
Bug 27380: Move get_prepped_report to object and use for svc/reports
Bug 27380: FIX Reports/Guided.t
Bug 27380: Add missing use statement

Description Nick Clemens (kidclamp) 2021-01-11 17:14:35 UTC
This exists:
https://github.com/bywatersolutions/koha-plugin-reports-plus

We should make koha do this
Comment 1 Nick Clemens (kidclamp) 2021-01-11 17:35:12 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens (kidclamp) 2021-01-11 20:14:35 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens (kidclamp) 2021-01-11 20:14:38 UTC Comment hidden (obsolete)
Comment 4 Andrew Fuerste-Henry 2021-01-11 20:36:10 UTC
If your report wraps your parameter in parens like:
WHERE barcode IN (<<Barcodes|list>>)
You end up with the barcodes you entered being parsed as:
WHERE barcode IN (('39999000010113','39999000010114'))
Which gives this error:
The following error was encountered:
The database returned the following error:
Operand should contain 1 column(s)

That's going to confuse anyone coming from the existing plugin, since the plugin says "To utilise the list functionality syntax should in: VALUE in (<<List parameter>>)." It requires the extra parens that cause this new patch to choke.

If we have to train folks not to include those parens, then so be it, but it'd be great if we could instead teach Koha to deal with them.
Comment 5 Nick Clemens (kidclamp) 2021-01-12 10:44:03 UTC
(In reply to Andrew Fuerste-Henry from comment #4)
> If your report wraps your parameter in parens like:
> WHERE barcode IN (<<Barcodes|list>>)
> You end up with the barcodes you entered being parsed as:
> WHERE barcode IN (('39999000010113','39999000010114'))
> Which gives this error:
> The following error was encountered:
> The database returned the following error:
> Operand should contain 1 column(s)
> 
> That's going to confuse anyone coming from the existing plugin, since the
> plugin says "To utilise the list functionality syntax should in: VALUE in
> (<<List parameter>>)." It requires the extra parens that cause this new
> patch to choke.
> 
> If we have to train folks not to include those parens, then so be it, but
> it'd be great if we could instead teach Koha to deal with them.

Talked with Andrew - I believe the syntax here is simpler and the original plug-in syntax should have been simplified. Sending back to NSO
Comment 6 Andrew Fuerste-Henry 2021-01-12 12:37:39 UTC Comment hidden (obsolete)
Comment 7 Andrew Fuerste-Henry 2021-01-12 12:37:42 UTC Comment hidden (obsolete)
Comment 8 Nick Clemens (kidclamp) 2021-02-01 14:55:41 UTC Comment hidden (obsolete)
Comment 9 Owen Leonard 2021-02-01 14:59:38 UTC Comment hidden (obsolete)
Comment 10 Owen Leonard 2021-02-01 14:59:41 UTC Comment hidden (obsolete)
Comment 11 Owen Leonard 2021-02-01 14:59:44 UTC Comment hidden (obsolete)
Comment 12 Katrin Fischer 2021-02-07 11:47:24 UTC
Created attachment 116464 [details] [review]
Bug 27380: Add 'list' option to Koha report parameters

TODO: Need to address the svc endpoints

To test:
1 - Create a 'New SQL report' like:
    SELECT * FROM items WHERE itemnumber IN <<Itemnumbers|list>>
2 - Run the report
3 - You should have a text area where you can enter various itemnumbers
4 - Enter some valid and invalid itemnumbers
5 - You get the info for the valid itemnumbers, no error for the others
6 - Test adding other params to the report and ensure things still work as expected

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2021-02-07 11:47:28 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2021-02-07 11:47:31 UTC
Created attachment 116466 [details] [review]
Bug 27380: (follow-up) Remove earlier declaration and unused assignment

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Katrin Fischer 2021-02-07 11:48:04 UTC
Love the feature, requires documentation! :)
Comment 16 Jonathan Druart 2021-02-12 11:27:20 UTC
Can you add an example on the wiki page please?

https://wiki.koha-community.org/wiki/SQL_Reports_Library
Comment 17 Jonathan Druart 2021-02-12 11:30:27 UTC
Created attachment 116818 [details] [review]
Bug 27380: Move get_prepped_report to object and use for svc/reports

This patch moves get_prepped_report to Koha:Report->prep_report
and adds some basic tests

To test:
1 - Using the report created in last test, hit the report svc api like:
http://localhost:8081/cgi-bin/koha/svc/report?id=6&param_name=One&sql_params=One&param_name=Listy|list&sql_params=2345%0D%0A423%0D%0A3%0D%0A2%0D%0A12
2 - Note the use of %0D%0A to separate list params
3 - Test with combinations with and without param_name specified
http://localhost:8081/cgi-bin/koha/svc/report?id=6&sql_params=5&sql_params=2345%0D%0A423%0D%0A3%0D%0A2%0D%0A12

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

JD Amended patch: Perltidy prep_report
Comment 18 Jonathan Druart 2021-02-12 12:12:00 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 19 Jonathan Druart 2021-02-12 13:31:34 UTC
Created attachment 116830 [details] [review]
Bug 27380: FIX Reports/Guided.t
Comment 20 Jonathan Druart 2021-02-12 14:42:12 UTC
(In reply to Jonathan Druart from comment #19)
> Created attachment 116830 [details] [review] [review]
> Bug 27380: FIX Reports/Guided.t

Patch pushed to master.
Comment 21 Jonathan Druart 2021-02-16 09:15:59 UTC
Created attachment 116908 [details] [review]
Bug 27380: Add missing use statement
Comment 22 Jonathan Druart 2021-02-16 09:23:25 UTC
(In reply to Jonathan Druart from comment #21)
> Created attachment 116908 [details] [review] [review]
> Bug 27380: Add missing use statement

Patched pushed to master.
Comment 23 Fridolin Somers 2021-02-19 14:54:13 UTC
Enhancement not pushed to 20.11.x