Bugzilla – Attachment 188468 Details for
Bug 38311
DataTables - Simplify the building of the dropdown list filters
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 38311: Do not use ID for column filters
Bug-38311-Do-not-use-ID-for-column-filters.patch (text/plain), 20.45 KB, created by
Owen Leonard
on 2025-10-27 11:00:59 UTC
(
hide
)
Description:
Bug 38311: Do not use ID for column filters
Filename:
MIME Type:
Creator:
Owen Leonard
Created:
2025-10-27 11:00:59 UTC
Size:
20.45 KB
patch
obsolete
>From 6e3d5ea544a38a74a94100f3a618ac5a041f7e68 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Thu, 31 Jul 2025 15:40:40 +0200 >Subject: [PATCH] Bug 38311: Do not use ID for column filters > >Signed-off-by: Owen Leonard <oleonard@myacpl.org> >--- > .../tables/items/catalogue_detail.inc | 29 ++++---- > .../prog/en/includes/patron-search.inc | 71 ++++++++++--------- > .../prog/en/modules/catalogue/itemsearch.tt | 35 +++++---- > .../prog/en/modules/circ/pendingbookings.tt | 5 +- > .../prog/en/modules/circ/pendingreserves.tt | 27 +++++-- > koha-tmpl/intranet-tmpl/prog/js/datatables.js | 17 +++-- > 6 files changed, 101 insertions(+), 83 deletions(-) > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers/tables/items/catalogue_detail.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers/tables/items/catalogue_detail.inc >index 003a1d37803..d74198bdaf0 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers/tables/items/catalogue_detail.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers/tables/items/catalogue_detail.inc >@@ -205,15 +205,13 @@ > }); > }); > >- const all_libraries = [% To.json(Branches.all) | $raw %]; >- const libraries_filters = all_libraries.map(e => { >+ const all_libraries = [% To.json(Branches.all) | $raw %].map(e => { > e["_id"] = e["branchcode"]; > e["_str"] = e["branchname"]; > return e; > }); > const libraries_names = new Map(all_libraries.map( l => [l.branchcode, l.branchname] )); >- const all_item_types = [% To.json(ItemTypes.Get) | $raw %]; >- const item_types_filters = all_item_types.map(e => { >+ const all_item_types = [% To.json(ItemTypes.Get) | $raw %].map(e => { > e["_id"] = e["itemtype"]; > e["_str"] = e["translated_description"]; > return e; >@@ -242,6 +240,13 @@ > location: new Map([% To.json(AuthorisedValues.GetDescriptionsByKohaField({ kohafield => 'items.location' })) | $raw %].map( av => [av.lib, av.authorised_value])), > }; > >+ let filters_options = { >+ item_types: all_item_types, >+ libraries: all_libraries, >+ libraries: all_libraries, >+ statuses: all_statuses, >+ }; >+ > [% IF Koha.Preference('URLLinkText') %] > const url_link_text = "[% Koha.Preference('URLLinkText') | html %]"; > [% ELSE %] >@@ -327,18 +332,6 @@ > return $("#" + tab_id + "_status select").val(); > }; > >- let filters_options = {}; >- >- let itype_col = $("#" + tab_id + "_table thead th[data-colname='itype']").index(); >- let holdingbranch_col = $("#" + tab_id + "_table thead th[data-colname='holdingbranch']").index(); >- let homebranch_col = $("#" + tab_id + "_table thead th[data-colname='homebranch']").index(); >- let status_col = $("#" + tab_id + "_table thead th[data-colname='status']").index(); >- >- if ( itype_col >= 0 ) filters_options[itype_col] = () => all_item_types; >- if ( holdingbranch_col >= 0 ) filters_options[holdingbranch_col] = () => all_libraries; >- if ( homebranch_col >= 0 ) filters_options[homebranch_col] = () => all_libraries; >- if ( status_col >= 0 ) filters_options[status_col] = () => all_statuses; >- > var items_table = $("#" + tab_id + '_table').kohaTable({ > ajax: { url: item_table_url }, > order: [], >@@ -391,6 +384,7 @@ > { > data: "me.item_type_id", // FIXME Cannot filter by biblioitem.itemtype > datatype: "coded_value:item_type", >+ dataFilter: "item_types", > className: "itype", > searchable: true, > orderable: true, >@@ -411,6 +405,7 @@ > { > data: "me.holding_library_id", > datatype: "coded_value:library", >+ dataFilter: "libraries", > className: "location", > searchable: true, > orderable: true, >@@ -421,6 +416,7 @@ > { > data: "me.home_library_id", > datatype: "coded_value:library", >+ dataFilter: "libraries", > className: "homebranch", > searchable: true, > orderable: true, >@@ -514,6 +510,7 @@ > { > data: "", > className: "status", >+ dataFilter: "statuses", > searchable: false, > orderable: false, > render: function (data, type, row, meta) { >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc >index a4f76e8e116..a5ef1af0d2a 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc >@@ -175,9 +175,9 @@ > [% CASE 'address-library' %] > <th>Address</th> > [% CASE 'branch' %] >- <th data-filter="libraries">Library</th> >+ <th>Library</th> > [% CASE 'category' %] >- <th data-filter="categories">Category</th> >+ <th>Category</th> > [% CASE 'dateexpiry' %] > <th>Expires on</th> > [% CASE 'borrowernotes' %] >@@ -189,17 +189,9 @@ > [% CASE 'account_balance' %] > <th>Fines</th> > [% CASE 'sort1' %] >- [% IF av_bsort1.size %] >- <th data-filter="av_bsort1">Sort 1</th> >- [% ELSE %] >- <th>Sort 1</th> >- [% END %] >+ <th>Sort 1</th> > [% CASE 'sort2' %] >- [% IF av_bsort2.size %] >- <th data-filter="av_bsort2">Sort 2</th> >- [% ELSE %] >- <th>Sort 2</th> >- [% END %] >+ <th>Sort 2</th> > [% CASE 'action' %] > <th class="no-export"> </th> > [% END %] >@@ -244,9 +236,7 @@ > </script> > [% END %] > <script> >- [% SET libraries = Branches.all %] >- [% SET categories = Categories.all.unblessed %] >- var categories = [% To.json(categories) | $raw %].map(e => { >+ var categories = [% To.json(Categories.all.unblessed) | $raw %].map(e => { > e['_id'] = e.categorycode.toLowerCase(); > e['_str'] = e.description; > return e; >@@ -255,7 +245,7 @@ > map[e._id] = e; > return map; > }, {}); >- var libraries = [% To.json(libraries) | $raw %].map(e => { >+ var libraries = [% To.json(Branches.all) | $raw %].map(e => { > e['_id'] = e.branchcode; > e['_str'] = e.branchname; > return e; >@@ -285,6 +275,13 @@ > return map; > }, {}); > >+ let filters_options = { >+ libraries, >+ categories, >+ av_bsort1, >+ av_bsort2, >+ }; >+ > [% IF Koha.Preference('ExtendedPatronAttributes') %] > [% SET extended_attribute_types = ExtendedAttributeTypes.codes( staff_searchable => 1, searched_by_default => 1 ) %] > [% IF ( extended_attribute_types ) %][% extended_attribute_types = [ extended_attribute_types ] %][% END %] >@@ -627,10 +624,11 @@ > } > [% CASE 'branch' %] > { >- "data": "library.name:me.library_id", >- "searchable": true, >- "orderable": true, >- "render": function( data, type, row, meta ) { >+ data: "library.name:me.library_id", >+ dataFilter: "libraries", >+ searchable: true, >+ orderable: true, >+ render: function( data, type, row, meta ) { > if( !singleBranchMode && row.library.library_id == logged_in_library_id ) { > return "<span class=\"currentlibrary\">" + escape_str(row.library.name) + "</span>"; > } else { >@@ -640,10 +638,11 @@ > } > [% CASE 'category' %] > { >- "data": "category_id", >- "searchable": true, >- "orderable": true, >- "render": function( data, type, row, meta ) { >+ data: "category_id", >+ dataFilter: "categories", >+ searchable: true, >+ orderable: true, >+ render: function( data, type, row, meta ) { > return escape_str(categories_map[data.toLowerCase()].description); > } > } >@@ -711,10 +710,13 @@ > > [% CASE 'sort1' %] > { >- "data": "statistics_1", >- "searchable": true, >- "orderable": true, >- "render": function( data, type, row, meta ) { >+ data: "statistics_1", >+ [% IF av_bsort1.size %] >+ dataFilter: "av_bsort1", >+ [% END %] >+ searchable: true, >+ orderable: true, >+ render: function( data, type, row, meta ) { > if (data === null) return ''; > let bsort1 = av_bsort1_map[data.toString()]; > return escape_str(bsort1 ? bsort1.lib : data); >@@ -723,10 +725,13 @@ > > [% CASE 'sort2' %] > { >- "data": "statistics_2", >- "searchable": true, >- "orderable": true, >- "render": function( data, type, row, meta ) { >+ data: "statistics_2", >+ [% IF av_bsort2.size %] >+ dataFilter: "av_bsort2", >+ [% END %] >+ searchable: true, >+ orderable: true, >+ render: function( data, type, row, meta ) { > if (data === null) return ''; > let bsort2 = av_bsort2_map[data.toString()]; > return escape_str(bsort2 ? bsort2.lib : data); >@@ -783,7 +788,7 @@ > }, > [% END %] > fixedHeader: false, >- }, typeof table_settings !== 'undefined' ? table_settings : null, 1, additional_filters, undefined, external_filter_nodes, parent_block.find(".search_description"), additional_search_descriptions ); >+ }, typeof table_settings !== 'undefined' ? table_settings : null, 1, additional_filters, filters_options, external_filter_nodes, parent_block.find(".search_description"), additional_search_descriptions ); > > patron_search_form.on('submit', filter); > patron_search_form.on('submit', update_search_type); >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt >index ad2f23ab244..7129b4eeb01 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt >@@ -656,15 +656,14 @@ > } ); > > let filters_options = { >- 4: () => all_ccodes, >- 9: () => all_libraries, >- 10: () => all_libraries, >- 11: () => all_locations, >- 12: () => all_item_types, >- 14: () => all_notforloans, >- 15: () => all_losts, >- 16: () => all_withdrawns, >- 17: () => all_damageds, >+ collection_codes: all_ccodes, >+ libraries: all_libraries, >+ locations: all_locations, >+ item_types: all_item_types, >+ av_notforloan: all_notforloans, >+ av_lost: all_losts, >+ av_withdrawn: all_withdrawns, >+ av_damaged: all_damageds, > }; > > var table_settings = [% TablesSettings.GetTableSettings( 'catalogue', 'itemsearch', 'results', 'json' ) | $raw %]; >@@ -696,20 +695,20 @@ > { 'name': 'title' }, > { 'name': 'publicationyear' }, > { 'name': 'publishercode' }, >- { 'name': 'ccode' }, >+ { 'name': 'ccode', dataFilter: 'collection_codes' }, > { 'name': 'barcode' }, > { 'name': 'itemnumber' }, > { 'name': 'enumchron' }, > { 'name': 'itemcallnumber' }, >- { 'name': 'homebranch' }, >- { 'name': 'holdingbranch' }, >- { 'name': 'location' }, >- { 'name': 'itype'}, >+ { 'name': 'homebranch', dataFilter: 'libraries' }, >+ { 'name': 'holdingbranch', dataFilter: 'libraries' }, >+ { 'name': 'location', dataFilter: 'locations' }, >+ { 'name': 'itype', dataFilter: 'item_types' }, > { 'name': 'stocknumber' }, >- { 'name': 'notforloan' }, >- { 'name': 'itemlost' }, >- { 'name': 'withdrawn' }, >- { 'name': 'damaged' }, >+ { 'name': 'notforloan', dataFilter: 'av_notforloan' }, >+ { 'name': 'itemlost', dataFilter: 'av_lost' }, >+ { 'name': 'withdrawn', dataFilter: 'av_widthdrawn' }, >+ { 'name': 'damaged', dataFilter: 'av_damaged' }, > { 'name': 'dateaccessioned' }, > { 'name': 'issues' }, > { 'name': 'datelastborrowed' }, >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingbookings.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingbookings.tt >index 12373098013..21cc22086f0 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingbookings.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingbookings.tt >@@ -154,14 +154,13 @@ > } > }; > >- [% SET libraries = Branches.all %] >- let all_libraries = [% To.json(libraries) | $raw %].map(e => { >+ let all_libraries = [% To.json(Branches.all) | $raw %].map(e => { > e['_id'] = e.branchcode; > e['_str'] = e.branchname; > return e; > }); > let filters_options = { >- [1] : () => all_libraries, >+ libraries: all_libraries, > }; > > var bookings_table_url = '/api/v1/bookings?'; >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt >index 9ee928f73f3..2b74dbe86c7 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt >@@ -316,10 +316,10 @@ > > $(document).ready(function() { > let filters_options = { >- [5] : (table_dt) => get_options(table_dt.column(5)), >- [10] : (table_dt) => get_options(table_dt.column(10)), >- [11] : (table_dt) => get_options(table_dt.column(11)), >- [15] : (table_dt) => get_options(table_dt.column(15)), >+ libraries: (table_dt) => get_options(table_dt.column(5)), >+ item_types: (table_dt) => get_options(table_dt.column(10)), >+ locations: (table_dt) => get_options(table_dt.column(11)), >+ pickup_locations : (table_dt) => get_options(table_dt.column(15)), > }; > > >@@ -327,6 +327,25 @@ > var holdst = $("#holdst").kohaTable( > { > pagingType: "full_numbers", >+ columns: [ >+ {name: "pull_items" }, >+ {name: "available_items" }, >+ {name: "patrons" }, >+ {name: "patron" }, >+ {name: "title" }, >+ {name: "libraries", dataFilter: "libraries" }, >+ {name: "barcodes" }, >+ {name: "call_numbers" }, >+ {name: "copy_numbers" }, >+ {name: "enumeration" }, >+ {name: "itemtypes", dataFilter: "item_types" }, >+ {name: "locations", dataFilter: "locations" }, >+ {name: "collection" }, >+ {name: "hold_date" }, >+ {name: "reserve_nodes" }, >+ {name: "pickup_location", dataFilter: "pickup_locations" }, >+ {name: "action" }, >+ ], > }, > table_settings, > true, >diff --git a/koha-tmpl/intranet-tmpl/prog/js/datatables.js b/koha-tmpl/intranet-tmpl/prog/js/datatables.js >index a598464fc5c..2a958ef20c3 100644 >--- a/koha-tmpl/intranet-tmpl/prog/js/datatables.js >+++ b/koha-tmpl/intranet-tmpl/prog/js/datatables.js >@@ -1049,29 +1049,28 @@ function _dt_add_filters(table_node, table_dt, filters_options = {}) { > let th = $(table_node).find( > "thead tr:eq(1) th:eq(%s)".format(visible_i) > ); >- var is_searchable = table_dt.settings()[0].aoColumns[i].bSearchable; >+ let col = table_dt.settings()[0].aoColumns[i]; >+ var is_searchable = col.bSearchable; > $(this) > .removeClass("sorting") > .removeClass("sorting_asc") > .removeClass("sorting_desc"); > $(this).data("th-id", i); >- if (is_searchable || $(this).data("filter") || filters_options[i]) { >+ if (is_searchable || col.dataFilter) { > let input_type = "input"; > let existing_search = column.search(); >- if ($(th).data("filter") || filters_options.hasOwnProperty(i)) { >+ if (col.dataFilter) { > input_type = "select"; > let filter_type = $(th).data("filter"); > let select = $( > '<select class="dt-select-filter"><option value=""></option></select' > ); > >- // FIXME eval here is bad and dangerous, how do we workaround that? >- if (!filters_options.hasOwnProperty(i)) { >- filters_options[i] = eval(filter_type); >- } else if (typeof filters_options[i] === "function") { >- filters_options[i] = filters_options[i](table_dt); >+ if (typeof filters_options[col.dataFilter] === "function") { >+ filters_options[col.dataFilter] = >+ filters_options[col.dataFilter](table_dt); > } >- $(filters_options[i]) >+ $(filters_options[col.dataFilter]) > .filter(function () { > return this._id !== "" && this._str !== ""; > }) >-- >2.39.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 38311
:
184926
|
184927
|
186209
|
187376
|
187377
|
188314
| 188468 |
188469