Summary: | Allow for parameters re-use on SQL reports | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi <tomascohen> |
Component: | Reports | Assignee: | Nick Clemens (kidclamp) <nick> |
Status: | CLOSED FIXED | QA Contact: | Katrin Fischer <katrin.fischer> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | claire_gravely, dcook, jonathan.druart, julian.maurice, jzairo, katrin.fischer, kyle, mtompset, nick, sally.healey, victor |
Version: | Main | Keywords: | Academy |
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This new feature allows parameters to be re-used in reports. When a report asks for two variables using the same name and type/authorised value they will be combined into a single input field on the form. i.e.
SELECT *
FROM items
WHERE homebranch=<<Branchcode|branches>> AND holdingbranch=<<Branchcode|branches>> AND itype=<<Item type|itemtypes>>
Will ask for only 2 parameters when run, Branchcode and Item type.
|
Version(s) released in: | |
Bug Depends on: | |||
Bug Blocks: | 21906, 21984, 21991 | ||
Attachments: |
Bug 9634: Allow for combining same paraneters in SQL reports
Bug 9634 [QA Followup] - Add new column to kohastructure.sql Bug 9634: Allow for combining same paraneters in SQL reports Bug 9634 [QA Followup] - Add new column to kohastructure.sql Bug 9634: Allow for combining same paraneters in SQL reports Bug 9634: Allow for combining same paraneters in SQL reports Bug 9634: Allow for combining same paraneters in SQL reports Bug 9634: Allow for combining same paraneters in SQL reports Bug 9634: Allow for combining same paraneters in SQL reports |
Description
Tomás Cohen Arazi
2013-02-15 17:56:33 UTC
Parameters are passed through CGI in a way they are processed in order. A way of mapping parameter value to the pair (name,authval) (kind of using it as an ID) is needed for the task. Created attachment 70592 [details] [review] Bug 9634: Allow for combining same paraneters in SQL reports This patch set adds a new colum to saved_sql 'combine_params' If set, parameters that have the same name and data type will only be asked for once from the user To test: 1 - Create a report that takes multiple parameters, e.g.: SELECT <<cat>> AS one, <<dog|branches>> AS two, <<dog|YES_NO>> as three, <<cat>> as four, <<dog|branches>> as five 2 - Run this report, note you are asked for five parameters 3 - Apply patch 4 - Update database 5 - Run report, note you are still asked for five parameters 6 - Edit report, note the 'Combine params' options, say YES!!!!! 7 - Re-run the report, you shoudl be asked for three parameters 8 - Verify the results reflect the supplied parameters 9 - Create a new report (perhaps something more substantive) 10 - Save with 'combine params' checke 11 - View in saved reports and see the new 'combine params' column 12 - Verify it is correctly set 13 - Run the report and verify correct results 14 - prove -v t/db_dependent/Reports/Guided.t Failed to include update for fresh install, only upgrade alter table. Created attachment 70685 [details] [review] Bug 9634 [QA Followup] - Add new column to kohastructure.sql Created attachment 70735 [details] [review] Bug 9634: Allow for combining same paraneters in SQL reports This patch set adds a new colum to saved_sql 'combine_params' If set, parameters that have the same name and data type will only be asked for once from the user To test: 1 - Create a report that takes multiple parameters, e.g.: SELECT <<cat>> AS one, <<dog|branches>> AS two, <<dog|YES_NO>> as three, <<cat>> as four, <<dog|branches>> as five 2 - Run this report, note you are asked for five parameters 3 - Apply patch 4 - Update database 5 - Run report, note you are still asked for five parameters 6 - Edit report, note the 'Combine params' options, say YES!!!!! 7 - Re-run the report, you shoudl be asked for three parameters 8 - Verify the results reflect the supplied parameters 9 - Create a new report (perhaps something more substantive) 10 - Save with 'combine params' checke 11 - View in saved reports and see the new 'combine params' column 12 - Verify it is correctly set 13 - Run the report and verify correct results 14 - prove -v t/db_dependent/Reports/Guided.t Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 70736 [details] [review] Bug 9634 [QA Followup] - Add new column to kohastructure.sql Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Is the new option really necessary ? I can't imagine a use case where someone would like to not use this feature. And if users really want to repeat themselves they can just rename one of the parameters. I added it only for old reports, if a library has been using a report with duplicated inputs this will be a change in behavior and may be unexpected/unwanted. Am happy to reconsider or explore alternate options, this seemed the simplest at the time (In reply to Nick Clemens from comment #8) > I added it only for old reports, if a library has been using a report with > duplicated inputs this will be a change in behavior and may be > unexpected/unwanted. > > Am happy to reconsider or explore alternate options, this seemed the > simplest at the time I hadn't thought about old reports. It makes sense... but what do you think about renaming all parameters (appending a number for instance) of existing reports in an updatedatabase script ? That works for me, Jonathan or Katrin, any input? If it sounds good I will try to work on that or you can feel free Julian (In reply to Nick Clemens from comment #10) > That works for me, Jonathan or Katrin, any input? If it sounds good I will > try to work on that or you can feel free Julian Changing status to ASSIGNED. Feel free to change it back directly to Signed off once the patch is ready. What I think is: Don't make the option, because the chance of messing up existing reports is very very slim. They will remain working as they work now if parameters are named differently. If there exist parameters who already are named the exactly same, the patch will take effect, but then we can also assume that this is indeed the wanted behaviour. Of course we should add a nice note to the release notes to highlight the new feature! Created attachment 73548 [details] [review] Bug 9634: Allow for combining same paraneters in SQL reports This patch combines report paramters with the same name and data type (authorised value) This presereves the past ability to provide all parameters as 'sql_params' in order to preserve existing public report links To test: 1 - Create a report that takes multiple parameters, e.g.: SELECT <<cat>> AS one, <<dog|branches>> AS two, <<dog|YES_NO>> as three, <<cat>> as four, <<dog|branches>> as five 2 - Run this report, note you are asked for five parameters 3 - Apply patch 4 - Update database 5 - Run report, note you are asked for three params 6 - Verify the results reflect the supplied parameters 7 - Export the report and verify results 8 - Alter the URL and provide 5 params as sql_params and no param_name variables i.e. http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Run+this+report&sql_params=Banana&sql_params=CPL&sql_params=0&sql_params=Orange&sql_params=LPL 9 - Verify results come out as expected Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Needs to be rebased on bug 20052 - adding get_prepped_report method to Koha/Report.pm and covering with tests (In reply to Nick Clemens from comment #14) > Needs to be rebased on bug 20052 - adding get_prepped_report method to > Koha/Report.pm and covering with tests Nvm, I didn't add this method - will be a future enh, but out of scope here Created attachment 73549 [details] [review] Bug 9634: Allow for combining same paraneters in SQL reports This patch combines report paramters with the same name and data type (authorised value) This presereves the past ability to provide all parameters as 'sql_params' in order to preserve existing public report links To test: 1 - Create a report that takes multiple parameters, e.g.: SELECT <<cat>> AS one, <<dog|branches>> AS two, <<dog|YES_NO>> as three, <<cat>> as four, <<dog|branches>> as five 2 - Run this report, note you are asked for five parameters 3 - Apply patch 4 - Update database 5 - Run report, note you are asked for three params 6 - Verify the results reflect the supplied parameters 7 - Export the report and verify results 8 - Alter the URL and provide 5 params as sql_params and no param_name variables i.e. http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Run+this+report&sql_params=Banana&sql_params=CPL&sql_params=0&sql_params=Orange&sql_params=LPL 9 - Verify results come out as expected Created attachment 73691 [details] [review] Bug 9634: Allow for combining same paraneters in SQL reports This patch combines report paramters with the same name and data type (authorised value) This presereves the past ability to provide all parameters as 'sql_params' in order to preserve existing public report links To test: 1 - Create a report that takes multiple parameters, e.g.: SELECT <<cat>> AS one, <<dog|branches>> AS two, <<dog|YES_NO>> as three, <<cat>> as four, <<dog|branches>> as five 2 - Run this report, note you are asked for five parameters 3 - Apply patch 4 - Update database 5 - Run report, note you are asked for three params 6 - Verify the results reflect the supplied parameters 7 - Export the report and verify results 8 - Alter the URL and provide 5 params as sql_params and no param_name variables i.e. http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Run+this+report&sql_params=Banana&sql_params=CPL&sql_params=0&sql_params=Orange&sql_params=LPL 9 - Verify results come out as expected. i.e. 5 different values that are those of the URL. Signed-off-by: Victor Grousset <victor.grousset@biblibre.com> Created attachment 73802 [details] [review] Bug 9634: Allow for combining same paraneters in SQL reports This patch combines report paramters with the same name and data type (authorised value) This presereves the past ability to provide all parameters as 'sql_params' in order to preserve existing public report links To test: 1 - Create a report that takes multiple parameters, e.g.: SELECT <<cat>> AS one, <<dog|branches>> AS two, <<dog|YES_NO>> as three, <<cat>> as four, <<dog|branches>> as five 2 - Run this report, note you are asked for five parameters 3 - Apply patch 4 - Update database 5 - Run report, note you are asked for three params 6 - Verify the results reflect the supplied parameters 7 - Export the report and verify results 8 - Alter the URL and provide 5 params as sql_params and no param_name variables i.e. http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Run+this+report&sql_params=Banana&sql_params=CPL&sql_params=0&sql_params=Orange&sql_params=LPL 9 - Verify results come out as expected. i.e. 5 different values that are those of the URL. Signed-off-by: Victor Grousset <victor.grousset@biblibre.com> Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> Stole this one from you Julian, hope it's ok! Created attachment 73807 [details] [review] Bug 9634: Allow for combining same paraneters in SQL reports This patch combines report paramters with the same name and data type (authorised value) This presereves the past ability to provide all parameters as 'sql_params' in order to preserve existing public report links To test: 1 - Create a report that takes multiple parameters, e.g.: SELECT <<cat>> AS one, <<dog|branches>> AS two, <<dog|YES_NO>> as three, <<cat>> as four, <<dog|branches>> as five 2 - Run this report, note you are asked for five parameters 3 - Apply patch 4 - Update database 5 - Run report, note you are asked for three params 6 - Verify the results reflect the supplied parameters 7 - Export the report and verify results 8 - Alter the URL and provide 5 params as sql_params and no param_name variables i.e. http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?reports=1&phase=Run+this+report&sql_params=Banana&sql_params=CPL&sql_params=0&sql_params=Orange&sql_params=LPL 9 - Verify results come out as expected. i.e. 5 different values that are those of the URL. Signed-off-by: Victor Grousset <victor.grousset@biblibre.com> Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Pushed to master for 18.05, thanks to everybody involved! Enhancement, not backported for 17.11.x |