Bug 11221

Summary: New patrons are created with invalid dateofbirth value if field is left empty
Product: Koha Reporter: Owen Leonard <oleonard>
Component: PatronsAssignee: Bugs List <koha-bugs>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: gmcharlt, jonathan.druart, kyle.m.hall
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 10403    
Bug Blocks:    
Attachments: Bug 11221: SQLHelper::_filter_hash does not manage well dates
Bug 11221: SQLHelper::_filter_hash does not manage well dates
Bug 11221: Follow up adding a unit test to test handling empty strings as dates
[PASSED QA] Bug 11221: SQLHelper::_filter_hash does not manage well dates
[PASSED QA] Bug 11221: Follow up adding a unit test to test handling empty strings as dates

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!