Comment on attachment 42964 [details] [review]
[SIGNED-OFF]Bug 14924 - Remove C4::Dates from members/memberentry.pl

Review of attachment 42964 [details] [review]:
-----------------------------------------------------------------

::: members/memberentry.pl
@@ +170,5 @@
>      foreach (qw(dateenrolled dateexpiry dateofbirth)) {
>          next unless exists $newdata{$_};
>          my $userdate = $newdata{$_} or next;
> +        if ( $userdate ) {
> +            $newdata{$_} = output_pref({ dt => dt_from_string( $userdate ), dateformat => 'iso', dateonly => 1 });

You have to care of bad formatted dates, otherwise the user will get a software error.

Hi Jonathan,

I tried to enter an invalid date, but we have the date picker for this date - it seems no longer to be possible to enter invalid dates for the due date.

Or do I miss a way to enter an invalid date?

(In reply to Marc Véron from comment #4)
> Hi Jonathan,
> 
> I tried to enter an invalid date, but we have the date picker for this date
> - it seems no longer to be possible to enter invalid dates for the due date.
> 
> Or do I miss a way to enter an invalid date?

I have tried with the date of birth on creating/updating a patron.
You get a JS alert box but are able to submit the form.

Ok, I can make it explode if I enter 0000-00-00 for date of birth:

---
Can't locate object method "ymd" via package "dateformat" (perhaps you forgot to load "dateformat"?) at /usr/share/kohaclone/Koha/DateUtils.pm line 206.
---

I think we need a generic function in Koha::DateUtils to test a date string against the 'dateformat' syspref.

You can try with: 32/01/1970

Actually it will explode if the date is not correct.

Created attachment 43220 [details] [review]
Bug 14924 - Remove C4::Dates from members/memberentry.pl

This patch removes C4::Dates from members/memberentry.pl

To test:
- Apply patch
- Add and edit patrons with and without birthdate as
  mandatory field (syspref BorrowerMandatoryField)
- Verify that dates (birtdate, registration, expiration)
  display and are stored correctly

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised. Tested with syspref BorrowerMandatoryField and
table borrowers.dateofbirth

Bug 14924 - (follow-up) Add date validation using eval

This patch adds a date validation (see comment #3).

To test:

- Apply patch
- Create a new user or edit an existing user
- Try valid dates for date of birth
- Try invalid dates as 0000-00-00 or 32/01/1970. You can can
  copy/paste such strings to the date field, ignore the warning
  message and submit. Verify that after submit you get a message
  "Date of birth is invalid"
- Try the same with enrollement /expiry dates

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised. Tested with invalid dates.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Amended patch: I have splitted them to make them much more readable
(some lines were removed then added).

(In reply to Jonathan Druart from comment #11)
> Amended patch: I have splitted them to make them much more readable
> (some lines were removed then added).

s/splitted/squashed

Patch pushed to master.

Thanks Marc!