Refactor action names and structure

* Renamed Actions to include consistent suffix.
* Config change: 'handle_duplicates' is now renamed to 'duplicate_policy'
* Refactored duplicate code into shared functions.
* Adding a functional serializer test.

Change-Id: I79fa06f7098df7cc7fe2a228a606a0f4f54b5510

DEVSTACK_GUIDE.md

@@ -2,7 +2,7 @@
 
 This is a guide to setting up StackTask in a running Devstack environment similar to how we have been running it for development purposes.
 
-This guide assumes you are running this in a clean ubuntu 14.04 virtual machine with sudo access. 
+This guide assumes you are running this in a clean ubuntu 14.04 virtual machine with sudo access.
 
 ## Deploy Devstack
 

README.md

@@ -243,6 +243,14 @@ Provided you have tox and its requirements installed running tests is very simpl
 ```
 $ tox
 ```
+To run a single test:
+```
+$ virtualenv venv
+$ source venv/bin/activate
+$ pip install -r requirements.txt -r test-requirements.txt
+$ python setup.py develop
+$ stacktask-api test stacktask.api.v1.tests.test_api_taskview.TaskViewTests.test_duplicate_tasks_new_user
+```
 
 ### Adding Actions:
 

conf/conf.yaml

@@ -124,13 +124,13 @@ TASK_SETTINGS:
         # The order of the actions is order of execution.
         #
         # default_actions:
-        #     - NewProject
+        #     - NewProjectAction
         #
         # Additonal actions for views
         # These will run after the default actions, in the given order.
         additional_actions:
-            - AddDefaultUsersToProject
+            - AddDefaultUsersToProjectAction
-            - NewProjectDefaultNetwork
+            - NewProjectDefaultNetworkAction
         notifications:
             standard:
                 EmailNotification:
@@ -163,7 +163,7 @@ TASK_SETTINGS:
                 notification: acknowledge
                 engines: False
     reset_password:
-        handle_duplicates: cancel
+        duplicate_policy: cancel
         emails:
             initial: null
             token:
@@ -173,7 +173,7 @@ TASK_SETTINGS:
                 subject: Password Reset Completed
                 template: password_reset_completed.txt
     force_password:
-        handle_duplicates: cancel
+        duplicate_policy: cancel
         emails:
             initial: null
             token:
@@ -191,22 +191,22 @@ TASK_SETTINGS:
 
 # Action settings:
 ACTION_SETTINGS:
-    NewProject:
+    NewProjectAction:
         default_roles:
             - project_admin
             - project_mod
             - heat_stack_owner
             - _member_
-    NewUser:
+    NewUserAction:
         allowed_roles:
             - project_admin
             - project_mod
             - heat_stack_owner
             - _member_
-    ResetUser:
+    ResetUserAction:
         blacklisted_roles:
             - admin
-    NewDefaultNetwork:
+    NewDefaultNetworkAction:
         RegionOne:
             network_name: default_network
             subnet_name: default_subnet
@@ -216,7 +216,7 @@ ACTION_SETTINGS:
                 - 193.168.1.2
                 - 193.168.1.3
             SUBNET_CIDR: 192.168.1.0/24
-    AddDefaultUsersToProject:
+    AddDefaultUsersToProjectAction:
         default_users:
             - admin
         default_roles:
@@ -238,4 +238,3 @@ ROLES_MAPPING:
         - project_mod
         - heat_stack_owner
         - _member_
-

stacktask/actions/models.py

@@ -218,10 +218,10 @@ class UserNameAction(UserAction):
             except ValueError:
                 pass
                 # nothing to remove
-            super(UserAction, self).__init__(*args, **kwargs)
+            super(UserNameAction, self).__init__(*args, **kwargs)
             self.username = self.email
         else:
-            super(UserAction, self).__init__(*args, **kwargs)
+            super(UserNameAction, self).__init__(*args, **kwargs)
 
     def _get_email(self):
         return self.email
@@ -236,8 +236,7 @@ class UserNameAction(UserAction):
         return user
 
 
-# TODO: rename to InviteUser
+class NewUserAction(UserNameAction):
-class NewUser(UserNameAction):
     """
     Setup a new user with a role on the given project.
     Creates the user if they don't exist, otherwise
@@ -434,7 +433,7 @@ class ProjectCreateBase(object):
 
 
 # TODO(adriant): Write tests for this action.
-class NewProject(BaseAction, ProjectCreateBase):
+class NewProjectAction(BaseAction, ProjectCreateBase):
     """
     Creates a new project for the current keystone_user.
 
@@ -447,7 +446,7 @@ class NewProject(BaseAction, ProjectCreateBase):
     ]
 
     def __init__(self, *args, **kwargs):
-        super(NewProject, self).__init__(*args, **kwargs)
+        super(NewProjectAction, self).__init__(*args, **kwargs)
         self.id_manager = user_store.IdentityManager()
 
     def _validate(self):
@@ -464,7 +463,7 @@ class NewProject(BaseAction, ProjectCreateBase):
                 self.add_note(
                     'Parent id does not match keystone user project.')
                 return False
-            return super(NewProject, self)._validate_parent_project()
+            return super(NewProjectAction, self)._validate_parent_project()
         return True
 
     def _pre_approve(self):
@@ -489,7 +488,7 @@ class NewProject(BaseAction, ProjectCreateBase):
             self.add_note("User already given roles.")
         else:
             default_roles = settings.ACTION_SETTINGS.get(
-                'NewProject', {}).get("default_roles", {})
+                'NewProjectAction', {}).get("default_roles", {})
 
             project_id = self.get_cache('project_id')
             keystone_user = self.action.task.keystone_user
@@ -520,7 +519,7 @@ class NewProject(BaseAction, ProjectCreateBase):
         pass
 
 
