Fill out repeating sections during validation process.

Also, when returning error messages, attempt to include the task data for the task that caused the error.
Also, when attempting to delete any file, respond with an API error explaining the issue, and log the details.

crc/api/common.py

@@ -3,7 +3,7 @@ from crc import ma, app
 
 class ApiError(Exception):
     def __init__(self, code, message, status_code=400,
-                 file_name="", task_id="", task_name="", tag=""):
+                 file_name="", task_id="", task_name="", tag="", task_data = {}):
         self.status_code = status_code
         self.code = code  # a short consistent string describing the error.
         self.message = message  # A detailed message that provides more information.
@@ -11,6 +11,7 @@ class ApiError(Exception):
         self.task_name = task_name or ""  # OPTIONAL: The name of the task in the BPMN Diagram.
         self.file_name = file_name or ""  # OPTIONAL: The file that caused the error.
         self.tag = tag or ""  # OPTIONAL: The XML Tag that caused the issue.
+        self.task_data = task_data or ""  # OPTIONAL: A snapshot of data connected to the task when error ocurred.
         Exception.__init__(self, self.message)
 
     @classmethod
@@ -20,6 +21,7 @@ class ApiError(Exception):
         instance.task_id = task.task_spec.name or ""
         instance.task_name = task.task_spec.description or ""
         instance.file_name = task.workflow.spec.file or ""
+        instance.task_data = task.data
         return instance
 
     @classmethod
@@ -35,7 +37,8 @@ class ApiError(Exception):
 
 class ApiErrorSchema(ma.Schema):
     class Meta:
-        fields = ("code", "message", "workflow_name", "file_name", "task_name", "task_id")
+        fields = ("code", "message", "workflow_name", "file_name", "task_name", "task_id",
+                  "task_data")
 
 
 @app.errorhandler(ApiError)

crc/models/api_models.py

@@ -31,6 +31,7 @@ class NavigationItem(object):
 
 class Task(object):
 
+    PROP_OPTIONS_REPEAT = "repeat"
     PROP_OPTIONS_FILE = "spreadsheet.name"
     PROP_OPTIONS_VALUE_COLUMN = "spreadsheet.value.column"
     PROP_OPTIONS_LABEL_COL = "spreadsheet.label.column"

crc/services/file_service.py

@@ -5,11 +5,13 @@ from datetime import datetime
 from uuid import UUID
 from xml.etree import ElementTree
 
+import flask
 from SpiffWorkflow.bpmn.parser.ValidationException import ValidationException
 from pandas import ExcelFile
 from sqlalchemy import desc
+from sqlalchemy.exc import IntegrityError
 
-from crc import session
+from crc import session, app
 from crc.api.common import ApiError
 from crc.models.file import FileType, FileDataModel, FileModel, LookupFileModel, LookupDataModel
 from crc.models.workflow import WorkflowSpecModel, WorkflowModel, WorkflowSpecDependencyFile
@@ -295,6 +297,7 @@ class FileService(object):
 
     @staticmethod
     def delete_file(file_id):
+        try:
             data_models = session.query(FileDataModel).filter_by(file_model_id=file_id).all()
             for dm in data_models:
                 lookup_files = session.query(LookupFileModel).filter_by(file_data_model_id=dm.id).all()
@@ -304,3 +307,7 @@ class FileService(object):
             session.query(FileDataModel).filter_by(file_model_id=file_id).delete()
             session.query(FileModel).filter_by(id=file_id).delete()
             session.commit()
+        except IntegrityError as ie:
+            app.logger.error("Failed to delete file: %i, due to %s" % (file_id, str(ie)))
+            raise ApiError('file_integrity_error', "You are attempting to delete a file that is "
+                                                   "required by other records in the system.")

crc/services/study_service.py

@@ -4,6 +4,7 @@ from typing import List
 
 import requests
 from SpiffWorkflow import WorkflowException
+from SpiffWorkflow.exceptions import WorkflowTaskExecException
 from ldap3.core.exceptions import LDAPSocketOpenError
 
 from crc import db, session, app
@@ -309,6 +310,8 @@ class StudyService(object):
         for workflow_spec in new_specs:
             try:
                 StudyService._create_workflow_model(study_model, workflow_spec)
+            except WorkflowTaskExecException as wtee:
+                errors.append(ApiError.from_task("workflow_execution_exception", str(wtee), wtee.task))
             except WorkflowException as we:
                 errors.append(ApiError.from_task_spec("workflow_execution_exception", str(we), we.sender))
         return errors

