Made some modifications to the Approval so that it knows exactly what versions of every file are being sent for approval

Added the following columns:
  * date_created - so we know when the file was created
  * renamed workflow_version to just "version", because everything has a version,  this is the version of the request.
  * workflow_hash - this is just a quick way to see what files and versions are associated with the request, it could be factored out.
  * study - a quick relationship link to the study, so that this model is easier to use.
  * workflow - ditto
  * approval_files - these is a list from a new link table that links an approval to specific files and versions.

The RequestApproval is logically sound, but still needs some additional pieces in place to be callable from a BPMN workflow diagram.

Altered the file service to pick up on changes to files vs adding new files, so that versions are picked up correctly as
users modify their submission - adding new files or replacing existing ones.  Deleting files worries me, and I will need to revisit this.

The damn base test keeps giving me a headache, so I made changes there to see if clearing and dropping the database each time won't allow the tests to pass more consistently.

Lots more tests around the file service to make sure it is versioning user uploaded files correctly.

The "Test Request Approval Script" tries to find to assure the correct behavior as this is likely to be called many times repeatedly and with little knowledge of the internal system.  So it should just "do the right thing".

crc/models/approval.py

@@ -1,8 +1,10 @@
 import enum
 
 from marshmallow import INCLUDE
+from sqlalchemy import func
 
 from crc import db, ma
+from crc.models.file import FileModel
 from crc.models.study import StudyModel
 from crc.models.workflow import WorkflowModel
 
@@ -14,16 +16,32 @@ class ApprovalStatus(enum.Enum):
     CANCELED = "CANCELED" # The document was replaced with a new version and this review is no longer needed.
 
 
+class ApprovalFile(db.Model):
+    id = db.Column(db.Integer, primary_key=True)
+    file_id = db.Column(db.Integer, db.ForeignKey(FileModel.id), nullable=False)
+    approval_id = db.Column(db.Integer, db.ForeignKey("approval.id"), nullable=False)
+    file_version = db.Column(db.Integer, nullable=False)
+
+    approval = db.relationship("ApprovalModel")
+    file = db.relationship(FileModel)
+
+
 class ApprovalModel(db.Model):
     __tablename__ = 'approval'
     id = db.Column(db.Integer, primary_key=True)
     study_id = db.Column(db.Integer, db.ForeignKey(StudyModel.id), nullable=False)
-    study = db.relationship(StudyModel, backref='approval')
     workflow_id = db.Column(db.Integer, db.ForeignKey(WorkflowModel.id), nullable=False)
-    workflow_version = db.Column(db.String)
     approver_uid = db.Column(db.String)  # Not linked to user model, as they may not have logged in yet.
     status = db.Column(db.String)
     message = db.Column(db.String)
+    date_created = db.Column(db.DateTime(timezone=True), default=func.now())
+    version = db.Column(db.Integer) # Incremented integer, so 1,2,3 as requests are made.
+    workflow_hash = db.Column(db.String) # A hash of the workflow at the moment the approval was created.
+
+    study = db.relationship(StudyModel)
+    workflow = db.relationship(WorkflowModel)
+    approval_files = db.relationship(ApprovalFile, back_populates="approval")
+
 
 
 class Approval(object):
@@ -31,9 +49,8 @@ class Approval(object):
     @classmethod
     def from_model(cls, model: ApprovalModel):
         instance = cls()
-
         instance.id = model.id
-        instance.workflow_version = model.workflow_version
+        instance.version = model.version
         instance.approver_uid = model.approver_uid
         instance.status = model.status
         instance.study_id = model.study_id

crc/models/file.py

@@ -85,6 +85,7 @@ class FileModel(db.Model):
     latest_version = db.Column(db.Integer, default=0)
 
 
+
 class FileModelSchema(SQLAlchemyAutoSchema):
     class Meta:
         model = FileModel

crc/models/workflow.py

@@ -80,5 +80,3 @@ class WorkflowModel(db.Model):
     total_tasks = db.Column(db.Integer, default=0)
     completed_tasks = db.Column(db.Integer, default=0)
     last_updated = db.Column(db.DateTime)
-    # todo: Add a version that represents the files associated with this workflow
-    # version = "32"

crc/scripts/request_review.py