-class NewProjectWithUser(UserNameAction, ProjectCreateBase):
+class NewProjectWithUserAction(UserNameAction, ProjectCreateBase):
     """
     Makes a new project for the given username. Will create the user if it
     doesn't exists.
@@ -534,7 +533,7 @@ class NewProjectWithUser(UserNameAction, ProjectCreateBase):
     ]
 
     def __init__(self, *args, **kwargs):
-        super(NewProjectWithUser, self).__init__(*args, **kwargs)
+        super(NewProjectWithUserAction, self).__init__(*args, **kwargs)
         self.id_manager = user_store.IdentityManager()
 
     def _validate(self):
@@ -617,7 +616,7 @@ class NewProjectWithUser(UserNameAction, ProjectCreateBase):
                 return
 
             default_roles = settings.ACTION_SETTINGS.get(
-                'NewProject', {}).get("default_roles", {})
+                'NewProjectAction', {}).get("default_roles", {})
 
             project_id = self.get_cache('project_id')
 
@@ -699,7 +698,7 @@ class NewProjectWithUser(UserNameAction, ProjectCreateBase):
                     user_id, project_id))
 
 
-class ResetUser(UserNameAction):
+class ResetUserAction(UserNameAction):
     """
     Simple action to reset a password for a given user.
     """
@@ -713,7 +712,7 @@ class ResetUser(UserNameAction):
     ]
 
     blacklist = settings.ACTION_SETTINGS.get(
-        'ResetUser', {}).get("blacklisted_roles", {})
+        'ResetUserAction', {}).get("blacklisted_roles", {})
 
     def _validate(self):
         id_manager = user_store.IdentityManager()
@@ -773,7 +772,7 @@ class ResetUser(UserNameAction):
         self.add_note('User %s password has been changed.' % self.username)
 
 
-class EditUserRoles(UserIdAction):
+class EditUserRolesAction(UserIdAction):
     """
     A class for adding or removing roles
     on a user for the given project.
@@ -918,8 +917,8 @@ def register_action_class(action_class, serializer_class):
     settings.ACTION_CLASSES.update(data)
 
 # Register each action model
-register_action_class(NewUser, serializers.NewUserSerializer)
+register_action_class(NewUserAction, serializers.NewUserSerializer)
 register_action_class(
-    NewProjectWithUser, serializers.NewProjectWithUserSerializer)
+    NewProjectWithUserAction, serializers.NewProjectWithUserSerializer)
-register_action_class(ResetUser, serializers.ResetUserSerializer)
+register_action_class(ResetUserAction, serializers.ResetUserSerializer)
-register_action_class(EditUserRoles, serializers.EditUserSerializer)
+register_action_class(EditUserRolesAction, serializers.EditUserRolesSerializer)

stacktask/actions/serializers.py

@@ -16,7 +16,7 @@ from rest_framework import serializers
 from django.conf import settings
 
 
-role_options = settings.ACTION_SETTINGS.get("NewUser", {}).get(
+role_options = settings.ACTION_SETTINGS.get("NewUserAction", {}).get(
     "allowed_roles", [])
 
 
@@ -41,7 +41,6 @@ class BaseUserIdSerializer(serializers.Serializer):
 class NewUserSerializer(BaseUserNameSerializer):
     roles = serializers.MultipleChoiceField(choices=role_options)
     project_id = serializers.CharField(max_length=200)
-    pass
 
 
 class NewProjectSerializer(serializers.Serializer):
@@ -60,7 +59,7 @@ class ResetUserSerializer(BaseUserNameSerializer):
     pass
 
 
-class EditUserSerializer(BaseUserIdSerializer):
+class EditUserRolesSerializer(BaseUserIdSerializer):
     roles = serializers.MultipleChoiceField(choices=role_options)
     remove = serializers.BooleanField(default=False)
     project_id = serializers.CharField(max_length=200)

stacktask/actions/tenant_setup/models.py

@@ -13,14 +13,13 @@
 #    under the License.
 
 from stacktask.actions.models import BaseAction
-from stacktask.actions.tenant_setup.serializers import (
+from stacktask.actions.tenant_setup import serializers
-    NewDefaultNetworkSerializer, NewProjectDefaultNetworkSerializer)
 from django.conf import settings
 from stacktask.actions.user_store import IdentityManager
 from stacktask.actions import openstack_clients
 
 
-class NewDefaultNetwork(BaseAction):
+class NewDefaultNetworkAction(BaseAction):
     """
     This action will setup all required basic networking
     resources so that a new user can launch instances
@@ -66,7 +65,7 @@ class NewDefaultNetwork(BaseAction):
         self.add_note('Region: %s exists.' % self.region)
 
         self.defaults = settings.ACTION_SETTINGS.get(
-            'NewDefaultNetwork', {}).get(self.region, {})
+            'NewDefaultNetworkAction', {}).get(self.region, {})
 
         if not self.defaults:
             self.add_note('ERROR: No default settings for given region.')
@@ -177,7 +176,7 @@ class NewDefaultNetwork(BaseAction):
         pass
 
 
-class NewProjectDefaultNetwork(NewDefaultNetwork):
+class NewProjectDefaultNetworkAction(NewDefaultNetworkAction):
     """
     A variant of NewDefaultNetwork that expects the project
     to not be created until after post_approve.
@@ -208,7 +207,7 @@ class NewProjectDefaultNetwork(NewDefaultNetwork):
         self.add_note('Region: %s exists.' % self.region)
 
         self.defaults = settings.ACTION_SETTINGS.get(
-            'NewDefaultNetwork', {}).get(self.region, {})
+            'NewDefaultNetworkAction', {}).get(self.region, {})
 
         if not self.defaults:
             self.add_note('ERROR: No default settings for given region.')
