Allow returning Response from admin @action callbacks

[christianbundy]

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.

Related issues

Closes #1330

Documentation: https://docs.djangoproject.com/en/4.1/ref/contrib/admin/actions/#actions-that-provide-intermediate-pages

[intgr]

Pinging @flaeppe for visibility.

[intgr]

Thanks for the PR!

Please add a test case to tests/typecheck/contrib/admin/test_decorators.yml as well. Preferrably into the existing test_admin_decorators_action testcase, as more tests slow down the test suite.

[intgr]

The failures from test suite aren't really issues, you just need to update the expected messages in the test cases. https://github.com/typeddjango/django-stubs/actions/runs/3983617421/jobs/6832674566

btw you can run individual tests locally with e.g. pytest -k errors_for_invalid_action_signature

[ngnpope]

It would be good to have some tests to prevent future regressions.

[christianbundy]

Thanks y'all! I appreciate the quick reviews.

Just pushed some changes to resolve these issues, please let me know what you think.

[intgr]

I think there was a good reason why these Callable[...] were duplicated before at each overload.

I'm not sure why exactly -- maybe mypy doesn't support TypeVars inside TypeVars -- but after your changes, this no longer raises an error with mypy at the location of the decorator:

    @admin.action(description="Some text here", permissions=["test"])
    def freestanding_action_fancy(modeladmin: int, request: int, queryset: int) -> None: ...

I suppose this logic could also use a few negative test cases -- checking that mypy correctly flags incorrect usage of the parameters or return types.

[flaeppe]

Yes, TypeVar can't be bound to a TypeAlias, it'll resolve to Any before it's matched with generic arguments of a class or a callable.

A few non-happy test cases should make that obvious.

[christianbundy]

Sounds good, I've reverted the previous change and added more tests to highlight this issue. I've left _DisplayCallableT alone, but I'd imagine it probably has a similar problem.

[ngnpope]

I see… Yes, if it's a problem, then the equivalent for display is also surely broken.

[christianbundy]

I wonder whether it'd be worthwhile to enable disallow_any_generics for this repo -- I put together a quick repro, and it looks like if we enable the rule we'll get warnings for this: https://mypy-play.net/?mypy=latest&python=3.11&flags=disallow-any-generics&gist=bf198a38181d36c2105ce3fef468d702

[ngnpope]

Yeah, maybe, but best looked into separately later.

[intgr]

I'm afraid the fallout from disallow_any_generics will be enormous. But I may be wrong :)

[christianbundy]

Found 630 errors in 155 files (checked 675 source files)

You aren't wrong. 😬 Here's a quick stab at it though: #1339

[intgr]

Thanks, this looks great! I'll merge this later unless someone else has additional feedback.