From d0af8e87232b1daa6ace7f85e1ddc1407faeb18e Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Fri, 17 Nov 2017 21:17:35 -0500 Subject: [PATCH] Add 'name' and 'description' attributes to ViewSet ViewSets may now provide their `name` and `description` attributes directly, instead of relying on view introspection to derive them. These attributes may also be provided with the view's initkwargs. The ViewSet `name` and `suffix` initkwargs are mutually exclusive. The `action` decorator now provides the `name` and `description` to the view's initkwargs. By default, these values are derived from the method name and its docstring. The `name` may be overridden by providing it as an argument to the decorator. The `get_view_name` and `get_view_description` hooks now provide the view instance to the handler, instead of the view class. The default implementations of these handlers now respect the `name`/`description`. --- rest_framework/decorators.py | 8 +++++++- rest_framework/views.py | 24 ++++++++++++++++++------ rest_framework/viewsets.py | 11 +++++++++++ tests/test_decorators.py | 7 ++++++- tests/test_description.py | 27 +++++++++++++++++++++++++++ tests/test_utils.py | 26 ++++++++++++++++++++++++++ tests/test_viewsets.py | 10 ++++++++++ 7 files changed, 105 insertions(+), 8 deletions(-) diff --git a/rest_framework/decorators.py b/rest_framework/decorators.py index c9b6f89c7..9f6b8101c 100644 --- a/rest_framework/decorators.py +++ b/rest_framework/decorators.py @@ -11,6 +11,7 @@ from __future__ import unicode_literals import types import warnings +from django.forms.utils import pretty_name from django.utils import six from rest_framework.views import APIView @@ -130,7 +131,7 @@ def schema(view_inspector): return decorator -def action(methods=None, detail=None, url_path=None, url_name=None, **kwargs): +def action(methods=None, detail=None, name=None, url_path=None, url_name=None, **kwargs): """ Mark a ViewSet method as a routable action. @@ -147,9 +148,14 @@ def action(methods=None, detail=None, url_path=None, url_name=None, **kwargs): def decorator(func): func.bind_to_methods = methods func.detail = detail + func.name = name if name else pretty_name(func.__name__) func.url_path = url_path if url_path else func.__name__ func.url_name = url_name if url_name else func.__name__.replace('_', '-') func.kwargs = kwargs + func.kwargs.update({ + 'name': func.name, + 'description': func.__doc__ or None + }) return func return decorator diff --git a/rest_framework/views.py b/rest_framework/views.py index 1f51517db..70af84816 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -21,31 +21,43 @@ from rest_framework.settings import api_settings from rest_framework.utils import formatting -def get_view_name(view_cls, suffix=None): +def get_view_name(view): """ Given a view class, return a textual name to represent the view. This name is used in the browsable API, and in OPTIONS responses. This function is the default for the `VIEW_NAME_FUNCTION` setting. """ - name = view_cls.__name__ + # Name may be set by some Views, such as a ViewSet. + name = getattr(view, 'name', None) + if name is not None: + return name + + name = view.__class__.__name__ name = formatting.remove_trailing_string(name, 'View') name = formatting.remove_trailing_string(name, 'ViewSet') name = formatting.camelcase_to_spaces(name) + + # Suffix may be set by some Views, such as a ViewSet. + suffix = getattr(view, 'suffix', None) if suffix: name += ' ' + suffix return name -def get_view_description(view_cls, html=False): +def get_view_description(view, html=False): """ Given a view class, return a textual description to represent the view. This name is used in the browsable API, and in OPTIONS responses. This function is the default for the `VIEW_DESCRIPTION_FUNCTION` setting. """ - description = view_cls.__doc__ or '' + # Description may be set by some Views, such as a ViewSet. + description = getattr(view, 'description', None) + if description is None: + description = view.__class__.__doc__ or '' + description = formatting.dedent(smart_text(description)) if html: return formatting.markup_description(description) @@ -224,7 +236,7 @@ class APIView(View): browsable API. """ func = self.settings.VIEW_NAME_FUNCTION - return func(self.__class__, getattr(self, 'suffix', None)) + return func(self) def get_view_description(self, html=False): """ @@ -232,7 +244,7 @@ class APIView(View): and in the browsable API. """ func = self.settings.VIEW_DESCRIPTION_FUNCTION - return func(self.__class__, html) + return func(self, html) # API policy instantiation methods diff --git a/rest_framework/viewsets.py b/rest_framework/viewsets.py index eacf44f8c..d7b2b49b7 100644 --- a/rest_framework/viewsets.py +++ b/rest_framework/viewsets.py @@ -52,7 +52,13 @@ class ViewSetMixin(object): instantiated view, we need to totally reimplement `.as_view`, and slightly modify the view function that is created and returned. """ + # The name and description initkwargs may be explicitly overridden for + # certain route confiugurations. eg, names of extra actions. + cls.name = None + cls.description = None + # The suffix initkwarg is reserved for displaying the viewset type. + # This initkwarg should have no effect if the name is provided. # eg. 'List' or 'Instance'. cls.suffix = None @@ -79,6 +85,11 @@ class ViewSetMixin(object): raise TypeError("%s() received an invalid keyword %r" % ( cls.__name__, key)) + # name and suffix are mutually exclusive + if 'name' in initkwargs and 'suffix' in initkwargs: + raise TypeError("%s() received both `name` and `suffix`, which are " + "mutually exclusive arguments." % (cls.__name__)) + def view(request, *args, **kwargs): self = cls(**initkwargs) # We also store the mapping of request methods to actions, diff --git a/tests/test_decorators.py b/tests/test_decorators.py index fc80b472d..c24287600 100644 --- a/tests/test_decorators.py +++ b/tests/test_decorators.py @@ -175,12 +175,17 @@ class ActionDecoratorTestCase(TestCase): def test_defaults(self): @action(detail=True) def test_action(request): - pass + """Description""" assert test_action.bind_to_methods == ['get'] assert test_action.detail is True + assert test_action.name == 'Test action' assert test_action.url_path == 'test_action' assert test_action.url_name == 'test-action' + assert test_action.kwargs == { + 'name': 'Test action', + 'description': 'Description', + } def test_detail_required(self): with pytest.raises(AssertionError) as excinfo: diff --git a/tests/test_description.py b/tests/test_description.py index ffd2636f2..702e56332 100644 --- a/tests/test_description.py +++ b/tests/test_description.py @@ -85,6 +85,22 @@ class TestViewNamesAndDescriptions(TestCase): pass assert MockView().get_view_name() == 'Mock' + def test_view_name_uses_name_attribute(self): + class MockView(APIView): + name = 'Foo' + assert MockView().get_view_name() == 'Foo' + + def test_view_name_uses_suffix_attribute(self): + class MockView(APIView): + suffix = 'List' + assert MockView().get_view_name() == 'Mock List' + + def test_view_name_preferences_name_over_suffix(self): + class MockView(APIView): + name = 'Foo' + suffix = 'List' + assert MockView().get_view_name() == 'Foo' + def test_view_description_uses_docstring(self): """Ensure view descriptions are based on the docstring.""" class MockView(APIView): @@ -112,6 +128,17 @@ class TestViewNamesAndDescriptions(TestCase): assert MockView().get_view_description() == DESCRIPTION + def test_view_description_uses_description_attribute(self): + class MockView(APIView): + description = 'Foo' + assert MockView().get_view_description() == 'Foo' + + def test_view_description_allows_empty_description(self): + class MockView(APIView): + """Description.""" + description = '' + assert MockView().get_view_description() == '' + def test_view_description_can_be_empty(self): """ Ensure that if a view has no docstring, diff --git a/tests/test_utils.py b/tests/test_utils.py index d63f266d6..a29ae2fa6 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -4,6 +4,7 @@ from __future__ import unicode_literals from django.conf.urls import url from django.test import TestCase, override_settings +from rest_framework.decorators import action from rest_framework.routers import SimpleRouter from rest_framework.serializers import ModelSerializer from rest_framework.utils import json @@ -43,6 +44,14 @@ class ResourceViewSet(ModelViewSet): serializer_class = ModelSerializer queryset = BasicModel.objects.all() + @action(detail=False) + def list_action(self, request, *args, **kwargs): + raise NotImplementedError + + @action(detail=True) + def detail_action(self, request, *args, **kwargs): + raise NotImplementedError + router = SimpleRouter() router.register(r'resources', ResourceViewSet) @@ -119,6 +128,23 @@ class BreadcrumbTests(TestCase): ('Resource Instance', '/resources/1/') ] + def test_modelviewset_list_action_breadcrumbs(self): + url = '/resources/list_action/' + assert get_breadcrumbs(url) == [ + ('Root', '/'), + ('Resource List', '/resources/'), + ('List action', '/resources/list_action/'), + ] + + def test_modelviewset_detail_action_breadcrumbs(self): + url = '/resources/1/detail_action/' + assert get_breadcrumbs(url) == [ + ('Root', '/'), + ('Resource List', '/resources/'), + ('Resource Instance', '/resources/1/'), + ('Detail action', '/resources/1/detail_action/'), + ] + class JsonFloatTests(TestCase): """ diff --git a/tests/test_viewsets.py b/tests/test_viewsets.py index caed6f2f6..d5c428e3a 100644 --- a/tests/test_viewsets.py +++ b/tests/test_viewsets.py @@ -96,6 +96,16 @@ class InitializeViewSetsTestCase(TestCase): "when calling `.as_view()` on a ViewSet. " "For example `.as_view({'get': 'list'})`") + def test_initialize_view_set_with_both_name_and_suffix(self): + with pytest.raises(TypeError) as excinfo: + BasicViewSet.as_view(name='', suffix='', actions={ + 'get': 'list', + }) + + assert str(excinfo.value) == ( + "BasicViewSet() received both `name` and `suffix`, " + "which are mutually exclusive arguments.") + def test_args_kwargs_request_action_map_on_self(self): """ Test a view only has args, kwargs, request, action_map