@@ -246,7 +245,7 @@ class NewProjectDefaultNetwork(NewDefaultNetwork):
         self.add_note('Region: %s exists.' % self.region)
 
         self.defaults = settings.ACTION_SETTINGS.get(
-            'NewDefaultNetwork', {}).get(self.region, {})
+            'NewDefaultNetworkAction', {}).get(self.region, {})
 
         if not self.defaults:
             self.add_note('ERROR: No default settings for given region.')
@@ -266,20 +265,19 @@ class NewProjectDefaultNetwork(NewDefaultNetwork):
             self._create_network()
 
 
-class AddDefaultUsersToProject(BaseAction):
+class AddDefaultUsersToProjectAction(BaseAction):
     """
     The purpose of this action is to add a given set of users after
     the creation of a new Project. This is mainly for administrative
     purposes, and for users involved with migrations, monitoring, and
-    general admin tasks that should de present by default.
+    general admin tasks that should be present by default.
     """
 
     def _validate_users(self):
-
         self.users = settings.ACTION_SETTINGS.get(
-            'AddDefaultUsersToProject', {}).get('default_users', [])
+            'AddDefaultUsersToProjectAction', {}).get('default_users', [])
         self.roles = settings.ACTION_SETTINGS.get(
-            'AddDefaultUsersToProject', {}).get('default_roles', [])
+            'AddDefaultUsersToProjectAction', {}).get('default_roles', [])
 
         id_manager = IdentityManager()
 
@@ -306,7 +304,6 @@ class AddDefaultUsersToProject(BaseAction):
             return False
 
     def _validate_project(self):
-
         self.project_id = self.action.task.cache.get('project_id', None)
 
         id_manager = IdentityManager()
@@ -319,10 +316,7 @@ class AddDefaultUsersToProject(BaseAction):
         return True
 
     def _validate(self):
-        if self._validate_users() and self._validate_project():
+        self.action.valid = self._validate_users() and self._validate_project()
-            self.action.valid = True
-        else:
-            self.action.valid = False
         self.action.save()
 
     def _pre_approve(self):
@@ -359,12 +353,15 @@ class AddDefaultUsersToProject(BaseAction):
 
 
 action_classes = {
-    'NewDefaultNetwork': (
+    'NewDefaultNetworkAction':
-        NewDefaultNetwork, NewDefaultNetworkSerializer),
+        (NewDefaultNetworkAction,
-    'NewProjectDefaultNetwork': (
+         serializers.NewDefaultNetworkSerializer),
-        NewProjectDefaultNetwork, NewProjectDefaultNetworkSerializer),
+    'NewProjectDefaultNetworkAction':
-    'AddDefaultUsersToProject': (AddDefaultUsersToProject, None)
+        (NewProjectDefaultNetworkAction,
+         serializers.NewProjectDefaultNetworkSerializer),
+    'AddDefaultUsersToProjectAction':
+        (AddDefaultUsersToProjectAction,
+         serializers.AddDefaultUsersToProjectSerializer)
 }
 
-
 settings.ACTION_CLASSES.update(action_classes)

stacktask/actions/tenant_setup/serializers.py

@@ -24,3 +24,7 @@ class NewDefaultNetworkSerializer(serializers.Serializer):
 class NewProjectDefaultNetworkSerializer(serializers.Serializer):
     setup_network = serializers.BooleanField(default=False)
     region = serializers.CharField(max_length=100)
+
+
+class AddDefaultUsersToProjectSerializer(serializers.Serializer):
+    pass

stacktask/actions/tenant_setup/tests.py

@@ -17,7 +17,8 @@ from django.test import TestCase
 import mock
 
 from stacktask.actions.tenant_setup.models import (
-    NewDefaultNetwork, NewProjectDefaultNetwork, AddDefaultUsersToProject)
+    NewDefaultNetworkAction, NewProjectDefaultNetworkAction,
+    AddDefaultUsersToProjectAction)
 from stacktask.api.models import Task
 from stacktask.api.v1 import tests
 from stacktask.api.v1.tests import FakeManager, setup_temp_cache
@@ -105,7 +106,7 @@ class ProjectSetupActionTests(TestCase):
             'project_id': 'test_project_id',
         }
 
-        action = NewDefaultNetwork(
+        action = NewDefaultNetworkAction(
             data, task=task, order=1)
 
         action.pre_approve()
@@ -156,7 +157,7 @@ class ProjectSetupActionTests(TestCase):
             'project_id': 'test_project_id',
         }
 
-        action = NewDefaultNetwork(
+        action = NewDefaultNetworkAction(
             data, task=task, order=1)
 
         action.pre_approve()
@@ -203,7 +204,7 @@ class ProjectSetupActionTests(TestCase):
             'project_id': 'test_project_id',
         }
 
-        action = NewDefaultNetwork(
+        action = NewDefaultNetworkAction(
             data, task=task, order=1)
 
         action.pre_approve()
@@ -261,7 +262,7 @@ class ProjectSetupActionTests(TestCase):
             'region': 'RegionOne',
         }
 
-        action = NewProjectDefaultNetwork(
+        action = NewProjectDefaultNetworkAction(
             data, task=task, order=1)
 
         action.pre_approve()
@@ -312,7 +313,7 @@ class ProjectSetupActionTests(TestCase):
             'region': 'RegionOne',
         }
 
-        action = NewProjectDefaultNetwork(
+        action = NewProjectDefaultNetworkAction(
             data, task=task, order=1)
 
         action.pre_approve()
@@ -347,7 +348,7 @@ class ProjectSetupActionTests(TestCase):
             'region': 'RegionOne',
         }
 
