From f93a64f126669864fec35d7e306b0750000d139d Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 30 Dec 2022 15:05:22 -0500 Subject: [PATCH 1/3] moved error display to own component w/ burnettk --- spiffworkflow-frontend/src/App.tsx | 93 ++++++++++--------- .../src/components/ErrorDisplay.tsx | 55 +++++++++++ .../src/hooks/PermissionService.tsx | 10 +- .../src/routes/ProcessModelShow.tsx | 26 ++++-- 4 files changed, 129 insertions(+), 55 deletions(-) create mode 100644 spiffworkflow-frontend/src/components/ErrorDisplay.tsx diff --git a/spiffworkflow-frontend/src/App.tsx b/spiffworkflow-frontend/src/App.tsx index 73489083..23eefa44 100644 --- a/spiffworkflow-frontend/src/App.tsx +++ b/spiffworkflow-frontend/src/App.tsx @@ -15,6 +15,7 @@ import { ErrorForDisplay } from './interfaces'; import { AbilityContext } from './contexts/Can'; import UserService from './services/UserService'; import { Notification } from './components/Notification'; +import ErrorDisplay from './components/ErrorDisplay'; export default function App() { const [errorObject, setErrorObject] = useState(null); @@ -31,51 +32,51 @@ export default function App() { const ability = defineAbility(() => {}); - let errorTag = null; - if (errorObject) { - let sentryLinkTag = null; - if (errorObject.sentry_link) { - sentryLinkTag = ( - - { - ': Find details about this error here (it may take a moment to become available): ' - } - - {errorObject.sentry_link} - - - ); - } - - let message =
{errorObject.message}
; - let title = 'Error:'; - if ('task_name' in errorObject && errorObject.task_name) { - title = `Error in python script:`; - message = ( - <> -
-
- Task: {errorObject.task_name} ({errorObject.task_id}) -
-
File name: {errorObject.file_name}
-
Line number in script task: {errorObject.line_number}
-
-
{errorObject.message}
- - ); - } - - errorTag = ( - setErrorObject(null)} - type="error" - > - {message} - {sentryLinkTag} - - ); - } + // let errorTag = null; + // if (errorObject) { + // let sentryLinkTag = null; + // if (errorObject.sentry_link) { + // sentryLinkTag = ( + // + // { + // ': Find details about this error here (it may take a moment to become available): ' + // } + // + // {errorObject.sentry_link} + // + // + // ); + // } + // + // let message =
{errorObject.message}
; + // let title = 'Error:'; + // if ('task_name' in errorObject && errorObject.task_name) { + // title = 'Error in python script:'; + // message = ( + // <> + //
+ //
+ // Task: {errorObject.task_name} ({errorObject.task_id}) + //
+ //
File name: {errorObject.file_name}
+ //
Line number in script task: {errorObject.line_number}
+ //
+ //
{errorObject.message}
+ // + // ); + // } + // + // errorTag = ( + // setErrorObject(null)} + // type="error" + // > + // {message} + // {sentryLinkTag} + // + // ); + // } return (
@@ -85,7 +86,7 @@ export default function App() { - {errorTag} + } /> diff --git a/spiffworkflow-frontend/src/components/ErrorDisplay.tsx b/spiffworkflow-frontend/src/components/ErrorDisplay.tsx new file mode 100644 index 00000000..ca0a343a --- /dev/null +++ b/spiffworkflow-frontend/src/components/ErrorDisplay.tsx @@ -0,0 +1,55 @@ +import { useContext } from 'react'; +import ErrorContext from 'src/contexts/ErrorContext'; +import { Notification } from './Notification'; + +export default function ErrorDisplay() { + const [errorObject, setErrorObject] = (useContext as any)(ErrorContext); + + let errorTag = null; + if (errorObject) { + let sentryLinkTag = null; + if (errorObject.sentry_link) { + sentryLinkTag = ( + + { + ': Find details about this error here (it may take a moment to become available): ' + } + + {errorObject.sentry_link} + + + ); + } + + let message =
{errorObject.message}
; + let title = 'Error:'; + if ('task_name' in errorObject && errorObject.task_name) { + title = 'Error in python script:'; + message = ( + <> +
+
+ Task: {errorObject.task_name} ({errorObject.task_id}) +
+
File name: {errorObject.file_name}
+
Line number in script task: {errorObject.line_number}
+
+
{errorObject.message}
+ + ); + } + + errorTag = ( + setErrorObject(null)} + type="error" + > + {message} + {sentryLinkTag} + + ); + } + + return errorTag; +} diff --git a/spiffworkflow-frontend/src/hooks/PermissionService.tsx b/spiffworkflow-frontend/src/hooks/PermissionService.tsx index fad496a7..f7ca4e4b 100644 --- a/spiffworkflow-frontend/src/hooks/PermissionService.tsx +++ b/spiffworkflow-frontend/src/hooks/PermissionService.tsx @@ -35,6 +35,14 @@ export const usePermissionFetcher = ( } }); ability.update(rules); + console.log('SETTING PERMISSIONS'); + const thePERMMap = (ability as any).j; + console.log( + 'thePERMMAP', + thePERMMap.get( + '/v1.0/process-models/misc:category_number_one:workflow_one/files' + ) + ); setPermissionsLoaded(true); }; if (Object.keys(permissionsToCheck).length !== 0) { @@ -47,5 +55,5 @@ export const usePermissionFetcher = ( } }); - return { ability, permissionsLoaded }; + return { ability, permissionsLoaded, setPermissionsLoaded }; }; diff --git a/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx b/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx index adfa203b..a547638f 100644 --- a/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx +++ b/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx @@ -73,9 +73,8 @@ export default function ProcessModelShow() { [targetUris.processInstanceCreatePath]: ['POST'], [targetUris.processModelFileCreatePath]: ['POST', 'PUT', 'GET', 'DELETE'], }; - const { ability, permissionsLoaded } = usePermissionFetcher( - permissionRequestData - ); + const { ability, permissionsLoaded, setPermissionsLoaded } = + usePermissionFetcher(permissionRequestData); const modifiedProcessModelId = modifyProcessIdentifierForPathParam( `${params.process_model_id}` @@ -234,6 +233,10 @@ export default function ProcessModelShow() { const elements = []; let icon = View; let actionWord = 'View'; + console.log( + 'targetUris.processModelFileCreatePath', + targetUris.processModelFileCreatePath + ); if (ability.can('PUT', targetUris.processModelFileCreatePath)) { icon = Edit; actionWord = 'Edit'; @@ -306,6 +309,17 @@ export default function ProcessModelShow() { if (!processModel || !permissionsLoaded) { return null; } + const permLoad = JSON.stringify(permissionsLoaded); + console.log('permLoad', permLoad); + const theMap = (ability as any).j; + // console.log('theMap', theMap[targetUris.processModelFileCreatePath]); + // console.log('theMap', theMap); + console.log( + 'theMap', + theMap.get( + '/v1.0/process-models/misc:category_number_one:workflow_one/files' + ) + ); let constructedTag; const tags = processModel.files.map((processModelFile: ProcessFile) => { const isPrimaryBpmnFile = @@ -327,11 +341,7 @@ export default function ProcessModelShow() { let fileLink = null; const fileUrl = profileModelFileEditUrl(processModelFile); if (fileUrl) { - if (ability.can('GET', targetUris.processModelFileCreatePath)) { - fileLink = {processModelFile.name}; - } else { - fileLink = {processModelFile.name}; - } + fileLink = {processModelFile.name}; } constructedTag = ( From 1354e9731bb1b1f9d29c3ec11bf45805ef7906f1 Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 30 Dec 2022 15:30:32 -0500 Subject: [PATCH 2/3] cleaned up some debug code w/ burnettk --- spiffworkflow-frontend/src/App.tsx | 47 ------------------- .../src/components/ErrorDisplay.tsx | 2 +- .../src/hooks/PermissionService.tsx | 10 +--- .../src/routes/ProcessModelShow.tsx | 29 +++++------- 4 files changed, 14 insertions(+), 74 deletions(-) diff --git a/spiffworkflow-frontend/src/App.tsx b/spiffworkflow-frontend/src/App.tsx index 23eefa44..ecf9fc54 100644 --- a/spiffworkflow-frontend/src/App.tsx +++ b/spiffworkflow-frontend/src/App.tsx @@ -14,7 +14,6 @@ import { ErrorForDisplay } from './interfaces'; import { AbilityContext } from './contexts/Can'; import UserService from './services/UserService'; -import { Notification } from './components/Notification'; import ErrorDisplay from './components/ErrorDisplay'; export default function App() { @@ -32,52 +31,6 @@ export default function App() { const ability = defineAbility(() => {}); - // let errorTag = null; - // if (errorObject) { - // let sentryLinkTag = null; - // if (errorObject.sentry_link) { - // sentryLinkTag = ( - // - // { - // ': Find details about this error here (it may take a moment to become available): ' - // } - // - // {errorObject.sentry_link} - // - // - // ); - // } - // - // let message =
{errorObject.message}
; - // let title = 'Error:'; - // if ('task_name' in errorObject && errorObject.task_name) { - // title = 'Error in python script:'; - // message = ( - // <> - //
- //
- // Task: {errorObject.task_name} ({errorObject.task_id}) - //
- //
File name: {errorObject.file_name}
- //
Line number in script task: {errorObject.line_number}
- //
- //
{errorObject.message}
- // - // ); - // } - // - // errorTag = ( - // setErrorObject(null)} - // type="error" - // > - // {message} - // {sentryLinkTag} - // - // ); - // } - return (
{/* @ts-ignore */} diff --git a/spiffworkflow-frontend/src/components/ErrorDisplay.tsx b/spiffworkflow-frontend/src/components/ErrorDisplay.tsx index ca0a343a..cdbed75a 100644 --- a/spiffworkflow-frontend/src/components/ErrorDisplay.tsx +++ b/spiffworkflow-frontend/src/components/ErrorDisplay.tsx @@ -1,5 +1,5 @@ import { useContext } from 'react'; -import ErrorContext from 'src/contexts/ErrorContext'; +import ErrorContext from '../contexts/ErrorContext'; import { Notification } from './Notification'; export default function ErrorDisplay() { diff --git a/spiffworkflow-frontend/src/hooks/PermissionService.tsx b/spiffworkflow-frontend/src/hooks/PermissionService.tsx index f7ca4e4b..fad496a7 100644 --- a/spiffworkflow-frontend/src/hooks/PermissionService.tsx +++ b/spiffworkflow-frontend/src/hooks/PermissionService.tsx @@ -35,14 +35,6 @@ export const usePermissionFetcher = ( } }); ability.update(rules); - console.log('SETTING PERMISSIONS'); - const thePERMMap = (ability as any).j; - console.log( - 'thePERMMAP', - thePERMMap.get( - '/v1.0/process-models/misc:category_number_one:workflow_one/files' - ) - ); setPermissionsLoaded(true); }; if (Object.keys(permissionsToCheck).length !== 0) { @@ -55,5 +47,5 @@ export const usePermissionFetcher = ( } }); - return { ability, permissionsLoaded, setPermissionsLoaded }; + return { ability, permissionsLoaded }; }; diff --git a/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx b/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx index a547638f..461c6d84 100644 --- a/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx +++ b/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx @@ -73,8 +73,9 @@ export default function ProcessModelShow() { [targetUris.processInstanceCreatePath]: ['POST'], [targetUris.processModelFileCreatePath]: ['POST', 'PUT', 'GET', 'DELETE'], }; - const { ability, permissionsLoaded, setPermissionsLoaded } = - usePermissionFetcher(permissionRequestData); + const { ability, permissionsLoaded } = usePermissionFetcher( + permissionRequestData + ); const modifiedProcessModelId = modifyProcessIdentifierForPathParam( `${params.process_model_id}` @@ -231,12 +232,17 @@ export default function ProcessModelShow() { isPrimaryBpmnFile: boolean ) => { const elements = []; + + // So there is a bug in here. Since we use a react context for error messages, and since + // its provider wraps the entire app, child components will re-render when there is an + // error displayed. This is normally fine, but it interacts badly with the casl ability.can + // functionality. We have observed that permissionsLoaded is never set to false. So when + // you run a process and it fails, for example, process model show will re-render, the ability + // will be cleared out and it will start fetching permissions from the server, but this + // component still thinks permissionsLoaded is telling the truth (it says true, but it's actually false). + // The only bad effect that we know of is that the Edit icon becomes an eye icon even for admins. let icon = View; let actionWord = 'View'; - console.log( - 'targetUris.processModelFileCreatePath', - targetUris.processModelFileCreatePath - ); if (ability.can('PUT', targetUris.processModelFileCreatePath)) { icon = Edit; actionWord = 'Edit'; @@ -309,17 +315,6 @@ export default function ProcessModelShow() { if (!processModel || !permissionsLoaded) { return null; } - const permLoad = JSON.stringify(permissionsLoaded); - console.log('permLoad', permLoad); - const theMap = (ability as any).j; - // console.log('theMap', theMap[targetUris.processModelFileCreatePath]); - // console.log('theMap', theMap); - console.log( - 'theMap', - theMap.get( - '/v1.0/process-models/misc:category_number_one:workflow_one/files' - ) - ); let constructedTag; const tags = processModel.files.map((processModelFile: ProcessFile) => { const isPrimaryBpmnFile = From 86eced9d6201302d88472846cebee7f1179926a2 Mon Sep 17 00:00:00 2001 From: jasquat Date: Fri, 30 Dec 2022 15:35:16 -0500 Subject: [PATCH 3/3] do not set baseUrl since it breaks auto-import and is not used otherwise w/ burnettk --- spiffworkflow-frontend/tsconfig.json | 1 - 1 file changed, 1 deletion(-) diff --git a/spiffworkflow-frontend/tsconfig.json b/spiffworkflow-frontend/tsconfig.json index f3d7e4a4..87997aba 100644 --- a/spiffworkflow-frontend/tsconfig.json +++ b/spiffworkflow-frontend/tsconfig.json @@ -1,6 +1,5 @@ { "compilerOptions": { - "baseUrl": ".", "esModuleInterop": true, "forceConsistentCasingInFileNames": true, "jsx": "react-jsx",