updates to disallow modifying a process instance when it is not in the correct state w/ burnettk

This commit is contained in:
jasquat 2022-12-19 12:55:25 -05:00
parent aec0416eed
commit f152195335
7 changed files with 85 additions and 35 deletions

View File

@ -798,7 +798,7 @@ paths:
schema: schema:
$ref: "#/components/schemas/Workflow" $ref: "#/components/schemas/Workflow"
/process-instances/{modified_process_model_identifier}/{process_instance_id}/terminate: /process-instance-terminate/{modified_process_model_identifier}/{process_instance_id}:
parameters: parameters:
- name: process_instance_id - name: process_instance_id
in: path in: path
@ -819,7 +819,7 @@ paths:
schema: schema:
$ref: "#/components/schemas/OkTrue" $ref: "#/components/schemas/OkTrue"
/process-instances/{modified_process_model_identifier}/{process_instance_id}/suspend: /process-instance-suspend/{modified_process_model_identifier}/{process_instance_id}:
parameters: parameters:
- name: process_instance_id - name: process_instance_id
in: path in: path
@ -840,7 +840,7 @@ paths:
schema: schema:
$ref: "#/components/schemas/OkTrue" $ref: "#/components/schemas/OkTrue"
/process-instances/{modified_process_model_identifier}/{process_instance_id}/resume: /process-instance-resume/{modified_process_model_identifier}/{process_instance_id}:
parameters: parameters:
- name: process_instance_id - name: process_instance_id
in: path in: path

View File

@ -30,6 +30,10 @@ class ProcessInstanceTaskDataCannotBeUpdatedError(Exception):
"""ProcessInstanceTaskDataCannotBeUpdatedError.""" """ProcessInstanceTaskDataCannotBeUpdatedError."""
class ProcessInstanceCannotBeDeletedError(Exception):
"""ProcessInstanceCannotBeDeletedError."""
class NavigationItemSchema(Schema): class NavigationItemSchema(Schema):
"""NavigationItemSchema.""" """NavigationItemSchema."""
@ -135,6 +139,15 @@ class ProcessInstanceModel(SpiffworkflowBaseDBModel):
"""Validate_status.""" """Validate_status."""
return self.validate_enum_field(key, value, ProcessInstanceStatus) return self.validate_enum_field(key, value, ProcessInstanceStatus)
def has_terminal_status(self) -> bool:
"""Has_terminal_status."""
return self.status in self.terminal_statuses()
@classmethod
def terminal_statuses(cls) -> list[str]:
"""Terminal_statuses."""
return ["complete", "error", "terminated"]
class ProcessInstanceModelSchema(Schema): class ProcessInstanceModelSchema(Schema):
"""ProcessInstanceModelSchema.""" """ProcessInstanceModelSchema."""

View File

