diff --git a/docs/api-guide/viewsets.md b/docs/api-guide/viewsets.md index b912e0dac..e7cf4d48f 100644 --- a/docs/api-guide/viewsets.md +++ b/docs/api-guide/viewsets.md @@ -127,7 +127,7 @@ You may inspect these attributes to adjust behaviour based on the current action ## Marking extra actions for routing -If you have ad-hoc methods that should be routable, you can mark them as such with the `@action` decorator. Like regular actions, extra actions may be intended for either a list of objects, or a single instance. To indicate this, set the `detail` argument to `True` or `False`. The router will configure its URL patterns accordingly. e.g., the `DefaultRouter` will configure detail actions to contain `pk` in their URL patterns. +If you have ad-hoc methods that should be routable, you can mark them as such with the `@action` decorator. Like regular actions, extra actions may be intended for either a single object, or an entire collection. To indicate this, set the `detail` argument to `True` or `False`. The router will configure its URL patterns accordingly. e.g., the `DefaultRouter` will configure detail actions to contain `pk` in their URL patterns. A more complete example of extra actions: @@ -174,7 +174,7 @@ The decorator can additionally take extra arguments that will be set for the rou def set_password(self, request, pk=None): ... -These decorator will route `GET` requests by default, but may also accept other HTTP methods by setting the `methods` argument. For example: +The `action` decorator will route `GET` requests by default, but may also accept other HTTP methods by setting the `methods` argument. For example: @action(detail=True, methods=['post', 'delete']) def unset_password(self, request, pk=None): @@ -186,7 +186,7 @@ To view all extra actions, call the `.get_extra_actions()` method. ### Routing additional HTTP methods for extra actions -Extra actions can be mapped to different `ViewSet` methods. For example, the above password set/unset methods could be consolidated into a single route. Note that additional mappings do not accept arguments. +Extra actions can map additional HTTP methods to separate `ViewSet` methods. For example, the above password set/unset methods could be consolidated into a single route. Note that additional mappings do not accept arguments. ```python @action(detail=True, methods=['put'], name='Change Password') diff --git a/rest_framework/decorators.py b/rest_framework/decorators.py index 60078947f..2d3bbe46f 100644 --- a/rest_framework/decorators.py +++ b/rest_framework/decorators.py @@ -131,7 +131,7 @@ def schema(view_inspector): return decorator -def action(methods=None, detail=None, name=None, url_path=None, url_name=None, **kwargs): +def action(methods=None, detail=None, url_path=None, url_name=None, **kwargs): """ Mark a ViewSet method as a routable action. @@ -145,18 +145,22 @@ def action(methods=None, detail=None, name=None, url_path=None, url_name=None, * "@action() missing required argument: 'detail'" ) + # name and suffix are mutually exclusive + if 'name' in kwargs and 'suffix' in kwargs: + raise TypeError("`name` and `suffix` are mutually exclusive arguments.") + def decorator(func): func.mapping = MethodMapper(func, 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 - }) + + # Set descriptive arguments for viewsets + if 'name' not in kwargs and 'suffix' not in kwargs: + func.kwargs['name'] = pretty_name(func.__name__) + func.kwargs['description'] = func.__doc__ or None return func return decorator diff --git a/rest_framework/views.py b/rest_framework/views.py index 70af84816..04951ed93 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -23,7 +23,7 @@ from rest_framework.utils import formatting def get_view_name(view): """ - Given a view class, return a textual name to represent the view. + Given a view instance, 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. @@ -48,7 +48,7 @@ def get_view_name(view): def get_view_description(view, html=False): """ - Given a view class, return a textual description to represent the view. + Given a view instance, 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. diff --git a/rest_framework/viewsets.py b/rest_framework/viewsets.py index 412475351..7146828d2 100644 --- a/rest_framework/viewsets.py +++ b/rest_framework/viewsets.py @@ -183,7 +183,8 @@ class ViewSetMixin(object): try: url_name = '%s-%s' % (self.basename, action.url_name) url = reverse(url_name, self.args, self.kwargs, request=self.request) - action_urls[action.name] = url + view = self.__class__(**action.kwargs) + action_urls[view.get_view_name()] = url except NoReverseMatch: pass # URL requires additional arguments, ignore diff --git a/tests/test_decorators.py b/tests/test_decorators.py index 7568513f3..77c488c34 100644 --- a/tests/test_decorators.py +++ b/tests/test_decorators.py @@ -179,7 +179,6 @@ class ActionDecoratorTestCase(TestCase): assert test_action.mapping == {'get': 'test_action'} 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 == { @@ -213,6 +212,47 @@ class ActionDecoratorTestCase(TestCase): for name in APIView.http_method_names: assert test_action.mapping[name] == name + def test_view_name_kwargs(self): + """ + 'name' and 'suffix' are mutually exclusive kwargs used for generating + a view's display name. + """ + # by default, generate name from method + @action(detail=True) + def test_action(request): + raise NotImplementedError + + assert test_action.kwargs == { + 'description': None, + 'name': 'Test action', + } + + # name kwarg supersedes name generation + @action(detail=True, name='test name') + def test_action(request): + raise NotImplementedError + + assert test_action.kwargs == { + 'description': None, + 'name': 'test name', + } + + # suffix kwarg supersedes name generation + @action(detail=True, suffix='Suffix') + def test_action(request): + raise NotImplementedError + + assert test_action.kwargs == { + 'description': None, + 'suffix': 'Suffix', + } + + # name + suffix is a conflict. + with pytest.raises(TypeError) as excinfo: + action(detail=True, name='test name', suffix='Suffix') + + assert str(excinfo.value) == "`name` and `suffix` are mutually exclusive arguments." + def test_method_mapping(self): @action(detail=False) def test_action(request): @@ -223,7 +263,7 @@ class ActionDecoratorTestCase(TestCase): raise NotImplementedError # The secondary handler methods should not have the action attributes - for name in ['mapping', 'detail', 'name', 'url_path', 'url_name', 'kwargs']: + for name in ['mapping', 'detail', 'url_path', 'url_name', 'kwargs']: assert hasattr(test_action, name) and not hasattr(test_action_post, name) def test_method_mapping_already_mapped(self): diff --git a/tests/test_utils.py b/tests/test_utils.py index a29ae2fa6..28b06b173 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -52,6 +52,14 @@ class ResourceViewSet(ModelViewSet): def detail_action(self, request, *args, **kwargs): raise NotImplementedError + @action(detail=True, name='Custom Name') + def named_action(self, request, *args, **kwargs): + raise NotImplementedError + + @action(detail=True, suffix='Custom Suffix') + def suffixed_action(self, request, *args, **kwargs): + raise NotImplementedError + router = SimpleRouter() router.register(r'resources', ResourceViewSet) @@ -145,6 +153,24 @@ class BreadcrumbTests(TestCase): ('Detail action', '/resources/1/detail_action/'), ] + def test_modelviewset_action_name_kwarg(self): + url = '/resources/1/named_action/' + assert get_breadcrumbs(url) == [ + ('Root', '/'), + ('Resource List', '/resources/'), + ('Resource Instance', '/resources/1/'), + ('Custom Name', '/resources/1/named_action/'), + ] + + def test_modelviewset_action_suffix_kwarg(self): + url = '/resources/1/suffixed_action/' + assert get_breadcrumbs(url) == [ + ('Root', '/'), + ('Resource List', '/resources/'), + ('Resource Instance', '/resources/1/'), + ('Resource Custom Suffix', '/resources/1/suffixed_action/'), + ] + class JsonFloatTests(TestCase): """ diff --git a/tests/test_viewsets.py b/tests/test_viewsets.py index 54b691e22..eac36f095 100644 --- a/tests/test_viewsets.py +++ b/tests/test_viewsets.py @@ -63,9 +63,28 @@ class ActionViewSet(GenericViewSet): raise NotImplementedError +class ActionNamesViewSet(GenericViewSet): + + def retrieve(self, request, *args, **kwargs): + return Response() + + @action(detail=True) + def unnamed_action(self, request, *args, **kwargs): + raise NotImplementedError + + @action(detail=True, name='Custom Name') + def named_action(self, request, *args, **kwargs): + raise NotImplementedError + + @action(detail=True, suffix='Custom Suffix') + def suffixed_action(self, request, *args, **kwargs): + raise NotImplementedError + + router = SimpleRouter() router.register(r'actions', ActionViewSet) router.register(r'actions-alt', ActionViewSet, basename='actions-alt') +router.register(r'names', ActionNamesViewSet, basename='names') urlpatterns = [ @@ -172,6 +191,19 @@ class GetExtraActionUrlMapTests(TestCase): def test_uninitialized_view(self): self.assertEqual(ActionViewSet().get_extra_action_url_map(), OrderedDict()) + def test_action_names(self): + # Action 'name' and 'suffix' kwargs should be respected + response = self.client.get('/api/names/1/') + view = response.renderer_context['view'] + + expected = OrderedDict([ + ('Custom Name', 'http://testserver/api/names/1/named_action/'), + ('Action Names Custom Suffix', 'http://testserver/api/names/1/suffixed_action/'), + ('Unnamed action', 'http://testserver/api/names/1/unnamed_action/'), + ]) + + self.assertEqual(view.get_extra_action_url_map(), expected) + @override_settings(ROOT_URLCONF='tests.test_viewsets') class ReverseActionTests(TestCase):