crc/services/workflow_service.py

@@ -7,8 +7,9 @@ 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
-from SpiffWorkflow.bpmn.workflow import BpmnWorkflow
+from SpiffWorkflow.camunda.specs.UserTask import EnumFormField
 from SpiffWorkflow.dmn.specs.BusinessRuleTask import BusinessRuleTask
+from SpiffWorkflow.exceptions import WorkflowTaskExecException
 from SpiffWorkflow.specs import CancelTask, StartTask
 from flask import g
 from jinja2 import Template
@@ -17,7 +18,6 @@ from crc import db, app
 from crc.api.common import ApiError
 from crc.models.api_models import Task, MultiInstanceType
 from crc.models.file import LookupDataModel
-from crc.models.protocol_builder import ProtocolBuilderStatus
 from crc.models.stats import TaskEventModel
 from crc.models.study import StudyModel
 from crc.models.user import UserModel
@@ -39,7 +39,9 @@ class WorkflowService(object):
      the workflow Processor should be hidden behind this service.
      This will help maintain a structure that avoids circular dependencies.
      But for now, this contains tools for converting spiff-workflow models into our
-     own API models with additional information and capabilities."""
+     own API models with additional information and capabilities and 
+     handles the testing of a workflow specification by completing it with
+     random selections, attempting to mimic a front end as much as possible. """
 
     @staticmethod
     def make_test_workflow(spec_id):
@@ -61,17 +63,25 @@ class WorkflowService(object):
         for study in db.session.query(StudyModel).filter(StudyModel.user_uid=="test"):
             StudyService.delete_study(study.id)
             db.session.commit()
-        db.session.query(UserModel).filter_by(uid="test").delete()
+
+        user = db.session.query(UserModel).filter_by(uid="test").first()
+        if user:
+            db.session.delete(user)
 
     @staticmethod
     def test_spec(spec_id):
-        """Runs a spec through it's paces to see if it results in any errors.  Not fool-proof, but a good
+        """Runs a spec through it's paces to see if it results in any errors.
-        sanity check."""
+          Not fool-proof, but a good sanity check.  Returns the final data
+          output form the last task if successful. """
 
         workflow_model = WorkflowService.make_test_workflow(spec_id)
 
         try:
             processor = WorkflowProcessor(workflow_model, validate_only=True)
+        except WorkflowTaskExecException as wtee:
+            WorkflowService.delete_test_data()
+            raise ApiError.from_task("workflow_execution_exception", str(wtee),
+                                          wtee.task)
         except WorkflowException as we:
             WorkflowService.delete_test_data()
             raise ApiError.from_task_spec("workflow_execution_exception", str(we),
@@ -87,11 +97,17 @@ class WorkflowService(object):
                         add_docs_and_forms=True)  # Assure we try to process the documenation, and raise those errors.
                     WorkflowService.populate_form_with_random_data(task, task_api)
                     task.complete()
+            except WorkflowTaskExecException as wtee:
+                WorkflowService.delete_test_data()
+                raise ApiError.from_task("workflow_execution_exception", str(wtee),
+                                         wtee.task)
             except WorkflowException as we:
                 WorkflowService.delete_test_data()
                 raise ApiError.from_task_spec("workflow_execution_exception", str(we),
                                               we.sender)
+
         WorkflowService.delete_test_data()
+        return processor.bpmn_workflow.last_task.data
 
     @staticmethod
     def populate_form_with_random_data(task, task_api):
@@ -101,22 +117,35 @@ class WorkflowService(object):
 
         form_data = {}
         for field in task_api.form.fields:
+            if field.has_property(Task.PROP_OPTIONS_REPEAT):
+                group = field.get_property(Task.PROP_OPTIONS_REPEAT)
+                if group not in form_data:
+                    form_data[group] = [{},{},{}]
+                for i in range(3):
+                    form_data[group][i][field.id] = WorkflowService.get_random_data_for_field(field, task)
+            else:
+                form_data[field.id] = WorkflowService.get_random_data_for_field(field, task)
+        if task.data is None:
+            task.data = {}
+        task.data.update(form_data)
+
+    @staticmethod
+    def get_random_data_for_field(field, task):
         if field.type == "enum":
             if len(field.options) > 0:
                 random_choice = random.choice(field.options)
                 if isinstance(random_choice, dict):
-                        form_data[field.id] = random.choice(field.options)['id']
+                    return random.choice(field.options)['id']
                 else:
                     # fixme: why it is sometimes an EnumFormFieldOption, and other times not?
