Opened 15 years ago

Closed 14 years ago

Last modified 13 years ago

#11940 closed (fixed)

ModelForm evaluates callable default values on form class creation

Reported by: Harm Geerts <hgeerts@…> Owned by: Luke Plant
Component: Forms Version: dev
Severity: Keywords:
Cc: dev@…, john@… 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

Using a ModelForm in combination with a field that defines a callable default value will cause the callable to be evaluated on form class creation.

class TestModel(models.Model):
    datetime = models.DateTimeField(default=datetime.datetime.now)

class TestModelForm(forms.ModelForm):
    class Meta:
        model = TestModel
>>> form = TestModelForm()
>>> print form['datetime']
<input type="text" name="datetime" value="2009-09-23 16:49:34" id="id_datetime" /><input type="hidden" name="initial-datetime" value="2009-09-23 16:49:34.561971" id="id_datetime" />
>>> # wait a bit
>>> form = TestModelForm()
>>> print form['datetime']
<input type="text" name="datetime" value="2009-09-23 16:49:34" id="id_datetime" /><input type="hidden" name="initial-datetime" value="2009-09-23 16:49:34.561971" id="id_datetime" />

The attached patch will fix this and preserve the callable default. It also implements Field.initial as a property that will evaluate callables when called.
I'm not sure this is desired but I found a case in the modeltests that depended on initial having a real value and I can imagine there is more code out there that expects this behaviour (in fact my regression test now depends on it). I'll leave it up to you to decide what to do with it.

The test case that depends on initial having a real value is in modeltests/model_formsets/models.py:886

Attachments (1)

modelform-callable-default.patch (3.1 KB ) - added by Harm Geerts <hgeerts@…> 15 years ago.

Download all attachments as: .zip

Change History (9)

by Harm Geerts <hgeerts@…>, 15 years ago

comment:1 by anonymous, 15 years ago

Cc: dev@… added

comment:2 by Russell Keith-Magee, 14 years ago

milestone: 1.2
Triage Stage: UnreviewedAccepted

comment:3 by John Paulett, 14 years ago

Cc: john@… added

comment:4 by Jacob, 14 years ago

Triage Stage: AcceptedReady for checkin

comment:5 by Luke Plant, 14 years ago

Owner: changed from nobody to Luke Plant
Status: newassigned

I don't think the 'initial' property is a good idea - I really don't like this behaviour:

a_field.initial != a_field.initial

It really violates expectations, and is likely to promote the same problem into other code paths i.e. eager evaluation of the initial value, instead of the ability to propagate the callable. Changing the test seems like a much better approach. This approach may lead to a potential backwards incompatibility, but I'm convinced it is very low impact (not worth noting anywhere). The one test which needs fixing is very unlike normal code or test code, and had similar code been written in a more normal context the developer would (or should) have realised that they had come across this very problem.

So I'll commit shortly with a slightly different patch.

comment:6 by Luke Plant, 14 years ago

Resolution: fixed
Status: assignedclosed

(In [12721]) Fixed #11940 - ModelForm evaluates callable default values on form class creation

Thanks to Harm Geerts for the report and initial patch.

comment:7 by Luke Plant, 14 years ago

(In [12722]) [1.1.X] Fixed #11940 - ModelForm evaluates callable default values on form class creation

Thanks to Harm Geerts for the report and initial patch.

Backport of [12721] from trunk

comment:8 by Jacob, 13 years ago

milestone: 1.2

Milestone 1.2 deleted

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