From 00d9276483a4fd4c1cc3bad3c5ba96a6f10c7018 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Fri, 22 Jan 2021 10:00:28 -0500 Subject: [PATCH 1/4] In spiff_task_to_api_task, when looping through fields, if the field doesn't have a value, we now try to set it with the default value In test_spec, we now check for FIELD_PROP_VALUE_EXPRESSION when setting default values --- crc/services/workflow_service.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crc/services/workflow_service.py b/crc/services/workflow_service.py index 82a4610d..ecc3770c 100644 --- a/crc/services/workflow_service.py +++ b/crc/services/workflow_service.py @@ -153,8 +153,11 @@ class WorkflowService(object): if WorkflowService.evaluate_property(Task.FIELD_PROP_HIDE_EXPRESSION, field, task): continue + # If we set the default with value_expression + if field.has_property(Task.FIELD_PROP_VALUE_EXPRESSION): + form_data[field.id] = WorkflowService.get_default_value(field, task) # If there is a default value, set it. - if hasattr(field,'default_value') and field.default_value: + elif hasattr(field, 'default_value') and field.default_value: form_data[field.id] = WorkflowService.get_default_value(field, task) # If we are only populating required fields, and this isn't required. stop here. @@ -488,6 +491,9 @@ class WorkflowService(object): task.form = spiff_task.task_spec.form for i, field in enumerate(task.form.fields): task.form.fields[i] = WorkflowService.process_options(spiff_task, field) + # If there is a default value, set it. + if field.id not in task.data and WorkflowService.get_default_value(field, spiff_task) is not None: + task.data[field.id] = WorkflowService.get_default_value(field, spiff_task) task.documentation = WorkflowService._process_documentation(spiff_task) # All ready tasks should have a valid name, and this can be computed for From 1ee6a11d4644fdfe1e4c104847de254ee3639b7a Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Fri, 22 Jan 2021 10:01:20 -0500 Subject: [PATCH 2/4] Test and associated data files for setting enum default values from value_expression. Ticket 162 --- .../Decision_Value_Expression.dmn | 40 ++++++++ .../enum_value_expression.bpmn | 95 +++++++++++++++++++ ..._workflow_enum_default_value_expression.py | 38 ++++++++ 3 files changed, 173 insertions(+) create mode 100644 tests/data/enum_value_expression/Decision_Value_Expression.dmn create mode 100644 tests/data/enum_value_expression/enum_value_expression.bpmn create mode 100644 tests/workflow/test_workflow_enum_default_value_expression.py diff --git a/tests/data/enum_value_expression/Decision_Value_Expression.dmn b/tests/data/enum_value_expression/Decision_Value_Expression.dmn new file mode 100644 index 00000000..11c7beba --- /dev/null +++ b/tests/data/enum_value_expression/Decision_Value_Expression.dmn @@ -0,0 +1,40 @@ + + + + + + + + + + user_input + + + + + + True + + + 'black' + + + + + False + + + 'white' + + + + + + + + 'grey' + + + + + diff --git a/tests/data/enum_value_expression/enum_value_expression.bpmn b/tests/data/enum_value_expression/enum_value_expression.bpmn new file mode 100644 index 00000000..083cc65d --- /dev/null +++ b/tests/data/enum_value_expression/enum_value_expression.bpmn @@ -0,0 +1,95 @@ + + + + + Flow_02xzhf3 + + + + + + + + + + + Flow_02xzhf3 + Flow_0d46qnz + + + Flow_0d46qnz + Flow_1d7sv9v + + + <h1>Hello {{ lookup_output }}</h1> + + + + + + + + + + + + + + Flow_1d7sv9v + Flow_01x96w8 + + + + <h1>Hello</h1> +<div>You picked {{ color_select.label }}</div> + Flow_01x96w8 + Flow_05tzoiy + + + Flow_05tzoiy + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/workflow/test_workflow_enum_default_value_expression.py b/tests/workflow/test_workflow_enum_default_value_expression.py new file mode 100644 index 00000000..d7ab0d37 --- /dev/null +++ b/tests/workflow/test_workflow_enum_default_value_expression.py @@ -0,0 +1,38 @@ +from tests.base_test import BaseTest + + +class TestWorkflowEnumDefault(BaseTest): + + def test_enum_default_from_value_expression(self): + workflow = self.create_workflow('enum_value_expression') + + first_task = self.get_workflow_api(workflow).next_task + self.assertEqual('Activity_UserInput', first_task.name) + workflow_api = self.get_workflow_api(workflow) + + result = self.complete_form(workflow_api, first_task, {'user_input': True}) + self.assertIn('user_input', result.next_task.data) + self.assertEqual(True, result.next_task.data['user_input']) + self.assertIn('lookup_output', result.next_task.data) + self.assertEqual('black', result.next_task.data['lookup_output']) + + workflow_api = self.get_workflow_api(workflow) + self.assertEqual('Activity_PickColor', self.get_workflow_api(workflow_api).next_task.name) + self.assertEqual({'value': 'black', 'label': 'Black'}, workflow_api.next_task.data['color_select']) + + # + workflow = self.create_workflow('enum_value_expression') + + first_task = self.get_workflow_api(workflow).next_task + self.assertEqual('Activity_UserInput', first_task.name) + workflow_api = self.get_workflow_api(workflow) + + result = self.complete_form(workflow_api, first_task, {'user_input': False}) + self.assertIn('user_input', result.next_task.data) + self.assertEqual(False, result.next_task.data['user_input']) + self.assertIn('lookup_output', result.next_task.data) + self.assertEqual('white', result.next_task.data['lookup_output']) + + workflow_api = self.get_workflow_api(workflow) + self.assertEqual('Activity_PickColor', self.get_workflow_api(workflow_api).next_task.name) + self.assertEqual({'value': 'white', 'label': 'White'}, workflow_api.next_task.data['color_select']) From 81e55b6055020433da15e1fcb868c90247fb14d5 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Wed, 27 Jan 2021 10:21:13 -0500 Subject: [PATCH 3/4] Cleaning up my smell. A task should only have default_value *or* value_expresion, not both. --- crc/services/workflow_service.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/crc/services/workflow_service.py b/crc/services/workflow_service.py index ecc3770c..96234a2b 100644 --- a/crc/services/workflow_service.py +++ b/crc/services/workflow_service.py @@ -153,11 +153,12 @@ class WorkflowService(object): if WorkflowService.evaluate_property(Task.FIELD_PROP_HIDE_EXPRESSION, field, task): continue - # If we set the default with value_expression - if field.has_property(Task.FIELD_PROP_VALUE_EXPRESSION): - form_data[field.id] = WorkflowService.get_default_value(field, task) - # If there is a default value, set it. - elif hasattr(field, 'default_value') and field.default_value: + # A task should only have default_value **or** value expression, not both. + if field.has_property(Task.FIELD_PROP_VALUE_EXPRESSION) and (hasattr(field, 'default_value') and field.default_value): + raise ApiError(code='default value and value_expression', + message='This task has both a default_value and value_expression. Please fix this to only have one or the other.') + # If we have a default_value or value_expression, try to set the default + if field.has_property(Task.FIELD_PROP_VALUE_EXPRESSION) or (hasattr(field, 'default_value') and field.default_value): form_data[field.id] = WorkflowService.get_default_value(field, task) # If we are only populating required fields, and this isn't required. stop here. From 420ef0b2b0266d14b326059bb2b6070b491ad899 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Wed, 27 Jan 2021 10:22:43 -0500 Subject: [PATCH 4/4] Test and data for cleaning up my smell. We now test explicitly whether a task has both a default_value and value_expression. If so, we raise an error. --- .../Decision_Value_Expression.dmn | 40 ++++++++ .../enum_value_expression_fail.bpmn | 95 +++++++++++++++++++ ..._workflow_enum_default_value_expression.py | 8 ++ 3 files changed, 143 insertions(+) create mode 100644 tests/data/enum_value_expression_fail/Decision_Value_Expression.dmn create mode 100644 tests/data/enum_value_expression_fail/enum_value_expression_fail.bpmn diff --git a/tests/data/enum_value_expression_fail/Decision_Value_Expression.dmn b/tests/data/enum_value_expression_fail/Decision_Value_Expression.dmn new file mode 100644 index 00000000..11c7beba --- /dev/null +++ b/tests/data/enum_value_expression_fail/Decision_Value_Expression.dmn @@ -0,0 +1,40 @@ + + + + + + + + + + user_input + + + + + + True + + + 'black' + + + + + False + + + 'white' + + + + + + + + 'grey' + + + + + diff --git a/tests/data/enum_value_expression_fail/enum_value_expression_fail.bpmn b/tests/data/enum_value_expression_fail/enum_value_expression_fail.bpmn new file mode 100644 index 00000000..ddbfc613 --- /dev/null +++ b/tests/data/enum_value_expression_fail/enum_value_expression_fail.bpmn @@ -0,0 +1,95 @@ + + + + + Flow_02xzhf3 + + + + + + + + + + + Flow_02xzhf3 + Flow_0d46qnz + + + Flow_0d46qnz + Flow_1d7sv9v + + + <h1>Hello {{ lookup_output }}</h1> + + + + + + + + + + + + + + Flow_1d7sv9v + Flow_01x96w8 + + + + <h1>Hello</h1> +<div>You picked {{ color_select.label }}</div> + Flow_01x96w8 + Flow_05tzoiy + + + Flow_05tzoiy + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/workflow/test_workflow_enum_default_value_expression.py b/tests/workflow/test_workflow_enum_default_value_expression.py index d7ab0d37..00c2abf6 100644 --- a/tests/workflow/test_workflow_enum_default_value_expression.py +++ b/tests/workflow/test_workflow_enum_default_value_expression.py @@ -1,4 +1,5 @@ from tests.base_test import BaseTest +import json class TestWorkflowEnumDefault(BaseTest): @@ -36,3 +37,10 @@ class TestWorkflowEnumDefault(BaseTest): workflow_api = self.get_workflow_api(workflow) self.assertEqual('Activity_PickColor', self.get_workflow_api(workflow_api).next_task.name) self.assertEqual({'value': 'white', 'label': 'White'}, workflow_api.next_task.data['color_select']) + + def test_enum_value_expression_and_default(self): + spec_model = self.load_test_spec('enum_value_expression_fail') + 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.assertEqual(json_data[0]['code'], 'default value and value_expression')