-        action = NewProjectDefaultNetwork(
+        action = NewProjectDefaultNetworkAction(
             data, task=task, order=1)
 
         action.pre_approve()
@@ -394,7 +395,7 @@ class ProjectSetupActionTests(TestCase):
             'region': 'RegionOne',
         }
 
-        action = NewProjectDefaultNetwork(
+        action = NewProjectDefaultNetworkAction(
             data, task=task, order=1)
 
         action.pre_approve()
@@ -467,7 +468,7 @@ class ProjectSetupActionTests(TestCase):
 
         task.cache = {'project_id': "test_project_id"}
 
-        action = AddDefaultUsersToProject({}, task=task, order=1)
+        action = AddDefaultUsersToProjectAction({}, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, True)
@@ -496,7 +497,7 @@ class ProjectSetupActionTests(TestCase):
 
         task.cache = {'project_id': "test_project_id"}
 
-        action = AddDefaultUsersToProject({}, task=task, order=1)
+        action = AddDefaultUsersToProjectAction({}, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, True)

stacktask/actions/tests.py

@@ -17,7 +17,8 @@ from django.test import TestCase
 import mock
 
 from stacktask.actions.models import (
-    EditUserRoles, NewProjectWithUser, NewUser, ResetUser)
+    EditUserRolesAction, NewProjectWithUserAction, NewUserAction,
+    ResetUserAction)
 from stacktask.api.models import Task
 from stacktask.api.v1 import tests
 from stacktask.api.v1.tests import FakeManager, setup_temp_cache
@@ -50,7 +51,7 @@ class ActionTests(TestCase):
             'roles': ['_member_']
         }
 
-        action = NewUser(data, task=task, order=1)
+        action = NewUserAction(data, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, True)
@@ -101,7 +102,7 @@ class ActionTests(TestCase):
             'roles': ['_member_']
         }
 
-        action = NewUser(data, task=task, order=1)
+        action = NewUserAction(data, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, True)
@@ -148,7 +149,7 @@ class ActionTests(TestCase):
             'roles': ['_member_']
         }
 
-        action = NewUser(data, task=task, order=1)
+        action = NewUserAction(data, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, True)
@@ -183,7 +184,7 @@ class ActionTests(TestCase):
             'roles': ['_member_']
         }
 
-        action = NewUser(data, task=task, order=1)
+        action = NewUserAction(data, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, False)
@@ -218,7 +219,7 @@ class ActionTests(TestCase):
             'project_name': 'test_project',
         }
 
-        action = NewProjectWithUser(data, task=task, order=1)
+        action = NewProjectWithUserAction(data, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, True)
@@ -265,7 +266,7 @@ class ActionTests(TestCase):
             'project_name': 'test_project',
         }
 
-        action = NewProjectWithUser(data, task=task, order=1)
+        action = NewProjectWithUserAction(data, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, True)
@@ -326,7 +327,7 @@ class ActionTests(TestCase):
             'project_name': 'test_project',
         }
 
-        action = NewProjectWithUser(data, task=task, order=1)
+        action = NewProjectWithUserAction(data, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, True)
@@ -378,7 +379,7 @@ class ActionTests(TestCase):
             'project_name': 'test_project',
         }
 
-        action = NewProjectWithUser(data, task=task, order=1)
+        action = NewProjectWithUserAction(data, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, False)
@@ -411,7 +412,7 @@ class ActionTests(TestCase):
             'project_name': 'test_project',
         }
 
-        action = ResetUser(data, task=task, order=1)
+        action = ResetUserAction(data, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, True)
@@ -446,7 +447,7 @@ class ActionTests(TestCase):
             'project_name': 'test_project',
         }
 
-        action = ResetUser(data, task=task, order=1)
+        action = ResetUserAction(data, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, False)
@@ -488,7 +489,7 @@ class ActionTests(TestCase):
             'remove': False
         }
 
-        action = EditUserRoles(data, task=task, order=1)
+        action = EditUserRolesAction(data, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, True)
@@ -534,7 +535,7 @@ class ActionTests(TestCase):
             'remove': False
         }
 
-        action = EditUserRoles(data, task=task, order=1)
+        action = EditUserRolesAction(data, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, True)
@@ -582,7 +583,7 @@ class ActionTests(TestCase):
             'remove': True
         }
 
-        action = EditUserRoles(data, task=task, order=1)
+        action = EditUserRolesAction(data, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, True)
@@ -627,7 +628,7 @@ class ActionTests(TestCase):
             'remove': True
         }
 
-        action = EditUserRoles(data, task=task, order=1)
+        action = EditUserRolesAction(data, task=task, order=1)
 
         action.pre_approve()
         self.assertEquals(action.valid, True)

stacktask/api/startup.py

@@ -15,27 +15,27 @@ def check_expected_taskviews():
                 "Expected taskviews are unregistered: %s" % missing_taskviews))
 
 
-def check_expected_actions():
+def check_configured_actions():
     """Check that all the expected actions have been registered."""
-    expected_actions = []
+    configured_actions = []
 
     for taskview in settings.ACTIVE_TASKVIEWS:
         task_class = settings.TASKVIEW_CLASSES.get(taskview)['class']
 
         try:
-            expected_actions += settings.TASK_SETTINGS.get(
+            configured_actions += settings.TASK_SETTINGS.get(
                 task_class.task_type, {})['default_actions']
         except KeyError:
-            expected_actions += task_class.default_actions
+            configured_actions += task_class.default_actions
-        expected_actions += settings.TASK_SETTINGS.get(
+        configured_actions += settings.TASK_SETTINGS.get(
             task_class.task_type, {}).get('additional_actions', [])
 
     missing_actions = list(
-        set(expected_actions) - set(settings.ACTION_CLASSES.keys()))
+        set(configured_actions) - set(settings.ACTION_CLASSES.keys()))
 
     if missing_actions:
         raise ActionNotFound(
-            "Expected actions are unregistered: %s" % missing_actions)
+            "Configured actions are unregistered: %s" % missing_actions)
 
 
 class APIConfig(AppConfig):
