Opened 3 years ago

Closed 23 months ago

Last modified 23 months ago

#18709 closed Bug (fixed)

Formset with SplitDateTimeField(initial=datetime.datetime.now) doesn't work correctly

Reported by: dekkers Owned by: Jeroen Dekkers <jeroen@…>
Component: Forms Version: 1.4
Severity: Normal Keywords:
Cc: jason@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The following code doesn't work (it's included as a test case in the patch):

class SplitDateTimeForm(forms.Form):
    when = forms.SplitDateTimeField(initial=datetime.datetime.now)

SplitDateTimeFormSet = forms.formsets.formset_factory(SplitDateTimeForm)

data = {
    'form-TOTAL_FORMS': '1',
    'form-INITIAL_FORMS': '0',
    'form-0-when_0': '1904-06-16',
    'form-0-when_1': '15:51:33',
}
formset = SplitDateTimeFormSet(data)
formset.is_valid()

It gives the following stack trace:

Traceback (most recent call last):
  File "/home/jeroen/github/django/tests/regressiontests/forms/tests/formsets.py", line 870, in test_formset_splitdatetimefield
    self.assertTrue(formset.is_valid())
  File "/home/jeroen/github/django/django/forms/formsets.py", line 271, in is_valid
    err = self.errors
  File "/home/jeroen/github/django/django/forms/formsets.py", line 249, in _get_errors
    self.full_clean()
  File "/home/jeroen/github/django/django/forms/formsets.py", line 292, in full_clean
    self._errors.append(form.errors)
  File "/home/jeroen/github/django/django/forms/forms.py", line 116, in _get_errors
    self.full_clean()
  File "/home/jeroen/github/django/django/forms/forms.py", line 269, in full_clean
    if self.empty_permitted and not self.has_changed():
  File "/home/jeroen/github/django/django/forms/forms.py", line 324, in has_changed
    return bool(self.changed_data)
  File "/home/jeroen/github/django/django/forms/forms.py", line 347, in _get_changed_data
    if field.widget._has_changed(initial_value, data_value):
  File "/home/jeroen/github/django/django/forms/widgets.py", line 847, in _has_changed
    initial = self.decompress(initial)
  File "/home/jeroen/github/django/django/forms/widgets.py", line 897, in decompress
    return [value.date(), value.time().replace(microsecond=0)]
AttributeError: 'builtin_function_or_method' object has no attribute 'date'

The problem is that _get_changed_data doesn't check whether initial is a callable and calls the _has_changed widget method with the callable as initial_value. Because the default _has_changed does a force_unicode before comparing, we never get an error for the simple widgets as force_unicode simply returns
u'<built-in method now of type object at 0x7f77b3be7c40>', so it just always returns True for such fields. Any widget that does something more with initial_value will fail however.

The patch fixes _get_changed_data to call the initial value when it is a callable.

Change History (10)

comment:1 Changed 3 years ago by dekkers

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 3 years ago by jwmayfield

  • Cc jason@… added
  • Owner changed from nobody to jwmayfield
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

Patch applies cleanly. Test fails before applying patch. Test passes after patch.

comment:3 Changed 3 years ago by jwmayfield

  • Triage Stage changed from Accepted to Ready for checkin

comment:4 Changed 3 years ago by jwmayfield

  • Owner jwmayfield deleted
  • Status changed from assigned to new

comment:5 Changed 23 months ago by galuszkak

It will be very nice to merge because it is breaking pull request for ticket #19019

comment:6 Changed 23 months ago by aaugustin

The patch looks good, but unfortunately it doesn't apply cleanly any longer.

If you bring it up to date, you can mark it as ready for checkin.

comment:7 Changed 23 months ago by aaugustin

  • Triage Stage changed from Ready for checkin to Accepted

comment:8 Changed 23 months ago by dekkers

  • Triage Stage changed from Accepted to Ready for checkin

I've rebased the commit so it applies again.

comment:9 Changed 23 months ago by Jeroen Dekkers <jeroen@…>

  • Owner set to Jeroen Dekkers <jeroen@…>
  • Resolution set to fixed
  • Status changed from new to closed

In d0788c277035727b7b070abd0f02d075acffc84f:

Fixed #18709 -- Check if initial_value is a callable

In _get_changed_data, check if initial_value is a callable and call it
if it is.

comment:10 Changed 23 months ago by Marc Tamlyn <marc.tamlyn@…>

In adeec00979d1b365535b8f26fda4a5f7173e975d:

Merge pull request #246 from dekkers/ticket_18709

Fixed #18709 -- Check if initial_value is a callable

Note: See TracTickets for help on using tickets.
Back to Top