#11183: BaseForm init leaves pointers pointing back to base_fields when widget
is a
Select
-------------------------------------+--------------------------------------
Reporter: margieroginski | Owner: nobody
Status: new | Milestone:
Component: Uncategorized | Version: 1.0
Resolution: | Keywords:
Stage: Unreviewed | Has_patch: 0
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
-------------------------------------+--------------------------------------
Changes (by lukeplant):
* needs_better_patch: => 0
* needs_tests: => 1
* needs_docs: => 0
Old description:
> == High Level Description ==
> When we create a form class, I think the deepcopy that is done does not
> do enough to correctly copy the class in the case when the widget is a
> Select widget. The result is that parts of the class are left pointing
> back to the base_fields data, and when one tries to override the queryset
> at during render(), it is not possible due pointers that are incorrectly
> pointing back to base_fields data.
>
> == Detail ==
> When a base_field in a form points to a Select widget, the widget
> contains a choices attribute. During initialiazation of a
> !ModelChoiceField, a !ModelChoiceIterator is created and the widget's
> choices attribute is set to point to it. This is all done by the
> following line from !ModelChoiceField::__init__();
> self.queryset = queryset
>
> When BaseForm:__init__() runs, it does a deepcopy of self.base_fields.
> When copying the fields, we call the Widget::__deepcopy__() method.
> However, this method copies only the attributes and does not copy the
> !ModelChoiceIterator that a select widget points to. I think that a copy
> should be made of this !ModelChoiceIterator, and furthermore, I think
> that after making the copy, the newly copied !ModelChoiceIterator's
> 'field' attribute needs to be updated so that it points to the copied
> field whose deepcopy caused it to be copied.
>
> I encountered this issue when I tried to override the queryset for a
> !ModelChoiceField with a line like this in the render() function of a
> widget that derives from a Select widget. Here is some example code that
> my usage:
>
> {{{
> class OwnerSelectWidget(widgets.Select):
> def __init__(self, rel, attrs=None):
> self.rel = rel
> super(OwnerSelectWidget, self).__init__(attrs)
>
> def render(self, name, value, attrs=None):
> key = self.rel.get_related_field().name
> self.choices.field.queryset =
> self.rel.to._default_manager.filter(**{key: value}) <<=== important
> rendered = super(OwnerSelectWidget, self).render(name, value,
> attrs)
>
> }}}
>
> When the self.choices.field.queryset assignment is made, it references
> through the !ModelChoiceIterator to the field associated with the widget.
> In the current code base, at this point int the code, the widget has been
> copied, but it still references the original !ModelChoiceIterator, which
> references the !ModelChoiceField that is in base_fields, which references
> the original "base_fields" widget. This is different than the 'self'
> widget, which is the copied version. The result is that the
> self.choices.field.queryset assignment updates the wrong widget (the one
> from base_fields) with the new queryset.
>
> I made a patch to my code that seems to work. However, I find the patch
> kind of ugly and I suspect someone that understands the code better than
> me might want to do this a different way. I did two things:
> * Created a __deepcopy__() method for the Select widget, which does the
> same thin that Widget::__deepcopy__() does, but also calls copy on the
> self.choices field
> * Created a small loop that follows the deepcopy in
> BaseForm:__init__(), which runs through all fields that were copied, and
> if the field contains a'widget' attribute and that widget attribute
> contains a 'choices' attribute and that 'choices' attribute contains a
> 'field' attribute, then it sets that 'field' attribute point to the newly
> copied field. This is the part that I think is pretty ugly.
>
> The attached svn diff shows the patch.
>
> I ran the django testsuite with the command ./runtests.py
> --settings=settings and all tests passed and I have verified via my own
> debugging that this solves my problem.
> }}}
New description:
== High Level Description ==
When we create a form class, I think the deepcopy that is done does not do
enough to correctly copy the class in the case when the widget is a Select
widget. The result is that parts of the class are left pointing back to
the base_fields data, and when one tries to override the queryset at
during render(), it is not possible due pointers that are incorrectly
pointing back to base_fields data.
== Detail ==
When a base_field in a form points to a Select widget, the widget contains
a choices attribute. During initialiazation of a !ModelChoiceField, a
!ModelChoiceIterator is created and the widget's choices attribute is set
to point to it. This is all done by the following line from
`ModelChoiceField::__init__()`:
{{{
self.queryset = queryset
}}}
When `BaseForm:__init__()` runs, it does a deepcopy of self.base_fields.
When copying the fields, we call the `Widget::__deepcopy__()` method.
However, this method copies only the attributes and does not copy the
!ModelChoiceIterator that a select widget points to. I think that a copy
should be made of this !ModelChoiceIterator, and furthermore, I think that
after making the copy, the newly copied !ModelChoiceIterator's 'field'
attribute needs to be updated so that it points to the copied field whose
deepcopy caused it to be copied.
I encountered this issue when I tried to override the queryset for a
!ModelChoiceField with a line like this in the render() function of a
widget that derives from a Select widget. Here is some example code that
my usage:
{{{
class OwnerSelectWidget(widgets.Select):
def __init__(self, rel, attrs=None):
self.rel = rel
super(OwnerSelectWidget, self).__init__(attrs)
def render(self, name, value, attrs=None):
key = self.rel.get_related_field().name
self.choices.field.queryset =
self.rel.to._default_manager.filter(**{key: value}) <<=== important
rendered = super(OwnerSelectWidget, self).render(name, value,
attrs)
}}}
When the self.choices.field.queryset assignment is made, it references
through the !ModelChoiceIterator to the field associated with the widget.
In the current code base, at this point int the code, the widget has been
copied, but it still references the original !ModelChoiceIterator, which
references the !ModelChoiceField that is in base_fields, which references
the original "base_fields" widget. This is different than the 'self'
widget, which is the copied version. The result is that the
self.choices.field.queryset assignment updates the wrong widget (the one
from base_fields) with the new queryset.
I made a patch to my code that seems to work. However, I find the patch
kind of ugly and I suspect someone that understands the code better than
me might want to do this a different way. I did two things:
* Created a `__deepcopy__()` method for the Select widget, which does
the same thin that `Widget::__deepcopy__()` does, but also calls copy on
the self.choices field
* Created a small loop that follows the deepcopy in
`BaseForm:__init__()`, which runs through all fields that were copied, and
if the field contains a'widget' attribute and that widget attribute
contains a 'choices' attribute and that 'choices' attribute contains a
'field' attribute, then it sets that 'field' attribute point to the newly
copied field. This is the part that I think is pretty ugly.
The attached svn diff shows the patch.
I ran the django testsuite with the command ./runtests.py
--settings=settings and all tests passed and I have verified via my own
debugging that this solves my problem.
Comment:
Thanks for your work debugging this. If you could create a regression
test that corners the problem, that would be great. Put it in
'tests/regressiontests/forms/regressions.py' (unless you can find a better
place for it).
--
Ticket URL: <http://code.djangoproject.com/ticket/11183#comment:1>
Django <http://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en
-~----------~----~----~----~------~----~------~--~---
