From 7f1dd7563b8f35394ec870e170f72a62dcdac954 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Tue, 10 Jul 2018 11:51:06 -0400 Subject: [PATCH] Add suffix support for actions Removes the newly introduced `action.name` in favor of leveraging the View's `.get_view_name()` method, which supports both name and suffix. --- rest_framework/decorators.py | 16 ++++++++++------ rest_framework/viewsets.py | 3 ++- tests/test_decorators.py | 3 +-- 3 files changed, 13 insertions(+), 9 deletions(-) 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/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..856cd9e19 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 == { @@ -223,7 +222,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):