Merge pull request #179 from sartography/feature/proper_changes_history

Proper changes history

crc/models/study.py

@@ -13,6 +13,7 @@ from crc.models.file import FileModel, SimpleFileSchema, FileSchema
 from crc.models.protocol_builder import ProtocolBuilderStatus, ProtocolBuilderStudy
 from crc.models.workflow import WorkflowSpecCategoryModel, WorkflowState, WorkflowStatus, WorkflowSpecModel, \
     WorkflowModel
+from crc.services.user_service import UserService
 
 
 class StudyStatus(enum.Enum):
@@ -28,6 +29,11 @@ class IrbStatus(enum.Enum):
     hsr_assigned = 'hsr number assigned'
 
 
+class StudyEventType(enum.Enum):
+    user = 'user'
+    automatic = 'automatic'
+
+
 class StudyModel(db.Model):
     __tablename__ = 'study'
     id = db.Column(db.Integer, primary_key=True)
@@ -44,6 +50,8 @@ class StudyModel(db.Model):
     requirements = db.Column(db.ARRAY(db.Integer), nullable=True)
     on_hold = db.Column(db.Boolean, default=False)
     enrollment_date = db.Column(db.DateTime(timezone=True), nullable=True)
+    # events = db.relationship("TaskEventModel")
+    events_history = db.relationship("StudyEvent", cascade="all, delete, delete-orphan")
 
     def update_from_protocol_builder(self, pbs: ProtocolBuilderStudy):
         self.hsr_number = pbs.HSRNUMBER
@@ -60,6 +68,18 @@ class StudyModel(db.Model):
             self.status = StudyStatus.hold
 
 
+class StudyEvent(db.Model):
+    __tablename__ = 'study_event'
+    id = db.Column(db.Integer, primary_key=True)
+    study_id = db.Column(db.Integer, db.ForeignKey(StudyModel.id), nullable=False)
+    study = db.relationship(StudyModel, back_populates='events_history')
+    create_date = db.Column(db.DateTime(timezone=True), default=func.now())
+    status = db.Column(db.Enum(StudyStatus))
+    comment = db.Column(db.String, default='')
+    event_type = db.Column(db.Enum(StudyEventType))
+    user_uid = db.Column(db.String, db.ForeignKey('user.uid'), nullable=True)
+
+
 class WorkflowMetadata(object):
     def __init__(self, id, name = None, display_name = None, description = None, spec_version = None,
                  category_id  = None, category_display_name  = None, state: WorkflowState  = None,
@@ -128,15 +148,16 @@ class CategorySchema(ma.Schema):
 class Study(object):
 
     def __init__(self, title, last_updated, primary_investigator_id, user_uid,
-                 id=None, status=None, irb_status=None,
+                 id=None, status=None, irb_status=None, comment="",
                  sponsor="", hsr_number="", ind_number="", categories=[],
-                 files=[], approvals=[], enrollment_date=None, **argsv):
+                 files=[], approvals=[], enrollment_date=None, events_history=[], **argsv):
         self.id = id
         self.user_uid = user_uid
         self.title = title
         self.last_updated = last_updated
         self.status = status
         self.irb_status = irb_status
+        self.comment = comment
         self.primary_investigator_id = primary_investigator_id
         self.sponsor = sponsor
         self.hsr_number = hsr_number
@@ -146,11 +167,13 @@ class Study(object):
         self.warnings = []
         self.files = files
         self.enrollment_date = enrollment_date
+        self.events_history = events_history
 
     @classmethod
     def from_model(cls, study_model: StudyModel):
         id = study_model.id # Just read some value, in case the dict expired, otherwise dict may be empty.
         args = dict((k, v) for k, v in study_model.__dict__.items() if not k.startswith('_'))
+        args['events_history'] = study_model.events_history  # For some reason this attribute is not picked up
         instance = cls(**args)
         return instance
 
@@ -165,18 +188,15 @@ class Study(object):
         if status == StudyStatus.open_for_enrollment:
             study_model.enrollment_date = self.enrollment_date
 
