Bug 27402

Summary: Add column filtering to the Datatables REST API wrapper
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: TemplatesAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: caroline.cyr-la-rose, fridolin.somers, martin.renvoize, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This development adds native DataTables column filtering to the REST API wrapper. This allows easily adding column filters to DataTables using the REST API. A sample implementation is added to the cities admin page, for reference.
Version(s) released in:
21.05.00
Bug Depends on: 24561, 27353    
Bug Blocks: 27467, 20212, 27465    
Attachments: Bug 27402: Add DT column filtering to the cities admin page
Bug 27402: Move to datatables.js
add fixedHeader
Bug 27402: Column filters must AND, not OR
Bug 27402: Add DT column filtering to the cities admin page
Bug 27402: Move to datatables.js
Bug 27402: Column filters must AND, not OR
Bug 27402: Add DT column filtering to the cities admin page
Bug 27402: Move to datatables.js
Bug 27402: Column filters must AND, not OR
Bug 27402: Reverse search filter texts
Bug 27402: Reverse search filter texts

Description Jonathan Druart 2021-01-12 09:03:50 UTC
Bug 24561 added a wrapper to use easily all the DataTables functionalities when a REST API route was used within Koha.

The filtering method used is working for the global search filter, but not if we have a filter on top of each column.
Comment 1 Jonathan Druart 2021-01-12 09:14:36 UTC
Created attachment 115051 [details] [review]
Bug 27402: Add DT column filtering to the cities admin page

Bug 24561 added a wrapper to use easily all the DataTables functionalities when a REST API route was used within Koha.

The filtering method used is working for the global search filter, but not if we have a filter on top of each column.

This patchset is going to, first, add the filters on top of each column
of the cities table, then the code will be moved to the DT REST API wrapper to make it reusable easily.

Test plan:

Generate some cities:
  use Koha::Cities;
  for ( 1 .. 42 ) {
      Koha::City->new({city_name => "name_$_", city_state => "state_$_", city_country => "country_$_", city_zipcode => "zipcode_$_" })->store;
  }
Hit /admin/cities.pl
Use the filters

The general filter must do a OR query on each of the cities' attributes,
when column filters must use AND
Comment 2 Jonathan Druart 2021-01-12 09:14:39 UTC
Created attachment 115052 [details] [review]
Bug 27402: Move to datatables.js

To make it reusable we need to move the code to datatables.js

So far only the cities table is using the column filters so there is no other
view to test.

Note that the existing implementation didn't work at all, and were base
on what is done on the detail.pl page (using table_filters.js)
Comment 3 Jonathan Druart 2021-01-12 09:21:04 UTC
Created attachment 115053 [details] [review]
add fixedHeader
Comment 4 Jonathan Druart 2021-01-12 09:21:39 UTC
Created attachment 115054 [details] [review]
Bug 27402: Column filters must AND, not OR

Test plan:
Use the column filters and the global filter
Confirm that the rows are filtered as you expect

QA Note: There is more code on top of this, see bug 20212.
Comment 5 Tomás Cohen Arazi 2021-01-26 13:12:56 UTC
I can't say I'm impressed by your skills, Jonathan. But this is super cool.
Comment 6 Tomás Cohen Arazi 2021-01-26 13:13:34 UTC
Created attachment 115813 [details] [review]
Bug 27402: Add DT column filtering to the cities admin page

Bug 24561 added a wrapper to use easily all the DataTables functionalities when a REST API route was used within Koha.

The filtering method used is working for the global search filter, but not if we have a filter on top of each column.

This patchset is going to, first, add the filters on top of each column
of the cities table, then the code will be moved to the DT REST API wrapper to make it reusable easily.

Test plan:

Generate some cities:
  use Koha::Cities;
  for ( 1 .. 42 ) {
      Koha::City->new({city_name => "name_$_", city_state => "state_$_", city_country => "country_$_", city_zipcode => "zipcode_$_" })->store;
  }
Hit /admin/cities.pl
Use the filters

The general filter must do a OR query on each of the cities' attributes,
when column filters must use AND

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Tomás Cohen Arazi 2021-01-26 13:13:38 UTC
Created attachment 115814 [details] [review]
Bug 27402: Move to datatables.js

To make it reusable we need to move the code to datatables.js

So far only the cities table is using the column filters so there is no other
view to test.

Note that the existing implementation didn't work at all, and were base
on what is done on the detail.pl page (using table_filters.js)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Tomás Cohen Arazi 2021-01-26 13:13:42 UTC
Created attachment 115815 [details] [review]
Bug 27402: Column filters must AND, not OR

Test plan:
Use the column filters and the global filter
Confirm that the rows are filtered as you expect

QA Note: There is more code on top of this, see bug 20212.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Tomás Cohen Arazi 2021-01-26 13:14:19 UTC
(In reply to Tomás Cohen Arazi from comment #5)
> I can't say I'm impressed by your skills, Jonathan. But this is super cool.

This tried to mean I'm already used to seeing you do smart things :-D
Comment 10 Martin Renvoize 2021-01-28 14:34:03 UTC
Created attachment 115935 [details] [review]
Bug 27402: Add DT column filtering to the cities admin page

Bug 24561 added a wrapper to use easily all the DataTables functionalities when a REST API route was used within Koha.

The filtering method used is working for the global search filter, but not if we have a filter on top of each column.

This patchset is going to, first, add the filters on top of each column
of the cities table, then the code will be moved to the DT REST API wrapper to make it reusable easily.

Test plan:

Generate some cities:
  use Koha::Cities;
  for ( 1 .. 42 ) {
      Koha::City->new({city_name => "name_$_", city_state => "state_$_", city_country => "country_$_", city_zipcode => "zipcode_$_" })->store;
  }
Hit /admin/cities.pl
Use the filters

The general filter must do a OR query on each of the cities' attributes,
when column filters must use AND

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2021-01-28 14:34:06 UTC
Created attachment 115936 [details] [review]
Bug 27402: Move to datatables.js

To make it reusable we need to move the code to datatables.js

So far only the cities table is using the column filters so there is no other
view to test.

Note that the existing implementation didn't work at all, and were base
on what is done on the detail.pl page (using table_filters.js)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2021-01-28 14:34:09 UTC
Created attachment 115937 [details] [review]
Bug 27402: Column filters must AND, not OR

Test plan:
Use the column filters and the global filter
Confirm that the rows are filtered as you expect

QA Note: There is more code on top of this, see bug 20212.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2021-01-28 14:34:33 UTC
This is really nice, and clearly well thought through.

Passing QA
Comment 14 Jonathan Druart 2021-01-28 15:18:11 UTC
Created attachment 115942 [details] [review]
Bug 27402: Reverse search filter texts
Comment 15 Jonathan Druart 2021-01-28 15:43:16 UTC
Created attachment 115966 [details] [review]
Bug 27402: Reverse search filter texts
Comment 16 Jonathan Druart 2021-01-28 16:34:53 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 17 Fridolin Somers 2021-02-01 13:45:35 UTC
Enhancement not pushed to 20.11.x