Opened 13 years ago
Closed 12 years ago
#18272 closed Bug (wontfix)
ModelChoiceField's queryset attribute persists between two form instances
Reported by: | Baptiste Mispelon | Owned by: | Spark23 |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Normal | Keywords: | modelchoicefield deepcopy |
Cc: | Triage Stage: | Design decision needed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Setup
Using the models from the tutorial:
from django.db import models class Poll(models.Model): question = models.CharField(max_length=200) pub_date = models.DateTimeField('date published') class Choice(models.Model): poll = models.ForeignKey(Poll) choice = models.CharField(max_length=200) votes = models.IntegerField()
And the following form:
from django import forms from polls.models import Poll class ChoiceForm(forms.Form): poll = forms.ModelChoiceField(queryset=Poll.objects.all()) def __init__(self, *args, **kwargs): """Change the choices attribute of the field `poll`.""" super(ChoiceForm, self).__init__(*args, **kwargs) queryset = self.fields['poll'].queryset choices = [(poll.pk, poll.pk) for poll in queryset] self.fields['poll'].choices = choices
Bug
When doing so, the queryset gets "stuck" on what it was the first time the form was instanciated: added Poll objects don't show up in it, and deleted polls are still present.
from datetime import datetime from polls.forms import ChoiceForm from polls.models import Poll Poll.objects.all().delete() # Just making sure. f = ChoiceForm() print f['poll'].as_widget() # '<select name="poll" id="id_poll">\n</select>' Poll.objects.create(question="What is your favourite colour?", pub_date=datetime.now()) f = ChoiceForm() print f['poll'].as_widget() # Expected: '<select name="poll" id="id_poll">\n<option value="1">1</option>\n</select>' # Result: '<select name="poll" id="id_poll">\n</select>'
Workaround
One workaround is to copy the queryset before iterating over it:
queryset = self.fields['poll'].queryset.all()
Analysis
I believe the issue lies in ModelChoiceField.__deepcopy__()
, in particular the line:
result.queryset = result.queryset
queryset
being a property, this ends up doing (among other things which I think aren't relevant here):
result._queryset = result._queryset
And since _queryset
has been iterated over, its cache is filled and it gets attached onto the deepcopied instance.
Fix
My naive fix would be to make a copy of result.queryset in the __deepcopy__
method, like so:
result.queryset = result.queryset.all()
But my understanding of the issue is limited and I probably don't know the ins and outs of this (especially considering the lengthy discussion around ticket #11183).
Change History (5)
comment:1 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:2 by , 12 years ago
comment:3 by , 12 years ago
This seems like a pretty far edge case. The internals already clear the cache as pointed out - specifically here:
https://github.com/django/django/commit/ee96c7eb2d#L0R120
while altering forms in init is a pretty common pattern, I don't think this would be a regularly encountered problem - though granted, I'm sure it would stump more than a few.
however calling .all() to get a copy is not 'free', and clearing the cache is a more precise way to avoid the problem for the implementation. I think that if you are going to get into this level of form override, your workaround is the correct way to deal with it - not to clone the queryset in deepcopy
I'd suggest this is a candidate for "wontfix"
comment:4 by , 12 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
As per ptone's comments, it is a confirmed issue, but edge case. The work around may need some documentation.
comment:5 by , 12 years ago
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
Closing as wontfix as suggested by Preston.
Note this commit: https://github.com/django/django/commit/ee96c7eb2d
rather sounds like it intended to do what is being asked for here, which is to ensure that the queryset associated with a model choice field be re-evaluated when necessary, and not stuck at whatever happened to be the choices associated with the queryset when the form was initially defined. I'm not entirely sure if code has changed in the interim such that the behavior has broken (the commit has associated tests so I would hope that has not happened) or if that commit didn't entirely cover the need here).