From d63b4a766cc41a1cfdcb2a6eecac0782e43b63f7 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 12 Nov 2013 15:07:54 +0100 Subject: [PATCH 1/3] Bug 11221: ensure that SQLHelper uses NULL rather than 0000-00-00 as default date value The default values for date fields is undef, so if a date field contains an empty string, we should insert NULL in the DB, not 0000-00-00. 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. This patch fixes a bug where editing or creating a patron record without setting the birth date results in 0000-00-00 rather than null being set as the dateofbirth value. 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 Signed-off-by: Katrin Fischer 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. Signed-off-by: Galen Charlton --- C4/SQLHelper.pm | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/C4/SQLHelper.pm b/C4/SQLHelper.pm index f1fa7b5276..b8670942c5 100644 --- a/C4/SQLHelper.pm +++ b/C4/SQLHelper.pm @@ -404,9 +404,15 @@ sub _filter_hash{ my $elements=join "|",@columns_filtered; foreach my $field (grep {/\b($elements)\b/} keys %$filter_input){ ## 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} && $filter_input->{$field} !~C4::Dates->regexp("iso")); + if ( $columns->{$field}{Type}=~/date/ ) { + if ( defined $filter_input->{$field} ) { + if ( $filter_input->{$field} eq q{} ) { + $filter_input->{$field} = undef; + } elsif ( $filter_input->{$field} !~ C4::Dates->regexp("iso") ) { + $filter_input->{$field} = format_date_in_iso($filter_input->{$field}); + } + } + } my ($tmpkeys, $localvalues)=_Process_Operands($filter_input->{$field},"$tablename.$field",$searchtype,$columns); if (@$tmpkeys){ push @values, @$localvalues; From 11fbcb30de22970290f2b1493de91cea0cddb4fc Mon Sep 17 00:00:00 2001 From: Chris Cormack Date: Sun, 17 Nov 2013 19:57:38 +1300 Subject: [PATCH 2/3] Bug 11221: (follow-up) add unit test to test handling empty strings as dates Signed-off-by: Katrin Fischer All tests pass. Signed-off-by: Galen Charlton --- t/db_dependent/Members.t | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t index 634ac0c4f4..4cc0a3e5f0 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -6,7 +6,7 @@ use strict; use warnings; -use Test::More tests => 22; +use Test::More tests => 23; use Data::Dumper; BEGIN { @@ -59,6 +59,7 @@ my %data = ( surname => $SURNAME, categorycode => $CATEGORYCODE, branchcode => $BRANCHCODE, + dateofbirth => '' ); my $addmem=AddMember(%data); @@ -74,6 +75,8 @@ ok ( $member->{firstname} eq $FIRSTNAME && , "Got member") or diag("Mismatching member details: ".Dumper(\%data, $member)); +ok($member->{dateofbirth} eq '', "Empty dates handled correctly"); + $member->{firstname} = $CHANGED_FIRSTNAME; $member->{email} = $EMAIL; $member->{ethnicity} = $ETHNICITY; @@ -166,7 +169,7 @@ is ($notice_email, $EMAIL, "GetNoticeEmailAddress returns correct value when Aut C4::Context->set_preference( 'AutoEmailPrimaryAddress', 'emailpro' ); C4::Context->clear_syspref_cache(); -my $notice_email = GetNoticeEmailAddress($member->{'borrowernumber'}); +$notice_email = GetNoticeEmailAddress($member->{'borrowernumber'}); is ($notice_email, $EMAILPRO, "GetNoticeEmailAddress returns correct value when AutoEmailPrimaryAddress is emailpro"); From 15812e80c8e6db286bf96e63e71f7239eaf50b25 Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Tue, 19 Nov 2013 15:35:25 +0000 Subject: [PATCH 3/3] Bug 11221: (follow-up) test for NULL rather than empty string Signed-off-by: Galen Charlton --- t/db_dependent/Members.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t index 4cc0a3e5f0..6904190bc9 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -75,7 +75,7 @@ ok ( $member->{firstname} eq $FIRSTNAME && , "Got member") or diag("Mismatching member details: ".Dumper(\%data, $member)); -ok($member->{dateofbirth} eq '', "Empty dates handled correctly"); +is($member->{dateofbirth}, undef, "Empty dates handled correctly"); $member->{firstname} = $CHANGED_FIRSTNAME; $member->{email} = $EMAIL;