|
Description
Kyle M Hall (khall)
2012-09-28 17:47:10 UTC
Created attachment 12601 [details] [review] Bug 8845 - Add ability search patrons by date of birth Adds the ability to search by dates of birth in iso format. Created attachment 12608 [details] [review] Bug 8845 - Add ability search patrons by date of birth Adds the ability to search by dates of birth in iso format. Signed-off-by: Marc Veron <veron@veron.ch> Searched with following formats: YYYYMMDD , YYYY-MM-DD , YYYY.MM.DD All searches are OK I think it's a problem that this patch doesn't take varying date formats into account. What about searches by DD-MM-YYYY or MM-DD-YYYY? A couple of possibilities for mitigating the problem, if the patch can't be amended to accept multiple formats: - Offer a hint about the date format required - Offer a datepicker which will return the correct format Both could be triggered upon selection of the "date of birth" option. I think the problem with coding is that e.g. 03-09-2012 would be ambiguous (3. September or 9. March?). Thats why I propose to give a hint, the best place would be in the selection itself, e.g. <option value='dateofbirth'>Date of birth (YYYYMMDD)</option> Marc (In reply to comment #4) > I think the problem with coding is that e.g. 03-09-2012 would be ambiguous > (3. September or 9. March?). Unless you used the dateformat system preference as a rule by which you parse the date. Does Koha have existing code for handling varying date inputs? Otherwise, I agree that putting a hint in the <option> is a good idea. Created attachment 12639 [details] [review] Bug 8845 - Add ability search patrons by date of birth - Add tooltip Changing target version so this bug report will show up in the list of 3.12-targeted features and doesn't get lost in the shuffle during feature freeze. Last patch needs a SO Created attachment 14293 [details] [review] Bug 8845: Followup Allow to give a date using the syspref format Comment on attachment 14293 [details] [review] Bug 8845: Followup Allow to give a date using the syspref format Kyle, This patch allows to enter a date given the syspref format. I let you decide if it is better or not. I mark it as obsolete in order to not interfere with your last patch. Created attachment 14367 [details] [review] Bug 8845: Followup Allow to give a date using the syspref format Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 14368 [details] [review] Bug 8845 - Add ability search patrons by date of birth - Add tooltip I think that is a great followup! I've updated the tooltip patch, if you can SO that patch, we can set the status to signed-off! (In reply to comment #10) > Comment on attachment 14293 [details] [review] > Bug 8845: Followup Allow to give a date using the syspref format > > Kyle, > This patch allows to enter a date given the syspref format. > I let you decide if it is better or not. > I mark it as obsolete in order to not interfere with your last patch. Kyle, your patch does not work: the dateformat variable is not set in the template (the tooltip is 'Dates of birth should be entered in the format ""'). (/!\ Work in progress in Bug 9108) Created attachment 14392 [details] [review] Bug 8845 - Add ability search patrons by date of birth - Add tooltip Sorry about that! Please retest this updated patch! Created attachment 14393 [details] [review] Bug 8845 - Add ability search patrons by date of birth - Add tooltip Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Works with all date formats Bug 8845: Followup Allow to give a date using the syspref format Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -------------------------- Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y Applying: Bug 8845: Followup Allow to give a date using the syspref format Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging members/member.pl CONFLICT (content): Merge conflict in members/member.pl Failed to merge in the changes. Patch failed at 0001 Bug 8845: Followup Allow to give a date using the syspref format When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". Created attachment 15556 [details] [review] Bug 8845 - Add ability search patrons by date of birth Adds the ability to search by dates of birth in iso format. Signed-off-by: Marc Veron <veron@veron.ch> Searched with following formats: YYYYMMDD , YYYY-MM-DD , YYYY.MM.DD All searches are OK Created attachment 15557 [details] [review] Bug 8845: Followup Allow to give a date using the syspref format Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 15558 [details] [review] Bug 8845 - Add ability search patrons by date of birth - Add tooltip Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Works with all date formats QA comment: This patch adds a new entry in the search fields dropdown list: date of birth. The functionality deals with all date formats and works as expected. Marked as Passed QA. Created attachment 18437 [details] [review] Bug 8845 - Add ability search patrons by date of birth Adds the ability to search by dates of birth in iso format. Signed-off-by: Marc Veron <veron@veron.ch> Searched with following formats: YYYYMMDD , YYYY-MM-DD , YYYY.MM.DD All searches are OK Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 18438 [details] [review] Bug 8845: Followup Allow to give a date using the syspref format Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 18439 [details] [review] Bug 8845 - Add ability search patrons by date of birth - Add tooltip Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Works with all date formats The patches do not apply. The base patch needs to be rebased to take 065fe316e into account, while the tooltip patch needs to be adjusted now that 7e10f6fb was pushed. Created attachment 18508 [details] [review] Bug 8845 - Add ability search patrons by date of birth Adds the ability to search by dates of birth in iso format. Signed-off-by: Marc Veron <veron@veron.ch> Searched with following formats: YYYYMMDD , YYYY-MM-DD , YYYY.MM.DD All searches are OK Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 18509 [details] [review] Bug 8845: Followup Allow to give a date using the syspref format Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 18510 [details] [review] Bug 8845 - Add ability search patrons by date of birth - Add tooltip Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Works with all date formats Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 18511 [details] [review] Bug 8845: FIX dateonly is the 4th parameter, not 3rd! Rebased patches. I added a patch in order to fix a variable order. Pushed to master. Thanks, Kyle and Jonathan! |