#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.Model
s 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 , 3 years ago
Cc: | added |
---|
comment:2 by , 3 years ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
comment:3 by , 3 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 , 3 years ago
Attachment: | failing-test.patch added |
---|
Failing test v2 (fixed _save and management form missing from POST data)
comment:4 by , 3 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 , 3 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
:src
Adding the required
_save
to thePOST
data, we enter the block here, but despite having the right queryset, we don't have any forms, the form 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_objects
queryset.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.