Bug 27380 - Add option for taking a list parameter in reports
Summary: Add option for taking a list parameter in reports
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 27712 29328
  Show dependency treegraph
 
Reported: 2021-01-11 17:14 UTC by Nick Clemens
Modified: 2022-06-06 20:29 UTC (History)
4 users (show)

See Also:
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


Attachments
Bug 27380: Add 'list' option to Koha report parameters (4.65 KB, patch)
2021-01-11 17:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27380: Add 'list' option to Koha report parameters (4.55 KB, patch)
2021-01-11 20:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27380: Move get_prepped_report to object and use for svc/reports (12.26 KB, patch)
2021-01-11 20:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27380: Add 'list' option to Koha report parameters (3.17 KB, patch)
2021-01-12 12:37 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 27380: Move get_prepped_report to object and use for svc/reports (3.28 KB, patch)
2021-01-12 12:37 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 27380: (follow-up) Remove earlier declaration and unused assignment (718 bytes, patch)
2021-02-01 14:55 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27380: Add 'list' option to Koha report parameters (4.60 KB, patch)
2021-02-01 14:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 27380: Move get_prepped_report to object and use for svc/reports (12.31 KB, patch)
2021-02-01 14:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 27380: (follow-up) Remove earlier declaration and unused assignment (768 bytes, patch)
2021-02-01 14:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 27380: Add 'list' option to Koha report parameters (4.66 KB, patch)
2021-02-07 11:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27380: Move get_prepped_report to object and use for svc/reports (12.37 KB, patch)
2021-02-07 11:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27380: (follow-up) Remove earlier declaration and unused assignment (826 bytes, patch)
2021-02-07 11:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27380: Move get_prepped_report to object and use for svc/reports (12.66 KB, patch)
2021-02-12 11:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27380: FIX Reports/Guided.t (1.02 KB, patch)
2021-02-12 13:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27380: Add missing use statement (587 bytes, patch)
2021-02-16 09:15 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 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 2021-01-11 17:35:12 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens 2021-01-11 20:14:35 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens 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 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 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