Bug 11221 - New patrons are created with invalid dateofbirth value if field is left empty
Summary: New patrons are created with invalid dateofbirth value if field is left empty
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 10403
Blocks:
  Show dependency treegraph
 
Reported: 2013-11-08 16:01 UTC by Owen Leonard
Modified: 2014-05-26 21:04 UTC (History)
3 users (show)

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


Attachments
Bug 11221: SQLHelper::_filter_hash does not manage well dates (1.93 KB, patch)
2013-11-12 14:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11221: SQLHelper::_filter_hash does not manage well dates (1.98 KB, patch)
2013-11-17 07:07 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 11221: Follow up adding a unit test to test handling empty strings as dates (1.61 KB, patch)
2013-11-17 07:08 UTC, Chris Cormack
Details | Diff | Splinter Review
[PASSED QA] Bug 11221: SQLHelper::_filter_hash does not manage well dates (2.30 KB, patch)
2013-11-18 07:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11221: Follow up adding a unit test to test handling empty strings as dates (1.69 KB, patch)
2013-11-18 07:46 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 Owen Leonard 2013-11-08 16:01:12 UTC
If you leave the date of birth field empty when you create a new patron an invalid value is saved to the database: '0000-00-00' instead of NULL. To reproduce:

1. Create a new patron. Leave dateofbirth empty.
2. Save the record.
3. Open the record for editing.
4. Save the record without making changes.
5. Koha gives you an error: "Date of birth is invalid."

According to git bisect this bug was caused by the fix for Bug 10403. I assume it was this change to SQLHelper.pm since all other changes in that bug are related to acquisitions:

--- a/C4/SQLHelper.pm
+++ b/C4/SQLHelper.pm
@@ -406,7 +406,7 @@ sub _filter_hash{
                ## supposed to be a hash of simple values, hashes of arrays could be implemented
                $filter_input->{$field}=format_date_in_iso($filter_input->{$field})
           if $columns->{$field}{Type}=~/date/ &&
-             $filter_input->{$field} !~C4::Dates->regexp("iso");
+             ($filter_input->{$field} && $filter_input->{$field} !~C4::Dates->regexp("iso"));
                my ($tmpkeys, $localvalues)=_Process_Operands($filter_input->{$field},"$tablename.$field",$searchtype,$columns);
Comment 1 Jonathan Druart 2013-11-12 14:14:57 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2013-11-12 14:15:31 UTC
I think this patch is correct but I don't have the time to provide unit tests and I don't know how to foresee all side effects.

We should have several looks at this patch!
Comment 3 Chris Cormack 2013-11-17 07:07:57 UTC Comment hidden (obsolete)
Comment 4 Chris Cormack 2013-11-17 07:08:36 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2013-11-18 07:45:17 UTC
Created attachment 22993 [details] [review]
[PASSED QA] Bug 11221: SQLHelper::_filter_hash does not manage well dates

The default values for date fields is undef, so if a date field contains
an empty string, we should insert NULL in the DB.
The format_date_in_iso routine should be only called if a date is
defined, is not equal to an empty string and does not match the iso
regex.

Partial test plan:
1. Create a new patron. Leave dateofbirth empty.
2. Save the record.
3. Open the record for editing.
4. Save the record without making changes.
5. Koha gives no error.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
Now when no date is given NULL is saved to the database.

Tested:
- Adding a patron without date of birth
- Editing the patron, entering a date of birth
- Editing the patron, deleting date of birth
All worked as expected.
Comment 6 Katrin Fischer 2013-11-18 07:46:24 UTC
Created attachment 22994 [details] [review]
[PASSED QA] Bug 11221: Follow up adding a unit test to test handling empty strings as dates

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests pass.
Comment 7 Galen Charlton 2013-11-19 15:51:06 UTC
Pushed to master and 3.14.x.  Thanks, Jonathan and Chris!