diff --git a/crc/api.yml b/crc/api.yml index dbc10fa4..833efd54 100644 --- a/crc/api.yml +++ b/crc/api.yml @@ -160,7 +160,6 @@ paths: parameters: - name: workflow_spec_id in: path - required: false description: The unique id of an existing workflow specification to modify. schema: type: string diff --git a/crc/models/study.py b/crc/models/study.py index d6209013..99d1d585 100644 --- a/crc/models/study.py +++ b/crc/models/study.py @@ -96,7 +96,7 @@ class WorkflowMetadata(object): def __init__(self, id, name = None, display_name = None, description = None, spec_version = None, category_id = None, category_display_name = None, state: WorkflowState = None, status: WorkflowStatus = None, total_tasks = None, completed_tasks = None, - is_review=None,display_order = None): + is_review=None,display_order = None, state_message = None): self.id = id self.name = name self.display_name = display_name @@ -105,6 +105,7 @@ class WorkflowMetadata(object): self.category_id = category_id self.category_display_name = category_display_name self.state = state + self.state_message = state_message self.status = status self.total_tasks = total_tasks self.completed_tasks = completed_tasks @@ -140,7 +141,7 @@ class WorkflowMetadataSchema(ma.Schema): model = WorkflowMetadata additional = ["id", "name", "display_name", "description", "total_tasks", "completed_tasks", "display_order", - "category_id", "is_review", "category_display_name"] + "category_id", "is_review", "category_display_name", "state_message"] unknown = INCLUDE diff --git a/crc/scripts/study_info.py b/crc/scripts/study_info.py index cabcb35a..19dab1af 100644 --- a/crc/scripts/study_info.py +++ b/crc/scripts/study_info.py @@ -80,7 +80,12 @@ class StudyInfo(Script): 'display': 'Optional', 'unique': 'Yes', 'user_id': 'asd3v', - 'error': 'Unable to locate a user with id asd3v in LDAP'} + 'error': 'Unable to locate a user with id asd3v in LDAP'}, + 'DEPT_CH': { + 'label': 'Department Chair', + 'display': 'Always', + 'unique': 'Yes', + 'user_id': 'lb3dp'} }, "documents": { 'AD_CoCApp': {'category1': 'Ancillary Document', 'category2': 'CoC Application', 'category3': '', diff --git a/crc/services/error_service.py b/crc/services/error_service.py index bbdd87fb..2d66f0eb 100644 --- a/crc/services/error_service.py +++ b/crc/services/error_service.py @@ -17,7 +17,8 @@ generic_message = """Workflow validation failed. For more information about the known_errors = {'Error is Non-default exclusive outgoing sequence flow without condition': {'hint': 'Add a Condition Type to your gateway path.'}, - 'Could not set task title on task (\w+) with \'(.*)\' property because \\1: Error evaluating expression \'(.*)\', "\'Box\' object has no attribute \'\\2\'"$': + 'Could not set task title on task (\w+) with \'(.*)\' property because \\1: Error evaluating ' + 'expression \'(.*)\', $': {'hint': 'You are overriding the title for task `{task_id}`, using the `{property}` extension, and it is causing an error. Look under the extensions tab for the task, and check the value you are setting for the property.', 'groups': {'task_id': 0, 'property': 1}}} diff --git a/crc/services/study_service.py b/crc/services/study_service.py index ed56d595..749a4e14 100644 --- a/crc/services/study_service.py +++ b/crc/services/study_service.py @@ -1,7 +1,7 @@ from copy import copy from datetime import datetime from typing import List -from crc.services.cache_service import timeit + import requests from SpiffWorkflow import WorkflowException from SpiffWorkflow.exceptions import WorkflowTaskExecException @@ -53,7 +53,6 @@ class StudyService(object): return studies @staticmethod - @timeit def get_study(study_id, study_model: StudyModel = None, do_status=True): """Returns a study model that contains all the workflows organized by category. IMPORTANT: This is intended to be a lightweight call, it should never involve @@ -73,7 +72,7 @@ class StudyService(object): study.last_activity_user = LdapService.user_info(last_event.user_uid).display_name study.last_activity_date = last_event.date study.categories = StudyService.get_categories() - workflow_metas = StudyService.__get_workflow_metas(study_id) + workflow_metas = StudyService._get_workflow_metas(study_id) files = FileService.get_files_for_study(study.id) files = (File.from_models(model, FileService.get_file_data(model.id), FileService.get_doc_dictionary()) for model in files) @@ -85,8 +84,9 @@ class StudyService(object): # this line is taking 99% of the time that is used in get_study. # see ticket #196 if do_status: - status = StudyService.__get_study_status(study_model) - study.warnings = StudyService.__update_status_of_workflow_meta(workflow_metas, status) + # __get_study_status() runs the master workflow to generate the status dictionary + status = StudyService._get_study_status(study_model) + study.warnings = StudyService._update_status_of_workflow_meta(workflow_metas, status) # Group the workflows into their categories. for category in study.categories: @@ -418,24 +418,36 @@ class StudyService(object): db.session.commit() @staticmethod - def __update_status_of_workflow_meta(workflow_metas, status): + def _update_status_of_workflow_meta(workflow_metas, status): # Update the status on each workflow warnings = [] for wfm in workflow_metas: - if wfm.name in status.keys(): - if not WorkflowState.has_value(status[wfm.name]): - warnings.append(ApiError("invalid_status", - "Workflow '%s' can not be set to '%s', should be one of %s" % ( - wfm.name, status[wfm.name], ",".join(WorkflowState.list()) - ))) - else: - wfm.state = WorkflowState[status[wfm.name]] - else: + wfm.state_message = '' + # do we have a status for you + if wfm.name not in status.keys(): warnings.append(ApiError("missing_status", "No status specified for workflow %s" % wfm.name)) + continue + if not isinstance(status[wfm.name], dict): + warnings.append(ApiError(code='invalid_status', + message=f'Status must be a dictionary with "status" and "message" keys. Name is {wfm.name}. Status is {status[wfm.name]}')) + continue + if 'status' not in status[wfm.name].keys(): + warnings.append(ApiError("missing_status", + "Workflow '%s' does not have a status setting" % wfm.name)) + continue + if not WorkflowState.has_value(status[wfm.name]['status']): + warnings.append(ApiError("invalid_state", + "Workflow '%s' can not be set to '%s', should be one of %s" % ( + wfm.name, status[wfm.name]['status'], ",".join(WorkflowState.list()) + ))) + continue + wfm.state = WorkflowState[status[wfm.name]['status']] + if 'message' in status[wfm.name].keys(): + wfm.state_message = status[wfm.name]['message'] return warnings @staticmethod - def __get_workflow_metas(study_id): + def _get_workflow_metas(study_id): # Add in the Workflows for each category workflow_models = db.session.query(WorkflowModel). \ join(WorkflowSpecModel). \ @@ -448,8 +460,7 @@ class StudyService(object): return workflow_metas @staticmethod - @timeit - def __get_study_status(study_model): + def _get_study_status(study_model): """Uses the Top Level Workflow to calculate the status of the study, and it's workflow models.""" master_specs = db.session.query(WorkflowSpecModel). \ diff --git a/crc/services/workflow_service.py b/crc/services/workflow_service.py index 291004a4..85403e5c 100644 --- a/crc/services/workflow_service.py +++ b/crc/services/workflow_service.py @@ -95,31 +95,38 @@ class WorkflowService(object): WorkflowService.delete_test_data() raise ApiError.from_workflow_exception("workflow_validation_exception", str(we), we) + count = 0 while not processor.bpmn_workflow.is_completed(): - try: - processor.bpmn_workflow.get_deep_nav_list() # Assure no errors with navigation. - processor.bpmn_workflow.do_engine_steps() - tasks = processor.bpmn_workflow.get_tasks(SpiffTask.READY) - for task in tasks: - if task.task_spec.lane is not None and task.task_spec.lane not in task.data: - raise ApiError.from_task("invalid_role", - f"This task is in a lane called '{task.task_spec.lane}', The " - f" current task data must have information mapping this role to " - f" a unique user id.", task) - task_api = WorkflowService.spiff_task_to_api_task( - task, - add_docs_and_forms=True) # Assure we try to process the documentation, and raise those errors. - # make sure forms have a form key - if hasattr(task_api, 'form') and task_api.form is not None and task_api.form.key == '': - raise ApiError(code='missing_form_key', - message='Forms must include a Form Key.', - task_id=task.id, - task_name=task.get_name()) - WorkflowService.populate_form_with_random_data(task, task_api, required_only) - processor.complete_task(task) - except WorkflowException as we: - WorkflowService.delete_test_data() - raise ApiError.from_workflow_exception("workflow_validation_exception", str(we), we) + if count < 100: # check for infinite loop + try: + processor.bpmn_workflow.get_deep_nav_list() # Assure no errors with navigation. + processor.bpmn_workflow.do_engine_steps() + tasks = processor.bpmn_workflow.get_tasks(SpiffTask.READY) + for task in tasks: + if task.task_spec.lane is not None and task.task_spec.lane not in task.data: + raise ApiError.from_task("invalid_role", + f"This task is in a lane called '{task.task_spec.lane}', The " + f" current task data must have information mapping this role to " + f" a unique user id.", task) + task_api = WorkflowService.spiff_task_to_api_task( + task, + add_docs_and_forms=True) # Assure we try to process the documentation, and raise those errors. + # make sure forms have a form key + if hasattr(task_api, 'form') and task_api.form is not None and task_api.form.key == '': + raise ApiError(code='missing_form_key', + message='Forms must include a Form Key.', + task_id=task.id, + task_name=task.get_name()) + WorkflowService.populate_form_with_random_data(task, task_api, required_only) + processor.complete_task(task) + count += 1 + except WorkflowException as we: + WorkflowService.delete_test_data() + raise ApiError.from_workflow_exception("workflow_validation_exception", str(we), we) + else: + raise ApiError.from_task(code='validation_loop', + message=f'There appears to be an infinite loop in the validation. Task is {task.task_spec.description}', + task=task) WorkflowService.delete_test_data() WorkflowService._process_documentation(processor.bpmn_workflow.last_task.parent.parent) diff --git a/crc/templates/mail_main_template.html b/crc/templates/mail_main_template.html index c8841ddc..b9f9ac7e 100644 --- a/crc/templates/mail_main_template.html +++ b/crc/templates/mail_main_template.html @@ -89,6 +89,10 @@ padding-top: 10px; } + td#logo-td { + width: 50px; + } + .footer, .header { clear: both; margin-top: 10px; @@ -361,7 +365,7 @@ -
+ diff --git a/deploy/requirements.txt b/deploy/requirements.txt index b9ab7b4c..f20a4a5c 100644 --- a/deploy/requirements.txt +++ b/deploy/requirements.txt @@ -34,10 +34,10 @@ imagesize==1.2.0 inflection==0.5.1 itsdangerous==1.1.0 jdcal==1.4.1 -jinja2==2.11.2 +jinja2==2.11.3 jsonschema==3.2.0 ldap3==2.8.1 -lxml==4.6.2 +lxml==4.6.3 mako==1.1.3 markdown==3.3.3 markupsafe==1.1.1 @@ -63,7 +63,7 @@ python-docx==0.8.10 python-editor==1.0.4 python-levenshtein==0.12.0 pytz==2020.4 -pyyaml==5.3.1 +pyyaml==5.4 recommonmark==0.6.0 requests==2.25.0 sentry-sdk==0.14.4 diff --git a/tests/data/decision_table_dictionary_output/decision_table_dictionary_output.bpmn b/tests/data/decision_table_dictionary_output/decision_table_dictionary_output.bpmn new file mode 100644 index 00000000..1117386f --- /dev/null +++ b/tests/data/decision_table_dictionary_output/decision_table_dictionary_output.bpmn @@ -0,0 +1,71 @@ + + + + Testing Workflow Status Messages + + SequenceFlow_0x4n744 + + + + + SequenceFlow_1o630oy + SequenceFlow_1foyag7 + + + + <div><span>Good Bye {{ dog.name }}</span></div> +<div><span>You are such a good {{ dog.breed }}</span></div> + + SequenceFlow_1foyag7 + SequenceFlow_1bc1ugw + + + SequenceFlow_1bc1ugw + + + + + + + + + SequenceFlow_0x4n744 + SequenceFlow_1o630oy + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/decision_table_dictionary_output/decision_table_dictionary_output.dmn b/tests/data/decision_table_dictionary_output/decision_table_dictionary_output.dmn new file mode 100644 index 00000000..3f74accf --- /dev/null +++ b/tests/data/decision_table_dictionary_output/decision_table_dictionary_output.dmn @@ -0,0 +1,69 @@ + + + + + + + name + + + + + + + 'Layla' + + + 'Layla' + + + 'Aussie' + + + + + 'Mona' + + + 'Mona' + + + 'Aussie Mix' + + + + + 'Jerry' + + + 'Jerry' + + + 'Aussie Mix' + + + + + 'Zoey' + + + 'Zoey' + + + 'Healer' + + + + + 'Etta' + + + 'Etta' + + + 'Healer Mix' + + + + + diff --git a/tests/data/infinite_loop/infinite_loop.bpmn b/tests/data/infinite_loop/infinite_loop.bpmn new file mode 100644 index 00000000..7fb41c13 --- /dev/null +++ b/tests/data/infinite_loop/infinite_loop.bpmn @@ -0,0 +1,97 @@ + + + + + Flow_0ldlhrt + + + + Flow_0ldlhrt + Flow_05mrx8v + Flow_0pddur1 + investigators = study_info('investigators') + + + + Investigators: {{ investigators }} + Flow_0pddur1 + Flow_03m3cuy + + + Flow_03m3cuy + Flow_05mrx8v + Flow_1212fe2 + + + + + hasattr(investigators, 'DEPT_CH') + + + Flow_14jn215 + + + + # Thank You + Flow_1212fe2 + Flow_14jn215 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/study/test_study_status_message.py b/tests/study/test_study_status_message.py new file mode 100644 index 00000000..6e2fbee2 --- /dev/null +++ b/tests/study/test_study_status_message.py @@ -0,0 +1,66 @@ +from tests.base_test import BaseTest +from crc import db, session +from crc.models.study import StudyModel +from crc.models.workflow import WorkflowState +from crc.services.study_service import StudyService + + +class TestStudyStatusMessage(BaseTest): + + """The workflow runs with a workflow_meta.name of `random_fact` + Add an entry to `status` dictionary with a key of `random_fact`""" + + def run_update_status(self, status): + # shared code + self.load_example_data() + study_model = session.query(StudyModel).first() + workflow_metas = StudyService._get_workflow_metas(study_model.id) + warnings = StudyService._update_status_of_workflow_meta(workflow_metas, status) + return workflow_metas, warnings + + def test_study_status_message(self): + # these are the passing tests + # we loop through each Workflow state + # (hidden,disabled,required,optional) + for state in WorkflowState: + # use state.value to set status['status'], + status = {'random_fact': + {'status': state.value, + 'message': 'This is my status message!'}} + + # call run_update_status(), + workflow_metas, warnings = self.run_update_status(status) + + # and assert the values of workflow_metas[0].state and workflow_metas[0].state_message + self.assertEqual(0, len(warnings)) + self.assertEqual(state, workflow_metas[0].state) + self.assertEqual('This is my status message!', workflow_metas[0].state_message) + + def test_study_status_message_bad_name(self): + # we don't have an entry for you in the status dictionary + status = {'bad_name': {'status': 'hidden', 'message': 'This is my status message!'}} + workflow_metas, warnings = self.run_update_status(status) + + self.assertEqual(1, len(warnings)) + self.assertEqual('missing_status', warnings[0].code) + self.assertEqual('No status specified for workflow random_fact', warnings[0].message) + + def test_study_status_message_not_dict(self): + # your entry in the status dictionary is not a dictionary + status = {'random_fact': 'This is my status message!'} + workflow_metas, warnings = self.run_update_status(status) + + self.assertEqual(1, len(warnings)) + self.assertEqual('invalid_status', warnings[0].code) + self.assertEqual('Status must be a dictionary with "status" and "message" keys. Name is random_fact. Status is This is my status message!', + warnings[0].message) + + def test_study_status_message_bad_state(self): + # you have an invalid state + # I.e., not in (hidden,disabled,required,optional) + status = {'random_fact': {'status': 'hide', 'message': 'This is my status message!'}} + workflow_metas, warnings = self.run_update_status(status) + self.assertEqual(1, len(warnings)) + self.assertEqual('invalid_state', warnings[0].code) + self.assertEqual('Workflow \'random_fact\' can not be set to \'hide\', should be one of hidden,disabled,required,optional', + warnings[0].message) diff --git a/tests/test_decision_table_dictionary_output.py b/tests/test_decision_table_dictionary_output.py new file mode 100644 index 00000000..a516825a --- /dev/null +++ b/tests/test_decision_table_dictionary_output.py @@ -0,0 +1,15 @@ +from tests.base_test import BaseTest + + +class TestDecisionTableDictionaryOutput(BaseTest): + + def test_decision_table_dictionary_output(self): + + workflow = self.create_workflow('decision_table_dictionary_output') + workflow_api = self.get_workflow_api(workflow) + first_task = workflow_api.next_task + + result = self.complete_form(workflow, first_task, {'name': 'Mona'}) + self.assertIn('dog', result.next_task.data) + self.assertEqual('Mona', result.next_task.data['dog']['name']) + self.assertEqual('Aussie Mix', result.next_task.data['dog']['breed']) diff --git a/tests/workflow/test_workflow_infinite_loop.py b/tests/workflow/test_workflow_infinite_loop.py new file mode 100644 index 00000000..a9846084 --- /dev/null +++ b/tests/workflow/test_workflow_infinite_loop.py @@ -0,0 +1,13 @@ +from tests.base_test import BaseTest +from crc import app +import json + + +class TestWorkflowInfiniteLoop(BaseTest): + + def test_workflow_infinite_loop(self): + self.load_example_data() + spec_model = self.load_test_spec('infinite_loop') + rv = self.app.get('/v1.0/workflow-specification/%s/validate' % spec_model.id, headers=self.logged_in_headers()) + json_data = json.loads(rv.get_data(as_text=True)) + self.assertIn('There appears to be an infinite loop', json_data[0]['message'])