Skip to content
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.

Sign up for GitHub

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

fix(app): fix view error details does nothing #12732

Merged
merged 5 commits into from
May 22, 2023
Merged

fix(app): fix view error details does nothing #12732

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dbaf4ec
fix(app): fix view error details does nothing
koji May 17, 2023
1524f53
remove the red frame
koji May 18, 2023
fdb6af9
fix content allignment
koji May 19, 2023
19ef618
fix linting error
koji May 19, 2023
109bdeb
display first error type
shlokamin May 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/src/assets/localization/en/run_details.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"comment_step": "Comment",
"comment": "Comment",
"complete_protocol_to_download": "Complete the protocol to download the run log",
"contact_information": "Please contact [email protected] with relevant information for assistance with troubleshooting.",
"current_step_pause_timer": "Timer",
"current_step_pause": "Current Step - Paused by User",
"current_step": "Current Step",
Expand All @@ -32,6 +33,7 @@
"end_of_protocol": "End of protocol",
"end_step_time": "End",
"end": "End",
"error_type": "Error: {{errorType}}",
"failed_step": "Failed step",
"ignore_stored_data": "Ignore stored data",
"labware_offset_data": "labware offset data",
Expand Down
100 changes: 26 additions & 74 deletions app/src/organisms/OnDeviceDisplay/RunningProtocol/RunFailedModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@ import {
COLORS,
TYPOGRAPHY,
DIRECTION_COLUMN,
BORDERS,
} from '@opentrons/components'
import { useStopRunMutation } from '@opentrons/react-api-client'
import { RunTimeCommand } from '@opentrons/shared-data'

