Bug 27380 - Add option for taking a list parameter in reports
Summary: Add option for taking a list parameter in reports
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-11 17:14 UTC by Nick Clemens
Modified: 2021-01-12 12:37 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


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

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
Created attachment 115044 [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
7 - You should be able to use the svc api to all the report, separating items with %0D in the url:
Comment 2 Nick Clemens 2021-01-11 20:14:35 UTC
Created attachment 115047 [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
Comment 3 Nick Clemens 2021-01-11 20:14:38 UTC
Created attachment 115048 [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
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
Created attachment 115084 [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: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 7 Andrew Fuerste-Henry 2021-01-12 12:37:42 UTC
Created attachment 115085 [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: Andrew Fuerste-Henry <andrew@bywatersolutions.com>