Skip to content

Commit

Permalink
fix(app, odd): run summery error list bug fixes (#15953)
Browse files Browse the repository at this point in the history
# Overview

Closes [RQA-2923](https://opentrons.atlassian.net/browse/RQA-2923).
show warnings/errors depending on the run status.
modal header.
show error details on ODD if run completed with errors.

## Test Plan and Hands on Testing

- run a protocol with ER errors on ODD and desktop app.
- make sure that when the run is complete you are able to see the list
of errors.
- warnings shown when run is successfully completed, errors if canceled
or failed.

## Changelog

- inject run status to `RunFailedModal`.
- changed modal title
- changed text to show warnings/errors depending on the run status. 
- ODD - show view error details if there is an error list.

## Risk assessment

low. bug fixes.


[RQA-2923]:
https://opentrons.atlassian.net/browse/RQA-2923?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
  • Loading branch information
TamarZanzouri authored Aug 12, 2024
1 parent 09ccbbc commit b99e7c2
Show file tree
Hide file tree
Showing 9 changed files with 83 additions and 48 deletions.
10 changes: 9 additions & 1 deletion app/src/assets/localization/en/run_details.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@
"end_step_time": "End",
"error_info": "Error {{errorCode}}: {{errorType}}",
"error_type": "Error: {{errorType}}",
"error_details": "Error details",
"no_of_error": "{{count}} error",
"no_of_errors": "{{count}} errors",
"failed_step": "Failed step",
"final_step": "Final Step",
"ignore_stored_data": "Ignore stored data",
Expand Down Expand Up @@ -98,6 +101,8 @@
"run_complete": "Run completed",
"run_complete_splash": "Run completed",
"run_completed": "Run completed.",
"run_completed_with_errors": "Run completed with errors.",
"run_completed_with_warnings": "Run completed with warnings.",
"run_cta_disabled": "Complete required steps on Protocol tab before starting the run",
"run_failed": "Run failed.",
"run_failed_modal_body": "Error occurred when protocol was {{command}}",
Expand Down Expand Up @@ -144,5 +149,8 @@
"view_analysis_error_details": "View <errorLink>error details</errorLink>",
"view_current_step": "View current step",
"view_error": "View error",
"view_error_details": "View error details"
"view_error_details": "View error details",
"warning_details": "Warning details",
"no_of_warning": "{{count}} warning",
"no_of_warnings": "{{count}} warnings"
}
1 change: 0 additions & 1 deletion app/src/molecules/Modal/ModalHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export function ModalHeader(props: ModalHeaderBaseProps): JSX.Element {
color={iconColor}
size="2rem"
alignSelf={ALIGN_CENTER}
marginRight={SPACING.spacing16}
/>
) : null}
<LegacyStyledText
Expand Down
22 changes: 15 additions & 7 deletions app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ export function ProtocolRunHeader({
setShowRunFailedModal={setShowRunFailedModal}
highestPriorityError={highestPriorityError}
commandErrorList={commandErrorList}
runStatus={runStatus}
/>
) : null}
<Flex
Expand Down Expand Up @@ -906,7 +907,8 @@ function TerminalRunBanner(props: TerminalRunProps): JSX.Element | null {
isRunCurrent,
} = props
const { t } = useTranslation('run_details')