import { StyledText } from '../../../atoms/text'
import { SmallButton } from '../../../atoms/buttons'
Expand All @@ -29,17 +27,12 @@ interface RunError {
interface RunFailedModalProps {
runId: string
setShowRunFailedModal: (showRunFailedModal: boolean) => void
failedStep?: number
failedCommand?: RunTimeCommand
errors?: RunError[]
}

// ToDo (kj:05/03/2023) This component is needed to refactor to handle error messages
export function RunFailedModal({
runId,
setShowRunFailedModal,
failedStep,
failedCommand,
errors,
}: RunFailedModalProps): JSX.Element | null {
const { t, i18n } = useTranslation(['run_details', 'shared'])
Expand All @@ -54,12 +47,6 @@ export function RunFailedModal({
iconColor: COLORS.white,
}

// Note (kj:04/12/2023) Error code hasn't been defined yet
// for now we use run's errors data
const errorName = errors[0].errorType
const errorCode = 'error-1000'
const errorMessages = errors.map((error: RunError) => error.detail)

const handleClose = (): void => {
setIsCanceling(true)
setShowRunFailedModal(false)
Expand All @@ -78,76 +65,41 @@ export function RunFailedModal({
return (
<Modal
header={modalHeader}
modalSize="large"
isError
onOutsideClick={() => setShowRunFailedModal(false)}
>
<Flex
flexDirection={DIRECTION_COLUMN}
gridGap={SPACING.spacing16}
marginTop={SPACING.spacing32}
gridGap={SPACING.spacing40}
padding={SPACING.spacing32}
>
<StyledText
fontSize={TYPOGRAPHY.fontSize22}
lineHeight={TYPOGRAPHY.lineHeight28}
fontWeight={TYPOGRAPHY.fontWeightBold}
>
{t('run_failed_modal_header', {
errorName: errorName,
errorCode: errorCode,
count: failedStep,
})}
</StyledText>
<StyledText
fontSize={TYPOGRAPHY.fontSize22}
lineHeight={TYPOGRAPHY.lineHeight28}
fontWeight={TYPOGRAPHY.fontWeightRegular}
>
{/* This will be added when we get a new error system */}
{'Error message'}
</StyledText>
<Flex
flexDirection={DIRECTION_COLUMN}
backgroundColor={COLORS.light1}
borderRadius={BORDERS.size3}
gridGap={SPACING.spacing8}
padding={SPACING.spacing16}
overflowY="scroll"
maxHeight="7.75rem"
>
<StyledText
fontSize={TYPOGRAPHY.fontSize20}
lineHeight={TYPOGRAPHY.lineHeight24}
fontWeight={TYPOGRAPHY.fontWeightSemiBold}
>
{t('run_failed_modal_body', {
command: failedCommand,
<Flex flexDirection={DIRECTION_COLUMN} gridGap={SPACING.spacing16}>
<StyledText as="p" fontWeight={TYPOGRAPHY.fontWeightBold}>
{/* (kj:05/17/2023) errorType will be added when the endpoint is ready */}
{t('error_type', {
errorType: '',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errorType is already returned from the runs endpoint right? we should already have it inside of the errorType prop in RunError:

interface RunError {
  id: string
  errorType: string
  createdAt: string
  detail: string
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it doesn't represented the entire error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the Desktop app, errorType isn't used but just parse errors[] and display details.

For this modal errorType came from a new error endpoint. Actually, I posted that current errorType that we can display doesn't represent the entire errors. But the design hasn't been update.

We can display errorType from the errors[0].details but I guess that may cause confusion because if there are more than 2 error details, a couple of errors might not related to the errorType completely.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i believe the only data we might get that dosent yet exist is the error code (cc @fsinapi).

let's sync on this during standup and try to get this merged before the afternoon

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the backend won't ever give us a top level error type — global error state was cut from scope. i updated the component to show the first error type in the list of errors for now. just verified with @sanni-t that this should give users sufficient info

})}
</StyledText>
<StyledText
fontSize={TYPOGRAPHY.fontSize20}
lineHeight={TYPOGRAPHY.lineHeight24}
fontWeight={TYPOGRAPHY.fontWeightSemiBold}
<Flex
flexDirection={DIRECTION_COLUMN}
gridGap={SPACING.spacing8}
overflowY="scroll"
maxHeight="5.375rem"
>
{errorMessages}
</StyledText>
</Flex>
<StyledText
fontSize={TYPOGRAPHY.fontSize22}
lineHeight={TYPOGRAPHY.lineHeight28}
fontWeight={TYPOGRAPHY.fontWeightRegular}
>
{t('run_failed_modal_description')}
</StyledText>
<Flex marginTop="1.75rem">
<SmallButton
width="100%"
buttonType="alert"
buttonText={i18n.format(t('shared:close'), 'titleCase')}
onClick={handleClose}
disabled={isCanceling}
/>
{errors.map(error => (
<StyledText as="p" key={error.id}>
{error.detail}
</StyledText>
))}
<StyledText as="p">{t('contact_information')}</StyledText>
</Flex>
</Flex>
<SmallButton
width="100%"
buttonType="alert"
buttonText={i18n.format(t('shared:close'), 'capitalize')}
onClick={handleClose}
disabled={isCanceling}
/>
</Flex>
</Modal>
)
Expand Down
2 changes: 0 additions & 2 deletions app/src/organisms/OnDeviceDisplay/RunningProtocol/__tests__/RunFailedModal.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ describe('RunFailedModal', () => {
props = {
runId: RUN_ID,
setShowRunFailedModal: mockFn,
failedStep: 1,
errors: mockErrors,
}
mockStopRun = jest.fn((_runId, opts) => opts.onSuccess())
Expand All @@ -64,7 +63,6 @@ describe('RunFailedModal', () => {
it('should render text and button', () => {
const [{ getByText }] = render(props)
getByText('Run failed')
getByText('ExceptionInProtocolError: error-1000 at protocol step 1')
getByText(
'ProtocolEngineError [line 16]: ModuleNotAttachedError: No available'
)
Expand Down
14 changes: 12 additions & 2 deletions app/src/pages/OnDeviceDisplay/RunSummary.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import {
ANALYTICS_PROTOCOL_RUN_AGAIN,
ANALYTICS_PROTOCOL_RUN_FINISH,
} from '../../redux/analytics'
import { RunFailedModal } from '../../organisms/OnDeviceDisplay/RunningProtocol'

import type { Run } from '@opentrons/api-client'
import type { OnDeviceRouteParams } from '../../App/types'
Expand Down Expand Up @@ -89,6 +90,9 @@ export function RunSummary(): JSX.Element {
const localRobot = useSelector(getLocalRobot)
const robotName = localRobot?.name ?? 'no name'
const robotAnalyticsData = useRobotAnalyticsData(robotName)
const [showRunFailedModal, setShowRunFailedModal] = React.useState<boolean>(
false
)

const runStatusText = isRunSucceeded
? t('run_complete')
Expand All @@ -109,8 +113,7 @@ export function RunSummary(): JSX.Element {
}

const handleViewErrorDetails = (): void => {
// Note (kj:04/28/2023) the current RunFailedModal is needed to refactor before hooking up
console.log('will be added')
setShowRunFailedModal(true)
}

const handleClickSplash = (): void => {
Expand Down Expand Up @@ -167,6 +170,13 @@ export function RunSummary(): JSX.Element {
justifyContent={JUSTIFY_SPACE_BETWEEN}
padding={SPACING.spacing40}
>
{showRunFailedModal ? (
<RunFailedModal
runId={runId}
setShowRunFailedModal={setShowRunFailedModal}
errors={runRecord?.data.errors}
/>
) : null}
<Flex
flexDirection={DIRECTION_COLUMN}
alignItems={ALIGN_FLEX_START}
Expand Down