Bug 32505

Summary: Cannot search by dateofbirth in specified dateformat
Product: Koha Reporter: Lucas Gass (lukeg) <lucas>
Component: PatronsAssignee: 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: MainKeywords: 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
To recreate:

1. Set DefaultPatronSearchFields to include dateofbirth
2. Set dateformat to something besides iso. I am using mm/dd/yyyy.
3. Set a patron to have a dateofbirth
4. Search for them in your specified dateformat. For me "04/24/1980".
It doesn't return your patron.
5. Try ISO format, "1980-04-24". It works. 

We should be able to find patrons in a search based on the dateformat specified.
Comment 1 Avery Campbell (Butte County Library (CA)) 2022-12-21 17:43:24 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).
Comment 2 Daniel Gaghan 2022-12-26 18:10:36 UTC
Yup we use DOB to search for patron so this is a big priority fix for us at PCCLD.
Comment 3 LINDA WATSON 2022-12-27 14:36:51 UTC
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.
Comment 4 Katrin Fischer 2022-12-27 21:59:10 UTC
(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.
Comment 5 Jonathan Druart 2023-01-02 16:19:48 UTC
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.
Comment 6 Lucas Gass (lukeg) 2023-01-03 14:50:54 UTC
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>
Comment 7 Jonathan Druart 2023-01-03 16:59:34 UTC
(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.
Comment 8 Martin Renvoize (ashimema) 2023-01-04 11:13:18 UTC
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.
Comment 9 Kyle M Hall (khall) 2023-01-06 19:21:04 UTC
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>
Comment 10 Martin Renvoize (ashimema) 2023-01-09 10:03:00 UTC
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.
Comment 11 Martin Renvoize (ashimema) 2023-01-09 10:04:44 UTC
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.
Comment 12 Martin Renvoize (ashimema) 2023-01-09 10:06:45 UTC
Ack, ignore that.. this is about the left side search, not the column filters.. dropping my patch and reverting to PQA
Comment 13 Suzanne 2023-01-12 21:18:43 UTC
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!
Comment 14 Martin Renvoize (ashimema) 2023-01-18 15:50:20 UTC
Nice work everyone!

Pushed to 23.05.x for the next release
Comment 15 Jacob O'Mara 2023-01-18 17:05:57 UTC
Nice work, thanks everyone!

Pushed to 22.11.x for the next release.
Comment 16 Lucas Gass (lukeg) 2023-01-20 20:52:46 UTC
Backported to 22.05.x for upcoming 22.05.09 release
Comment 17 Arthur Suzuki 2023-01-26 13:16:23 UTC
This one conflicts a lot when trying to apply on 21.11.x.
Won't backport.
Comment 18 Lucas Gass (lukeg) 2023-02-24 17:21:20 UTC
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.
Comment 19 Lucas Gass (lukeg) 2023-02-24 17:23:34 UTC
Created attachment 147377 [details] [review]
Bug 32505: Use moment.js for 22.05.x
Comment 20 Lucas Gass (lukeg) 2023-02-24 17:25:52 UTC
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.
Comment 21 Jonathan Druart 2023-02-28 07:37:22 UTC
(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.
Comment 22 Lucas Gass (lukeg) 2023-03-01 17:15:17 UTC
(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?
Comment 23 Lucas Gass (lukeg) 2023-03-03 20:23:21 UTC
I am filed Bug 33132 to address this for oldstable