[Django] #37117: Admin: Change form actions should use ModelAdmin.get_queryset(request)

[Django]

#37117: Admin: Change form actions should use ModelAdmin.get_queryset(request)
-------------------------------------+-------------------------------------
Reporter: Natalia | Owner: Natalia Bidart
Bidart |
Type: Bug | Status: assigned
Component: | Version: 6.1
contrib.admin |
Severity: Release | Keywords:
blocker |
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
The change form action path in `_changeform_view` builds the queryset
passed to action functions using
`self.model._default_manager.get_queryset()` (options.py, introduced in
f30acb1, #12090) instead of `self.get_queryset(request)`. The two diverge
whenever a `ModelAdmin` overrides `get_queryset()` to use a non-default
manager. Some common real-world case being a soft-delete (example below)
or tenant-scoped pattern where the default manager filters out certain
rows and `get_queryset()` opts into a broader set.

When a change form action is invoked on an object that is visible via
`get_queryset(request)` but excluded by `_default_manager`, the action
receives an empty queryset and silently does nothing. This could also have
consequences on annotated queries.

To reproduce, given a model and model admin as below, try to "restore" a
deleted `Document` instance. Using the `main` code, the action results in
a "0 document(s) restored" since the action receives an empty queryset
(ActiveDocumentManager excludes deleted=True) and calls
queryset.update(deleted=False) on zero rows. The document remains deleted.

{{{#!python
# models.py

from django.db import models


class ActiveDocumentManager(models.Manager):
def get_queryset(self):
return super().get_queryset().filter(deleted=False)


class Document(models.Model):
title = models.CharField(max_length=200)
deleted = models.BooleanField(default=False)

objects = ActiveDocumentManager() # default: active documents only
all_objects = models.Manager() # unfiltered

def __str__(self):
return self.title

# admin.py

from django.contrib import admin
from django.contrib.admin.options import ActionLocation

from .models import Document


@admin.action(description="Restore document",
location=ActionLocation.CHANGE_FORM)
def restore_document(modeladmin, request, queryset):
queryset.update(deleted=False)


@admin.register(Document)
class DocumentAdmin(admin.ModelAdmin):
actions = [restore_document]
list_display = ["title", "deleted"]

def get_queryset(self, request):
# Use all_objects so admins can view and act on soft-deleted
documents.
return Document.all_objects.all()
}}}

Working on a patch including a regression test.
--
Ticket URL: <https://code.djangoproject.com/ticket/37117>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#37117: Admin: Change form actions should use ModelAdmin.get_queryset(request)
-------------------------------------+-------------------------------------

Reporter: Natalia Bidart | Owner: Natalia

| Bidart
Type: Bug | Status: assigned

Component: contrib.admin | Version: 6.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Natalia Bidart):

[https://github.com/django/django/pull/21344 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/37117#comment:1>

[Django]

#37117: Admin: Change form actions should use ModelAdmin.get_queryset(request)
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Natalia
| Bidart
Type: Bug | Status: assigned
Component: contrib.admin | Version: 6.1
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* needs_better_patch: 0 => 1
* stage: Unreviewed => Accepted

Comment:

Nice catch!
--
Ticket URL: <https://code.djangoproject.com/ticket/37117#comment:2>

[Django]

#37117: Admin: Change form actions should use ModelAdmin.get_queryset(request)
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Natalia
| Bidart
Type: Bug | Status: assigned
Component: contrib.admin | Version: 6.1
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/37117#comment:3>

[Django]

#37117: Admin: Change form actions should use ModelAdmin.get_queryset(request)
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Natalia
| Bidart

Type: Bug | Status: closed
Component: contrib.admin | Version: 6.1
Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by nessita <124304+nessita@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"a53e02af6d7a07d0367b534f7107a0e2a37e3d34" a53e02a]:
{{{#!CommitTicketReference repository=""
revision="a53e02af6d7a07d0367b534f7107a0e2a37e3d34"
Fixed #37117 -- Used ModelAdmin.get_queryset() for change form actions.

Refs #37105, #12090.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37117#comment:4>

[Django]

#37117: Admin: Change form actions should use ModelAdmin.get_queryset(request)
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Natalia
| Bidart
Type: Bug | Status: closed
Component: contrib.admin | Version: 6.1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Natalia <124304+nessita@…>):

In [changeset:"1825a1c53d8d13b5d5587fdf9c40a248831fb083" 1825a1c5]:
{{{#!CommitTicketReference repository=""
revision="1825a1c53d8d13b5d5587fdf9c40a248831fb083"
[6.1.x] Fixed #37117 -- Used ModelAdmin.get_queryset() for change form
actions.

Refs #37105, #12090.

Backport of a53e02af6d7a07d0367b534f7107a0e2a37e3d34 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37117#comment:5>