From 33dde5e374b566fcda28a15be7ec3b6d3f5edee8 Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Mon, 5 Aug 2024 16:39:13 -0400 Subject: [PATCH] refactor(app): Split RunProgressMeter (#15893) Closes RQA-2842 Migrates the logic for building RunProgressMeter's step copy to its own utility, cleaning up the order of operations slightly. Refactors the final step text from nothing to "N/A" when the run is canceled before starting. --- .../__tests__/RunProgressMeter.test.tsx | 24 +++- .../organisms/RunProgressMeter/constants.ts | 15 +++ .../organisms/RunProgressMeter/hooks/index.ts | 1 + .../hooks/useRunProgressCopy.tsx | 118 ++++++++++++++++++ app/src/organisms/RunProgressMeter/index.tsx | 81 ++++-------- 5 files changed, 177 insertions(+), 62 deletions(-) create mode 100644 app/src/organisms/RunProgressMeter/constants.ts create mode 100644 app/src/organisms/RunProgressMeter/hooks/index.ts create mode 100644 app/src/organisms/RunProgressMeter/hooks/useRunProgressCopy.tsx diff --git a/app/src/organisms/RunProgressMeter/__tests__/RunProgressMeter.test.tsx b/app/src/organisms/RunProgressMeter/__tests__/RunProgressMeter.test.tsx index e6d4d4dba4e..0cab1ef5adb 100644 --- a/app/src/organisms/RunProgressMeter/__tests__/RunProgressMeter.test.tsx +++ b/app/src/organisms/RunProgressMeter/__tests__/RunProgressMeter.test.tsx @@ -9,6 +9,7 @@ import { RUN_STATUS_IDLE, RUN_STATUS_RUNNING, RUN_STATUS_SUCCEEDED, + RUN_STATUS_STOPPED, } from '@opentrons/api-client' import { i18n } from '../../../i18n' @@ -34,6 +35,7 @@ import { import { RunProgressMeter } from '..' import { renderWithProviders } from '../../../__testing-utils__' import { useLastRunCommand } from '../../Devices/hooks/useLastRunCommand' +import { useRunningStepCounts } from '../../../resources/protocols/hooks' import type { RunCommandSummary } from '@opentrons/api-client' import type * as ApiClient from '@opentrons/react-api-client' @@ -52,6 +54,7 @@ vi.mock('../../Devices/hooks') vi.mock('../../../atoms/ProgressBar') vi.mock('../../InterventionModal') vi.mock('../../Devices/hooks/useLastRunCommand') +vi.mock('../../../resources/protocols/hooks') const render = (props: React.ComponentProps) => { return renderWithProviders(, { @@ -88,6 +91,11 @@ describe('RunProgressMeter', () => { .thenReturn({ key: NON_DETERMINISTIC_COMMAND_KEY } as RunCommandSummary) vi.mocked(useNotifyRunQuery).mockReturnValue({ data: null } as any) + vi.mocked(useRunningStepCounts).mockReturnValue({ + totalStepCount: null, + currentStepNumber: null, + hasRunDiverged: true, + }) props = { runId: NON_DETERMINISTIC_RUN_ID, @@ -100,7 +108,7 @@ describe('RunProgressMeter', () => { it('should show only the total count of commands in run and not show the meter when protocol is non-deterministic', () => { vi.mocked(useCommandQuery).mockReturnValue({ data: null } as any) render(props) - expect(screen.getByText('Current Step ?/?')).toBeTruthy() + expect(screen.getByText('Current Step ?/?:')).toBeTruthy() expect(screen.queryByText('MOCK PROGRESS BAR')).toBeFalsy() }) it('should give the correct info when run status is idle', () => { @@ -141,7 +149,19 @@ describe('RunProgressMeter', () => { it('should render the correct run status when run status is completed', () => { vi.mocked(useCommandQuery).mockReturnValue({ data: null } as any) vi.mocked(useRunStatus).mockReturnValue(RUN_STATUS_SUCCEEDED) + vi.mocked(useRunningStepCounts).mockReturnValue({ + totalStepCount: 10, + currentStepNumber: 10, + hasRunDiverged: false, + }) + render(props) + screen.getByText('Final Step 10/10:') + }) + + it('should render the correct step info when the run is cancelled before running', () => { + vi.mocked(useCommandQuery).mockReturnValue({ data: null } as any) + vi.mocked(useRunStatus).mockReturnValue(RUN_STATUS_STOPPED) render(props) - screen.getByText('Final Step ?/?') + screen.getByText('Final Step: N/A') }) }) diff --git a/app/src/organisms/RunProgressMeter/constants.ts b/app/src/organisms/RunProgressMeter/constants.ts new file mode 100644 index 00000000000..525cb55c5d3 --- /dev/null +++ b/app/src/organisms/RunProgressMeter/constants.ts @@ -0,0 +1,15 @@ +import { + RUN_STATUS_FAILED, + RUN_STATUS_FINISHING, + RUN_STATUS_STOPPED, + RUN_STATUS_SUCCEEDED, +} from '@opentrons/api-client' + +import type { RunStatus } from '@opentrons/api-client' + +export const TERMINAL_RUN_STATUSES: RunStatus[] = [ + RUN_STATUS_STOPPED, + RUN_STATUS_FAILED, + RUN_STATUS_FINISHING, + RUN_STATUS_SUCCEEDED, +] diff --git a/app/src/organisms/RunProgressMeter/hooks/index.ts b/app/src/organisms/RunProgressMeter/hooks/index.ts new file mode 100644 index 00000000000..1b8689ea274 --- /dev/null +++ b/app/src/organisms/RunProgressMeter/hooks/index.ts @@ -0,0 +1 @@ +export * from './useRunProgressCopy' diff --git a/app/src/organisms/RunProgressMeter/hooks/useRunProgressCopy.tsx b/app/src/organisms/RunProgressMeter/hooks/useRunProgressCopy.tsx new file mode 100644 index 00000000000..9484a785f82 --- /dev/null +++ b/app/src/organisms/RunProgressMeter/hooks/useRunProgressCopy.tsx @@ -0,0 +1,118 @@ +import { + RUN_STATUS_BLOCKED_BY_OPEN_DOOR, + RUN_STATUS_IDLE, +} from '@opentrons/api-client' +import * as React from 'react' +import { useTranslation } from 'react-i18next' +import { getCommandTextData } from '../../../molecules/Command/utils/getCommandTextData' +import { LegacyStyledText } from '@opentrons/components' +import { CommandText } from '../../../molecules/Command' +import { TERMINAL_RUN_STATUSES } from '../constants' + +import type { CommandDetail, RunStatus } from '@opentrons/api-client' +import type { + CompletedProtocolAnalysis, + RobotType, + RunTimeCommand, +} from '@opentrons/shared-data' + +interface UseRunProgressResult { + currentStepContents: React.ReactNode + stepCountStr: string | null + progressPercentage: number +} + +interface UseRunProgressProps { + runStatus: RunStatus | null + currentStepNumber: number | null + totalStepCount: number | null + analysis: CompletedProtocolAnalysis | null + hasRunDiverged: boolean + runCommandDetails: CommandDetail | null + robotType: RobotType + analysisCommands: RunTimeCommand[] +} + +// TODO(jh, 08-05-24): Testing is sufficiently covered by RunProgressMeter, but we should migrate relevant tests to this +// hook after devising a better way to test i18n outside of a component. +export function useRunProgressCopy({ + runStatus, + currentStepNumber, + totalStepCount, + hasRunDiverged, + runCommandDetails, + analysisCommands, + robotType, + analysis, +}: UseRunProgressProps): UseRunProgressResult { + const { t } = useTranslation('run_details') + + const runHasNotBeenStarted = + (currentStepNumber === 0 && + runStatus === RUN_STATUS_BLOCKED_BY_OPEN_DOOR) || + runStatus === RUN_STATUS_IDLE + + const currentStepContents = ((): JSX.Element | null => { + if (runHasNotBeenStarted) { + return {t('not_started_yet')} + } else if (analysis != null && !hasRunDiverged) { + return ( + + ) + } else if ( + analysis != null && + hasRunDiverged && + runCommandDetails != null + ) { + return ( + + ) + } else { + return null + } + })() + + const progressPercentage = runHasNotBeenStarted + ? 0 + : ((currentStepNumber as number) / analysisCommands.length) * 100 + + const stepCountStr = ((): string | null => { + if (runStatus == null) { + return null + } else { + const isTerminalStatus = TERMINAL_RUN_STATUSES.includes(runStatus) + const stepType = isTerminalStatus ? t('final_step') : t('current_step') + + if (runStatus === RUN_STATUS_IDLE) { + return `${stepType}:` + } else if (isTerminalStatus && currentStepNumber == null) { + return `${stepType}: N/A` + } else { + const getCountString = (): string => { + const current = currentStepNumber ?? '?' + const total = totalStepCount ?? '?' + + return `${current}/${total}` + } + + const countString = getCountString() + + return `${stepType} ${countString}:` + } + } + })() + + return { + currentStepContents, + stepCountStr, + progressPercentage, + } +} diff --git a/app/src/organisms/RunProgressMeter/index.tsx b/app/src/organisms/RunProgressMeter/index.tsx index c1c674d03b3..5f120904a41 100644 --- a/app/src/organisms/RunProgressMeter/index.tsx +++ b/app/src/organisms/RunProgressMeter/index.tsx @@ -22,18 +22,13 @@ import { import { useCommandQuery } from '@opentrons/react-api-client' import { RUN_STATUS_IDLE, - RUN_STATUS_STOPPED, - RUN_STATUS_FAILED, RUN_STATUS_FINISHING, - RUN_STATUS_SUCCEEDED, RUN_STATUS_RUNNING, - RUN_STATUS_BLOCKED_BY_OPEN_DOOR, } from '@opentrons/api-client' import { useMostRecentCompletedAnalysis } from '../LabwarePositionCheck/useMostRecentCompletedAnalysis' import { getModalPortalEl } from '../../App/portal' import { Tooltip } from '../../atoms/Tooltip' -import { CommandText } from '../../molecules/Command' import { useRunStatus } from '../RunTimeControl/hooks' import { InterventionModal } from '../InterventionModal' import { ProgressBar } from '../../atoms/ProgressBar' @@ -44,17 +39,9 @@ import { useNotifyRunQuery, useNotifyAllCommandsQuery, } from '../../resources/runs' -import { getCommandTextData } from '../../molecules/Command/utils/getCommandTextData' import { useRunningStepCounts } from '../../resources/protocols/hooks' - -import type { RunStatus } from '@opentrons/api-client' - -const TERMINAL_RUN_STATUSES: RunStatus[] = [ - RUN_STATUS_STOPPED, - RUN_STATUS_FAILED, - RUN_STATUS_FINISHING, - RUN_STATUS_SUCCEEDED, -] +import { TERMINAL_RUN_STATUSES } from './constants' +import { useRunProgressCopy } from './hooks' interface RunProgressMeterProps { runId: string @@ -104,36 +91,6 @@ export function RunProgressMeter(props: RunProgressMeterProps): JSX.Element { const { downloadRunLog } = useDownloadRunLog(robotName, runId) - const stepCountStr = `${currentStepNumber ?? '?'}/${totalStepCount ?? '?'}` - - const runHasNotBeenStarted = - (currentStepNumber === 0 && - runStatus === RUN_STATUS_BLOCKED_BY_OPEN_DOOR) || - runStatus === RUN_STATUS_IDLE - - let currentStepContents: React.ReactNode = null - if (runHasNotBeenStarted) { - currentStepContents = ( - {t('not_started_yet')} - ) - } else if (analysis != null && !hasRunDiverged) { - currentStepContents = ( - - ) - } else if (analysis != null && hasRunDiverged && runCommandDetails != null) { - currentStepContents = ( - - ) - } - React.useEffect(() => { if ( lastRunCommand != null && @@ -158,6 +115,21 @@ export function RunProgressMeter(props: RunProgressMeterProps): JSX.Element { downloadRunLog() } + const { + progressPercentage, + stepCountStr, + currentStepContents, + } = useRunProgressCopy({ + runStatus, + robotType, + currentStepNumber, + totalStepCount, + analysis, + analysisCommands, + runCommandDetails: runCommandDetails ?? null, + hasRunDiverged, + }) + return ( <> {interventionModalCommandKey != null && @@ -184,15 +156,9 @@ export function RunProgressMeter(props: RunProgressMeterProps): JSX.Element { {`${ - runStatus != null && TERMINAL_RUN_STATUSES.includes(runStatus) - ? t('final_step') - : t('current_step') - }${ - runStatus === RUN_STATUS_IDLE - ? ':' - : ` ${stepCountStr}${currentStepContents != null ? ': ' : ''}` - }`} + > + {stepCountStr} + {currentStepContents} @@ -226,12 +192,7 @@ export function RunProgressMeter(props: RunProgressMeterProps): JSX.Element { {!hasRunDiverged ? (