When you can't delete a file, mark it as archived. Don't include archived files in new approval requests.

crc/api.yml

@@ -468,7 +468,7 @@ paths:
                 $ref: "#/components/schemas/File"
     delete:
       operationId: crc.api.file.delete_file
-      summary: Removes an existing file
+      summary: Removes an existing file.  In the event the file can not be deleted, it is archived.
       tags:
         - Files
       responses:

crc/api/approval.py

@@ -15,9 +15,9 @@ from crc.services.ldap_service import LdapService
 
 def get_approvals(everything=False):
     if everything:
-        approvals = ApprovalService.get_all_approvals(ignore_cancelled=True)
+        approvals = ApprovalService.get_all_approvals(include_cancelled=True)
     else:
-        approvals = ApprovalService.get_approvals_per_user(g.user.uid, ignore_cancelled=True)
+        approvals = ApprovalService.get_approvals_per_user(g.user.uid, include_cancelled=False)
     results = ApprovalSchema(many=True).dump(approvals)
     return results
 
@@ -33,7 +33,7 @@ def get_approvals_for_study(study_id=None):
 def get_csv():
     """A huge bit of a one-off for RRT, but 3 weeks of midnight work can convince a
     man to do just about anything"""
-    approvals = ApprovalService.get_all_approvals(ignore_cancelled=True)
+    approvals = ApprovalService.get_all_approvals(include_cancelled=False)
     output = []
     errors = []
     ldapService = LdapService()

crc/models/file.py

@@ -82,7 +82,10 @@ class FileModel(db.Model):
     workflow_spec_id = db.Column(db.String, db.ForeignKey('workflow_spec.id'), nullable=True)
     workflow_id = db.Column(db.Integer, db.ForeignKey('workflow.id'), nullable=True)
     irb_doc_code = db.Column(db.String, nullable=True) # Code reference to the irb_documents.xlsx reference file.
-
+    # A request was made to delete the file, but we can't because there are
+    # active approvals or running workflows that depend on it.  So we archive
+    # it instead, hide it in the interface.
+    archived = db.Column(db.Boolean, default=False)
 
 class File(object):
     @classmethod

crc/services/approval_service.py

@@ -15,7 +15,7 @@ class ApprovalService(object):
     """Provides common tools for working with an Approval"""
 
     @staticmethod
-    def __one_approval_from_study(study, approver_uid = None, ignore_cancelled=False):
+    def __one_approval_from_study(study, approver_uid = None, include_cancelled=True):
         """Returns one approval, with all additional approvals as 'related_approvals',
         the main approval can be pinned to an approver with an optional argument.
         Will return null if no approvals exist on the study."""
@@ -23,7 +23,7 @@ class ApprovalService(object):
         related_approvals = []
         query = db.session.query(ApprovalModel).\
             filter(ApprovalModel.study_id == study.id)
-        if ignore_cancelled:
+        if not include_cancelled:
             query=query.filter(ApprovalModel.status != ApprovalStatus.CANCELED.value)
 
         approvals = query.all()
@@ -40,35 +40,38 @@ class ApprovalService(object):
         return main_approval
 
     @staticmethod
-    def get_approvals_per_user(approver_uid, ignore_cancelled=False):
+    def get_approvals_per_user(approver_uid, include_cancelled=False):
         """Returns a list of approval objects (not db models) for the given
          approver. """
         studies = db.session.query(StudyModel).join(ApprovalModel).\
             filter(ApprovalModel.approver_uid == approver_uid).all()
         approvals = []
         for study in studies:
-            approval = ApprovalService.__one_approval_from_study(study, approver_uid, ignore_cancelled)
+            approval = ApprovalService.__one_approval_from_study(study, approver_uid, include_cancelled)
             if approval:
                 approvals.append(approval)
         return approvals
 
     @staticmethod
-    def get_all_approvals(ignore_cancelled=False):
+    def get_all_approvals(include_cancelled=True):
         """Returns a list of all approval objects (not db models), one record
         per study, with any associated approvals grouped under the first approval."""
         studies = db.session.query(StudyModel).all()
         approvals = []
         for study in studies:
-            approval = ApprovalService.__one_approval_from_study(study, ignore_cancelled=ignore_cancelled)
+            approval = ApprovalService.__one_approval_from_study(study, include_cancelled=include_cancelled)
             if approval:
                 approvals.append(approval)
         return approvals
 
     @staticmethod
-    def get_approvals_for_study(study_id):
+    def get_approvals_for_study(study_id, include_cancelled=True):
         """Returns an array of Approval objects for the study, it does not
          compute the related approvals."""
-        db_approvals = session.query(ApprovalModel).filter_by(study_id=study_id).all()
+        query = session.query(ApprovalModel).filter_by(study_id=study_id)
+        if not include_cancelled:
+            query = query.filter(ApprovalModel.status != ApprovalStatus.CANCELED.value)
+        db_approvals = query.all()
         return [Approval.from_model(approval_model) for approval_model in db_approvals]
 
 
@@ -101,7 +104,7 @@ class ApprovalService(object):
         # Construct as hash of the latest files to see if things have changed since
         # the last approval.
         workflow = db.session.query(WorkflowModel).filter(WorkflowModel.id == workflow_id).first()
-        workflow_data_files = FileService.get_workflow_data_files(workflow_id)
+        workflow_data_files = FileService.get_workflow_data_files(workflow_id, include_archives=False)
         current_data_file_ids = list(data_file.id for data_file in workflow_data_files)
 
         if len(current_data_file_ids) == 0:

crc/services/file_service.py

