#33375 closed Bug (wontfix)
Admin changelist_formset does not use the Admin queryset
| Reported by: | François Freitag | Owned by: | nobody |
|---|---|---|---|
| Component: | contrib.admin | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | François Freitag | Triage Stage: | Unreviewed |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
I’m implementing soft-deletion for models.Models with a BooleanField named deleted. For the entire system (except Django admin), an object deleted=True no longer exist. That is implemented by setting the default manager to an objects manager that ignores deleted=True rows.
For the admin, I’m overriding the queryset to include deleted=True.
When using list_editable = ["deleted"], the soft-deleted objects cannot be re-activated. The reason is that the id field in the changelist_formset uses the _default_manager (objects).
I do not want to make include_deleted the default manager, as any model field in the system would use that instead of the objects, unless overridden.
IMO, the admin should be using the result of get_queryset() for the changelist_formset id field Queryset.
Happy to attempt patching if the ticket is accepted.
Attachments (1)
Change History (6)
comment:1 by , 4 years ago
| Cc: | added |
|---|
comment:2 by , 4 years ago
| Resolution: | → needsinfo |
|---|---|
| Status: | new → closed |
comment:3 by , 4 years ago
| Resolution: | needsinfo |
|---|---|
| Status: | closed → new |
Hi Carlton,
Thanks for looking into it! The ManagementForm was indeed missing from my patch. It was also missing _save=on.
With these changes, the issue triggers in the correct code path. I’ll update the attached patch immediately.
modified_objects is indeed the views queryset, including soft-deleted objects. However, the formset id field used the default queryset. Here’s the behavior I’m seeing while step-debugging the code:
> /home/freitafr/dev/django/django/contrib/admin/options.py(1765)changelist_view()
-> if formset.is_valid():
(Pdb) l
1760 raise PermissionDenied
1761 FormSet = self.get_changelist_formset(request)
1762 breakpoint()
1763 modified_objects = self._get_list_editable_queryset(request, FormSet.get_default_prefix())
1764 formset = cl.formset = FormSet(request.POST, request.FILES, queryset=modified_objects)
1765 -> if formset.is_valid():
1766 changecount = 0
1767 for form in formset.forms:
1768 if form.has_changed():
1769 obj = self.save_form(request, form, change=True)
1770 self.save_model(request, obj, form, change=True)
(Pdb) n
> /home/freitafr/dev/django/django/contrib/admin/options.py(1795)changelist_view()
-> if formset:
(Pdb) p formset.is_valid()
False
(Pdb) p formset.errors
[{'id': ['Select a valid choice. That choice is not one of the available choices.']}, {}]
by , 4 years ago
| Attachment: | failing-test.patch added |
|---|
Failing test v2 (fixed _save and management form missing from POST data)
comment:4 by , 4 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
Thanks for the update François.
[{'id': ['Select a valid choice. That choice is not one of the available choices.']}, {}]
So just looking at that, you have a ModelChoiceField with the wrong QuerySet, and sure enough...
(Pdb) form.fields
{'deleted': <django.forms.fields.BooleanField object at 0x104a136a0>, 'id': <django.forms.models.ModelChoiceField object at 0x104a13a90>}
(Pdb) form.fields['id'].queryset
<QuerySet [<SoftDeletable: SoftDeletable object (2)>]>
Why? Because in BaseModelFormSet.add_fields() we add a just such a field using the default manager.
I suspect that's untouchable, and just a limitation of the _soft delete_ approach.
I'd look at overriding ModelAdmin.get_changelist_form (or maybe ModelAdmin.get_changelist_formset) to provide the base form class declaring the id ModelChoiceField with the correct (for your case) QuerySet.
I'm going to mark as wontfix. If you come up with a suggestion, happy to take a look (but... :)
comment:5 by , 4 years ago
Thanks for the answer!
I suspected it would be quite involved to override the auto-generated form field queryset. If no-one feels strongly it should be done, I’m fine implementing workarounds in my project.
Hi François.
Can I ask you to dig a bit deeper, I'm not sure the test is right...
The relevant section from the
changelist_view:# Handle POSTed bulk-edit data. if request.method == 'POST' and cl.list_editable and '_save' in request.POST: if not self.has_change_permission(request): raise PermissionDenied FormSet = self.get_changelist_formset(request) modified_objects = self._get_list_editable_queryset(request, FormSet.get_default_prefix()) formset = cl.formset = FormSet(request.POST, request.FILES, queryset=modified_objects) if formset.is_valid():src
Adding the required
_saveto thePOSTdata, we enter the block here, but despite having the right queryset, we don't have any forms, and the formset isn't valid:I guess this is the missing management form data (but I've run out of time to dig as it is).
If you look into
_get_list_editable_queryset(), you'll see thatself.get_queryset()is used, which gives us the hoped formodified_objectsqueryset.No doubt you're seeing something, but there's not enough quite yet to see what it is (with available time).
I'll mark as needsinfo. Please reopen when you can add more.
Thanks.