Bug 14794 - Searching patron by birthday returns no results if format incorrect
Summary: Searching patron by birthday returns no results if format incorrect
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-08 11:34 UTC by Joonas Kylmälä
Modified: 2020-06-04 20:32 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.01


Attachments
Bug 14794: Display JS tooltip whenever dateofbirth is being searched (2.86 KB, patch)
2019-06-07 14:55 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 14794: Display JS tooltip whenever dateofbirth is being searched (2.91 KB, patch)
2019-06-07 18:39 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 14794: (follow-up) Add tooltip to filter box (2.47 KB, patch)
2019-06-10 13:34 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 14794: Display JS tooltip whenever dateofbirth is being searched (2.97 KB, patch)
2019-06-13 05:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14794: (follow-up) Add tooltip to filter box (2.53 KB, patch)
2019-06-13 05:52 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Joonas Kylmälä 2015-09-08 11:34:10 UTC
When searching patrons by patrons' birthday in staff side (Home › Patrons & choose search field Date of Birth), it seems like the web page gives never an answer to the user. It just keeps showing that "Processing..." thingie.

So this happened when I gave only the day and month of the full birth day / or if I gave only the year of the birthday. With the full xx/xx/xxxx form it works.
Comment 1 Marc Véron 2015-09-08 16:48:30 UTC
I did following tests:

Set datefomat syspref to dd/mm/yyy

Do a search from the header section on Home  > Patrons
Expand search option with [+]
Select "Date of birth"
A small hint appears explaining that dates should be entered as DD/MM/YYYY
Select "Contains"
Search for 16/09/1946  (I have one patron with this birth date)
Result OK

Do the same with "Starts with"
Result OK

Do the same with 16/09
Do the same with 16.09.1946
Result (for both): 
Software error: The given date (16.09.1946) does not match the date format (metric) at /usr/share/kohaclone/Koha/DateUtils.pm line 144.

Repeat search in the filter section at the left:
Select "Date of birth" (No hint about format appears)
Search for 16/09/1946
Result OK

Do the same with 16/09 or 16.09.1946
Result: Data table appears with endless "Processing..." as described by Joonas

Setting importance to "Minor" (The component mostly works, but causes some irritation to users.)
Comment 2 Marc Véron 2015-09-08 18:54:23 UTC
For the case of the search at the left (endless 'Processing'):

It dies inside a call to dt_from_string($searchmember) in
svc/members/search, line 51

For the search from the header, it dies dies inside a call to dt_from_string($searchmember) in
members/member.pl , line 70

dt_from_string  dies in Koha/DateUtils.pm at line 144 if it does not get a valid date string (in the current syspref format or in the iso format).

I think that this part of dt_from_string should go to a separate sub.
See 
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13601#c27