@@ -195,7 +195,7 @@ class FileService(object):
 
     @staticmethod
     def get_files(workflow_spec_id=None, workflow_id=None,
-                  name=None, is_reference=False, irb_doc_code=None):
+                  name=None, is_reference=False, irb_doc_code=None, include_archives=True):
         query = session.query(FileModel).filter_by(is_reference=is_reference)
         if workflow_spec_id:
             query = query.filter_by(workflow_spec_id=workflow_spec_id)
@@ -208,6 +208,10 @@ class FileService(object):
 
         if name:
             query = query.filter_by(name=name)
+
+        if not include_archives:
+            query = query.filter(FileModel.archived == False)
+
         query = query.order_by(FileModel.id)
 
         results = query.all()
@@ -238,11 +242,11 @@ class FileService(object):
             return latest_data_files
 
     @staticmethod
-    def get_workflow_data_files(workflow_id=None):
+    def get_workflow_data_files(workflow_id=None, include_archives=True):
         """Returns all the FileDataModels related to a running workflow -
         So these are the latest data files that were uploaded or generated
         that go along with this workflow.  Not related to the spec in any way"""
-        file_models = FileService.get_files(workflow_id=workflow_id)
+        file_models = FileService.get_files(workflow_id=workflow_id, include_archives=include_archives)
         latest_data_files = []
         for file_model in file_models:
             latest_data_files.append(FileService.get_file_data(file_model.id))
@@ -316,6 +320,10 @@ class FileService(object):
             session.query(FileModel).filter_by(id=file_id).delete()
             session.commit()
         except IntegrityError as ie:
-            app.logger.error("Failed to delete file: %i, due to %s" % (file_id, str(ie)))
+            # We can't delete the file or file data, because it is referenced elsewhere,
-            raise ApiError('file_integrity_error', "You are attempting to delete a file that is "
+            # but we can at least mark it as deleted on the table.
-                                                   "required by other records in the system.")
+            session.rollback()
+            file_model = session.query(FileModel).filter_by(id=file_id).first()
+            file_model.archived = True
+            session.commit()
+            app.logger.error("Failed to delete file, so archiving it instead. %i, due to %s" % (file_id, str(ie)))

tests/test_files_api.py

@@ -9,6 +9,8 @@ from crc.models.workflow import WorkflowSpecModel
 from crc.services.file_service import FileService
 from crc.services.workflow_processor import WorkflowProcessor
 from example_data import ExampleDataLoader
+from crc.services.approval_service import ApprovalService
+from crc.models.approval import ApprovalModel, ApprovalStatus
 
 
 class TestFilesApi(BaseTest):
@@ -218,6 +220,41 @@ class TestFilesApi(BaseTest):
         rv = self.app.get('/v1.0/file/%i' % file.id, headers=self.logged_in_headers())
         self.assertEqual(404, rv.status_code)
 
+    def test_delete_file_after_approval(self):
+        self.create_reference_document()
+        workflow = self.create_workflow("empty_workflow")
+        FileService.add_workflow_file(workflow_id=workflow.id,
+                                      name="anything.png", content_type="text",
+                                      binary_data=b'5678', irb_doc_code="UVACompl_PRCAppr")
+        FileService.add_workflow_file(workflow_id=workflow.id,
+                                      name="anotother_anything.png", content_type="text",
+                                      binary_data=b'1234', irb_doc_code="Study_App_Doc")
+
+        ApprovalService.add_approval(study_id=workflow.study_id, workflow_id=workflow.id, approver_uid="dhf8r")
+
+        file = session.query(FileModel).\
+            filter(FileModel.workflow_id == workflow.id).\
+            filter(FileModel.name == "anything.png").first()
+        self.assertFalse(file.archived)
+        rv = self.app.get('/v1.0/file/%i' % file.id, headers=self.logged_in_headers())
+        self.assert_success(rv)
+
+        rv = self.app.delete('/v1.0/file/%i' % file.id, headers=self.logged_in_headers())
+        self.assert_success(rv)
+
+        session.refresh(file)
+        self.assertTrue(file.archived)
+
+        ApprovalService.add_approval(study_id=workflow.study_id, workflow_id=workflow.id, approver_uid="dhf8r")
+
+        approvals = session.query(ApprovalModel)\
+            .filter(ApprovalModel.status == ApprovalStatus.PENDING.value)\
+            .filter(ApprovalModel.study_id == workflow.study_id).all()
+
+        self.assertEquals(1, len(approvals))
+        self.assertEquals(1, len(approvals[0].approval_files))
+
+
     def test_change_primary_bpmn(self):
         self.load_example_data()
         spec = session.query(WorkflowSpecModel).first()

tests/test_workflow_spec_validation_api.py

@@ -71,7 +71,7 @@ class TestWorkflowSpecValidation(BaseTest):
         self.load_example_data()
         errors = self.validate_workflow("invalid_expression")
         self.assertEqual(2, len(errors))
-        self.assertEqual("workflow_execution_exception", errors[0]['code'])
+        self.assertEqual("workflow_validation_exception", errors[0]['code'])
         self.assertEqual("ExclusiveGateway_003amsm", errors[0]['task_id'])
         self.assertEqual("Has Bananas Gateway", errors[0]['task_name'])
         self.assertEqual("invalid_expression.bpmn", errors[0]['file_name'])
@@ -92,7 +92,7 @@ class TestWorkflowSpecValidation(BaseTest):
         self.load_example_data()
         errors = self.validate_workflow("invalid_script")
         self.assertEqual(2, len(errors))
-        self.assertEqual("workflow_execution_exception", errors[0]['code'])
+        self.assertEqual("workflow_validation_exception", errors[0]['code'])
         self.assertTrue("NoSuchScript" in errors[0]['message'])
         self.assertEqual("Invalid_Script_Task", errors[0]['task_id'])
         self.assertEqual("An Invalid Script Reference", errors[0]['task_name'])