Summary: | Cannot search by dateofbirth in specified dateformat | ||
---|---|---|---|
Product: | Koha | Reporter: | Lucas Gass (lukeg) <lucas> |
Component: | Patrons | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Kyle M Hall (khall) <kyle> |
Severity: | normal | ||
Priority: | P5 - low | CC: | acampbell, andrew, arthur.suzuki, chughesman, daniel.gaghan, george, gmcharlt, jonathan.druart, kyle.m.hall, kyle, linda.watson, martin.renvoize, smhodson123extra |
Version: | Main | Keywords: | regression |
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34226 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
23.05.00,22.11.02,22.05.09
|
|
Circulation function: | |||
Bug Depends on: | 14874, 30310 | ||
Bug Blocks: | 32559 | ||
Attachments: |
Bug 32505: Restore patron search by formatted dob
Bug 32505: Restore patron search by formatted dob Bug 32505: Restore patron search by formatted dob Bug 32505: [ALT] Use column 'type' to pre-process search patterns Bug 32505: Use moment.js for 22.05.x |
Description
Lucas Gass (lukeg)
2022-12-20 16:08:51 UTC
"We should be able to find patrons in a search based on the dateformat specified." +1 on this. Maybe even having a system preference that sets our locale dateformat (we use mm/dd but many areas use dd/mm and yyyy/mm/dd). mm/dd/yyyy or mm-dd-yyyy is our locale dateformat so staff are much more used to typing dates in this dateformat (especially since it was possible before our recent update to 22.05). Yup we use DOB to search for patron so this is a big priority fix for us at PCCLD. We are also having the same issue at Marion County Public Library System after the update. Unable to search for date of birth with slashes. (In reply to Avery Campbell (Butte County Library (CA)) from comment #1) > "We should be able to find patrons in a search based on the dateformat > specified." > +1 on this. Maybe even having a system preference that sets our locale > dateformat (we use mm/dd but many areas use dd/mm and yyyy/mm/dd). There already is a system preference to set the date format: DateFormat :) This has worked before, I am marking it as a regression. I am linking the original bug that added the feature, but I think maybe the patron search rewrite could be to blame for the problem as this was not a well known feature. Created attachment 144922 [details] [review] Bug 32505: Restore patron search by formatted dob Prior to the rewrite of the patron searches (bug 30063 and friends) it was possible to search for a patron using their date of birth, formatted following the dateformat syspref. Now it only works if the date is iso formatted. This patch is providing a fix to restore the behaviour, but does not make it consistent. Only searching patrons by date of birth is fixed here, when we actually want to fix the problem for the REST API DT wrapper instead. Test plan: Search for patrons given their date of birth. You need to select "date of birth" in the "Search field" dropdown, or set it in DefaultPatronSearchFields. Note that the column filtering is still not working, but I guess it was the case already in older versions. Ideally we will need to have a "datatype" attribute passed to the the 'th' and retrieved from datatables.js to add this same trick and build the query appropriately. Created attachment 144937 [details] [review] Bug 32505: Restore patron search by formatted dob Prior to the rewrite of the patron searches (bug 30063 and friends) it was possible to search for a patron using their date of birth, formatted following the dateformat syspref. Now it only works if the date is iso formatted. This patch is providing a fix to restore the behaviour, but does not make it consistent. Only searching patrons by date of birth is fixed here, when we actually want to fix the problem for the REST API DT wrapper instead. Test plan: Search for patrons given their date of birth. You need to select "date of birth" in the "Search field" dropdown, or set it in DefaultPatronSearchFields. Note that the column filtering is still not working, but I guess it was the case already in older versions. Ideally we will need to have a "datatype" attribute passed to the the 'th' and retrieved from datatables.js to add this same trick and build the query appropriately. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> (In reply to Jonathan Druart from comment #5) > Note that the column filtering is still not working, but I guess it was > the case already in older versions. Ideally we will need to have a > "datatype" attribute passed to the the 'th' and retrieved from > datatables.js to add this same trick and build the query appropriately. See bug 32559. Is this at the right level? I'm wondering if we aught to instead have this bound to a data type in the field name or something so it can be used more generically in the ajax driven datatables stuff. I have a vague recollection of trying this stuff in another bug that got stuck. Created attachment 145096 [details] [review] Bug 32505: Restore patron search by formatted dob Prior to the rewrite of the patron searches (bug 30063 and friends) it was possible to search for a patron using their date of birth, formatted following the dateformat syspref. Now it only works if the date is iso formatted. This patch is providing a fix to restore the behaviour, but does not make it consistent. Only searching patrons by date of birth is fixed here, when we actually want to fix the problem for the REST API DT wrapper instead. Test plan: Search for patrons given their date of birth. You need to select "date of birth" in the "Search field" dropdown, or set it in DefaultPatronSearchFields. Note that the column filtering is still not working, but I guess it was the case already in older versions. Ideally we will need to have a "datatype" attribute passed to the the 'th' and retrieved from datatables.js to add this same trick and build the query appropriately. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 145135 [details] [review] Bug 32505: [ALT] Use column 'type' to pre-process search patterns This patch adds the option to pass 'type' at the 'column' definition level and then binds on that type to add date parsing to queries. Could you take a quick look at my alternative patch Jonathan? If you disagree with it, then we can just drop it and revert the status to PQA, but I wanted to show my option at the datatables.js level. Ack, ignore that.. this is about the left side search, not the column filters.. dropping my patch and reverting to PQA I'm not too familiar or understanding with what is happening with possible patches to fix this, but just want to put in my two cents that we would love to see this resolved, as we use it all the time to avoid duplicate accounts. We want to be able to search by birthday without having to select "Date of Birth" as the type of search and certainly in our preferred format of mm/dd/yyyy. It would be a great administrative preference for libraries to be able to enter the format they would like to use to search birthdays, including dash or slash or both. Thanks! Nice work everyone! Pushed to 23.05.x for the next release Nice work, thanks everyone! Pushed to 22.11.x for the next release. Backported to 22.05.x for upcoming 22.05.09 release This one conflicts a lot when trying to apply on 21.11.x. Won't backport. This actually depends on Bug 30310 which is not in 22.05.x, so this still does not work in 22.05.x. I can be blamed here for not testing thoroughly enough before backporting. Created attachment 147377 [details] [review] Bug 32505: Use moment.js for 22.05.x I added a patch that will work with 22.05.x and moment.js. If someone can give me a sign-off/QA I could get it in for 22.05.10. (In reply to Lucas Gass from comment #20) > I added a patch that will work with 22.05.x and moment.js. If someone can > give me a sign-off/QA I could get it in for 22.05.10. Why don't you use .format("YYYY-MM-DD")? I think it would be better to fix date_to_rfc3339 to avoid future backports to fail like this one. (In reply to Jonathan Druart from comment #21) > (In reply to Lucas Gass from comment #20) > > I added a patch that will work with 22.05.x and moment.js. If someone can > > give me a sign-off/QA I could get it in for 22.05.10. > > Why don't you use .format("YYYY-MM-DD")? I think it would be better to fix > date_to_rfc3339 to avoid future backports to fail like this one. By backporting Bug 30310? I am filed Bug 33132 to address this for oldstable |