From 832163dad64298f00efe4c4b1b210bd6f766f8b0 Mon Sep 17 00:00:00 2001 From: Christian Bundy Date: Wed, 25 Jan 2023 03:16:05 -0800 Subject: [PATCH] Allow returning Response from admin @action callbacks (#1331) Problem: The Django docs say that you can "return an HttpResponse (or subclass) from your action", but the stubs currently say that actions can only return `None`. Solution: Update stubs to return `HttpResponse | None` instead. * Fix admin action return type * Use HttpResponseBase to support StreamingHttpResponse * Use `T | None` rather than `Optional[T]` * Add freestanding actions to test * Add negative test cases Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- django-stubs/contrib/admin/decorators.pyi | 10 ++++--- django-stubs/contrib/admin/options.pyi | 4 +-- .../contrib/admin/test_decorators.yml | 29 +++++++++++++++++-- .../typecheck/contrib/admin/test_options.yml | 2 +- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/django-stubs/contrib/admin/decorators.pyi b/django-stubs/contrib/admin/decorators.pyi index adcad67ad..66664f37f 100644 --- a/django-stubs/contrib/admin/decorators.pyi +++ b/django-stubs/contrib/admin/decorators.pyi @@ -6,7 +6,7 @@ from django.contrib.admin.sites import AdminSite from django.db.models import Combinable, QuerySet from django.db.models.base import Model from django.db.models.expressions import BaseExpression -from django.http import HttpRequest +from django.http import HttpRequest, HttpResponseBase from django.utils.functional import _StrOrPromise from typing_extensions import TypeAlias @@ -17,20 +17,22 @@ _QuerySet = TypeVar("_QuerySet", bound=QuerySet) # This is deliberately different from _DisplayT defined in contrib.admin.options _DisplayCallable: TypeAlias = Union[Callable[[_ModelAdmin, _Model], Any], Callable[[_Model], Any]] # noqa: Y037 _DisplayCallableT = TypeVar("_DisplayCallableT", bound=_DisplayCallable) +_ActionReturn = TypeVar("_ActionReturn", bound=HttpResponseBase | None) @overload def action( - function: Callable[[_ModelAdmin, _Request, _QuerySet], None], + function: Callable[[_ModelAdmin, _Request, _QuerySet], _ActionReturn], permissions: Sequence[str] | None = ..., description: _StrOrPromise | None = ..., -) -> Callable[[_ModelAdmin, _Request, _QuerySet], None]: ... +) -> Callable[[_ModelAdmin, _Request, _QuerySet], _ActionReturn]: ... @overload def action( *, permissions: Sequence[str] | None = ..., description: _StrOrPromise | None = ..., ) -> Callable[ - [Callable[[_ModelAdmin, _Request, _QuerySet], None]], Callable[[_ModelAdmin, _Request, _QuerySet], None] + [Callable[[_ModelAdmin, _Request, _QuerySet], _ActionReturn]], + Callable[[_ModelAdmin, _Request, _QuerySet], _ActionReturn], ]: ... @overload def display( diff --git a/django-stubs/contrib/admin/options.pyi b/django-stubs/contrib/admin/options.pyi index af42ea939..994485ba5 100644 --- a/django-stubs/contrib/admin/options.pyi +++ b/django-stubs/contrib/admin/options.pyi @@ -27,7 +27,7 @@ from django.forms.models import ( ) from django.forms.widgets import Media from django.http.request import HttpRequest -from django.http.response import HttpResponse, HttpResponseRedirect +from django.http.response import HttpResponse, HttpResponseBase, HttpResponseRedirect from django.template.response import _TemplateForResponseT from django.urls.resolvers import URLPattern from django.utils.datastructures import _ListOrTuple @@ -131,7 +131,7 @@ class BaseModelAdmin(Generic[_ModelT]): _DisplayT: TypeAlias = _ListOrTuple[str | Callable[[_ModelT], str | bool]] _ModelAdmin = TypeVar("_ModelAdmin", bound=ModelAdmin) -_ActionCallable: TypeAlias = Callable[[_ModelAdmin, HttpRequest, QuerySet[_ModelT]], None] +_ActionCallable: TypeAlias = Callable[[_ModelAdmin, HttpRequest, QuerySet[_ModelT]], HttpResponseBase | None] class ModelAdmin(BaseModelAdmin[_ModelT]): list_display: _DisplayT diff --git a/tests/typecheck/contrib/admin/test_decorators.yml b/tests/typecheck/contrib/admin/test_decorators.yml index 9f16d8f5f..2b21443e0 100644 --- a/tests/typecheck/contrib/admin/test_decorators.yml +++ b/tests/typecheck/contrib/admin/test_decorators.yml @@ -60,7 +60,7 @@ from django.contrib import admin from django.db.models import QuerySet - from django.http import HttpRequest + from django.http import FileResponse, HttpRequest, HttpResponse class MyModel(models.Model): ... @@ -73,10 +73,21 @@ @admin.action(description="Some text here", permissions=["test"]) def freestanding_action_fancy(modeladmin: "MyModelAdmin", request: HttpRequest, queryset: QuerySet[MyModel]) -> None: ... + @admin.action + def freestanding_action_http_response(modeladmin: "MyModelAdmin", request: HttpRequest, queryset: QuerySet[MyModel]) -> HttpResponse: ... + + @admin.action + def freestanding_action_file_response(modeladmin: "MyModelAdmin", request: HttpRequest, queryset: QuerySet[MyModel]) -> FileResponse: ... + + @admin.action # E: Value of type variable "_ModelAdmin" of "action" cannot be "int" + def freestanding_action_invalid_bare(modeladmin: int, request: HttpRequest, queryset: QuerySet[MyModel]) -> None: ... + + @admin.action(description="Some text here", permissions=["test"]) # E: Value of type variable "_ModelAdmin" of function cannot be "int" + def freestanding_action_invalid_fancy(modeladmin: int, request: HttpRequest, queryset: QuerySet[MyModel]) -> None: ... @admin.register(MyModel) class MyModelAdmin(admin.ModelAdmin[MyModel]): - actions = [freestanding_action_bare, freestanding_action_fancy, "method_action_bare", "method_action_fancy"] + actions = [freestanding_action_bare, freestanding_action_fancy, "method_action_bare", "method_action_fancy", freestanding_action_http_response, freestanding_action_file_response] @admin.action def method_action_bare(self, request: HttpRequest, queryset: QuerySet[MyModel]) -> None: ... @@ -84,6 +95,20 @@ @admin.action(description="Some text here", permissions=["test"]) def method_action_fancy(self, request: HttpRequest, queryset: QuerySet[MyModel]) -> None: ... + @admin.action(description="Some text here", permissions=["test"]) + def method_action_http_response(self, request: HttpRequest, queryset: QuerySet[MyModel]) -> HttpResponse: ... + + @admin.action(description="Some text here", permissions=["test"]) + def method_action_file_response(self, request: HttpRequest, queryset: QuerySet[MyModel]) -> FileResponse: ... + + @admin.action # E: Value of type variable "_QuerySet" of "action" cannot be "int" + def method_action_invalid_bare(self, request: HttpRequest, queryset: int) -> None: ... + + @admin.action(description="Some text here", permissions=["test"]) # E: Value of type variable "_QuerySet" of function cannot be "int" + def method_action_invalid_fancy(self, request: HttpRequest, queryset: int) -> None: ... + def method(self) -> None: reveal_type(self.method_action_bare) # N: Revealed type is "def (django.http.request.HttpRequest, django.db.models.query._QuerySet[main.MyModel, main.MyModel])" reveal_type(self.method_action_fancy) # N: Revealed type is "def (django.http.request.HttpRequest, django.db.models.query._QuerySet[main.MyModel, main.MyModel])" + reveal_type(self.method_action_http_response) # N: Revealed type is "def (django.http.request.HttpRequest, django.db.models.query._QuerySet[main.MyModel, main.MyModel]) -> django.http.response.HttpResponse" + reveal_type(self.method_action_file_response) # N: Revealed type is "def (django.http.request.HttpRequest, django.db.models.query._QuerySet[main.MyModel, main.MyModel]) -> django.http.response.FileResponse" diff --git a/tests/typecheck/contrib/admin/test_options.yml b/tests/typecheck/contrib/admin/test_options.yml index b9c9ac7ed..8365e8ced 100644 --- a/tests/typecheck/contrib/admin/test_options.yml +++ b/tests/typecheck/contrib/admin/test_options.yml @@ -140,7 +140,7 @@ pass class A(admin.ModelAdmin): - actions = [an_action] # E: List item 0 has incompatible type "Callable[[None], None]"; expected "Union[Callable[[Any, HttpRequest, _QuerySet[Any, Any]], None], str]" + actions = [an_action] # E: List item 0 has incompatible type "Callable[[None], None]"; expected "Union[Callable[[Any, HttpRequest, _QuerySet[Any, Any]], Optional[HttpResponseBase]], str]" - case: errors_for_invalid_model_admin_generic main: | from django.contrib.admin import ModelAdmin