[Bug 42915] New: Filtering Holds table by patron name or pickup library is broken
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Bug ID: 42915 Summary: Filtering Holds table by patron name or pickup library is broken Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: blawlor@clamsnet.org QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, lisette@bywatersolutions.com, tomascohen@gmail.com Target Milestone: --- To replicate: 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a patron name or pickup location 3. See 'No matching records found' 4. Note: filtering by patron barcode still works This regression seems like it was introduced by bug 23269 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |23269 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23269 [Bug 23269] Long hold queues are slowing the service -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |regression -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Depends on| |42357 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42357 [Bug 42357] Holds table is missing patron name -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Sponsorship status|--- |Sponsored Assignee|koha-bugs@lists.koha-commun |blawlor@clamsnet.org |ity.org | Status|NEW |Needs Signoff Patch complexity|--- |Small patch Comma delimited| |CLAMS list of Sponsors| | --- Comment #1 from Brendan Lawlor <blawlor@clamsnet.org> --- I think this is the right approach, but it touches datatables.js so it could possibly have side effects in other tables. Please test filtering in other tables too. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #2 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 201099 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201099&action=edit Bug 42915: Fix holds table filtering by patron name and cardnumber This patch adds a new searchData attribute to the patron column definition for the holds table. The data attribute needs to be the cardnumber for the API to work, so this allows searching on multiple fields. It modifies datatables.js build_query() function to use searchData if it exists or else it falls back to using the data attribute. To test: 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a patron name or pickup location 3. See 'No matching records found' 4. Apply patch and repeat step 2 5. Filtering by patron name works! 6. Confirm filtering by cardnumber still works too Sponsored-by: CLAMS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #3 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 201100 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201100&action=edit Bug 42915: Fix holds table filtering by pickup library name and branchcode This patch adds a searchValue attribute to the column definition for pickup library. The API needs the data attribute to be the library id, so this allows filtering by library name or branchcode. 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a pickup library name or branchcode 3. See 'No matching records found' 4. Apply patch and repeat step 2 5. Filtering works! 6. Note: if two libraries share the same beginning text you get no results :( For example: Fairview and Fairfield, you need 5 characters Sponsored-by: CLAMS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201099|0 |1 is obsolete| | Attachment #201100|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #4 from David Nind <david@davidnind.com> --- Created attachment 201116 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201116&action=edit Bug 42915: Fix holds table filtering by patron name and cardnumber This patch adds a new searchData attribute to the patron column definition for the holds table. The data attribute needs to be the cardnumber for the API to work, so this allows searching on multiple fields. It modifies datatables.js build_query() function to use searchData if it exists or else it falls back to using the data attribute. To test: 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a patron name or pickup location 3. See 'No matching records found' 4. Apply patch and repeat step 2 5. Filtering by patron name works! 6. Confirm filtering by cardnumber still works too Sponsored-by: CLAMS Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 201117 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201117&action=edit Bug 42915: Fix holds table filtering by pickup library name and branchcode This patch adds a searchValue attribute to the column definition for pickup library. The API needs the data attribute to be the library id, so this allows filtering by library name or branchcode. 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a pickup library name or branchcode 3. See 'No matching records found' 4. Apply patch and repeat step 2 5. Filtering works! 6. Note: if two libraries share the same beginning text you get no results :( For example: Fairview and Fairfield, you need 5 characters Sponsored-by: CLAMS Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@gmail.com --- Comment #6 from Jonathan Druart <jonathan.druart@gmail.com> --- You should not need to update datatables.js, everything is covered already. Why do you use searchData? Adjusting "data" should be enough. For the libraries, have a look at members/holdshistory.tt: datatype: "coded_value:library", dataFilter: "libraries", Let me know if you have any questions! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #7 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 201298 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201298&action=edit Bug 42915: Use coded value for library filtering Patch from commit da36b26 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201298|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #8 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 201299 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201299&action=edit Bug 42915: Use coded value for library filtering Patch from commit be9d6af -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #9 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 201319 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201319&action=edit Bug 42915: Use coded value for library filtering in holds table This patch adds coded_values for libraries to the holds table and sets the data type in the pickup library column definition. This enables searching holds by pickup location. 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a pickup library name or branchcode 3. See 'No matching records found' 4. Apply patch and repeat step 2 5. Filtering works! 6. Note: if two libraries share the same beginning text both results are returned :) For example: 'Fair' returns holds for pickup at Fairview and Fairfield Sponsored-by: CLAMS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #10 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 201320 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201320&action=edit Bug 42915: Add patron names to data in column def in holds table This patch adds the patron firstname and surname to the searchable data in the patron column definition for the holds table and removes unnecessary code from the render function. This enables searching holds by patron names. To test: 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a patron name 3. See 'No matching records found' 4. Apply patch and repeat step 2 5. Filtering by patron name works! 6. Confirm filtering by cardnumber still works too 7. Set system preference HidePatronName to 'Don't show' 8. Confirm searching by patron names continues to work :) Sponsored-by: CLAMS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201116|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201117|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201299|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #11 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- (In reply to Brendan Lawlor from comment #10)
7. Set system preference HidePatronName to 'Don't show' 8. Confirm searching by patron names continues to work :)
I don't think this is the behavior we want. In testing I made a hold for Henry and a hold for Ronnie. When I set HidePatronName to 'Don't show,' a search for "Henry" on the holds table still filters the list down to only Henry's hold, even though I can no longer see his name. For a staff member that has access to the holds list but does not have access Henry's patron record, we've just revealed a name they should not have access to (albeit in a weird roundabout way). This is pretty niche and doesn't feel highly insecure, so I'm certainly open to being overruled. Other than this issue, everything worked as expected. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42984 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42984 [Bug 42984] Filtering by date, expiration date, and details on biblio holds table -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201319|0 |1 is obsolete| | Attachment #201320|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #12 from Brendan Lawlor <blawlor@clamsnet.org> --- (In reply to Andrew Fuerste-Henry from comment #11)
(In reply to Brendan Lawlor from comment #10)
7. Set system preference HidePatronName to 'Don't show' 8. Confirm searching by patron names continues to work :)
I don't think this is the behavior we want.
Thanks for testing and clarification on the expected behavior. I fixed this by conditionally setting data like this: data: HidePatronName ? "patron.cardnumber" : "patron.surname:patron.firstname:patron.cardnumber", I'm having trouble attaching the patch to bugzilla right now, but I will try again later -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #13 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 201435 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201435&action=edit Bug 42915: Use coded value for library filtering in holds table This patch adds coded_values for libraries to the holds table and sets the data type in the pickup library column definition. This enables searching holds by pickup location. 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a pickup library name or branchcode 3. See 'No matching records found' 4. Apply patch and repeat step 2 5. Filtering works! 6. Note: if two libraries share the same beginning text both results are returned :) For example: 'Fair' returns holds for pickup at Fairview and Fairfield Sponsored-by: CLAMS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #14 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 201436 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201436&action=edit Bug 42915: Add patron names to data in column def in holds table This patch adds the patron firstname and surname to the searchable data in the patron column definition for the holds table and removes unnecessary code from the render function. This enables searching holds by patron names. To test: 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a patron name 3. See 'No matching records found' 4. Apply patch and repeat step 2 5. Filtering by patron name works! 6. Confirm filtering by cardnumber still works too 7. Set system preference HidePatronName to 'Don't show' 8. Confirm searching by patron names doesn't work ;) Sponsored-by: CLAMS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201435|0 |1 is obsolete| | Attachment #201436|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #15 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 201445 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201445&action=edit Bug 42915: Use coded value for library filtering in holds table This patch adds coded_values for libraries to the holds table and sets the data type in the pickup library column definition. This enables searching holds by pickup location. 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a pickup library name or branchcode 3. See 'No matching records found' 4. Apply patch and repeat step 2 5. Filtering works! 6. Note: if two libraries share the same beginning text both results are returned :) For example: 'Fair' returns holds for pickup at Fairview and Fairfield Sponsored-by: CLAMS Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #16 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 201446 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201446&action=edit Bug 42915: Add patron names to data in column def in holds table This patch adds the patron firstname and surname to the searchable data in the patron column definition for the holds table and removes unnecessary code from the render function. This enables searching holds by patron names. To test: 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a patron name 3. See 'No matching records found' 4. Apply patch and repeat step 2 5. Filtering by patron name works! 6. Confirm filtering by cardnumber still works too 7. Set system preference HidePatronName to 'Don't show' 8. Confirm searching by patron names doesn't work ;) Sponsored-by: CLAMS Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #17 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Brendan Lawlor from comment #12)
(In reply to Andrew Fuerste-Henry from comment #11)
(In reply to Brendan Lawlor from comment #10)
7. Set system preference HidePatronName to 'Don't show' 8. Confirm searching by patron names continues to work :)
I don't think this is the behavior we want.
Thanks for testing and clarification on the expected behavior.
I fixed this by conditionally setting data like this: data: HidePatronName ? "patron.cardnumber" : "patron.surname:patron.firstname:patron.cardnumber",
I'm having trouble attaching the patch to bugzilla right now, but I will try again later
This is not correct. If it's considered a privacy issue (what it is) we should not deal with the restriction in client-side. Tomas dealt with that on bug 42503. I am not sure we should deal with that on this bug. I would remove the JS test and fix the endpoint on a separate bug. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #18 from Jonathan Druart <jonathan.druart@gmail.com> --- [FAIL] koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc SKIP filters FAIL valid_template : filter not found Missing [% USE raw %] -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Kimberly <kimberly.diebolt@edcgov.us> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kimberly.diebolt@edcgov.us -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #21 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 201875 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201875&action=edit Bug 42915: Add patron names to data in column def in holds table This patch adds the patron firstname and surname to the searchable data in the patron column definition for the holds table and removes unnecessary code from the render function. This enables searching holds by patron names. To test: 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a patron name 3. See 'No matching records found' 4. Apply patch and repeat step 2 5. Filtering by patron name works! 6. Confirm filtering by cardnumber still works too 7. Set system preference HidePatronName to 'Don't show' 8. Confirm searching by patron names doesn't work ;) Sponsored-by: CLAMS Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #22 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 201876 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201876&action=edit Bug 42915: (follow-up) Fix holds table display on patron record holds tab To test: 1. Have a patron with multiple holds 2. Go to the patron record's hold tab 3. Confirm the table loads and no errors are logged to the console Sponsored-by: CLAMS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201445|0 |1 is obsolete| | Attachment #201446|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #19 from Brendan Lawlor <blawlor@clamsnet.org> --- Thanks, Joubu! I added the missing filter and also fixed an issue with the table not loading on the patron record holds tab. I also made sure to test the holds to pull and holds awaiting pickup tables. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #20 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 201874 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201874&action=edit Bug 42915: Use coded value for library filtering in holds table This patch adds coded_values for libraries to the holds table and sets the data type in the pickup library column definition. This enables searching holds by pickup location. 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a pickup library name or branchcode 3. See 'No matching records found' 4. Apply patch and repeat step 2 5. Filtering works! 6. Note: if two libraries share the same beginning text both results are returned :) For example: 'Fair' returns holds for pickup at Fairview and Fairfield Sponsored-by: CLAMS Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> 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=42915 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201874|0 |1 is obsolete| | Attachment #201875|0 |1 is obsolete| | Attachment #201876|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #23 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201917 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201917&action=edit Bug 42915: Use coded value for library filtering in holds table This patch adds coded_values for libraries to the holds table and sets the data type in the pickup library column definition. This enables searching holds by pickup location. 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a pickup library name or branchcode 3. See 'No matching records found' 4. Apply patch and repeat step 2 5. Filtering works! 6. Note: if two libraries share the same beginning text both results are returned :) For example: 'Fair' returns holds for pickup at Fairview and Fairfield Sponsored-by: CLAMS Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #24 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201918 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201918&action=edit Bug 42915: Add patron names to data in column def in holds table This patch adds the patron firstname and surname to the searchable data in the patron column definition for the holds table and removes unnecessary code from the render function. This enables searching holds by patron names. To test: 1. Have holds on a bib for different patrons and pickup locations 2. Try filtering by a patron name 3. See 'No matching records found' 4. Apply patch and repeat step 2 5. Filtering by patron name works! 6. Confirm filtering by cardnumber still works too 7. Set system preference HidePatronName to 'Don't show' 8. Confirm searching by patron names doesn't work ;) Sponsored-by: CLAMS Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #25 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201919 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201919&action=edit Bug 42915: (follow-up) Fix holds table display on patron record holds tab To test: 1. Have a patron with multiple holds 2. Go to the patron record's hold tab 3. Confirm the table loads and no errors are logged to the console Sponsored-by: CLAMS Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #26 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201920 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201920&action=edit Bug 42915: (QA follow-up) Declare all_libraries once, unconditionally, in request.tt holds_table.inc is INCLUDEd in a loop from reserve/request.tt whenever HoldsSplitQueue splits the holds queue by branch/itemtype, or when holds for more than one biblio are shown at once (multi_hold). Each inclusion appended its own <script> block declaring `all_libraries`, and since top-level `let`/`const` in separate inline <script> tags share the page's global lexical scope, the second and subsequent occurrences threw "Identifier 'all_libraries' has already been declared", breaking those script blocks and the pickup-library filtering that depends on `all_libraries` being set. Worse, that script block is only emitted when a biblio's holds are actually rendered (biblioloo.total_holds > 0), but holds.js is loaded unconditionally by reserve/request.tt and reads `all_libraries` at top-level parse time. Placing the very first hold on a title (0 existing holds) therefore threw: ReferenceError: all_libraries is not defined breaking every hold-management handler defined below that line in holds.js. Fix both by declaring `all_libraries` exactly once, unconditionally, in request.tt's existing page-global script block (alongside the other unconditional globals such as HidePatronName and HoldsSplitQueue), before Asset.js("js/holds.js") loads, instead of duplicating it inside the per-title/per-split holds_table.inc include. To test: 1. Search for a title with no existing holds and start placing one 2. Without this patch: browser console shows "ReferenceError: all_libraries is not defined" as soon as the page loads 3. With this patch: no console errors 4. Set HoldsSplitQueue to "pickup library", place holds on a title for two different pickup libraries, confirm both split tables render with working patron-name and pickup-library filtering and no console errors 5. Confirm the patron's own Holds tab (moremember.pl/circulation.pl) still loads without console errors -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #27 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201921 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201921&action=edit Bug 42915: (QA follow-up) Restore fallback message for holds with no patron data The "Add patron names to data in column def in holds table" patch dropped the null-patron handling in the patron column's render function, so a hold row whose patron embed comes back null (e.g. a user without permission to view the patron on that hold, per Bug 23269's test plan) now renders a blank cell instead of a useful message. Restore the fallback, sourced from `all_libraries` (populated by this same table's include) rather than the separate `libraries` global, and check `row.patron` directly instead of the now-composite `data` column key. To test: 1. Have holds on a bib from patrons the logged in user cannot see (e.g. IndependentBranches, or a reduced permission set) 2. View the record's Holds tab 3. Confirm rows with no visible patron show "A patron from library <library>" instead of a blank Patron cell Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #28 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201922 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201922&action=edit Bug 42915: (QA follow-up) Remove unused filters_options variable filters_options was introduced with the coded_values library filtering patch but never referenced anywhere in the file. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Rebecca Coert <rcoert@arlingtonva.us> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rcoert@arlingtonva.us -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #29 from Jonathan Druart <jonathan.druart@gmail.com> --- Bug 42915: (QA follow-up) Restore fallback message for holds with no patron data + library ? library._str : row.pickup_library_id There is no escape_str call for the library's name. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@openfifth.co.u | |k --- Comment #30 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- (In reply to Jonathan Druart from comment #29)
Bug 42915: (QA follow-up) Restore fallback message for holds with no patron data + library ? library._str : row.pickup_library_id
There is no escape_str call for the library's name.
Good catch, but pre-existing, this column never had escape_str to begin with, so it's a different scope. There are arguably other columns in the same table that need the same treatment too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.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=42915 --- Comment #31 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Thanks everyone! Pushed to main for 26.11! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Michelle Spinney <mspinney@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mspinney@clamsnet.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Keywords| |rel_25_05_candidate, | |rel_25_11_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 ayoung <ayoung@oslri.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ayoung@oslri.net --- Comment #32 from ayoung <ayoung@oslri.net> --- Any chance we could backport this to 25.11? Thanks!! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Laura O'Neil <laura@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julia.greider@norwichlibrar | |y.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au Keywords|rel_25_05_candidate |rel_26_05_candidate --- Comment #33 from David Cook <dcook@prosentient.com.au> --- (In reply to Pedro Amorim (ammopt) from comment #31)
Thanks everyone! Pushed to main for 26.11!
(In reply to ayoung from comment #32)
Any chance we could backport this to 25.11? Thanks!!
Yeah, this is pretty bad. It should really be backported to 26.05 and 25.11 ASAP. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #34 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #33)
(In reply to Pedro Amorim (ammopt) from comment #31)
Thanks everyone! Pushed to main for 26.11!
(In reply to ayoung from comment #32)
Any chance we could backport this to 25.11? Thanks!!
Yeah, this is pretty bad. It should really be backported to 26.05 and 25.11 ASAP.
I see... it doesn't apply cleanly to 25.11... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #35 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #34)
(In reply to David Cook from comment #33)
(In reply to Pedro Amorim (ammopt) from comment #31)
Thanks everyone! Pushed to main for 26.11!
(In reply to ayoung from comment #32)
Any chance we could backport this to 25.11? Thanks!!
Yeah, this is pretty bad. It should really be backported to 26.05 and 25.11 ASAP.
I see... it doesn't apply cleanly to 25.11...
I take that back. It seems like older perl-git-bz has just not been following dependencies. Weeee. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #36 from David Cook <dcook@prosentient.com.au> --- Ok bug 42915 does apply to 25.11 cleanly if you manually apply bug 42357 first as perl-git-bz doesn't appear to be handling the dependency automatically... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #37 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 201917 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201917 Bug 42915: Use coded value for library filtering in holds table Review of attachment 201917: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=42915&attachment=201917) ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/js/holds.js @@ +105,4 @@
}; })(jQuery);
+var coded_values = {
You almost never want to be declaring a global variable like this in a Javascript file. The reason it's a problem here is because this variable gets set *before* all_libraries even gets defined. I imagine in the first version when all_libraries got set in holds_table.inc it probably worked and then it broke when it was moved to requests.tt. Putting it into a a ready() functoin or DOMContentLoaded event handler would've been a lot better. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #38 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 202968 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202968&action=edit Bug 42915: (QA follow-up) Defer coded_values declaration until document ready holds.js declared `coded_values` as a bare top-level global, evaluated immediately at script-parse time, reading `all_libraries` which is only ever defined by the including template (request.tt, or patron-detail-tabs.inc for circulation.tt/moremember.tt). This works today only because those templates happen to emit their `all_libraries` script block before Asset.js("js/holds.js") in the rendered HTML - a fragile, easy-to-break ordering dependency between separate template files, not something holds.js itself can guarantee. Move the assignment into the existing $(document).ready() handler, setting it explicitly on `window` so it remains visible as a global to datatables.js's build_query(), which reads `coded_values` unqualified. This defers evaluation until all synchronous inline scripts (including whichever template defined all_libraries) have already run, regardless of future include/ordering changes. To test: 1. Place holds on a title for two different pickup libraries 2. Confirm the holds table on reserve/request.tt, the patron's Holds tab (circulation.pl and moremember.pl) all load with no console errors 3. Confirm filtering the holds table by pickup library name/branchcode still works on all three pages -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #39 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Good catch David, follow-up added. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=42915 --- Comment #40 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Thanks everyone! Pushed to main for 26.11! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #41 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Follow-up pushed. Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #42 from David Cook <dcook@prosentient.com.au> --- I'm assuming by "Try filtering" we're saying use the "Search box"? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #43 from ayoung <ayoung@oslri.net> --- Yes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|26.11.00 |26.11.00,26.05.03 released in| | Status|Pushed to main |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 --- Comment #44 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to 26.05.x for the upcoming 26.05.03 release. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42915 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_26_05_candidate | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org