From 951710d762ae879028d56ac9f36e6d2c64f45dcb Mon Sep 17 00:00:00 2001 From: Dan Funk Date: Tue, 19 May 2020 16:11:43 -0400 Subject: [PATCH 1/5] ldap lookup. Refactored calls into a new lookup_service to keep things tidy. New keys for all enum/auto-complete fields: PROP_OPTIONS_FILE = "spreadsheet.name" PROP_OPTIONS_VALUE_COLUMN = "spreadsheet.value.column" PROP_OPTIONS_LABEL_COL = "spreadsheet.label.column" PROP_LDAP_LOOKUP = "ldap.lookup" FIELD_TYPE_AUTO_COMPLETE = "autocomplete" --- crc/api/workflow.py | 10 +- crc/models/api_models.py | 10 +- crc/services/ldap_service.py | 27 +++- crc/services/lookup_service.py | 143 ++++++++++++++++++ crc/services/workflow_service.py | 100 +----------- tests/base_test.py | 9 +- .../enum_options_from_file.bpmn | 28 ++-- .../enum_options_with_search.bpmn | 29 ++-- tests/data/ldap_lookup/ldap_lookup.bpmn | 47 ++++++ tests/test_ldap_service.py | 1 + tests/test_lookup_service.py | 79 ++++++++++ tests/test_tasks_api.py | 15 ++ tests/test_workflow_service.py | 69 +-------- 13 files changed, 360 insertions(+), 207 deletions(-) create mode 100644 crc/services/lookup_service.py create mode 100644 tests/data/ldap_lookup/ldap_lookup.bpmn create mode 100644 tests/test_lookup_service.py diff --git a/crc/api/workflow.py b/crc/api/workflow.py index 5d5b2af5..4ad02f0e 100644 --- a/crc/api/workflow.py +++ b/crc/api/workflow.py @@ -7,6 +7,7 @@ from crc.models.file import FileModel, LookupDataSchema from crc.models.workflow import WorkflowModel, WorkflowSpecModelSchema, WorkflowSpecModel, WorkflowSpecCategoryModel, \ WorkflowSpecCategoryModelSchema from crc.services.file_service import FileService +from crc.services.lookup_service import LookupService from crc.services.study_service import StudyService from crc.services.workflow_processor import WorkflowProcessor from crc.services.workflow_service import WorkflowService @@ -217,9 +218,9 @@ def delete_workflow_spec_category(cat_id): def lookup(workflow_id, task_id, field_id, query, limit): """ - given a field in a task, attempts to find the lookup table associated with that field - and runs a full-text query against it to locate the values and labels that would be - returned to a type-ahead box. + given a field in a task, attempts to find the lookup table or function associated + with that field and runs a full-text query against it to locate the values and + labels that would be returned to a type-ahead box. """ workflow_model = session.query(WorkflowModel).filter_by(id=workflow_id).first() if not workflow_model: @@ -236,6 +237,5 @@ def lookup(workflow_id, task_id, field_id, query, limit): if not field: raise ApiError("unknown_field", "No field named %s in task %s" % (task_id, spiff_task.task_spec.name)) - lookup_table = WorkflowService.get_lookup_table(spiff_task, field) - lookup_data = WorkflowService.run_lookup_query(lookup_table, query, limit) + lookup_data = LookupService.lookup(spiff_task, field, query, limit) return LookupDataSchema(many=True).dump(lookup_data) \ No newline at end of file diff --git a/crc/models/api_models.py b/crc/models/api_models.py index 4e7d4304..4b279965 100644 --- a/crc/models/api_models.py +++ b/crc/models/api_models.py @@ -31,10 +31,12 @@ class NavigationItem(object): class Task(object): - ENUM_OPTIONS_FILE_PROP = "enum.options.file" - EMUM_OPTIONS_VALUE_COL_PROP = "enum.options.value.column" - EMUM_OPTIONS_LABEL_COL_PROP = "enum.options.label.column" - EMUM_OPTIONS_AS_LOOKUP = "enum.options.lookup" + PROP_OPTIONS_FILE = "spreadsheet.name" + PROP_OPTIONS_VALUE_COLUMN = "spreadsheet.value.column" + PROP_OPTIONS_LABEL_COL = "spreadsheet.label.column" + PROP_LDAP_LOOKUP = "ldap.lookup" + FIELD_TYPE_AUTO_COMPLETE = "autocomplete" + def __init__(self, id, name, title, type, state, form, documentation, data, multi_instance_type, multi_instance_count, multi_instance_index, process_name, properties): diff --git a/crc/services/ldap_service.py b/crc/services/ldap_service.py index ea3e155c..fd76ad78 100644 --- a/crc/services/ldap_service.py +++ b/crc/services/ldap_service.py @@ -12,20 +12,19 @@ class LdapUserInfo(object): self.display_name = entry.displayName.value self.given_name = ", ".join(entry.givenName) self.email = entry.mail.value - self.telephone_number= ", ".join(entry.telephoneNumber) + self.telephone_number = ", ".join(entry.telephoneNumber) self.title = ", ".join(entry.title) self.department = ", ".join(entry.uvaDisplayDepartment) self.affiliation = ", ".join(entry.uvaPersonIAMAffiliation) self.sponsor_type = ", ".join(entry.uvaPersonSponsoredType) - - + self.uid = entry.uid.value class LdapService(object): search_base = "ou=People,o=University of Virginia,c=US" - attributes = ['cn', 'displayName', 'givenName', 'mail', 'objectClass', 'UvaDisplayDepartment', + attributes = ['uid', 'cn', 'displayName', 'givenName', 'mail', 'objectClass', 'UvaDisplayDepartment', 'telephoneNumber', 'title', 'uvaPersonIAMAffiliation', 'uvaPersonSponsoredType'] - search_string = "(&(objectclass=person)(uid=%s))" + uid_search_string = "(&(objectclass=person)(uid=%s))" def __init__(self): if app.config['TESTING']: @@ -46,9 +45,25 @@ class LdapService(object): self.conn.unbind() def user_info(self, uva_uid): - search_string = LdapService.search_string % uva_uid + search_string = LdapService.uid_search_string % uva_uid self.conn.search(LdapService.search_base, search_string, attributes=LdapService.attributes) if len(self.conn.entries) < 1: raise ApiError("missing_ldap_record", "Unable to locate a user with id %s in LDAP" % uva_uid) entry = self.conn.entries[0] return(LdapUserInfo(entry)) + + def search_users(self, query, limit): + search_string = LdapService.uid_search_string % query + self.conn.search(LdapService.search_base, search_string, attributes=LdapService.attributes) + + # Entries are returned as a generator, accessing entries + # can make subsequent calls to the ldap service, so limit + # those here. + count = 0 + results = [] + for entry in self.conn.entries: + if count > limit: + break + results.append(LdapUserInfo(entry)) + count += 1 + return results diff --git a/crc/services/lookup_service.py b/crc/services/lookup_service.py new file mode 100644 index 00000000..5460174d --- /dev/null +++ b/crc/services/lookup_service.py @@ -0,0 +1,143 @@ +from pandas import ExcelFile + +from crc import db +from crc.api.common import ApiError +from crc.models.api_models import Task +from crc.models.file import FileDataModel, LookupFileModel, LookupDataModel +from crc.services.file_service import FileService +from crc.services.ldap_service import LdapService + + +class LookupService(object): + + """Provides tools for doing lookups for auto-complete fields. + This can currently take two forms: + 1) Lookup from spreadsheet data associated with a workflow specification. + in which case we store the spreadsheet data in a lookup table with full + text indexing enabled, and run searches against that table. + 2) Lookup from LDAP records. In which case we call out to an external service + to pull back detailed records and return them. + + I could imagine this growing to include other external services as tools to handle + lookup fields. I could also imagine using some sort of local cache so we don't + unnecessarily pound on external services for repeat searches for the same records. + """ + + @staticmethod + def lookup(spiff_task, field, query, limit): + """Executes the lookup for the given field.""" + if field.type != Task.FIELD_TYPE_AUTO_COMPLETE: + raise ApiError.from_task("invalid_field_type", + "Field '%s' must be an autocomplete field to use lookups." % field.label, + task=spiff_task) + + # If this field has an associated options file, then do the lookup against that field. + if field.has_property(Task.PROP_OPTIONS_FILE): + lookup_table = LookupService.get_lookup_table(spiff_task, field) + return LookupService._run_lookup_query(lookup_table, query, limit) + # If this is a ldap lookup, use the ldap service to provide the fields to return. + elif field.has_property(Task.PROP_LDAP_LOOKUP): + return LookupService._run_ldap_query(query, limit) + else: + raise ApiError.from_task("unknown_lookup_option", + "Lookup supports using spreadsheet options or ldap options, and neither was" + "provided.") + + @staticmethod + def get_lookup_table(spiff_task, field): + """ Checks to see if the options are provided in a separate lookup table associated with the + workflow, and if so, assures that data exists in the database, and return a model than can be used + to locate that data. + + Returns: an array of LookupData, suitable for returning to the api. + """ + if field.has_property(Task.PROP_OPTIONS_FILE): + if not field.has_property(Task.PROP_OPTIONS_VALUE_COLUMN) or \ + not field.has_property(Task.PROP_OPTIONS_LABEL_COL): + raise ApiError.from_task("invalid_emum", + "For enumerations based on an xls file, you must include 3 properties: %s, " + "%s, and %s" % (Task.PROP_OPTIONS_FILE, + Task.PROP_OPTIONS_VALUE_COLUMN, + Task.PROP_OPTIONS_LABEL_COL), + task=spiff_task) + + # Get the file data from the File Service + file_name = field.get_property(Task.PROP_OPTIONS_FILE) + value_column = field.get_property(Task.PROP_OPTIONS_VALUE_COLUMN) + label_column = field.get_property(Task.PROP_OPTIONS_LABEL_COL) + data_model = FileService.get_workflow_file_data(spiff_task.workflow, file_name) + lookup_model = LookupService.get_lookup_table_from_data_model(data_model, value_column, label_column) + return lookup_model + + @staticmethod + def get_lookup_table_from_data_model(data_model: FileDataModel, value_column, label_column): + """ In some cases the lookup table can be very large. This method will add all values to the database + in a way that can be searched and returned via an api call - rather than sending the full set of + options along with the form. It will only open the file and process the options if something has + changed. """ + + lookup_model = db.session.query(LookupFileModel) \ + .filter(LookupFileModel.file_data_model_id == data_model.id) \ + .filter(LookupFileModel.value_column == value_column) \ + .filter(LookupFileModel.label_column == label_column).first() + + if not lookup_model: + xls = ExcelFile(data_model.data) + df = xls.parse(xls.sheet_names[0]) # Currently we only look at the fist sheet. + if value_column not in df: + raise ApiError("invalid_emum", + "The file %s does not contain a column named % s" % (data_model.file_model.name, + value_column)) + if label_column not in df: + raise ApiError("invalid_emum", + "The file %s does not contain a column named % s" % (data_model.file_model.name, + label_column)) + + lookup_model = LookupFileModel(label_column=label_column, value_column=value_column, + file_data_model_id=data_model.id) + + db.session.add(lookup_model) + for index, row in df.iterrows(): + lookup_data = LookupDataModel(lookup_file_model=lookup_model, + value=row[value_column], + label=row[label_column], + data=row.to_json()) + db.session.add(lookup_data) + db.session.commit() + + return lookup_model + + @staticmethod + def _run_lookup_query(lookup_file_model, query, limit): + db_query = LookupDataModel.query.filter(LookupDataModel.lookup_file_model == lookup_file_model) + + query = query.strip() + if len(query) > 1: + if ' ' in query: + terms = query.split(' ') + new_terms = [] + for t in terms: + new_terms.append(t + ":*") + query = '|'.join(new_terms) + else: + query = "%s:*" % query + db_query = db_query.filter(LookupDataModel.label.match(query)) + + # db_query = db_query.filter(text("lookup_data.label @@ to_tsquery('simple', '%s')" % query)) + + return db_query.limit(limit).all() + + @staticmethod + def _run_ldap_query(query, limit): + users = LdapService().search_users(query, limit) + + """Converts the user models into something akin to the + LookupModel in models/file.py, so this can be returned in the same way + we return a lookup data model.""" + user_list = [] + for user in users: + user_list.append( {"value": user.uid, + "label": user.display_name + "(" + user.uid + ")", + "data": user.__dict__ + }) + return user_list \ No newline at end of file diff --git a/crc/services/workflow_service.py b/crc/services/workflow_service.py index faee089a..6fbb0f30 100644 --- a/crc/services/workflow_service.py +++ b/crc/services/workflow_service.py @@ -1,5 +1,7 @@ from datetime import datetime +import jinja2 +from SpiffWorkflow import Task as SpiffTask, WorkflowException from SpiffWorkflow.bpmn.specs.ManualTask import ManualTask from SpiffWorkflow.bpmn.specs.ScriptTask import ScriptTask from SpiffWorkflow.bpmn.specs.UserTask import UserTask @@ -7,20 +9,16 @@ from SpiffWorkflow.bpmn.workflow import BpmnWorkflow from SpiffWorkflow.dmn.specs.BusinessRuleTask import BusinessRuleTask from SpiffWorkflow.specs import CancelTask, StartTask from flask import g -from pandas import ExcelFile -from sqlalchemy import func +from jinja2 import Template from crc import db, app from crc.api.common import ApiError from crc.models.api_models import Task, MultiInstanceType -import jinja2 -from jinja2 import Template - -from crc.models.file import FileDataModel, LookupFileModel, LookupDataModel +from crc.models.file import LookupDataModel from crc.models.stats import TaskEventModel from crc.services.file_service import FileService +from crc.services.lookup_service import LookupService from crc.services.workflow_processor import WorkflowProcessor, CustomBpmnScriptEngine -from SpiffWorkflow import Task as SpiffTask, WorkflowException class WorkflowService(object): @@ -178,10 +176,10 @@ class WorkflowService(object): @staticmethod def process_options(spiff_task, field): - lookup_model = WorkflowService.get_lookup_table(spiff_task, field) + lookup_model = LookupService.get_lookup_table(spiff_task, field) - # If lookup is set to true, do not populate options, a lookup will happen later. - if field.has_property(Task.EMUM_OPTIONS_AS_LOOKUP) and field.get_property(Task.EMUM_OPTIONS_AS_LOOKUP): + # If this is an auto-complete field, do not populate options, a lookup will happen later. + if field.type == Task.FIELD_TYPE_AUTO_COMPLETE: pass else: data = db.session.query(LookupDataModel).filter(LookupDataModel.lookup_file_model == lookup_model).all() @@ -190,88 +188,6 @@ class WorkflowService(object): for d in data: field.options.append({"id": d.value, "name": d.label}) - @staticmethod - def get_lookup_table(spiff_task, field): - """ Checks to see if the options are provided in a separate lookup table associated with the - workflow, and if so, assures that data exists in the database, and return a model than can be used - to locate that data. """ - if field.has_property(Task.ENUM_OPTIONS_FILE_PROP): - if not field.has_property(Task.EMUM_OPTIONS_VALUE_COL_PROP) or \ - not field.has_property(Task.EMUM_OPTIONS_LABEL_COL_PROP): - raise ApiError.from_task("invalid_emum", - "For enumerations based on an xls file, you must include 3 properties: %s, " - "%s, and %s" % (Task.ENUM_OPTIONS_FILE_PROP, - Task.EMUM_OPTIONS_VALUE_COL_PROP, - Task.EMUM_OPTIONS_LABEL_COL_PROP), - task=spiff_task) - - # Get the file data from the File Service - file_name = field.get_property(Task.ENUM_OPTIONS_FILE_PROP) - value_column = field.get_property(Task.EMUM_OPTIONS_VALUE_COL_PROP) - label_column = field.get_property(Task.EMUM_OPTIONS_LABEL_COL_PROP) - data_model = FileService.get_workflow_file_data(spiff_task.workflow, file_name) - lookup_model = WorkflowService._get_lookup_table_from_data_model(data_model, value_column, label_column) - return lookup_model - - @staticmethod - def _get_lookup_table_from_data_model(data_model: FileDataModel, value_column, label_column): - """ In some cases the lookup table can be very large. This method will add all values to the database - in a way that can be searched and returned via an api call - rather than sending the full set of - options along with the form. It will only open the file and process the options if something has - changed. """ - - lookup_model = db.session.query(LookupFileModel) \ - .filter(LookupFileModel.file_data_model_id == data_model.id) \ - .filter(LookupFileModel.value_column == value_column) \ - .filter(LookupFileModel.label_column == label_column).first() - - if not lookup_model: - xls = ExcelFile(data_model.data) - df = xls.parse(xls.sheet_names[0]) # Currently we only look at the fist sheet. - if value_column not in df: - raise ApiError("invalid_emum", - "The file %s does not contain a column named % s" % (data_model.file_model.name, - value_column)) - if label_column not in df: - raise ApiError("invalid_emum", - "The file %s does not contain a column named % s" % (data_model.file_model.name, - label_column)) - - lookup_model = LookupFileModel(label_column=label_column, value_column=value_column, - file_data_model_id=data_model.id) - - db.session.add(lookup_model) - for index, row in df.iterrows(): - lookup_data = LookupDataModel(lookup_file_model=lookup_model, - value=row[value_column], - label=row[label_column], - data=row.to_json()) - db.session.add(lookup_data) - db.session.commit() - - return lookup_model - - @staticmethod - def run_lookup_query(lookupFileModel, query, limit): - db_query = LookupDataModel.query.filter(LookupDataModel.lookup_file_model == lookupFileModel) - - query = query.strip() - if len(query) > 1: - if ' ' in query: - terms = query.split(' ') - query = "" - new_terms = [] - for t in terms: - new_terms.append(t + ":*") - query = '|'.join(new_terms) - else: - query = "%s:*" % query - db_query = db_query.filter(LookupDataModel.label.match(query)) - - # db_query = db_query.filter(text("lookup_data.label @@ to_tsquery('simple', '%s')" % query)) - - return db_query.limit(limit).all() - @staticmethod def log_task_action(processor, spiff_task, action): task = WorkflowService.spiff_task_to_api_task(spiff_task) diff --git a/tests/base_test.py b/tests/base_test.py index 3511b361..290b1506 100644 --- a/tests/base_test.py +++ b/tests/base_test.py @@ -77,20 +77,21 @@ class BaseTest(unittest.TestCase): app.config.from_object('config.testing') cls.ctx = app.test_request_context() cls.app = app.test_client() + cls.ctx.push() db.create_all() @classmethod def tearDownClass(cls): - db.drop_all() + cls.ctx.pop() session.remove() pass def setUp(self): - self.ctx.push() + pass def tearDown(self): - ExampleDataLoader.clean_db() # This does not seem to work, some colision of sessions. - self.ctx.pop() + ExampleDataLoader.clean_db() + session.flush() self.auths = {} def logged_in_headers(self, user=None, redirect_url='http://some/frontend/url'): diff --git a/tests/data/enum_options_from_file/enum_options_from_file.bpmn b/tests/data/enum_options_from_file/enum_options_from_file.bpmn index 8080327f..6497d1f7 100644 --- a/tests/data/enum_options_from_file/enum_options_from_file.bpmn +++ b/tests/data/enum_options_from_file/enum_options_from_file.bpmn @@ -1,5 +1,5 @@ - + SequenceFlow_0lvudp8 @@ -14,9 +14,9 @@ - - - + + + @@ -27,20 +27,20 @@ - - - - - - - - - - + + + + + + + + + + diff --git a/tests/data/enum_options_with_search/enum_options_with_search.bpmn b/tests/data/enum_options_with_search/enum_options_with_search.bpmn index 7c682bb8..584dd261 100644 --- a/tests/data/enum_options_with_search/enum_options_with_search.bpmn +++ b/tests/data/enum_options_with_search/enum_options_with_search.bpmn @@ -1,5 +1,5 @@ - + SequenceFlow_0lvudp8 @@ -14,10 +14,9 @@ @@ -28,20 +27,20 @@ - - - - - - - - - - + + + + + + + + + + diff --git a/tests/data/ldap_lookup/ldap_lookup.bpmn b/tests/data/ldap_lookup/ldap_lookup.bpmn new file mode 100644 index 00000000..8f89c0d8 --- /dev/null +++ b/tests/data/ldap_lookup/ldap_lookup.bpmn @@ -0,0 +1,47 @@ + + + + + SequenceFlow_0lvudp8 + + + + SequenceFlow_02vev7n + + + + + + + + + + + + + SequenceFlow_0lvudp8 + SequenceFlow_02vev7n + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/test_ldap_service.py b/tests/test_ldap_service.py index f4a56f47..8bd80107 100644 --- a/tests/test_ldap_service.py +++ b/tests/test_ldap_service.py @@ -18,6 +18,7 @@ class TestLdapService(BaseTest): def test_get_single_user(self): user_info = self.ldap_service.user_info("lb3dp") self.assertIsNotNone(user_info) + self.assertEqual("lb3dp", user_info.uid) self.assertEqual("Laura Barnes", user_info.display_name) self.assertEqual("Laura", user_info.given_name) self.assertEqual("lb3dp@virginia.edu", user_info.email) diff --git a/tests/test_lookup_service.py b/tests/test_lookup_service.py new file mode 100644 index 00000000..89be6168 --- /dev/null +++ b/tests/test_lookup_service.py @@ -0,0 +1,79 @@ +from crc import session +from crc.models.file import FileDataModel, FileModel, LookupFileModel, LookupDataModel +from crc.services.file_service import FileService +from crc.services.lookup_service import LookupService +from crc.services.workflow_processor import WorkflowProcessor +from crc.services.workflow_service import WorkflowService +from tests.base_test import BaseTest + + +class TestLookupService(BaseTest): + + def test_create_lookup_file_multiple_times_does_not_update_database(self): + spec = self.load_test_spec('enum_options_from_file') + file_model = session.query(FileModel).filter(FileModel.name == "customer_list.xls").first() + file_data_model = session.query(FileDataModel).filter(FileDataModel.file_model == file_model).first() + LookupService.get_lookup_table_from_data_model(file_data_model, "CUSTOMER_NUMBER", "CUSTOMER_NAME") + LookupService.get_lookup_table_from_data_model(file_data_model, "CUSTOMER_NUMBER", "CUSTOMER_NAME") + LookupService.get_lookup_table_from_data_model(file_data_model, "CUSTOMER_NUMBER", "CUSTOMER_NAME") + lookup_records = session.query(LookupFileModel).all() + self.assertIsNotNone(lookup_records) + self.assertEqual(1, len(lookup_records)) + lookup_record = lookup_records[0] + lookup_data = session.query(LookupDataModel).filter(LookupDataModel.lookup_file_model == lookup_record).all() + self.assertEquals(19, len(lookup_data)) + # Using the same table with different lookup lable or value, does create additional records. + LookupService.get_lookup_table_from_data_model(file_data_model, "CUSTOMER_NAME", "CUSTOMER_NUMBER") + lookup_records = session.query(LookupFileModel).all() + self.assertIsNotNone(lookup_records) + self.assertEqual(2, len(lookup_records)) + FileService.delete_file(file_model.id) ## Assure we can delete the file. + + def test_some_full_text_queries(self): + self.load_test_spec('enum_options_from_file') + file_model = session.query(FileModel).filter(FileModel.name == "customer_list.xls").first() + self.assertIsNotNone(file_model) + file_data_model = session.query(FileDataModel).filter(FileDataModel.file_model == file_model).first() + lookup_table = LookupService.get_lookup_table_from_data_model(file_data_model, "CUSTOMER_NUMBER", "CUSTOMER_NAME") + + results = LookupService._run_lookup_query(lookup_table, "medicines", limit=10) + self.assertEquals(1, len(results), "words in the middle of label are detected.") + self.assertEquals("The Medicines Company", results[0].label) + + results = LookupService._run_lookup_query(lookup_table, "", limit=10) + self.assertEquals(10, len(results), "Blank queries return everything, to the limit") + + results = LookupService._run_lookup_query(lookup_table, "UVA", limit=10) + self.assertEquals(1, len(results), "Beginning of label is found.") + self.assertEquals("UVA - INTERNAL - GM USE ONLY", results[0].label) + + results = LookupService._run_lookup_query(lookup_table, "uva", limit=10) + self.assertEquals(1, len(results), "case does not matter.") + self.assertEquals("UVA - INTERNAL - GM USE ONLY", results[0].label) + + + + results = LookupService._run_lookup_query(lookup_table, "medici", limit=10) + self.assertEquals(1, len(results), "partial words are picked up.") + self.assertEquals("The Medicines Company", results[0].label) + + results = LookupService._run_lookup_query(lookup_table, "Genetics Savings", limit=10) + self.assertEquals(1, len(results), "multiple terms are picked up..") + self.assertEquals("Genetics Savings & Clone, Inc.", results[0].label) + + results = LookupService._run_lookup_query(lookup_table, "Genetics Sav", limit=10) + self.assertEquals(1, len(results), "prefix queries still work with partial terms") + self.assertEquals("Genetics Savings & Clone, Inc.", results[0].label) + + results = LookupService._run_lookup_query(lookup_table, "Gen Sav", limit=10) + self.assertEquals(1, len(results), "prefix queries still work with ALL the partial terms") + self.assertEquals("Genetics Savings & Clone, Inc.", results[0].label) + + results = LookupService._run_lookup_query(lookup_table, "Inc", limit=10) + self.assertEquals(7, len(results), "short terms get multiple correct results.") + self.assertEquals("Genetics Savings & Clone, Inc.", results[0].label) + + # Fixme: Stop words are taken into account on the query side, and haven't found a fix yet. + #results = WorkflowService.run_lookup_query(lookup_table.id, "in", limit=10) + #self.assertEquals(7, len(results), "stop words are not removed.") + #self.assertEquals("Genetics Savings & Clone, Inc.", results[0].label) diff --git a/tests/test_tasks_api.py b/tests/test_tasks_api.py index edbb95d5..7cf3c8a2 100644 --- a/tests/test_tasks_api.py +++ b/tests/test_tasks_api.py @@ -336,6 +336,21 @@ class TestTasksApi(BaseTest): results = json.loads(rv.get_data(as_text=True)) self.assertEqual(5, len(results)) + def test_lookup_endpoint_for_task_ldap_field_lookup(self): + self.load_example_data() + workflow = self.create_workflow('ldap_lookup') + # get the first form + workflow = self.get_workflow_api(workflow) + task = workflow.next_task + field_id = task.form['fields'][0]['id'] + # lb3dp is a user record in the mock ldap responses for tests. + rv = self.app.get('/v1.0/workflow/%i/task/%s/lookup/%s?query=%s&limit=5' % + (workflow.id, task.id, field_id, 'lb3dp'), + headers=self.logged_in_headers(), + content_type="application/json") + self.assert_success(rv) + results = json.loads(rv.get_data(as_text=True)) + self.assertEqual(1, len(results)) def test_sub_process(self): self.load_example_data() diff --git a/tests/test_workflow_service.py b/tests/test_workflow_service.py index d80d84e6..95385b80 100644 --- a/tests/test_workflow_service.py +++ b/tests/test_workflow_service.py @@ -1,6 +1,7 @@ from crc import session from crc.models.file import FileDataModel, FileModel, LookupFileModel, LookupDataModel from crc.services.file_service import FileService +from crc.services.lookup_service import LookupService from crc.services.workflow_processor import WorkflowProcessor from crc.services.workflow_service import WorkflowService from tests.base_test import BaseTest @@ -76,7 +77,7 @@ class TestWorkflowService(BaseTest): spec = self.load_test_spec('enum_options_from_file') file_model = session.query(FileModel).filter(FileModel.name == "customer_list.xls").first() file_data_model = session.query(FileDataModel).filter(FileDataModel.file_model == file_model).first() - WorkflowService._get_lookup_table_from_data_model(file_data_model, "CUSTOMER_NUMBER", "CUSTOMER_NAME") + LookupService.get_lookup_table_from_data_model(file_data_model, "CUSTOMER_NUMBER", "CUSTOMER_NAME") lookup_records = session.query(LookupFileModel).all() self.assertIsNotNone(lookup_records) self.assertEqual(1, len(lookup_records)) @@ -101,71 +102,5 @@ class TestWorkflowService(BaseTest): search_results = LookupDataModel.query.filter(LookupDataModel.label.match("bio:*")).all() self.assertEquals(2, len(search_results)) - def test_create_lookup_file_multiple_times_does_not_update_database(self): - spec = self.load_test_spec('enum_options_from_file') - file_model = session.query(FileModel).filter(FileModel.name == "customer_list.xls").first() - file_data_model = session.query(FileDataModel).filter(FileDataModel.file_model == file_model).first() - WorkflowService._get_lookup_table_from_data_model(file_data_model, "CUSTOMER_NUMBER", "CUSTOMER_NAME") - WorkflowService._get_lookup_table_from_data_model(file_data_model, "CUSTOMER_NUMBER", "CUSTOMER_NAME") - WorkflowService._get_lookup_table_from_data_model(file_data_model, "CUSTOMER_NUMBER", "CUSTOMER_NAME") - lookup_records = session.query(LookupFileModel).all() - self.assertIsNotNone(lookup_records) - self.assertEqual(1, len(lookup_records)) - lookup_record = lookup_records[0] - lookup_data = session.query(LookupDataModel).filter(LookupDataModel.lookup_file_model == lookup_record).all() - self.assertEquals(19, len(lookup_data)) - # Using the same table with different lookup lable or value, does create additional records. - WorkflowService._get_lookup_table_from_data_model(file_data_model, "CUSTOMER_NAME", "CUSTOMER_NUMBER") - lookup_records = session.query(LookupFileModel).all() - self.assertIsNotNone(lookup_records) - self.assertEqual(2, len(lookup_records)) - FileService.delete_file(file_model.id) ## Assure we can delete the file. - - def test_some_full_text_queries(self): - self.load_test_spec('enum_options_from_file') - file_model = session.query(FileModel).filter(FileModel.name == "customer_list.xls").first() - file_data_model = session.query(FileDataModel).filter(FileDataModel.file_model == file_model).first() - lookup_table = WorkflowService._get_lookup_table_from_data_model(file_data_model, "CUSTOMER_NUMBER", "CUSTOMER_NAME") - lookup_data = session.query(LookupDataModel).filter(LookupDataModel.lookup_file_model == lookup_table).all() - - results = WorkflowService.run_lookup_query(lookup_table, "medicines", limit=10) - self.assertEquals(1, len(results), "words in the middle of label are detected.") - self.assertEquals("The Medicines Company", results[0].label) - - results = WorkflowService.run_lookup_query(lookup_table, "", limit=10) - self.assertEquals(10, len(results), "Blank queries return everything, to the limit") - - results = WorkflowService.run_lookup_query(lookup_table, "UVA", limit=10) - self.assertEquals(1, len(results), "Beginning of label is found.") - self.assertEquals("UVA - INTERNAL - GM USE ONLY", results[0].label) - - results = WorkflowService.run_lookup_query(lookup_table, "uva", limit=10) - self.assertEquals(1, len(results), "case does not matter.") - self.assertEquals("UVA - INTERNAL - GM USE ONLY", results[0].label) - - results = WorkflowService.run_lookup_query(lookup_table, "medici", limit=10) - self.assertEquals(1, len(results), "partial words are picked up.") - self.assertEquals("The Medicines Company", results[0].label) - - results = WorkflowService.run_lookup_query(lookup_table, "Genetics Savings", limit=10) - self.assertEquals(1, len(results), "multiple terms are picked up..") - self.assertEquals("Genetics Savings & Clone, Inc.", results[0].label) - - results = WorkflowService.run_lookup_query(lookup_table, "Genetics Sav", limit=10) - self.assertEquals(1, len(results), "prefix queries still work with partial terms") - self.assertEquals("Genetics Savings & Clone, Inc.", results[0].label) - - results = WorkflowService.run_lookup_query(lookup_table, "Gen Sav", limit=10) - self.assertEquals(1, len(results), "prefix queries still work with ALL the partial terms") - self.assertEquals("Genetics Savings & Clone, Inc.", results[0].label) - - results = WorkflowService.run_lookup_query(lookup_table, "Inc", limit=10) - self.assertEquals(7, len(results), "short terms get multiple correct results.") - self.assertEquals("Genetics Savings & Clone, Inc.", results[0].label) - - # Fixme: Stop words are taken into account on the query side, and haven't found a fix yet. - #results = WorkflowService.run_lookup_query(lookup_table.id, "in", limit=10) - #self.assertEquals(7, len(results), "stop words are not removed.") - #self.assertEquals("Genetics Savings & Clone, Inc.", results[0].label) From c4f2bd8dc6e7e8f6745384c400b4c1a35121c6a8 Mon Sep 17 00:00:00 2001 From: Dan Funk Date: Tue, 19 May 2020 16:23:20 -0400 Subject: [PATCH 2/5] Quick cleanup, adding a space --- crc/services/lookup_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crc/services/lookup_service.py b/crc/services/lookup_service.py index 5460174d..f9d023bc 100644 --- a/crc/services/lookup_service.py +++ b/crc/services/lookup_service.py @@ -137,7 +137,7 @@ class LookupService(object): user_list = [] for user in users: user_list.append( {"value": user.uid, - "label": user.display_name + "(" + user.uid + ")", + "label": user.display_name + " (" + user.uid + ")", "data": user.__dict__ }) return user_list \ No newline at end of file From 481a9ed04cf7f9961d4cdd476638452704fa1382 Mon Sep 17 00:00:00 2001 From: Aaron Louie Date: Tue, 19 May 2020 21:51:54 -0400 Subject: [PATCH 3/5] Gets image ids from task data and Injects images into jinja docx --- Pipfile.lock | 70 +++++++++++++------------- crc/scripts/complete_template.py | 60 +++++++++++++++++++--- crc/services/file_service.py | 32 +++++++----- crc/services/workflow_processor.py | 4 ++ crc/services/workflow_service.py | 2 +- crc/static/templates/placeholder.docx | Bin 0 -> 4082 bytes crc/static/templates/placeholder.png | Bin 0 -> 6231 bytes 7 files changed, 114 insertions(+), 54 deletions(-) create mode 100644 crc/static/templates/placeholder.docx create mode 100644 crc/static/templates/placeholder.png diff --git a/Pipfile.lock b/Pipfile.lock index 4f8b70c9..38ab9ef7 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -401,35 +401,35 @@ }, "lxml": { "hashes": [ - "sha256:06d4e0bbb1d62e38ae6118406d7cdb4693a3fa34ee3762238bcb96c9e36a93cd", - "sha256:0701f7965903a1c3f6f09328c1278ac0eee8f56f244e66af79cb224b7ef3801c", - "sha256:1f2c4ec372bf1c4a2c7e4bb20845e8bcf8050365189d86806bad1e3ae473d081", - "sha256:4235bc124fdcf611d02047d7034164897ade13046bda967768836629bc62784f", - "sha256:5828c7f3e615f3975d48f40d4fe66e8a7b25f16b5e5705ffe1d22e43fb1f6261", - "sha256:585c0869f75577ac7a8ff38d08f7aac9033da2c41c11352ebf86a04652758b7a", - "sha256:5d467ce9c5d35b3bcc7172c06320dddb275fea6ac2037f72f0a4d7472035cea9", - "sha256:63dbc21efd7e822c11d5ddbedbbb08cd11a41e0032e382a0fd59b0b08e405a3a", - "sha256:7bc1b221e7867f2e7ff1933165c0cec7153dce93d0cdba6554b42a8beb687bdb", - "sha256:8620ce80f50d023d414183bf90cc2576c2837b88e00bea3f33ad2630133bbb60", - "sha256:8a0ebda56ebca1a83eb2d1ac266649b80af8dd4b4a3502b2c1e09ac2f88fe128", - "sha256:90ed0e36455a81b25b7034038e40880189169c308a3df360861ad74da7b68c1a", - "sha256:95e67224815ef86924fbc2b71a9dbd1f7262384bca4bc4793645794ac4200717", - "sha256:afdb34b715daf814d1abea0317b6d672476b498472f1e5aacbadc34ebbc26e89", - "sha256:b4b2c63cc7963aedd08a5f5a454c9f67251b1ac9e22fd9d72836206c42dc2a72", - "sha256:d068f55bda3c2c3fcaec24bd083d9e2eede32c583faf084d6e4b9daaea77dde8", - "sha256:d5b3c4b7edd2e770375a01139be11307f04341ec709cf724e0f26ebb1eef12c3", - "sha256:deadf4df349d1dcd7b2853a2c8796593cc346600726eff680ed8ed11812382a7", - "sha256:df533af6f88080419c5a604d0d63b2c33b1c0c4409aba7d0cb6de305147ea8c8", - "sha256:e4aa948eb15018a657702fee0b9db47e908491c64d36b4a90f59a64741516e77", - "sha256:e5d842c73e4ef6ed8c1bd77806bf84a7cb535f9c0cf9b2c74d02ebda310070e1", - "sha256:ebec08091a22c2be870890913bdadd86fcd8e9f0f22bcb398abd3af914690c15", - "sha256:edc15fcfd77395e24543be48871c251f38132bb834d9fdfdad756adb6ea37679", - "sha256:f2b74784ed7e0bc2d02bd53e48ad6ba523c9b36c194260b7a5045071abbb1012", - "sha256:fa071559f14bd1e92077b1b5f6c22cf09756c6de7139370249eb372854ce51e6", - "sha256:fd52e796fee7171c4361d441796b64df1acfceb51f29e545e812f16d023c4bbc", - "sha256:fe976a0f1ef09b3638778024ab9fb8cde3118f203364212c198f71341c0715ca" + "sha256:06748c7192eab0f48e3d35a7adae609a329c6257495d5e53878003660dc0fec6", + "sha256:0790ddca3f825dd914978c94c2545dbea5f56f008b050e835403714babe62a5f", + "sha256:1aa7a6197c1cdd65d974f3e4953764eee3d9c7b67e3966616b41fab7f8f516b7", + "sha256:22c6d34fdb0e65d5f782a4d1a1edb52e0a8365858dafb1c08cb1d16546cf0786", + "sha256:2754d4406438c83144f9ffd3628bbe2dcc6d62b20dbc5c1ec4bc4385e5d44b42", + "sha256:27ee0faf8077c7c1a589573b1450743011117f1aa1a91d5ae776bbc5ca6070f2", + "sha256:2b02c106709466a93ed424454ce4c970791c486d5fcdf52b0d822a7e29789626", + "sha256:2d1ddce96cf15f1254a68dba6935e6e0f1fe39247de631c115e84dd404a6f031", + "sha256:4f282737d187ae723b2633856085c31ae5d4d432968b7f3f478a48a54835f5c4", + "sha256:51bb4edeb36d24ec97eb3e6a6007be128b720114f9a875d6b370317d62ac80b9", + "sha256:7eee37c1b9815e6505847aa5e68f192e8a1b730c5c7ead39ff317fde9ce29448", + "sha256:7fd88cb91a470b383aafad554c3fe1ccf6dfb2456ff0e84b95335d582a799804", + "sha256:9144ce36ca0824b29ebc2e02ca186e54040ebb224292072250467190fb613b96", + "sha256:925baf6ff1ef2c45169f548cc85204433e061360bfa7d01e1be7ae38bef73194", + "sha256:a636346c6c0e1092ffc202d97ec1843a75937d8c98aaf6771348ad6422e44bb0", + "sha256:a87dbee7ad9dce3aaefada2081843caf08a44a8f52e03e0a4cc5819f8398f2f4", + "sha256:a9e3b8011388e7e373565daa5e92f6c9cb844790dc18e43073212bb3e76f7007", + "sha256:afb53edf1046599991fb4a7d03e601ab5f5422a5435c47ee6ba91ec3b61416a6", + "sha256:b26719890c79a1dae7d53acac5f089d66fd8cc68a81f4e4bd355e45470dc25e1", + "sha256:b7462cdab6fffcda853338e1741ce99706cdf880d921b5a769202ea7b94e8528", + "sha256:b77975465234ff49fdad871c08aa747aae06f5e5be62866595057c43f8d2f62c", + "sha256:c47a8a5d00060122ca5908909478abce7bbf62d812e3fc35c6c802df8fb01fe7", + "sha256:c79e5debbe092e3c93ca4aee44c9a7631bdd407b2871cb541b979fd350bbbc29", + "sha256:d8d40e0121ca1606aa9e78c28a3a7d88a05c06b3ca61630242cded87d8ce55fa", + "sha256:ee2be8b8f72a2772e72ab926a3bccebf47bb727bda41ae070dc91d1fb759b726", + "sha256:f95d28193c3863132b1f55c1056036bf580b5a488d908f7d22a04ace8935a3a9", + "sha256:fadd2a63a2bfd7fb604508e553d1cf68eca250b2fbdbd81213b5f6f2fbf23529" ], - "version": "==4.5.0" + "version": "==4.5.1" }, "mako": { "hashes": [ @@ -543,10 +543,10 @@ }, "packaging": { "hashes": [ - "sha256:3c292b474fda1671ec57d46d739d072bfd495a4f51ad01a055121d81e952b7a3", - "sha256:82f77b9bee21c1bafbf35a84905d604d5d1223801d639cf3ed140bd651c08752" + "sha256:4357f74f47b9c12db93624a82154e9b120fa8293699949152b22065d556079f8", + "sha256:998416ba6962ae7fbd6596850b80e17859a5753ba17c32284f67bfff33784181" ], - "version": "==20.3" + "version": "==20.4" }, "pandas": { "hashes": [ @@ -783,7 +783,7 @@ "spiffworkflow": { "editable": true, "git": "https://github.com/sartography/SpiffWorkflow.git", - "ref": "070d80fd670e129aae7ee949b3e66cc744520e49" + "ref": "7dc54f1205de7006bdda6d966dc957e558f3c7f3" }, "sqlalchemy": { "hashes": [ @@ -919,10 +919,10 @@ }, "packaging": { "hashes": [ - "sha256:3c292b474fda1671ec57d46d739d072bfd495a4f51ad01a055121d81e952b7a3", - "sha256:82f77b9bee21c1bafbf35a84905d604d5d1223801d639cf3ed140bd651c08752" + "sha256:4357f74f47b9c12db93624a82154e9b120fa8293699949152b22065d556079f8", + "sha256:998416ba6962ae7fbd6596850b80e17859a5753ba17c32284f67bfff33784181" ], - "version": "==20.3" + "version": "==20.4" }, "pluggy": { "hashes": [ diff --git a/crc/scripts/complete_template.py b/crc/scripts/complete_template.py index 4fc7eb16..ab5ced15 100644 --- a/crc/scripts/complete_template.py +++ b/crc/scripts/complete_template.py @@ -1,12 +1,14 @@ import copy +import re from io import BytesIO import jinja2 -from docxtpl import DocxTemplate, Listing +from docx.shared import Inches +from docxtpl import DocxTemplate, Listing, InlineImage from crc import session from crc.api.common import ApiError -from crc.models.file import CONTENT_TYPES +from crc.models.file import CONTENT_TYPES, FileModel, FileDataModel from crc.models.workflow import WorkflowModel from crc.scripts.script import Script from crc.services.file_service import FileService @@ -46,7 +48,7 @@ Takes two arguments: def process_template(self, task, study_id, *args, **kwargs): """Entry point, mostly worried about wiring it all up.""" - if len(args) != 2: + if len(args) < 2 or len(args) > 3: raise ApiError(code="missing_argument", message="The CompleteTemplate script requires 2 arguments. The first argument is " "the name of the docx template to use. The second " @@ -60,20 +62,66 @@ Takes two arguments: message="The given task does not match the given study.") file_data_model = FileService.get_workflow_file_data(task.workflow, file_name) - return self.make_template(BytesIO(file_data_model.data), task.data) + # Get images from file/files fields + image_file_data = [] + if len(args) == 3: + images_field_str = re.sub(r'[\[\]]', '', args[2]) + images_field_keys = [v.strip() for v in images_field_str.strip().split(',')] + for field_key in images_field_keys: + if field_key in task.data: + v = task.data[field_key] + file_ids = v if isinstance(v, list) else [v] - def make_template(self, binary_stream, context): + for file_id in file_ids: + if isinstance(file_id, str) and file_id.isnumeric(): + file_id = int(file_id) + + if file_id is not None and isinstance(file_id, int): + if not task.workflow.data[WorkflowProcessor.VALIDATION_PROCESS_KEY]: + # Get the actual image data + image_file_model = session.query(FileModel).filter_by(id=file_id).first() + image_file_data_model = FileService.get_file_data(file_id, image_file_model) + if image_file_data_model is not None: + image_file_data.append(image_file_data_model) + + else: + raise ApiError( + code="not_a_file_id", + message="The CompleteTemplate script requires 2-3 arguments. The third argument should " + "be a comma-delimited list of File IDs") + + return self.make_template(BytesIO(file_data_model.data), task.data, image_file_data) + + def make_template(self, binary_stream, context, image_file_data=None): doc = DocxTemplate(binary_stream) doc_context = copy.deepcopy(context) doc_context = self.rich_text_update(doc_context) + doc_context = self.append_images(doc, doc_context, image_file_data) jinja_env = jinja2.Environment(autoescape=True) doc.render(doc_context, jinja_env) target_stream = BytesIO() doc.save(target_stream) - target_stream.seek(0) # move to the beginning of the stream. + target_stream.seek(0) # move to the beginning of the stream. return target_stream + def append_images(self, template, context, image_file_data): + context['images'] = {} + if image_file_data is not None: + for file_data_model in image_file_data: + fm = file_data_model.file_model + if fm is not None: + context['images'][fm.id] = { + 'name': fm.name, + 'url': '/v1.0/file/%s/data' % fm.id, + 'image': self.make_image(file_data_model, template) + } + + return context + + def make_image(self, file_data_model, template): + return InlineImage(template, BytesIO(file_data_model.data), width=Inches(6.5)) + def rich_text_update(self, context): """This is a bit of a hack. If we find that /n characters exist in the data, we want these to come out in the final document without requiring someone to predict it in the diff --git a/crc/services/file_service.py b/crc/services/file_service.py index b19b7d6e..74575cc4 100644 --- a/crc/services/file_service.py +++ b/crc/services/file_service.py @@ -156,18 +156,26 @@ class FileService(object): query = session.query(FileModel).filter_by(is_reference=is_reference) if workflow_spec_id: query = query.filter_by(workflow_spec_id=workflow_spec_id) - if study_id: - query = query.filter_by(study_id=study_id) - if workflow_id: - query = query.filter_by(workflow_id=workflow_id) - if task_id: - query = query.filter_by(task_id=str(task_id)) - if form_field_key: - query = query.filter_by(form_field_key=form_field_key) - if name: - query = query.filter_by(name=name) - if irb_doc_code: - query = query.filter_by(irb_doc_code=irb_doc_code) + if all(v is None for v in [study_id, workflow_id, task_id, form_field_key]): + query = query.filter_by( + study_id=None, + workflow_id=None, + task_id=None, + form_field_key=None, + ) + else: + if study_id: + query = query.filter_by(study_id=study_id) + if workflow_id: + query = query.filter_by(workflow_id=workflow_id) + if task_id: + query = query.filter_by(task_id=str(task_id)) + if form_field_key: + query = query.filter_by(form_field_key=form_field_key) + if name: + query = query.filter_by(name=name) + if irb_doc_code: + query = query.filter_by(irb_doc_code=irb_doc_code) results = query.all() return results diff --git a/crc/services/workflow_processor.py b/crc/services/workflow_processor.py index 7f06c47b..8fe126bc 100644 --- a/crc/services/workflow_processor.py +++ b/crc/services/workflow_processor.py @@ -287,6 +287,10 @@ class WorkflowProcessor(object): form_data[field.id] = random.randint(1, 1000) elif field.type == 'boolean': form_data[field.id] = random.choice([True, False]) + elif field.type == 'file': + form_data[field.id] = random.randint(1, 100) + elif field.type == 'files': + form_data[field.id] = random.randrange(1, 100) else: form_data[field.id] = WorkflowProcessor._random_string() if task.data is None: diff --git a/crc/services/workflow_service.py b/crc/services/workflow_service.py index 6fbb0f30..c5294f1f 100644 --- a/crc/services/workflow_service.py +++ b/crc/services/workflow_service.py @@ -36,7 +36,7 @@ class WorkflowService(object): @classmethod def test_spec(cls, spec_id): - """Runs a spec through it's paces to see if it results in any errors. Not full proof, but a good + """Runs a spec through it's paces to see if it results in any errors. Not fool-proof, but a good sanity check.""" version = WorkflowProcessor.get_latest_version_string(spec_id) spec = WorkflowProcessor.get_spec(spec_id, version) diff --git a/crc/static/templates/placeholder.docx b/crc/static/templates/placeholder.docx new file mode 100644 index 0000000000000000000000000000000000000000..7c27415f11191589262f65e72d9b321365587b1b GIT binary patch literal 4082 zcmaJ^2T)V%5~f3f2q+*`y7Vg2t4IwcAiX2K1u#gD0sq0j{?%+CEj}?VYbkLjUM@p3D9`Ta>ZcD{JGUlo!3Kj%PK1t>DChzGq<@ zIQB`u(W3&RJUCnf{m4rrP;#PG(mQlUMS|05N}|^!+1MiIGkPfrJq9Zk zGE#x%x!mr?rhUrScpAzOf$y5PAKa2I$d{Ua<=t=FM$)vedmtwp&Q#AAd%@kI)UKvM zUYyY=%yq)4IIJRY*IOp@RH(h@VML+)sZrcj8J#hW>aefY@>b0)nSv#@+9AhYGX9Z6 z&-#2P9OmI8MQ9A$WgE$p~FAn>hA)+?jHb^a1IC%5B2j! zB1VkRl60}_%37{lZ{L&%nq@8|at1!S!cT%_Vu{FQ=5KcMul&9$B?hF|fo!Zkc7N^D zq4?cCIEwCG1AjI)$<--ys#lgjtO`|1ou-vxF*Ir|H~`?WzAMrJ7}ZoKl>z}|vmR<~ zg7k~M3-3!AHye(Q#yMQ;9tXF~u#l%1twz-9*06{OveQ4NP}$PtddeqGyE6w~eYt(} zYn9?|hCnEJ`#N9D`%O!W({7g8=gB}=;vM8R(&WsP){D21WTC1l1zMxuW+GLs+1k`z zt;37NA@wGjNE8kL6mn6?{cGK@tPE<@7Z}OQmT;|iQ43c^y*GTWi_?bXVfQBIwX8?H z?AEroSK;@8N3!>lWQ6)J=*}G_u?cnscz-1)fZN8wC&NjM$L^2d(w_v^#s9h+0o_bf zd*g0N2H3WxC~WSnU)L9D?V@-AJ%cFE$jVhWv)9#L)VFW>x70-s!u_}Zf`+eIZeTPl zy3QvtHsut_^9U04c#E4UXf01GKhuo9VHRjJ4J`da#Wa$bo41%WP2pfr)g@0Xi_;!- zH!z-frGRwy#W3+TAuI)rA>O7+(tgUEn2nOaH8Jh zz02t=mO|unXX7K?P&*UzwS#VI@P}}WUawiiV4_V6PG^qlJ+01qAirD?pYhYhMyZGh>)Yd*#Th)?3Sd9+eYyW&z73gAxa@v0z|fmq>#;BvVZ}> z@CedG8^QT*>;Y_DO2W}`KiPhq((hiBs9toTN%LeWeSl!g6#rwmUPqsisU4syIJ$@J zELr0fNKY78?7KAItHQhKZ}%KbMPX>9nE-|>zhp6H-Rff6heSwS3s6k$Z=aVSD8M-8B8eG_V<5umI`r7Rb10)4ck%T^iYy4 zir%Bk9!FD?pI#li=}l(t12VXjDwBRkFy5+GIPG>`=cqVz#0U~d$s`_3R!BTj4diH;zVB6P^1<7>^ zSRV^K{zz|KZ5c9D=&P*7aO=ms(XJh@A$pIqV?npQwE>yzBSyq=`V#K!(2H$*+V zz?LdWz=*avO^e%nmegbAt29kEDth$FJBLpyWTX*r0o{A<&kb61TDF5HeFA-I%C2NL z=P1J|(FewrmJNNWLrk`(N(ZW)7A_Q?L*~OOD_l2h@8eWh=#0tPB3e~P^Ai=-LbrYp zURNK6iUZ$L=A?fD_um>i*-Ry7Ly#Ffw%a0Pvwqgewq%z-+9gq_$VfVElX@nmC8XJ1 z)3XkI-T_+9qy#gMXhjaM4X>oVL@&T62CR!Lov5?KEJYC(>J`5BwT^x3@QAFPPQQUm zR@Sxf&6U7?Y~c`yt_mT`_reKwn%rmNlY`7;z;j7`uF&%C%TWV`k1|rxo$;qct^RcpvP0HZgQR_%hOA?iKZLsH(|wjoS4z$Ni{Z=-Wx6D!BHiebdwR@ zu@mjp9|nzNFiY_;V)`nBiSrhmoKxKaXa|c?0Szh=C^3IxN1bozmOJ(|e6Hbm0lwXraG>KyysI~*$yLPH z>S7yCEg&cn81#DVd<~l0uF;_NVE9HC%4IsIguaV8b#x&$inCUncBcK)W!VDI-9WWX z&-`<|&lsE&+Mno+4&JZ*+_XY&58KhCvOn}~pB+ED{w@BKtzmywb= zLVnJ|uoe_8+nxjUh(ukiwj$7Wf2MgR_?*9`je`CsQZDf;z12MoRT4Gk(i$TApERfA zia5B*uAk%Ma&cJ=k|x&DJ6iCN9?;B4ADhe!7~ofpiea(pn#8@0XdG0-5lOq~fD%xs z=!Me*UL5HK?i}%tLgc~*<|~%43q*!mAA~H6#X!Mh}zu)zEz1vS5s4~_rX))wh z@0D)JX)MQ(jK(^yG4;+kb#@QORFLE|vodN3td}Pp36|F_m&CcG+^Q~GUR+L6ozbo? zX||^xmp@!w(?-rM^@PL*xcWYr_<)#Vpn^Mk=b8XH?eEO(ZOPe-2HS+Dc=n0_MrGd! zJ4eCa>@XZsrf8(xq|ejxeTy1If$F>o7tRgsqoQ&6cG4Zt)WOfpCr|J-avWjJkHl!Nu9URu&lamZdR!N1@qb9!w_z6*!tnD0iY*bZgYT1 z+1p1y7=;xj=B=766~k!4avkb!Gzl-YlN#vy)VkVJt>r*7I8KpVT<#p#b2;Q9Y2V9I z;?CEiKHwRXc-nX&3gDH%@>qBc@VP0*l{fOqS<;LoweT+pkFNu1xgnaaSqAd2eS}E1 z7Ho@ECnoPQAd4xKR&hyWNw*A5Eg;Ws^b5(uThjxYy&A~|Ls|OMx{~vNW=n!+BF6*R zZEa}F(6kNO+lUvI%hQd2J(U?acA4O19d){4`RE|GNL8b$<6h$=<&n??m}u z|C8hVclVQhM4CM9A}WZ#z} zTV#z2ktIur6xkBuou21?|9tXnjk&N~Dpkhz}0KlhDB$xsK zKnS~lxjETHrbkAV{otVxZTtWLA-R7AsBbE}#)d#YQ#~C(={t!fb_49DZKMqVR3!24 zJF#iyCa-uOU2Nc7Q#KK%Dk&Wok8MI(#%jpF)J-G$bBJ?ngV1*ztEzI( zD`@Auy!o-iemu;A7gzyxnZ@O#f+Dj{4_qWAB3& z0>7P0y!)=NarNv*f8WLfhLQW{>wB7ucfYI-G_I|TRoOW8spAP>U0Ux&#}@G8X|ZBh zWNZQO|0=ClVDoJcq#yQag#~&pNNKHyD=GzLDF;e7g>O51bnn%FJ-kS|yI;N(?%=j( zIH0vUTHUZ+akt9DuCCSlYTeY~#^+V9thp?ci;XRE-QF*hG*tr@7B6qT$D0L_)Q+zD zfpv<68+^E<)<=xBWc>rBdzAxr9ZlCS=3gG`BvC{_)16JPrg)o|57U2z$p9*!|7QLO z(@3rLKNapw(zwr>7HnM0U%l&lqc6<2G^Dvw1mFm^xRfL;)O2n9?;y$zi~JBN{P#`& z_b_ov(1QOBJPHQJhrbvX;IR~ZaUCP;^SKCcBXAgSaaQ=R26X(gR#qb5csf;Na`LTEyHqs&I2gzox!8y$dy$28fHQ;R zTc0=XBJ=31Y?%V+yQ{BEzd1VEat8dpKODjC4KAc59s0*v(2imKT2_EE+7+jfvt*Ha zzN(mtyIi9Dzp%)hM~!zTGj`Wg!fUA$Zz-`+EI4O7YFvar8H-GJe8NzM#gIgxMcsxX zV7>GT=TEh-{i_?C%zhA6ja6z(zLYOF$2fhu*IR_KHnO>TF@;YfEOYCwFf8V|2y|;L zB1s+iLq~}HWm-dKe7CPi&vqojTNcHe0N*AyQ!19?gpn#6eqn*;KdqX|1HXm;b=}XM zxMCzku9^MHs%scvA@DNPo|gH$oZQq~lkexAmRglGug#@zD8v<7nM}XQ4`27&F8!!Y zi+A)s)Q-mU;em>66>$TZ;%GFOv**ym!0x=?&0E;|BOoBH>`#W1BDY=qV!SxCfZI4| zxYefYJmpeyqQEgEoL}S2PgA+phdJ4-X@fiPYH(nHjs^9L@bG+(kIFn(q__pRF#31n z!yIb-QD~_-SIv)l7`+B1a@^NQ*+KJxJ%sk~`uA7=RJD;-Uc^t7X^OG!Say34S4zg1G?()V0(%pM-jb;|s;?KzbLfaq0Z^_%$&Ca@C zR#fo5n`dBxR0=V-rt1&tz}ZF(^b)0OX_*kF1Kfz18MtWRdtx2R5AI7X!UwpD?;&?%7jvMs$Z6EHXt4>D><>RLYPFOKT^1 zlB+Vih*as7oO3Xo29pbA1*gXEoHX>#sI2)cGjEO|cbn`{*jBy~Au|<(J1qn6 z3_tf=3KqCwIGb&0{w`97l9h8j^vL1v)MMP*mRIUwvyY|RZKusN1PR; zN>&`vY4sITL^yCjTL; zO=07Y&go@0wwib)sDp-gL=6p7ms!-RY15ioS`lCB758JR3(kwB*D6dE3!*^^;Di}8 zh?W$UuldnBp+)dnf~U3`Qq`4bG*7jQtB&-JB{rh5?VD>$;)T>G(4M3gp!ORcuX#tZ z`zaz-3&H%tFNpiB+_sg=uWU&?kYWvDQKap;VYk?La7pFuZw1#B3nJ;A{3m6P7l9Sf z2^?nW!*mTKNrw3Qdal7VB{?3-r0dpTZ<@KKQof1eWs4i)d^w~+jky*D z8ZqA%*HL-Q)H4L zG;81Z$>S@jUUQ=)xrLfsr*cFs=1A&?Px=$`RhRgq;(VP;+NZ;}#L%8bFEwo*mvwr| zprb&xF&Cx|8E*!~VUQeb-pSiH?P7^6)U`V;4k;YEIcs?r!j!FY>h_X#v|lb`>+e~B zOpqKdW9?3}r;MWcAxqKR-+s(elQB@H5PJ5z`fW@s2d#v|bZhLsi+i{_3w3HRX%~T<@ImZ3eB|($W z4+a-{E@PcB6KaBg+n&Z2e9J47*&;nc{Jie`3@3Z+KVF1OuU{{z<$S)gmkI}s_?F*S zT^_OB-{gWZkF?Mcz|)9Pw@XP4Wk~4F0cWmyq<$XW_D;-X8M(<6!)qvoE^NKZGF$t$ z`X@&5y!Q&ajHNie-OOUearHhQ@F3zP&XSJ|r@bE6~tBJCe zdZux8V!XwoPz%SsmU$vNzah_!TT$Uk3EM26NlrXrn`rNJ z@8)=nrJSyckGEuOaf}pYKUiU{iJEhdxW31SL4IU6-R3Y2%^>Qzgn(8FlhZbxlTw;w0tE~ zVh+8hdHIqvv)_<00)uov0$}3+vOWPHLrH!;yq!tTqJPzX=vR!4EoNBGT(H$#tnt94*1ZWQuJlC| z%AoHu|Ll~Nn8tExo#lgcL%{|a@)KjkJmWKF+%Y^2~N* zXoRmMw9Tbkyb1G-C3k7~%B3W<$3wv{8;V7Y~%WsRP)&(B$*1e0G^6vlx3)eRP*jaA8* z*Gpro_X(+(xOsu)9@ZkQxMva@r+b4+;jFghg~debSAdhM-yuRQr6y_oT6(dhC5wXm+mo+8IK1ZJb@D z0@`A}ww(DVz6z2vd^y;He!E2j z}8b@S>2Iz6uLHb5TOg=(6i(vBn;YqLD(Cp?z(0r$4Wwp z=cvq3YI9S-h)u*^PLSdTTkVC#JxHcWOEcuh&yGC#)=TLfitxb^_eZdBft@(Ly~Z zV8UuQ)TJjU^X=uR!5DW7UbiMWgIYxDSz8~DLh#a9ZcRZ9D0FRv@xDc!zRpQQNh;A7 zUPm&EBSBK`fA0Oaj&u^&4x-m;&t)Vn{OlOE=7ETe2arPPf}^@5Dfb#aY*=95sIwxH zLcev{@f)}IG0gYG#PFOC>h2S1gYh*a08D4lfH-rn+sG)@Vl>a3{STVDi+661Kam3$ zvo+;v*pZ-9`Y? zGKdI2!MB6n7(}|zMkIjVY%C`T6*>P@LhQ8I+NF!Z2UnuIT-Z@sK_3F}D)NE5I4CX> z8V`6W+xHq2dX-F?FyYAA~KM9(@zkoQX0x0&?7`N*Yohb9~l1Ld&-l z6oBlkO*4A*ds@2k!>8lH&yI70OPJAl>L*I%m?S_oLd@+LAni4O_%0%XCK%I$Q-M!! zuDyCu%|{#30eS=me~o=Vo3F-}ZOR6!n8LuPv^*vF7(_)RK=Dn;-qu4q5m;&}3SPa9 z#)W&h&Gsf0>8l|D4v_~bEAPj)L}1QO5^%Wr{XKkpC*(y8C_{6z_VWRRjAbaQgB<5x z@*RYMLvPcLu&94|=b9zV#5F+?v}$(rkWK>>z&}WwD*Z~p02HErwXcdB?HoVYpF5np z$SKJCP_l*tsG`;VG$2bNw3XM+O z?w>ll_zA}8Iqfj3auBzRLwL4tp3b)vvz!UT0Qfb>UOqvT#3zVyx(I}IQi{?N-tsuh z!cpQZ>Nxu^i*5y-B$onD7Z@(1E9}X*~gC;iVPZONL@_XbwnCi zQsyd~vWhN4K+f_=P^~KrSbL;{HVi2yjs@>pb8-MHg`wmNA{vo>XUB(uLzeYZ{kABa zIv&Vat37+H!m~@7ji(I&V0wFRV!!iQBr1-K8)gUn%S)QhTn|OATx}%(<#F0McQeSz zlzWUH((*y%IS>R|x?jM6L)%wY$uV_m`(k)5n&4IYV;!?J@e1of;Rfl_fAKWhZdnB&KJ zU|e-57}m*Px%;c=wOihD4lFqFI>07rhEbZ=4p?hCSSx8QF2$-o7$8x5?Q5t&XTcu? z>I|b;wGD7fRm<8a3=d4nP#MB&G;S=|e>~hZF15t84^r4?j>@0@+q0imSl3!^IVCP| z6?F$-z$rK6msy@76T?q_OUr(T1I$$F`bB*`PR9*LGAT)DCF_32SLLRsb%3Pn*F$c9 zu)w3-6@y>dJ8i*DNk_?)jNA9%IPU3&#B(=Gbw668rFXorT^$g@xza>5NM&>zolliT*D@3t1PGG(f#OD1pD(bQAaUpY=Vi_7 z3ZGZZeExP%rXNp4fp1K_`hK^mS>>$jYI>oQ!z$g1YfA(QEGCi58c(iTpJdfwLRzU5 z9Fc6jQ1%Rvk)L{3cl*;7Z4nB6qPg^7V8PQAh zvf;l(Ynm1UdGL0=yaj)Ee5CB)U99k**>zrV<+Tp!r@KJXwZN~3Q#Bv73hCZD@%LDR z_YI4XvTqx-F|a7C2>)0#R%G&{+tk=38+pN^Fl^L@{g-LvYMWW9DT zU`_9Pv9XCPl*xGFh0GVj@tSj6VLt}woQ2ar=br5v0usj@#L>?c(>N>Wbz)b z^BMN6YTR5I34-M7p|DXm-RQ*>c1`%R6wxCY#nPetC4$`}?yUYm@<&_nnrE27O@ z`!E%3%{@RIM-dVNKE?t*x+ub*-}^P{I&-{QpU#Hv_PC>9)DG TL8sXF008|<#)MKGGW~x5f{HQg literal 0 HcmV?d00001 From 2b5687c3a3e3c2bf22a79d465db6847bf7648665 Mon Sep 17 00:00:00 2001 From: Aaron Louie Date: Wed, 20 May 2020 00:10:32 -0400 Subject: [PATCH 4/5] Fixes pernicious bug where template document versions were not being updated properly, and template completion script was not honoring version numbers --- crc/scripts/complete_template.py | 76 ++++++++++++++++++------------ crc/services/file_service.py | 15 ++++-- crc/services/workflow_processor.py | 16 +++++-- 3 files changed, 68 insertions(+), 39 deletions(-) diff --git a/crc/scripts/complete_template.py b/crc/scripts/complete_template.py index ab5ced15..4939a3b5 100644 --- a/crc/scripts/complete_template.py +++ b/crc/scripts/complete_template.py @@ -29,12 +29,12 @@ Takes two arguments: def do_task_validate_only(self, task, study_id, *args, **kwargs): """For validation only, process the template, but do not store it in the database.""" - self.process_template(task, study_id, *args, **kwargs) + self.process_template(task, study_id, None, *args, **kwargs) def do_task(self, task, study_id, *args, **kwargs): workflow_id = task.workflow.data[WorkflowProcessor.WORKFLOW_ID_KEY] - final_document_stream = self.process_template(task, study_id, *args, **kwargs) workflow = session.query(WorkflowModel).filter(WorkflowModel.id == workflow_id).first() + final_document_stream = self.process_template(task, study_id, workflow, *args, **kwargs) file_name = args[0] irb_doc_code = args[1] FileService.add_task_file(study_id=study_id, @@ -46,7 +46,7 @@ Takes two arguments: binary_data=final_document_stream.read(), irb_doc_code=irb_doc_code) - def process_template(self, task, study_id, *args, **kwargs): + def process_template(self, task, study_id, workflow=None, *args, **kwargs): """Entry point, mostly worried about wiring it all up.""" if len(args) < 2 or len(args) > 3: raise ApiError(code="missing_argument", @@ -61,38 +61,56 @@ Takes two arguments: raise ApiError(code="invalid_argument", message="The given task does not match the given study.") - file_data_model = FileService.get_workflow_file_data(task.workflow, file_name) + file_data_model = None + if workflow is not None: + # Get the workflow's latest files + joined_file_data_models = WorkflowProcessor\ + .get_file_models_for_version(workflow.workflow_spec_id, workflow.spec_version) + + for joined_file_data in joined_file_data_models: + if joined_file_data.file_model.name == file_name: + file_data_model = session.query(FileDataModel).filter_by(id=joined_file_data.id).first() + + if workflow is None or file_data_model is None: + file_data_model = FileService.get_workflow_file_data(task.workflow, file_name) # Get images from file/files fields - image_file_data = [] if len(args) == 3: - images_field_str = re.sub(r'[\[\]]', '', args[2]) - images_field_keys = [v.strip() for v in images_field_str.strip().split(',')] - for field_key in images_field_keys: - if field_key in task.data: - v = task.data[field_key] - file_ids = v if isinstance(v, list) else [v] - - for file_id in file_ids: - if isinstance(file_id, str) and file_id.isnumeric(): - file_id = int(file_id) - - if file_id is not None and isinstance(file_id, int): - if not task.workflow.data[WorkflowProcessor.VALIDATION_PROCESS_KEY]: - # Get the actual image data - image_file_model = session.query(FileModel).filter_by(id=file_id).first() - image_file_data_model = FileService.get_file_data(file_id, image_file_model) - if image_file_data_model is not None: - image_file_data.append(image_file_data_model) - - else: - raise ApiError( - code="not_a_file_id", - message="The CompleteTemplate script requires 2-3 arguments. The third argument should " - "be a comma-delimited list of File IDs") + image_file_data = self.get_image_file_data(args[2], task) + else: + image_file_data = None return self.make_template(BytesIO(file_data_model.data), task.data, image_file_data) + def get_image_file_data(self, fields_str, task): + image_file_data = [] + images_field_str = re.sub(r'[\[\]]', '', fields_str) + images_field_keys = [v.strip() for v in images_field_str.strip().split(',')] + for field_key in images_field_keys: + if field_key in task.data: + v = task.data[field_key] + file_ids = v if isinstance(v, list) else [v] + + for file_id in file_ids: + if isinstance(file_id, str) and file_id.isnumeric(): + file_id = int(file_id) + + if file_id is not None and isinstance(file_id, int): + if not task.workflow.data[WorkflowProcessor.VALIDATION_PROCESS_KEY]: + # Get the actual image data + image_file_model = session.query(FileModel).filter_by(id=file_id).first() + image_file_data_model = FileService.get_file_data(file_id, image_file_model) + if image_file_data_model is not None: + image_file_data.append(image_file_data_model) + + else: + raise ApiError( + code="not_a_file_id", + message="The CompleteTemplate script requires 2-3 arguments. The third argument should " + "be a comma-delimited list of File IDs") + + return image_file_data + def make_template(self, binary_stream, context, image_file_data=None): doc = DocxTemplate(binary_stream) doc_context = copy.deepcopy(context) diff --git a/crc/services/file_service.py b/crc/services/file_service.py index 74575cc4..f5932836 100644 --- a/crc/services/file_service.py +++ b/crc/services/file_service.py @@ -116,7 +116,7 @@ class FileService(object): version=file_model.latest_version ).with_for_update().first() md5_checksum = UUID(hashlib.md5(binary_data).hexdigest()) - if (file_data_model is not None and md5_checksum == file_data_model.md5_hash): + if (file_data_model is not None) and (md5_checksum == file_data_model.md5_hash): # This file does not need to be updated, it's the same file. return file_model @@ -141,12 +141,17 @@ class FileService(object): file_model.primary_process_id = WorkflowProcessor.get_process_id(bpmn) file_model.latest_version = version - file_data_model = FileDataModel(data=binary_data, file_model=file_model, version=version, - md5_hash=md5_checksum, last_updated=datetime.now()) + new_file_data_model = FileDataModel( + data=binary_data, file_model_id=file_model.id, file_model=file_model, + version=version, md5_hash=md5_checksum, last_updated=datetime.now() + ) - session.add_all([file_model, file_data_model]) + session.add_all([file_model, new_file_data_model]) session.commit() session.flush() # Assure the id is set on the model before returning it. + + db_file_model = session.query(FileModel).filter_by(id=file_model.id).first() + print('db_file_model', db_file_model) return file_model @staticmethod @@ -202,7 +207,7 @@ 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 it's data""" + """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/crc/services/workflow_processor.py b/crc/services/workflow_processor.py index 8fe126bc..8170fac4 100644 --- a/crc/services/workflow_processor.py +++ b/crc/services/workflow_processor.py @@ -212,7 +212,7 @@ class WorkflowProcessor(object): return full_version @staticmethod - def __get_file_models_for_version(workflow_spec_id, version): + def get_file_models_for_version(workflow_spec_id, version): file_id_strings = re.findall('\((.*)\)', version)[0].split(".") file_ids = [int(i) for i in file_id_strings] files = session.query(FileDataModel)\ @@ -237,12 +237,17 @@ class WorkflowProcessor(object): .all() @staticmethod - def get_spec(workflow_spec_id, version): + def get_spec(workflow_spec_id, version=None): """Returns the requested version of the specification, - or the lastest version if none is specified.""" + or the latest version if none is specified.""" parser = WorkflowProcessor.get_parser() process_id = None - file_data_models = WorkflowProcessor.__get_file_models_for_version(workflow_spec_id, version) + + if version is None: + file_data_models = WorkflowProcessor.__get_latest_file_models(workflow_spec_id) + else: + file_data_models = WorkflowProcessor.get_file_models_for_version(workflow_spec_id, version) + for file_data in file_data_models: if file_data.file_model.type == FileType.bpmn: bpmn: ElementTree.Element = ElementTree.fromstring(file_data.data) @@ -321,7 +326,8 @@ class WorkflowProcessor(object): Returns the new version. """ version = WorkflowProcessor.get_latest_version_string(self.workflow_spec_id) - spec = WorkflowProcessor.get_spec(self.workflow_spec_id, version) + spec = WorkflowProcessor.get_spec(self.workflow_spec_id) # Force latest version by NOT specifying version + # spec = WorkflowProcessor.get_spec(self.workflow_spec_id, version) bpmn_workflow = BpmnWorkflow(spec, script_engine=self._script_engine) bpmn_workflow.data = self.bpmn_workflow.data for task in bpmn_workflow.get_tasks(SpiffTask.READY): From 58189285adaff3d45651d9424ccb921b8f47673c Mon Sep 17 00:00:00 2001 From: Aaron Louie Date: Wed, 20 May 2020 00:12:48 -0400 Subject: [PATCH 5/5] Cleans up --- crc/scripts/complete_template.py | 2 +- crc/services/file_service.py | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/crc/scripts/complete_template.py b/crc/scripts/complete_template.py index 4939a3b5..64ab9531 100644 --- a/crc/scripts/complete_template.py +++ b/crc/scripts/complete_template.py @@ -108,7 +108,7 @@ Takes two arguments: code="not_a_file_id", message="The CompleteTemplate script requires 2-3 arguments. The third argument should " "be a comma-delimited list of File IDs") - + return image_file_data def make_template(self, binary_stream, context, image_file_data=None): diff --git a/crc/services/file_service.py b/crc/services/file_service.py index f5932836..e361c332 100644 --- a/crc/services/file_service.py +++ b/crc/services/file_service.py @@ -1,3 +1,4 @@ +import hashlib import json import os from datetime import datetime @@ -11,7 +12,6 @@ from crc.api.common import ApiError from crc.models.file import FileType, FileDataModel, FileModel, LookupFileModel, LookupDataModel from crc.models.workflow import WorkflowSpecModel from crc.services.workflow_processor import WorkflowProcessor -import hashlib class FileService(object): @@ -150,8 +150,6 @@ class FileService(object): session.commit() session.flush() # Assure the id is set on the model before returning it. - db_file_model = session.query(FileModel).filter_by(id=file_model.id).first() - print('db_file_model', db_file_model) return file_model @staticmethod