From f883eb2bbe1e477203ca90f818cf508073aaa4ba Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Thu, 4 Mar 2021 10:42:13 -0500 Subject: [PATCH] We now include field.id in the ApiError message for hidden and default validation. (The field must have a default value or value expression) While working on this ticket I found a logic error in the test. I expected both default_value and value_expression. It turns out the default_value is always there, it's just null if we don't set it. Modified the existing test for hidden and required, and added tests to make sure my passing logic is now correct. --- crc/services/workflow_service.py | 5 +- .../hidden_required_field.bpmn | 6 +- .../hidden_required_field_pass.bpmn | 93 ++++++++++++++++++ ...hidden_required_field_pass_expression.bpmn | 96 +++++++++++++++++++ .../test_workflow_hidden_required_field.py | 11 +++ 5 files changed, 207 insertions(+), 4 deletions(-) create mode 100644 tests/data/hidden_required_field_pass/hidden_required_field_pass.bpmn create mode 100644 tests/data/hidden_required_field_pass_expression/hidden_required_field_pass_expression.bpmn diff --git a/crc/services/workflow_service.py b/crc/services/workflow_service.py index acceee10..3287aa7e 100644 --- a/crc/services/workflow_service.py +++ b/crc/services/workflow_service.py @@ -156,9 +156,10 @@ class WorkflowService(object): # If a field is hidden and required, it must have a default value or value_expression if field.has_property(Task.FIELD_PROP_HIDE_EXPRESSION) and field.has_validation(Task.FIELD_CONSTRAINT_REQUIRED): - if not field.has_property(Task.FIELD_PROP_VALUE_EXPRESSION) or not (hasattr(field, 'default_value')): + if not field.has_property(Task.FIELD_PROP_VALUE_EXPRESSION) and \ + (not (hasattr(field, 'default_value')) or field.default_value is None): raise ApiError(code='hidden and required field missing default', - message='Fields that are required but can be hidden must have either a default value or a value_expression', + message=f'Field "{field.id}" is required but can be hidden. It must have either a default value or a value_expression', task_id='task.id', task_name=task.get_name()) diff --git a/tests/data/hidden_required_field/hidden_required_field.bpmn b/tests/data/hidden_required_field/hidden_required_field.bpmn index 5e4ace06..6c1eafcd 100644 --- a/tests/data/hidden_required_field/hidden_required_field.bpmn +++ b/tests/data/hidden_required_field/hidden_required_field.bpmn @@ -1,5 +1,5 @@ - + Flow_0zt7wv5 @@ -32,7 +32,9 @@ if not 'hide_yes_no' in globals(): - <H1>Good Bye{% if name %} {{ name }}{% endif %}</H1> + <H1>Good Bye{% if name %} {{ name }}{% endif %}</H1> +<div><span>Color is {{ color }}</span></div> + Flow_1qkjkbh Flow_1udbzd6 diff --git a/tests/data/hidden_required_field_pass/hidden_required_field_pass.bpmn b/tests/data/hidden_required_field_pass/hidden_required_field_pass.bpmn new file mode 100644 index 00000000..cdf75fa7 --- /dev/null +++ b/tests/data/hidden_required_field_pass/hidden_required_field_pass.bpmn @@ -0,0 +1,93 @@ + + + + + Flow_0zt7wv5 + + + + + + + + + + + + + + + + Flow_0fb4w15 + Flow_0c2rym0 + + + + Flow_0cm6imh + Flow_0fb4w15 + if not 'require_yes_no' in globals(): + require_yes_no = True +if not 'hide_yes_no' in globals(): + hide_yes_no = True + + + + <H1>Good Bye{% if name %} {{ name }}{% endif %}</H1> + + Flow_0c2rym0 + Flow_1udbzd6 + + + Flow_1udbzd6 + + + + <H1>Hello</H1> + Flow_0zt7wv5 + Flow_0cm6imh + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/hidden_required_field_pass_expression/hidden_required_field_pass_expression.bpmn b/tests/data/hidden_required_field_pass_expression/hidden_required_field_pass_expression.bpmn new file mode 100644 index 00000000..52140d1c --- /dev/null +++ b/tests/data/hidden_required_field_pass_expression/hidden_required_field_pass_expression.bpmn @@ -0,0 +1,96 @@ + + + + + Flow_0zt7wv5 + + + + + + + + + + + + + + + + + Flow_0fb4w15 + Flow_0c2rym0 + + + + Flow_0cm6imh + Flow_0fb4w15 + if not 'require_yes_no' in globals(): + require_yes_no = True +if not 'hide_yes_no' in globals(): + hide_yes_no = True +if not 'value_expression_value' in globals(): + value_expression_value = 'World' + + + + <H1>Good Bye{% if name %} {{ name }}{% endif %}</H1> + + Flow_0c2rym0 + Flow_1udbzd6 + + + Flow_1udbzd6 + + + + <H1>Hello</H1> + Flow_0zt7wv5 + Flow_0cm6imh + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/workflow/test_workflow_hidden_required_field.py b/tests/workflow/test_workflow_hidden_required_field.py index 9c635992..b77dbb24 100644 --- a/tests/workflow/test_workflow_hidden_required_field.py +++ b/tests/workflow/test_workflow_hidden_required_field.py @@ -14,6 +14,17 @@ class TestWorkflowHiddenRequiredField(BaseTest): self.assertEqual(json_data[0]['code'], 'hidden and required field missing default') self.assertIn('task_id', json_data[0]) self.assertIn('task_name', json_data[0]) + self.assertIn('Field "name" is required but can be hidden', json_data[0]['message']) + + def test_require_default_pass(self): + spec_model = self.load_test_spec('hidden_required_field_pass') + rv = self.app.get('/v1.0/workflow-specification/%s/validate' % spec_model.id, headers=self.logged_in_headers()) + self.assertEqual(0, len(rv.json)) + + def test_require_default_pass_expression(self): + spec_model = self.load_test_spec('hidden_required_field_pass_expression') + rv = self.app.get('/v1.0/workflow-specification/%s/validate' % spec_model.id, headers=self.logged_in_headers()) + self.assertEqual(0, len(rv.json)) def test_default_used(self): # If a field is hidden and required, make sure we use the default value