@@ -55,4 +55,4 @@ class APIConfig(AppConfig):
         check_expected_taskviews()
 
         # Now check if all the actions those views expecte are present.
-        check_expected_actions()
+        check_configured_actions()

stacktask/api/v1/openstack.py

@@ -166,14 +166,12 @@ class UserDetail(tasks.TaskView):
 
 class UserRoles(tasks.TaskView):
 
-    default_actions = ['EditUserRoles', ]
+    default_actions = ['EditUserRolesAction', ]
     task_type = 'edit_roles'
 
     @utils.mod_or_admin
     def get(self, request, user_id):
-        """
+        """ Get role info based on the user id. """
-        Get user info based on the user id.
-        """
         id_manager = user_store.IdentityManager()
         user = id_manager.get_user(user_id)
         project_id = request.keystone_user['project_id']
@@ -184,45 +182,30 @@ class UserRoles(tasks.TaskView):
         return Response({"roles": roles})
 
     @utils.mod_or_admin
-    def put(self, request, user_id, format=None):
+    def put(self, args, **kwargs):
-        """
+        """ Add user roles to the current project. """
-        Add user roles to the current project.
+        kwargs['remove_role'] = False
-        """
+        return self._edit_user(args, **kwargs)
-        request.data['remove'] = False
-        if 'project_id' not in request.data:
-            request.data['project_id'] = request.keystone_user['project_id']
-        request.data['user_id'] = user_id
-
-        self.logger.info("(%s) - New EditUserRoles request." % timezone.now())
-        processed, status = self.process_actions(request)
-
-        errors = processed.get('errors', None)
-        if errors:
-            self.logger.info("(%s) - Validation errors with registration." %
-                             timezone.now())
-            return Response(errors, status=status)
-
-        task = processed['task']
-        self.logger.info("(%s) - AutoApproving EditUserRoles request."
-                         % timezone.now())
-        response_dict, status = self.approve(request, task)
-
-        add_task_id_for_roles(request, processed, response_dict, ['admin'])
-
-        return Response(response_dict, status=status)
 
     @utils.mod_or_admin
-    def delete(self, request, user_id, format=None):
+    def delete(self, args, **kwargs):
+        """ Revoke user roles to the current project.
+
+        This only supports Active users
         """
-        Revoke user roles to the current project.
+        kwargs['remove_role'] = True
-        This only supports Active users.
+        return self._edit_user(args, **kwargs)
-        """
+
-        request.data['remove'] = True
+    def _edit_user(self, request, user_id, remove_role=False, format=None):
+        """ Helper function to add or remove roles from a user """
+        request.data['remove'] = remove_role
         if 'project_id' not in request.data:
             request.data['project_id'] = request.keystone_user['project_id']
         request.data['user_id'] = user_id
 
-        self.logger.info("(%s) - New EditUser request." % timezone.now())
+        self.logger.info("(%s) - New EditUser %s request." % (
+            timezone.now(), request.method
+            ))
         processed, status = self.process_actions(request)
 
         errors = processed.get('errors', None)

stacktask/api/v1/tasks.py

@@ -21,6 +21,7 @@ from stacktask.api.v1.views import APIViewWithLogger
 from stacktask.api.v1.utils import (
     send_email, create_notification, create_token, create_task_hash,
     add_task_id_for_roles)
+from stacktask.exceptions import SerializerMissingException
 
 
 from django.conf import settings
@@ -67,6 +68,69 @@ class TaskView(APIViewWithLogger):
         return Response({'actions': actions,
                          'required_fields': required_fields})
 
+    def _instantiate_action_serializers(self, request, class_conf):
+        action_serializer_list = []
+
+        action_names = (
+            class_conf.get('default_actions', []) or
+            self.default_actions[:])
+        action_names += class_conf.get('additional_actions', [])
+
+        # instantiate all action serializers and check validity
+        valid = True
+        for action_name in action_names:
+            action_class, serializer_class = \
+                settings.ACTION_CLASSES[action_name]
+
+            # instantiate serializer class
+            if not serializer_class:
+                raise SerializerMissingException(
+                    "No serializer defined for action %s" % action_name)
+            serializer = serializer_class(data=request.data)
+
+            action_serializer_list.append({
+                'name': action_name,
+                'action': action_class,
+                'serializer': serializer})
+
+            if serializer and not serializer.is_valid():
+                valid = False
+
+        if not valid:
+            errors = {}
+            for action in action_serializer_list:
+                if action['serializer']:
+                    errors.update(action['serializer'].errors)
+            return {'errors': errors}, 400
+
+        return action_serializer_list
+
+    def _handle_duplicates(self, class_conf, hash_key):
+        duplicate_tasks = Task.objects.filter(
+            hash_key=hash_key,
+            completed=0,
+            cancelled=0)
+
+        if not duplicate_tasks:
+            return False
+
+        duplicate_policy = class_conf.get("duplicate_policy", "")
+        if duplicate_policy == "cancel":
+            self.logger.info(
+                "(%s) - Task is a duplicate - Cancelling old tasks." %
+                timezone.now())
+            for task in duplicate_tasks:
+                task.cancelled = True
+                task.save()
+            return False
+
+        self.logger.info(
+            "(%s) - Task is a duplicate - Ignoring new task." %
+            timezone.now())
+        return (
+            {'errors': ['Task is a duplicate of an existing task']},
+            409)
+
     def process_actions(self, request):
         """
         Will ensure the request data contains the required data
