From 1324533865b4996d5e7812e785355eeb87162413 Mon Sep 17 00:00:00 2001 From: Dan Funk Date: Thu, 4 Jun 2020 09:49:42 -0400 Subject: [PATCH] Some additional cleanup - when a file is "archived" it is no longer returned for any endpoints about files, but it is directly accessible via id, in the event some request is made for it at a later date. --- crc/api.yml | 2 +- crc/api/approval.py | 2 +- crc/services/approval_service.py | 2 +- crc/services/file_service.py | 17 ++++++++------- tests/test_file_service.py | 33 ++++++++++++++++++++++++++++- tests/test_files_api.py | 36 +++++++++++++++++++++++++++++++- tests/test_tasks_api.py | 1 - 7 files changed, 80 insertions(+), 13 deletions(-) diff --git a/crc/api.yml b/crc/api.yml index a3fd835b..07e61251 100644 --- a/crc/api.yml +++ b/crc/api.yml @@ -468,7 +468,7 @@ paths: $ref: "#/components/schemas/File" delete: operationId: crc.api.file.delete_file - summary: Removes an existing file. In the event the file can not be deleted, it is archived. + summary: Removes an existing file. In the event the file can not be deleted, it is marked as "archived" in the database and is no longer returned unless specifically requested by id. tags: - Files responses: diff --git a/crc/api/approval.py b/crc/api/approval.py index 1408fec5..4f413aa4 100644 --- a/crc/api/approval.py +++ b/crc/api/approval.py @@ -31,7 +31,7 @@ def get_approvals_for_study(study_id=None): # ----- Being decent into madness ---- # def get_csv(): - """A huge bit of a one-off for RRT, but 3 weeks of midnight work can convince a + """A damn lie, it's a json file. 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(include_cancelled=False) output = [] diff --git a/crc/services/approval_service.py b/crc/services/approval_service.py index a71f4ba1..b6605f7b 100644 --- a/crc/services/approval_service.py +++ b/crc/services/approval_service.py @@ -104,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, include_archives=False) + workflow_data_files = FileService.get_workflow_data_files(workflow_id) current_data_file_ids = list(data_file.id for data_file in workflow_data_files) if len(current_data_file_ids) == 0: diff --git a/crc/services/file_service.py b/crc/services/file_service.py index cda16a88..c0046a26 100644 --- a/crc/services/file_service.py +++ b/crc/services/file_service.py @@ -96,6 +96,7 @@ class FileService(object): 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).\ + filter(FileModel.archived == False).\ order_by(FileModel.id).all() @staticmethod @@ -139,6 +140,7 @@ class FileService(object): else: file_model.type = FileType[file_extension] file_model.content_type = content_type + file_model.archived = False # Unarchive the file if it is archived. if latest_data_model is None: version = 1 @@ -188,14 +190,15 @@ class FileService(object): def get_files_for_study(study_id, irb_doc_code=None): query = session.query(FileModel).\ join(WorkflowModel).\ - filter(WorkflowModel.study_id == study_id) + filter(WorkflowModel.study_id == study_id).\ + filter(FileModel.archived == False) if irb_doc_code: query = query.filter(FileModel.irb_doc_code == irb_doc_code) return query.all() @staticmethod def get_files(workflow_spec_id=None, workflow_id=None, - name=None, is_reference=False, irb_doc_code=None, include_archives=True): + name=None, is_reference=False, irb_doc_code=None): query = session.query(FileModel).filter_by(is_reference=is_reference) if workflow_spec_id: query = query.filter_by(workflow_spec_id=workflow_spec_id) @@ -209,8 +212,7 @@ class FileService(object): if name: query = query.filter_by(name=name) - if not include_archives: - query = query.filter(FileModel.archived == False) + query = query.filter(FileModel.archived == False) query = query.order_by(FileModel.id) @@ -242,11 +244,11 @@ class FileService(object): return latest_data_files @staticmethod - def get_workflow_data_files(workflow_id=None, include_archives=True): + def get_workflow_data_files(workflow_id=None): """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, include_archives=include_archives) + file_models = FileService.get_files(workflow_id=workflow_id) latest_data_files = [] for file_model in file_models: latest_data_files.append(FileService.get_file_data(file_model.id)) @@ -274,7 +276,8 @@ class FileService(object): @staticmethod def get_workflow_file_data(workflow, file_name): - """Given a SPIFF Workflow Model, tracks down a file with the given name in the database and returns its data""" + """This method should be deleted, find where it is used, and remove this method. + Given a SPIFF Workflow Model, tracks down a file with the given name in the database and returns its data""" workflow_spec_model = FileService.find_spec_model_in_db(workflow) if workflow_spec_model is None: diff --git a/tests/test_file_service.py b/tests/test_file_service.py index 705fef95..02a70ce8 100644 --- a/tests/test_file_service.py +++ b/tests/test_file_service.py @@ -1,8 +1,9 @@ from tests.base_test import BaseTest + +from crc import db 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.""" @@ -46,12 +47,42 @@ class TestFileService(BaseTest): name="anything.png", content_type="text", binary_data=b'5678') + def test_replace_archive_file_unarchives_the_file_and_updates(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_workflow_file(workflow_id=workflow.id, + irb_doc_code=irb_code, + name="anything.png", content_type="text", + binary_data=b'1234') + + # Archive the file + file_models = FileService.get_workflow_files(workflow_id=workflow.id) + self.assertEquals(1, len(file_models)) + file_model = file_models[0] + file_model.archived = True + db.session.add(file_model) + + # Assure that the file no longer comes back. + file_models = FileService.get_workflow_files(workflow_id=workflow.id) + self.assertEquals(0, len(file_models)) + + # Add the file again with different data + FileService.add_workflow_file(workflow_id=workflow.id, + irb_doc_code=irb_code, + 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)) file_data = FileService.get_workflow_data_files(workflow_id=workflow.id) self.assertEquals(1, len(file_data)) self.assertEquals(2, file_data[0].version) + self.assertEquals(b'5678', file_data[0].data) def test_add_file_from_form_allows_multiple_files_with_different_names(self): self.load_example_data() diff --git a/tests/test_files_api.py b/tests/test_files_api.py index 4c8c3eb6..2d14a8b5 100644 --- a/tests/test_files_api.py +++ b/tests/test_files_api.py @@ -3,7 +3,7 @@ import json from tests.base_test import BaseTest -from crc import session +from crc import session, db from crc.models.file import FileModel, FileType, FileSchema, FileModelSchema from crc.models.workflow import WorkflowSpecModel from crc.services.file_service import FileService @@ -48,6 +48,7 @@ class TestFilesApi(BaseTest): json_data = json.loads(rv.get_data(as_text=True)) self.assertEqual(2, len(json_data)) + def test_create_file(self): self.load_example_data() spec = session.query(WorkflowSpecModel).first() @@ -91,6 +92,39 @@ class TestFilesApi(BaseTest): self.assert_success(rv) + def test_archive_file_no_longer_shows_up(self): + self.load_example_data() + self.create_reference_document() + workflow = self.create_workflow('file_upload_form') + processor = WorkflowProcessor(workflow) + task = processor.next_task() + data = {'file': (io.BytesIO(b"abcdef"), 'random_fact.svg')} + correct_name = task.task_spec.form.fields[0].id + + data = {'file': (io.BytesIO(b"abcdef"), 'random_fact.svg')} + rv = self.app.post('/v1.0/file?study_id=%i&workflow_id=%s&task_id=%i&form_field_key=%s' % + (workflow.study_id, workflow.id, task.id, correct_name), data=data, follow_redirects=True, + content_type='multipart/form-data', headers=self.logged_in_headers()) + + self.assert_success(rv) + rv = self.app.get('/v1.0/file?workflow_id=%s' % workflow.id, headers=self.logged_in_headers()) + self.assert_success(rv) + self.assertEquals(1, len(json.loads(rv.get_data(as_text=True)))) + + file_model = db.session.query(FileModel).filter(FileModel.workflow_id == workflow.id).all() + self.assertEquals(1, len(file_model)) + file_model[0].archived = True + db.session.commit() + + rv = self.app.get('/v1.0/file?workflow_id=%s' % workflow.id, headers=self.logged_in_headers()) + self.assert_success(rv) + self.assertEquals(0, len(json.loads(rv.get_data(as_text=True)))) + + + + + + def test_set_reference_file(self): file_name = "irb_document_types.xls" data = {'file': (io.BytesIO(b"abcdef"), "does_not_matter.xls")} diff --git a/tests/test_tasks_api.py b/tests/test_tasks_api.py index 37849867..52c5f39d 100644 --- a/tests/test_tasks_api.py +++ b/tests/test_tasks_api.py @@ -208,7 +208,6 @@ class TestTasksApi(BaseTest): self.assert_success(rv) - def test_get_documentation_populated_in_end(self): self.load_example_data() workflow = self.create_workflow('random_fact')