-        # change = {
+        study_event = StudyEvent(
-        #     'status': ProtocolBuilderStatus(self.protocol_builder_status).value,
+            study=study_model,
-        #     'comment': '' if not hasattr(self, 'comment') else self.comment,
+            status=status,
-        #     'date': str(datetime.datetime.now())
+            comment='' if not hasattr(self, 'comment') else self.comment,
-        # }
+            event_type=StudyEventType.user,
-
+            user_uid=UserService.current_user().uid if UserService.has_user() else None,
-        # if study_model.changes_history:
+        )
-        #     changes_history = json.loads(study_model.changes_history)
+        db.session.add(study_event)
-        #     changes_history.append(change)
+        db.session.commit()
-        # else:
-        #     changes_history = [change]
-        # study_model.changes_history = json.dumps(changes_history)
 
 
     def model_args(self):
@@ -207,6 +227,15 @@ class StudyForUpdateSchema(ma.Schema):
         return Study(**data)
 
 
+class StudyEventSchema(ma.Schema):
+
+    id = fields.Integer(required=False)
+    create_date = fields.DateTime()
+    status = EnumField(StudyStatus, by_value=True)
+    comment = fields.String(allow_none=True)
+    event_type = EnumField(StudyEvent, by_value=True)
+
+
 class StudySchema(ma.Schema):
 
     id = fields.Integer(required=False, allow_none=True)
@@ -220,11 +249,13 @@ class StudySchema(ma.Schema):
     files = fields.List(fields.Nested(FileSchema), dump_only=True)
     approvals = fields.List(fields.Nested('ApprovalSchema'), dump_only=True)
     enrollment_date = fields.Date(allow_none=True)
+    events_history = fields.List(fields.Nested('StudyEventSchema'), dump_only=True)
 
     class Meta:
         model = Study
         additional = ["id", "title", "last_updated", "primary_investigator_id", "user_uid",
-                      "sponsor", "ind_number", "approvals", "files", "enrollment_date"]
+                      "sponsor", "ind_number", "approvals", "files", "enrollment_date",
+                      "events_history"]
         unknown = INCLUDE
 
     @marshmallow.post_load

crc/services/study_service.py

@@ -12,7 +12,7 @@ from crc.api.common import ApiError
 from crc.models.file import FileModel, FileModelSchema, File
 from crc.models.ldap import LdapSchema
 from crc.models.protocol_builder import ProtocolBuilderStudy, ProtocolBuilderStatus
-from crc.models.study import StudyModel, Study, StudyStatus, Category, WorkflowMetadata
+from crc.models.study import StudyModel, Study, StudyStatus, Category, WorkflowMetadata, StudyEvent
 from crc.models.task_event import TaskEventModel, TaskEvent
 from crc.models.workflow import WorkflowSpecCategoryModel, WorkflowModel, WorkflowSpecModel, WorkflowState, \
     WorkflowStatus
@@ -53,6 +53,7 @@ class StudyService(object):
         loading up and executing all the workflows in a study to calculate information."""
         if not study_model:
             study_model = session.query(StudyModel).filter_by(id=study_id).first()
+
         study = Study.from_model(study_model)
         study.categories = StudyService.get_categories()
         workflow_metas = StudyService.__get_workflow_metas(study_id)
@@ -77,9 +78,11 @@ class StudyService(object):
     @staticmethod
     def delete_study(study_id):
         session.query(TaskEventModel).filter_by(study_id=study_id).delete()
+        # session.query(StudyEvent).filter_by(study_id=study_id).delete()
         for workflow in session.query(WorkflowModel).filter_by(study_id=study_id):
             StudyService.delete_workflow(workflow)
-        session.query(StudyModel).filter_by(id=study_id).delete()
+        study = session.query(StudyModel).filter_by(id=study_id).first()
+        session.delete(study)
         session.commit()
 
     @staticmethod

migrations/versions/69081f1ff387_.py

@@ -0,0 +1,41 @@
+"""empty message
+
+Revision ID: 69081f1ff387
+Revises: 1c3f88dbccc3
+Create Date: 2020-08-12 09:58:36.886096
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '69081f1ff387'
+down_revision = '1c3f88dbccc3'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.create_table('study_event',
+    sa.Column('id', sa.Integer(), nullable=False),
+    sa.Column('study_id', sa.Integer(), nullable=False),
+    sa.Column('create_date', sa.DateTime(timezone=True), nullable=True),
+    sa.Column('status', sa.Enum('in_progress', 'hold', 'open_for_enrollment', 'abandoned', name='studystatusenum'), nullable=True),
+    sa.Column('comment', sa.String(), nullable=True),
+    sa.Column('event_type', sa.Enum('user', 'automatic', name='studyeventtype'), nullable=True),
+    sa.Column('user_uid', sa.String(), nullable=True),
+    sa.ForeignKeyConstraint(['study_id'], ['study.id'], ),
+    sa.ForeignKeyConstraint(['user_uid'], ['user.uid'], ),
+    sa.PrimaryKeyConstraint('id')
+    )
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_table('study_event')
+    op.execute('drop type studystatusenum')
+    op.execute('drop type studyeventtype')
+    # ### end Alembic commands ###

