Opened 7 years ago

Closed 7 years ago

Last modified 5 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: master
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: UI/UX:

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@…> 7 years ago.

Download all attachments as: .zip

Change History (9)

Changed 7 years ago by Harm Geerts <hgeerts@…>

comment:1 Changed 7 years ago by anonymous

Cc: dev@… added

comment:2 Changed 7 years ago by Russell Keith-Magee

milestone: 1.2
Triage Stage: UnreviewedAccepted

comment:3 Changed 7 years ago by John Paulett

Cc: john@… added

comment:4 Changed 7 years ago by Jacob

Triage Stage: AcceptedReady for checkin

comment:5 Changed 7 years ago by Luke Plant

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 Changed 7 years ago by Luke Plant

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 Changed 7 years ago by Luke Plant

(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 Changed 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

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