Add a form under report's result that allow to configure and draw a chart (pie, bar, line and combination).
Created attachment 55412 [details] [review] Bug 17282 - Ability to create charts for SQL reports Add a form under report's result that allow to configure and draw a chart (pie, bar, line and combination). Pie: Usefull only for a two-column report's result bar: Horizontal: Can be horizontal or vertical (check/uncheck horizontal checkbox), Group: allows to group columns (stacked bar chart), Line: show some columns as line in a bar chart (combination) line: line chart :) This patch adds 2 new js libraries: d3js and c3js: - c3.min.css - c3.min.js - d3.min.js Test plan: - Apply this patch, - make reports, - draw charts, - that's all
Created attachment 55442 [details] [review] [SIGNED-OFF]Bug 17282: Ability to create charts for SQL reports Add a form under report's result that allow to configure and draw a chart (pie, bar, line and combination). Pie: Usefull only for a two-column report's result bar: Horizontal: Can be horizontal or vertical (check/uncheck horizontal checkbox), Group: allows to group columns (stacked bar chart), Line: show some columns as line in a bar chart (combination) line: line chart :) This patch adds 2 new js libraries: d3js and c3js: - c3.min.css - c3.min.js - d3.min.js Test plan: - Apply this patch, - make reports, - draw charts, - that's all Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Works as advertised
Could be better if we can only use the dropdown menu to download the chart. I propose this patch, if you think that is bad idea, please make it obsolete.
Created attachment 55444 [details] [review] Bug 17282: (follow-up) Use drop-down menu for download chart instead To Test 1) Execute a report 2) Click in drop-down menu "Download" shows only three options 3) Draw a chart 4) Click again in drop-down menu "Download" shows a new option "Download chart"
Created attachment 55480 [details] [review] Bug 17282: (follow-up) Use drop-down menu for download chart instead To Test 1) Execute a report 2) Click in drop-down menu "Download" shows only three options 3) Draw a chart 4) Click again in drop-down menu "Download" shows a new option "Download chart" Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
QA comments: 1/ Would it be possible to add a "loading please wait" when processing the graph? 2/ Execute a report which raises an error, you will get a JS error: 3/ If I delete a column, the scrollbar goes to the top 4/ I can delete all the columns and click on "draw", nothing happens. It should not bz possible to delete the first column I'd say. 5/ With the following query: SELECT i.homebranch, h.branchcode, monthname(h.reservedate) AS month, year(h.reservedate) AS year, count(*) FROM old_reserves h LEFT JOIN items i USING (itemnumber) WHERE h.branchcode != i.homebranch AND year(h.reservedate) = '2015' GROUP BY month(h.reservedate), h.branchcode With chart type: "Pie", x column: "month" and y: "count(*)", I get the following JS error: TypeError: columns[index] is undefined Marked as Failed QA.
(In reply to Jonathan Druart from comment #6) > 2/ Execute a report which raises an error, you will get a JS error: Have you an exemple of report that raise error ?
(In reply to Alex Arnaud from comment #7) > (In reply to Jonathan Druart from comment #6) > > > 2/ Execute a report which raises an error, you will get a JS error: > > Have you an exemple of report that raise error ? SELECT itemtype, count(*) FROM items group by itemtype; should do the trick
Created attachment 55582 [details] [review] Bug 17282 - Fix QA comments: - Don't scroll to the top when deleting a column, - prevents to delete all columns, - better handling of headers/results deleting
Created attachment 55586 [details] [review] Bug 17282 - Fix QA comments: - Don't scroll to the top when deleting a column, - prevents to delete all columns, - better handling of headers/results deleting
Created attachment 55588 [details] [review] Bug 17282 - Ability to exclude the last line in report results This patch add a checkbox (exclude last line). This is useful when this last is the result of a sql rollup which is the sum of each column and make the chart not consistent
Created attachment 55604 [details] [review] Bug 17282: Ability to create charts for SQL reports Add a form under report's result that allow to configure and draw a chart (pie, bar, line and combination). Pie: Usefull only for a two-column report's result bar: Horizontal: Can be horizontal or vertical (check/uncheck horizontal checkbox), Group: allows to group columns (stacked bar chart), Line: show some columns as line in a bar chart (combination) line: line chart :) This patch adds 2 new js libraries: d3js and c3js: - c3.min.css - c3.min.js - d3.min.js Test plan: - Apply this patch, - make reports, - draw charts, - that's all Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Works as advertised Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 55605 [details] [review] Bug 17282: (follow-up) Use drop-down menu for download chart instead To Test 1) Execute a report 2) Click in drop-down menu "Download" shows only three options 3) Draw a chart 4) Click again in drop-down menu "Download" shows a new option "Download chart" Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 55606 [details] [review] Bug 17282 - Fix QA comments: - Don't scroll to the top when deleting a column, - prevents to delete all columns, - better handling of headers/results deleting Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 55607 [details] [review] Bug 17282 - Ability to exclude the last line in report results This patch add a checkbox (exclude last line). This is useful when this last is the result of a sql rollup which is the sum of each column and make the chart not consistent Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Currently, graphs are built using the data of the page (Because this the only available variable in the template). For reports with paging, this means that all data are not took in account. I'm wondering if we should not make that possible. How ? Get all data with an AJAX call during making the graph ? It seems to be a big work to AJAXify this part of the code. Feedback are welcome
(In reply to Alex Arnaud from comment #16) > Currently, graphs are built using the data of the page (Because this the > only available variable in the template). For reports with paging, this > means that all data are not took in account. I'm wondering if we should not > make that possible. How ? Get all data with an AJAX call during making the > graph ? It seems to be a big work to AJAXify this part of the code. > > Feedback are welcome It seems reasonable, for a first version, to built the graph with the data displayed on the page. But it would be nice to tell the user how it works with a small info box.
> It seems reasonable, for a first version, to built the graph with the data > displayed on the page. > But it would be nice to tell the user how it works with a small info box. +1
(In reply to Jonathan Druart from comment #6) Some of the comments from comment 6 have not been fixed!
Created attachment 55868 [details] [review] Bug 17282: Use fieldset.rows
Created attachment 55869 [details] [review] Bug 17282: Remove use of onclick
Created attachment 62529 [details] [review] Bug 17282 - Fix QA comments - Add a loading message, - prevent JS error when report fails, - fix (bootstrap 3 ?) displaying problems
Looking forward to this feature! Wanted to test but first patch does not apply on newest master (lot of commits today). automatischer Merge von koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt KONFLIKT (Inhalt): Merge-Konflikt in koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt automatischer Merge von koha-tmpl/intranet-tmpl/prog/en/includes/chart.inc KONFLIKT (hinzufügen/hinzufügen): Merge-Konflikt in koha-tmpl/intranet-tmpl/prog/en/includes/chart.inc
Created attachment 63565 [details] [review] Bug 17282: Ability to create charts for SQL reports Add a form under report's result that allow to configure and draw a chart (pie, bar, line and combination). Pie: Usefull only for a two-column report's result bar: Horizontal: Can be horizontal or vertical (check/uncheck horizontal checkbox), Group: allows to group columns (stacked bar chart), Line: show some columns as line in a bar chart (combination) line: line chart :) This patch adds 2 new js libraries: d3js and c3js: - c3.min.css - c3.min.js - d3.min.js Test plan: - Apply this patch, - make reports, - draw charts, - that's all Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Works as advertised Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 63566 [details] [review] Bug 17282: (follow-up) Use drop-down menu for download chart instead To Test 1) Execute a report 2) Click in drop-down menu "Download" shows only three options 3) Draw a chart 4) Click again in drop-down menu "Download" shows a new option "Download chart" Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 63567 [details] [review] Bug 17282 - Fix QA comments: - Don't scroll to the top when deleting a column, - prevents to delete all columns, - better handling of headers/results deleting Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 63568 [details] [review] Bug 17282 - Ability to exclude the last line in report results This patch add a checkbox (exclude last line). This is useful when this last is the result of a sql rollup which is the sum of each column and make the chart not consistent Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 63569 [details] [review] Bug 17282: Use fieldset.rows
Created attachment 63570 [details] [review] Bug 17282: Remove use of onclick
Created attachment 63571 [details] [review] Bug 17282 - Fix QA comments - Add a loading message, - prevent JS error when report fails, - fix (bootstrap 3 ?) displaying problems
Could you please rebase? I am getting fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt). on the last patch.
Created attachment 67225 [details] [review] Bug 17282: Ability to create charts for SQL reports Add a form under report's result that allow to configure and draw a chart (pie, bar, line and combination). Pie: Usefull only for a two-column report's result bar: Horizontal: Can be horizontal or vertical (check/uncheck horizontal checkbox), Group: allows to group columns (stacked bar chart), Line: show some columns as line in a bar chart (combination) line: line chart :) This patch adds 2 new js libraries: d3js and c3js: - c3.min.css - c3.min.js - d3.min.js Test plan: - Apply this patch, - make reports, - draw charts, - that's all Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Works as advertised Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 67226 [details] [review] Bug 17282: (follow-up) Use drop-down menu for download chart instead To Test 1) Execute a report 2) Click in drop-down menu "Download" shows only three options 3) Draw a chart 4) Click again in drop-down menu "Download" shows a new option "Download chart" Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 67227 [details] [review] Bug 17282 - Fix QA comments: - Don't scroll to the top when deleting a column, - prevents to delete all columns, - better handling of headers/results deleting Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 67228 [details] [review] Bug 17282 - Ability to exclude the last line in report results This patch add a checkbox (exclude last line). This is useful when this last is the result of a sql rollup which is the sum of each column and make the chart not consistent Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 67229 [details] [review] Bug 17282: Use fieldset.rows
Created attachment 67230 [details] [review] Bug 17282: Remove use of onclick
Created attachment 67231 [details] [review] Bug 17282 - Fix QA comments - Add a loading message, - prevent JS error when report fails, - fix (bootstrap 3 ?) displaying problems
Hi Alex, very cool feature! Some small things to fix: - Add information about the new Javascript libraries to the about page with the license etc. - QA tools - just make it "" instead. FAIL koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt FAIL forbidden patterns forbidden pattern: simple-quote string (line 966) - Maybe it would be nicer to display the settings on top, activated by a new button in the toolbar. What do you think? At the moment the problem I see that a lot of libraries like to print that page for a quick overview and the new settings will print on an extra page at the bottom. Also if the table is long, you have to scroll down quite a bit. Deadlines are close now, I will try to come back to this fast.
Created attachment 69174 [details] [review] Bug 17282 - Add information about the new Javascript libraries in the about page - fix a QA warning - Move chart settings on top and make them hidden by default - Add options to get all results
Created attachment 69429 [details] [review] Bug 17282 - Fix QA comments - Add a loading message, - prevent JS error when report fails, - fix (bootstrap 3 ?) displaying problems
Created attachment 69430 [details] [review] Bug 17282 - Add information about the new Javascript libraries in the about page - fix a QA warning - Move chart settings on top and make them hidden by default - Add options to get all results
Last 2 patches fix conflict on staff-global.css
Created attachment 69493 [details] [review] Bug 17282 - Add information about the new Javascript libraries in the about page - fix a QA warning - Move chart settings on top and make them hidden by default - Add options to get all results Patch applies and functions as expected when following test plan. Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
*** Bug 13219 has been marked as a duplicate of this bug. ***
Patch doesn't apply, can you please rebase?
Created attachment 79182 [details] [review] Bug 17282: Ability to create charts for SQL reports Add a form under report's result that allow to configure and draw a chart (pie, bar, line and combination). Pie: Usefull only for a two-column report's result bar: Horizontal: Can be horizontal or vertical (check/uncheck horizontal checkbox), Group: allows to group columns (stacked bar chart), Line: show some columns as line in a bar chart (combination) line: line chart :) This patch adds 2 new js libraries: d3js and c3js: - c3.min.css - c3.min.js - d3.min.js Test plan: - Apply this patch, - execute a report, - click on show chart settings button (in the tool bar), - draw chart (click on draw button), - check the chart Features: - Include all rows (ignore pagination), - Download the chart (svg), - Choose x column and y columns, - Exclude last line (Rollup)
Created attachment 79230 [details] [review] Bug 17282: Ability to create charts for SQL reports Add a form under report's result that allow to configure and draw a chart (pie, bar, line and combination). Pie: Usefull only for a two-column report's result bar: Horizontal: Can be horizontal or vertical (check/uncheck horizontal checkbox), Group: allows to group columns (stacked bar chart), Line: show some columns as line in a bar chart (combination) line: line chart :) This patch adds 2 new js libraries: d3js and c3js: - c3.min.css - c3.min.js - d3.min.js Test plan: - Apply this patch, - execute a report, - click on show chart settings button (in the tool bar), - draw chart (click on draw button), - check the chart Features: - Include all rows (ignore pagination), - Download the chart (svg), - Choose x column and y columns, - Exclude last line (Rollup) Signed-off-by: Michal Denare <black23@gmail.com>
Nice feature, I really love it. Thank You ... Mike
I started to testing 21393 with this one, so my QA tools are on steroids now ;) Few comments 1) Missing filters: FAIL koha-tmpl/intranet-tmpl/prog/en/includes/chart.inc FAIL filters missing_filter at line 26 ( <option value="[% header.cell %]">[% header.cell %]</option>) missing_filter at line 46 ( <a class="chart-column-delete" href="#" data-column="[% column %]">) missing_filter at line 72 ( <option value="[% i %]">[% i %]</option>) missing_filter at line 47 ( <img src="[% interface %]/[% theme %]/img/x.png" alt="Delete" />) missing_filter at line 24 ( <option value="[% supposed_x %]" selected>[% supposed_x %]</option>) missing_filter at line 70 ( <option value="[% i %]" selected>[% i %]</option>) missing_filter at line 56 ( <option value="[% h.cell %]" selected>[% h.cell %]</option>) missing_filter at line 52 ( <select id="y_[% column %]" name="y">) missing_filter at line 58 ( <option value="[% h.cell %]">[% h.cell %]</option>) missing_filter at line 43 ( <fieldset class="chart-column-conf" id="column_[% column %]" style="display: inline !important;">) missing_filter at line 66 ( <label for="group_[% column %]">Group:</label>) missing_filter at line 81 ( <input class="column-line" id="column-line" name="[% header.cell %]" type="checkbox">) missing_filter at line 67 ( <select id="group_[% column %]" name="group">) missing_filter at line 45 ( Column [% column %]) missing_filter at line 94 ( [% header_row.unshift(item) %]) missing_filter at line 51 ( <label for="y_[% column %]" >y:</label>) missing_filter at line 80 ( <label for="line_[% column %]">line:</label>) missing_filter at line 53 ( <option value="[% supposed_x %]" selected>[% supposed_x %]</option>) FAIL koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt FAIL filters missing_filter at line 1108 ( $('#chart').prepend('<div style="font-size: 1rem; text-align: center;">' + "[% name %]" + '</div>');) missing_filter at line 1058 ( results = [% results.json %]) missing_filter at line 1055 ( results = [% allresults.json %]) missing_filter at line 1051 ( headers = [% header_row.json %];) 2) Could you please split the patch? At least staff-global.css should not be in main patch, as it is generated by RM and here should be only for ease the testing.
Created attachment 79308 [details] [review] Bug 17282: Ability to create charts for SQL reports Add a form under report's result that allow to configure and draw a chart (pie, bar, line and combination). Pie: Usefull only for a two-column report's result bar: Horizontal: Can be horizontal or vertical (check/uncheck horizontal checkbox), Group: allows to group columns (stacked bar chart), Line: show some columns as line in a bar chart (combination) line: line chart :) This patch adds 2 new js libraries: d3js and c3js: - c3.min.css - c3.min.js - d3.min.js Test plan: - Apply this patch, - execute a report, - click on show chart settings button (in the tool bar), - draw chart (click on draw button), - check the chart Features: - Include all rows (ignore pagination), - Download the chart (svg), - Choose x column and y columns, - Exclude last line (Rollup) Signed-off-by: Michal Denare <black23@gmail.com>
Created attachment 79309 [details] [review] Bug 17282: [DO NOT PUSH] provide staff-global.css
Created attachment 79310 [details] [review] Bug 17282: add template's variable filters
Created attachment 79311 [details] [review] Bug 17282: Ability to create charts for SQL reports Add a form under report's result that allow to configure and draw a chart (pie, bar, line and combination). Pie: Usefull only for a two-column report's result bar: Horizontal: Can be horizontal or vertical (check/uncheck horizontal checkbox), Group: allows to group columns (stacked bar chart), Line: show some columns as line in a bar chart (combination) line: line chart :) This patch adds 2 new js libraries: d3js and c3js: - c3.min.css - c3.min.js - d3.min.js Test plan: - Apply this patch, - execute a report, - click on show chart settings button (in the tool bar), - draw chart (click on draw button), - check the chart Features: - Include all rows (ignore pagination), - Download the chart (svg), - Choose x column and y columns, - Exclude last line (Rollup) Signed-off-by: Michal Denare <black23@gmail.com> Signed-off-by: Michal Denar <black23ěgmail.com> Signed-off-by: Your Full Name <your_email>
Created attachment 79312 [details] [review] Bug 17282: [DO NOT PUSH] provide staff-global.css Signed-off-by: Michal Denar <black23ěgmail.com> Signed-off-by: Your Full Name <your_email>
Created attachment 79313 [details] [review] Bug 17282: add template's variable filters Signed-off-by: Michal Denar <black23ěgmail.com> Signed-off-by: Your Full Name <your_email>
Sorry I've no idea what default name and email was added :-(
Hi Michal, how did you sign off? I notice there is this line in your patches today: Signed-off-by: Your Full Name <your_email>, but also a correct sign-off line: Signed-off-by: Michal Denar <black23ěgmail.com>
Shouldn't that ě also be an @ sign in the signoff too?
Created attachment 79337 [details] [review] Bug 17282: Ability to create charts for SQL reports Add a form under report's result that allow to configure and draw a chart (pie, bar, line and combination). Pie: Usefull only for a two-column report's result bar: Horizontal: Can be horizontal or vertical (check/uncheck horizontal checkbox), Group: allows to group columns (stacked bar chart), Line: show some columns as line in a bar chart (combination) line: line chart :) This patch adds 2 new js libraries: d3js and c3js: - c3.min.css - c3.min.js - d3.min.js Test plan: - Apply this patch, - execute a report, - click on show chart settings button (in the tool bar), - draw chart (click on draw button), - check the chart Features: - Include all rows (ignore pagination), - Download the chart (svg), - Choose x column and y columns, - Exclude last line (Rollup) Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 79338 [details] [review] Bug 17282: [DO NOT PUSH] provide staff-global.css Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 79339 [details] [review] Bug 17282: add template's variable filters Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
This is very cool. I think some of the lines in chart.inc need filters? They are not failing the test but want to make sure before pushing.
(In reply to Nick Clemens from comment #63) > I think some of the lines in chart.inc need filters? They are not failing > the test but want to make sure before pushing. Which ones? Did you see the last patch?
(In reply to Jonathan Druart from comment #64) > (In reply to Nick Clemens from comment #63) > > I think some of the lines in chart.inc need filters? They are not failing > > the test but want to make sure before pushing. > > Which ones? Did you see the last patch? /me is embarassed, my bad
Awesome work all! Pushed to master for 18.11
Great enhancement, won't be backported to 18.05.x series.
Be great to have some release notes text for this.. highlight it a bit during 18.11 release.