From 0ddb66fd577841270900f47b3d4871db729ff31f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20V=C3=A9ron?= Date: Thu, 5 Nov 2015 23:51:50 +0100 Subject: [PATCH] Bug 14969: (followup) Handle invalid date input This patch fixes software errors if invalid dates are given while adding or editing a subscription (see comment #5). Subscriptions with invalid dates will be saved with empty dates. A warning will appear on the subscription detail page. This will be a rare case because normally datepickers are used for date entry. Notes: - Before C4::Dates removal, wrong dates like 33/33/2033 did not complain, they created valid but fantasy dates in the future. - A more sophisticated date validation can be implemented in a separate bug after completion of C4::Dates removal. To test: - Create and edit subscriptions with valid dates, verify that subscriptions are correctly defined - Bypass datepickers by manually entering dates like 00/00/0000 or 33/33/2999, verify that subscriptions are saved, but a warning appears on the subscription's detail page. --- .../prog/en/modules/serials/subscription-detail.tt | 3 ++ serials/subscription-add.pl | 48 ++++++++++++++++---- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-detail.tt index dc0e246..948bd4f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-detail.tt @@ -43,6 +43,9 @@ $(document).ready(function() { [% INCLUDE 'serials-toolbar.inc' %]

Subscription for [% bibliotitle%] [% IF closed %](closed)[% END %]

+ [% IF ! ( startdate && enddate && firstacquidate ) %] +
One ore more date(s) not properly defined. Please edit subscription to check/define them.
+ [% END %] [% IF ( abouttoexpire ) %] [% UNLESS closed %]
Subscription will expire [% enddate %]. Renew this subscription.
diff --git a/serials/subscription-add.pl b/serials/subscription-add.pl index 8795438..4385e91 100755 --- a/serials/subscription-add.pl +++ b/serials/subscription-add.pl @@ -15,6 +15,11 @@ # You should have received a copy of the GNU General Public License # along with Koha; if not, see . +# FIXME: Processing of invalid dates (00/00/0000 or 33/33/2999) needs +# some more attention. At the moment subscription will be saved with +# empty dates, subscription detail page will display a warning if +# startdate and/or enddate are empty. + use strict; use warnings; @@ -324,22 +329,43 @@ sub redirect_add_subscription { my $skip_serialseq = $query->param('skip_serialseq'); my $startdate = $query->param('startdate'); - $startdate = eval { dt_from_string( $startdate ); } if ( $startdate ); - $startdate = output_pref( { dt => $startdate, dateonly => 1, dateformat => 'iso' } ) if ( $startdate ); + my $startdate_dt; + $startdate_dt = eval { dt_from_string( $startdate ); } if ( $startdate ); + if ( $startdate_dt ) { + $startdate = output_pref( { dt => $startdate_dt, dateonly => 1, dateformat => 'iso' } ); + } + else { + $startdate = output_pref( { dt => dt_from_string, dateonly => 1, dateformat => 'iso' } ); + } my $enddate = $query->param('enddate'); - $enddate = eval { dt_from_string( $enddate ); } if ( $enddate ); - $enddate = output_pref( { dt => $enddate, dateonly => 1, dateformat => 'iso' } ) if ( $enddate ); + my $enddate_dt; + $enddate_dt = eval { dt_from_string( $enddate ); } if ( $enddate ); + if ( $enddate_dt ) { + $enddate = output_pref( { dt => $enddate_dt, dateonly => 1, dateformat => 'iso' } ); + } + else { + $enddate = undef; + } + my $firstacquidate = $query->param('firstacquidate'); - $firstacquidate = eval { dt_from_string( $firstacquidate ); } if ( $firstacquidate ); - $firstacquidate = output_pref( { dt => $enddate, dateonly => 1, dateformat => 'iso' } ) if ( $firstacquidate ); + my $firstacquidate_dt; + $firstacquidate_dt = eval { dt_from_string( $firstacquidate ); } if ( $firstacquidate ); + if ($firstacquidate_dt ) { + $firstacquidate = output_pref( { dt => $firstacquidate_dt, dateonly => 1, dateformat => 'iso' } ); + } + else { + $firstacquidate = undef; + } if(!defined $enddate || $enddate eq '') { if($subtype eq "issues") { - $enddate = _guess_enddate($firstacquidate, $periodicity, $numberlength, $weeklength, $monthlength); + $enddate = eval { _guess_enddate($firstacquidate, $periodicity, $numberlength, $weeklength, $monthlength); }; } else { - $enddate = _guess_enddate($startdate, $periodicity, $numberlength, $weeklength, $monthlength); + $enddate = eval { _guess_enddate($startdate, $periodicity, $numberlength, $weeklength, $monthlength); }; } + #FIXME We set $enddate to empty string if there was a problem with guessing; + $enddate = ''; } my $subscriptionid = NewSubscription( @@ -418,10 +444,12 @@ sub redirect_mod_subscription { # Guess end date if(!defined $enddate || $enddate eq '') { if($subtype eq "issues") { - $enddate = _guess_enddate($nextacquidate, $periodicity, $numberlength, $weeklength, $monthlength); + $enddate = eval { _guess_enddate($nextacquidate, $periodicity, $numberlength, $weeklength, $monthlength); }; } else { - $enddate = _guess_enddate($startdate, $periodicity, $numberlength, $weeklength, $monthlength); + $enddate = eval {_guess_enddate($startdate, $periodicity, $numberlength, $weeklength, $monthlength); }; } + #FIXME We set $enddate totoday if it is empty; + $enddate = output_pref( { dt => dt_from_string, dateonly => 1, dateformat => 'iso' } ) unless ( $enddate ); } my $nextexpected = GetNextExpected($subscriptionid); -- 1.7.10.4