Django

Code

Ticket #7499 (closed: fixed)

Opened 6 months ago

Last modified 3 months ago

TimeField may contain the sub-second times but cannot validate them

Reported by: honeyman Assigned to: kevin
Milestone: 1.0 Component: Forms
Version: SVN Keywords: time aug22sprint
Cc: kevin.mcconnell@gmail.com Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

If we create a form containing a TimeField? which has the initial value set to time object which contains the sub-second data (for example, to time.max), the form is generated with the value which cannot be validated. Example:

>>> from django import newforms as forms
>>> from datetime import time
>>>
>>> class MyForm(forms.Form):
...     mytime = forms.TimeField(initial = time.max)
...
>>> f1 = MyForm()
>>> # Imagine we've posted this form and are now parsing the POST request
... f1a = MyForm({'mytime': str(time.max)})
>>>
>>> print f1
<tr><th><label for="id_mytime">Mytime:</label></th><td><input type="text" name="mytime" value="23:59
:59.999999" id="id_mytime" /></td></tr>
>>>
>>> print f1a
<tr><th><label for="id_mytime">Mytime:</label></th><td><ul class="errorlist"><li>Enter a valid time.
</li></ul><input type="text" name="mytime" value="23:59:59.999999" id="id_mytime" /></td></tr>
>>>
>>> print f1a.is_bound, f1a.is_valid()
True False

As Django (Python) cannot parse the sub-second time in the text string, my understanding is that Django should never print it in the field, trimming the value to just '%H:%M:%S'.

This is likely applicable to DateTimeField? too.

Attachments

time_input_widget.diff (3.7 kB) - added by kevin on 08/22/08 19:43:22.
Adds a TimeInput? widget
time_input_widget.2.diff (3.5 kB) - added by kevin on 08/22/08 20:07:49.
Updated patch
time_input_widget.3.diff (1.3 kB) - added by kevin on 08/23/08 21:53:08.
Check TimeInput? argument type (patched against r8507)

Change History

08/08/08 16:22:23 changed by ericholscher

  • needs_better_patch changed.
  • needs_docs changed.
  • stage changed from Unreviewed to Design decision needed.
  • needs_tests changed.
  • milestone set to 1.0 maybe.

08/22/08 19:04:57 changed by jacob

  • stage changed from Design decision needed to Accepted.
  • milestone changed from 1.0 maybe to 1.0.

08/22/08 19:08:56 changed by kevin

  • owner changed from nobody to kevin.
  • status changed from new to assigned.

08/22/08 19:43:22 changed by kevin

  • attachment time_input_widget.diff added.

Adds a TimeInput? widget

08/22/08 19:46:08 changed by kevin

  • cc set to kevin.mcconnell@gmail.com.
  • keywords changed from time to time aug22sprint.
  • has_patch set to 1.

The patch adds a TimeInput widget, which functions similarly to DateTimeInput but with a suitable format string.

08/22/08 20:07:49 changed by kevin

  • attachment time_input_widget.2.diff added.

Updated patch

08/22/08 20:10:33 changed by kevin

After discussion with Malcolm, I've updated the patch to truncate the time down to seconds (using replace(microsecond=0)) rather than using the format string (which wasn't quite so explicit).

08/23/08 12:34:28 changed by mtredinnick

  • status changed from assigned to closed.
  • resolution set to fixed.

Fixed in [8491].

08/23/08 19:28:28 changed by honeyman

  • status changed from closed to reopened.
  • resolution deleted.

I almost forgot about this ticket, but it hit me again just now... After an "svn update" of Django, I just got "Caught an exception while rendering: replace() takes no keyword arguments" in /forms/widgets.py, line 311, in render "value = value.replace(microsecond=0)". Seems that I was trying to initialize this widget using an unicode string (rather than a datetime.time), and it worked perfectly before; but now it fails (cause string object also does have a replace() method, with a different syntax though). Should it be considered a regression, or does such widget now have to be initialized with datetime.time since now?

08/23/08 21:51:20 changed by kevin

That's a good point, I didn't think about directly instantiating the widgets like that.

Perhaps it would be safer to check the type of the argument (since replace could be a common method name)? I know some folks have strong feelings about doing that in Python, but it seems like it would be safer.

I'll attach a new patch that does that.

08/23/08 21:53:08 changed by kevin

  • attachment time_input_widget.3.diff added.

Check TimeInput? argument type (patched against r8507)

08/24/08 13:12:08 changed by mtredinnick

  • stage changed from Accepted to Ready for checkin.

08/25/08 14:09:45 changed by jacob

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [8549]) Updated TimeInput changes from [8491] to allow time widgets to be used with unicode values. Fixes #7499.


Add/Change #7499 (TimeField may contain the sub-second times but cannot validate them)




Change Properties
Action