launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/dumb-date-formats into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #134063 OOPS registering sprint or meeting using date format DD-M-YYYY
  https://bugs.launchpad.net/bugs/134063

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/dumb-date-formats/+merge/50385

Summary
=======

On the new sprint page, an OOPS can be triggered when dates that zope.date.parse can't understand get entered. Rather than throwing back an error, parse munges the dates into a format it can deal with, resulting in data that throws of db constraints.

This branch updates the widget to check if the date string is in any supported format, and kicks back an error message (rather than an OOPS) if it isn't.

Preimplementation
=================

Spoke with Curtis Hovey

Proposed Fix
============

Update the widget so it kicks back the input *before* the widget converts it into an erroneous datetime object.

Implementation
==============

The DateTimeWidget in lp.apps.widgets.date is provided with a new attribute, supported_input_formats, which is a list of format strings for datetime.strptime. 

It also has a new method, _checkSupportedFormat, which uses the list of format strings to see if a meaningful datetime can be extracted from the input. If not, an error is raised.

Tests
=====

bin/test -m lp.app.widgets.tests

Demo & QA
=========

Try to enter the dates listed in the bug; they should come back with errors, rather than triggering an OOPS.

Lint
=====

make lint output:

= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
  lib/lp/app/widgets/date.py
-- 
https://code.launchpad.net/~jcsackett/launchpad/dumb-date-formats/+merge/50385
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/dumb-date-formats into lp:launchpad.

=== modified file 'lib/lp/app/widgets/date.py'
--- lib/lp/app/widgets/date.py	2011-02-01 21:01:02 +0000
+++ lib/lp/app/widgets/date.py	2011-02-18 19:55:00 +0000
@@ -109,6 +109,15 @@
     """
 
     timeformat = '%Y-%m-%d %H:%M:%S'
+    supported_input_formats = [
+        '%Y-%m-%d %H:%M:%S%z',
+        '%Y-%m-%d %H:%M:%S',
+        '%Y-%m-%d',
+        '%m-%d-%Y %H:%M:%S%z',
+        '%m-%d-%Y %H:%M:%S',
+        '%m-%d-%Y',
+        ]
+
     required_time_zone = None
     display_zone = True
     from_date = None
@@ -306,6 +315,35 @@
             raise self._error
         return value
 
+    def _checkSupportedFormat(self, input):
+        """Checks that the input is in a usable format.
+
+          >>> from zope.publisher.browser import TestRequest
+          >>> from zope.schema import Field
+          >>> field = Field(__name__='foo', title=u'Foo')
+          >>> widget = DateTimeWidget(field, TestRequest())
+
+        Dates in unsupported formats result in a ConversionError:
+
+          >>> widget._checkSupportedFormat('15-5-2010')  #doctest: +ELLIPSIS
+          Traceback (most recent call last):
+            ...
+          ConversionError: ('Invalid date value', ...)
+
+        """
+        for fmt in self.supported_input_formats:
+            try:
+                datetime.strptime(input, fmt)
+            except (ValueError), e:
+                if 'unconverted data remains' in e.message:
+                    return
+                else:
+                    failure = e
+            else:
+                return
+        if failure:
+            raise ConversionError('Invalid date value', failure)
+
     def _toFieldValue(self, input):
         """Return parsed input (datetime) as a date."""
         return self._parseInput(input)
@@ -346,6 +384,7 @@
         """
         if input == self._missing:
             return self.context.missing_value
+        self._checkSupportedFormat(input)
         try:
             year, month, day, hour, minute, second, dummy_tz = parse(input)
             second, micro = divmod(second, 1.0)
@@ -560,6 +599,7 @@
 
 class DatetimeDisplayWidget(DisplayWidget):
     """Display timestamps in the users preferred time zone"""
+
     def __call__(self):
         time_zone = getUtility(ILaunchBag).time_zone
         if self._renderedValueSet():