From 099e2fe2b700c388e4939b2b2505773056d3383e Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 18 Dec 2020 15:16:11 +0100 Subject: [PATCH] Bug 7806: Fix remaining occurrences of 0000-00-00 We should remove all SQL queries that contain 0000-00-00 and finally assume we do not longer have such value in our DB (for date type) We already dealt with such values in previous update DB entries. The 2 added by this one haven't been replaced already. The code will now assume that either a valid date exist, or NULL/undef. Test plan: QA review is needed and test of the different places where code is modified. Not sure about the change from reports/issues_avg_stats.pl Signed-off-by: Martin Renvoize Signed-off-by: Marcel de Rooy Signed-off-by: Jonathan Druart --- C4/Letters.pm | 1 - C4/Serials.pm | 15 ++------------- C4/Suggestions.pm | 19 ++++++++++--------- cataloguing/value_builder/dateaccessioned.pl | 2 +- circ/waitingreserves.pl | 2 +- .../data/mysql/atomicupdate/bug_7806.perl | 19 +++++++++++++++++++ members/memberentry.pl | 1 - misc/cronjobs/serialsUpdate.pl | 2 +- reports/issues_avg_stats.pl | 2 -- reserve/request.pl | 4 ++-- serials/subscription-add.pl | 15 +++------------ suggestion/suggestion.pl | 2 +- t/db_dependent/Circulation/dateexpiry.t | 4 ++-- 13 files changed, 42 insertions(+), 46 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_7806.perl diff --git a/C4/Letters.pm b/C4/Letters.pm index 9383b71a3f..5f1ab87a36 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -860,7 +860,6 @@ sub _parseletter { # Dates replacement my $replacedby = defined ($val) ? $val : ''; if ( $replacedby - and not $replacedby =~ m|0000-00-00| and not $replacedby =~ m|9999-12-31| and $replacedby =~ m|^\d{4}-\d{2}-\d{2}( \d{2}:\d{2}:\d{2})?$| ) { diff --git a/C4/Serials.pm b/C4/Serials.pm index 35983d67db..b277871bee 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -354,12 +354,6 @@ sub PrepareSerialsData { my $previousnote = ""; foreach my $subs (@{$lines}) { - for my $datefield ( qw(publisheddate planneddate) ) { - # handle 0000-00-00 dates - if (defined $subs->{$datefield} and $subs->{$datefield} =~ m/^00/) { - $subs->{$datefield} = undef; - } - } $subs->{ "status" . $subs->{'status'} } = 1; if ( grep { $_ == $subs->{status} } ( EXPECTED, LATE, MISSING_STATUSES, CLAIMED ) ) { $subs->{"checked"} = 1; @@ -1236,13 +1230,8 @@ sub GetNextExpected { $nextissue = $sth->fetchrow_hashref; } foreach(qw/planneddate publisheddate/) { - if ( !defined $nextissue->{$_} ) { - # or should this default to 1st Jan ??? - $nextissue->{$_} = strftime( '%Y-%m-%d', localtime ); - } - $nextissue->{$_} = ($nextissue->{$_} ne '0000-00-00') - ? $nextissue->{$_} - : undef; + # or should this default to 1st Jan ??? + $nextissue->{$_} //= strftime( '%Y-%m-%d', localtime ); } return $nextissue; diff --git a/C4/Suggestions.pm b/C4/Suggestions.pm index 11cbf96fb7..e1df70cece 100644 --- a/C4/Suggestions.pm +++ b/C4/Suggestions.pm @@ -187,20 +187,21 @@ sub SearchSuggestion { } # filter on date fields + my $dtf = Koha::Database->new->schema->storage->datetime_parser; foreach my $field (qw( suggesteddate manageddate accepteddate )) { my $from = $field . "_from"; my $to = $field . "_to"; my $from_dt; $from_dt = eval { dt_from_string( $suggestion->{$from} ) } if ( $suggestion->{$from} ); - my $from_sql = '0000-00-00'; - $from_sql = output_pref({ dt => $from_dt, dateformat => 'iso', dateonly => 1 }) - if ($from_dt); - $debug && warn "SQL for start date ($field): $from_sql"; - if ( $suggestion->{$from} || $suggestion->{$to} ) { - push @query, qq{ AND suggestions.$field BETWEEN ? AND ? }; - push @sql_params, $from_sql; - push @sql_params, - output_pref({ dt => dt_from_string( $suggestion->{$to} ), dateformat => 'iso', dateonly => 1 }) || output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }); + my $to_dt; + $to_dt = eval { dt_from_string( $suggestion->{$to} ) } if ( $suggestion->{$to} ); + if ( $from_dt ) { + push @query, qq{ AND suggestions.$field >= ?}; + push @sql_params, $dtf->format_date($from_dt); + } + if ( $to_dt ) { + push @query, qq{ AND suggestions.$field <= ?}; + push @sql_params, $dtf->format_date($to_dt); } } diff --git a/cataloguing/value_builder/dateaccessioned.pl b/cataloguing/value_builder/dateaccessioned.pl index 9479fb3f9c..c88f65cc84 100755 --- a/cataloguing/value_builder/dateaccessioned.pl +++ b/cataloguing/value_builder/dateaccessioned.pl @@ -53,7 +53,7 @@ function Click$function_name(event) { function set_to_today( id, force ) { /* The force parameter is used in Click but not in Focus ! */ if (! id) { alert(_("Bad id ") + id + _(" sent to set_to_today()")); return 0; } - if (\$("#" + id).val() == '' || \$("#" + id).val() == '0000-00-00' || force ) { + if (\$("#" + id).val() == '' || force ) { \$("#" + id).val("$date"); } } diff --git a/circ/waitingreserves.pl b/circ/waitingreserves.pl index 6db24ceb48..1a8d74ade4 100755 --- a/circ/waitingreserves.pl +++ b/circ/waitingreserves.pl @@ -89,7 +89,7 @@ my $holds = Koha::Holds->waiting->search({ priority => 0, ( $all_branches ? () : my $today = Date_to_Days(&Today); while ( my $hold = $holds->next ) { - next unless ($hold->waitingdate && $hold->waitingdate ne '0000-00-00'); + next unless $hold->waitingdate; my ( $expire_year, $expire_month, $expire_day ) = split (/-/, $hold->expirationdate); my $calcDate = Date_to_Days( $expire_year, $expire_month, $expire_day ); diff --git a/installer/data/mysql/atomicupdate/bug_7806.perl b/installer/data/mysql/atomicupdate/bug_7806.perl new file mode 100644 index 0000000000..d30776d548 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_7806.perl @@ -0,0 +1,19 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + + eval { + local $dbh->{PrintError} = 0; + $dbh->do(q| + UPDATE aqorders + SET datecancellationprinted = NULL + WHERE datecancellationprinted = '0000-00-00' + |); + $dbh->do(q| + UPDATE old_issues + SET returndate = NULL + WHERE returndate = '0000-00-00' + |); + + }; + NewVersion( $DBversion, 7806, "Remove remaining possible 0000-00-00 values"); +} diff --git a/members/memberentry.pl b/members/memberentry.pl index f08a29f877..e6be7854a0 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -210,7 +210,6 @@ if ( $op eq 'insert' || $op eq 'modify' || $op eq 'save' || $op eq 'duplicate' ) if ( $formatteddate ) { $newdata{$_} = $formatteddate; } else { - ($userdate eq '0000-00-00') and warn "Data error: $_ is '0000-00-00'"; $template->param( "ERROR_$_" => 1 ); push(@errors,"ERROR_$_"); } diff --git a/misc/cronjobs/serialsUpdate.pl b/misc/cronjobs/serialsUpdate.pl index d69cf0f6b4..758527252f 100755 --- a/misc/cronjobs/serialsUpdate.pl +++ b/misc/cronjobs/serialsUpdate.pl @@ -119,7 +119,7 @@ while ( my $issue = $sth->fetchrow_hashref ) { my $subscription = &GetSubscription( $issue->{subscriptionid} ); my $publisheddate = $issue->{publisheddate}; - if ( $subscription && $publisheddate && $publisheddate ne "0000-00-00" ) { + if ( $subscription && $publisheddate ) { my $freqdata = GetSubscriptionFrequency($subscription->{'periodicity'}); my $nextpublisheddate = GetNextDate( $subscription, $publisheddate, $freqdata ); my $today = output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }); diff --git a/reports/issues_avg_stats.pl b/reports/issues_avg_stats.pl index ca074b5dfd..7e7406af38 100755 --- a/reports/issues_avg_stats.pl +++ b/reports/issues_avg_stats.pl @@ -449,8 +449,6 @@ sub calculate { $emptycol=1 if (!defined($col)); $col = "zzEMPTY" if (!defined($col)); $row = "zzEMPTY" if (!defined($row)); - # fill returndate to avoid an error with date calc (needed for all non returned issues) - $returndate= join '-',Date::Calc::Today if $returndate eq '0000-00-00'; # DateCalc returns => 0:0:WK:DD:HH:MM:SS the weeks, days, hours, minutes, # and seconds between the two $loanlength = Delta_Days(split(/-/,$issuedate),split (/-/,$returndate)) ; diff --git a/reserve/request.pl b/reserve/request.pl index 13d0879f3f..1751a0aa92 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -205,7 +205,7 @@ if ($borrowernumber_hold && !$action) { # we check the date expiry of the borrower (only if there is an expiry date, otherwise, set to 1 (warn) my $expiry_date = $patron->dateexpiry; my $expiry = 0; # flag set if patron account has expired - if ($expiry_date and $expiry_date ne '0000-00-00' and + if ($expiry_date and Date_to_Days(split /-/,$date) > Date_to_Days(split /-/,$expiry_date)) { $expiry = 1; } @@ -250,7 +250,7 @@ if ($club_hold && !$borrowernumber_hold && !$action) { } my $expiry_date = $enrollment->patron->dateexpiry; $member->{expiry} = 0; # flag set if patron account has expired - if ($expiry_date and $expiry_date ne '0000-00-00' and + if ($expiry_date and Date_to_Days(split /-/,$date) > Date_to_Days(split /-/,$expiry_date)) { $member->{expiry} = 1; } diff --git a/serials/subscription-add.pl b/serials/subscription-add.pl index fc8d6cca4a..8e55ce05af 100755 --- a/serials/subscription-add.pl +++ b/serials/subscription-add.pl @@ -84,18 +84,9 @@ if ($op eq 'modify' || $op eq 'dup' || $op eq 'modsubscription') { print $query->redirect("/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=$subscriptionid"); } $firstissuedate = $subs->{firstacquidate} || ''; # in iso format. - for (qw(startdate firstacquidate histstartdate enddate histenddate)) { - next unless defined $subs->{$_}; - # TODO : Handle date formats properly. - if ($subs->{$_} eq '0000-00-00') { - $subs->{$_} = '' - } else { - $subs->{$_} = $subs->{$_}; - } - } - if (!defined $subs->{letter}) { - $subs->{letter}= q{}; - } + if (!defined $subs->{letter}) { + $subs->{letter}= q{}; + } my $nextexpected = GetNextExpected($subscriptionid); $nextexpected->{'isfirstissue'} = $nextexpected->{planneddate} eq $firstissuedate ; $subs->{nextacquidate} = $nextexpected->{planneddate} if($op eq 'modify'); diff --git a/suggestion/suggestion.pl b/suggestion/suggestion.pl index 4f491962dc..64b4106b2d 100755 --- a/suggestion/suggestion.pl +++ b/suggestion/suggestion.pl @@ -39,7 +39,7 @@ use URI::Escape; sub Init{ my $suggestion= shift @_; # "Managed by" is used only when a suggestion is being edited (not when created) - if ($suggestion->{'suggesteddate'} eq "0000-00-00" ||$suggestion->{'suggesteddate'} eq "") { + if ($suggestion->{'suggesteddate'} eq "") { # new suggestion $suggestion->{suggesteddate} = dt_from_string; $suggestion->{'suggestedby'} = C4::Context->userenv->{"number"} unless ($suggestion->{'suggestedby'}); diff --git a/t/db_dependent/Circulation/dateexpiry.t b/t/db_dependent/Circulation/dateexpiry.t index ca25f75254..9a668947e1 100755 --- a/t/db_dependent/Circulation/dateexpiry.t +++ b/t/db_dependent/Circulation/dateexpiry.t @@ -105,9 +105,9 @@ sub calc_date_due { # first test with empty expiry date # note that this expiry date will never lead to an issue btw !! - $patron->{dateexpiry} = '0000-00-00'; + $patron->{dateexpiry} = undef; my $d = C4::Circulation::CalcDateDue( $today, $item->effective_itemtype, $branch->{branchcode}, $patron ); - is( ref $d eq "DateTime" && $d->mdy() =~ /^\d+/, 1, "CalcDateDue with expiry 0000-00-00" ); + is( ref $d eq "DateTime" && $d->mdy() =~ /^\d+/, 1, "CalcDateDue with expiry undef" ); # second test expiry date==today my $d2 = output_pref( { dt => $today, dateonly => 1, dateformat => 'sql' } ); -- 2.39.5