Description
Nick Clemens (kidclamp)
2021-01-11 17:14:35 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: 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 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¶m_name=One&sql_params=One¶m_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 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. (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 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> 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¶m_name=One&sql_params=One¶m_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> Created attachment 116160 [details] [review] Bug 27380: (follow-up) Remove earlier declaration and unused assignment Created attachment 116161 [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> Created attachment 116162 [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¶m_name=One&sql_params=One¶m_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> Created attachment 116163 [details] [review] Bug 27380: (follow-up) Remove earlier declaration and unused assignment Signed-off-by: Owen Leonard <oleonard@myacpl.org> 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> Created attachment 116465 [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¶m_name=One&sql_params=One¶m_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> 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> Love the feature, requires documentation! :) Can you add an example on the wiki page please? https://wiki.koha-community.org/wiki/SQL_Reports_Library 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¶m_name=One&sql_params=One¶m_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 Pushed to master for 21.05, thanks to everybody involved! Created attachment 116830 [details] [review] Bug 27380: FIX Reports/Guided.t (In reply to Jonathan Druart from comment #19) > Created attachment 116830 [details] [review] [review] > Bug 27380: FIX Reports/Guided.t Patch pushed to master. Created attachment 116908 [details] [review] Bug 27380: Add missing use statement (In reply to Jonathan Druart from comment #21) > Created attachment 116908 [details] [review] [review] > Bug 27380: Add missing use statement Patched pushed to master. Enhancement not pushed to 20.11.x |