Django

Code

Ticket #3489 (closed: fixed)

Opened 2 years ago

Last modified 1 year ago

Altering self.fields in Form.__init__() fails

Reported by: Ronny Pfannschmidt Assigned to: insin
Milestone: Component: Forms
Version: SVN Keywords:
Cc: Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description (Last modified by adrian)

take a look at the test 

"""
>>> TestForm({'text':"foo"}).is_valid()
True

>>> TestForm({'text':"foo"},need_name=True).is_valid()
False

>>> TestForm({'text':"foo"}).is_valid() #this one will fail
True

"""
from django.newforms import *
class TestForm(Form):
    name = CharField(max_length=30)
    text = CharField(widget=Textarea)
    def __init__(self,*k,**kw):
        need_name=kw.pop("need_name",False)
        Form.__init__(self,*k,**kw)

        if need_name:
            self.fields["name"].required=True

Just assigning need_name to self.fields["name"].required would create a race condition for multithreaded servers.

Attachments

deepcopy.diff (1.8 kB) - added by Jonathan Buchanan <jonathan.buchanan@gmail.com> on 03/21/07 20:18:38.
Copying of fields with deepcopy and regression test
sprint-patch1.diff (2.5 kB) - added by insin on 09/13/07 17:23:53.
Previous patch no longer works due to a deepcopy error

Change History

02/13/07 11:24:12 changed by adrian

  • description changed.
  • needs_better_patch changed.
  • needs_tests changed.
  • summary changed from altering local newforms Formfields fails cause self.fields isnt a deep copy to Altering self.fields in Form.__init__() fails.
  • needs_docs changed.
  • stage changed from Unreviewed to Accepted.

Fixed formatting in description.

02/13/07 12:04:47 changed by adrian

  • status changed from new to closed.
  • resolution set to fixed.

(In [4504]) Fixed #3489 -- Changed newforms to use copy.copy() in constructing self.fields, so changes to self.fields in a given form instance do not affect other instances

03/20/07 19:42:58 changed by Jonathan Buchanan <jonathan.buchanan@gmail.com>

  • status changed from closed to reopened.
  • resolution deleted.

Should copy.deepcopy() be used instead? I ran into problems when modifying widgets, as each field's widget will still be a reference rather than a copy. 

from django import newforms as forms

class UserForm(forms.Form):
    username = forms.CharField(max_length=30)

    def __init__(self, disable_name=False, *args, **kwargs):
        super(UserForm, self).__init__(*args, **kwargs)
        if disable_name:
            self.fields['username'].widget.attrs['disabled'] = 'disabled'
        
>>> f1 = UserForm()
>>> print f1['username']
<input id="id_username" type="text" name="username" maxlength="30" />
>>> f2 = UserForm(disable_name=True)
>>> print f2['username']
<input disabled="disabled" id="id_username" type="text" name="username" maxlength="30" />
>>> print f1['username']
<input disabled="disabled" id="id_username" type="text" name="username" maxlength="30" />

03/21/07 20:18:38 changed by Jonathan Buchanan <jonathan.buchanan@gmail.com>

  • attachment deepcopy.diff added.

Copying of fields with deepcopy and regression test

07/07/07 06:54:14 changed by Jonathan Buchanan <jonathan.buchanan@gmail.com>

  • has_patch set to 1.

09/13/07 16:16:25 changed by insin

  • owner changed from nobody to insin.
  • status changed from reopened to new.

Reassigning to myself since I've already written a patch and regression tests - will check if it still applies cleanly.

09/13/07 17:23:53 changed by insin

  • attachment sprint-patch1.diff added.

Previous patch no longer works due to a deepcopy error

09/13/07 17:27:14 changed by insin

The deepcopy implementation I've just attached a patch for seems like a bit of a hack to me, but the tests pass - can I get a second opinion?

09/13/07 22:20:48 changed by mtredinnick

I don't think this patch is hackish at all. It's exactly what is required when you write a deepcopy method: copy the bits you need and that's it. This is fine.

09/13/07 22:29:39 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

(In [6156]) Fixed #3489 -- Added proper deepcopying to form fields so that widget instances get copied as well. Patch from Jonathan Buchanan and insin.

01/04/08 10:07:35 changed by jacob

  • reporter changed from Ronny Pfannschmidt <ronny.pfannschmidt@gmx.de> to Ronny Pfannschmidt.

Add/Change #3489 (Altering self.fields in Form.__init__() fails)




Change Properties
Action