Opened 13 years ago
Closed 12 years ago
#17478 closed Bug (fixed)
Overridding model formset queryset in BaseModelFormSet
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
In https://docs.djangoproject.com/en/1.3/topics/forms/modelforms/#changing-the-queryset there are two methods described to override default queryset. Second method won't work in Django 1.3:
from django.forms.models import BaseModelFormSet class BaseAuthorFormSet(BaseModelFormSet): def __init__(self, *args, **kwargs): super(BaseAuthorFormSet, self).__init__(*args, **kwargs) self.queryset = Author.objects.filter(name__startswith='O')
Calling BaseModelFormSet __init__ will call BaseFormSet's __init__ and it will in turn call self._construct_forms(). self._construct_forms use queryset we try to override but it will be to late for this when the program reach our self.queryset = Author.objects.filter(name__startswith='O') line.
BaseFormSet's _construct_forms is only called once to populate forms attribute. We can defer it a bit using properties to allow queryset to be overridden. So instead for:
def _construct_forms(self): # instantiate all the forms and put them in self.forms self.forms = [] for i in xrange(self.total_form_count()): self.forms.append(self._construct_form(i))
we can do:
def _construct_forms(self): if hasattr(self, "_forms"): return self._forms # instantiate all the forms and put them in self.forms self._forms = [] for i in xrange(self.total_form_count()): self._forms.append(self._construct_form(i)) return self._forms forms = property(_construct_forms)
This change will also require _construct_forms call to be removed from BaseFormSet's __init__.
Deferring forms creation should give us some time to override queryset attribute in BaseAuthorFormSet's __init__.
Attachments (1)
Change History (8)
comment:1 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 13 years ago
Please mind that queryset can also be passed as positional argument, so it can be in either args or kwargs. The correct way would be:
from django.forms.models import BaseModelFormSet class BaseAuthorFormSet(BaseModelFormSet): def __init__(self, *args, **kwargs): queryset = Author.objects.filter(name__startswith='O') if len(args) > 4: args[4] = queryset else: kwargs['queryset'] = queryset super(BaseAuthorFormSet, self).__init__(*args, **kwargs)
But it's a bit ugly in my opinion. For example we rely on arguments ordering, etc.
I think my solution (moving queryset evaluation out of __init___) is better, forms attribute is read-only anyway. We should not force child classes to worry about what parent class does in its __init__ and create ugly hacks like args/kwargs modifications. Well, at least in my opinion ;)
comment:3 by , 12 years ago
This bug is still there :(
@claudep work around works, Thanks!
Maybe a note or change in docs is enough
comment:4 by , 12 years ago
There was a concrete proposal by the original poster, now we need someone to turn the proposal into a real patch, including tests. I might give a try...
comment:5 by , 12 years ago
Has patch: | set |
---|---|
Version: | 1.3 → master |
The attached patch does fix the issue (hopefully), but also bring a little backwards incompatibility.
Previously, initializing a formset with an empty dict would raise a ValidationError
at instanciation time. Now it is raised only when formally validating the formset (hence the TestIsBoundBehavior
modification) or for any operation that triggers the forms creation. I don't think it should matter much, but I'd like to hear some other committer.
by , 12 years ago
Attachment: | 17478-1.diff added |
---|
comment:6 by , 12 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
I would have preferred to document setting kwargs.setdefault('queryset', Author.objects.filter(name__startswith='Charles'))
but, as pointed out by @tomaszswiderski, this wouldn't work if queryset
is passed as an arg.
I always thought it was a bit odd that a ValidationError
raised on instantiation while all the other form API classes wait for validation before sanitizing data anyway.
All tests pass on Python 2.7.4/3.3.1 SQLite3. Marking as RFC.
comment:7 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Fixed in [ef79582e8630cb3c119caed52130c9671188addd] (sorry for the missing hash in the commit message).
Note that the order of the two calls in
__init__
was switched as a resolution of ticket:11735. It seems this was not the correct fix.What about documenting the subclass this way: