Created attachment 144533 [details] Screenshot of the problem In record details page when activating filters in items table, no input appears for barcode column, it is only a text.
Also confirmed on current master.
It's not specific to barcode. If you add an URL to the item that makes an additional column after barcode appear. Now the URL field and actions are not filterable. It appears to always be the last 2 columns?
Created attachment 146528 [details] [review] Bug 32447: Clean up table configuraton settings for holdings table
I was not able to fix the issue, but cleaned up the table configuration some in the process. Maybe it will help.
(In reply to Katrin Fischer from comment #2) > It's not specific to barcode. If you add an URL to the item that makes an > additional column after barcode appear. Now the URL field and actions are > not filterable. It appears to always be the last 2 columns? And then... it is specific to barcode. I think maybe some change I made in between moved it, but now we are back to barcode not being filterable, even with other new columns appearing in between? I don't understand the datatables well enough to see what's going on here.
Andreas, are you working on a patch? I stared at this forever... I really want to see this fixed (and see what the solution was!)
(In reply to Katrin Fischer from comment #6) > Andreas, are you working on a patch? I stared at this forever... I really > want to see this fixed (and see what the solution was!) Indeed I am, Katrin. I actually stared at this forever too, and almost broke my F11 key while debugging the code in Firefox's Developer Tools over the last few days ;-) I realised that if you undo the changes from commit 018a981b9b the barcode search box appears again and searching for barcodes is working, but there's still the issue of how the DataTable behaves when you start hiding colums. Anyway, I have a patch ready which fixes the issue described here, and also fixes the issue from Bug 32448. As a matter of fact, I think that both bugs are a bit similar since it's the presence of hidden columns that causes the erratic DataTable. I'm now in the process of preparing a Selenium unit test that will help us detect problems with this particular DataTable in the future. Since you're eager to see the solution ;-) I uploaded the patch here: https://diffy.org/diff/ceabcff747159 (toggling the mode to side-by-side makes it easier to read) BTW, I didn't know which of the two Bugs I should assign myself to, so I went for this one as it was the first one reported.
Apparently, there's just one more case where a DataTable (DT) can be filtered with search <input>s and that takes place in reports/itemslost.pl (Reports > Lost items > Results): $ git grep --name-only table_filters.js -- ':!*.po' koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt koha-tmpl/intranet-tmpl/prog/en/modules/reports/itemslost.tt Because the lost items results page is structured differently compared to the bibliographic record details page (for example the header cells don't have ids or data-colname attributes), the patch mentioned earlier inadvertently breaks the 'Activate filters' link with the following message logged in the console: Uncaught Error: Syntax error, unrecognized expression: tr.columnFilter # input I actually managed to fix that and got the link to work either by manually adding unique ids to the <th> elements _or_ by changing the related jQuery selector in my patch to not depend on the cell id (the latter is probably a better idea until we sort out the template file in question on a separate bug). Then, clicking on 'Activate filters' worked but I run into a different issue: the <input> elements were not being created by the columnFilter DT plugin. To fix that, I temporarily modified the function bound to the 'Activate filters' button's click() event so that it always passes bRedrawFilters == true when calling activate_filters(), effectively redrawing the DT filters every time their visibility (not to be confused with column visibility!) is toggled. Now the <input>s appeared and search was working properly save for the erratic behaviour whenever you toggled a column (outlined in Bug 32448). To fix that I added the necessary columnsInit() function to koha-tmpl/intranet-tmpl/prog/en/modules/reports/itemslost.tt and reverted the temporary change I made above so that it would now pass bRedrawFilters == false (which had to be the optimal solution as you should't have to redraw when you just want to display, right?) There was a glitch, though: unless I toggled the visibility of at least one column *before* activating the DT filters, the DT filters would not render when I clicked on the 'Activate filters' button. And here's why: In my patch I take advantage of the already existing code in koha-tmpl/intranet-tmpl/prog/en/includes/columns_settings.inc that attaches an event handler function to the DataTables column-visibility.dt event, which in turn calls columnsInit() (should that function be defined elsewhere): 213 table.dataTable(new_parameters); 214 table.DataTable().on("column-visibility.dt", function(){ 215 if( typeof columnsInit == 'function' ){ 216 // This function can be created separately and used to trigger 217 // an event after the DataTable has loaded AND column visibility 218 // has been updated according to the table's configuration 219 columnsInit(); 220 } 221 }).columns( hidden_ids ).visible( false ); The only modification I made to the above code was to change the columnsInit() call to also pass on the DT table object: 219 columnsInit(this); To make this work I also added the following code in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt: 1969 function columnsInit(table) { 1970 activate_filters(table.id, false, true); 1971 } That way, whenever a table column is added to/removed from the table via the "⚙ Columns" button, the DT filters are redrawn and their search boxes work just fine. There's a crucial point to be made, however, which is this: Because the 'Lost items' results DT currently has no hidden columns (per the admin/columns_settings.yml column settings), the column-visibility.dt event does _not_ fire upon page load to trigger the initial render of the table filters. To avoid this, the simple-yet-not-optimal solution is to simply call activate_filters() with bRedrawFilters == true all the time!
Unfortunately, I can't get my Selenium unit test to work consistently as it fails randomly while testing the 'Holdings' and 'Other holdings' DataTables. Most of the time it passes, but sometimes it fails with: Unable to locate element: //*[@id="otherholdings_table_activate_filters"] Also, sometimes the 'Holdings' DataTable will not include both items from the bibliographic record I'm creating within the unit test, and this causes my tests to fail. I've added a 10-second sleep to ensure Zebra has indexed the records by the time I'm drawing the table, but it doesn't seem to make a difference. I have some difficulty in designing the subtest for the Lost items report too, as the 'Activate filters' link element does not react when clicked and does not expand the table filters. Clicking on the same link in the first two tests works fine, so I'm at loss as to what's causing this... For these reasons, I'm just going to attach only my patch for the time being and leave the Selenium unit test for later. I've manually tested the proposed changes extensively and also wrote a detailed test plan -- hopefully that should be enough to move this forward.
Created attachment 147024 [details] [review] Bug 32447: Fix DataTable filtering when hidden columns are in place The bibliographic record's details page in the Staff interface includes a 'Holdings' table at the bottom with information for each item attached to the record. When activating the filters in this table, there is no input field for the barcode column but just bold text. This broke in v22.11.00, the related commit being 018a981b9b from Bug 29282 where two new hidden columns were added to that table. We can fix this by taking advantage of the existing code in koha-tmpl/intranet-tmpl/prog/en/includes/columns_settings.inc (introduced by commit dfb7af91af6 from Bug 23307) which allows us to create and hook our own custom columnsInit() function in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt to redraw the DataTable filters upon page load if a column is marked as 'is_hidden: 1' in admin/columns_settings.yml, or if a column is added/removed via the "⚙ Columns" button (both are handled by the DataTables column-visibility.dt event). Redrawing the filters via the above method also fixes the issue described in Bug 32448. Test plan: 1) Confirm the erratic DataTable behaviour outlined above 2) Apply this patch and reload all JS assets (hit CTRL-F5) 3) Confirm that you now see the correct input text field for the 'Barcode' column 4) Confirm that you can search for barcodes or in any other column successfully 5) Try toggling the visibility of the columns and making as many search variations as possible -- it should all now work without any glitches! For extra credit ;-) you can also test the 'Other holdings' table by setting the SeparateHoldings SysPref to 'Separate'.
I've uploaded my proposed Selenium unit test here: https://gist.github.com/a-roussos/438e69fc9d52bf9feb69760baf1efae6
Created attachment 147684 [details] [review] Bug 32447: Clean up table configuraton settings for holdings table Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 147685 [details] [review] Bug 32447: Fix DataTable filtering when hidden columns are in place The bibliographic record's details page in the Staff interface includes a 'Holdings' table at the bottom with information for each item attached to the record. When activating the filters in this table, there is no input field for the barcode column but just bold text. This broke in v22.11.00, the related commit being 018a981b9b from Bug 29282 where two new hidden columns were added to that table. We can fix this by taking advantage of the existing code in koha-tmpl/intranet-tmpl/prog/en/includes/columns_settings.inc (introduced by commit dfb7af91af6 from Bug 23307) which allows us to create and hook our own custom columnsInit() function in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt to redraw the DataTable filters upon page load if a column is marked as 'is_hidden: 1' in admin/columns_settings.yml, or if a column is added/removed via the "⚙ Columns" button (both are handled by the DataTables column-visibility.dt event). Redrawing the filters via the above method also fixes the issue described in Bug 32448. Test plan: 1) Confirm the erratic DataTable behaviour outlined above 2) Apply this patch and reload all JS assets (hit CTRL-F5) 3) Confirm that you now see the correct input text field for the 'Barcode' column 4) Confirm that you can search for barcodes or in any other column successfully 5) Try toggling the visibility of the columns and making as many search variations as possible -- it should all now work without any glitches! For extra credit ;-) you can also test the 'Other holdings' table by setting the SeparateHoldings SysPref to 'Separate'. Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Followed the test plan (with and without SeparateHoldings). Works perfectly! Signed off.
Created attachment 147729 [details] [review] Bug 32447: Clean up table configuraton settings for holdings table Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 147730 [details] [review] Bug 32447: Fix DataTable filtering when hidden columns are in place The bibliographic record's details page in the Staff interface includes a 'Holdings' table at the bottom with information for each item attached to the record. When activating the filters in this table, there is no input field for the barcode column but just bold text. This broke in v22.11.00, the related commit being 018a981b9b from Bug 29282 where two new hidden columns were added to that table. We can fix this by taking advantage of the existing code in koha-tmpl/intranet-tmpl/prog/en/includes/columns_settings.inc (introduced by commit dfb7af91af6 from Bug 23307) which allows us to create and hook our own custom columnsInit() function in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt to redraw the DataTable filters upon page load if a column is marked as 'is_hidden: 1' in admin/columns_settings.yml, or if a column is added/removed via the "⚙ Columns" button (both are handled by the DataTables column-visibility.dt event). Redrawing the filters via the above method also fixes the issue described in Bug 32448. Test plan: 1) Confirm the erratic DataTable behaviour outlined above 2) Apply this patch and reload all JS assets (hit CTRL-F5) 3) Confirm that you now see the correct input text field for the 'Barcode' column 4) Confirm that you can search for barcodes or in any other column successfully 5) Try toggling the visibility of the columns and making as many search variations as possible -- it should all now work without any glitches! For extra credit ;-) you can also test the 'Other holdings' table by setting the SeparateHoldings SysPref to 'Separate'. Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Pushed to master for 23.05. Nice work everyone, thanks!
Nice work, thanks everyone! Pushed to 22.11.x for the next release.
Doesn't apply cleanly to 22.05.x, if needed please rebase.