From 56c8c0c6334d7a03298137467c759c7803dc79bf Mon Sep 17 00:00:00 2001 From: jasquat Date: Tue, 15 Nov 2022 17:35:16 -0500 Subject: [PATCH 1/2] added some permissions to the process model show page w/ burnettk --- .../src/spiffworkflow_backend/api.yml | 8 +- .../routes/process_api_blueprint.py | 4 +- .../services/spec_file_service.py | 12 +- .../integration/test_nested_groups.py | 12 +- .../integration/test_process_api.py | 4 +- .../unit/test_spec_file_service.py | 9 +- spiffworkflow-frontend/.eslintrc.js | 1 + spiffworkflow-frontend/src/App.tsx | 2 +- .../src/components/ProcessModelForm.tsx | 5 +- .../src/components/ReactDiagramEditor.tsx | 59 ++++-- spiffworkflow-frontend/src/contexts/Can.tsx | 2 +- .../src/hooks/PermissionService.tsx | 14 +- .../src/hooks/UriListForPermissions.tsx | 4 +- .../src/routes/ProcessGroupShow.tsx | 30 +-- .../src/routes/ProcessModelShow.tsx | 181 ++++++++++-------- 15 files changed, 200 insertions(+), 147 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml index dfea2ce8..79a218ee 100755 --- a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml @@ -258,7 +258,6 @@ paths: description: The number of models to show per page. Defaults to page 10. schema: type: integer - # process_model_list get: operationId: spiffworkflow_backend.routes.process_api_blueprint.process_model_list summary: Return a list of process models for a given process group @@ -273,9 +272,10 @@ paths: type: array items: $ref: "#/components/schemas/ProcessModel" - # process_model_add + + /process-models/{modified_process_group_id}: post: - operationId: spiffworkflow_backend.routes.process_api_blueprint.process_model_add + operationId: spiffworkflow_backend.routes.process_api_blueprint.process_model_create summary: Creates a new process model with the given parameters. tags: - Process Models @@ -371,7 +371,7 @@ paths: application/json: schema: $ref: "#/components/schemas/OkTrue" - # process_model_list + /processes: get: operationId: spiffworkflow_backend.routes.process_api_blueprint.process_list diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py index 789047cf..222aeded 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_api_blueprint.py @@ -232,10 +232,10 @@ def process_group_show( return make_response(jsonify(process_group), 200) -def process_model_add( +def process_model_create( body: Dict[str, Union[str, bool, int]] ) -> flask.wrappers.Response: - """Add_process_model.""" + """Process_model_create.""" process_model_info = ProcessModelInfoSchema().load(body) if process_model_info is None: raise ApiError( diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py index 0b5ae9dd..a1bebb58 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py @@ -240,11 +240,13 @@ class SpecFileService(FileSystemService): SpecFileService.update_correlation_cache(ref) @staticmethod - def clear_caches_for_file(file_name: str, process_model_info: ProcessModelInfo) -> None: - """Clear all caches related to a file""" - db.session.query(SpecReferenceCache).\ - filter(SpecReferenceCache.file_name == file_name).\ - filter(SpecReferenceCache.process_model_id == process_model_info.id).delete() + def clear_caches_for_file( + file_name: str, process_model_info: ProcessModelInfo + ) -> None: + """Clear all caches related to a file.""" + db.session.query(SpecReferenceCache).filter( + SpecReferenceCache.file_name == file_name + ).filter(SpecReferenceCache.process_model_id == process_model_info.id).delete() # fixme: likely the other caches should be cleared as well, but we don't have a clean way to do so yet. @staticmethod diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_nested_groups.py b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_nested_groups.py index 9a48ebf1..3a12acf6 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_nested_groups.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_nested_groups.py @@ -143,7 +143,6 @@ class TestNestedGroups(BaseTest): response = client.get( # noqa: F841 target_uri, headers=self.logged_in_headers(user) ) - print("test_nested_groups") def test_add_nested_group( self, @@ -153,10 +152,6 @@ class TestNestedGroups(BaseTest): with_super_admin_user: UserModel, ) -> None: """Test_add_nested_group.""" - # user = self.find_or_create_user() - # self.add_permissions_to_user( - # user, target_uri=target_uri, permission_names=["read", "create"] - # ) process_group_a = ProcessGroup( id="group_a", display_name="Group A", @@ -194,16 +189,14 @@ class TestNestedGroups(BaseTest): data=json.dumps(ProcessGroupSchema().dump(process_group_c)), ) - print("test_add_nested_group") - - def test_process_model_add( + def test_process_model_create( self, app: Flask, client: FlaskClient, with_db_and_bpmn_file_cleanup: None, with_super_admin_user: UserModel, ) -> None: - """Test_process_model_add.""" + """Test_process_model_create.""" process_group_a = ProcessGroup( id="group_a", display_name="Group A", @@ -242,7 +235,6 @@ class TestNestedGroups(BaseTest): content_type="application/json", data=json.dumps(ProcessModelInfoSchema().dump(process_model)), ) - print("test_process_model_add") def test_process_group_show( self, diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py index 85ee4a05..5596e72a 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py @@ -105,14 +105,14 @@ class TestProcessApi(BaseTest): assert response.json is not None assert response.json == expected_response_body - def test_process_model_add( + def test_process_model_create( self, app: Flask, client: FlaskClient, with_db_and_bpmn_file_cleanup: None, with_super_admin_user: UserModel, ) -> None: - """Test_add_new_process_model.""" + """Test_process_model_create.""" process_group_id = "test_process_group" process_group_display_name = "Test Process Group" # creates the group directory, and the json file diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_spec_file_service.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_spec_file_service.py index b5e5aa3c..81db1171 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_spec_file_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_spec_file_service.py @@ -119,22 +119,22 @@ class TestSpecFileService(BaseTest): == self.call_activity_nested_relative_file_path ) - def test_change_the_identifier_cleans_up_cache ( + def test_change_the_identifier_cleans_up_cache( self, app: Flask, client: FlaskClient, with_db_and_bpmn_file_cleanup: None, with_super_admin_user: UserModel, ) -> None: - """ When a BPMN processes identifier is changed in a file, the old id - is removed from the cache""" + """When a BPMN processes identifier is changed in a file, the old id + is removed from the cache.""" old_identifier = "ye_old_identifier" process_id_lookup = SpecReferenceCache( identifier=old_identifier, relative_path=self.call_activity_nested_relative_file_path, file_name=self.bpmn_file_name, process_model_id=f"{self.process_group_id}/{self.process_model_id}", - type='process' + type="process", ) db.session.add(process_id_lookup) db.session.commit() @@ -157,7 +157,6 @@ class TestSpecFileService(BaseTest): == self.call_activity_nested_relative_file_path ) - def test_load_reference_information( self, app: Flask, diff --git a/spiffworkflow-frontend/.eslintrc.js b/spiffworkflow-frontend/.eslintrc.js index ebec84ef..b6829ff4 100644 --- a/spiffworkflow-frontend/.eslintrc.js +++ b/spiffworkflow-frontend/.eslintrc.js @@ -36,6 +36,7 @@ module.exports = { ], 'react/react-in-jsx-scope': 'off', 'react/require-default-props': 'off', + 'import/prefer-default-export': 'off', 'no-unused-vars': [ 'error', { diff --git a/spiffworkflow-frontend/src/App.tsx b/spiffworkflow-frontend/src/App.tsx index 68e4b638..d7e4866f 100644 --- a/spiffworkflow-frontend/src/App.tsx +++ b/spiffworkflow-frontend/src/App.tsx @@ -24,7 +24,7 @@ export default function App() { [errorMessage] ); - const ability = defineAbility((can: any) => {}); + const ability = defineAbility(() => {}); let errorTag = null; if (errorMessage) { diff --git a/spiffworkflow-frontend/src/components/ProcessModelForm.tsx b/spiffworkflow-frontend/src/components/ProcessModelForm.tsx index 8cca5793..15e4c270 100644 --- a/spiffworkflow-frontend/src/components/ProcessModelForm.tsx +++ b/spiffworkflow-frontend/src/components/ProcessModelForm.tsx @@ -74,10 +74,7 @@ export default function ProcessModelForm({ if (hasErrors) { return; } - let path = `/process-models`; - if (mode === 'edit') { - path = `/process-models/${modifiedProcessModelPath}`; - } + const path = `/process-models/${modifiedProcessModelPath}`; let httpMethod = 'POST'; if (mode === 'edit') { httpMethod = 'PUT'; diff --git a/spiffworkflow-frontend/src/components/ReactDiagramEditor.tsx b/spiffworkflow-frontend/src/components/ReactDiagramEditor.tsx index 37a947e4..1f765157 100644 --- a/spiffworkflow-frontend/src/components/ReactDiagramEditor.tsx +++ b/spiffworkflow-frontend/src/components/ReactDiagramEditor.tsx @@ -52,10 +52,14 @@ import TouchModule from 'diagram-js/lib/navigation/touch'; // @ts-expect-error TS(7016) FIXME import ZoomScrollModule from 'diagram-js/lib/navigation/zoomscroll'; +import { Can } from '@casl/react'; import HttpService from '../services/HttpService'; import ButtonWithConfirmation from './ButtonWithConfirmation'; import { makeid } from '../helpers'; +import { useUriListForPermissions } from '../hooks/UriListForPermissions'; +import { PermissionsToCheck } from '../interfaces'; +import { usePermissionFetcher } from '../hooks/PermissionService'; type OwnProps = { processModelId: string; @@ -107,6 +111,13 @@ export default function ReactDiagramEditor({ const alreadyImportedXmlRef = useRef(false); + const { targetUris } = useUriListForPermissions(); + const permissionRequestData: PermissionsToCheck = { + [targetUris.processModelShowPath]: ['PUT'], + [targetUris.processModelFileShowPath]: ['POST', 'GET', 'PUT', 'DELETE'], + }; + const { ability } = usePermissionFetcher(permissionRequestData); + useEffect(() => { if (diagramModelerState) { return; @@ -517,20 +528,40 @@ export default function ReactDiagramEditor({ if (diagramType !== 'readonly') { return ( <> - - {fileName && ( - - )} - {onSetPrimaryFile && ( - - )} - + + + + + {fileName && ( + + )} + + + {onSetPrimaryFile && ( + + )} + + + + ); } diff --git a/spiffworkflow-frontend/src/contexts/Can.tsx b/spiffworkflow-frontend/src/contexts/Can.tsx index 80458fd6..ff446af8 100644 --- a/spiffworkflow-frontend/src/contexts/Can.tsx +++ b/spiffworkflow-frontend/src/contexts/Can.tsx @@ -1,5 +1,5 @@ import { createContext } from 'react'; -import { AbilityBuilder, Ability } from '@casl/ability'; +import { Ability } from '@casl/ability'; import { createContextualCan } from '@casl/react'; export const AbilityContext = createContext(new Ability()); diff --git a/spiffworkflow-frontend/src/hooks/PermissionService.tsx b/spiffworkflow-frontend/src/hooks/PermissionService.tsx index 400f29da..be39b9f9 100644 --- a/spiffworkflow-frontend/src/hooks/PermissionService.tsx +++ b/spiffworkflow-frontend/src/hooks/PermissionService.tsx @@ -12,19 +12,17 @@ export const usePermissionFetcher = ( useEffect(() => { const processPermissionResult = (result: PermissionCheckResponseBody) => { const { can, cannot, rules } = new AbilityBuilder(Ability); - for (const [url, permissionVerbResults] of Object.entries( - result.results - )) { - for (const [permissionVerb, hasPermission] of Object.entries( - permissionVerbResults - )) { + Object.keys(result.results).forEach((url: string) => { + const permissionVerbResults = result.results[url]; + Object.keys(permissionVerbResults).forEach((permissionVerb: string) => { + const hasPermission = permissionVerbResults[permissionVerb]; if (hasPermission) { can(permissionVerb, url); } else { cannot(permissionVerb, url); } - } - } + }); + }); ability.update(rules); }; HttpService.makeCallToBackend({ diff --git a/spiffworkflow-frontend/src/hooks/UriListForPermissions.tsx b/spiffworkflow-frontend/src/hooks/UriListForPermissions.tsx index 8491b1e9..c86f1e31 100644 --- a/spiffworkflow-frontend/src/hooks/UriListForPermissions.tsx +++ b/spiffworkflow-frontend/src/hooks/UriListForPermissions.tsx @@ -5,8 +5,10 @@ export const useUriListForPermissions = () => { const targetUris = { processGroupListPath: `/v1.0/process-groups`, processGroupShowPath: `/v1.0/process-groups/${params.process_group_id}`, - processModelListPath: `/v1.0/process-models`, + processModelCreatePath: `/v1.0/process-models/${params.process_group_id}`, processModelShowPath: `/v1.0/process-models/${params.process_model_id}`, + processModelFileCreatePath: `/v1.0/process-models/${params.process_model_id}/files`, + processModelFileShowPath: `/v1.0/process-models/${params.process_model_id}/files/${params.file_name}`, processInstanceListPath: `/v1.0/process-instances`, processInstanceActionPath: `/v1.0/process-models/${params.process_model_id}/process-instances`, }; diff --git a/spiffworkflow-frontend/src/routes/ProcessGroupShow.tsx b/spiffworkflow-frontend/src/routes/ProcessGroupShow.tsx index 932ab713..7bcda61b 100644 --- a/spiffworkflow-frontend/src/routes/ProcessGroupShow.tsx +++ b/spiffworkflow-frontend/src/routes/ProcessGroupShow.tsx @@ -35,6 +35,8 @@ export default function ProcessGroupShow() { const { targetUris } = useUriListForPermissions(); const permissionRequestData: PermissionsToCheck = { [targetUris.processGroupListPath]: ['POST'], + [targetUris.processGroupShowPath]: ['PUT'], + [targetUris.processModelCreatePath]: ['POST'], }; const { ability } = usePermissionFetcher(permissionRequestData); @@ -159,23 +161,29 @@ export default function ProcessGroupShow() { - - + + + + +

