Merge "Refactor Tasks to work as a Subcontroller"

doc/source/webapi/v1.rst

@@ -24,6 +24,9 @@ Stories
 .. rest-controller:: storyboard.api.v1.stories:StoriesController
    :webprefix: /v1/stories
 
+.. rest-controller:: storyboard.api.v1.stories:TasksNestedController
+   :webprefix: /v1/stories/<story_id>/tasks
+
 Comments and Timeline events
 ============================
 .. rest-controller:: storyboard.api.v1.timeline:CommentsController
@@ -34,7 +37,7 @@ Comments and Timeline events
 
 Tasks
 =====
-.. rest-controller:: storyboard.api.v1.tasks:TasksController
+.. rest-controller:: storyboard.api.v1.tasks:TasksPrimaryController
    :webprefix: /v1/tasks
 
 Branches

storyboard/api/v1/stories.py

@@ -26,6 +26,7 @@ import wsmeext.pecan as wsme_pecan
 from storyboard.api.auth import authorization_checks as checks
 from storyboard.api.v1.search import search_engine
 from storyboard.api.v1.tags import TagsController
+from storyboard.api.v1.tasks import TasksNestedController
 from storyboard.api.v1.timeline import CommentsController
 from storyboard.api.v1.timeline import TimeLineEventsController
 from storyboard.api.v1 import validations
@@ -194,6 +195,7 @@ class StoriesController(rest.RestController):
 
     comments = CommentsController()
     events = TimeLineEventsController()
+    tasks = TasksNestedController()
     tags = TagsController()
 
     @decorators.db_exceptions

storyboard/api/v1/tasks.py

@@ -38,7 +38,63 @@ CONF = cfg.CONF
 SEARCH_ENGINE = search_engine.get_engine()
 
 
-class TasksController(rest.RestController):
+def milestone_is_valid(milestone_id):
+        milestone = milestones_api.milestone_get(milestone_id)
+
+        if not milestone:
+            raise exc.NotFound(_("Milestone %d not found.") %
+                               milestone_id)
+
+        if milestone['expired']:
+            abort(400, _("Can't associate task to expired milestone."))
+
+
+def post_timeline_events(original_task, updated_task):
+        # If both the assignee_id and the status were changed there will be
+        # two separate comments in the activity log.
+
+        author_id = request.current_user_id
+        specific_change = False
+
+        if original_task.status != updated_task.status:
+            events_api.task_status_changed_event(
+                story_id=original_task.story_id,
+                task_id=original_task.id,
+                task_title=original_task.title,
+                author_id=author_id,
+                old_status=original_task.status,
+                new_status=updated_task.status)
+            specific_change = True
+
+        if original_task.priority != updated_task.priority:
+            events_api.task_priority_changed_event(
+                story_id=original_task.story_id,
+                task_id=original_task.id,
+                task_title=original_task.title,
+                author_id=author_id,
+                old_priority=original_task.priority,
+                new_priority=updated_task.priority)
+            specific_change = True
+
+        if original_task.assignee_id != updated_task.assignee_id:
+            events_api.task_assignee_changed_event(
+                story_id=original_task.story_id,
+                task_id=original_task.id,
+                task_title=original_task.title,
+                author_id=author_id,
+                old_assignee_id=original_task.assignee_id,
+                new_assignee_id=updated_task.assignee_id)
+            specific_change = True
+
+        if not specific_change:
+            events_api.task_details_changed_event(
+                story_id=original_task.story_id,
+                task_id=original_task.id,
+                task_title=original_task.title,
+                author_id=author_id)
+
+
+class TasksPrimaryController(rest.RestController):
     """Manages tasks."""
 
     _custom_actions = {"search": ["GET"]}
@@ -128,16 +184,6 @@ class TasksController(rest.RestController):
 
         return [wmodels.Task.from_db_model(s) for s in tasks]
 
