Code

Opened 8 years ago

Closed 6 years ago

#899 closed defect (fixed)

Formfield doesn't display default value

Reported by: ross@… Owned by: PhiR
Component: Forms Version: master
Severity: normal Keywords: form_for_model, sprintsept14
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

If my model contains this:

currency = meta.CharField(maxlength=4,default="Foo")

And a form template does this:

{{form.currency}}

I'd expect the resulting input widget to be pre-initialised to "Foo". However, it isn't.

Attachments (3)

initial_formfield.patch (6.3 KB) - added by antisvin@… 7 years ago.
Use get_default model fields method to set initial values for forms.
django-use-model-default-as-newforms-initial.diff (1.1 KB) - added by David Danier <goliath.mailinglist@…> 7 years ago.
This should do it for current trunk (formfield() nicely refactored using super())
899.diff (2.0 KB) - added by PhiR 7 years ago.
same patch against trunk, but with the tests requested by Malcolm

Download all attachments as: .zip

Change History (16)

comment:1 Changed 8 years ago by anonymous

  • Component changed from Template system to Metasystem

comment:2 Changed 8 years ago by adrian

  • Resolution set to wontfix
  • Status changed from new to closed

The formfields framework is decoupled from models.

comment:3 Changed 7 years ago by antisvin@…

  • Component changed from Metasystem to django.newforms
  • Has patch set
  • Needs tests set
  • Resolution wontfix deleted
  • Status changed from closed to reopened
  • Version set to SVN

Now that there's formfield method especially for generating forms from model fields, initial values can be easily obtained from model.Obvious patch attached.

Changed 7 years ago by antisvin@…

Use get_default model fields method to set initial values for forms.

comment:4 Changed 7 years ago by SmileyChris

  • Triage Stage changed from Unreviewed to Ready for checkin

comment:5 Changed 7 years ago by Brian Rosner <brosner@…>

Marked #4019 as duplicate of this. It appears the patch on #4019 is essentially the same thing as this one.

comment:6 Changed 7 years ago by David Danier <goliath.mailinglist@…>

Sorry for the duplicate, I really didn't see this one year old ticket.

About the patches:
If I read ModelField.get_default() right this returns "" (empty string) as an last option, even if no default is provided. ModelField.get_default() actually generates some default if no default is provided. That's why I tested Field.has_default() in my code. Even if this has no real consequence in HTML-forms (value="" is the default that gets sent by the browser if no input is made), it only seemed unclean to me, because FormField.initial is different to ModelField.default, but it looks the same and may lead to confusion or mistakes (when working with the forms outside of some generic views/the admin).

comment:7 Changed 7 years ago by mtredinnick

  • Triage Stage changed from Ready for checkin to Accepted

This looks like the right approach for newforms, but it needs tests before it can be committed. Doesn't need to test every field, but it should show how the functionality works for a couple of examples so that we can verify we don't break it.

In particular, I would like to see that in the case of form_for_instance() constructing the form, we don't use the default values, but, rather, use the existing values from the instance attributes, whatever they may be. So having tests for a form_for_model() and a form_for_instance() case would be the minimu required here.

Changed 7 years ago by David Danier <goliath.mailinglist@…>

This should do it for current trunk (formfield() nicely refactored using super())

comment:8 Changed 7 years ago by PhiR

  • Owner changed from nobody to PhiR
  • Status changed from reopened to new

Changed 7 years ago by PhiR

same patch against trunk, but with the tests requested by Malcolm

comment:9 Changed 7 years ago by PhiR

  • Keywords form_for_model added
  • Needs tests unset
  • Status changed from new to assigned

I added regression tests to make sure we get the expected behavior in both form_for_model (default from the class's field) and form_for_instance (instance value).

comment:10 Changed 7 years ago by George Vilches <gav@…>

  • Keywords form_for_model, sprintsept14 added; form_for_model removed

comment:11 Changed 7 years ago by PhiR

#4967 is a duplicate, closed in favor of this one

comment:12 Changed 7 years ago by ubernostrum

  • Triage Stage changed from Accepted to Ready for checkin

A user on IRC reports that the patch works and tests pass, so marking ready for checkin.

comment:13 Changed 6 years ago by mtredinnick

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

(In [6568]) Fixed #899 -- Use model field default values as formfield initial values in
form_for_model(). Patch from David Danier and PhiR. Thanks.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.