Some libraries would want to hide columns in tables. A configuration page would allow to select specific columns to show/hide in tables.
Created attachment 17978 [details] [review] Bug 10212: Columns configuration for tables - Add Colvis files
Created attachment 17979 [details] [review] Bug 10212: Columns configuration for tables This development introduces ColVis into Koha and provides a configuration page for columns visibility. ColVis is a plugin for DataTables. It allows to change the visibility of the columns in the table. * This development adds: - the js and css file for ColVis - a new DB table 'columns_settings' - a new template plugin 'ColumnsSettings' - a new package C4::Utils::DataTables::ColumnsSettings - a new admin page admin/columns_settings.pl * How it works: A yaml file is created (admin/columns_settings.yml) in order to take an inventory of all tables where ColVis is implemented. This file is read to create the list of modules, pages and tables in the configuration page. There are 3 possible keys in the yml: - is_hidden: default is 0 The column will be hidden. - cannot_be_toggled: default is 0. ColVis will allow to hide/show the column. - cannot_be_modified: default is 0 Default values (in the yml) won't be modifiable. When a user changes (or saves) the configuration for one module, all columns are added to the DB table. The values in the DB get the upper hand on the yaml values. * Humm, strange? It seems weird to have 2 storages for the same values. But I think it will be easy to add an entry and maintain the yaml rather than adding a new row (and new entry in updatedatabase script) in the DB. * Test plan: 1/ Execute the updatedatabase in order to create the new table. 2/ Take a look to the yml structure. 3/ Go on the checkouts page (circ/circulation.pl). 4/ Check that you cannot hide the 3 last columns on the issues table. 5/ Check that you cannot hide the 2 last columns on the holds table. 6/ Try to hide/show columns. 7/ Go on the columns configuration page (admin/columns_settings.pl). 8/ Only the last tab are filled with data. Check/uncheck checkboxes and save. 9/ Go on the checkouts/holds page and check that the behavior is what you expected. 10/ Give me some feedback :) * To go further: We can imagine that the configuration is saved for each user (and not globally like it is made with this patch).
This enhancement is not ready for QA (UT and doc are missing). I would like to have some feedback about it before completing it. I switch the status to "Needs SO" for the visibility but it could be "Needs Feedback" :) Note: - I don't know if "accordion" is the best choice for the configuration page. - Tables don't have a good width in some cases. This should be fixed with some css.
(In reply to comment #3) > - I don't know if "accordion" is the best choice for the configuration page. Where is the configuration page? I get a 404 error when I follow the link on the Administration home page. On the circulation page I get a JavaScript error: TypeError: a.aoColumns[c] is undefined /intranet-tmpl/prog/en/lib/jquery/plugins/jquery.dataTables.min.js Line 29
...and I get this error when I try to navigate directly to /admin/columns_settings.pl: Template process failed: file error - /koha-tmpl/intranet-tmpl/prog/en/modules/admin/columns_settings.tt: not found at /C4/Templates.pm line 132.
Created attachment 18861 [details] [review] Bug 10212: Columns configuration for tables commit a5942aa75818830145886b9d4cc4af5521867d7f Author: Jonathan Druart <jonathan.druart@biblibre.com> AuthorDate: Tue May 7 14:09:59 2013 +0200 Commit: Jonathan Druart <jonathan.druart@biblibre.com> CommitDate: Wed Jun 12 09:15:06 2013 +0200 Bug 10212: Columns configuration for tables Erk, I forgot to add the .tt file to the patch :-/
Owen, does the last patch fix your error?
(In reply to Jonathan Druart from comment #7) > Owen, does the last patch fix your error? I can now read the configuration settings page without errors, and it seems to work with the circulation checkouts table. However, it had no effect on the holds table. I also don't see a "show/hide columns" control on the holds tab. I still get a JavaScript error on the circulation page. Perhaps this is the cause? TypeError: a.aoColumns[c] is undefined /intranet-tmpl/prog/en/lib/jquery/plugins/jquery.dataTables.min.js Line 29 In my informal tests it didn't seem to slow down loading of the circulation page compared to master. I'd like to make the "show/hide columns" box less obtrusive, but I don't have any suggestions for how to do that at the moment. Is the yml file translatable?
(In reply to Owen Leonard from comment #8) > I still get a JavaScript error on the circulation page. Perhaps this is the > cause? > > TypeError: a.aoColumns[c] is undefined > /intranet-tmpl/prog/en/lib/jquery/plugins/jquery.dataTables.min.js > Line 29 I will submit a new rebased patch. I missed a column for the holds table. > In my informal tests it didn't seem to slow down loading of the circulation > page compared to master. I'd like to make the "show/hide columns" box less > obtrusive, but I don't have any suggestions for how to do that at the moment. Why do you think it is obtrusive? It is a small box on the top right. Maybe we can imagine (another) syspref for turning off this feature. > Is the yml file translatable? Why do you want to translate it? In fact the yaml file will be only used by Koha developers. Thanks for testing!
Created attachment 20625 [details] [review] Bug 10212: Columns configuration for tables
(In reply to Jonathan Druart from comment #9) > Why do you want to translate it? In fact the yaml file will be only used by > Koha developers. On the configuration page the tables are listed by their IDs: "holdst" and "issuest." These are not understandable labels if you're not familiar with the page structure. I was thinking it would be nice to be able to give these better labels, but they would have to be translatable.
Created attachment 20640 [details] [review] Bug 10212: Make the table names translatable
Applying: Bug 10212: Columns configuration for tables Using index info to reconstruct a base tree... M installer/data/mysql/kohastructure.sql M installer/data/mysql/updatedatabase.pl M koha-tmpl/intranet-tmpl/prog/en/js/datatables.js Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/js/datatables.js Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging installer/data/mysql/kohastructure.sql Patch failed at 0001 Bug 10212: Columns configuration for tables The copy of the patch that failed is found in: /home/christopher/git/koha/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".
Created attachment 21178 [details] [review] Bug 10212: Columns configuration for tables - Add Colvis files
Created attachment 21179 [details] [review] Bug 10212: Columns configuration for tables This development introduces ColVis into Koha and provides a configuration page for columns visibility. ColVis is a plugin for DataTables. It allows to change the visibility of the columns in the table. * This development adds: - the js and css file for ColVis - a new DB table 'columns_settings' - a new template plugin 'ColumnsSettings' - a new package C4::Utils::DataTables::ColumnsSettings - a new admin page admin/columns_settings.pl * How it works: A yaml file is created (admin/columns_settings.yml) in order to take an inventory of all tables where ColVis is implemented. This file is read to create the list of modules, pages and tables in the configuration page. There are 3 possible keys in the yml: - is_hidden: default is 0 The column will be hidden. - cannot_be_toggled: default is 0. ColVis will allow to hide/show the column. - cannot_be_modified: default is 0 Default values (in the yml) won't be modifiable. When a user changes (or saves) the configuration for one module, all columns are added to the DB table. The values in the DB get the upper hand on the yaml values. * Humm, strange? It seems weird to have 2 storages for the same values. But I think it will be easy to add an entry and maintain the yaml rather than adding a new row (and new entry in updatedatabase script) in the DB. * Test plan: 1/ Execute the updatedatabase in order to create the new table. 2/ Take a look to the yml structure. 3/ Go on the checkouts page (circ/circulation.pl). 4/ Check that you cannot hide the 3 last columns on the issues table. 5/ Check that you cannot hide the 2 last columns on the holds table. 6/ Try to hide/show columns. 7/ Go on the columns configuration page (admin/columns_settings.pl). 8/ Only the last tab are filled with data. Check/uncheck checkboxes and save. 9/ Go on the checkouts/holds page and check that the behavior is what you expected. 10/ Give me some feedback :) * To go further: We can imagine that the configuration is saved for each user (and not globally like it is made with this patch).
Created attachment 21180 [details] [review] Bug 10212: Make the table names translatable
Applying: Bug 10212: Columns configuration for tables Using index info to reconstruct a base tree... M installer/data/mysql/updatedatabase.pl M koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Patch failed at 0001 Bug 10212: Columns configuration for tables The copy of the patch that failed is found in: /home/christopher/git/koha/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".
Created attachment 21583 [details] [review] Bug 10212: Columns configuration for tables - Add Colvis files
Created attachment 21584 [details] [review] Bug 10212: Columns configuration for tables This development introduces ColVis into Koha and provides a configuration page for columns visibility. ColVis is a plugin for DataTables. It allows to change the visibility of the columns in the table. * This development adds: - the js and css file for ColVis - a new DB table 'columns_settings' - a new template plugin 'ColumnsSettings' - a new package C4::Utils::DataTables::ColumnsSettings - a new admin page admin/columns_settings.pl * How it works: A yaml file is created (admin/columns_settings.yml) in order to take an inventory of all tables where ColVis is implemented. This file is read to create the list of modules, pages and tables in the configuration page. There are 3 possible keys in the yml: - is_hidden: default is 0 The column will be hidden. - cannot_be_toggled: default is 0. ColVis will allow to hide/show the column. - cannot_be_modified: default is 0 Default values (in the yml) won't be modifiable. When a user changes (or saves) the configuration for one module, all columns are added to the DB table. The values in the DB get the upper hand on the yaml values. * Humm, strange? It seems weird to have 2 storages for the same values. But I think it will be easy to add an entry and maintain the yaml rather than adding a new row (and new entry in updatedatabase script) in the DB. * Test plan: 1/ Execute the updatedatabase in order to create the new table. 2/ Take a look to the yml structure. 3/ Go on the checkouts page (circ/circulation.pl). 4/ Check that you cannot hide the 3 last columns on the issues table. 5/ Check that you cannot hide the 2 last columns on the holds table. 6/ Try to hide/show columns. 7/ Go on the columns configuration page (admin/columns_settings.pl). 8/ Only the last tab are filled with data. Check/uncheck checkboxes and save. 9/ Go on the checkouts/holds page and check that the behavior is what you expected. 10/ Give me some feedback :) * To go further: We can imagine that the configuration is saved for each user (and not globally like it is made with this patch).
Created attachment 21585 [details] [review] Bug 10212: Make the table names translatable
Applying: Bug 10212: Columns configuration for tables Using index info to reconstruct a base tree... M circ/circulation.pl M installer/data/mysql/kohastructure.sql M installer/data/mysql/updatedatabase.pl M koha-tmpl/intranet-tmpl/prog/en/js/datatables.js Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/js/datatables.js Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging installer/data/mysql/kohastructure.sql CONFLICT (content): Merge conflict in installer/data/mysql/kohastructure.sql Auto-merging circ/circulation.pl Patch failed at 0001 Bug 10212: Columns configuration for tables The copy of the patch that failed is found in: /home/christopher/git/koha/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".
Just db conflicts
Created attachment 24083 [details] [review] Bug 10212: Columns configuration for tables - Add Colvis files
Created attachment 24084 [details] [review] Bug 10212: Columns configuration for tables This development introduces ColVis into Koha and provides a configuration page for columns visibility. ColVis is a plugin for DataTables. It allows to change the visibility of the columns in the table. * This development adds: - the js and css file for ColVis - a new DB table 'columns_settings' - a new template plugin 'ColumnsSettings' - a new package C4::Utils::DataTables::ColumnsSettings - a new admin page admin/columns_settings.pl * How it works: A yaml file is created (admin/columns_settings.yml) in order to take an inventory of all tables where ColVis is implemented. This file is read to create the list of modules, pages and tables in the configuration page. There are 3 possible keys in the yml: - is_hidden: default is 0 The column will be hidden. - cannot_be_toggled: default is 0. ColVis will allow to hide/show the column. - cannot_be_modified: default is 0 Default values (in the yml) won't be modifiable. When a user changes (or saves) the configuration for one module, all columns are added to the DB table. The values in the DB get the upper hand on the yaml values. * Humm, strange? It seems weird to have 2 storages for the same values. But I think it will be easy to add an entry and maintain the yaml rather than adding a new row (and new entry in updatedatabase script) in the DB. * Test plan: 1/ Execute the updatedatabase in order to create the new table. 2/ Take a look to the yml structure. 3/ Go on the checkouts page (circ/circulation.pl). 4/ Check that you cannot hide the 3 last columns on the issues table. 5/ Check that you cannot hide the 2 last columns on the holds table. 6/ Try to hide/show columns. 7/ Go on the columns configuration page (admin/columns_settings.pl). 8/ Only the last tab are filled with data. Check/uncheck checkboxes and save. 9/ Go on the checkouts/holds page and check that the behavior is what you expected. 10/ Give me some feedback :) * To go further: We can imagine that the configuration is saved for each user (and not globally like it is made with this patch).
Created attachment 24085 [details] [review] Bug 10212: Make the table names translatable
Rebased patches.
Test after placing syspref UseTablesortForCirc = enable It works for checkout table and for holds table : column is hided and you can add it with the link "show/hide columns" "if Cannot be toggled" in admin. If "Cannot be toggled" is check you can't see the column name with the link "show/hide columns" It's a wonderful patch and it'll be great if we can have the thing in the serials module.
Patch tested with a sandbox, by sonia bouis <sonia.bouis@univ-lyon3.fr>
Created attachment 24149 [details] [review] Bug 10212: Columns configuration for tables - Add Colvis files Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr>
Created attachment 24150 [details] [review] Bug 10212: Columns configuration for tables This development introduces ColVis into Koha and provides a configuration page for columns visibility. ColVis is a plugin for DataTables. It allows to change the visibility of the columns in the table. * This development adds: - the js and css file for ColVis - a new DB table 'columns_settings' - a new template plugin 'ColumnsSettings' - a new package C4::Utils::DataTables::ColumnsSettings - a new admin page admin/columns_settings.pl * How it works: A yaml file is created (admin/columns_settings.yml) in order to take an inventory of all tables where ColVis is implemented. This file is read to create the list of modules, pages and tables in the configuration page. There are 3 possible keys in the yml: - is_hidden: default is 0 The column will be hidden. - cannot_be_toggled: default is 0. ColVis will allow to hide/show the column. - cannot_be_modified: default is 0 Default values (in the yml) won't be modifiable. When a user changes (or saves) the configuration for one module, all columns are added to the DB table. The values in the DB get the upper hand on the yaml values. * Humm, strange? It seems weird to have 2 storages for the same values. But I think it will be easy to add an entry and maintain the yaml rather than adding a new row (and new entry in updatedatabase script) in the DB. * Test plan: 1/ Execute the updatedatabase in order to create the new table. 2/ Take a look to the yml structure. 3/ Go on the checkouts page (circ/circulation.pl). 4/ Check that you cannot hide the 3 last columns on the issues table. 5/ Check that you cannot hide the 2 last columns on the holds table. 6/ Try to hide/show columns. 7/ Go on the columns configuration page (admin/columns_settings.pl). 8/ Only the last tab are filled with data. Check/uncheck checkboxes and save. 9/ Go on the checkouts/holds page and check that the behavior is what you expected. 10/ Give me some feedback :) * To go further: We can imagine that the configuration is saved for each user (and not globally like it is made with this patch). Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr>
Created attachment 24151 [details] [review] Bug 10212: Make the table names translatable Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr>
I will provide unit tests for new routines.
Created attachment 24167 [details] [review] Bug 10212: Columns configuration for tables - QA FIX - use YAML is missing in package - get_tables and get_excluded are useless routines and can be removed Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 24168 [details] [review] Bug 10212: Columns configuration for tables - Unit tests Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
FAIL Koha/Template/Plugin/ColumnsSettings.pm OK pod FAIL forbidden patterns forbidden pattern: Koha is now under the GPLv3 license (line 9) OK valid OK critic FAIL koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt FAIL forbidden patterns forbidden pattern: To include datatables files, please use the include file (see bug 10868) (line 20) OK tt_valid OK valid_template
Created attachment 24577 [details] [review] Bug 10212: FIX QA issues FAIL Koha/Template/Plugin/ColumnsSettings.pm FAIL forbidden patterns forbidden pattern: Koha is now under the GPLv3 license (line 9) FAIL koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt FAIL forbidden patterns forbidden pattern: To include datatables files, please use the include file (see bug 10868) (line 20)
Sorry, lots of conflicts: Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 10212: Columns configuration for tables - Add Colvis files Applying: Bug 10212: Columns configuration for tables Using index info to reconstruct a base tree... M circ/circulation.pl M installer/data/mysql/kohastructure.sql M installer/data/mysql/updatedatabase.pl M koha-tmpl/intranet-tmpl/prog/en/js/datatables.js M koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt M koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt <stdin>:223: new blank line at EOF. + warning: 1 line adds whitespace errors. Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/js/datatables.js CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/js/datatables.js Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Created attachment 27700 [details] [review] Bug 10212: Columns configuration for tables - Add Colvis files Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr>
Created attachment 27701 [details] [review] Bug 10212: Columns configuration for tables This development introduces ColVis into Koha and provides a configuration page for columns visibility. ColVis is a plugin for DataTables. It allows to change the visibility of the columns in the table. * This development adds: - the js and css file for ColVis - a new DB table 'columns_settings' - a new template plugin 'ColumnsSettings' - a new package C4::Utils::DataTables::ColumnsSettings - a new admin page admin/columns_settings.pl * How it works: A yaml file is created (admin/columns_settings.yml) in order to take an inventory of all tables where ColVis is implemented. This file is read to create the list of modules, pages and tables in the configuration page. There are 3 possible keys in the yml: - is_hidden: default is 0 The column will be hidden. - cannot_be_toggled: default is 0. ColVis will allow to hide/show the column. - cannot_be_modified: default is 0 Default values (in the yml) won't be modifiable. When a user changes (or saves) the configuration for one module, all columns are added to the DB table. The values in the DB get the upper hand on the yaml values. * Humm, strange? It seems weird to have 2 storages for the same values. But I think it will be easy to add an entry and maintain the yaml rather than adding a new row (and new entry in updatedatabase script) in the DB. * Test plan: 1/ Execute the updatedatabase in order to create the new table. 2/ Take a look to the yml structure. 3/ Go on the checkouts page (circ/circulation.pl). 4/ Check that you cannot hide the 3 last columns on the issues table. 5/ Check that you cannot hide the 2 last columns on the holds table. 6/ Try to hide/show columns. 7/ Go on the columns configuration page (admin/columns_settings.pl). 8/ Only the last tab are filled with data. Check/uncheck checkboxes and save. 9/ Go on the checkouts/holds page and check that the behavior is what you expected. 10/ Give me some feedback :) * To go further: We can imagine that the configuration is saved for each user (and not globally like it is made with this patch). Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr>
Created attachment 27702 [details] [review] Bug 10212: Make the table names translatable Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr>
Created attachment 27703 [details] [review] Bug 10212: Columns configuration for tables - QA FIX - use YAML is missing in package - get_tables and get_excluded are useless routines and can be removed Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 27704 [details] [review] Bug 10212: Columns configuration for tables - Unit tests Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 27705 [details] [review] Bug 10212: FIX QA issues FAIL Koha/Template/Plugin/ColumnsSettings.pm FAIL forbidden patterns forbidden pattern: Koha is now under the GPLv3 license (line 9) FAIL koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt FAIL forbidden patterns forbidden pattern: To include datatables files, please use the include file (see bug 10868) (line 20)
There are conflicts for the usual suspects and one in circulation.tt
Created attachment 29075 [details] [review] Bug 10212: Columns configuration for tables - Add Colvis files Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr>
Created attachment 29076 [details] [review] Bug 10212: Columns configuration for tables This development introduces ColVis into Koha and provides a configuration page for columns visibility. ColVis is a plugin for DataTables. It allows to change the visibility of the columns in the table. * This development adds: - the js and css file for ColVis - a new DB table 'columns_settings' - a new template plugin 'ColumnsSettings' - a new package C4::Utils::DataTables::ColumnsSettings - a new admin page admin/columns_settings.pl * How it works: A yaml file is created (admin/columns_settings.yml) in order to take an inventory of all tables where ColVis is implemented. This file is read to create the list of modules, pages and tables in the configuration page. There are 3 possible keys in the yml: - is_hidden: default is 0 The column will be hidden. - cannot_be_toggled: default is 0. ColVis will allow to hide/show the column. - cannot_be_modified: default is 0 Default values (in the yml) won't be modifiable. When a user changes (or saves) the configuration for one module, all columns are added to the DB table. The values in the DB get the upper hand on the yaml values. * Humm, strange? It seems weird to have 2 storages for the same values. But I think it will be easy to add an entry and maintain the yaml rather than adding a new row (and new entry in updatedatabase script) in the DB. * Test plan: 1/ Execute the updatedatabase in order to create the new table. 2/ Take a look to the yml structure. 3/ Go on the checkouts page (circ/circulation.pl). 4/ Check that you cannot hide the 3 last columns on the issues table. 5/ Check that you cannot hide the 2 last columns on the holds table. 6/ Try to hide/show columns. 7/ Go on the columns configuration page (admin/columns_settings.pl). 8/ Only the last tab are filled with data. Check/uncheck checkboxes and save. 9/ Go on the checkouts/holds page and check that the behavior is what you expected. 10/ Give me some feedback :) * To go further: We can imagine that the configuration is saved for each user (and not globally like it is made with this patch). Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr>
Created attachment 29077 [details] [review] Bug 10212: Make the table names translatable Signed-off-by: sonia bouis <sonia.bouis@univ-lyon3.fr>
Created attachment 29078 [details] [review] Bug 10212: Columns configuration for tables - QA FIX - use YAML is missing in package - get_tables and get_excluded are useless routines and can be removed Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 29079 [details] [review] Bug 10212: Columns configuration for tables - Unit tests Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 29080 [details] [review] Bug 10212: FIX QA issues FAIL Koha/Template/Plugin/ColumnsSettings.pm FAIL forbidden patterns forbidden pattern: Koha is now under the GPLv3 license (line 9) FAIL koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt FAIL forbidden patterns forbidden pattern: To include datatables files, please use the include file (see bug 10868) (line 20)
Last patches fix conflicts with bug 12089.
I am sorry Jonathan, with the changes to circ this will no longer apply :( Maybe another table could be used for the POC to avoid more conflicts while we are tying to work through this? Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 10212: Columns configuration for tables - Add Colvis files Applying: Bug 10212: Columns configuration for tables Using index info to reconstruct a base tree... M circ/circulation.pl M installer/data/mysql/updatedatabase.pl M koha-tmpl/intranet-tmpl/prog/en/js/datatables.js M koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt <stdin>:223: new blank line at EOF. + warning: 1 line adds whitespace errors. Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/js/datatables.js CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/js/datatables.js Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging circ/circulation.pl CONFLICT (content): Merge conflict in circ/circulation.pl Failed to merge in the changes. Patch failed at 0001 Bug 10212: Columns configuration for tables The copy of the patch that failed is found in: /home/katrin/kohaclone/.git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-10212-Columns-configuration-for-tables-o21Nap.patch
Created attachment 29537 [details] [review] Bug 10212: Columns configuration for tables This development introduces ColVis into Koha and provides a configuration page for columns visibility. ColVis is a plugin for DataTables. It allows to change the visibility of the columns in the table. * This development adds: - the js and css file for ColVis - a new DB table 'columns_settings' - a new template plugin 'ColumnsSettings' - a new package C4::Utils::DataTables::ColumnsSettings - a new admin page admin/columns_settings.pl * How it works: A yaml file is created (admin/columns_settings.yml) in order to take an inventory of all tables where ColVis is implemented. This file is read to create the list of modules, pages and tables in the configuration page. There are 3 possible keys in the yml: - is_hidden: default is 0 The column will be hidden. - cannot_be_toggled: default is 0. ColVis will allow to hide/show the column. - cannot_be_modified: default is 0 Default values (in the yml) won't be modifiable. When a user changes (or saves) the configuration for one module, all columns are added to the DB table. The values in the DB get the upper hand on the yaml values. * Humm, strange? It seems weird to have 2 storages for the same values. But I think it will be easy to add an entry and maintain the yaml rather than adding a new row (and new entry in updatedatabase script) in the DB. * To go further: We can imagine that the configuration is saved for each user (and not globally like it is made with this patch). This patch cannot be tested as it, you need to apply the "POC" patch.
Created attachment 29538 [details] [review] Bug 10212: Columns configuration for tables - DB changes
Created attachment 29539 [details] [review] Bug 10212: Columns configuration for tables - Unit tests test plan: Verify the prove t/db_dependent/ColumnsSettings.t returns green.
Created attachment 29540 [details] [review] Bug 10212: Columns configuration for tables - Add Colvis files This patch only adds Colvis js and css files.
Created attachment 29541 [details] [review] Bug 10212: Columns configuration for tables - POC This patch proves that this feature is useful with a concret use case. Test plan: 1/ Execute the updatedatabase in order to create the new table. 2/ Take a look to the yml structure. 3/ Go on the admin currency page (admin/currency.pl). 4/ Check that you cannot hide the 2 first columns on the table. 5/ Try to hide/show columns. 6/ Go on the columns configuration page (admin/columns_settings.pl). 7/ Only the admin tab are filled with data. Check/uncheck checkboxes and save. 8/ Go on the admin currency page and check that the behavior is what you expected. 9/ Give me some feedback :)
I add some significant changes in the last patch set : 1/ Use DBIC 2/ Reorganize patches for a better readability 3/ Remove changes in circ/circulation.pl|tt. I now use admin/currency.tt. Hopefully, this will be easier to maintain (less conflict). This new patch set needs a (quick?) new signoff please.
Hi Jonathan, thx for providing new patches, this is a fun feature! Starting with a code review: 1) QA script I had this error before I ran the database update: FAIL koha-tmpl/intranet-tmpl/prog/en/modules/admin/currency.tt OK forbidden patterns FAIL valid_template DBIx::Class::Storage::DBI::_dbh_execute(): Table 'koha.columns_settings' doesn't exist at /home/katrin/kohaclone/C4/Utils/DataTables/ColumnsSettings.pm line 34 After the database update QA script was ok. 2) Copyright - update? +# Copyright BibLibre 2013 3) Add new library to About page? jquery.dataTables.colvis.js 4) Capitalization Is Hidden by default Now onto testing: 5) Configuration page Nice accordeon :) Changes save correctly. a) After saving a new configuration, it would be nice if the section you worked in remained open, instead of it jumping back to the first - Acquisitions. b) I am not sure why currency is displayed twice in different font size? 6) Currencies page The table showed according to what I had configured. Changes made with the options on the page were remembered during my session. Table shows as normal with Javascript deactivated. a) The options to toggle/hide could be positioned a bit closer to the table, maybe we could even use an icon later? Just something we could try to improve a bit more.
Overall this looks good to me. I only have a few minor comments: - The plugin should be added to /intranet-tmpl/lib/jquery/plugins instead of to the language-specific directory - The page <title> for columns_settings.tt is "Tables" instead of "Columns settings." - Spelling: Use American English spellings "Catalog" and "Cataloging" on columns_settings.tt
One more thing: A link to columns configuration should be added to admin-menu.inc.
Created attachment 29701 [details] [review] Bug 10212: Columns configuration for tables This development introduces ColVis into Koha and provides a configuration page for columns visibility. ColVis is a plugin for DataTables. It allows to change the visibility of the columns in the table. * This development adds: - the js and css file for ColVis - a new DB table 'columns_settings' - a new template plugin 'ColumnsSettings' - a new package C4::Utils::DataTables::ColumnsSettings - a new admin page admin/columns_settings.pl * How it works: A yaml file is created (admin/columns_settings.yml) in order to take an inventory of all tables where ColVis is implemented. This file is read to create the list of modules, pages and tables in the configuration page. There are 3 possible keys in the yml: - is_hidden: default is 0 The column will be hidden. - cannot_be_toggled: default is 0. ColVis will allow to hide/show the column. - cannot_be_modified: default is 0 Default values (in the yml) won't be modifiable. When a user changes (or saves) the configuration for one module, all columns are added to the DB table. The values in the DB get the upper hand on the yaml values. * Humm, strange? It seems weird to have 2 storages for the same values. But I think it will be easy to add an entry and maintain the yaml rather than adding a new row (and new entry in updatedatabase script) in the DB. * To go further: We can imagine that the configuration is saved for each user (and not globally like it is made with this patch). This patch cannot be tested as it, you need to apply the "POC" patch.
Created attachment 29702 [details] [review] Bug 10212: Columns configuration for tables - DB changes
Created attachment 29703 [details] [review] Bug 10212: Columns configuration for tables - Unit tests test plan: Verify the prove t/db_dependent/ColumnsSettings.t returns green.
Created attachment 29704 [details] [review] Bug 10212: Columns configuration for tables - Add Colvis files This patch only adds Colvis js and css files.
Created attachment 29705 [details] [review] Bug 10212: Columns configuration for tables - POC This patch proves that this feature is useful with a concret use case. Test plan: 1/ Execute the updatedatabase in order to create the new table. 2/ Take a look to the yml structure. 3/ Go on the admin currency page (admin/currency.pl). 4/ Check that you cannot hide the 2 first columns on the table. 5/ Try to hide/show columns. 6/ Go on the columns configuration page (admin/columns_settings.pl). 7/ Only the admin tab are filled with data. Check/uncheck checkboxes and save. 8/ Go on the admin currency page and check that the behavior is what you expected. 9/ Give me some feedback :)
Katrin and Owen, Thanks for your feedbacks! (In reply to Katrin Fischer from comment #59) > 2) Copyright - update? > +# Copyright BibLibre 2013 Done. > 3) Add new library to About page? > jquery.dataTables.colvis.js Oops, forgot. Will be done soon. > 4) Capitalization > Is Hidden by default Done. > 5) Configuration page > a) After saving a new configuration, it would be nice if the section you > worked in remained open, instead of it jumping back to the first - > Acquisitions. Done. > b) I am not sure why currency is displayed twice in different font size? The first one is the "page name", the second one is the "table name". It will often be the same, but in some case, there will be 2 tables per page. > 6) Currencies page > a) The options to toggle/hide could be positioned a bit closer to the table, > maybe we could even use an icon later? Just something we could try to > improve a bit more. Not sure I am a really bad designer and I will certainly break something if I change the datatable css file. I would prefer to let this job to a css expert. (In reply to Owen Leonard from comment #60) > - The plugin should be added to /intranet-tmpl/lib/jquery/plugins instead of > to the language-specific directory Done. > - The page <title> for columns_settings.tt is "Tables" instead of "Columns > settings." Done. > - Spelling: Use American English spellings "Catalog" and "Cataloging" on > columns_settings.tt Done. (In reply to Owen Leonard from comment #61) > One more thing: A link to columns configuration should be added to > admin-menu.inc. Done.
Created attachment 29718 [details] [review] Bug 10212: Add Colvis to the about page
Created attachment 29830 [details] [review] Bug 10212: Move colvis files to the include file
Created attachment 29836 [details] [review] [SIGNED-OFF] Bug 10212: Columns configuration for tables This development introduces ColVis into Koha and provides a configuration page for columns visibility. ColVis is a plugin for DataTables. It allows to change the visibility of the columns in the table. * This development adds: - the js and css file for ColVis - a new DB table 'columns_settings' - a new template plugin 'ColumnsSettings' - a new package C4::Utils::DataTables::ColumnsSettings - a new admin page admin/columns_settings.pl * How it works: A yaml file is created (admin/columns_settings.yml) in order to take an inventory of all tables where ColVis is implemented. This file is read to create the list of modules, pages and tables in the configuration page. There are 3 possible keys in the yml: - is_hidden: default is 0 The column will be hidden. - cannot_be_toggled: default is 0. ColVis will allow to hide/show the column. - cannot_be_modified: default is 0 Default values (in the yml) won't be modifiable. When a user changes (or saves) the configuration for one module, all columns are added to the DB table. The values in the DB get the upper hand on the yaml values. * Humm, strange? It seems weird to have 2 storages for the same values. But I think it will be easy to add an entry and maintain the yaml rather than adding a new row (and new entry in updatedatabase script) in the DB. * To go further: We can imagine that the configuration is saved for each user (and not globally like it is made with this patch). This patch cannot be tested as it, you need to apply the "POC" patch. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 29837 [details] [review] [SIGNED-OFF] Bug 10212: Columns configuration for tables - DB changes Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 29838 [details] [review] [SIGNED-OFF] Bug 10212: Columns configuration for tables - Unit tests test plan: Verify the prove t/db_dependent/ColumnsSettings.t returns green. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 29839 [details] [review] [SIGNED-OFF] Bug 10212: Columns configuration for tables - Add Colvis files This patch only adds Colvis js and css files. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 29840 [details] [review] [SIGNED-OFF] Bug 10212: Columns configuration for tables - POC This patch proves that this feature is useful with a concret use case. Test plan: 1/ Execute the updatedatabase in order to create the new table. 2/ Take a look to the yml structure. 3/ Go on the admin currency page (admin/currency.pl). 4/ Check that you cannot hide the 2 first columns on the table. 5/ Try to hide/show columns. 6/ Go on the columns configuration page (admin/columns_settings.pl). 7/ Only the admin tab are filled with data. Check/uncheck checkboxes and save. 8/ Go on the admin currency page and check that the behavior is what you expected. 9/ Give me some feedback :) Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 29841 [details] [review] [SIGNED-OFF] Bug 10212: Add Colvis to the about page Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 29842 [details] [review] [SIGNED-OFF] Bug 10212: Move colvis files to the include file Signed-off-by: Owen Leonard <oleonard@myacpl.org>
(In reply to Katrin Fischer from comment #59) > a) The options to toggle/hide could be positioned a bit closer to the table, > maybe we could even use an icon later? I think this could be improved but I don't have a good suggestion right now. I'm going to keep thinking about it.
Created attachment 29882 [details] [review] [PASSED QA] Bug 10212: Columns configuration for tables This development introduces ColVis into Koha and provides a configuration page for columns visibility. ColVis is a plugin for DataTables. It allows to change the visibility of the columns in the table. * This development adds: - the js and css file for ColVis - a new DB table 'columns_settings' - a new template plugin 'ColumnsSettings' - a new package C4::Utils::DataTables::ColumnsSettings - a new admin page admin/columns_settings.pl * How it works: A yaml file is created (admin/columns_settings.yml) in order to take an inventory of all tables where ColVis is implemented. This file is read to create the list of modules, pages and tables in the configuration page. There are 3 possible keys in the yml: - is_hidden: default is 0 The column will be hidden. - cannot_be_toggled: default is 0. ColVis will allow to hide/show the column. - cannot_be_modified: default is 0 Default values (in the yml) won't be modifiable. When a user changes (or saves) the configuration for one module, all columns are added to the DB table. The values in the DB get the upper hand on the yaml values. * Humm, strange? It seems weird to have 2 storages for the same values. But I think it will be easy to add an entry and maintain the yaml rather than adding a new row (and new entry in updatedatabase script) in the DB. * To go further: We can imagine that the configuration is saved for each user (and not globally like it is made with this patch). This patch cannot be tested as it, you need to apply the "POC" patch. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests and QA script, more comments on last patch.
Created attachment 29883 [details] [review] [PASSED QA] Bug 10212: Columns configuration for tables - DB changes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 29884 [details] [review] [PASSED QA] Bug 10212: Columns configuration for tables - Unit tests test plan: Verify the prove t/db_dependent/ColumnsSettings.t returns green. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 29885 [details] [review] [PASSED QA] Bug 10212: Columns configuration for tables - Add Colvis files This patch only adds Colvis js and css files. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 29886 [details] [review] [PASSED QA] Bug 10212: Columns configuration for tables - POC This patch proves that this feature is useful with a concret use case. Test plan: 1/ Execute the updatedatabase in order to create the new table. 2/ Take a look to the yml structure. 3/ Go on the admin currency page (admin/currency.pl). 4/ Check that you cannot hide the 2 first columns on the table. 5/ Try to hide/show columns. 6/ Go on the columns configuration page (admin/columns_settings.pl). 7/ Only the admin tab are filled with data. Check/uncheck checkboxes and save. 8/ Go on the admin currency page and check that the behavior is what you expected. 9/ Give me some feedback :) Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 29887 [details] [review] [PASSED QA] Bug 10212: Add Colvis to the about page Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 29888 [details] [review] [PASSED QA] Bug 10212: Move colvis files to the include file Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests and QA script. Implements the option to change the default columns that appear in a table while giving the user the option to change the columns individually for a session. As an example, this feature is implemented for the currencies table. Works well for all possible combinations in configuration.
Comment on attachment 29882 [details] [review] [PASSED QA] Bug 10212: Columns configuration for tables Review of attachment 29882 [details] [review]: ----------------------------------------------------------------- ::: C4/Utils/DataTables/ColumnsSettings.pm @@ +66,5 @@ > + for my $c (@$columns) { > + $c->{is_hidden} //= 0; > + $c->{cannot_be_toggled} //= 0; > + > + my $column = $schema->resultset('ColumnsSetting')->search( Have you considered using "update_or_create" instead of these 3 methods? Admittedly, I'm not sure what about the significance of "$column = $column->first;" below. However, if that's the case, I'd suggest "find_or_new" or "find_or_create". http://search.cpan.org/~ribasushi/DBIx-Class-0.08270/lib/DBIx/Class/ResultSet.pm#update_or_create my $column = $schema->resultset('ColumnsSetting')->update_or_create({ module => $c->{module}, page => $c->{page}, tablename => $c->{tablename}, columnname => $c->{columnname}, is_hidden => $c->{is_hidden}, cannot_be_toggled => $c->{cannot_be_toggled}, }); Regardless of whether you choose to go with this method, I think it would be a good idea to add a primary key constraint to your DbiC methods. I haven't looked at the proposed table structure, so I don't know if there are any, but probably a good idea.
I haven't tested any of these patches, but excited to have a mechanism for controlling column visibility. I thought about adding the ColVis files locally, so I'm happy to see a patch for them to go into Koha proper.
Created attachment 29947 [details] [review] Bug 10212: Use the update_or_create DBIC method when possible Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
(In reply to David Cook from comment #85) > ::: C4/Utils/DataTables/ColumnsSettings.pm > @@ +66,5 @@ > > + for my $c (@$columns) { > > + $c->{is_hidden} //= 0; > > + $c->{cannot_be_toggled} //= 0; > > + > > + my $column = $schema->resultset('ColumnsSetting')->search( > > Have you considered using "update_or_create" instead of these 3 methods? Hum... don't repeat what you saw ;)
(In reply to Jonathan Druart from comment #88) > (In reply to David Cook from comment #85) > > ::: C4/Utils/DataTables/ColumnsSettings.pm > > @@ +66,5 @@ > > > + for my $c (@$columns) { > > > + $c->{is_hidden} //= 0; > > > + $c->{cannot_be_toggled} //= 0; > > > + > > > + my $column = $schema->resultset('ColumnsSetting')->search( > > > > Have you considered using "update_or_create" instead of these 3 methods? > > Hum... don't repeat what you saw ;) Hehe. No worries. I've been reading a lot about DBIC this week, and thought it might be relevant :).
Created attachment 31158 [details] [review] Bug 10212: (qa followup) accordion collapsed by default By default, the accordion shows the first element (Acquisitions) which is empty, and missleading. This patch collapses it by default. Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Created attachment 31159 [details] [review] Bug 10212: (qa followup) accordion collapsed by default By default, the accordion shows the first element (Acquisitions) which is empty, and missleading. This patch collapses it by default. The expected behaviour is that if we are rendering because the user saved its changes, the modified accordion should be expanded, so I added a test for the $panel variable. Template comment added for future references about it. Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Patches pushed to master. Thanks Jonathan!
> TypeError: a.aoColumns[c] is https://www.stepcalculator.com/ undefined > /intranet-tmpl/prog/en/lib/jquery/plugins/jquery.dataTables.min.js > Line 29
(In reply to martha simons from comment #93) > > TypeError: a.aoColumns[c] is https://www.stepcalculator.com/ undefined > > /intranet-tmpl/prog/en/lib/jquery/plugins/jquery.dataTables.min.js > > Line 29 Hi Martha, please file a new bug with a step-by-step description on how to get to this error. Thank you.