django-rest-framework icon indicating copy to clipboard operation
django-rest-framework copied to clipboard

Improve viewset action docs

Open k0t3n opened this issue 11 months ago • 3 comments

Hi, I encountered a problem when overriding viewset-level attributes doesn't work for actions if a route is configured as .as_view().

Example:

# views.py
class TestView(viewsets.ModelViewSet):
    @action(methods=['get'], detail=False, url_name='test_action', permission_classes=[IsAuthenticated,])
    def test_action(self, request):
        return Response()

# urls.py
urlpatterns = [
    path('test_urlpattern/', TestViewSet.as_view({'get': 'test_action'},)),
]

# tests.py
@pytest.mark.django_db
def test_case(client):
    url = reverse('test_urlpattern')
    response = client.get(url)
    assert response.status_code == 200  # expected 401!

You can override it using initkwargs only

# urls.py
urlpatterns = [
    # permission will be added
    path('test_urlpattern/', TestViewSet.as_view({'get': 'test_action'}, permission_classes=[IsAuthenticated, ])),
]

According to the docs, it's not clear this feature works for Router configuration only. No exceptions, warnings, or even related github issues were found regarding this.

Image

k0t3n avatar Jan 29 '25 16:01 k0t3n

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Apr 27 '25 21:04 stale[bot]

isn't .as_view only work for class based views only? do we need this in viewsets?

auvipy avatar Apr 28 '25 03:04 auvipy

isn't .as_view only work for class based views only? do we need this in viewsets?

If your question is whether the drf needs .as_view() in view sets, I believe yes, at least because lots of users' code may rely on it. My suggestion is to add some sort of validation or warning on passing arguments to .as_view(), since if you try to reroute GET requests to a specific function (action) the requests would just be passed to the default get() handler no matter what you configured before.

k0t3n avatar Apr 28 '25 08:04 k0t3n

Yes, I agree that we could make this a bit clearer. The ViewSets page mentions this as_view() API:

If we need to, we can bind this viewset into two separate views, like so:

user_list = UserViewSet.as_view({'get': 'list'})
user_detail = UserViewSet.as_view({'get': 'retrieve'})

Typically we wouldn't do this, but would instead register the viewset with a router, and allow the urlconf to be automatically generated.

The last sentence sort of discourages using it this way, but it's there... Later, the same page introduces actions.

A short note telling to not use them together in the actions section wouldn't hurt, yes.


Some technical notes

Routers do some work to convert dynamic routes from actions into routes and pass the options from kwargs:

https://github.com/encode/django-rest-framework/blob/5c07060ce0dcbdad1e38be8a509f5c23b6e9fc63/rest_framework/routers.py#L212-L224

Doing this TestViewSet.as_view({'get': 'test_action'} in your urls.py bypasses competely this work, and instructs DRF to use the test_action function for all GET request on the viewset.

If you look at how the action decorator is implemented, it mostly sets some attributes on the decorated method, to be picked up by the router, it doesn't change its runtime behaviour by checking its input or transforming its output.

browniebroke avatar Jul 06 '25 15:07 browniebroke

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Oct 18 '25 09:10 stale[bot]

I want to fix docs.

mahdighadiriii avatar Nov 11 '25 10:11 mahdighadiriii