Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#3632 closed (wontfix)

initial values lost when calling newforms.form_for_instance() with a custom form arg

Reported by: ludo@… Owned by: thejaswi_puthraya
Component: Forms Version: master
Severity: Keywords: form_for_instance, decsprint01
Cc: gary.wilson@…, mtrier@…, larlet@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Initial values are lost when calling newforms.form_for_instance() with a custom form arg, eg

f = forms.form_for_instance(u) is ok

f = forms.form_for_instance(u, form=MyCustomForm) is not

The problem seems to be in DeclarativeFieldsMetaclass.new where the 'base_fields' attribute gets wiped out when using a custom form class. A tentative patch, probably broken as my understanding of metaclass programming is nil, attached to this ticket.

Attachments (6)

forms.diff (1.7 KB) - added by ludo@… 8 years ago.
tentative patch to newforms/forms.py
forms.2.diff (720 bytes) - added by ludo@… 8 years ago.
revised patch
forms.py (808 bytes) - added by ludo@… 8 years ago.
this one seems to work satisfactorily
forms.py.diff (800 bytes) - added by empty <mtrier@…> 7 years ago.
Updated patch against trunk 6781
forms.py.2.diff (792 bytes) - added by empty <mtrier@…> 7 years ago.
Updated because SortedDictFromList was removed
forms.py.3.diff (2.6 KB) - added by empty <mtrier@…> 7 years ago.
Patch with tests

Download all attachments as: .zip

Change History (17)

Changed 8 years ago by ludo@…

tentative patch to newforms/forms.py

Changed 8 years ago by ludo@…

revised patch

comment:1 Changed 8 years ago by ludo@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

None of the patches work, as the fields in both the custom form class and the one generated in form_for_instance() are the same.

Changed 8 years ago by ludo@…

this one seems to work satisfactorily

comment:2 Changed 8 years ago by ludo@…

  • Summary changed from initial values lost when calling newforms.form_for_instance() with a custom form arg [patch] to [patch] initial values lost when calling newforms.form_for_instance() with a custom form arg

comment:3 Changed 8 years ago by Simon G. <dev@…>

  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 8 years ago by Gary Wilson <gary.wilson@…>

  • Cc gary.wilson@… added

comment:5 Changed 7 years ago by anonymous

  • Cc mtrier@… added

Changed 7 years ago by empty <mtrier@…>

Updated patch against trunk 6781

comment:6 Changed 7 years ago by david

  • Cc larlet@… added

comment:7 Changed 7 years ago by thejaswi_puthraya

  • Owner changed from nobody to thejaswi_puthraya

Changed 7 years ago by empty <mtrier@…>

Updated because SortedDictFromList was removed

comment:8 Changed 7 years ago by thejaswi_puthraya

  • Keywords decsprint01 added
  • Triage Stage changed from Accepted to Ready for checkin

works with the patch of empty (Michael Trier).

Changed 7 years ago by empty <mtrier@…>

Patch with tests

comment:9 Changed 7 years ago by Simon G <dev@…>

  • Summary changed from [patch] initial values lost when calling newforms.form_for_instance() with a custom form arg to initial values lost when calling newforms.form_for_instance() with a custom form arg

comment:10 Changed 7 years ago by mtredinnick

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

I don't think this is worth fixing. It adds some extra code for a case that is better handled in another way (we already take base_fields from parent classes, so this introduces a *third* way of altering base_fields; it's getting a bit out of hand).

#6337 fixes the edge pieces of this for ModelForms, so the fix for this problem is "use ModelForms", rather than us adding some extra code that would only be used by form_for_instance() and thus is deprecated the moment it lands in trunk.

Sorry if that seems a little harsh for those wanting this feature, but it's a matter of trying to only introduce code that will live a long and healthy life.

comment:11 Changed 7 years ago by mtredinnick

(In [7112]) Fixed #6337. Refs #3632 -- Fixed ModelForms subclassing, to the extent that it can be made to work.

This ended up being an almost complete rewrite of ModelForms.new, but
should be backwards compatible (although the text of one error message has
changed, which is only user visible and only if you pass in invalid code).

Documentation updated, also.

This started out as a patch from semenov (many thanks!), but by the time all
the problems were hammered out, little of the original was left. Still, it was
a good starting point.

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