-
Notifications
You must be signed in to change notification settings - Fork 106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Workflow page error message #670
Merged
Assem-Uber
merged 13 commits into
uber:release/4.0.0
from
Assem-Hafez:feature/CDNC-9155/workflow-page-error-msg
Sep 20, 2024
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
ef7857e
group history events
Assem-Hafez 3fc3dda
timeline load more
Assem-Hafez b82298e
Initialize Queries UI (#665)
adhityamamallan afe7bd0
Merge remote-tracking branch 'upstream/release/4.0.0' into feature/CD…
Assem-Hafez d5f8195
get workflow rest api
Assem-Hafez e517900
add type to decaode url param
Assem-Hafez c9c9d93
timeline group component
Assem-Hafez f2c3bf0
add animations
Assem-Hafez 986cd90
lint fix
Assem-Hafez ade5c7e
error message
Assem-Hafez cdbce5e
Merge remote-tracking branch 'upstream/release/4.0.0' into feature/CD…
Assem-Hafez 5086747
use Internal error boundary
Assem-Hafez ae1c0cf
fix lint
Assem-Hafez File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
18 changes: 2 additions & 16 deletions
18
...rkflow)/domains/[domain]/[cluster]/workflows/[workflowId]/[runId]/[workflowTab]/error.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,4 @@ | ||
'use client'; | ||
import ErrorPanel from '@/components/error-panel/error-panel'; | ||
import WorkflowPageTabsError from '@/views/workflow-page/workflow-page-tabs-error/workflow-page-tabs-error'; | ||
|
||
export default function WorkflowTabsError({ | ||
error, | ||
reset, | ||
}: Readonly<{ | ||
error: Error; | ||
reset: () => void; | ||
}>) { | ||
return ( | ||
<ErrorPanel | ||
error={error} | ||
message="Failed to load workflow content" | ||
reset={reset} | ||
/> | ||
); | ||
} | ||
export default WorkflowPageTabsError; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
13 changes: 13 additions & 0 deletions
13
src/views/workflow-page/config/workflow-page-tabs-error.config.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import getWorkflowPageErrorConfig from '../helpers/get-workflow-page-error-config'; | ||
import { type WorkflowPageTabsErrorConfig } from '../workflow-page-tabs-error/workflow-page-tabs-error.types'; | ||
|
||
const workflowPageTabsErrorConfig: WorkflowPageTabsErrorConfig = { | ||
summary: (err) => | ||
getWorkflowPageErrorConfig(err, 'Failed to load workflow summary'), | ||
history: (err) => | ||
getWorkflowPageErrorConfig(err, 'Failed to load workflow history'), | ||
queries: (err) => | ||
getWorkflowPageErrorConfig(err, 'Failed to load workflow queries'), | ||
} as const; | ||
|
||
export default workflowPageTabsErrorConfig; |
24 changes: 24 additions & 0 deletions
24
src/views/workflow-page/helpers/__tests__/get-workflow-page-error-config.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import { RequestError } from '@/utils/request/request-error'; | ||
|
||
import getWorkflowPageErrorConfig from '../get-workflow-page-error-config'; | ||
|
||
describe('getWorkflowPageErrorConfig', () => { | ||
it('returns default error config for regular error', () => { | ||
expect( | ||
getWorkflowPageErrorConfig(new Error('Test error'), 'default error') | ||
).toEqual({ | ||
message: 'default error', | ||
actions: [{ kind: 'retry', label: 'Retry' }], | ||
}); | ||
}); | ||
|
||
it('returns "Workflow not found" error config when request errors with 404', () => { | ||
expect( | ||
getWorkflowPageErrorConfig(new RequestError('test error', 404)) | ||
).toEqual({ | ||
message: 'Workflow not found', | ||
omitLogging: true, | ||
actions: [{ kind: 'retry', label: 'Retry' }], | ||
}); | ||
}); | ||
}); |
20 changes: 20 additions & 0 deletions
20
src/views/workflow-page/helpers/get-workflow-page-error-config.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
import { RequestError } from '@/utils/request/request-error'; | ||
import { type DomainPageTabErrorConfig } from '@/views/domain-page/domain-page-tabs-error/domain-page-tabs-error.types'; | ||
|
||
export default function getWorkflowPageErrorConfig( | ||
err: Error, | ||
defaultErrorMessage: string = 'Failed to load workflow' | ||
): DomainPageTabErrorConfig { | ||
if (err instanceof RequestError && err.status === 404) { | ||
return { | ||
message: 'Workflow not found', | ||
actions: [{ kind: 'retry', label: 'Retry' }], | ||
omitLogging: true, | ||
}; | ||
} | ||
|
||
return { | ||
message: defaultErrorMessage, | ||
actions: [{ kind: 'retry', label: 'Retry' }], | ||
}; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
69 changes: 69 additions & 0 deletions
69
src/views/workflow-page/workflow-page-tabs-error/__tests__/workflow-page-tabs-error.test.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
import React from 'react'; | ||
|
||
import * as navigationModule from 'next/navigation'; | ||
|
||
import { render, screen } from '@/test-utils/rtl'; | ||
|
||
import WorkflowPageTabsError from '../workflow-page-tabs-error'; | ||
import type { WorkflowPageTabsErrorConfig } from '../workflow-page-tabs-error.types'; | ||
|
||
jest.mock('@/components/error-panel/error-panel', () => | ||
jest.fn(({ message }: { message: string }) => <div>{message}</div>) | ||
); | ||
|
||
jest.mock('next/navigation', () => ({ | ||
...jest.requireActual('next/navigation'), | ||
useParams: jest.fn(() => ({ | ||
domain: 'test-domain', | ||
cluster: 'test-cluster', | ||
workflowTab: 'summary', | ||
})), | ||
})); | ||
|
||
jest.mock( | ||
'../../config/workflow-page-tabs-error.config', | ||
() => | ||
({ | ||
summary: () => ({ | ||
message: 'summary error', | ||
}), | ||
history: () => ({ | ||
message: 'history error', | ||
}), | ||
queries: () => ({ | ||
message: 'queries error', | ||
}), | ||
}) as const satisfies WorkflowPageTabsErrorConfig | ||
); | ||
|
||
describe('WorkflowPageTabsError', () => { | ||
beforeEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
it('renders tab error correctly when workflow tab exists in config', () => { | ||
setup(); | ||
expect(screen.getByText('summary error')).toBeInTheDocument(); | ||
}); | ||
|
||
it('renders tab error with generic text when workflow tab does not exist in config', () => { | ||
jest.spyOn(navigationModule, 'useParams').mockReturnValue({ | ||
domain: 'test-domain', | ||
cluster: 'test-cluster', | ||
workflowTab: 'invalid', | ||
}); | ||
setup(); | ||
expect( | ||
screen.getByText('Failed to load workflow content') | ||
).toBeInTheDocument(); | ||
}); | ||
}); | ||
|
||
function setup() { | ||
render( | ||
<WorkflowPageTabsError | ||
error={new Error('something bad happened')} | ||
reset={() => {}} | ||
/> | ||
); | ||
} |
37 changes: 37 additions & 0 deletions
37
src/views/workflow-page/workflow-page-tabs-error/workflow-page-tabs-error.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import { useParams } from 'next/navigation'; | ||
|
||
import ErrorPanel from '@/components/error-panel/error-panel'; | ||
|
||
import workflowPageTabsErrorConfig from '../config/workflow-page-tabs-error.config'; | ||
import { type WorkflowPageTabName } from '../workflow-page-tab-content/workflow-page-tab-content.types'; | ||
|
||
import { type Props } from './workflow-page-tabs-error.types'; | ||
|
||
export default function WorkflowPageTabsError({ error, reset }: Props) { | ||
const { workflowTab } = useParams(); | ||
const getConfig = | ||
workflowPageTabsErrorConfig[workflowTab as WorkflowPageTabName]; | ||
|
||
if (typeof getConfig !== 'function') { | ||
return ( | ||
<div> | ||
<ErrorPanel | ||
error={error} | ||
message={'Failed to load workflow content'} | ||
reset={reset} | ||
/> | ||
</div> | ||
); | ||
} | ||
|
||
const errorConfig = getConfig(error); | ||
return ( | ||
<ErrorPanel | ||
error={error} | ||
message={errorConfig.message} | ||
actions={errorConfig.actions} | ||
omitLogging={errorConfig.omitLogging} | ||
reset={reset} | ||
/> | ||
); | ||
} |
18 changes: 18 additions & 0 deletions
18
src/views/workflow-page/workflow-page-tabs-error/workflow-page-tabs-error.types.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import { type Props as ErrorPanelProps } from '@/components/error-panel/error-panel.types'; | ||
|
||
import { type WorkflowPageTabName } from '../workflow-page-tab-content/workflow-page-tab-content.types'; | ||
|
||
export type WorkflowPageTabErrorConfig = Omit< | ||
ErrorPanelProps, | ||
'error' | 'reset' | ||
>; | ||
|
||
export type WorkflowPageTabsErrorConfig = Record< | ||
WorkflowPageTabName, | ||
(err: Error) => WorkflowPageTabErrorConfig | ||
>; | ||
|
||
export type Props = { | ||
error: Error; | ||
reset: () => void; | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're removing PageSection here, we need to add it to every workflow tab. Did you make the change in History?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, also i will be making changes later along with the virtual list to handle use cases like query page expand the whole page