-                        form_data[field.id] = random_choice.id ## Assume it is an EnumFormFieldOption
+                    return random_choice.id  ## Assume it is an EnumFormFieldOption
             else:
                 raise ApiError.from_task("invalid_enum", "You specified an enumeration field (%s),"
-                                                             " with no options" % field.id,
+                                                         " with no options" % field.id, task)
-                                             task)
         elif field.type == "autocomplete":
             lookup_model = LookupService.get_lookup_model(task, field)
-                if field.has_property(Task.PROP_LDAP_LOOKUP):
+            if field.has_property(Task.PROP_LDAP_LOOKUP):  # All ldap records get the same person.
-                    form_data[field.id] = {
+                return {
                         "label": "dhf8r",
                         "value": "Dan Funk",
                         "data": {
@@ -126,8 +155,7 @@ class WorkflowService(object):
                             "email_address": "dhf8r@virginia.edu",
                             "department": "Depertment of Psychocosmographictology",
                             "affiliation": "Rousabout",
-                            "sponsor_type": "Staff"
+                            "sponsor_type": "Staff"}
-                        }
                         }
             elif lookup_model:
                 data = db.session.query(LookupDataModel).filter(
@@ -135,23 +163,22 @@ class WorkflowService(object):
                 options = []
                 for d in data:
                     options.append({"id": d.value, "name": d.label})
-                    form_data[field.id] = random.choice(options)
+                return random.choice(options)
             else:
                 raise ApiError.from_task("invalid_autocomplete", "The settings for this auto complete field "
                                                                  "are incorrect: %s " % field.id, task)
         elif field.type == "long":
-                form_data[field.id] = random.randint(1, 1000)
+            return random.randint(1, 1000)
         elif field.type == 'boolean':
-                form_data[field.id] = random.choice([True, False])
+            return random.choice([True, False])
         elif field.type == 'file':
-                form_data[field.id] = random.randint(1, 100)
+            # fixme: produce some something sensible for files.
+            return random.randint(1, 100)
+            # fixme: produce some something sensible for files.
         elif field.type == 'files':
-                form_data[field.id] = random.randrange(1, 100)
+            return random.randrange(1, 100)
         else:
-                form_data[field.id] = WorkflowService._random_string()
+            return WorkflowService._random_string()
-        if task.data is None:
-            task.data = {}
-        task.data.update(form_data)
 
     def __get_options(self):
         pass

tests/data/repeat_form/repeat_form.bpmn

@@ -0,0 +1,47 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<bpmn:definitions xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" xmlns:camunda="http://camunda.org/schema/1.0/bpmn" xmlns:di="http://www.omg.org/spec/DD/20100524/DI" id="Definitions_1v1rp1q" targetNamespace="http://bpmn.io/schema/bpmn" exporter="Camunda Modeler" exporterVersion="3.7.3">
+  <bpmn:process id="Repeat" isExecutable="true">
+    <bpmn:startEvent id="StartEvent_1">
+      <bpmn:outgoing>SequenceFlow_0lvudp8</bpmn:outgoing>
+    </bpmn:startEvent>
+    <bpmn:sequenceFlow id="SequenceFlow_0lvudp8" sourceRef="StartEvent_1" targetRef="Task_14svgcu" />
+    <bpmn:endEvent id="EndEvent_0q4qzl9">
+      <bpmn:incoming>SequenceFlow_02vev7n</bpmn:incoming>
+    </bpmn:endEvent>
+    <bpmn:sequenceFlow id="SequenceFlow_02vev7n" sourceRef="Task_14svgcu" targetRef="EndEvent_0q4qzl9" />
+    <bpmn:userTask id="Task_14svgcu" name="Repeating Form" camunda:formKey="RepeatForm">
+      <bpmn:extensionElements>
+        <camunda:formData>
+          <camunda:formField id="name" label="Add a cat name" type="string" defaultValue="couger buttons">
+            <camunda:properties>
+              <camunda:property id="repeat" value="cats" />
+            </camunda:properties>
+          </camunda:formField>
+        </camunda:formData>
+      </bpmn:extensionElements>
+      <bpmn:incoming>SequenceFlow_0lvudp8</bpmn:incoming>
+      <bpmn:outgoing>SequenceFlow_02vev7n</bpmn:outgoing>
+    </bpmn:userTask>
+  </bpmn:process>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
+    <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Repeat">
+      <bpmndi:BPMNEdge id="SequenceFlow_02vev7n_di" bpmnElement="SequenceFlow_02vev7n">
+        <di:waypoint x="370" y="117" />
+        <di:waypoint x="432" y="117" />
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNEdge id="SequenceFlow_0lvudp8_di" bpmnElement="SequenceFlow_0lvudp8">
+        <di:waypoint x="215" y="117" />
+        <di:waypoint x="270" y="117" />
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNShape id="_BPMNShape_StartEvent_2" bpmnElement="StartEvent_1">
+        <dc:Bounds x="179" y="99" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="EndEvent_0q4qzl9_di" bpmnElement="EndEvent_0q4qzl9">
+        <dc:Bounds x="432" y="99" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="UserTask_18ly1yq_di" bpmnElement="Task_14svgcu">
+        <dc:Bounds x="270" y="77" width="100" height="80" />
+      </bpmndi:BPMNShape>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+</bpmn:definitions>

tests/test_workflow_spec_validation_api.py

@@ -3,17 +3,16 @@ from unittest.mock import patch
 
 from tests.base_test import BaseTest
 
-from crc.services.protocol_builder import ProtocolBuilderService
 from crc import session, app
 from crc.api.common import ApiErrorSchema
 from crc.models.protocol_builder import ProtocolBuilderStudySchema
 from crc.models.workflow import WorkflowSpecModel
+from crc.services.workflow_service import WorkflowService
 
 
 class TestWorkflowSpecValidation(BaseTest):
 
     def validate_workflow(self, workflow_name):
-        self.load_example_data()
         spec_model = self.load_test_spec(workflow_name)
         rv = self.app.get('/v1.0/workflow-specification/%s/validate' % spec_model.id, headers=self.logged_in_headers())
         self.assert_success(rv)
@@ -22,6 +21,7 @@ class TestWorkflowSpecValidation(BaseTest):
 
     def test_successful_validation_of_test_workflows(self):
         app.config['PB_ENABLED'] = False  # Assure this is disabled.
+        self.load_example_data()
         self.assertEqual(0, len(self.validate_workflow("parallel_tasks")))
         self.assertEqual(0, len(self.validate_workflow("decision_table")))
         self.assertEqual(0, len(self.validate_workflow("docx")))
@@ -60,6 +60,7 @@ class TestWorkflowSpecValidation(BaseTest):
         self.assertEqual(0, len(errors), json.dumps(errors))
 
     def test_invalid_expression(self):
+        self.load_example_data()
         errors = self.validate_workflow("invalid_expression")
         self.assertEqual(1, len(errors))
         self.assertEqual("workflow_execution_exception", errors[0]['code'])
@@ -68,8 +69,11 @@ class TestWorkflowSpecValidation(BaseTest):
         self.assertEqual("invalid_expression.bpmn", errors[0]['file_name'])
         self.assertEqual('ExclusiveGateway_003amsm: Error evaluating expression \'this_value_does_not_exist==true\', '
                          'name \'this_value_does_not_exist\' is not defined', errors[0]["message"])
+        self.assertIsNotNone(errors[0]['task_data'])
+        self.assertIn("has_bananas", errors[0]['task_data'])
 
     def test_validation_error(self):
+        self.load_example_data()
         errors = self.validate_workflow("invalid_spec")
         self.assertEqual(1, len(errors))
         self.assertEqual("workflow_validation_error", errors[0]['code'])
@@ -77,6 +81,7 @@ class TestWorkflowSpecValidation(BaseTest):
         self.assertEqual("invalid_spec.bpmn", errors[0]['file_name'])
 
     def test_invalid_script(self):
+        self.load_example_data()
         errors = self.validate_workflow("invalid_script")
         self.assertEqual(1, len(errors))
         self.assertEqual("workflow_execution_exception", errors[0]['code'])
@@ -84,3 +89,10 @@ class TestWorkflowSpecValidation(BaseTest):
         self.assertEqual("Invalid_Script_Task", errors[0]['task_id'])
         self.assertEqual("An Invalid Script Reference", errors[0]['task_name'])
         self.assertEqual("invalid_script.bpmn", errors[0]['file_name'])
+
+    def test_repeating_sections_correctly_populated(self):
+        self.load_example_data()
+        spec_model = self.load_test_spec('repeat_form')
+        final_data = WorkflowService.test_spec(spec_model.id)
+        self.assertIsNotNone(final_data)
+        self.assertIn('cats', final_data)