Add Task History and GET Task API (#53)

* Working migrate

* Working tests

* Works virtually :)

* Working db

* Fix db

* Add locked status

* Working roughly

* Fix broken tests

* Refactor

* Working with tests

* Fix tests (hopefully)

* Fix again

* Fix tests (hopefully)

* Fix tests

* Working get task

* Inital working get task API

* Tidy up code

* Tidyups after review

migrations/versions/45d483cb16dc_.py → migrations/versions/b93421cec5bd_.py

@@ -1,8 +1,8 @@
 """empty message
 
-Revision ID: 45d483cb16dc
+Revision ID: b93421cec5bd
 Revises: 
-Create Date: 2017-03-08 14:25:50.618066
+Create Date: 2017-03-10 14:25:33.683640
 
 """
 from alembic import op
@@ -11,7 +11,7 @@
 
 
 # revision identifiers, used by Alembic.
-revision = '45d483cb16dc'
+revision = 'b93421cec5bd'
 down_revision = None
 branch_labels = None
 depends_on = None
@@ -47,11 +47,24 @@ def upgrade():
     sa.PrimaryKeyConstraint('id', 'project_id')
     )
     op.create_index(op.f('ix_tasks_project_id'), 'tasks', ['project_id'], unique=False)
+    op.create_table('task_history',
+    sa.Column('id', sa.Integer(), nullable=False),
+    sa.Column('task_id', sa.Integer(), nullable=False),
+    sa.Column('project_id', sa.Integer(), nullable=False),
+    sa.Column('action', sa.String(), nullable=False),
+    sa.Column('action_text', sa.String(), nullable=True),
+    sa.Column('action_date', sa.DateTime(), nullable=False),
+    sa.ForeignKeyConstraint(['task_id', 'project_id'], ['tasks.id', 'tasks.project_id'], ),
+    sa.PrimaryKeyConstraint('id')
+    )
+    op.create_index('idx_task_history_composite', 'task_history', ['task_id', 'project_id'], unique=False)
     # ### end Alembic commands ###
 
 
 def downgrade():
     # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_index('idx_task_history_composite', table_name='task_history')
+    op.drop_table('task_history')
     op.drop_index(op.f('ix_tasks_project_id'), table_name='tasks')
     op.drop_table('tasks')
     op.drop_table('projects')

server/__init__.py

@@ -77,11 +77,12 @@ def init_flask_restful_routes(app):
     from server.api.health_check_api import HealthCheckAPI
     from server.api.projects_api import ProjectsAPI
     from server.api.swagger_docs_api import SwaggerDocsAPI
-    from server.api.tasks_api import LockTaskAPI, UnlockTaskAPI
+    from server.api.tasks_api import TaskAPI, LockTaskAPI, UnlockTaskAPI
 
     api.add_resource(SwaggerDocsAPI, '/api/docs')
     api.add_resource(HealthCheckAPI, '/api/health-check')
     api.add_resource(ProjectsAPI,    '/api/v1/project', methods=['PUT'])
     api.add_resource(ProjectsAPI,    '/api/v1/project/<int:project_id>', endpoint="get", methods=['GET'])
+    api.add_resource(TaskAPI,        '/api/v1/project/<int:project_id>/task/<int:task_id>')
     api.add_resource(LockTaskAPI,    '/api/v1/project/<int:project_id>/task/<int:task_id>/lock')
     api.add_resource(UnlockTaskAPI,  '/api/v1/project/<int:project_id>/task/<int:task_id>/unlock')

server/api/tasks_api.py

@@ -1,7 +1,51 @@
-from flask_restful import Resource, current_app
+from flask_restful import Resource, current_app, request
 from server.services.task_service import TaskService, TaskServiceError
 
 
+class TaskAPI(Resource):
+
+    def get(self, project_id, task_id):
+        """
+        Get task details
+        ---
+        tags:
+            - tasks
+        produces:
+            - application/json
+        parameters:
+            - name: project_id
+              in: path
+              description: The ID of the project the task is associated with
+              required: true
+              type: integer
+              default: 1
+            - name: task_id
+              in: path
+              description: The unique task ID
+              required: true
+              type: integer
+              default: 1
+        responses:
+            200:
+                description: Task found
+            404:
+                description: Task not found
+            500:
+                description: Internal Server Error
+        """
+        try:
+            task = TaskService().get_task_as_dto(task_id, project_id)
+
+            if task is None:
+                return {"Error": "Task Not Found"}, 404
+
+            return task, 200
+        except Exception as e:
+            error_msg = f'Task GET API - unhandled error: {str(e)}'
+            current_app.logger.critical(error_msg)
+            return {"Error": error_msg}, 500
+
+
 class LockTaskAPI(Resource):
 
     def post(self, project_id, task_id):