const completedWithErrors =
commandErrorList?.data != null && commandErrorList.data.length > 0
const handleRunSuccessClick = (): void => {
handleClearClick()
}
Expand All @@ -933,15 +935,22 @@ function TerminalRunBanner(props: TerminalRunProps): JSX.Element | null {

const buildErrorBanner = (): JSX.Element => {
return (
<Banner type="error" iconMarginLeft={SPACING.spacing4}>
<Banner
type={runStatus === RUN_STATUS_SUCCEEDED ? 'warning' : 'error'}
iconMarginLeft={SPACING.spacing4}
>
<Flex justifyContent={JUSTIFY_SPACE_BETWEEN} width="100%">
<LegacyStyledText>
{highestPriorityError != null
? t('error_info', {
errorType: highestPriorityError?.errorType,
errorCode: highestPriorityError?.errorCode,
})
: 'Run completed with errors.'}
: `${
runStatus === RUN_STATUS_SUCCEEDED
? t('run_completed_with_warnings')
: t('run_completed_with_errors')
}`}
</LegacyStyledText>

<LinkButton
Expand All @@ -958,14 +967,13 @@ function TerminalRunBanner(props: TerminalRunProps): JSX.Element | null {
if (
runStatus === RUN_STATUS_SUCCEEDED &&
isRunCurrent &&
!isResetRunLoading
!isResetRunLoading &&
!completedWithErrors
) {
return buildSuccessBanner()
} else if (
highestPriorityError != null ||
(commandErrorList?.data != null &&
commandErrorList.data.length > 0 &&
!isResetRunLoading)
(completedWithErrors && !isResetRunLoading)
) {
return buildErrorBanner()
} else {
Expand Down
26 changes: 22 additions & 4 deletions app/src/organisms/Devices/ProtocolRun/RunFailedModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ import {
import { LegacyModal } from '../../../molecules/LegacyModal'
import { useDownloadRunLog } from '../hooks'

import type { RunError, RunCommandErrors } from '@opentrons/api-client'
import type {
RunError,
RunCommandErrors,
RunStatus,
} from '@opentrons/api-client'
import { RUN_STATUS_SUCCEEDED } from '@opentrons/api-client'
import type { LegacyModalProps } from '../../../molecules/LegacyModal'
import type { RunCommandError } from '@opentrons/shared-data'

Expand All @@ -45,6 +50,7 @@ interface RunFailedModalProps {
setShowRunFailedModal: (showRunFailedModal: boolean) => void
highestPriorityError?: RunError | null
commandErrorList?: RunCommandErrors | null
runStatus: RunStatus | null
}

export function RunFailedModal({
Expand All @@ -53,11 +59,17 @@ export function RunFailedModal({
setShowRunFailedModal,
highestPriorityError,
commandErrorList,
runStatus,
}: RunFailedModalProps): JSX.Element | null {
const { i18n, t } = useTranslation(['run_details', 'shared', 'branded'])
const modalProps: LegacyModalProps = {
type: 'error',
title: t('run_failed_modal_title'),
type: runStatus === RUN_STATUS_SUCCEEDED ? 'warning' : 'error',
title:
commandErrorList == null || commandErrorList?.data.length === 0
? t('run_failed_modal_title')
: runStatus === RUN_STATUS_SUCCEEDED
? t('warning_details')
: t('error_details'),
onClose: () => {
setShowRunFailedModal(false)
},
Expand Down Expand Up @@ -95,7 +107,13 @@ export function RunFailedModal({
errorType: errors[0].errorType,
errorCode: errors[0].errorCode,
})
: `${errors.length} errors`}
: runStatus === RUN_STATUS_SUCCEEDED
? t(errors.length > 1 ? 'no_of_warnings' : 'no_of_warning', {
count: errors.length,
})
: t(errors.length > 1 ? 'no_of_errors' : 'no_of_error', {
count: errors.length,
})}
</LegacyStyledText>
<Flex css={ERROR_MESSAGE_STYLE}>
{' '}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -855,18 +855,7 @@ describe('ProtocolRunHeader', () => {
.thenReturn(RUN_STATUS_SUCCEEDED)
render()

screen.getByText('Run completed.')
})
it('clicking close on a terminal run banner closes the run context', async () => {
vi.mocked(useNotifyRunQuery).mockReturnValue({
data: { data: mockSucceededRun },
} as UseQueryResult<OpentronsApiClient.Run>)
when(vi.mocked(useRunStatus))
.calledWith(RUN_ID)
.thenReturn(RUN_STATUS_SUCCEEDED)
render()

fireEvent.click(screen.queryAllByTestId('Banner_close-button')[0])
screen.getByText('Run completed with warnings.')
})

it('does not display the "run successful" banner if the successful run is not current', async () => {
Expand Down Expand Up @@ -981,22 +970,6 @@ describe('ProtocolRunHeader', () => {
})
})

it('renders banner with spinner if currently closing current run', async () => {
vi.mocked(useNotifyRunQuery).mockReturnValue({
data: { data: mockSucceededRun },
} as UseQueryResult<OpentronsApiClient.Run>)
when(vi.mocked(useRunStatus))
.calledWith(RUN_ID)
.thenReturn(RUN_STATUS_SUCCEEDED)
when(vi.mocked(useCloseCurrentRun)).calledWith().thenReturn({
isClosingCurrentRun: true,
closeCurrentRun: mockCloseCurrentRun,
})
render()
screen.getByText('Run completed.')
screen.getByLabelText('ot-spinner')
})

it('renders door close banner when the robot door is open', () => {
const mockOpenDoorStatus = {
data: { status: 'open', doorRequiredClosedForProtocol: true },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { i18n } from '../../../../i18n'
import { useDownloadRunLog } from '../../hooks'
import { RunFailedModal } from '../RunFailedModal'

import { RUN_STATUS_FAILED } from '@opentrons/api-client'
import type { RunError } from '@opentrons/api-client'
import { fireEvent, screen } from '@testing-library/react'

Expand Down Expand Up @@ -39,6 +40,7 @@ describe('RunFailedModal - DesktopApp', () => {
runId: RUN_ID,
setShowRunFailedModal: vi.fn(),
highestPriorityError: mockError,
runStatus: RUN_STATUS_FAILED,
}
vi.mocked(useDownloadRunLog).mockReturnValue({
downloadRunLog: vi.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ import { SmallButton } from '../../../atoms/buttons'
import { Modal } from '../../../molecules/Modal'

import type { ModalHeaderBaseProps } from '../../../molecules/Modal/types'
import type { RunCommandErrors, RunError } from '@opentrons/api-client'
import type {
RunCommandErrors,
RunError,
RunStatus,
} from '@opentrons/api-client'
import { RUN_STATUS_SUCCEEDED } from '@opentrons/api-client'

import type { RunCommandError } from '@opentrons/shared-data'

Expand All @@ -29,13 +34,15 @@ interface RunFailedModalProps {
setShowRunFailedModal: (showRunFailedModal: boolean) => void
errors?: RunError[]
commandErrorList?: RunCommandErrors
runStatus: RunStatus | null
}

export function RunFailedModal({
runId,
setShowRunFailedModal,
errors,
commandErrorList,
runStatus,
}: RunFailedModalProps): JSX.Element | null {
const { t, i18n } = useTranslation(['run_details', 'shared', 'branded'])
const navigate = useNavigate()
Expand All @@ -48,7 +55,12 @@ export function RunFailedModal({
)
return null
const modalHeader: ModalHeaderBaseProps = {
title: t('run_failed_modal_title'),
title:
commandErrorList == null || commandErrorList?.data.length === 0
? t('run_failed_modal_title')
: runStatus === RUN_STATUS_SUCCEEDED
? t('warning_details')
: t('error_details'),
}

const highestPriorityError = getHighestPriorityError(errors ?? [])
Expand Down Expand Up @@ -85,7 +97,13 @@ export function RunFailedModal({
errorType: errors[0].errorType,
errorCode: errors[0].errorCode,
})
: `${errors.length} errors`}
: runStatus === RUN_STATUS_SUCCEEDED
? t(errors.length > 1 ? 'no_of_warnings' : 'no_of_warning', {
count: errors.length,
})
: t(errors.length > 1 ? 'no_of_errors' : 'no_of_error', {
count: errors.length,
})}
</LegacyStyledText>
<Flex
width="100%"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { i18n } from '../../../../i18n'
import { RunFailedModal } from '../RunFailedModal'

import type { NavigateFunction } from 'react-router-dom'
import { RUN_STATUS_FAILED } from '@opentrons/api-client'

vi.mock('@opentrons/react-api-client')

Expand Down Expand Up @@ -100,6 +101,7 @@ describe('RunFailedModal', () => {
runId: RUN_ID,
setShowRunFailedModal: mockFn,
errors: mockErrors,
runStatus: RUN_STATUS_FAILED,
}

vi.mocked(useStopRunMutation).mockReturnValue({
Expand Down
15 changes: 11 additions & 4 deletions app/src/pages/RunSummary/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,10 @@ export function RunSummary(): JSX.Element {
isRunCurrent,
})

let headerText = t('run_complete_splash')
let headerText =
commandErrorList != null && commandErrorList.data.length > 0
? t('run_completed_with_warnings')
: t('run_completed_splash')
if (runStatus === RUN_STATUS_FAILED) {
headerText = t('run_failed_splash')
} else if (runStatus === RUN_STATUS_STOPPED) {
Expand Down Expand Up @@ -347,6 +350,7 @@ export function RunSummary(): JSX.Element {
setShowRunFailedModal={setShowRunFailedModal}
errors={runRecord?.data.errors}
commandErrorList={commandErrorList}
runStatus={runStatus}
/>
) : null}
<Flex
Expand Down Expand Up @@ -414,7 +418,8 @@ export function RunSummary(): JSX.Element {
height="17rem"
css={showRunAgainSpinner ? RUN_AGAIN_CLICKED_STYLE : undefined}
/>
{!didRunSucceed ? (
{(commandErrorList != null && commandErrorList?.data.length > 0) ||
!didRunSucceed ? (
<LargeButton
flex="1"
iconName="info"
Expand All @@ -423,8 +428,10 @@ export function RunSummary(): JSX.Element {
buttonText={t('view_error_details')}
height="17rem"
disabled={
runRecord?.data.errors == null ||
runRecord?.data.errors.length === 0
(runRecord?.data.errors == null ||
runRecord?.data.errors.length === 0) &&
(commandErrorList == null ||
commandErrorList?.data.length === 0)
}
/>
) : null}
Expand Down

0 comments on commit b99e7c2

Please sign in to comment.