tests/study/test_study_api.py

@@ -11,7 +11,7 @@ from crc.models.protocol_builder import ProtocolBuilderStatus, \
     ProtocolBuilderStudySchema
 from crc.models.approval import ApprovalStatus
 from crc.models.task_event import TaskEventModel
-from crc.models.study import StudyModel, StudySchema, StudyStatus
+from crc.models.study import StudyEvent, StudyModel, StudySchema, StudyStatus, StudyEventType
 from crc.models.workflow import WorkflowSpecModel, WorkflowModel
 from crc.services.file_service import FileService
 from crc.services.workflow_processor import WorkflowProcessor
@@ -134,10 +134,12 @@ class TestStudyApi(BaseTest):
 
     def test_update_study(self):
         self.load_example_data()
+        update_comment = 'Updating the study'
         study: StudyModel = session.query(StudyModel).first()
         study.title = "Pilot Study of Fjord Placement for Single Fraction Outcomes to Cortisol Susceptibility"
         study_schema = StudySchema().dump(study)
         study_schema['status'] = StudyStatus.in_progress.value
+        study_schema['comment'] = update_comment
         rv = self.app.put('/v1.0/study/%i' % study.id,
                           content_type="application/json",
                           headers=self.logged_in_headers(),
@@ -147,6 +149,13 @@ class TestStudyApi(BaseTest):
         self.assertEqual(study.title, json_data['title'])
         self.assertEqual(study.status.value, json_data['status'])
 
+        # Making sure events history is being properly recorded
+        study_event = session.query(StudyEvent).first()
+        self.assertIsNotNone(study_event)
+        self.assertEqual(study_event.status, StudyStatus.in_progress)
+        self.assertEqual(study_event.comment, update_comment)
+        self.assertEqual(study_event.user_uid, self.test_uid)
+
     @patch('crc.services.protocol_builder.ProtocolBuilderService.get_investigators')  # mock_studies
     @patch('crc.services.protocol_builder.ProtocolBuilderService.get_required_docs')  # mock_docs
     @patch('crc.services.protocol_builder.ProtocolBuilderService.get_study_details')  # mock_details
@@ -245,8 +254,15 @@ class TestStudyApi(BaseTest):
     def test_delete_study_with_workflow_and_status(self):
         self.load_example_data()
         workflow = session.query(WorkflowModel).first()
+        stats1 = StudyEvent(
+            study_id=workflow.study_id,
+            status=StudyStatus.in_progress,
+            comment='Some study status change event',
+            event_type=StudyEventType.user,
+            user_uid=self.users[0]['uid'],
+        )
         stats2 = TaskEventModel(study_id=workflow.study_id, workflow_id=workflow.id, user_uid=self.users[0]['uid'])
-        session.add(stats2)
+        session.add_all([stats1, stats2])
         session.commit()
         rv = self.app.delete('/v1.0/study/%i' % workflow.study_id, headers=self.logged_in_headers())
         self.assert_success(rv)

tests/study/test_study_details_documents.py

@@ -19,7 +19,7 @@ class TestStudyDetailsDocumentsScript(BaseTest):
 
     """
             1. get a list of all documents related to the study.
-            2. For this study, is this document required accroding to the protocol builder? 
+            2. For this study, is this document required accroding to the protocol builder?
             3. For ALL uploaded documents, what the total number of files that were uploaded? per instance of this document naming
             convention that we are implementing for the IRB.
     """

tests/study/test_study_service.py

@@ -84,7 +84,7 @@ class TestStudyService(BaseTest):
         # Assure the workflow is now started, and knows the total and completed tasks.
         studies = StudyService.get_studies_for_user(user)
         workflow = next(iter(studies[0].categories[0].workflows)) # Workflows is a set.
-#        self.assertEqual(WorkflowStatus.user_input_required, workflow.status)
+        # self.assertEqual(WorkflowStatus.user_input_required, workflow.status)
         self.assertTrue(workflow.total_tasks > 0)
         self.assertEqual(0, workflow.completed_tasks)
         self.assertIsNotNone(workflow.spec_version)