Bugzilla – Attachment 184926 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.40 KB, created by
Jonathan Druart
on 2025-07-31 13:48:03 UTC
(
hide
)
Description:
Bug 38311: Do not use ID for column filters
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2025-07-31 13:48:03 UTC
Size:
20.40 KB
patch
obsolete
>From 76353b2e885a081262550a86684d583927b49cb6 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 > >--- > .../tables/items/catalogue_detail.inc | 26 +++---- > .../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 | 19 +++-- > 6 files changed, 102 insertions(+), 81 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 fdbfec2cef3..04509626bb9 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; >@@ -241,6 +239,13 @@ > collection_code: new Map([% To.json(AuthorisedValues.GetDescriptionsByKohaField({ kohafield => 'items.ccode' })) | $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 %] >@@ -326,15 +331,6 @@ > return $("#" + tab_id + "_status select").val(); > }; > >- let offset = 2; >- [% UNLESS Koha.Preference('LocalCoverImages') %]offset--;[% END %] >- let filters_options = { >- [offset] : () => all_item_types, >- [offset+1] : () => all_libraries, >- [offset+2] : () => all_libraries, >- [offset+7] : () => all_statuses, >- }; >- > var items_table = $("#" + tab_id + '_table').kohaTable({ > ajax: { url: item_table_url }, > order: [], >@@ -387,6 +383,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, >@@ -407,6 +404,7 @@ > { > data: "me.holding_library_id", > datatype: "coded_value:library", >+ dataFilter: "libraries", > className: "location", > searchable: true, > orderable: true, >@@ -417,6 +415,7 @@ > { > data: "me.home_library_id", > datatype: "coded_value:library", >+ dataFilter: "libraries", > className: "homebranch", > searchable: true, > orderable: true, >@@ -507,6 +506,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 85184bdc6aa..a53b8653c0d 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 8cdc7597911..1079e6183c0 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt >@@ -632,15 +632,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 %]; >@@ -672,20 +671,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 af828b6ec63..e837080674f 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt >@@ -310,10 +310,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)), > }; > > >@@ -321,6 +321,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 faef829049b..60e52e31f2f 100644 >--- a/koha-tmpl/intranet-tmpl/prog/js/datatables.js >+++ b/koha-tmpl/intranet-tmpl/prog/js/datatables.js >@@ -1035,29 +1035,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 !== ""; > }) >@@ -1066,7 +1065,7 @@ function _dt_add_filters(table_node, table_dt, filters_options = {}) { > > if ( > table_dt.settings()[0].ajax !== null && >- table_node.attr("id") !== "item_search" >+ $(table_node).attr("id") !== "item_search" > ) { > optionValue = `^${this._id}$`; > } >-- >2.34.1
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