@@ -0,0 +1,71 @@
+from datetime import datetime
+
+from sqlalchemy import desc
+
+from crc import db
+from crc.models.approval import ApprovalModel, ApprovalStatus, ApprovalFile
+from crc.scripts.script import Script
+from crc.services.file_service import FileService
+
+
+class RequestApproval(Script):
+    """This still needs to be fully wired up as a Script task callable from the workflow
+    But the basic logic is here just to get the tests passing and logic sound. """
+
+    def get_description(self):
+        return "Creates an approval request on this workflow, by the given approver_uid"
+
+    def add_approval(self, study_id, workflow_id, approver_uid):
+        """we might have multiple approvals for a workflow, so I would expect this
+        method to get called many times."""
+
+        # Find any existing approvals for this workflow and approver.
+        latest_approval_request = db.session.query(ApprovalModel).\
+            filter(ApprovalModel.workflow_id == workflow_id). \
+            filter(ApprovalModel.approver_uid == approver_uid). \
+            order_by(desc(ApprovalModel.version)).first()
+
+        # Construct as hash of the latest files to see if things have changed since
+        # the last approval.
+        latest_files = FileService.get_workflow_files(workflow_id)
+        current_workflow_hash = self.generate_workflow_hash(latest_files)
+
+        # If an existing approval request exists and no changes were made, do nothing.
+        # If there is an existing approval request for a previous version of the workflow
+        # then add a new request, and cancel any waiting/pending requests.
+        if latest_approval_request:
+            # We could just compare the ApprovalFile lists here and do away with this hash.
+            if latest_approval_request.workflow_hash == current_workflow_hash:
+                return  # This approval already exists.
+            else:
+                latest_approval_request.status = ApprovalStatus.CANCELED.value
+                db.session.add(latest_approval_request)
+                version = latest_approval_request.version + 1
+        else:
+            version = 1
+
+        model = ApprovalModel(study_id=study_id, workflow_id=workflow_id,
+                              approver_uid=approver_uid, status=ApprovalStatus.WAITING.value,
+                              message="", date_created=datetime.now(),
+                              version=version, workflow_hash=current_workflow_hash)
+        approval_files = self.create_approval_files(latest_files, model)
+        db.session.add(model)
+        db.session.add_all(approval_files)
+        db.session.commit()
+
+    def create_approval_files(self, files, approval):
+        """Currently based exclusively on the status of files associated with a workflow."""
+        file_approval_models = []
+        for file in files:
+            file_approval_models.append(ApprovalFile(file_id=file.id,
+                                                     approval=approval,
+                                                     file_version=file.latest_version))
+        return file_approval_models
+
+    def generate_workflow_hash(self, files):
+        """Currently based exclusively on the status of files associated with a workflow."""
+        version_array = []
+        for file in files:
+            version_array.append(str(file.id) + "[" + str(file.latest_version) + "]")
+        full_version = "-".join(version_array)
+        return full_version

crc/services/file_service.py

@@ -48,14 +48,24 @@ class FileService(object):
                            "When uploading files, the form field id must match a known document in the "
                            "irb_docunents.xslx reference file.  This code is not found in that file '%s'" % form_field_key)
 
-        file_model = FileModel(
+        """Assure this is unique to the workflow, task, and document code AND the Name
-            study_id=study_id,
+           Because we will allow users to upload multiple files for the same form field 
-            workflow_id=workflow_id,
+            in some cases """
-            task_id=task_id,
+        file_model = session.query(FileModel)\
-            name=name,
+            .filter(FileModel.workflow_id == workflow_id)\
-            form_field_key=form_field_key,
+            .filter(FileModel.task_id == str(task_id))\
-            irb_doc_code=form_field_key
+            .filter(FileModel.name == name)\
-        )
+            .filter(FileModel.irb_doc_code == form_field_key).first()
+
+        if not file_model:
+            file_model = FileModel(
+                study_id=study_id,
+                workflow_id=workflow_id,
+                task_id=task_id,
+                name=name,
+                form_field_key=form_field_key,
+                irb_doc_code=form_field_key
+            )
         return FileService.update_file(file_model, binary_data, content_type)
 
     @staticmethod
@@ -78,17 +88,31 @@ class FileService(object):
     @staticmethod
     def add_task_file(study_id, workflow_id, workflow_spec_id, task_id, name, content_type, binary_data,
                       irb_doc_code=None):