@@ -36,7 +80,7 @@ def post(self, project_id, task_id):
                 description: Internal Server Error
         """
         try:
-            task = TaskService.set_locked_status(task_id=task_id, project_id=project_id, is_locked=True)
+            task = TaskService().lock_task(task_id, project_id)
 
             if task is None:
                 return {"Error": "Task Not Found"}, 404
@@ -73,21 +117,49 @@ def post(self, project_id, task_id):
               required: true
               type: integer
               default: 1
+            - in: body
+              name: body
+              required: true
+              description: JSON object for unlocking a task
+              schema:
+                  id: TaskUpdate
+                  required:
+                      - status
+                  properties:
+                      status:
+                          type: string
+                          description: The new status for the task
+                          default: DONE
+                      comment:
+                          type: string
+                          description: Optional user comment about the task
+                          default: Mapping makes me feel good!
         responses:
             200:
                 description: Task unlocked
+            400:
+                description: Client Error
             404:
                 description: Task not found
             500:
                 description: Internal Server Error
         """
         try:
-            task = TaskService.set_locked_status(task_id=task_id, project_id=project_id, is_locked=False)
+            data = request.get_json()
+            status = data['status']
+            comment = data['comment'] if 'comment' in data else None
+
+            if status == '':
+                return {"Error": "Status not supplied"}, 400
+
+            task = TaskService().unlock_task(task_id, project_id, status, comment)
 
             if task is None:
                 return {"Error": "Task Not Found"}, 404
 
             return {"Status": "Success"}, 200
+        except TaskServiceError as e:
+            return {"Error": str(e)}, 400
         except Exception as e:
             error_msg = f'Task Lock API - unhandled error: {str(e)}'
             current_app.logger.critical(error_msg)

server/config.py

@@ -1,13 +1,15 @@
 import logging
 import os
+from server.models.utils import DateTimeEncoder
 
 
 class EnvironmentConfig:
     """
     Base class for config that is shared between environments
     """
     LOG_LEVEL = logging.ERROR
-    SQLALCHEMY_DATABASE_URI = os.environ['TASKING_MANAGER_DB']
+    RESTFUL_JSON = {'cls': DateTimeEncoder}
+    SQLALCHEMY_DATABASE_URI = os.getenv('TASKING_MANAGER_DB', None)
 
 
 class StagingConfig(EnvironmentConfig):

server/models/project.py

@@ -1,10 +1,9 @@
-import datetime
 import geojson
 from enum import Enum
 from geoalchemy2 import Geometry
 from server import db
 from server.models.task import Task
-from server.models.utils import InvalidData, InvalidGeoJson, ST_SetSRID, ST_GeomFromGeoJSON
+from server.models.utils import InvalidData, InvalidGeoJson, ST_SetSRID, ST_GeomFromGeoJSON, current_datetime
 
 
 class AreaOfInterest(db.Model):
@@ -58,7 +57,7 @@ class Project(db.Model):
     aoi_id = db.Column(db.Integer, db.ForeignKey('areas_of_interest.id'))
     area_of_interest = db.relationship(AreaOfInterest, cascade="all")
     tasks = db.relationship(Task, backref='projects', cascade="all, delete, delete-orphan")
-    created = db.Column(db.DateTime, default=datetime.datetime.utcnow)
+    created = db.Column(db.DateTime, default=current_datetime())
 
     def __init__(self, project_name, aoi):
         """
@@ -92,7 +91,7 @@ def delete(self):
     @staticmethod
     def as_dto(project_id):
         """
-        Creates a Project DTO suitable of transmitting via the API
+        Creates a Project DTO suitable for transmitting via the API
         :param project_id: project_id in scope
         :return: Project DTO dict
         """

server/models/task.py

@@ -1,14 +1,72 @@
+import datetime
 import geojson
 from enum import Enum
 from geoalchemy2 import Geometry
 from server import db
-from server.models.utils import InvalidData, InvalidGeoJson, ST_GeomFromGeoJSON, ST_SetSRID
+from server.models.utils import InvalidData, InvalidGeoJson, ST_GeomFromGeoJSON, ST_SetSRID, current_datetime
 
 
-class TaskStatus(Enum):
+class TaskAction(Enum):
+    """
+    Describes the possible actions that can happen to to a task, that we'll record history for
+    """
+    LOCKED = 1
+    STATE_CHANGE = 2
+    COMMENT = 3
+
+
+class TaskHistory(db.Model):
     """
