Bug 14924 - Remove C4::Dates from members/memberentry.pl
Summary: Remove C4::Dates from members/memberentry.pl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Marc Véron
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 14870
  Show dependency treegraph
 
Reported: 2015-09-29 15:25 UTC by Marc Véron
Modified: 2016-12-05 21:23 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14924 - Remove C4::Dates from members/memberentry.pl (3.66 KB, patch)
2015-09-29 15:38 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 14924 - Remove C4::Dates from members/memberentry.pl (3.82 KB, patch)
2015-09-29 21:23 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
Bug 14924 - (follow-up) Add date validation (2.22 KB, patch)
2015-10-01 20:09 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14924 - (follow-up) Add date validation using eval (2.16 KB, patch)
2015-10-02 13:57 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 14924 - (follow-up) Add date validation using eval (2.28 KB, patch)
2015-10-07 21:50 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
Bug 14924 - Remove C4::Dates from members/memberentry.pl (4.63 KB, patch)
2015-10-08 08:32 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Véron 2015-09-29 15:25:54 UTC
This patch removes C4::Dates from members/memberentry.pl
Comment 1 Marc Véron 2015-09-29 15:38:28 UTC Comment hidden (obsolete)
Comment 2 Héctor Eduardo Castro Avalos 2015-09-29 21:23:11 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2015-10-01 08:32:11 UTC
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.
Comment 4 Marc Véron 2015-10-01 08:42:00 UTC
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?
Comment 5 Jonathan Druart 2015-10-01 09:51:47 UTC
(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.
Comment 6 Marc Véron 2015-10-01 13:38:50 UTC
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.
Comment 7 Jonathan Druart 2015-10-01 15:17:03 UTC
You can try with: 32/01/1970

Actually it will explode if the date is not correct.
Comment 8 Marc Véron 2015-10-01 20:09:26 UTC Comment hidden (obsolete)
Comment 9 Marc Véron 2015-10-02 13:57:24 UTC Comment hidden (obsolete)
Comment 10 Héctor Eduardo Castro Avalos 2015-10-07 21:50:11 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2015-10-08 08:32:43 UTC
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).
Comment 12 Jonathan Druart 2015-10-08 08:56:01 UTC
(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
Comment 13 Tomás Cohen Arazi 2015-10-08 14:00:41 UTC
Patch pushed to master.

Thanks Marc!