Bug 38071 - "Clear filter" on catalogue details page always disabled
Summary: "Clear filter" on catalogue details page always disabled
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 33568
Blocks: 38108 38110
  Show dependency treegraph
 
Reported: 2024-10-03 12:22 UTC by Nick Clemens (kidclamp)
Modified: 2024-11-15 16:17 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00, 24.05.06
Circulation function:


Attachments
Bug 38071: Preparation (2.21 KB, patch)
2024-10-04 12:43 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 38071: Apply kohaTableToggleClearFilter logic to staff holdings table (1.07 KB, patch)
2024-10-04 12:43 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 38071: Apply kohaTableToggleClearFilter logic to patrons table (1.98 KB, patch)
2024-10-04 12:43 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 38071: Add behavior to work with filters (1.69 KB, patch)
2024-10-04 12:43 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 38071: Make it so that 'clear filter' also clears the filter values (2.05 KB, patch)
2024-10-04 12:43 UTC, Pedro Amorim
Details | Diff | Splinter Review
[ALTERNATIVE PATCH] Bug 38071: Ensure search.dt is triggered (2.18 KB, patch)
2024-10-07 08:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38071: Ensure search.dt is triggered (2.24 KB, patch)
2024-10-07 09:01 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 38071: Ensure search.dt is triggered (2.30 KB, patch)
2024-10-07 16:16 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2024-10-03 12:22:20 UTC
To recreate:
1 - Browse to details record for a biblio with items
2 - Click 'Show filters'
3 - Select/type in some filters
4 - Note 'Clear filters' is grayed out
5 - Try to click it, no response
Comment 1 Pedro Amorim 2024-10-04 12:43:03 UTC
Created attachment 172390 [details] [review]
Bug 38071: Preparation

Move kohaTable's toggleable clear filter button to its own function
Comment 2 Pedro Amorim 2024-10-04 12:43:05 UTC
Created attachment 172391 [details] [review]
Bug 38071: Apply kohaTableToggleClearFilter logic to staff holdings table
Comment 3 Pedro Amorim 2024-10-04 12:43:08 UTC
Created attachment 172392 [details] [review]
Bug 38071: Apply kohaTableToggleClearFilter logic to patrons table

Same reason as the holdings table, this table is initialized at a later
time
Comment 4 Pedro Amorim 2024-10-04 12:43:11 UTC
Created attachment 172393 [details] [review]
Bug 38071: Add behavior to work with filters

Consider column filters values when deciding if the 'clear filter'
should be enabled or not.
Comment 5 Pedro Amorim 2024-10-04 12:43:13 UTC
Created attachment 172394 [details] [review]
Bug 38071: Make it so that 'clear filter' also clears the filter values