@@ -75,91 +139,49 @@ class TaskView(APIViewWithLogger):
         based on running of the the pre_approve validation
         function on all the actions.
         """
-
         class_conf = settings.TASK_SETTINGS.get(
             self.task_type, settings.DEFAULT_TASK_SETTINGS)
 
-        actions = (
+        # Action serializers
-            class_conf.get('default_actions', []) or
+        action_serializer_list = self._instantiate_action_serializers(
-            self.default_actions[:])
+            request, class_conf)
 
-        actions += class_conf.get('additional_actions', [])
+        if isinstance(action_serializer_list, tuple):
+            return action_serializer_list
 
-        action_list = []
+        hash_key = create_task_hash(self.task_type, action_serializer_list)
 
-        valid = True
+        # Handle duplicates
-        for action in actions:
+        duplicate_error = self._handle_duplicates(class_conf, hash_key)
-            action_class, action_serializer = settings.ACTION_CLASSES[action]
+        if duplicate_error:
-
+            return duplicate_error
-            # instantiate serializer class
-            if action_serializer is not None:
-                serializer = action_serializer(data=request.data)
-            else:
-                serializer = None
-
-            action_list.append({
-                'name': action,
-                'action': action_class,
-                'serializer': serializer})
-
-            if serializer is not None and not serializer.is_valid():
-                valid = False
-
-        if not valid:
-            errors = {}
-            for action in action_list:
-                if action['serializer'] is not None:
-                    errors.update(action['serializer'].errors)
-            return {'errors': errors}, 400
-
-        hash_key = create_task_hash(self.task_type, action_list)
-        duplicate_tasks = Task.objects.filter(
-            hash_key=hash_key,
-            completed=0,
-            cancelled=0)
-
-        if duplicate_tasks:
-            duplicate_policy = class_conf.get("handle_duplicates", "")
-            if duplicate_policy == "cancel":
-                self.logger.info(
-                    "(%s) - Task is a duplicate - Cancelling old tasks." %
-                    timezone.now())
-                for task in duplicate_tasks:
-                    task.cancelled = True
-                    task.save()
-            else:
-                self.logger.info(
-                    "(%s) - Task is a duplicate - Ignoring new task." %
-                    timezone.now())
-                return (
-                    {'errors': ['Task is a duplicate of an existing task']},
-                    409)
 
+        # Instantiate Task
         ip_address = request.META['REMOTE_ADDR']
         keystone_user = request.keystone_user
-
         try:
             task = Task.objects.create(
-                ip_address=ip_address, keystone_user=keystone_user,
+                ip_address=ip_address,
+                keystone_user=keystone_user,
                 project_id=keystone_user['project_id'],
                 task_type=self.task_type,
                 hash_key=hash_key)
         except KeyError:
             task = Task.objects.create(
-                ip_address=ip_address, keystone_user=keystone_user,
+                ip_address=ip_address,
+                keystone_user=keystone_user,
                 task_type=self.task_type,
                 hash_key=hash_key)
         task.save()
 
-        for i, action in enumerate(action_list):
+        # Instantiate actions with serializers
-            if action['serializer'] is not None:
+        for i, action in enumerate(action_serializer_list):
-                data = action['serializer'].validated_data
+            data = action['serializer'].validated_data
-            else:
-                data = {}
 
             # construct the action class
             action_instance = action['action'](
-                data=data, task=task,
+                data=data,
+                task=task,
                 order=i
             )
 
@@ -186,12 +208,45 @@ class TaskView(APIViewWithLogger):
                 }
                 return response_dict, 200
 
-        # send initial conformation email:
+        # send initial confirmation email:
         email_conf = class_conf.get('emails', {}).get('initial', None)
         send_email(task, email_conf)
 
         return {'task': task}, 200
 
+    def _create_token(self, task):
+        token = create_token(task)
+        try:
+            class_conf = settings.TASK_SETTINGS.get(
+                self.task_type, settings.DEFAULT_TASK_SETTINGS)
+
+            # will throw a key error if the token template has not
+            # been specified
+            email_conf = class_conf['emails']['token']
+            send_email(task, email_conf, token)
+            return {'notes': ['created token']}, 200
+        except KeyError as e:
+            notes = {
+                'errors':
+                    [("Error: '%s' while sending " +
+                      "token. See task " +
+                      "itself for details.") % e]
+            }
+            create_notification(task, notes, error=True)
+
+            import traceback
+            trace = traceback.format_exc()
+            self.logger.critical(("(%s) - Exception escaped!" +
+                                  " %s\n Trace: \n%s") %
+                                 (timezone.now(), e, trace))
+
+            response_dict = {
+                'errors':
+                    ["Error: Something went wrong on the " +
+                     "server. It will be looked into shortly."]
+            }
+            return response_dict, 500
+
     def approve(self, request, task):
         """
         Approves the task and runs the post_approve steps.
@@ -208,126 +263,90 @@ class TaskView(APIViewWithLogger):
         task.save()
 
         action_models = task.actions
-        actions = []
+        actions = [act.get_action() for act in action_models]
-
-        valid = True
         need_token = False
-        for action in action_models:
-            act = action.get_action()
-            actions.append(act)
 
