From b99e7c2fdfbff7d02a768444867a184c51f37c73 Mon Sep 17 00:00:00 2001 From: TamarZanzouri Date: Mon, 12 Aug 2024 14:02:41 -0400 Subject: [PATCH] fix(app, odd): run summery error list bug fixes (#15953) # 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 --- .../assets/localization/en/run_details.json | 10 ++++++- app/src/molecules/Modal/ModalHeader.tsx | 1 - .../Devices/ProtocolRun/ProtocolRunHeader.tsx | 22 +++++++++----- .../Devices/ProtocolRun/RunFailedModal.tsx | 26 ++++++++++++++--- .../__tests__/ProtocolRunHeader.test.tsx | 29 +------------------ .../__tests__/RunFailedModal.test.tsx | 2 ++ .../RunningProtocol/RunFailedModal.tsx | 24 +++++++++++++-- .../__tests__/RunFailedModal.test.tsx | 2 ++ app/src/pages/RunSummary/index.tsx | 15 +++++++--- 9 files changed, 83 insertions(+), 48 deletions(-) diff --git a/app/src/assets/localization/en/run_details.json b/app/src/assets/localization/en/run_details.json index ab9a8114f5d..9da392eeddf 100644 --- a/app/src/assets/localization/en/run_details.json +++ b/app/src/assets/localization/en/run_details.json @@ -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", @@ -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}}", @@ -144,5 +149,8 @@ "view_analysis_error_details": "View error details", "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" } diff --git a/app/src/molecules/Modal/ModalHeader.tsx b/app/src/molecules/Modal/ModalHeader.tsx index a305b67b8bc..546ed8981ce 100644 --- a/app/src/molecules/Modal/ModalHeader.tsx +++ b/app/src/molecules/Modal/ModalHeader.tsx @@ -43,7 +43,6 @@ export function ModalHeader(props: ModalHeaderBaseProps): JSX.Element { color={iconColor} size="2rem" alignSelf={ALIGN_CENTER} - marginRight={SPACING.spacing16} /> ) : null} ) : null} 0 const handleRunSuccessClick = (): void => { handleClearClick() } @@ -933,7 +935,10 @@ function TerminalRunBanner(props: TerminalRunProps): JSX.Element | null { const buildErrorBanner = (): JSX.Element => { return ( - + {highestPriorityError != null @@ -941,7 +946,11 @@ function TerminalRunBanner(props: TerminalRunProps): JSX.Element | null { errorType: highestPriorityError?.errorType, errorCode: highestPriorityError?.errorCode, }) - : 'Run completed with errors.'} + : `${ + runStatus === RUN_STATUS_SUCCEEDED + ? t('run_completed_with_warnings') + : t('run_completed_with_errors') + }`} 0 && - !isResetRunLoading) + (completedWithErrors && !isResetRunLoading) ) { return buildErrorBanner() } else { diff --git a/app/src/organisms/Devices/ProtocolRun/RunFailedModal.tsx b/app/src/organisms/Devices/ProtocolRun/RunFailedModal.tsx index d193623aaa8..37d624fedc4 100644 --- a/app/src/organisms/Devices/ProtocolRun/RunFailedModal.tsx +++ b/app/src/organisms/Devices/ProtocolRun/RunFailedModal.tsx @@ -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' @@ -45,6 +50,7 @@ interface RunFailedModalProps { setShowRunFailedModal: (showRunFailedModal: boolean) => void highestPriorityError?: RunError | null commandErrorList?: RunCommandErrors | null + runStatus: RunStatus | null } export function RunFailedModal({ @@ -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) }, @@ -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, + })} {' '} diff --git a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx index 45949fcfbf9..b30505837a6 100644 --- a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx +++ b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx @@ -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) - 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 () => { @@ -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) - 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 }, diff --git a/app/src/organisms/Devices/ProtocolRun/__tests__/RunFailedModal.test.tsx b/app/src/organisms/Devices/ProtocolRun/__tests__/RunFailedModal.test.tsx index 5e768e20359..affc52d8d94 100644 --- a/app/src/organisms/Devices/ProtocolRun/__tests__/RunFailedModal.test.tsx +++ b/app/src/organisms/Devices/ProtocolRun/__tests__/RunFailedModal.test.tsx @@ -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' @@ -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(), diff --git a/app/src/organisms/OnDeviceDisplay/RunningProtocol/RunFailedModal.tsx b/app/src/organisms/OnDeviceDisplay/RunningProtocol/RunFailedModal.tsx index 636cf850ffd..eaadfbb8aef 100644 --- a/app/src/organisms/OnDeviceDisplay/RunningProtocol/RunFailedModal.tsx +++ b/app/src/organisms/OnDeviceDisplay/RunningProtocol/RunFailedModal.tsx @@ -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' @@ -29,6 +34,7 @@ interface RunFailedModalProps { setShowRunFailedModal: (showRunFailedModal: boolean) => void errors?: RunError[] commandErrorList?: RunCommandErrors + runStatus: RunStatus | null } export function RunFailedModal({ @@ -36,6 +42,7 @@ export function RunFailedModal({ setShowRunFailedModal, errors, commandErrorList, + runStatus, }: RunFailedModalProps): JSX.Element | null { const { t, i18n } = useTranslation(['run_details', 'shared', 'branded']) const navigate = useNavigate() @@ -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 ?? []) @@ -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, + })} { runId: RUN_ID, setShowRunFailedModal: mockFn, errors: mockErrors, + runStatus: RUN_STATUS_FAILED, } vi.mocked(useStopRunMutation).mockReturnValue({ diff --git a/app/src/pages/RunSummary/index.tsx b/app/src/pages/RunSummary/index.tsx index eb73749aa99..a244f85acb6 100644 --- a/app/src/pages/RunSummary/index.tsx +++ b/app/src/pages/RunSummary/index.tsx @@ -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) { @@ -347,6 +350,7 @@ export function RunSummary(): JSX.Element { setShowRunFailedModal={setShowRunFailedModal} errors={runRecord?.data.errors} commandErrorList={commandErrorList} + runStatus={runStatus} /> ) : null} - {!didRunSucceed ? ( + {(commandErrorList != null && commandErrorList?.data.length > 0) || + !didRunSucceed ? ( ) : null}