Opened 10 years ago

Closed 9 years ago

#3815 closed (wontfix)

form_for_* methods should be able to use a Form superclass

Reported by: jon.i.austin@… Owned by: nobody
Component: Forms Version: master
Severity: Keywords: newforms
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

hi,

I'm not sure if this is a bug or not, but according to the newforms docs, one should be able to subclass an existing form class and add fields to it.
This works just fine if the ancestor form class is explicitly defined in the code.

However if one creates a new Form class using newforms.form_for_model(MyModelClass) and tries to subclass the resulting class it doesn't work.
i.e. using the Polls model from the tutorial:

from polls import models
from django.newforms import form_for_model
PollForm = form_for_model(models.Poll)

class PollFormWithTest(PollForm):
   ....:     test = charField()

testPollWithTest = PollFormWithTest()

print testPollWithTest.fields
{'question': <django.newforms.fields.CharField object at 0xb745b62c>, 'pub_date': <django.newforms.fields.DateTimeField object at 0xb745b58c>}

print testPollWithTest.test
<django.newforms.fields.CharField object at 0xb745b52c>

as you can see test is added as an base attribute to the instance, not as a field.

Is this perhaps because the PollForm is an instance of a class and not an actual class definition?
If so is there a way around this, and could it perhaps be added to the docs?

Also: if the above is the case the docstring for newforms.form_for_model should probably be changed:
Returns a Form class for the given Django model class.
to: s/a Form class/an instance of a Form class/

thanks!

jon

Attachments (3)

subclass_form.diff (1.3 KB) - added by Chris Beaven 9 years ago.
subclass_form.2.diff (1.4 KB) - added by Chris Beaven 9 years ago.
safer: makes sure base_fields is a dict instance
subclass_form.3.diff (1.6 KB) - added by Chris Beaven 9 years ago.
another test

Download all attachments as: .zip

Change History (11)

comment:1 Changed 10 years ago by Chris Beaven

Resolution: invalid
Status: newclosed

Try form_for_model(models.Poll, form=Form) (BaseForm is the default superclass for these methods)

comment:2 Changed 10 years ago by jon austin <jon.i.austin@…>

ah, thanks much, I should have looked closer at the django code, where it explicitly states Form adds the fields attribute. lesson learned. :)

oh, and if anyone else reads this -- form_for_model does of course return a class not an instance ... as I would have seen had i studied the form_for_model code a bit...

comment:3 Changed 9 years ago by matt@…

Resolution: invalid
Status: closedreopened

I might be missing something, but this no longer seems to work.

Using revision 5808, form_for_model(models.Poll, form=Form) seems to remove all the fields ordinarily generated by form_for_model. When you subclass it, additional fields are added correctly, but obviously this defeats the purpose of subclassing in the first place.

Details and an example are at [http://groups.google.com/group/django-users/browse_thread/thread/b1e0ba0406f51da/62adfe8729ead1fd?lnk=gst&q=subclassing+form_for_model&rnum=1#62adfe8729ead1fd
]

In the mean time, this seems to work:

ModelForm = forms.models.form_for_model(Model)

class ModelFormWithName(ModelForm):
    ModelForm.base_fields['name'] = forms.CharField(max_length=31,widget=forms.TextInput(), label='Name')

comment:4 Changed 9 years ago by Chris Beaven

Triage Stage: UnreviewedDesign decision needed

It used to work back in the day... I guess the meta class magic changed between then and now. I just tried this after applying my patch in #5050 and it works, so I'll keep this open as a Design Decision for now.

Changed 9 years ago by Chris Beaven

Attachment: subclass_form.diff added

Changed 9 years ago by Chris Beaven

Attachment: subclass_form.2.diff added

safer: makes sure base_fields is a dict instance

Changed 9 years ago by Chris Beaven

Attachment: subclass_form.3.diff added

another test

comment:5 Changed 9 years ago by Chris Beaven

Has patch: set
Summary: subclassed newforms does not work with ancestor class created with form_for_model(ModelClass)form_for_* methods should be able to use a Form superclass
Triage Stage: Design decision neededReady for checkin

This ticket has mutated to a different issue slightly, but I'll keep it open and just re-summarize it.

Using a Form (or subclass of Form) as a superclass should work fine. My patch is simple and adds this functionality, with no possible side effects that I can see

Promoting to checkin.

comment:6 Changed 9 years ago by Malcolm Tredinnick

Triage Stage: Ready for checkinAccepted

Given the plan to deprecate/remove form_for_* based on Joseph's new work with ModelForm classes, I'm going to punt this for the time being. We can close or re-promote once a resolution has been reached on the other thing.

comment:7 Changed 9 years ago by Brian Rosner

ModelForm has been committed to trunk. I would think this ticket can be consider fixed?

comment:8 Changed 9 years ago by Brian Rosner

Resolution: wontfix
Status: reopenedclosed

Considering form_for_* are now replaced by ModelForm this is no longer needed. You can accomplish inheritance with ModelForm as of r7112.

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