-            if not act.valid:
+        valid = all([act.valid for act in actions])
-                valid = False
+        if not valid:
-
-        if valid:
-            for action in actions:
-                try:
-                    action.post_approve()
-                except Exception as e:
-                    notes = {
-                        'errors':
-                            [("Error: '%s' while approving task. " +
-                              "See task itself for details.") % e]
-                    }
-                    create_notification(task, notes, error=True)
-
-                    import traceback
-                    trace = traceback.format_exc()
-                    self.logger.critical(("(%s) - Exception escaped! %s\n" +
-                                          "Trace: \n%s") %
-                                         (timezone.now(), e, trace))
-
-                    response_dict = {
-                        'errors':
-                            ["Error: Something went wrong on the server. " +
-                             "It will be looked into shortly."]
-                    }
-                    return response_dict, 500
-
-                if not action.valid:
-                    valid = False
-                if action.need_token:
-                    need_token = True
-
-            if valid:
-                if need_token:
-                    token = create_token(task)
-                    try:
-                        class_conf = settings.TASK_SETTINGS.get(
-                            self.task_type, settings.DEFAULT_TASK_SETTINGS)
-
-                        # will throw a key error if the token template has not
-                        # been specified
-                        email_conf = class_conf['emails']['token']
-                        send_email(task, email_conf, token)
-                        return {'notes': ['created token']}, 200
-                    except KeyError as e:
-                        notes = {
-                            'errors':
-                                [("Error: '%s' while sending " +
-                                  "token. See task " +
-                                  "itself for details.") % e]
-                        }
-                        create_notification(task, notes, error=True)
-
-                        import traceback
-                        trace = traceback.format_exc()
-                        self.logger.critical(("(%s) - Exception escaped!" +
-                                              " %s\n Trace: \n%s") %
-                                             (timezone.now(), e, trace))
-
-                        response_dict = {
-                            'errors':
-                                ["Error: Something went wrong on the " +
-                                 "server. It will be looked into shortly."]
-                        }
-                        return response_dict, 500
-                else:
-                    for action in actions:
-                        try:
-                            action.submit({})
-                        except Exception as e:
-                            notes = {
-                                'errors':
-                                    [("Error: '%s' while submitting " +
-                                      "task. See task " +
-                                      "itself for details.") % e]
-                            }
-                            create_notification(task, notes, error=True)
-
-                            import traceback
-                            trace = traceback.format_exc()
-                            self.logger.critical(("(%s) - Exception escaped!" +
-                                                  " %s\n Trace: \n%s") %
-                                                 (timezone.now(), e, trace))
-
-                            response_dict = {
-                                'errors':
-                                    ["Error: Something went wrong on the " +
-                                     "server. It will be looked into shortly."]
-                            }
-                            return response_dict, 500
-
-                    task.completed = True
-                    task.completed_on = timezone.now()
-                    task.save()
-
-                    # Sending confirmation email:
-                    class_conf = settings.TASK_SETTINGS.get(
-                        self.task_type, settings.DEFAULT_TASK_SETTINGS)
-                    email_conf = class_conf.get(
-                        'emails', {}).get('completed', None)
-                    send_email(task, email_conf)
-                    return {'notes': "Task completed successfully."}, 200
             return {'errors': ['actions invalid']}, 400
-        return {'errors': ['actions invalid']}, 400
+
+        # post_approve all actions
+        for action in actions:
+            try:
+                action.post_approve()
+            except Exception as e:
+                notes = {
+                    'errors':
+                        [("Error: '%s' while approving task. " +
+                          "See task itself for details.") % e]
+                }
+                create_notification(task, notes, error=True)
+
+                import traceback
+                trace = traceback.format_exc()
+                self.logger.critical(("(%s) - Exception escaped! %s\n" +
+                                      "Trace: \n%s") %
+                                     (timezone.now(), e, trace))
+
+                response_dict = {
+                    'errors':
+                        ["Error: Something went wrong on the server. " +
+                         "It will be looked into shortly."]
+                }
+                return response_dict, 500
+
+        valid = all([act.valid for act in actions])
+        if not valid:
+            return {'errors': ['actions invalid']}, 400
+
+        need_token = any([act.need_token for act in actions])
+        if need_token:
+            return self._create_token(task)
+
+        # submit all actions
+        for action in actions:
+            try:
+                action.submit({})
+            except Exception as e:
+                notes = {
+                    'errors':
+                        [("Error: '%s' while submitting " +
+                          "task. See task " +
+                          "itself for details.") % e]
+                }
+                create_notification(task, notes, error=True)
+
+                import traceback
+                trace = traceback.format_exc()
+                self.logger.critical(("(%s) - Exception escaped!" +
+                                      " %s\n Trace: \n%s") %
+                                     (timezone.now(), e, trace))
+
+                response_dict = {
+                    'errors':
+                        ["Error: Something went wrong on the " +
+                         "server. It will be looked into shortly."]
+                }
+                return response_dict, 500
+
+        task.completed = True
+        task.completed_on = timezone.now()
+        task.save()
+
+        # Sending confirmation email:
+        class_conf = settings.TASK_SETTINGS.get(
+            self.task_type, settings.DEFAULT_TASK_SETTINGS)
+        email_conf = class_conf.get(
+            'emails', {}).get('completed', None)
+        send_email(task, email_conf)
+        return {'notes': "Task completed successfully."}, 200
 
 
 class CreateProject(TaskView):
 
     task_type = "create_project"
 
-    default_actions = ["NewProjectWithUser", ]
+    default_actions = ["NewProjectWithUserAction", ]
 
     def post(self, request, format=None):
         """
@@ -374,7 +393,7 @@ class InviteUser(TaskView):
 
     task_type = "invite_user"
 
-    default_actions = ['NewUser', ]
+    default_actions = ['NewUserAction', ]
 
     @utils.mod_or_admin
     def get(self, request):
@@ -396,9 +415,6 @@ class InviteUser(TaskView):
                 request.data['project_id'] is None):
             request.data['project_id'] = request.keystone_user['project_id']
 
-        # TODO: First check if the user already exists or is pending
-        # We should not allow duplicate invites.
-
         processed, status = self.process_actions(request)
 
         errors = processed.get('errors', None)
@@ -422,7 +438,7 @@ class ResetPassword(TaskView):
 
     task_type = "reset_password"
 
-    default_actions = ['ResetUser', ]
+    default_actions = ['ResetUserAction', ]
 
     def post(self, request, format=None):
         """
