diff --git a/.github/workflows/backend_tests.yml b/.github/workflows/backend_tests.yml index 979c3716..c9c6cbe9 100644 --- a/.github/workflows/backend_tests.yml +++ b/.github/workflows/backend_tests.yml @@ -273,10 +273,10 @@ jobs: nox --force-color --session=coverage -- xml - name: Upload coverage report - uses: codecov/codecov-action@v3.1.0 + uses: codecov/codecov-action@v3.1.3 - name: SonarCloud Scan - uses: sonarsource/sonarcloud-github-action@master + uses: sonarsource/sonarcloud-github-action@v1.8 # thought about just skipping dependabot # if: ${{ github.actor != 'dependabot[bot]' }} # but figured all pull requests seems better, since none of them will have access to sonarcloud. diff --git a/.github/workflows/docker_image_for_main_builds.yml b/.github/workflows/docker_image_for_main_builds.yml index 12d0fff6..f8a1162e 100644 --- a/.github/workflows/docker_image_for_main_builds.yml +++ b/.github/workflows/docker_image_for_main_builds.yml @@ -65,7 +65,7 @@ jobs: - name: Write app version info working-directory: spiffworkflow-frontend - run: echo $DOCKER_METADATA_OUTPUT_JSON | jq '.labels' > version_info.json + run: echo "$DOCKER_METADATA_OUTPUT_JSON" | jq '.labels' > version_info.json - name: Build and push Frontend Docker image uses: docker/build-push-action@v4.0.0 with: @@ -109,7 +109,7 @@ jobs: - name: Write app version info working-directory: spiffworkflow-backend - run: echo $DOCKER_METADATA_OUTPUT_JSON | jq '.labels' > version_info.json + run: echo "$DOCKER_METADATA_OUTPUT_JSON" | jq '.labels' > version_info.json - name: Build and push Backend Docker image uses: docker/build-push-action@v4.0.0 with: diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/human_task.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/human_task.py index bca4cb81..e66dd59b 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/human_task.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/human_task.py @@ -49,6 +49,7 @@ class HumanTaskModel(SpiffworkflowBaseDBModel): # task_id came first which is why it's a string and task_model_id is the int and foreignkey task_model_id: int = db.Column(ForeignKey(TaskModel.id), nullable=True, index=True) # type: ignore + task_model = relationship(TaskModel) task_id: str = db.Column(db.String(50)) task_name: str = db.Column(db.String(255)) task_title: str = db.Column(db.String(50)) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/models/task.py b/spiffworkflow-backend/src/spiffworkflow_backend/models/task.py index c9bf311b..9fb8d6ff 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/models/task.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/task.py @@ -3,6 +3,7 @@ import enum from dataclasses import dataclass from typing import Any from typing import Optional +from typing import TYPE_CHECKING from typing import Union import marshmallow @@ -18,6 +19,11 @@ from spiffworkflow_backend.models.db import SpiffworkflowBaseDBModel from spiffworkflow_backend.models.json_data import JsonDataModel from spiffworkflow_backend.models.task_definition import TaskDefinitionModel +if TYPE_CHECKING: + from spiffworkflow_backend.models.human_task_user import ( # noqa: F401 + HumanTaskModel, + ) + class TaskNotFoundError(Exception): pass @@ -52,6 +58,7 @@ class TaskModel(SpiffworkflowBaseDBModel): guid: str = db.Column(db.String(36), nullable=False, unique=True) bpmn_process_id: int = db.Column(ForeignKey(BpmnProcessModel.id), nullable=False, index=True) # type: ignore bpmn_process = relationship(BpmnProcessModel, back_populates="tasks") + human_tasks = relationship("HumanTaskModel", back_populates="task_model", cascade="delete") process_instance_id: int = db.Column(ForeignKey("process_instance.id"), nullable=False, index=True) # find this by looking up the "workflow_name" and "task_spec" from the properties_json @@ -110,6 +117,7 @@ class Task: event_definition: Union[dict[str, Any], None] = None, call_activity_process_identifier: Optional[str] = None, calling_subprocess_task_id: Optional[str] = None, + error_message: Optional[str] = None, ): """__init__.""" self.id = id @@ -147,6 +155,7 @@ class Task: self.properties = properties # Arbitrary extension properties from BPMN editor. if self.properties is None: self.properties = {} + self.error_message = error_message @property def serialized(self) -> dict[str, Any]: @@ -183,6 +192,7 @@ class Task: "event_definition": self.event_definition, "call_activity_process_identifier": self.call_activity_process_identifier, "calling_subprocess_task_id": self.calling_subprocess_task_id, + "error_message": self.error_message, } @classmethod diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py index 40c68885..9a1796ff 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py @@ -405,14 +405,29 @@ def _interstitial_stream(process_instance_id: int) -> Generator[str, Optional[st reported_ids.append(spiff_task.id) yield f"data: {current_app.json.dumps(task)} \n\n" last_task = spiff_task - processor.do_engine_steps(execution_strategy_name="run_until_user_message") - processor.do_engine_steps(execution_strategy_name="one_at_a_time") - spiff_task = processor.next_task() + try: + processor.do_engine_steps(execution_strategy_name="one_at_a_time") + processor.do_engine_steps(execution_strategy_name="run_until_user_message") + processor.save() # Fixme - maybe find a way not to do this on every loop? + except WorkflowTaskException as wfe: + api_error = ApiError.from_workflow_exception( + "engine_steps_error", "Failed complete an automated task.", exp=wfe + ) + yield f"data: {current_app.json.dumps(api_error)} \n\n" + except Exception as e: + api_error = ApiError( + error_code="engine_steps_error", + message=f"Failed complete an automated task. Error was: {str(e)}", + status_code=400, + ) + yield f"data: {current_app.json.dumps(api_error)} \n\n" + # Note, this has to be done in case someone leaves the page, # which can otherwise cancel this function and leave completed tasks un-registered. - processor.save() # Fixme - maybe find a way not to do this on every loop? + spiff_task = processor.next_task() + + # Always provide some response, in the event no instructions were provided. if len(reported_ids) == 0: - # Always provide some response, in the event no instructions were provided. task = ProcessInstanceService.spiff_task_to_api_task(processor, processor.next_task()) yield f"data: {current_app.json.dumps(task)} \n\n" diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py index 224e20fe..d3f62959 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py @@ -1741,8 +1741,8 @@ class ProcessInstanceProcessor: def next_task(self) -> SpiffTask: """Returns the next task that should be completed even if there are parallel tasks and multiple options are available. - If the process_instance is complete - it will return the final end task. + If the process_instance is complete it will return the final end task. + If the process_instance is in an error state it will return the task that is erroring. """ # If the whole blessed mess is done, return the end_event task in the tree # This was failing in the case of a call activity where we have an intermediate EndEvent @@ -1769,8 +1769,12 @@ class ProcessInstanceProcessor: waiting_tasks = self.bpmn_process_instance.get_tasks(TaskState.WAITING) if len(waiting_tasks) > 0: return waiting_tasks[0] - else: - return # We have not tasks to return. + + # If there are no ready tasks, and not waiting tasks, return the latest error. + error_task = None + for task in SpiffTask.Iterator(self.bpmn_process_instance.task_tree, TaskState.ERROR): + error_task = task + return error_task # Get a list of all completed user tasks (Non engine tasks) completed_user_tasks = self.completed_user_tasks() diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py index db3e62e1..de409783 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py @@ -462,6 +462,12 @@ class ProcessInstanceService: serialized_task_spec = processor.serialize_task_spec(spiff_task.task_spec) + # Grab the last error message. + error_message = None + for event in processor.process_instance_model.process_instance_events: + for detail in event.error_details: + error_message = detail.message + task = Task( spiff_task.id, spiff_task.task_spec.name, @@ -479,6 +485,7 @@ class ProcessInstanceService: event_definition=serialized_task_spec.get("event_definition"), call_activity_process_identifier=call_activity_process_identifier, calling_subprocess_task_id=calling_subprocess_task_id, + error_message=error_message, ) return task diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py index 27d56d35..00994496 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py @@ -634,7 +634,7 @@ class TaskService: isinstance(exception, ApiError) and exception.error_code == "task_error" ): task_line_number = exception.line_number - task_line_contents = exception.error_line + task_line_contents = exception.error_line[0:255] task_trace = exception.task_trace task_offset = exception.offset diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py index 2268e0e5..cfdbabd8 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_process_instance_processor.py @@ -311,6 +311,7 @@ class TestProcessInstanceProcessor(BaseTest): ProcessInstanceService.complete_form_task(processor, spiff_manual_task, {}, initiator_user, human_task_one) assert process_instance.status == "complete" + # this test has been failing intermittently for some time on windows, perhaps ever since it was first added def test_properly_resets_process_to_given_task_with_call_activity( self, app: Flask, @@ -350,12 +351,14 @@ class TestProcessInstanceProcessor(BaseTest): ProcessInstanceService.complete_form_task(processor, spiff_manual_task, {}, initiator_user, human_task_one) processor.suspend() - task_model_to_reset_to = ( + all_task_models_matching_top_level_subprocess_script = ( TaskModel.query.join(TaskDefinitionModel) .filter(TaskDefinitionModel.bpmn_identifier == "top_level_subprocess_script") .order_by(TaskModel.id.desc()) # type: ignore - .first() + .all() ) + assert len(all_task_models_matching_top_level_subprocess_script) == 1 + task_model_to_reset_to = all_task_models_matching_top_level_subprocess_script[0] assert task_model_to_reset_to is not None ProcessInstanceProcessor.reset_process(process_instance, task_model_to_reset_to.guid) @@ -364,7 +367,7 @@ class TestProcessInstanceProcessor(BaseTest): process_instance = ProcessInstanceModel.query.filter_by(id=process_instance.id).first() processor = ProcessInstanceProcessor(process_instance) - # make sure we reset to the task we expected + # make sure we did actually reset to the task we expected ready_or_waiting_tasks = processor.get_all_ready_or_waiting_tasks() top_level_subprocess_script_spiff_task = next( task for task in ready_or_waiting_tasks if task.task_spec.name == "top_level_subprocess_script" @@ -373,7 +376,11 @@ class TestProcessInstanceProcessor(BaseTest): processor.resume() processor.do_engine_steps(save=True, execution_strategy_name="greedy") + # reload again, just in case, since the assertion where it says there should be 1 active_human_task + # is failing intermittently on windows, so just debugging. + process_instance = ProcessInstanceModel.query.filter_by(id=process_instance.id).first() assert len(process_instance.active_human_tasks) == 1 + human_task_one = process_instance.active_human_tasks[0] spiff_manual_task = processor.bpmn_process_instance.get_task_from_id(UUID(human_task_one.task_id)) ProcessInstanceService.complete_form_task(processor, spiff_manual_task, {}, initiator_user, human_task_one) diff --git a/spiffworkflow-frontend/public/interstitial/errored.png b/spiffworkflow-frontend/public/interstitial/errored.png new file mode 100644 index 00000000..c931b7b1 Binary files /dev/null and b/spiffworkflow-frontend/public/interstitial/errored.png differ diff --git a/spiffworkflow-frontend/src/components/ErrorDisplay.tsx b/spiffworkflow-frontend/src/components/ErrorDisplay.tsx index 67e6e18d..40116973 100644 --- a/spiffworkflow-frontend/src/components/ErrorDisplay.tsx +++ b/spiffworkflow-frontend/src/components/ErrorDisplay.tsx @@ -109,6 +109,7 @@ export default function ErrorDisplay() { if (errorObject) { const title = 'Error:'; + window.scrollTo(0, 0); // Scroll back to the top of the page errorTag = ( removeError()} type="error"> diff --git a/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx b/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx index 6a1562af..3247fec0 100644 --- a/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx +++ b/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx @@ -1190,7 +1190,7 @@ export default function ProcessInstanceListTable({ return null; }} placeholder="Start typing username" - titleText="Process Initiator" + titleText="Started By" selectedItem={processInitiatorSelection} /> ); @@ -1199,7 +1199,7 @@ export default function ProcessInstanceListTable({ { @@ -1365,68 +1365,30 @@ export default function ProcessInstanceListTable({ const formatter = reportColumnFormatters[column.accessor] ?? defaultFormatter; const value = row[column.accessor]; - const modifiedModelId = modifyProcessIdentifierForPathParam( - row.process_model_identifier - ); - const navigateToProcessInstance = () => { - navigate(`${processInstanceShowPathPrefix}/${modifiedModelId}/${row.id}`); - }; - const navigateToProcessModel = () => { - navigate(`/admin/process-models/${modifiedModelId}`); - }; if (column.accessor === 'status') { return ( - // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions - + {formatter(row, value)} ); } if (column.accessor === 'process_model_display_name') { - const pmStyle = { background: 'rgba(0, 0, 0, .02)' }; - return ( - // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions - - {formatter(row, value)} - - ); + return {formatter(row, value)} ; } if (column.accessor === 'waiting_for') { - return ( - // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions - - {getWaitingForTableCellComponent(row)} - - ); + return {getWaitingForTableCellComponent(row)} ; } if (column.accessor === 'updated_at_in_seconds') { return ( ); } return ( // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions - + {formatter(row, value)} ); @@ -1484,9 +1446,22 @@ export default function ProcessInstanceListTable({ } const rowStyle = { cursor: 'pointer' }; - + const modifiedModelId = modifyProcessIdentifierForPathParam( + row.process_model_identifier + ); + const navigateToProcessInstance = () => { + navigate( + `${processInstanceShowPathPrefix}/${modifiedModelId}/${row.id}` + ); + }; return ( - + // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions + {currentRow} ); diff --git a/spiffworkflow-frontend/src/components/ProcessModelSearch.tsx b/spiffworkflow-frontend/src/components/ProcessModelSearch.tsx index 3a6331a3..8f135652 100644 --- a/spiffworkflow-frontend/src/components/ProcessModelSearch.tsx +++ b/spiffworkflow-frontend/src/components/ProcessModelSearch.tsx @@ -15,7 +15,7 @@ export default function ProcessModelSearch({ processModels, selectedItem, onChange, - titleText = 'Process model', + titleText = 'Process', }: OwnProps) { const getParentGroupsDisplayName = (processModel: ProcessModel) => { if (processModel.parent_groups) { diff --git a/spiffworkflow-frontend/src/interfaces.ts b/spiffworkflow-frontend/src/interfaces.ts index 93db3af5..c9667445 100644 --- a/spiffworkflow-frontend/src/interfaces.ts +++ b/spiffworkflow-frontend/src/interfaces.ts @@ -81,6 +81,7 @@ export interface ProcessInstanceTask { potential_owner_usernames?: string; assigned_user_group_identifier?: string; + error_message?: string; } export interface ProcessReference { diff --git a/spiffworkflow-frontend/src/routes/ProcessInterstitial.tsx b/spiffworkflow-frontend/src/routes/ProcessInterstitial.tsx index bdd56699..59e7e8ae 100644 --- a/spiffworkflow-frontend/src/routes/ProcessInterstitial.tsx +++ b/spiffworkflow-frontend/src/routes/ProcessInterstitial.tsx @@ -10,6 +10,7 @@ import { getBasicHeaders } from '../services/HttpService'; import InstructionsForEndUser from '../components/InstructionsForEndUser'; import ProcessBreadcrumb from '../components/ProcessBreadcrumb'; import { ProcessInstanceTask } from '../interfaces'; +import useAPIError from '../hooks/UseApiError'; export default function ProcessInterstitial() { const [data, setData] = useState([]); @@ -20,6 +21,7 @@ export default function ProcessInterstitial() { const userTasks = useMemo(() => { return ['User Task', 'Manual Task']; }, []); + const { addError } = useAPIError(); useEffect(() => { fetchEventSource( @@ -27,9 +29,13 @@ export default function ProcessInterstitial() { { headers: getBasicHeaders(), onmessage(ev) { - const task = JSON.parse(ev.data); - setData((prevData) => [...prevData, task]); - setLastTask(task); + const retValue = JSON.parse(ev.data); + if ('error_code' in retValue) { + addError(retValue); + } else { + setData((prevData) => [...prevData, retValue]); + setLastTask(retValue); + } }, onclose() { setState('CLOSED'); @@ -85,6 +91,8 @@ export default function ProcessInterstitial() { return Waiting ....; case 'COMPLETED': return Completed; + case 'ERROR': + return Errored; default: return getStatus(); } @@ -104,6 +112,10 @@ export default function ProcessInterstitial() { if (shouldRedirect(myTask)) { return
Redirecting you to the next task now ...
; } + if (myTask.error_message) { + return
{myTask.error_message}
; + } + return (
@@ -147,7 +159,7 @@ export default function ProcessInterstitial() { Task: {d.title} - + {userMessage(d)} diff --git a/spiffworkflow-frontend/src/routes/TaskShow.tsx b/spiffworkflow-frontend/src/routes/TaskShow.tsx index 56ea9bc2..b8d3d10f 100644 --- a/spiffworkflow-frontend/src/routes/TaskShow.tsx +++ b/spiffworkflow-frontend/src/routes/TaskShow.tsx @@ -117,10 +117,10 @@ export default function TaskShow() { const processResult = (result: ProcessInstanceTask) => { setTask(result); setDisabled(false); - if (!result.can_complete) { navigateToInterstitial(result); } + window.scrollTo(0, 0); // Scroll back to the top of the page /* Disable call to load previous tasks -- do not display menu. const url = `/v1.0/process-instances/for-me/${modifyProcessIdentifierForPathParam(