diff --git a/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx b/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx index c09dbbc2..e702fc86 100644 --- a/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx +++ b/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx @@ -106,9 +106,10 @@ export default function ProcessModelShow() { const { targetUris } = useUriListForPermissions(); const permissionRequestData: PermissionsToCheck = { - [targetUris.processModelShowPath]: ['GET', 'PUT'], + [targetUris.processModelShowPath]: ['PUT'], [targetUris.processInstanceListPath]: ['GET'], [targetUris.processInstanceActionPath]: ['POST'], + [targetUris.processModelFileCreatePath]: ['POST', 'GET', 'DELETE'], }; const { ability } = usePermissionFetcher(permissionRequestData); @@ -263,50 +264,62 @@ export default function ProcessModelShow() { ) => { const elements = []; elements.push( - - - - - - -
+ + + + + + + + +
+ {processModelFileList()} From 93d9aa845f85e89b0b7dcac99bc5167b90fe801a Mon Sep 17 00:00:00 2001 From: jasquat Date: Tue, 15 Nov 2022 17:38:37 -0500 Subject: [PATCH 2/2] pyl w/ burnettk --- .../services/process_instance_processor.py | 12 ++++++------ .../unit/test_spec_file_service.py | 3 +-- 2 files changed, 7 insertions(+), 8 deletions(-) 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 440b8b55..be32a2f0 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py @@ -701,11 +701,11 @@ class ProcessInstanceProcessor: "bpmn_file_full_path_from_bpmn_process_identifier: bpmn_process_identifier is unexpectedly None" ) - spec_reference = SpecReferenceCache.query.filter_by( - identifier=bpmn_process_identifier - ).filter_by( - type='process' - ).first() + spec_reference = ( + SpecReferenceCache.query.filter_by(identifier=bpmn_process_identifier) + .filter_by(type="process") + .first() + ) bpmn_file_full_path = None if spec_reference is None: bpmn_file_full_path = ( @@ -1021,7 +1021,7 @@ class ProcessInstanceProcessor: spiff_logger = logging.getLogger("spiff") for handler in spiff_logger.handlers: if hasattr(handler, "bulk_insert_logs"): - handler.bulk_insert_logs() # type: ignoreidentifier + handler.bulk_insert_logs() # type: ignore db.session.commit() except WorkflowTaskExecException as we: diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_spec_file_service.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_spec_file_service.py index 81db1171..9f5c5f8a 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_spec_file_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_spec_file_service.py @@ -126,8 +126,7 @@ class TestSpecFileService(BaseTest): with_db_and_bpmn_file_cleanup: None, with_super_admin_user: UserModel, ) -> None: - """When a BPMN processes identifier is changed in a file, the old id - is removed from the cache.""" + """When a BPMN processes identifier is changed in a file, the old id is removed from the cache.""" old_identifier = "ye_old_identifier" process_id_lookup = SpecReferenceCache( identifier=old_identifier,