@@ -473,33 +489,33 @@ class EditUser(TaskView):
 
     task_type = "edit_user"
 
-    default_actions = ['EditUserRoles', ]
+    default_actions = ['EditUserRolesAction', ]
 
     @utils.mod_or_admin
     def get(self, request):
         class_conf = settings.TASK_SETTINGS.get(
             self.task_type, settings.DEFAULT_TASK_SETTINGS)
 
-        actions = (
+        action_names = (
             class_conf.get('default_actions', []) or
             self.default_actions[:])
 
-        actions += class_conf.get('additional_actions', [])
+        action_names += class_conf.get('additional_actions', [])
         role_blacklist = class_conf.get('role_blacklist', [])
 
-        required_fields = []
+        required_fields = set()
 
-        for action in actions:
+        for action_name in action_names:
-            action_class, action_serializer = settings.ACTION_CLASSES[action]
+            action_class, action_serializer = \
-            for field in action_class.required:
+                settings.ACTION_CLASSES[action_name]
-                if field not in required_fields:
+            required_fields |= action_class.required
-                    required_fields.append(field)
 
         user_list = []
         id_manager = IdentityManager()
         project_id = request.keystone_user['project_id']
         project = id_manager.get_project(project_id)
 
+        # todo: move to interface class
         for user in id_manager.list_users(project):
             skip = False
             roles = []
@@ -514,8 +530,8 @@ class EditUser(TaskView):
                               "email": user.username,
                               "roles": roles})
 
-        return Response({'actions': actions,
+        return Response({'actions': action_names,
-                         'required_fields': required_fields,
+                         'required_fields': list(required_fields),
                          'users': user_list})
 
     @utils.mod_or_admin

stacktask/api/v1/tests/test_api_taskview.py

@@ -29,6 +29,42 @@ class TaskViewTests(APITestCase):
     and tokens are created/updated.
     """
 
+    def test_bad_data(self):
+        """
+        Simple test to confirm the serializers are correctly processing
+        wrong data or missing fields.
+        """
+        project = mock.Mock()
+        project.id = 'test_project_id'
+        project.name = 'test_project'
+        project.roles = {}
+
+        setup_temp_cache({'test_project': project}, {})
+
+        url = "/v1/actions/InviteUser"
+        headers = {
+            'project_name': "test_project",
+            'project_id': "test_project_id",
+            'roles': "project_admin,_member_,project_mod",
+            'username': "test@example.com",
+            'user_id': "test_user_id",
+            'authenticated': True
+        }
+        data = {'wrong_email_field': "test@example.com", 'roles': ["_member_"],
+                'project_id': 'test_project_id'}
+        response = self.client.post(url, data, format='json', headers=headers)
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(response.data, {'email': ['This field is required.']})
+
+        data = {'email': "not_a_valid_email", 'roles': ["not_a_valid_role"],
+                'project_id': 'test_project_id'}
+        response = self.client.post(url, data, format='json', headers=headers)
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(
+            response.data, {
+                'email': ['Enter a valid email address.'],
+                'roles': ['"not_a_valid_role" is not a valid choice.']})
+
     @mock.patch('stacktask.actions.models.user_store.IdentityManager',
                 FakeManager)
     def test_new_user(self):

stacktask/exceptions.py

@@ -28,3 +28,7 @@ class TaskViewNotFound(BaseException):
 
 class ActionNotFound(BaseException):
     """Attempting to setup Action that has not been registered."""
+
+
+class SerializerMissingException(BaseException):
+    """ Serializer configured but it does not exist """

stacktask/test_settings.py

@@ -119,14 +119,14 @@ TASK_SETTINGS = {
     },
     'create_project': {
         'additional_actions': [
-            'AddDefaultUsersToProject',
+            'AddDefaultUsersToProjectAction',
-            'NewProjectDefaultNetwork'
+            'NewProjectDefaultNetworkAction'
         ],
         'default_region': 'RegionOne',
         'default_parent_id': None,
     },
     'reset_password': {
-        'handle_duplicates': 'cancel',
+        'duplicate_policy': 'cancel',
         'emails': {
             'token': {
                 'template': 'password_reset_token.txt',
@@ -139,7 +139,7 @@ TASK_SETTINGS = {
         }
     },
     'force_password': {
-        'handle_duplicates': 'cancel',
+        'duplicate_policy': 'cancel',
         'emails': {
             'token': {
                 'template': 'initial_password_token.txt',
@@ -154,18 +154,18 @@ TASK_SETTINGS = {
 }
 
 ACTION_SETTINGS = {
-    'NewProject': {
+    'NewProjectAction': {
         'default_roles': {
             "project_admin", "project_mod", "_member_", "heat_stack_owner"
         },
     },
-    'NewUser': {
+    'NewUserAction': {
         'allowed_roles': ['project_mod', 'project_admin', "_member_"]
     },
-    'ResetUser': {
+    'ResetUserAction': {
         'blacklisted_roles': ['admin']
     },
-    'NewDefaultNetwork': {
+    'NewDefaultNetworkAction': {
         'RegionOne': {
             'DNS_NAMESERVERS': ['193.168.1.2', '193.168.1.3'],
             'SUBNET_CIDR': '192.168.1.0/24',
@@ -175,7 +175,7 @@ ACTION_SETTINGS = {
             'subnet_name': 'somesubnet'
         }
     },
-    'AddDefaultUsersToProject': {
+    'AddDefaultUsersToProjectAction': {
         'default_users': [
             'admin',
         ],