@ -53,6 +53,9 @@ from spiffworkflow_backend.models.principal import PrincipalModel
from spiffworkflow_backend.models.process_group import ProcessGroup from spiffworkflow_backend.models.process_group import ProcessGroup
from spiffworkflow_backend.models.process_group import ProcessGroupSchema from spiffworkflow_backend.models.process_group import ProcessGroupSchema
from spiffworkflow_backend.models.process_instance import ProcessInstanceApiSchema from spiffworkflow_backend.models.process_instance import ProcessInstanceApiSchema
from spiffworkflow_backend.models.process_instance import (
ProcessInstanceCannotBeDeletedError,
)
from spiffworkflow_backend.models.process_instance import ProcessInstanceModel from spiffworkflow_backend.models.process_instance import ProcessInstanceModel
from spiffworkflow_backend.models.process_instance import ProcessInstanceModelSchema from spiffworkflow_backend.models.process_instance import ProcessInstanceModelSchema
from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus from spiffworkflow_backend.models.process_instance import ProcessInstanceStatus
@ -580,6 +583,13 @@ def process_instance_run(
process_instance = ProcessInstanceService().get_process_instance( process_instance = ProcessInstanceService().get_process_instance(
process_instance_id process_instance_id
) )
if process_instance.status != "not_started":
raise ApiError(
error_code="process_instance_not_runnable",
message=f"Process Instance ({process_instance.id}) is currently running or has already run.",
status_code=400,
)
processor = ProcessInstanceProcessor(process_instance) processor = ProcessInstanceProcessor(process_instance)
if do_engine_steps: if do_engine_steps:
@ -938,7 +948,7 @@ def process_instance_list(
if report_filter.initiated_by_me is True: if report_filter.initiated_by_me is True:
process_instance_query = process_instance_query.filter( process_instance_query = process_instance_query.filter(
ProcessInstanceModel.status.in_(["complete", "error", "terminated"]) # type: ignore ProcessInstanceModel.status.in_(ProcessInstanceModel.terminal_statuses()) # type: ignore
) )
process_instance_query = process_instance_query.filter_by( process_instance_query = process_instance_query.filter_by(
process_initiator=g.user process_initiator=g.user
@ -947,7 +957,7 @@ def process_instance_list(
# TODO: not sure if this is exactly what is wanted # TODO: not sure if this is exactly what is wanted
if report_filter.with_tasks_completed_by_me is True: if report_filter.with_tasks_completed_by_me is True:
process_instance_query = process_instance_query.filter( process_instance_query = process_instance_query.filter(
ProcessInstanceModel.status.in_(["complete", "error", "terminated"]) # type: ignore ProcessInstanceModel.status.in_(ProcessInstanceModel.terminal_statuses()) # type: ignore
) )
# process_instance_query = process_instance_query.join(UserModel, UserModel.id == ProcessInstanceModel.process_initiator_id) # process_instance_query = process_instance_query.join(UserModel, UserModel.id == ProcessInstanceModel.process_initiator_id)
# process_instance_query = process_instance_query.add_columns(UserModel.username) # process_instance_query = process_instance_query.add_columns(UserModel.username)
@ -976,7 +986,7 @@ def process_instance_list(
if report_filter.with_tasks_completed_by_my_group is True: if report_filter.with_tasks_completed_by_my_group is True:
process_instance_query = process_instance_query.filter( process_instance_query = process_instance_query.filter(
ProcessInstanceModel.status.in_(["complete", "error", "terminated"]) # type: ignore ProcessInstanceModel.status.in_(ProcessInstanceModel.terminal_statuses()) # type: ignore
) )
process_instance_query = process_instance_query.join( process_instance_query = process_instance_query.join(
SpiffStepDetailsModel, SpiffStepDetailsModel,
@ -1165,6 +1175,12 @@ def process_instance_delete(
"""Create_process_instance.""" """Create_process_instance."""
process_instance = find_process_instance_by_id_or_raise(process_instance_id) process_instance = find_process_instance_by_id_or_raise(process_instance_id)
if not process_instance.has_terminal_status():
raise ProcessInstanceCannotBeDeletedError(
f"Process instance ({process_instance.id}) cannot be deleted since it does not have a terminal status. "
f"Current status is {process_instance.status}."
)
# (Pdb) db.session.delete # (Pdb) db.session.delete
# <bound method delete of <sqlalchemy.orm.scoping.scoped_session object at 0x103eaab30>> # <bound method delete of <sqlalchemy.orm.scoping.scoped_session object at 0x103eaab30>>
db.session.query(SpiffLoggingModel).filter_by( db.session.query(SpiffLoggingModel).filter_by(

View File

@ -1375,7 +1375,7 @@ class TestProcessApi(BaseTest):
assert response.json is not None assert response.json is not None
response = client.post( response = client.post(
f"/v1.0/process-instances/{self.modify_process_identifier_for_path_param(process_model_identifier)}/{process_instance_id}/terminate", f"/v1.0/process-instance-terminate/{self.modify_process_identifier_for_path_param(process_model_identifier)}/{process_instance_id}",
headers=self.logged_in_headers(with_super_admin_user), headers=self.logged_in_headers(with_super_admin_user),
) )
assert response.status_code == 200 assert response.status_code == 200
@ -1396,15 +1396,13 @@ class TestProcessApi(BaseTest):
) -> None: ) -> None:
"""Test_process_instance_delete.""" """Test_process_instance_delete."""
process_group_id = "my_process_group" process_group_id = "my_process_group"
process_model_id = "user_task" process_model_id = "sample"
bpmn_file_name = "user_task.bpmn" bpmn_file_location = "sample"
bpmn_file_location = "user_task"
process_model_identifier = self.create_group_and_model_with_bpmn( process_model_identifier = self.create_group_and_model_with_bpmn(
client, client,
with_super_admin_user, with_super_admin_user,
process_group_id=process_group_id, process_group_id=process_group_id,
process_model_id=process_model_id, process_model_id=process_model_id,
bpmn_file_name=bpmn_file_name,
bpmn_file_location=bpmn_file_location, bpmn_file_location=bpmn_file_location,
) )
@ -1420,11 +1418,13 @@ class TestProcessApi(BaseTest):
headers=self.logged_in_headers(with_super_admin_user), headers=self.logged_in_headers(with_super_admin_user),
) )
assert response.json is not None assert response.json is not None
assert response.status_code == 200
delete_response = client.delete( delete_response = client.delete(
f"/v1.0/process-instances/{self.modify_process_identifier_for_path_param(process_model_identifier)}/{process_instance_id}", f"/v1.0/process-instances/{self.modify_process_identifier_for_path_param(process_model_identifier)}/{process_instance_id}",
headers=self.logged_in_headers(with_super_admin_user), headers=self.logged_in_headers(with_super_admin_user),
) )
assert delete_response.json["ok"] is True
assert delete_response.status_code == 200 assert delete_response.status_code == 200
def test_task_show( def test_task_show(
@ -2421,7 +2421,7 @@ class TestProcessApi(BaseTest):
assert process_instance.status == "user_input_required" assert process_instance.status == "user_input_required"
client.post( client.post(
f"/v1.0/process-instances/{self.modify_process_identifier_for_path_param(process_model_identifier)}/{process_instance_id}/suspend", f"/v1.0/process-instance-suspend/{self.modify_process_identifier_for_path_param(process_model_identifier)}/{process_instance_id}",
headers=self.logged_in_headers(with_super_admin_user), headers=self.logged_in_headers(with_super_admin_user),
) )
process_instance = ProcessInstanceService().get_process_instance( process_instance = ProcessInstanceService().get_process_instance(
@ -2429,15 +2429,25 @@ class TestProcessApi(BaseTest):
) )
assert process_instance.status == "suspended" assert process_instance.status == "suspended"
# TODO: Why can I run a suspended process instance?
response = client.post( response = client.post(
f"/v1.0/process-instances/{self.modify_process_identifier_for_path_param(process_model_identifier)}/{process_instance_id}/run", f"/v1.0/process-instances/{self.modify_process_identifier_for_path_param(process_model_identifier)}/{process_instance_id}/run",
headers=self.logged_in_headers(with_super_admin_user), headers=self.logged_in_headers(with_super_admin_user),
) )
process_instance = ProcessInstanceService().get_process_instance(
process_instance_id
)
assert process_instance.status == "suspended"
assert response.status_code == 400
# task = response.json['next_task'] response = client.post(
f"/v1.0/process-instance-resume/{self.modify_process_identifier_for_path_param(process_model_identifier)}/{process_instance_id}",
print("test_process_instance_suspend") headers=self.logged_in_headers(with_super_admin_user),
)
assert response.status_code == 200
process_instance = ProcessInstanceService().get_process_instance(
process_instance_id
)
assert process_instance.status == "waiting"
def test_script_unit_test_run( def test_script_unit_test_run(
self, self,

View File

@ -0,0 +1,5 @@
export default class ProcessInstanceClass {
static terminalStatuses() {
return ['complete', 'error', 'terminated'];
}
}

View File

@ -11,6 +11,9 @@ export const useUriListForPermissions = () => {
processGroupShowPath: `/v1.0/process-groups/${params.process_group_id}`, processGroupShowPath: `/v1.0/process-groups/${params.process_group_id}`,
processInstanceCreatePath: `/v1.0/process-instances/${params.process_model_id}`, processInstanceCreatePath: `/v1.0/process-instances/${params.process_model_id}`,
processInstanceActionPath: `/v1.0/process-instances/${params.process_model_id}/${params.process_instance_id}`, processInstanceActionPath: `/v1.0/process-instances/${params.process_model_id}/${params.process_instance_id}`,
processInstanceResumePath: `/v1.0/process-instance-resume/${params.process_model_id}/${params.process_instance_id}`,
processInstanceSuspendPath: `/v1.0/process-instance-suspend/${params.process_model_id}/${params.process_instance_id}`,
processInstanceTerminatePath: `/v1.0/process-instance-terminate/${params.process_model_id}/${params.process_instance_id}`,
processInstanceListPath: '/v1.0/process-instances', processInstanceListPath: '/v1.0/process-instances',
processInstanceLogListPath: `/v1.0/logs/${params.process_model_id}/${params.process_instance_id}`, processInstanceLogListPath: `/v1.0/logs/${params.process_model_id}/${params.process_instance_id}`,
processInstanceReportListPath: '/v1.0/process-instances/reports', processInstanceReportListPath: '/v1.0/process-instances/reports',

View File

@ -45,6 +45,7 @@ import {
ProcessInstanceTask, ProcessInstanceTask,
} from '../interfaces'; } from '../interfaces';
import { usePermissionFetcher } from '../hooks/PermissionService'; import { usePermissionFetcher } from '../hooks/PermissionService';
import ProcessInstanceClass from '../classes/ProcessInstanceClass';
export default function ProcessInstanceShow() { export default function ProcessInstanceShow() {
const navigate = useNavigate(); const navigate = useNavigate();
@ -74,9 +75,9 @@ export default function ProcessInstanceShow() {
[targetUris.processInstanceActionPath]: ['DELETE'], [targetUris.processInstanceActionPath]: ['DELETE'],
[targetUris.processInstanceLogListPath]: ['GET'], [targetUris.processInstanceLogListPath]: ['GET'],
[targetUris.processModelShowPath]: ['PUT'], [targetUris.processModelShowPath]: ['PUT'],
[`${targetUris.processInstanceActionPath}/suspend`]: ['POST'], [`${targetUris.processInstanceResumePath}`]: ['POST'],
[`${targetUris.processInstanceActionPath}/terminate`]: ['POST'], [`${targetUris.processInstanceSuspendPath}`]: ['POST'],
[`${targetUris.processInstanceActionPath}/resume`]: ['POST'], [`${targetUris.processInstanceTerminatePath}`]: ['POST'],
}; };
const { ability, permissionsLoaded } = usePermissionFetcher( const { ability, permissionsLoaded } = usePermissionFetcher(
permissionRequestData permissionRequestData
@ -146,7 +147,7 @@ export default function ProcessInstanceShow() {
const terminateProcessInstance = () => { const terminateProcessInstance = () => {
HttpService.makeCallToBackend({ HttpService.makeCallToBackend({
path: `${targetUris.processInstanceActionPath}/terminate`, path: `${targetUris.processInstanceTerminatePath}`,
successCallback: refreshPage, successCallback: refreshPage,
httpMethod: 'POST', httpMethod: 'POST',
}); });
@ -154,7 +155,7 @@ export default function ProcessInstanceShow() {
const suspendProcessInstance = () => { const suspendProcessInstance = () => {
HttpService.makeCallToBackend({ HttpService.makeCallToBackend({
path: `${targetUris.processInstanceActionPath}/suspend`, path: `${targetUris.processInstanceSuspendPath}`,
successCallback: refreshPage, successCallback: refreshPage,
httpMethod: 'POST', httpMethod: 'POST',
}); });
@ -162,7 +163,7 @@ export default function ProcessInstanceShow() {
const resumeProcessInstance = () => { const resumeProcessInstance = () => {
HttpService.makeCallToBackend({ HttpService.makeCallToBackend({
path: `${targetUris.processInstanceActionPath}/resume`, path: `${targetUris.processInstanceResumePath}`,
successCallback: refreshPage, successCallback: refreshPage,
httpMethod: 'POST', httpMethod: 'POST',
}); });
@ -333,7 +334,7 @@ export default function ProcessInstanceShow() {
const terminateButton = () => { const terminateButton = () => {
if ( if (
processInstance && processInstance &&
!['complete', 'terminated', 'error'].includes(processInstance.status) !ProcessInstanceClass.terminalStatuses().includes(processInstance.status)
) { ) {
return ( return (
<ButtonWithConfirmation <ButtonWithConfirmation
@ -353,9 +354,9 @@ export default function ProcessInstanceShow() {
const suspendButton = () => { const suspendButton = () => {
if ( if (
processInstance && processInstance &&
!['complete', 'terminated', 'error', 'suspended'].includes( !ProcessInstanceClass.terminalStatuses()
processInstance.status .concat(['suspended'])
) .includes(processInstance.status)
) { ) {
return ( return (
<Button <Button
@ -608,21 +609,23 @@ export default function ProcessInstanceShow() {
}; };
const buttonIcons = () => { const buttonIcons = () => {
if (!processInstance) {
return null;
}
const elements = []; const elements = [];
if ( if (ability.can('POST', `${targetUris.processInstanceTerminatePath}`)) {
ability.can('POST', `${targetUris.processInstanceActionPath}/terminate`)
) {
elements.push(terminateButton()); elements.push(terminateButton());
} }
if ( if (ability.can('POST', `${targetUris.processInstanceSuspendPath}`)) {
ability.can('POST', `${targetUris.processInstanceActionPath}/suspend`)
) {
elements.push(suspendButton()); elements.push(suspendButton());
} }
if (ability.can('POST', `${targetUris.processInstanceActionPath}/resume`)) { if (ability.can('POST', `${targetUris.processInstanceResumePath}`)) {
elements.push(resumeButton()); elements.push(resumeButton());
} }
if (ability.can('DELETE', targetUris.processInstanceActionPath)) { if (
ability.can('DELETE', targetUris.processInstanceActionPath) &&
ProcessInstanceClass.terminalStatuses().includes(processInstance.status)
) {
elements.push( elements.push(
<ButtonWithConfirmation <ButtonWithConfirmation
data-qa="process-instance-delete" data-qa="process-instance-delete"
@ -630,7 +633,7 @@ export default function ProcessInstanceShow() {
renderIcon={TrashCan} renderIcon={TrashCan}
iconDescription="Delete" iconDescription="Delete"
hasIconOnly hasIconOnly
description={`Delete Process Instance: ${processInstance}`} description={`Delete Process Instance: ${processInstance.id}`}
onConfirmation={deleteProcessInstance} onConfirmation={deleteProcessInstance}
confirmButtonLabel="Delete" confirmButtonLabel="Delete"
/> />