Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#11940 closed (fixed)

ModelForm evaluates callable default values on form class creation

Reported by: Harm Geerts <hgeerts@…> Owned by: lukeplant
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@…> 6 years ago.

Download all attachments as: .zip

Change History (9)

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

comment:1 Changed 6 years ago by anonymous

  • Cc dev@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 6 years ago by russellm

  • milestone set to 1.2
  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 6 years ago by jpaulett

  • Cc john@… added

comment:4 Changed 5 years ago by jacob

  • Triage Stage changed from Accepted to Ready for checkin

comment:5 Changed 5 years ago by lukeplant

  • Owner changed from nobody to lukeplant
  • Status changed from new to assigned

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 5 years ago by lukeplant

  • Resolution set to fixed
  • Status changed from assigned to closed

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

(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 4 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

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