1 - Browse to details record for a biblio with items
2 - Click 'Show filters'
3 - Select/type in some filters
4 - Note 'Clear filters' is grayed out
5 - Try to click it, no response
6 - Apply patches. Repeat (make sure to use the search text input as well as the column filters.
7 - Repeat the same test plan but for the patrons table
8 - Verify all is working as intended
Comment 6 David Nind 2024-10-06 21:45:36 UTC
I had a go at testing:

1. Everything seems to be working correctly for the patrons area.

2. For the holdings table, I don't think it is working as expected - maybe these should be other bugs to fix them up:
   2.1 The standard DataTable header is not shown for the holdings table until "Show filters" is selected
      (that is:
      Showing 1 to 4 of 4 entries
      -------------------------------------
      Show 20 entries First Previous1 Next Last Search: _______ Clear filter
      ....)
   2.2 While the "Clear filter" action is no longer greyed out (when used) and now works, actually entering something to filter on no longer seems to work as expected (I'm not sure it worked before the patch either). For example record 162, Intermediate Perl, search for "Midway" results in "No matching records found".
   2.3 Dates in the filters (either the DataTables search or in the table column filter) no longer seem to work. For example: start typing 09/04 for either option
Comment 7 Pedro Amorim 2024-10-07 08:41:27 UTC
(In reply to David Nind from comment #6)
> I had a go at testing:
> 
> 1. Everything seems to be working correctly for the patrons area.
> 
> 2. For the holdings table, I don't think it is working as expected - maybe
> these should be other bugs to fix them up:
>    2.1 The standard DataTable header is not shown for the holdings table
> until "Show filters" is selected
>       (that is:
>       Showing 1 to 4 of 4 entries
>       -------------------------------------
>       Show 20 entries First Previous1 Next Last Search: _______ Clear filter
>       ....)
>    2.2 While the "Clear filter" action is no longer greyed out (when used)
> and now works, actually entering something to filter on no longer seems to
> work as expected (I'm not sure it worked before the patch either). For
> example record 162, Intermediate Perl, search for "Midway" results in "No
> matching records found".
>    2.3 Dates in the filters (either the DataTables search or in the table
> column filter) no longer seem to work. For example: start typing 09/04 for
> either option

Thank you David, I'm convinced these are all different bugs.

2.1) This is the case before the patches. I don't remember why but I think it was decided to group all of that on the "Show/Hide filters". Not caused by these patches.
2.2) I believe this is because "midway" does not exist on the holdings data. Midway's code is "MPL". Search for "MPL", you'll get Midway. Definitely a separate issue.
2.3) This is a date display problem I think. Not related here.
The date displayed is "09/04/2024" but the data contains "2014-09-04". If you search for the latter, you get the results for the former.
Comment 8 Jonathan Druart 2024-10-07 08:51:04 UTC
Created attachment 172444 [details] [review]
[ALTERNATIVE PATCH] Bug 38071: Ensure search.dt is triggered

I don't really understand why the previous code is not working, but
the search.dt event handler is not correctly set when initComplete is
passed to the constructor (for instance from the patrons search)
Comment 9 Pedro Amorim 2024-10-07 09:01:31 UTC
Created attachment 172446 [details] [review]
Bug 38071: Ensure search.dt is triggered

I don't really understand why the previous code is not working, but
the search.dt event handler is not correctly set when initComplete is
passed to the constructor (for instance from the patrons search)

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 10 Pedro Amorim 2024-10-07 09:02:51 UTC
Joubu's patch is much better than mine. Fixes the issue the correct way and does not go beyond the scope of this bug.
Comment 11 Jonathan Druart 2024-10-07 09:13:00 UTC
(In reply to Nick Clemens (kidclamp) from comment #0)
> To recreate:
> 1 - Browse to details record for a biblio with items
> 2 - Click 'Show filters'
> 3 - Select/type in some filters
> 4 - Note 'Clear filters' is grayed out
> 5 - Try to click it, no response

I think this is a different "bug" that was the title describes.
I am not sure what you are describing is a bug, it has always worked like that. The "Clear filter" button is not plural, and its purpose is to clean the "global search". If we want it to reset the whole table, it should be on its own bug. But that's not an easy one: we have tables where filters are outside of the table (like ERM's agreements).
We could have another "reset all" button, eventually.
Comment 12 Martin Renvoize (ashimema) 2024-10-07 16:16:03 UTC
Created attachment 172494 [details] [review]
Bug 38071: Ensure search.dt is triggered

I don't really understand why the previous code is not working, but
the search.dt event handler is not correctly set when initComplete is
passed to the constructor (for instance from the patrons search)

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 David Nind 2024-10-07 20:26:09 UTC
I've created two "related" bugs - see comment #6 and #7:

- Bug 38108 - Staff interface holdings table - table navigation only shows when "Show filters" is selected

- Bug 38110 - Staff interface tables with filters - date format needs to be YYYY-MM-DD, need to use authorized value codes
Comment 14 Katrin Fischer 2024-10-11 09:15:57 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 15 Lucas Gass (lukeg) 2024-11-15 16:17:38 UTC
Backported to 24.05.x for upcoming 24.05.06