-    Enum describing available Task Statuses
+    Describes the history associated with a task
     """
+    __tablename__ = "task_history"
+
+    project_id = db.Column(db.Integer, db.ForeignKey('projects.id'), index=True, primary_key=True)
+
+    id = db.Column(db.Integer, primary_key=True)
+    task_id = db.Column(db.Integer, nullable=False)
+    project_id = db.Column(db.Integer, nullable=False)
+    action = db.Column(db.String, nullable=False)
+    action_text = db.Column(db.String)
+    action_date = db.Column(db.DateTime, nullable=False, default=current_datetime())
+
+    __table_args__ = (db.ForeignKeyConstraint([task_id, project_id], ['tasks.id', 'tasks.project_id']),
+                      db.Index('idx_task_history_composite', 'task_id', 'project_id'), {})
+
+    def __init__(self, task_id, project_id):
+        self.task_id = task_id
+        self.project_id = project_id
+
+    def set_task_locked_action(self):
+        self.action = TaskAction.LOCKED.name
+
+    @staticmethod
+    def update_task_locked_with_duration(task_id, project_id):
+        """
+        Calculates the duration a task was locked for and sets it on the history record
+        :param task_id: Task in scope
+        :param project_id: Project ID in scope
+        :return:
+        """
+        last_locked = TaskHistory.query.filter_by(task_id=task_id, project_id=project_id, action=TaskAction.LOCKED.name,
+                                                  action_text=None).one()
+
+        duration_task_locked = datetime.datetime.utcnow() - last_locked.action_date
+        # Cast duration to isoformat for later transmission via api
+        last_locked.action_text = (datetime.datetime.min + duration_task_locked).time().isoformat()
+        db.session.commit()
+
+    def set_comment_action(self, comment):
+        self.action = TaskAction.COMMENT.name
+        self.action_text = comment
+
+    def set_state_change_action(self, new_state):
+        self.action = TaskAction.STATE_CHANGE.name
+        self.action_text = new_state.name
+
+
+class TaskStatus(Enum):
+    """ Enum describing available Task Statuses """
     READY = 0
     INVALIDATED = 1
     DONE = 2
@@ -18,9 +76,8 @@ class TaskStatus(Enum):
 
 
 class Task(db.Model):
-    """
-    Describes an individual mapping Task
-    """
+    """ Describes an individual mapping Task """
+
     __tablename__ = "tasks"
 
     # Table has composite PK on (id and project_id)
@@ -32,6 +89,7 @@ class Task(db.Model):
     geometry = db.Column(Geometry('MULTIPOLYGON', srid=4326))
     task_status = db.Column(db.Integer, default=TaskStatus.READY.value)
     task_locked = db.Column(db.Boolean, default=False)
+    task_history = db.relationship(TaskHistory, cascade="all")
 
     @classmethod
     def from_geojson_feature(cls, task_id, task_feature):
@@ -68,21 +126,30 @@ def from_geojson_feature(cls, task_id, task_feature):
         return task
 
     @staticmethod
-    def get(project_id, task_id):
+    def get(task_id, project_id):
         """
         Gets specified task
-        :param project_id: project ID in scope
         :param task_id: task ID in scope
+        :param project_id: project ID in scope
         :return: Task if found otherwise None
         """
         return Task.query.filter_by(id=task_id, project_id=project_id).one_or_none()
 
     def update(self):
-        """
-        Updates the DB with the current state of the Task
-        """
+        """ Updates the DB with the current state of the Task """
         db.session.commit()
 
+    def lock_task(self):
+        """ Lock task and save in DB  """
+        self.task_locked = True
+        self.update()
+
+    def unlock_task(self):
+        """ Unlock task and ensure duration task locked is saved in History """
+        TaskHistory.update_task_locked_with_duration(self.id, self.project_id)
+        self.task_locked = False
+        self.update()
+
     @staticmethod
     def get_tasks_as_geojson_feature_collection(project_id):
         """
@@ -103,3 +170,29 @@ def get_tasks_as_geojson_feature_collection(project_id):
             tasks_features.append(feature)
 
         return geojson.FeatureCollection(tasks_features)
+
+    @staticmethod
+    def as_dto(task_id, project_id):
+        """
+        Creates a Task DTO suitable for transmitting via the API
+        :param task_id: Task ID in scope
+        :param project_id: Project ID in scope
+        :return: JSON serializable Task DTO
+        """
+        task = Task.get(task_id, project_id)
+
+        if task is None:
+            return None
+
+        task_history = []
+        for action in task.task_history:
+            if action.action_text is None:
+                continue  # Don't return any history without action text
+
+            history = dict(action=action.action, actionText=action.action_text, actionDate=action.action_date)
+            task_history.append(history)
+
+        task_dto = dict(taskId=task.id, projectId=task.project_id, taskStatus=TaskStatus(task.task_status).name,
+                        taskLocked=task.task_locked, taskHistory=task_history)
+
+        return task_dto