-        """Create a new file and associate it with an executing task within a workflow."""
+
-        file_model = FileModel(
+        """Assure this is unique to the workflow, task, and document code.  Disregard name."""
-            study_id=study_id,
+        file_model = session.query(FileModel)\
-            workflow_id=workflow_id,
+            .filter(FileModel.workflow_id == workflow_id)\
-            workflow_spec_id=workflow_spec_id,
+            .filter(FileModel.task_id == str(task_id))\
-            task_id=task_id,
+            .filter(FileModel.irb_doc_code == irb_doc_code).first()
-            name=name,
+
-            irb_doc_code=irb_doc_code
+        if not file_model:
-        )
+            """Create a new file and associate it with an executing task within a workflow."""
+            file_model = FileModel(
+                study_id=study_id,
+                workflow_id=workflow_id,
+                workflow_spec_id=workflow_spec_id,
+                task_id=task_id,
+                name=name,
+                irb_doc_code=irb_doc_code
+            )
         return FileService.update_file(file_model, binary_data, content_type)
 
+    @staticmethod
+    def get_workflow_files(workflow_id):
+        """Returns all the file models associated with a running workflow."""
+        return session.query(FileModel).filter(FileModel.workflow_id == workflow_id).\
+            order_by(FileModel.id).all()
+
     @staticmethod
     def add_reference_file(name, content_type, binary_data):
         """Create a file with the given name, but not associated with a spec or workflow.
@@ -109,6 +133,7 @@ class FileService(object):
         return file_extension.lower().strip()[1:]
 
     @staticmethod
+
     def update_file(file_model, binary_data, content_type):
         session.flush()  # Assure the database is up-to-date before running this.
 

tests/base_test.py

@@ -85,7 +85,7 @@ class BaseTest(unittest.TestCase):
     @classmethod
     def tearDownClass(cls):
         cls.ctx.pop()
-        session.remove()
+        db.drop_all()
         pass
 
     def setUp(self):
@@ -151,7 +151,7 @@ class BaseTest(unittest.TestCase):
     def load_test_spec(dir_name, master_spec=False, category_id=None):
         """Loads a spec into the database based on a directory in /tests/data"""
         if session.query(WorkflowSpecModel).filter_by(id=dir_name).count() > 0:
-            return
+            return session.query(WorkflowSpecModel).filter_by(id=dir_name).first()
         filepath = os.path.join(app.root_path, '..', 'tests', 'data', dir_name, "*")
         return ExampleDataLoader().create_spec(id=dir_name, name=dir_name, filepath=filepath, master_spec=master_spec,
                                                category_id=category_id)
@@ -205,9 +205,31 @@ class BaseTest(unittest.TestCase):
         content_type = CONTENT_TYPES[file_extension[1:]]
         file_service.update_file(file_model, data, content_type)
 
+    def create_user(self, uid="dhf8r", email="daniel.h.funk@gmail.com", display_name="Hoopy Frood"):
+        user = session.query(UserModel).filter(UserModel.uid == uid).first()
+        if user is None:
+            user = UserModel(uid=uid, email_address=email, display_name=display_name)
+            db.session.add(user)
+            db.session.commit()
+        return user
+
+    def create_study(self, uid="dhf8r", title="Beer conception in the bipedal software engineer"):
+        study = session.query(StudyModel).first()
+        if study is None:
+            user = self.create_user(uid=uid)
+            study = StudyModel(title=title, protocol_builder_status=ProtocolBuilderStatus.ACTIVE,
+                               user_uid=user.uid)
+            db.session.add(study)
+            db.session.commit()
+        return study
+
     def create_workflow(self, workflow_name, study=None, category_id=None):
-        if study == None:
+        db.session.flush()
-            study = session.query(StudyModel).first()
+        workflow = db.session.query(WorkflowSpecModel).filter(WorkflowSpecModel.name == workflow_name).first()
+        if workflow:
+            return workflow
+        if study is None:
+            study = self.create_study()
         spec = self.load_test_spec(workflow_name, category_id=category_id)
         workflow_model = StudyService._create_workflow_model(study, spec)
         return workflow_model

tests/test_file_service.py

@@ -0,0 +1,76 @@
+from tests.base_test import BaseTest
+from crc.services.file_service import FileService
+from crc.services.workflow_processor import WorkflowProcessor
+
+
+class TestFileService(BaseTest):
+    """Largely tested via the test_file_api, and time is tight, but adding new tests here."""
+
+    def test_add_file_from_task_increments_version_and_replaces_on_subsequent_add(self):
+        self.load_example_data()
+        self.create_reference_document()
+        workflow = self.create_workflow('file_upload_form')
+        processor = WorkflowProcessor(workflow)
+        task = processor.next_task()
+        irb_code = "UVACompl_PRCAppr"  # The first file referenced in pb required docs.
+        FileService.add_task_file(study_id=workflow.study_id, workflow_id=workflow.id,
+                                  workflow_spec_id=workflow.workflow_spec_id,
+                                  task_id=task.id,
+                                  name="anything.png", content_type="text",
+                                  binary_data=b'1234', irb_doc_code=irb_code)
+        # Add the file again with different data
+        FileService.add_task_file(study_id=workflow.study_id, workflow_id=workflow.id,
+                                  workflow_spec_id=workflow.workflow_spec_id,
+                                  task_id=task.id,
+                                  name="anything.png", content_type="text",
+                                  binary_data=b'5678', irb_doc_code=irb_code)
+
+        file_models = FileService.get_workflow_files(workflow_id=workflow.id)
+        self.assertEquals(1, len(file_models))
+        self.assertEquals(2, file_models[0].latest_version)
+
+    def test_add_file_from_form_increments_version_and_replaces_on_subsequent_add_with_same_name(self):
+        self.load_example_data()
+        self.create_reference_document()
+        workflow = self.create_workflow('file_upload_form')
+        processor = WorkflowProcessor(workflow)
+        task = processor.next_task()
+        irb_code = "UVACompl_PRCAppr"  # The first file referenced in pb required docs.
+        FileService.add_form_field_file(study_id=workflow.study_id, workflow_id=workflow.id,
+                                  task_id=task.id,
+                                  form_field_key=irb_code,
+                                  name="anything.png", content_type="text",
+                                  binary_data=b'1234')
+        # Add the file again with different data
+        FileService.add_form_field_file(study_id=workflow.study_id, workflow_id=workflow.id,
+                                  form_field_key=irb_code,
+                                  task_id=task.id,
+                                  name="anything.png", content_type="text",
+                                  binary_data=b'5678')
+
+        file_models = FileService.get_workflow_files(workflow_id=workflow.id)
+        self.assertEquals(1, len(file_models))
+        self.assertEquals(2, file_models[0].latest_version)
+
+    def test_add_file_from_form_allows_multiple_files_with_different_names(self):
+        self.load_example_data()
+        self.create_reference_document()
+        workflow = self.create_workflow('file_upload_form')
+        processor = WorkflowProcessor(workflow)
+        task = processor.next_task()
+        irb_code = "UVACompl_PRCAppr"  # The first file referenced in pb required docs.
+        FileService.add_form_field_file(study_id=workflow.study_id, workflow_id=workflow.id,
+                                  task_id=task.id,
+                                  form_field_key=irb_code,
+                                  name="anything.png", content_type="text",
+                                  binary_data=b'1234')
+        # Add the file again with different data
+        FileService.add_form_field_file(study_id=workflow.study_id, workflow_id=workflow.id,
+                                  form_field_key=irb_code,
+                                  task_id=task.id,
+                                  name="a_different_thing.png", content_type="text",
+                                  binary_data=b'5678')
+        file_models = FileService.get_workflow_files(workflow_id=workflow.id)
+        self.assertEquals(2, len(file_models))
+        self.assertEquals(1, file_models[0].latest_version)
+        self.assertEquals(1, file_models[1].latest_version)

tests/test_request_approval_script.py

@@ -0,0 +1,100 @@
+from tests.base_test import BaseTest
+
+from crc.services.file_service import FileService
+from crc.services.workflow_processor import WorkflowProcessor
+
+from crc.scripts.request_review import RequestApproval
+from crc import db
+from crc.models.approval import ApprovalModel
+
+
+class TestRequestApprovalScript(BaseTest):
+
+    def test_create_approval_record(self):
+        workflow = self.create_workflow("empty_workflow")
+        script = RequestApproval()
+        script.add_approval(study_id=workflow.study_id, workflow_id=workflow.id, approver_uid="dhf8r")
+        self.assertEquals(1, db.session.query(ApprovalModel).count())
+        model = db.session.query(ApprovalModel).first()
+        self.assertEquals(workflow.study_id, model.study_id)
+        self.assertEquals(workflow.id, model.workflow_id)
+        self.assertEquals("dhf8r", model.approver_uid)
+        self.assertEquals(1, model.version)
+        self.assertIsNotNone(model.workflow_hash)
+
+    def test_new_requests_dont_add_if_approval_exists_for_current_workflow(self):
+        workflow = self.create_workflow("empty_workflow")
+        script = RequestApproval()
+        script.add_approval(study_id=workflow.study_id, workflow_id=workflow.id, approver_uid="dhf8r")
+        script.add_approval(study_id=workflow.study_id, workflow_id=workflow.id, approver_uid="dhf8r")
+        self.assertEquals(1, db.session.query(ApprovalModel).count())
+        model = db.session.query(ApprovalModel).first()
+        self.assertEquals(1, model.version)
+
+    def test_new_approval_requests_after_file_modification_create_new_requests(self):
+        self.load_example_data()
+        self.create_reference_document()
+        workflow = self.create_workflow('empty_workflow')
+        processor = WorkflowProcessor(workflow)
+        task = processor.next_task()
+
+        script = RequestApproval()
+        script.add_approval(study_id=workflow.study_id, workflow_id=workflow.id, approver_uid="dhf8r")
+
+        irb_code_1 = "UVACompl_PRCAppr"  # The first file referenced in pb required docs.
+        FileService.add_task_file(study_id=workflow.study_id, workflow_id=workflow.id,
+                                  workflow_spec_id=workflow.workflow_spec_id,
+                                  task_id=task.id,
+                                  name="anything.png", content_type="text",
+                                  binary_data=b'5678', irb_doc_code=irb_code_1)
+
+        script.add_approval(study_id=workflow.study_id, workflow_id=workflow.id, approver_uid="dhf8r")
+        self.assertEquals(2, db.session.query(ApprovalModel).count())
+        models = db.session.query(ApprovalModel).order_by(ApprovalModel.version).all()
+        self.assertEquals(1, models[0].version)
+        self.assertEquals(2, models[1].version)
+
+
+    def test_generate_workflow_hash_and_version(self):
+        self.load_example_data()
+        self.create_reference_document()
+        workflow = self.create_workflow('empty_workflow')
+        processor = WorkflowProcessor(workflow)
+        task = processor.next_task()
+        irb_code_1 = "UVACompl_PRCAppr"  # The first file referenced in pb required docs.
+        irb_code_2 = "NonUVAIRB_AssuranceForm"  # The second file in above.
+        # Add a task file to the workflow.
+        FileService.add_task_file(study_id=workflow.study_id, workflow_id=workflow.id,
+                                  workflow_spec_id=workflow.workflow_spec_id,
+                                  task_id=task.id,
+                                  name="anything.png", content_type="text",
+                                  binary_data=b'5678', irb_doc_code=irb_code_1)
+        # Add a two form field files with the same irb_code, but
+        FileService.add_form_field_file(study_id=workflow.study_id, workflow_id=workflow.id,
+                                  task_id=task.id,
+                                  form_field_key=irb_code_2,
+                                  name="anything.png", content_type="text",
+                                  binary_data=b'1234')
+        FileService.add_form_field_file(study_id=workflow.study_id, workflow_id=workflow.id,
+                                  form_field_key=irb_code_2,
+                                  task_id=task.id,
+                                  name="another_anything.png", content_type="text",
+                                  binary_data=b'5678')
+
+        script = RequestApproval()
+
+        # Workflow hash should look be id[1]-id[1]-id[1]
+        # Sould be three files, each with a version of 1.
+        # where id is the file id, which we don't know, thus the regex.
+        latest_files = FileService.get_workflow_files(workflow.id)
+        self.assertRegexpMatches(script.generate_workflow_hash(latest_files), "\d+\[1\]-\d+\[1\]-\d+\[1\]")
+
+        # Replace last file
+        # should now be id[1]-id[1]-id[2]
+        FileService.add_form_field_file(study_id=workflow.study_id, workflow_id=workflow.id,
+                                  form_field_key=irb_code_2,
+                                  task_id=task.id,
+                                  name="another_anything.png", content_type="text",
+                                  binary_data=b'9999')
+        self.assertRegexpMatches(script.generate_workflow_hash(latest_files), "\d+\[1\]-\d+\[1\]-\d+\[2\]")
+