diff --git a/crc/api/data_store.py b/crc/api/data_store.py index 11f25685..86d76d6d 100644 --- a/crc/api/data_store.py +++ b/crc/api/data_store.py @@ -74,7 +74,6 @@ def update_datastore(id, body): def add_datastore(body): """ add a new datastore item """ - print(body) if body.get(id, None): raise ApiError('id_specified', 'You may not specify an id for a new datastore item') diff --git a/crc/scripts/enum_label.py b/crc/scripts/enum_label.py index 0feebc89..91564271 100644 --- a/crc/scripts/enum_label.py +++ b/crc/scripts/enum_label.py @@ -33,8 +33,6 @@ pet_label = enum_label(task='task_pet_form',field='pet',value='1') // might r # get the field information for the provided task_name (NOT the current task) workflow_model = db.session.query(WorkflowModel).filter(WorkflowModel.id == workflow_id).first() field = self.find_field(task_name, field_name, spiff_task.workflow) - print(field) - if field.type == Task.FIELD_TYPE_AUTO_COMPLETE: return self.lookup_label(workflow_model, task_name, field, value) elif field.has_property(Task.FIELD_PROP_SPREADSHEET_NAME): diff --git a/crc/scripts/fact_service.py b/crc/scripts/fact_service.py index 19e5cb3f..21e6da87 100644 --- a/crc/scripts/fact_service.py +++ b/crc/scripts/fact_service.py @@ -24,7 +24,6 @@ class FactService(Script): self.do_task(task, study_id, workflow_id, **kwargs) def do_task(self, task, study_id, workflow_id, **kwargs): - print(task.data) if "type" not in task.data: raise Exception("No Fact Provided.") @@ -41,6 +40,4 @@ class FactService(Script): details = "unknown fact type." #self.add_data_to_task(task, details) - - print(details) return details diff --git a/crc/services/git_service.py b/crc/services/git_service.py index 80f992bd..855b587b 100644 --- a/crc/services/git_service.py +++ b/crc/services/git_service.py @@ -66,7 +66,6 @@ class GitService(object): repo = self.setup_repo(remote_path, directory) except Exception as e: - print(e) app.logger.error(e) raise ApiError(code='unknown_exception', message=f'There was an unknown exception. Original message is: {e}') @@ -118,11 +117,11 @@ class GitService(object): try: repo.remotes.origin.pull() except GitCommandError as ce: - print(ce) + raise ApiError(code='git_command_error', + message='Error Running Git Command:' + str(ce)) else: raise ApiError(code='dirty_repo', message='You have modified or untracked files. Please fix this before attempting to pull.') - print(repo) return repo def merge_with_branch(self, branch): diff --git a/crc/services/jinja_service.py b/crc/services/jinja_service.py index 22c27780..ca92f196 100644 --- a/crc/services/jinja_service.py +++ b/crc/services/jinja_service.py @@ -31,17 +31,36 @@ Please Introduce yourself. Hi Dan, This is a jinja template too! Cool Right? """ + template_pattern = re.compile('{ ?% ?include\s*[\'\"](\w+)[\'\"]\s*?%}', re.DOTALL) @staticmethod def get_content(input_template, data): - templates = data + templates = {} + references = JinjaService.template_references(input_template) + for ref in references: + if ref in data.keys(): + templates[ref] = data[ref] + else: + raise ApiError("missing_template", f"Your documentation imports a template that doest not exist: {ref}") templates['main_template'] = input_template jinja2_env = Environment(loader=DictLoader(templates)) - # We just make a call here and let any errors percolate up to the calling method template = jinja2_env.get_template('main_template') - return template.render(**data) + try: + result = template.render(**data) + except AttributeError as ae: + if str(ae) == '\'NoneType\' object has no attribute \'splitlines\'': + raise ApiError("template_error", "Error processing template. You may have be using a wordwrap " + "with a field that has no value.") + return result + + + @staticmethod + def template_references(input_template): + """Using regex, determine what other templates are included, and return a list of those names.""" + matches = JinjaService.template_pattern.findall(input_template) + return matches # # The rest of this is for using Word documents as Jinja templates # diff --git a/crc/services/lookup_service.py b/crc/services/lookup_service.py index defe3a03..331e7e7a 100644 --- a/crc/services/lookup_service.py +++ b/crc/services/lookup_service.py @@ -84,7 +84,6 @@ class LookupService(object): # to rebuild. workflow_spec = WorkflowSpecService().get_spec(workflow.workflow_spec_id) timestamp = SpecFileService.timestamp(workflow_spec, lookup_model.file_name) - print(f"*** Comparing {timestamp} and {lookup_model.file_timestamp}") # Assures we have the same timestamp, as storage in the database might create slight variations in # the floating point values, just assure they values match to within a second. is_current = int(timestamp - lookup_model.file_timestamp) == 0 diff --git a/crc/services/update_service.py b/crc/services/update_service.py index 9a791ee7..1184ba6f 100644 --- a/crc/services/update_service.py +++ b/crc/services/update_service.py @@ -31,7 +31,6 @@ SPEC_SCHEMA = WorkflowSpecModelSchema() def remove_all_json_files(path): for json_file in pathlib.Path(path).glob('*.json'): - print("removing ", json_file) os.remove(json_file) @@ -41,7 +40,6 @@ def update_workflows_for_category(path, schemas, category_id): new_path = os.path.join(path, schema.id) if (os.path.exists(orig_path)): os.rename(orig_path, new_path) - print(new_path) update_spec(new_path, schema, category_id) diff --git a/crc/services/workflow_service.py b/crc/services/workflow_service.py index 0633f4fd..36ce4985 100755 --- a/crc/services/workflow_service.py +++ b/crc/services/workflow_service.py @@ -1,10 +1,9 @@ import copy import json -import sys -import time -import traceback import random import string +import sys +import traceback from datetime import datetime from typing import List @@ -19,7 +18,7 @@ from SpiffWorkflow.dmn.specs.BusinessRuleTask import BusinessRuleTask from SpiffWorkflow.exceptions import WorkflowTaskExecException from SpiffWorkflow.specs import CancelTask, StartTask from SpiffWorkflow.util.deep_merge import DeepMerge -from SpiffWorkflow.util.metrics import timeit, firsttime, sincetime +from sentry_sdk import capture_message, push_scope from sqlalchemy.exc import InvalidRequestError from crc import db, app, session @@ -32,7 +31,6 @@ from crc.models.task_event import TaskEventModel from crc.models.user import UserModel from crc.models.workflow import WorkflowModel, WorkflowStatus from crc.services.data_store_service import DataStoreBase - from crc.services.document_service import DocumentService from crc.services.jinja_service import JinjaService from crc.services.lookup_service import LookupService @@ -42,8 +40,6 @@ from crc.services.user_service import UserService from crc.services.workflow_processor import WorkflowProcessor from crc.services.workflow_spec_service import WorkflowSpecService -from sentry_sdk import capture_message, push_scope - class WorkflowService(object): TASK_ACTION_COMPLETE = "COMPLETE" @@ -203,12 +199,19 @@ class WorkflowService(object): 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', + if hasattr(task_api, 'form') and task_api.form is not None: + if 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) + WorkflowService.populate_form_with_random_data(task, task_api, required_only) + if not WorkflowService.validate_form(task, task_api): + # In the process of completing the form, it is possible for fields to become required + # based on later fields. If the form has incomplete, but required fields (validate_form) + # then try to populate the form again, with this new information. + WorkflowService.populate_form_with_random_data(task, task_api, required_only) + processor.complete_task(task) if test_until == task.task_spec.name: raise ApiError.from_task( @@ -238,6 +241,26 @@ class WorkflowService(object): WorkflowService.delete_test_data(workflow_model) return processor.bpmn_workflow.last_task.data + @staticmethod + def validate_form(task, task_api): + for field in task_api.form.fields: + if WorkflowService.is_required_field(field, task): + if not field.id in task.data or task.data[field.id] is None: + return False + return True + + @staticmethod + def is_required_field(field, task): + # Get Required State + is_required = False + if (field.has_validation(Task.FIELD_CONSTRAINT_REQUIRED) and + field.get_validation(Task.FIELD_CONSTRAINT_REQUIRED)): + is_required = True + if (field.has_property(Task.FIELD_PROP_REQUIRED_EXPRESSION) and + WorkflowService.evaluate_property(Task.FIELD_PROP_REQUIRED_EXPRESSION, field, task)): + is_required = True + return is_required + @staticmethod def populate_form_with_random_data(task, task_api, required_only): """populates a task with random data - useful for testing a spec.""" @@ -256,6 +279,8 @@ class WorkflowService(object): form_data[field.id] = None for field in task_api.form.fields: + is_required = WorkflowService.is_required_field(field, task) + # Assure we have a field type if field.type is None: raise ApiError(code='invalid_form_data', @@ -284,19 +309,13 @@ class WorkflowService(object): task=task) # If a field is hidden and required, it must have a default value - if field.has_property(Task.FIELD_PROP_HIDE_EXPRESSION) and field.has_validation( - Task.FIELD_CONSTRAINT_REQUIRED): - if field.default_value is None: - raise ApiError(code='hidden and required field missing default', - message=f'Field "{field.id}" is required but can be hidden. It must have a default value.', - task_id='task.id', - task_name=task.get_name()) - - # If the field is hidden and not required, it should not produce a value. - if field.has_property(Task.FIELD_PROP_HIDE_EXPRESSION) and not field.has_validation( - Task.FIELD_CONSTRAINT_REQUIRED): - if WorkflowService.evaluate_property(Task.FIELD_PROP_HIDE_EXPRESSION, field, task): - continue + # if field.has_property(Task.FIELD_PROP_HIDE_EXPRESSION) and field.has_validation( + # Task.FIELD_CONSTRAINT_REQUIRED): + # if field.default_value is None: + # raise ApiError(code='hidden and required field missing default', + # message=f'Field "{field.id}" is required but can be hidden. It must have a def1ault value.', + # task_id='task.id', + # task_name=task.get_name()) # If we have a default_value, try to set the default if field.default_value: @@ -306,21 +325,21 @@ class WorkflowService(object): raise ApiError.from_task("bad default value", f'The default value "{field.default_value}" in field {field.id} ' f'could not be understood or evaluated. ', task=task) - if not field.has_property(Task.FIELD_PROP_REPEAT): + # If we have a good default value, and we aren't dealing with a repeat, we can stop here. + if form_data[field.id] is not None and not field.has_property(Task.FIELD_PROP_REPEAT): continue else: form_data[field.id] = None + # If the field is hidden we can leave it as none. + if field.has_property(Task.FIELD_PROP_HIDE_EXPRESSION): + if WorkflowService.evaluate_property(Task.FIELD_PROP_HIDE_EXPRESSION, field, task): + continue # If we are only populating required fields, and this isn't required. stop here. if required_only: - if (not field.has_validation(Task.FIELD_CONSTRAINT_REQUIRED) or - field.get_validation(Task.FIELD_CONSTRAINT_REQUIRED).lower().strip() != "true"): + if not is_required: continue # Don't include any fields that aren't specifically marked as required. - if field.has_property(Task.FIELD_PROP_REQUIRED_EXPRESSION): - result = WorkflowService.evaluate_property(Task.FIELD_PROP_REQUIRED_EXPRESSION, field, task) - if not result and required_only: - continue # Don't complete fields that are not required. # If it is read only, stop here. if field.has_property("read_only") and field.get_property( @@ -1083,14 +1102,6 @@ class WorkflowService(object): db.session.commit() return workflow_model - @staticmethod - def get_standalone_workflow_specs(): - return spec_service.standalone.values() - - @staticmethod - def get_library_workflow_specs(): - return spec_service.libraries.values() - @staticmethod def delete_workflow_spec_task_events(spec_id): session.query(TaskEventModel).filter(TaskEventModel.workflow_spec_id == spec_id).delete() diff --git a/crc/static/jinja_extensions.py b/crc/static/jinja_extensions.py deleted file mode 100644 index fcbc7b55..00000000 --- a/crc/static/jinja_extensions.py +++ /dev/null @@ -1,6 +0,0 @@ -from crc.api.file import get_document_directory - - -def render_files(study_id,irb_codes): - files = get_document_directory(study_id) - print(files) \ No newline at end of file diff --git a/tests/data/required_expressions/required_expressions.bpmn b/tests/data/required_expressions/required_expressions.bpmn new file mode 100644 index 00000000..a4e38ea7 --- /dev/null +++ b/tests/data/required_expressions/required_expressions.bpmn @@ -0,0 +1,83 @@ + + + + + SequenceFlow_0lvudp8 + + + + Flow_0payrur + + + + + + + + + + + + + + + + + + + + + + + + SequenceFlow_0lvudp8 + SequenceFlow_02vev7n + + + + SequenceFlow_02vev7n + Flow_0payrur + # By directly referencing the variables +# we can assure that whatever happens in +# validation, we won't have an error. +if boolean_field: + result = required_if_true +else: + result = required_if_false + +# Note that hidden fields with a default +# value should always exist. +if not always_set == "always": + should_never_get_here + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/test_jinja_service.py b/tests/test_jinja_service.py index 54fa8d7e..eefb70fd 100644 --- a/tests/test_jinja_service.py +++ b/tests/test_jinja_service.py @@ -76,7 +76,22 @@ class TestJinjaService(BaseTest): self.assertEquals("Word Document creation error : unexpected '%'", ae.exception.message) self.assertEquals(14, ae.exception.line_number) + def test_find_template_references(self): + test_string = """ + { % include 'template_1' %} + { % include + 'template_2' %} + """ + self.assertEqual(['template_1', 'template_2'], JinjaService().template_references(test_string)) + + def test_better_error_message_for_wordwrap(self): + data = {"my_val": None} + my_tempate = "{{my_val | wordwrap(70)}}" + with self.assertRaises(ApiError) as e: + result = JinjaService().get_content(my_tempate, data) + self.assertEqual(e.exception.message, 'Error processing template. You may have be using a wordwrap ' + 'with a field that has no value.') def test_jinja_service_properties(self): pass diff --git a/tests/workflow/test_workflow_hidden_required_field.py b/tests/workflow/test_workflow_hidden_required_field.py index 90e5c052..d733dbee 100644 --- a/tests/workflow/test_workflow_hidden_required_field.py +++ b/tests/workflow/test_workflow_hidden_required_field.py @@ -1,9 +1,12 @@ +from unittest import skip + from tests.base_test import BaseTest import json class TestWorkflowHiddenRequiredField(BaseTest): + @skip("Maybe we don't need to require a default for required hidden fields after all.") def test_require_default(self): # We have a field that can be hidden and required. # Validation should fail if we don't have a default value. diff --git a/tests/workflow/test_workflow_spec_validation_api.py b/tests/workflow/test_workflow_spec_validation_api.py index 36224d17..0edf50ec 100644 --- a/tests/workflow/test_workflow_spec_validation_api.py +++ b/tests/workflow/test_workflow_spec_validation_api.py @@ -173,3 +173,17 @@ class TestWorkflowSpecValidation(BaseTest): self.create_reference_document() errors = self.validate_workflow("date_value_expression") self.assertEqual(0, len(errors)) + + def test_fields_required_based_on_later_fields_correctly_populates(self): + """Say you have a form, where the first field is required only if the + SECOND field is checked true. This assures such a case will validate and + that the variables that should exist (because they are required) do exist. + + As a bonus test, we also assert that a default field is always present + regardless of it's hidden status. + """ + self.load_test_spec('empty_workflow', master_spec=True) + self.create_reference_document() + errors = self.validate_workflow("required_expressions") + self.assertEqual(0, len(errors)) +