|
Description
Jonathan Druart
2019-12-03 19:33:43 UTC
Created attachment 96223 [details] [review] Bug 24156: DB changes Add a new table tables_settings to store default_display_length and default_sort_order. Created attachment 96224 [details] [review] Bug 24156: DBIC changes Created attachment 96225 [details] [review] Bug 24156: move ColumnsSettings to TablesSettings We are preparing the ground with this patch. As the "Columns settings" page will now add the ability to modify settings for the whole table, it makes sense to rename the file and the variables. Note that the controller script (admin/columns_settings.pl) and the yml (admin/columns_settings.yml) files have not been moved to not break shortcuts and abits people could have. But if QA decides, it could be easy to do. Created attachment 96226 [details] [review] Bug 24156: Add columns_settings.yml changes Modification of the structure of the yml file. It was table: - column list It now becomes table: default_display_length: X default_sort_order: Y columns: - column list Created attachment 96227 [details] [review] Bug 24156: Add missing columns to yml There were 2 columns missing Created attachment 96228 [details] [review] Bug 24156: Make sort order and number of items to display configurable (basket page) This patch is the main patch of this patchset, you will find the description and the test plan. The idea of this new enhancement is to add the ability to define the default sort order and the default number of rows displayed on the acquisition basket page. The existing "columns settings" feature was replaced by a "tables settings" feature. To prepare the ground, there were some works that were needed: * rename variables and files * Modify the structure of the yml files * Create a new DB table to store the tables settings Test plan: 0) a. Execute the update DB entry to create the new table b. Restart all (to get a new version of the yml file, that is cached by memcached) c. Create several orders for a given basket 1) Go to the basket view page => The default values are the same than without this patchset, the number of entries to display is set to "20" and the table is sorted by basket number (first column) 2) Go to the "Columns settings" page 3) Unfold the "Acquisition" tab => Notice the 2 dropdown lists at the bottom of the basket table 4) Select different values for "Default display length" and "Default sort order" 5) Refresh the basket view page => Notice that the default settings are now effective on the table QA note: We can decide to replace the different occurrences of "Columns settings" by "Tables settings" if needed. Created attachment 96229 [details] [review] Bug 24156: Adjust other tables in the yml file To match the new structure, we adjust the other tables. Created attachment 96230 [details] [review] Bug 24156: Make sort order and number of items to display configurable (basket page) This patch is the main patch of this patchset, you will find the description and the test plan. The idea of this new enhancement is to add the ability to define the default sort order and the default number of rows displayed on the acquisition basket page. The existing "columns settings" feature was replaced by a "tables settings" feature. To prepare the ground, there were some works that were needed: * rename variables and files * Modify the structure of the yml files * Create a new DB table to store the tables settings Test plan: 0) a. Execute the update DB entry to create the new table b. Restart all (to get a new version of the yml file, that is cached by memcached) c. Create several orders for a given basket 1) Go to the basket view page => The default values are the same than without this patchset, the number of entries to display is set to "20" and the table is sorted by basket number (first column) 2) Go to the "Columns settings" page 3) Unfold the "Acquisition" tab => Notice the 2 dropdown lists at the bottom of the basket table 4) Select different values for "Default display length" and "Default sort order" 5) Refresh the basket view page => Notice that the default settings are now effective on the table QA note: We can decide to replace the different occurrences of "Columns settings" by "Tables settings" if needed. Sponsored-by: Institute of Technology Tallaght I notice the choices offered under "Default sort order" include columns which are configured in the individual DataTable configuration to be unsortable. I wonder if we need to move those settings into columns_settings.yml too? (In reply to Owen Leonard from comment #9) > I notice the choices offered under "Default sort order" include columns > which are configured in the individual DataTable configuration to be > unsortable. I wonder if we need to move those settings into > columns_settings.yml too? Hi Owen, Thanks for taking a look at this. The "default sort order" dropdown list includes all the columns of the table, indeed. That could lead to inconsistencies but dealing with that on this page will add much more complexity. Moving the sortable bit to this config page is a good idea, if we agree on such move I think it should be done for all the tables in one go, which can be quite hard to do. I keep that in mind and will think about it to see what could be possible. Created attachment 98449 [details] [review] Bug 24156: DB changes Add a new table tables_settings to store default_display_length and default_sort_order. Created attachment 98450 [details] [review] Bug 24156: DBIC changes Created attachment 98451 [details] [review] Bug 24156: move ColumnsSettings to TablesSettings We are preparing the ground with this patch. As the "Columns settings" page will now add the ability to modify settings for the whole table, it makes sense to rename the file and the variables. Note that the controller script (admin/columns_settings.pl) and the yml (admin/columns_settings.yml) files have not been moved to not break shortcuts and abits people could have. But if QA decides, it could be easy to do. Created attachment 98452 [details] [review] Bug 24156: Add columns_settings.yml changes Modification of the structure of the yml file. It was table: - column list It now becomes table: default_display_length: X default_sort_order: Y columns: - column list Created attachment 98453 [details] [review] Bug 24156: Add missing columns to yml There were 2 columns missing Created attachment 98454 [details] [review] Bug 24156: Adjust other tables in the yml file To match the new structure, we adjust the other tables. Created attachment 98455 [details] [review] Bug 24156: Make sort order and number of items to display configurable (basket page) This patch is the main patch of this patchset, you will find the description and the test plan. The idea of this new enhancement is to add the ability to define the default sort order and the default number of rows displayed on the acquisition basket page. The existing "columns settings" feature was replaced by a "tables settings" feature. To prepare the ground, there were some works that were needed: * rename variables and files * Modify the structure of the yml files * Create a new DB table to store the tables settings Test plan: 0) a. Execute the update DB entry to create the new table b. Restart all (to get a new version of the yml file, that is cached by memcached) c. Create several orders for a given basket 1) Go to the basket view page => The default values are the same than without this patchset, the number of entries to display is set to "20" and the table is sorted by basket number (first column) 2) Go to the "Columns settings" page 3) Unfold the "Acquisition" tab => Notice the 2 dropdown lists at the bottom of the basket table 4) Select different values for "Default display length" and "Default sort order" 5) Refresh the basket view page => Notice that the default settings are now effective on the table QA note: We can decide to replace the different occurrences of "Columns settings" by "Tables settings" if needed. Sponsored-by: Institute of Technology Tallaght Created attachment 98456 [details] [review] Bug 24156: Fix set sort order to the first column In that case it's equal to 0 and we need to adjust the test condition. Created attachment 98843 [details] [review] Bug 24156: move ColumnsSettings to TablesSettings We are preparing the ground with this patch. As the "Columns settings" page will now add the ability to modify settings for the whole table, it makes sense to rename the file and the variables. Note that the controller script (admin/columns_settings.pl) and the yml (admin/columns_settings.yml) files have not been moved to not break shortcuts and abits people could have. But if QA decides, it could be easy to do. Created attachment 98844 [details] [review] Bug 24156: Add columns_settings.yml changes Modification of the structure of the yml file. It was table: - column list It now becomes table: default_display_length: X default_sort_order: Y columns: - column list Created attachment 98845 [details] [review] Bug 24156: Add missing columns to yml There were 2 columns missing Created attachment 98846 [details] [review] Bug 24156: Adjust other tables in the yml file To match the new structure, we adjust the other tables. Created attachment 98847 [details] [review] Bug 24156: Make sort order and number of items to display configurable (basket page) This patch is the main patch of this patchset, you will find the description and the test plan. The idea of this new enhancement is to add the ability to define the default sort order and the default number of rows displayed on the acquisition basket page. The existing "columns settings" feature was replaced by a "tables settings" feature. To prepare the ground, there were some works that were needed: * rename variables and files * Modify the structure of the yml files * Create a new DB table to store the tables settings Test plan: 0) a. Execute the update DB entry to create the new table b. Restart all (to get a new version of the yml file, that is cached by memcached) c. Create several orders for a given basket 1) Go to the basket view page => The default values are the same than without this patchset, the number of entries to display is set to "20" and the table is sorted by basket number (first column) 2) Go to the "Columns settings" page 3) Unfold the "Acquisition" tab => Notice the 2 dropdown lists at the bottom of the basket table 4) Select different values for "Default display length" and "Default sort order" 5) Refresh the basket view page => Notice that the default settings are now effective on the table QA note: We can decide to replace the different occurrences of "Columns settings" by "Tables settings" if needed. Sponsored-by: Institute of Technology Tallaght Created attachment 98848 [details] [review] Bug 24156: Fix set sort order to the first column In that case it's equal to 0 and we need to adjust the test condition. Patches rebased against master. Created attachment 99007 [details] [review] Bug 24156: DB changes Add a new table tables_settings to store default_display_length and default_sort_order. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 99008 [details] [review] Bug 24156: DBIC changes Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 99009 [details] [review] Bug 24156: move ColumnsSettings to TablesSettings We are preparing the ground with this patch. As the "Columns settings" page will now add the ability to modify settings for the whole table, it makes sense to rename the file and the variables. Note that the controller script (admin/columns_settings.pl) and the yml (admin/columns_settings.yml) files have not been moved to not break shortcuts and abits people could have. But if QA decides, it could be easy to do. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 99010 [details] [review] Bug 24156: Add columns_settings.yml changes Modification of the structure of the yml file. It was table: - column list It now becomes table: default_display_length: X default_sort_order: Y columns: - column list Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 99011 [details] [review] Bug 24156: Add missing columns to yml There were 2 columns missing Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 99012 [details] [review] Bug 24156: Adjust other tables in the yml file To match the new structure, we adjust the other tables. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 99013 [details] [review] Bug 24156: Make sort order and number of items to display configurable (basket page) This patch is the main patch of this patchset, you will find the description and the test plan. The idea of this new enhancement is to add the ability to define the default sort order and the default number of rows displayed on the acquisition basket page. The existing "columns settings" feature was replaced by a "tables settings" feature. To prepare the ground, there were some works that were needed: * rename variables and files * Modify the structure of the yml files * Create a new DB table to store the tables settings Test plan: 0) a. Execute the update DB entry to create the new table b. Restart all (to get a new version of the yml file, that is cached by memcached) c. Create several orders for a given basket 1) Go to the basket view page => The default values are the same than without this patchset, the number of entries to display is set to "20" and the table is sorted by basket number (first column) 2) Go to the "Columns settings" page 3) Unfold the "Acquisition" tab => Notice the 2 dropdown lists at the bottom of the basket table 4) Select different values for "Default display length" and "Default sort order" 5) Refresh the basket view page => Notice that the default settings are now effective on the table QA note: We can decide to replace the different occurrences of "Columns settings" by "Tables settings" if needed. Sponsored-by: Institute of Technology Tallaght Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 99014 [details] [review] Bug 24156: Fix set sort order to the first column In that case it's equal to 0 and we need to adjust the test condition. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> Jonathan, can you rebase please ? Created attachment 105823 [details] [review] Bug 24156: DB changes Add a new table tables_settings to store default_display_length and default_sort_order. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 105824 [details] [review] Bug 24156: DBIC changes Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 105825 [details] [review] Bug 24156: move ColumnsSettings to TablesSettings We are preparing the ground with this patch. As the "Columns settings" page will now add the ability to modify settings for the whole table, it makes sense to rename the file and the variables. Note that the controller script (admin/columns_settings.pl) and the yml (admin/columns_settings.yml) files have not been moved to not break shortcuts and abits people could have. But if QA decides, it could be easy to do. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 105826 [details] [review] Bug 24156: Add columns_settings.yml changes Modification of the structure of the yml file. It was table: - column list It now becomes table: default_display_length: X default_sort_order: Y columns: - column list Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 105827 [details] [review] Bug 24156: Adjust other tables in the yml file To match the new structure, we adjust the other tables. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 105828 [details] [review] Bug 24156: Make sort order and number of items to display configurable (basket page) This patch is the main patch of this patchset, you will find the description and the test plan. The idea of this new enhancement is to add the ability to define the default sort order and the default number of rows displayed on the acquisition basket page. The existing "columns settings" feature was replaced by a "tables settings" feature. To prepare the ground, there were some works that were needed: * rename variables and files * Modify the structure of the yml files * Create a new DB table to store the tables settings Test plan: 0) a. Execute the update DB entry to create the new table b. Restart all (to get a new version of the yml file, that is cached by memcached) c. Create several orders for a given basket 1) Go to the basket view page => The default values are the same than without this patchset, the number of entries to display is set to "20" and the table is sorted by basket number (first column) 2) Go to the "Columns settings" page 3) Unfold the "Acquisition" tab => Notice the 2 dropdown lists at the bottom of the basket table 4) Select different values for "Default display length" and "Default sort order" 5) Refresh the basket view page => Notice that the default settings are now effective on the table QA note: We can decide to replace the different occurrences of "Columns settings" by "Tables settings" if needed. Sponsored-by: Institute of Technology Tallaght Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 105829 [details] [review] Bug 24156: Fix set sort order to the first column In that case it's equal to 0 and we need to adjust the test condition. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> Comment on attachment 105828 [details] [review] Bug 24156: Make sort order and number of items to display configurable (basket page) Review of attachment 105828 [details] [review]: ----------------------------------------------------------------- ::: C4/Utils/DataTables/TablesSettings.pm @@ +59,4 @@ > return $columns; > } > > +sub get_table_settings { No pod covering ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/columns_settings.tt @@ +4,3 @@ > [% SET footerjs = 1 %] > [% SET panel_id = 0 %] > +[% USE Dumper %] Debug thing ? @@ +100,5 @@ > + <select name="[% table_id | html %]_default_display_length"> > + [% IF table_settings.default_display_length == 10 %] > + <option value="10" selected="selected">10</option> > + [% ELSE %] > + [% END %] This makes the "10" option never visible t/db_dependent/Koha/Template/Plugin/ColumnsSettings.t fail (In reply to Jonathan Druart from comment #6) > QA note: We can decide to replace the different occurrences of "Columns > settings" > by "Tables settings" if needed. Since default display length is not column related, i agree. At least the visible string (Admin columns settings title). Not a blocker Created attachment 106183 [details] [review] Bug 24156: DB changes Add a new table tables_settings to store default_display_length and default_sort_order. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 106184 [details] [review] Bug 24156: DBIC changes Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 106185 [details] [review] Bug 24156: move ColumnsSettings to TablesSettings We are preparing the ground with this patch. As the "Columns settings" page will now add the ability to modify settings for the whole table, it makes sense to rename the file and the variables. Note that the controller script (admin/columns_settings.pl) and the yml (admin/columns_settings.yml) files have not been moved to not break shortcuts and abits people could have. But if QA decides, it could be easy to do. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 106186 [details] [review] Bug 24156: Add columns_settings.yml changes Modification of the structure of the yml file. It was table: - column list It now becomes table: default_display_length: X default_sort_order: Y columns: - column list Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 106187 [details] [review] Bug 24156: Adjust other tables in the yml file To match the new structure, we adjust the other tables. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 106188 [details] [review] Bug 24156: Make sort order and number of items to display configurable (basket page) This patch is the main patch of this patchset, you will find the description and the test plan. The idea of this new enhancement is to add the ability to define the default sort order and the default number of rows displayed on the acquisition basket page. The existing "columns settings" feature was replaced by a "tables settings" feature. To prepare the ground, there were some works that were needed: * rename variables and files * Modify the structure of the yml files * Create a new DB table to store the tables settings Test plan: 0) a. Execute the update DB entry to create the new table b. Restart all (to get a new version of the yml file, that is cached by memcached) c. Create several orders for a given basket 1) Go to the basket view page => The default values are the same than without this patchset, the number of entries to display is set to "20" and the table is sorted by basket number (first column) 2) Go to the "Columns settings" page 3) Unfold the "Acquisition" tab => Notice the 2 dropdown lists at the bottom of the basket table 4) Select different values for "Default display length" and "Default sort order" 5) Refresh the basket view page => Notice that the default settings are now effective on the table QA note: We can decide to replace the different occurrences of "Columns settings" by "Tables settings" if needed. Sponsored-by: Institute of Technology Tallaght Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 106189 [details] [review] Bug 24156: Fix set sort order to the first column In that case it's equal to 0 and we need to adjust the test condition. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 106190 [details] [review] Bug 24156: Fix soem QA failures * Add POD to get_table_settings * Remove USE Dumper debug statement * Add missing "10" entry * Fix newly created test file (and renamed) Created attachment 106191 [details] [review] Bug 24156: Rename 'Columns settings' with 'Tables settings' on the UI All fixed, Alex. Thanks! Created attachment 106219 [details] [review] Bug 24156: Rename 'Columns settings' with 'Tables settings' on the UI Created attachment 106220 [details] [review] Bug 24156: DB changes Add a new table tables_settings to store default_display_length and default_sort_order. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 106221 [details] [review] Bug 24156: DBIC changes Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 106222 [details] [review] Bug 24156: move ColumnsSettings to TablesSettings We are preparing the ground with this patch. As the "Columns settings" page will now add the ability to modify settings for the whole table, it makes sense to rename the file and the variables. Note that the controller script (admin/columns_settings.pl) and the yml (admin/columns_settings.yml) files have not been moved to not break shortcuts and abits people could have. But if QA decides, it could be easy to do. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 106223 [details] [review] Bug 24156: Add columns_settings.yml changes Modification of the structure of the yml file. It was table: - column list It now becomes table: default_display_length: X default_sort_order: Y columns: - column list Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 106224 [details] [review] Bug 24156: Adjust other tables in the yml file To match the new structure, we adjust the other tables. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 106225 [details] [review] Bug 24156: Make sort order and number of items to display configurable (basket page) This patch is the main patch of this patchset, you will find the description and the test plan. The idea of this new enhancement is to add the ability to define the default sort order and the default number of rows displayed on the acquisition basket page. The existing "columns settings" feature was replaced by a "tables settings" feature. To prepare the ground, there were some works that were needed: * rename variables and files * Modify the structure of the yml files * Create a new DB table to store the tables settings Test plan: 0) a. Execute the update DB entry to create the new table b. Restart all (to get a new version of the yml file, that is cached by memcached) c. Create several orders for a given basket 1) Go to the basket view page => The default values are the same than without this patchset, the number of entries to display is set to "20" and the table is sorted by basket number (first column) 2) Go to the "Columns settings" page 3) Unfold the "Acquisition" tab => Notice the 2 dropdown lists at the bottom of the basket table 4) Select different values for "Default display length" and "Default sort order" 5) Refresh the basket view page => Notice that the default settings are now effective on the table QA note: We can decide to replace the different occurrences of "Columns settings" by "Tables settings" if needed. Sponsored-by: Institute of Technology Tallaght Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 106226 [details] [review] Bug 24156: Fix set sort order to the first column In that case it's equal to 0 and we need to adjust the test condition. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 106227 [details] [review] Bug 24156: Fix soem QA failures * Add POD to get_table_settings * Remove USE Dumper debug statement * Add missing "10" entry * Fix newly created test file (and renamed) Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 106228 [details] [review] Bug 24156: Rename 'Columns settings' with 'Tables settings' on the UI Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> Created attachment 106245 [details] [review] Bug 24156: (follow-up) UI - Use "Table settings" After a discussion with Owen we ended up with this decision. Pushed to master for 20.11, thanks to everybody involved! not backporting this enhancement to the 20.05.x series |