[Bug 30673] New: Improve is_valid_date function for validating date strings
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 Bug ID: 30673 Summary: Improve is_valid_date function for validating date strings Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: minor Priority: P5 - low Component: Templates Assignee: oleonard@myacpl.org Reporter: oleonard@myacpl.org QA Contact: testopia@bugs.koha-community.org The is_valid_date() function in calendar.js has two jobs: Confirm that the date string matches the current date format according to a regular expression; and confirm that the date is an actual date. Both must be true. For example: "2002-03-14" matches the date format "Y-m-d" and is a valid date -- valid date. "03/14/2002" doesn't match the date format "Y-m-d" even though it is a valid date -- invalid date. "38-44-2103" doesn't match the date format "Y-m-d" and isn't a valid date -- invalid date. is_valid_date() needs some changes in order for it to correctly handle custom date formats being specified which override the default date format (from the system preference). It also needs a new way to check that a date string represents a valid date. The old jQueryUI method looks similar but behaves differently. $.datepicker.parseDate() would throw an exception if the date were invalid. flatpickr.parseDate() will try to interpret an invalid date as if it were valid. For example: flatpickr.parseDate("2002-48-66", "Y-m-d") -> Date Sat Feb 04 2006 00:00:00 flatpickr.parseDate("2002-48-66", "m/d/Y") -> Date Mon Aug 02 2027 00:00:00 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 --- Comment #1 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 134615 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134615&action=edit Bug 30673: Improve is_valid_date function for validating date strings This patch corrects a couple of issues with our date validation in both the staff interface and the OPAC. The "is_valid_date" function needs to be able to accept a custom date format when one has been defined to override the default Flatpickr configuration. The function also needs to do better checking of the date itself following the transition to Flatpickr. jQueryUI's parse method had better built-in error handling, whereas Flatpickr's simply tries its best to convert a string into a date. We can use the existing Date_from_syspref function to check that the date string can be converted to a valid JavaScript Date object. To test, apply the patch and test date input fields in a few places in the staff interface, e.g. the patron edit form, the circulation statistics wizard, or the overdues page. Also check these specific pages: - With the dateformat system preference set to something other than "yyyy-mm-dd" open an item for editing. The "Date acquired" date picker should work correctly and fill the date in "yyyy-mm-dd" format (overriding the Flatpickr default). - Check Serial -> Claims to confirm that the "To" and "From" filters work correctly. - Test that various dateformat preference settings work in all cases. In the OPAC, test that date picker inputs work correctly on pages like the "Place hold" page, the "Your personal details" page, and "Your summary" -> "Holds" -> "Suspend." -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |30514 CC| |jonathan.druart+koha@gmail. | |com Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30514 [Bug 30514] Error in date format check following datepicker removal -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 --- Comment #2 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- - var dateformat_pref = "[% Koha.Preference('dateformat ') | html %]"; + var dateformat_pref = "[% Koha.Preference('dateformat') | html %]"; This needs to be moved to its own bug (for backport), see bug 30706. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 --- Comment #3 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- 1. - var MSG_PLEASE_ENTER_A_VALID_DATE = ( __("Please enter a valid date (should match %s).") ); + var MSG_PLEASE_ENTER_A_VALID_DATE = ( _("Please enter a valid date (should match %s).") ); Only at the OPAC, is that change expected? If so, why not at the intranet? 2. That's ugly. But we could go with this code for a quick fix. A correct fix would be to switch to dayjs (see bug 30310) and use dayjs('1970-00-00', 'YYYY-MM-DD', true).isValid() See https://day.js.org/docs/en/parse/string-format We should not need to deal with that by ourselves. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Well, no need to wait for dayjs, momentjs has the same syntax https://momentjs.com/docs/#/parsing/string-formats/ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 --- Comment #5 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 134735 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134735&action=edit Bug 30673: Use moment.js to improve our date parsing -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #5)
Created attachment 134735 [details] [review] Bug 30673: Use moment.js to improve our date parsing
That's not working well enough but I think we should go into this direction. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134735|Bug-30673-Use-momentjs-to-i |[DO NOT APPLY TO TEST FIRST filename|mprove-our-date-parsing.pat |PATCH] |ch |Bug-30673-Use-momentjs-to-i | |mprove-our-date-parsing.pat | |ch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134735|[DO NOT APPLY TO TEST FIRST |Bug-30673-Use-momentjs-to-i filename|PATCH] |mprove-our-date-parsing.pat |Bug-30673-Use-momentjs-to-i |ch |mprove-our-date-parsing.pat | |ch | Attachment #134735|Bug 30673: Use moment.js to |[DO NOT APPLY TO TEST FIRST description|improve our date parsing |PATCH] Bug 30673: Use | |moment.js to improve our | |date parsing -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134615|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=30673 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134615|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134615|0 |1 is obsolete| | --- Comment #7 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 134740 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134740&action=edit Bug 30673: Improve is_valid_date function for validating date strings This patch corrects a couple of issues with our date validation in both the staff interface and the OPAC. The "is_valid_date" function needs to be able to accept a custom date format when one has been defined to override the default Flatpickr configuration. The function also needs to do better checking of the date itself following the transition to Flatpickr. jQueryUI's parse method had better built-in error handling, whereas Flatpickr's simply tries its best to convert a string into a date. We can use the existing Date_from_syspref function to check that the date string can be converted to a valid JavaScript Date object. To test, apply the patch and test date input fields in a few places in the staff interface, e.g. the patron edit form, the circulation statistics wizard, or the overdues page. Also check these specific pages: - With the dateformat system preference set to something other than "yyyy-mm-dd" open an item for editing. The "Date acquired" date picker should work correctly and fill the date in "yyyy-mm-dd" format (overriding the Flatpickr default). - Check Serial -> Claims to confirm that the "To" and "From" filters work correctly. - Test that various dateformat preference settings work in all cases. In the OPAC, test that date picker inputs work correctly on pages like the "Place hold" page, the "Your personal details" page, and "Your summary" -> "Holds" -> "Suspend." -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 --- Comment #8 from Owen Leonard <oleonard@myacpl.org> --- (In reply to Jonathan Druart from comment #3)
Only at the OPAC, is that change expected? If so, why not at the intranet?
The OPAC calendar.inc shouldn't use the double underscore because it is a template, not JavaScript. In the staff interface some calendar JS has been split out into a JS file, where the double underscore is correct. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Should we only test the second patch here? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 --- Comment #10 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Katrin Fischer from comment #9)
Should we only test the second patch here?
Yes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I've tested the second patch and could not spot any issues, but there is a mismatch with the test plan: - With the dateformat system preference set to something other than "yyyy-mm-dd" open an item for editing. The "Date acquired" date picker should work correctly and fill the date in "yyyy-mm-dd" format (overriding the Flatpickr default). I was using DD.MM.YYYY and the date showed like this in the input field. Changing and storing a different format appeared to work well tho. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 Katrin Fischer <katrin.fischer@bsz-bw.de> 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=30673 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134740|0 |1 is obsolete| | --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 136552 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136552&action=edit Bug 30673: Improve is_valid_date function for validating date strings This patch corrects a couple of issues with our date validation in both the staff interface and the OPAC. The "is_valid_date" function needs to be able to accept a custom date format when one has been defined to override the default Flatpickr configuration. The function also needs to do better checking of the date itself following the transition to Flatpickr. jQueryUI's parse method had better built-in error handling, whereas Flatpickr's simply tries its best to convert a string into a date. We can use the existing Date_from_syspref function to check that the date string can be converted to a valid JavaScript Date object. To test, apply the patch and test date input fields in a few places in the staff interface, e.g. the patron edit form, the circulation statistics wizard, or the overdues page. Also check these specific pages: - With the dateformat system preference set to something other than "yyyy-mm-dd" open an item for editing. The "Date acquired" date picker should work correctly and fill the date in "yyyy-mm-dd" format (overriding the Flatpickr default). - Check Serial -> Claims to confirm that the "To" and "From" filters work correctly. - Test that various dateformat preference settings work in all cases. In the OPAC, test that date picker inputs work correctly on pages like the "Place hold" page, the "Your personal details" page, and "Your summary" -> "Holds" -> "Suspend." Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=31149 --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #6)
(In reply to Jonathan Druart from comment #5)
Created attachment 134735 [details] [review] [review] Bug 30673: Use moment.js to improve our date parsing
That's not working well enough but I think we should go into this direction.
Moved to bug 31149. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134735|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=30673 --- Comment #14 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- IMO we should go with bug 31149, not this patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Should we mark this WONTFIX then or would this still be good for backports/older versions? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 --- Comment #16 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Katrin Fischer from comment #15)
Should we mark this WONTFIX then or would this still be good for backports/older versions?
I think so. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 --- Comment #17 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #16)
(In reply to Katrin Fischer from comment #15)
Should we mark this WONTFIX then or would this still be good for backports/older versions?
I think so.
I think that we should go with bug 31149 and don't spend more time in this mess. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30673 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |MOVED Status|Signed Off |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org