Bug 30183 - ILL table search filtering broken
Summary: ILL table search filtering broken
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: master
Hardware: All All
: P3 major (vote)
Assignee: Martin Renvoize
QA Contact:
URL:
Keywords: regression
Depends on: 29240
Blocks:
  Show dependency treegraph
 
Reported: 2022-02-25 15:02 UTC by Andrew Isherwood
Modified: 2022-03-28 14:01 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.05


Attachments
Bug 30183: Fetch flatPickr instance prior to custom search definition (1.84 KB, patch)
2022-03-14 14:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30183: Fetch flatPickr instance prior to custom search definition (2.18 KB, patch)
2022-03-14 17:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30183: Fetch flatPickr instance prior to custom search definition (2.25 KB, patch)
2022-03-18 14:31 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Isherwood 2022-02-25 15:02:28 UTC
To replicate:

- View a table of ILL requests
- Attempt to filter the table by typing in the search box
- Note nothing happens and the console displays:

Uncaught TypeError: Cannot read properties of undefined (reading '0')
    at Array.<anonymous> (ill-list-table_21.1101000.js:501:75)

Looking at the history of ill-list-table.js there looks to be a couple of commits that have moved away from using datepicker which appear to have resulted in the breakage:

https://github.com/PTFS-Europe/koha/commit/b70a9a29d77f4109b082fe6ed57cc44f3c2bae4f#diff-6fe7f0230c90310d4a449891a8179faa0952c9f0d307845fce1de4553e3c9fc0

changed the definition of illfilter_dateplaced_* variables to use Flatpickr, and:

https://github.com/PTFS-Europe/koha/commit/30196fcdf3f5d726ad79c4f8983b17e9ad06b46a#diff-6fe7f0230c90310d4a449891a8179faa0952c9f0d307845fce1de4553e3c9fc0

removed the variable definitions, which appears to be what is resulting in the error
Comment 1 Martin Renvoize 2022-03-14 14:33:53 UTC
Created attachment 131661 [details] [review]
Bug 30183: Fetch flatPickr instance prior to custom search definition

This patch updates teh search customisations in ill-list-table to
properly fetch the existing flatpickr instances for date searches.
Comment 2 Tomás Cohen Arazi 2022-03-14 17:25:56 UTC
Created attachment 131668 [details] [review]
Bug 30183: Fetch flatPickr instance prior to custom search definition

This patch updates teh search customisations in ill-list-table to
properly fetch the existing flatpickr instances for date searches.

To test:
1. Have at least one ILL request on the ILL requests page
2. Open the broser inspector
3. Choose a date on the left hand form
=> FAIL: Errors in the console, search doesn't work
4. Apply this patch
5. Reload
6. Repeat 2-3
=> SUCCESS: No more errors, filtering works!
7. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Jonathan Druart 2022-03-18 14:31:40 UTC
Created attachment 131913 [details] [review]
Bug 30183: Fetch flatPickr instance prior to custom search definition

This patch updates teh search customisations in ill-list-table to
properly fetch the existing flatpickr instances for date searches.

To test:
1. Have at least one ILL request on the ILL requests page
2. Open the broser inspector
3. Choose a date on the left hand form
=> FAIL: Errors in the console, search doesn't work
4. Apply this patch
5. Reload
6. Repeat 2-3
=> SUCCESS: No more errors, filtering works!
7. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 4 Jonathan Druart 2022-03-18 14:32:01 UTC
It works but it's awkward. I was expecting to see $(selector).val() to retrieve the formatted value, then use one of our JS function to get a Date() object.
Comment 5 Fridolin Somers 2022-03-22 20:19:36 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 6 Andreas Jonsson 2022-03-23 08:06:21 UTC
I just fixed this issue for a production system that we recently upgraded to 21.11.03 and was going to report it when I found this bug report.  I think it would be nicer to avoid unnecessarily referencing the flatpickr data directly:

           $.fn.dataTable.ext.search.push(function(settings, data, dataIndex) {
                const d = (s) => s ? new Date(s) : null;
                const t = (value, filter, test) => {
                    da = d(value);
                    if (!da) {
                        return true;
                    }
                    const fv = $('#illfilter_' + filter).val();
                    db = d(fv);
                    if (!db) {
                        return true;
                    }
                    return test(da, db);
                };
                return t(data[14], 'dateplaced_start',   (a, b) => a >= b) &&
                       t(data[14], 'dateplaced_end',     (a, b) => a <= b) &&
                       t(data[16], 'datemodified_start', (a, b) => a >= b) &&
                       t(data[16], 'datemodified_end',   (a, b) => a <= b);
            });
Comment 7 Kyle M Hall 2022-03-25 13:42:30 UTC
Pushed to 21.11.x for 21.11.05