Skip to content

Commit

Permalink
Allow returning Response from admin @action callbacks (#1331)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
christianbundy and pre-commit-ci[bot] authored Jan 25, 2023
1 parent 9874789 commit 832163d
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 9 deletions.
10 changes: 6 additions & 4 deletions django-stubs/contrib/admin/decorators.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions django-stubs/contrib/admin/options.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
29 changes: 27 additions & 2 deletions tests/typecheck/contrib/admin/test_decorators.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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): ...
Expand All @@ -73,17 +73,42 @@
@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: ...
@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"
2 changes: 1 addition & 1 deletion tests/typecheck/contrib/admin/test_options.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 832163d

Please sign in to comment.