-    def _milestone_is_valid(self, milestone_id):
-        milestone = milestones_api.milestone_get(milestone_id)
-
-        if not milestone:
-            raise exc.NotFound(_("Milestone %d not found.") %
-                               milestone_id)
-
-        if milestone['expired']:
-            abort(400, _("Can't associate task to expired milestone."))
-
     @decorators.db_exceptions
     @secure(checks.authenticated)
     @wsme_pecan.wsexpose(wmodels.Task, body=wmodels.Task)
@@ -155,7 +201,7 @@ class TasksController(rest.RestController):
                 abort(400,
                       _("Milestones can only be associated with merged tasks"))
 
-            self._milestone_is_valid(task.milestone_id)
+            milestone_is_valid(task.milestone_id)
 
         creator_id = request.current_user_id
         task.creator_id = creator_id
@@ -184,9 +230,6 @@ class TasksController(rest.RestController):
         if task.creator_id and task.creator_id != original_task.creator_id:
             abort(400, _("You can't change author of task."))
 
-        updated_task = tasks_api.task_update(task_id,
-                                             task.as_dict(omit_unset=True))
-
         if task.milestone_id:
             if original_task['status'] != 'merged' and task.status != 'merged':
                 abort(400,
@@ -197,7 +240,7 @@ class TasksController(rest.RestController):
                 abort(400,
                       _("Milestones can only be associated with merged tasks"))
 
-            self._milestone_is_valid(task.milestone_id)
+            milestone_is_valid(task.milestone_id)
 
         task_dict = task.as_dict(omit_unset=True)
 
@@ -207,55 +250,11 @@ class TasksController(rest.RestController):
         updated_task = tasks_api.task_update(task_id, task_dict)
 
         if updated_task:
-            self._post_timeline_events(original_task, updated_task)
+            post_timeline_events(original_task, updated_task)
             return wmodels.Task.from_db_model(updated_task)
         else:
             raise exc.NotFound(_("Task %s not found") % task_id)
 
-    def _post_timeline_events(self, original_task, updated_task):
-        # If both the assignee_id and the status were changed there will be
-        # two separate comments in the activity log.
-
-        author_id = request.current_user_id
-        specific_change = False
-
-        if original_task.status != updated_task.status:
-            events_api.task_status_changed_event(
-                story_id=original_task.story_id,
-                task_id=original_task.id,
-                task_title=original_task.title,
-                author_id=author_id,
-                old_status=original_task.status,
-                new_status=updated_task.status)
-            specific_change = True
-
-        if original_task.priority != updated_task.priority:
-            events_api.task_priority_changed_event(
-                story_id=original_task.story_id,
-                task_id=original_task.id,
-                task_title=original_task.title,
-                author_id=author_id,
-                old_priority=original_task.priority,
-                new_priority=updated_task.priority)
-            specific_change = True
-
-        if original_task.assignee_id != updated_task.assignee_id:
-            events_api.task_assignee_changed_event(
-                story_id=original_task.story_id,
-                task_id=original_task.id,
-                task_title=original_task.title,
-                author_id=author_id,
-                old_assignee_id=original_task.assignee_id,
-                new_assignee_id=updated_task.assignee_id)
-            specific_change = True
-
-        if not specific_change:
-            events_api.task_details_changed_event(
-                story_id=original_task.story_id,
-                task_id=original_task.id,
-                task_title=original_task.title,
-                author_id=author_id)
-
     @decorators.db_exceptions
     @secure(checks.authenticated)
     @wsme_pecan.wsexpose(wmodels.Task, int, status_code=204)
@@ -266,6 +265,9 @@ class TasksController(rest.RestController):
         """
         original_task = tasks_api.task_get(task_id)
 
+        if not original_task:
+            raise exc.NotFound(_("Task %s not found.") % task_id)
+
         events_api.task_deleted_event(
             story_id=original_task.story_id,
             task_id=original_task.id,
@@ -289,3 +291,202 @@ class TasksController(rest.RestController):
                                           limit=limit)
 
         return [wmodels.Task.from_db_model(task) for task in tasks]
+
+
+class TasksNestedController(rest.RestController):
+    """Manages tasks through the /stories/<story_id>/tasks endpoint."""
+
+    validation_post_schema = validations.TASKS_POST_SCHEMA
+    validation_put_schema = validations.TASKS_PUT_SCHEMA
+
+    @decorators.db_exceptions
+    @secure(checks.guest)
+    @wsme_pecan.wsexpose(wmodels.Task, int, int)
+    def get_one(self, story_id, task_id):
+        """Retrieve details about one task.
+
+        :param story_id: An ID of the story.
+        :param task_id: An ID of the task.
+        """
+        task = tasks_api.task_get(task_id)
+
+        if task:
+            if task.story_id != story_id:
+                abort(400, _("URL story_id and task.story_id do not match"))
+            return wmodels.Task.from_db_model(task)
+        else:
+            raise exc.NotFound(_("Task %s not found") % task_id)
+
+    @decorators.db_exceptions
+    @secure(checks.guest)
+    @wsme_pecan.wsexpose([wmodels.Task], int, wtypes.text, int, int, int, int,
+                         int, [wtypes.text], [wtypes.text], int, int,
+                         wtypes.text, wtypes.text)
+    def get_all(self, story_id, title=None, assignee_id=None, project_id=None,
+                project_group_id=None, branch_id=None, milestone_id=None,
+                status=None, priority=None, marker=None, limit=None,
+                sort_field='id', sort_dir='asc'):
+        """Retrieve definitions of all of the tasks.
+
+        :param story_id: filter tasks by story ID.
+        :param title: search by task title.
+        :param assignee_id: filter tasks by who they are assigned to.
+        :param project_id: filter the tasks based on project.
+        :param project_group_id: filter tasks based on project group.
+        :param branch_id: filter tasks based on branch_id.
+        :param milestone_id: filter tasks based on milestone.
+        :param status: filter tasks by status.
+        :param priority: filter tasks by priority.
+        :param marker: The resource id where the page should begin.
+        :param limit: The number of tasks to retrieve.
+        :param sort_field: The name of the field to sort on.
+        :param sort_dir: sort direction for results (asc, desc).
+        """
+
+        # Boundary check on limit.
+        if limit is None:
+            limit = CONF.page_size_default
+        limit = min(CONF.page_size_maximum, max(1, limit))
+
+        # Resolve the marker record.
+        marker_task = tasks_api.task_get(marker)
+
+        tasks = tasks_api \
+            .task_get_all(title=title,
+                          story_id=story_id,
+                          assignee_id=assignee_id,
+                          project_id=project_id,
+                          project_group_id=project_group_id,
+                          branch_id=branch_id,
+                          milestone_id=milestone_id,
+                          status=status,
+                          priority=priority,
+                          sort_field=sort_field,
+                          sort_dir=sort_dir,
+                          marker=marker_task,
+                          limit=limit)
+        task_count = tasks_api \
+            .task_get_count(title=title,
+                            story_id=story_id,
+                            assignee_id=assignee_id,
+                            project_id=project_id,
+                            project_group_id=project_group_id,
+                            branch_id=branch_id,
+                            milestone_id=milestone_id,
+                            status=status,
+                            priority=priority)
+
+        # Apply the query response headers.
+        response.headers['X-Limit'] = str(limit)
+        response.headers['X-Total'] = str(task_count)
+        if marker_task:
+            response.headers['X-Marker'] = str(marker_task.id)
+
+        return [wmodels.Task.from_db_model(s) for s in tasks]
+
+    @decorators.db_exceptions
+    @secure(checks.authenticated)
+    @wsme_pecan.wsexpose(wmodels.Task, int, body=wmodels.Task)
+    def post(self, story_id, task):
+        """Create a new task.
+
+        :param story_id: An ID of the story.
+        :param task: a task within the request body.
+        """
+
+        if task.creator_id and task.creator_id != request.current_user_id:
+            abort(400, _("You can't select author of task."))
+
+        creator_id = request.current_user_id
+        task.creator_id = creator_id
+
+        if not task.story_id:
+            task.story_id = story_id
+
+        if task.story_id != story_id:
+            abort(400, _("URL story_id and task.story_id do not match"))
+
+        if task.milestone_id:
+            if task.status != 'merged':
+                abort(400,
+                      _("Milestones can only be associated with merged tasks"))
+
+            milestone_is_valid(task.milestone_id)
+
+        created_task = tasks_api.task_create(task.as_dict())
+
+        events_api.task_created_event(story_id=task.story_id,
+                                      task_id=created_task.id,
+                                      task_title=created_task.title,
+                                      author_id=creator_id)
+
+        return wmodels.Task.from_db_model(created_task)
+
+    @decorators.db_exceptions
+    @secure(checks.authenticated)
+    @wsme_pecan.wsexpose(wmodels.Task, int, int, body=wmodels.Task)
+    def put(self, story_id, task_id, task):
+        """Modify this task.
+
+        :param story_id: An ID of the story.
+        :param task_id: An ID of the task.
+        :param task: a task within the request body.
+        """
+
+        original_task = tasks_api.task_get(task_id)
+
+        if original_task.story_id != story_id:
+            abort(400, _("URL story_id and task.story_id do not match"))
+
+        if task.creator_id and task.creator_id != original_task.creator_id:
+            abort(400, _("You can't change author of task."))
+
+        if task.milestone_id:
+            if original_task['status'] != 'merged' and task.status != 'merged':
+                abort(400,
+                      _("Milestones can only be associated with merged tasks"))
+
+            if (original_task['status'] == 'merged' and
+                    task.status and task.status != 'merged'):
+                abort(400,
+                      _("Milestones can only be associated with merged tasks"))
+
+            milestone_is_valid(task.milestone_id)
+
+        task_dict = task.as_dict(omit_unset=True)
+
+        if task.status and task.status != 'merged':
+            task_dict['milestone_id'] = None
+
+        updated_task = tasks_api.task_update(task_id, task_dict)
+
+        if updated_task:
+            post_timeline_events(original_task, updated_task)
+            return wmodels.Task.from_db_model(updated_task)
+        else:
+            raise exc.NotFound(_("Task %s not found") % task_id)
+
+    @decorators.db_exceptions
+    @secure(checks.authenticated)
+    @wsme_pecan.wsexpose(wmodels.Task, int, int, status_code=204)
+    def delete(self, story_id, task_id):
+        """Delete this task.
+
+        :param story_id: An ID of the story.
+        :param task_id: An ID of the task.
+        """
+        original_task = tasks_api.task_get(task_id)
+
+        if not original_task:
+            raise exc.NotFound(_("Task %s not found.") % task_id)
+
+        if original_task.story_id != story_id:
+            abort(400, _("URL story_id and task.story_id do not match"))
+
+        events_api.task_deleted_event(
+            story_id=original_task.story_id,
+            task_id=original_task.id,
+            task_title=original_task.title,
+            author_id=request.current_user_id)
+
+        tasks_api.task_delete(task_id)

storyboard/api/v1/v1_controller.py

@@ -24,7 +24,7 @@ from storyboard.api.v1.subscriptions import SubscriptionsController
 from storyboard.api.v1.system_info import SystemInfoController
 from storyboard.api.v1.tags import TagsController
 from storyboard.api.v1.task_statuses import TaskStatusesController
-from storyboard.api.v1.tasks import TasksController
+from storyboard.api.v1.tasks import TasksPrimaryController
 from storyboard.api.v1.teams import TeamsController
 from storyboard.api.v1.users import UsersController
 
@@ -39,7 +39,7 @@ class V1Controller(object):
     milestones = MilestonesController()
     stories = StoriesController()
     tags = TagsController()
-    tasks = TasksController()
+    tasks = TasksPrimaryController()
     task_statuses = TaskStatusesController()
     subscriptions = SubscriptionsController()
     subscription_events = SubscriptionEventsController()

storyboard/tests/api/test_tasks.py

@@ -17,9 +17,9 @@ import six.moves.urllib.parse as urlparse
 from storyboard.tests import base
 
 
-class TestTasks(base.FunctionalTest):
+class TestTasksPrimary(base.FunctionalTest):
     def setUp(self):
-        super(TestTasks, self).setUp()
+        super(TestTasksPrimary, self).setUp()
         self.resource = '/tasks'
 
         self.task_01 = {
@@ -49,6 +49,104 @@ class TestTasks(base.FunctionalTest):
         self.assertEqual(1, created_task['story_id'])
 
 
+class TestTasksNestedController(base.FunctionalTest):
+    def setUp(self):
+        super(TestTasksNestedController, self).setUp()
+        self.resource = '/stories/1/tasks'
+
+        self.task_01 = {
+            'title': 'StoryBoard',
+            'status': 'todo',
+            'story_id': 1
+        }
+        self.default_headers['Authorization'] = 'Bearer valid_superuser_token'
+
+    def test_tasks_endpoint(self):
+        response = self.get_json(self.resource)
+        self.assertEqual(3, len(response))
+
+    def test_get(self):
+        response = self.get_json(self.resource + "/1")
+        # Get an existing task under a given story
+
+        self.assertIsNotNone(response)
+
+    def test_get_for_wrong_story(self):
+        response = self.get_json(self.resource + "/4", expect_errors=True)
+        # Get an existing task under a given story
+
+        self.assertEqual(400, response.status_code)
+
+    def test_create(self):
+        result = self.post_json(self.resource, {
+            'title': 'StoryBoard',
+            'status': 'todo',
+            'story_id': 1
+        })
+
+        # Retrieve the created task
+        created_task = self \
+            .get_json('%s/%d' % ("/tasks", result.json['id']))
+        self.assertEqual(result.json['id'], created_task['id'])
+        self.assertEqual('StoryBoard', created_task['title'])
+        self.assertEqual('todo', created_task['status'])
+        self.assertEqual(1, created_task['story_id'])
+
+    def test_create_id_in_url(self):
+        result = self.post_json(self.resource, {
+            'title': 'StoryBoard',
+            'status': 'todo',
+        })
+        # story_id is not set in the body. URL should handle that
+
+        # Retrieve the created task
+        created_task = self \
+            .get_json('%s/%d' % ("/tasks", result.json['id']))
+        self.assertEqual(result.json['id'], created_task['id'])
+        self.assertEqual('StoryBoard', created_task['title'])
+        self.assertEqual('todo', created_task['status'])
+        self.assertEqual(1, created_task['story_id'])
+
+    def test_create_error(self):
+        result = self.post_json(self.resource, {
+            'title': 'StoryBoard',
+            'status': 'todo',
+            'story_id': 100500
+        }, expect_errors=True)
+
+        # task.story_id does not match the URL
+        self.assertEqual(400, result.status_code)
+
+    def test_update(self):
+        original_task = self.get_json(self.resource)[0]
+        original_id = original_task["id"]
+
+        result = self.put_json(self.resource + "/%s" % original_id, {
+            'title': 'task_updated',
+        })
+
+        self.assertEqual(200, result.status_code)
+
+    def test_update_error(self):
+        original_task = self.get_json(self.resource)[0]
+        original_id = original_task["id"]
+
+        result = self.put_json(self.resource + "/%s" % original_id, {
+            'title': 'task_updated',
+            'story_id': 100500
+        }, expect_errors=True)
+
+        self.assertEqual(400, result.status_code)
+
+    def test_delete(self):
+        result = self.delete(self.resource + "/1")
+        self.assertEqual(204, result.status_code)
+
+    def test_delete_for_wrong_story(self):
+        result = self.delete(self.resource + "/4", expect_errors=True)
+        self.assertEqual(400, result.status_code)
+
+
 class TestTaskSearch(base.FunctionalTest):
     def setUp(self):
         super(TestTaskSearch, self).setUp()