It could then be used for pre-testing date strings from user inputs like the ones in this bug.
Comment 3 Joonas Kylmälä 2015-09-11 07:52:23 UTC
(In reply to Marc Véron from comment #2)
> For the case of the search at the left (endless 'Processing'):
> 
> It dies inside a call to dt_from_string($searchmember) in
> svc/members/search, line 51
> 
> For the search from the header, it dies dies inside a call to
> dt_from_string($searchmember) in
> members/member.pl , line 70
> 
> dt_from_string  dies in Koha/DateUtils.pm at line 144 if it does not get a
> valid date string (in the current syspref format or in the iso format).
> 
> I think that this part of dt_from_string should go to a separate sub.
> See 
> http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13601#c27
> 
> It could then be used for pre-testing date strings from user inputs like the
> ones in this bug.

I can agree with you that we should move it to another sub and then we could test it. 

However, I think the user should be able to do a search with just a year / day / month set, not all of them.. We could do it so that if the user wants to search by year the input could be, for example, 1990/??/?? or maybe just 1990. With days and months it is hard to separate them from each other, so that's why I used the marking ?. So if someone wants to search patrons by birthday day it would be ????/??/20 (yyyy/mm/dd).

If we want to do my proposal we would need to add a new argument (flag) in the testing subroutine's arguments to indicate that this is a date string in ????/??/?? format, where some ? can be digits.

But if we now just focus on solving the endless processing text showing, lets fix that first, and maybe I'll do another bug report for my proposal!
Comment 4 Jonathan Druart 2015-09-14 07:49:19 UTC
I think we need a patch to fix the software error (maybe JS side).
We could imagine to accept a year also (YYYY), but something like ????/??/20 looks to me a bit too much.
Comment 5 Nick Clemens 2019-06-07 14:55:52 UTC
Created attachment 90424 [details] [review]
Bug 14794: Display JS tooltip whenever dateofbirth is being searched

This updates the code that adds a tooltip for the correct date format
to display after the initial search

To test:
1 - Do a patron search, choosing date of birth as the search field
2 - Note on choosing that field you get a tooltip defining the format
3 - Do a search
4 - Note tooltip is gone, but date fo birth is still selected as the field to search
5 - Apply patch, reload
6 - Note tooltip shows after initial search and until field is changed
Comment 6 Nick Clemens 2019-06-07 14:57:04 UTC
It seems the code no longer hangs, this just improves visiblity of tooltip
Comment 7 Maryse Simard 2019-06-07 18:39:01 UTC
Created attachment 90428 [details] [review]
Bug 14794: Display JS tooltip whenever dateofbirth is being searched

This updates the code that adds a tooltip for the correct date format
to display after the initial search

To test:
1 - Do a patron search, choosing date of birth as the search field
2 - Note on choosing that field you get a tooltip defining the format
3 - Do a search
4 - Note tooltip is gone, but date fo birth is still selected as the field to search
5 - Apply patch, reload
6 - Note tooltip shows after initial search and until field is changed

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Comment 8 Jonathan Druart 2019-06-07 22:55:29 UTC
Hi Nick, I do not understand what you are trying to fix.

Do not we want to add a JS check to make sure the date is valid?

Moreover if you want the tooltip to be displayed when the input has the focus, maybe you should bind the on focus event?
Comment 9 Katrin Fischer 2019-06-10 10:29:54 UTC
I like the tool tip idea, although I'd omit the '' (I know people who will enter them :) ) Checking the format would be nice, but is not a blocker here for me.

But: I think we need the same on the filters on the left side.
Comment 10 Nick Clemens 2019-06-10 13:34:35 UTC
Created attachment 90455 [details] [review]
Bug 14794: (follow-up) Add tooltip to filter box
Comment 11 Nick Clemens 2019-06-10 13:49:10 UTC
(In reply to Jonathan Druart from comment #8)
> Hi Nick, I do not understand what you are trying to fix.
> 
> Do not we want to add a JS check to make sure the date is valid?
> 
> Moreover if you want the tooltip to be displayed when the input has the
> focus, maybe you should bind the on focus event?

It is not so much a 'fix' but hopefully provides feedback to the user on what they must do there. I am not changing behvaiour, just trying to make current behaviour more clear.

I think making that field a date entry/calendar would probably be helpful, but I think that can be a separate enhancement.

The tooltip shows when the dropdown is changed, or on focus, so I think doing it on change makes sense - you select dateofbirth, you see what you must enter
Comment 12 Jonathan Druart 2019-06-10 14:50:08 UTC
About the JS check, I actually thought we add a JS function to check the date formatting, and so it would have been easy to reuse it here. But we don't. We have a CheckDate function (in members.js) to make sure it matches DD/MM/YYYY.
Comment 13 Katrin Fischer 2019-06-13 05:52:07 UTC
Created attachment 90551 [details] [review]
Bug 14794: Display JS tooltip whenever dateofbirth is being searched

This updates the code that adds a tooltip for the correct date format
to display after the initial search

To test:
1 - Do a patron search, choosing date of birth as the search field
2 - Note on choosing that field you get a tooltip defining the format
3 - Do a search
4 - Note tooltip is gone, but date fo birth is still selected as the field to search
5 - Apply patch, reload
6 - Note tooltip shows after initial search and until field is changed

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2019-06-13 05:52:10 UTC
Created attachment 90552 [details] [review]
Bug 14794: (follow-up) Add tooltip to filter box

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Martin Renvoize 2019-06-13 11:07:33 UTC
Nice work!

Pushed to master for 19.11.00
Comment 16 Fridolin Somers 2019-06-17 07:55:47 UTC
Pushed to 19.05.x for 19.05.01
Comment 17 Lucas Gass 2019-06-26 14:41:49 UTC
backported to 18.11.x for 18.11.07