[Bug 38484] New: Filters on the "Holds to pull" page is broken
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 Bug ID: 38484 Summary: Filters on the "Holds to pull" page is broken Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Staff interface Assignee: jonathan.druart@gmail.com Reporter: jonathan.druart@gmail.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com Has been broken by bug 36640. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |36640 Keywords| |rel_24_11_candidate, | |RM_priority Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36640 [Bug 36640] Upgrade DataTables from 1.13.6 to 2.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 --- Comment #1 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 174788 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174788&action=edit Bug 38484: (bug 36640 follow-up) Fix filters on "Holds to pull" On "Bug 36640: Remove fnAddFilters" I assume that it was only used for debounce. But actually it was used on this table to add filtering. This patch adjusts how we deal with the filters on this table, reusing the standard "add_filters" and "filters_options" parameters of KohaTable. It also passes the DT object allowing to use DT's api. Here it is useful to retrieve the data in the column. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #174788|0 |1 is obsolete| | --- Comment #2 from Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> --- Created attachment 174806 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174806&action=edit Bug 38484: (bug 36640 follow-up) Fix filters on "Holds to pull" On "Bug 36640: Remove fnAddFilters" I assume that it was only used for debounce. But actually it was used on this table to add filtering. This patch adjusts how we deal with the filters on this table, reusing the standard "add_filters" and "filters_options" parameters of KohaTable. It also passes the DT object allowing to use DT's api. Here it is useful to retrieve the data in the column. Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |emily.lamancusa@montgomeryc | |ountymd.gov -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #3 from David Cook <dcook@prosentient.com.au> --- Would've been good to have a test plan for this, but taking a look now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- With this patch, I'm now getting "Incorrect use of <label for=FORM_ELEMENT>" warnings in the console. Looks like it's probably because the FORM_ELEMENT are converted by Flatpickr into hidden inputs. Not really a blocker... but worth noting... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- The filter boxes are too narrow as well. With this patch, they're smaller and don't have equal distance whitespace between the filter box and the edge of the table cell. Also not a blocker but noting it... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #4)
With this patch, I'm now getting "Incorrect use of <label for=FORM_ELEMENT>" warnings in the console. Looks like it's probably because the FORM_ELEMENT are converted by Flatpickr into hidden inputs.
Not really a blocker... but worth noting...
Actually, that looks pre-existing. Not sure why it didn't come up before the patch, but I unapplied the patch, and it's still there. So you can disregard here... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- Noting that the patch has added more filter boxes. Before patch, there were no boxes for Available collections, Earliest hold date, Hold notes, and Action. Those first three are OK, but we probably don't want one on Action. Still don't think that's a blocker though... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 174806 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174806 Bug 38484: (bug 36640 follow-up) Fix filters on "Holds to pull" Review of attachment 174806: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=38484&attachment=174806) ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt @@ +1,2 @@
[% USE raw %] +[% USE To %]
Does this line get used? I can't see it used anywhere. @@ +321,5 @@
}); }
$(document).ready(function() { + [% SET libraries = Branches.all %]
This line also seems unused? ::: koha-tmpl/intranet-tmpl/prog/js/datatables.js @@ +808,4 @@
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)
I don't know what this line does, but I'm going to assume it's OK. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 --- Comment #9 from David Cook <dcook@prosentient.com.au> --- I'm going to Pass QA since this fixes an important functional problem, and we're close to the release, so I think we should move forward. But a few things to look at there still... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #174806|0 |1 is obsolete| | --- Comment #10 from David Cook <dcook@prosentient.com.au> --- Created attachment 174907 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174907&action=edit Bug 38484: (bug 36640 follow-up) Fix filters on "Holds to pull" On "Bug 36640: Remove fnAddFilters" I assume that it was only used for debounce. But actually it was used on this table to add filtering. This patch adjusts how we deal with the filters on this table, reusing the standard "add_filters" and "filters_options" parameters of KohaTable. It also passes the DT object allowing to use DT's api. Here it is useful to retrieve the data in the column. Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |dcook@prosentient.com.au |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 --- Comment #11 from Jonathan Druart <jonathan.druart@gmail.com> --- This patch makes the filters look like other table's filters. So yes there is certainly a change in the style, but it's expected. (In reply to David Cook from comment #7)
Noting that the patch has added more filter boxes.
Before patch, there were no boxes for Available collections, Earliest hold date, Hold notes, and Action.
Those first three are OK, but we probably don't want one on Action.
Filter on action is not expected, patch coming. (In reply to David Cook from comment #8)
Comment on attachment 174806 [details] [review] Bug 38484: (bug 36640 follow-up) Fix filters on "Holds to pull"
Review of attachment 174806 [details] [review]: -----------------------------------------------------------------
::: koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt @@ +1,2 @@
[% USE raw %] +[% USE To %]
Does this line get used? I can't see it used anywhere.
Unused, will be removed.
@@ +321,5 @@
}); }
$(document).ready(function() { + [% SET libraries = Branches.all %]
This line also seems unused?
Unused, will be removed.
::: koha-tmpl/intranet-tmpl/prog/js/datatables.js @@ +808,4 @@
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)
I don't know what this line does, but I'm going to assume it's OK.
This page is different, we build the dropdown filters with the content of the table, not all the data in DB (what we need to do when using the REST API/ajax pagination). Here we pass the DataTable object so _dt_add_filters needs to loop over the columns of the table and retrieve its data. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 --- Comment #12 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 174908 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174908&action=edit Bug 38484: Remove filters for 'Actions' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 --- Comment #13 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 174909 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174909&action=edit Bug 38484: Remove unused USE and SET And tidy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 --- Comment #14 from Jonathan Druart <jonathan.druart@gmail.com> --- Thanks for the thorough review, David! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_24_11_candidate, | |RM_priority | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.11! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |RESOLVED CC| |lucas@bywatersolutions.com Resolution|--- |FIXED --- Comment #16 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- 24.05.x merge conflcist, rebase if needed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38484 Bug 38484 depends on bug 36640, which changed state. Bug 36640 Summary: Upgrade DataTables from 1.13.6 to 2.x https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36640 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org