From 0ff7b51d3e289c05b00ce70f0341c8222e2817a5 Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Fri, 10 May 2024 16:50:41 -0400 Subject: [PATCH 1/5] feat(app, react-api-client): add retry failed command during error recovery Adds the ability to retry command that caused the run to enter error recovery. Resumes the run after completing the failed command. --- .../localization/en/error_recovery.json | 1 + .../__tests__/ProtocolRunHeader.test.tsx | 4 + ...HistoricalProtocolRunOverflowMenu.test.tsx | 2 + .../ErrorRecoveryWizard.tsx | 133 +++++++++++++ .../ErrorRecoveryFlows/RecoveryInProgress.tsx | 8 +- .../RecoveryOptions/ResumeRun.tsx | 16 +- .../__tests__/ResumeRun.test.tsx | 64 +++++- .../__tests__/SelectRecoveryOptions.test.tsx | 3 +- .../__tests__/BeforeBeginning.test.tsx | 3 +- .../__tests__/ErrorRecoveryFlows.test.tsx | 167 ++++++++-------- .../__tests__/ErrorRecoveryWizard.test.tsx | 187 ++++++++++++++++++ .../__tests__/RecoveryInProgress.test.tsx | 22 ++- .../__tests__/useRecoveryCommands.test.ts | 106 ++++++++++ .../__tests__/utils.test.ts | 95 ++++++++- .../organisms/ErrorRecoveryFlows/constants.ts | 9 + .../organisms/ErrorRecoveryFlows/index.tsx | 134 ++++--------- app/src/organisms/ErrorRecoveryFlows/types.ts | 9 +- .../ErrorRecoveryFlows/useRecoveryCommands.ts | 68 +++++++ app/src/organisms/ErrorRecoveryFlows/utils.ts | 186 +++++++++++------ .../RunTimeControl/__tests__/hooks.test.tsx | 3 + app/src/organisms/RunTimeControl/hooks.ts | 6 + .../__tests__/ProtocolSetup.test.tsx | 2 + .../__tests__/RunningProtocol.test.tsx | 30 +++ app/src/pages/RunningProtocol/index.tsx | 29 ++- .../src/runs/__fixtures__/runActions.ts | 7 + .../useResumeRunFromRecoveryMutation.test.tsx | 64 ++++++ .../__tests__/useRunActionMutations.test.tsx | 11 ++ react-api-client/src/runs/index.ts | 2 + .../runs/useResumeRunFromRecoveryMutation.ts | 53 +++++ .../src/runs/useRunActionMutations.ts | 10 + 30 files changed, 1159 insertions(+), 275 deletions(-) create mode 100644 app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx create mode 100644 app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryWizard.test.tsx create mode 100644 app/src/organisms/ErrorRecoveryFlows/__tests__/useRecoveryCommands.test.ts create mode 100644 app/src/organisms/ErrorRecoveryFlows/useRecoveryCommands.ts create mode 100644 react-api-client/src/runs/__tests__/useResumeRunFromRecoveryMutation.test.tsx create mode 100644 react-api-client/src/runs/useResumeRunFromRecoveryMutation.ts diff --git a/app/src/assets/localization/en/error_recovery.json b/app/src/assets/localization/en/error_recovery.json index f606e5fdf33..6ba2520aebb 100644 --- a/app/src/assets/localization/en/error_recovery.json +++ b/app/src/assets/localization/en/error_recovery.json @@ -15,5 +15,6 @@ "run_will_resume": "The run will resume from the point at which the error occurred. Take any necessary actions to correct the problem first. If the step is completed successfully, the protocol continues.", "stand_back": "Stand back, robot is in motion", "stand_back_resuming": "Stand back, resuming current step", + "stand_back_retrying": "Stand back, retrying current command", "view_recovery_options": "View recovery options" } diff --git a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx index f7d1b09f80a..f6ff94e7b13 100644 --- a/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx +++ b/app/src/organisms/Devices/ProtocolRun/__tests__/ProtocolRunHeader.test.tsx @@ -278,10 +278,12 @@ describe('ProtocolRunHeader', () => { pause: () => {}, stop: () => {}, reset: () => {}, + resumeFromRecovery: () => {}, isPlayRunActionLoading: false, isPauseRunActionLoading: false, isStopRunActionLoading: false, isResetRunLoading: false, + isResumeRunFromRecoveryActionLoading: false, }) when(vi.mocked(useRunStatus)).calledWith(RUN_ID).thenReturn(RUN_STATUS_IDLE) when(vi.mocked(useRunTimestamps)).calledWith(RUN_ID).thenReturn({ @@ -777,10 +779,12 @@ describe('ProtocolRunHeader', () => { pause: () => {}, stop: () => {}, reset: () => {}, + resumeFromRecovery: () => {}, isPlayRunActionLoading: false, isPauseRunActionLoading: false, isStopRunActionLoading: false, isResetRunLoading: true, + isResumeRunFromRecoveryActionLoading: false, }) render() diff --git a/app/src/organisms/Devices/__tests__/HistoricalProtocolRunOverflowMenu.test.tsx b/app/src/organisms/Devices/__tests__/HistoricalProtocolRunOverflowMenu.test.tsx index c436bc04960..a78a253e8b0 100644 --- a/app/src/organisms/Devices/__tests__/HistoricalProtocolRunOverflowMenu.test.tsx +++ b/app/src/organisms/Devices/__tests__/HistoricalProtocolRunOverflowMenu.test.tsx @@ -82,10 +82,12 @@ describe('HistoricalProtocolRunOverflowMenu', () => { pause: () => {}, stop: () => {}, reset: () => {}, + resumeFromRecovery: () => {}, isPlayRunActionLoading: false, isPauseRunActionLoading: false, isStopRunActionLoading: false, isResetRunLoading: false, + isResumeRunFromRecoveryActionLoading: false, }) when(useAllCommandsQuery) .calledWith( diff --git a/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx b/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx new file mode 100644 index 00000000000..8f27a717f63 --- /dev/null +++ b/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx @@ -0,0 +1,133 @@ +import * as React from 'react' +import { createPortal } from 'react-dom' +import { useSelector } from 'react-redux' + +import { + BORDERS, + COLORS, + DIRECTION_COLUMN, + Flex, + POSITION_ABSOLUTE, +} from '@opentrons/components' + +import { getIsOnDevice } from '../../redux/config' +import { getTopPortalEl } from '../../App/portal' +import { BeforeBeginning } from './BeforeBeginning' +import { SelectRecoveryOption, ResumeRun } from './RecoveryOptions' +import { ErrorRecoveryHeader } from './ErrorRecoveryHeader' +import { RecoveryInProgress } from './RecoveryInProgress' +import { getErrorKind, useRouteUpdateActions } from './utils' +import { useRecoveryCommands } from './useRecoveryCommands' +import { RECOVERY_MAP } from './constants' + +import type { FailedCommand, IRecoveryMap, RecoveryContentProps } from './types' + +export interface ErrorRecoveryFlowsProps { + runId: string + failedCommand: FailedCommand | null +} + +export function ErrorRecoveryWizard({ + runId, + failedCommand, +}: ErrorRecoveryFlowsProps): JSX.Element { + /** + * Recovery Route: A logically-related collection of recovery steps or a single step if unrelated to any existing recovery route. + * Recovery Step: Analogous to a "step" in other wizard flows. + */ + const [recoveryMap, setRecoveryMap] = React.useState({ + route: RECOVERY_MAP.ROBOT_IN_MOTION.ROUTE, // Initialize the route to "in motion", since wizard always start with a home command. + step: RECOVERY_MAP.ROBOT_IN_MOTION.STEPS.IN_MOTION, + }) + + const errorKind = getErrorKind(failedCommand?.error?.errorType) + const isOnDevice = useSelector(getIsOnDevice) + const routeUpdateActions = useRouteUpdateActions({ + recoveryMap, + setRecoveryMap, + }) + const recoveryCommands = useRecoveryCommands({ + runId, + failedCommand, + }) + + useInitialPipetteHome(recoveryCommands, routeUpdateActions) + + return ( + + ) +} + +function ErrorRecoveryComponent(props: RecoveryContentProps): JSX.Element { + return createPortal( + + + + , + getTopPortalEl() + ) +} + +export function ErrorRecoveryContent(props: RecoveryContentProps): JSX.Element { + const buildBeforeBeginning = (): JSX.Element => { + return + } + + const buildSelectRecoveryOption = (): JSX.Element => { + return + } + + const buildRecoveryInProgress = (): JSX.Element => { + return + } + + const buildResumeRun = (): JSX.Element => { + return + } + + switch (props.recoveryMap.route) { + case RECOVERY_MAP.BEFORE_BEGINNING.ROUTE: + return buildBeforeBeginning() + case RECOVERY_MAP.OPTION_SELECTION.ROUTE: + return buildSelectRecoveryOption() + case RECOVERY_MAP.RESUME.ROUTE: + return buildResumeRun() + case RECOVERY_MAP.ROBOT_IN_MOTION.ROUTE: + case RECOVERY_MAP.ROBOT_RESUMING.ROUTE: + case RECOVERY_MAP.ROBOT_RETRYING_COMMAND.ROUTE: + return buildRecoveryInProgress() + default: + return buildSelectRecoveryOption() + } +} + +// Home the Z-axis of all attached pipettes on Error Recovery launch. +export function useInitialPipetteHome( + recoveryCommands: ReturnType, + routeUpdateActions: ReturnType +): void { + const { homePipetteZAxes } = recoveryCommands + const { setRobotInMotion } = routeUpdateActions + + React.useEffect(() => { + void setRobotInMotion(true) + .then(() => homePipetteZAxes()) + .then(() => setRobotInMotion(false)) + }, []) +} diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryInProgress.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryInProgress.tsx index a30d8dd2f0a..27123352933 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryInProgress.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryInProgress.tsx @@ -9,7 +9,11 @@ import type { RobotMovingRoute, RecoveryContentProps } from './types' export function RecoveryInProgress({ recoveryMap, }: RecoveryContentProps): JSX.Element { - const { ROBOT_IN_MOTION, ROBOT_RESUMING } = RECOVERY_MAP + const { + ROBOT_IN_MOTION, + ROBOT_RESUMING, + ROBOT_RETRYING_COMMAND, + } = RECOVERY_MAP const { t } = useTranslation('error_recovery') const { route } = recoveryMap @@ -19,6 +23,8 @@ export function RecoveryInProgress({ return t('stand_back') case ROBOT_RESUMING.ROUTE: return t('stand_back_resuming') + case ROBOT_RETRYING_COMMAND.ROUTE: + return t('stand_back_retrying') default: return t('stand_back') } diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ResumeRun.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ResumeRun.tsx index 982e1a01129..d702db177f9 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ResumeRun.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/ResumeRun.tsx @@ -11,17 +11,27 @@ import { StyledText, } from '@opentrons/components' +import { RECOVERY_MAP } from '../constants' import { RecoveryFooterButtons } from './shared' import type { RecoveryContentProps } from '../types' export function ResumeRun({ isOnDevice, - onComplete, routeUpdateActions, + recoveryCommands, }: RecoveryContentProps): JSX.Element | null { + const { ROBOT_RETRYING_COMMAND } = RECOVERY_MAP const { t } = useTranslation('error_recovery') - const { goBackPrevStep } = routeUpdateActions + + const { retryFailedCommand, resumeRun } = recoveryCommands + const { goBackPrevStep, setRobotInMotion } = routeUpdateActions + + const primaryBtnOnClick = (): Promise => { + return setRobotInMotion(true, ROBOT_RETRYING_COMMAND.ROUTE) // Show the "retrying" motion screen while exiting ER. + .then(() => retryFailedCommand()) + .then(() => resumeRun()) + } if (isOnDevice) { return ( @@ -50,7 +60,7 @@ export function ResumeRun({ diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/ResumeRun.test.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/ResumeRun.test.tsx index ca7d83e297d..b0f147f4255 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/ResumeRun.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/ResumeRun.test.tsx @@ -1,6 +1,6 @@ import * as React from 'react' import { vi, describe, it, expect, beforeEach } from 'vitest' -import { screen, fireEvent } from '@testing-library/react' +import { screen, fireEvent, waitFor } from '@testing-library/react' import { renderWithProviders } from '../../../../__testing-utils__' import { i18n } from '../../../../i18n' @@ -16,20 +16,19 @@ const render = (props: React.ComponentProps) => { } describe('RecoveryFooterButtons', () => { - const { RESUME } = RECOVERY_MAP + const { RESUME, ROBOT_RETRYING_COMMAND } = RECOVERY_MAP let props: React.ComponentProps - let mockOnComplete: Mock let mockGoBackPrevStep: Mock beforeEach(() => { - mockOnComplete = vi.fn() mockGoBackPrevStep = vi.fn() const mockRouteUpdateActions = { goBackPrevStep: mockGoBackPrevStep } as any props = { isOnDevice: true, + recoveryCommands: {} as any, + failedCommand: {} as any, errorKind: ERROR_KINDS.GENERAL_ERROR, - onComplete: mockOnComplete, routeUpdateActions: mockRouteUpdateActions, recoveryMap: { route: RESUME.ROUTE, @@ -38,7 +37,7 @@ describe('RecoveryFooterButtons', () => { } }) - it('renders appropriate copy and click behavior', () => { + it('renders appropriate copy and click behavior', async () => { render(props) screen.getByText('Are you sure you want to resume?') @@ -46,13 +45,60 @@ describe('RecoveryFooterButtons', () => { 'The run will resume from the point at which the error occurred.' ) - const primaryBtn = screen.getByRole('button', { name: 'Confirm' }) const secondaryBtn = screen.getByRole('button', { name: 'Go back' }) - fireEvent.click(primaryBtn) fireEvent.click(secondaryBtn) - expect(mockOnComplete).toHaveBeenCalled() expect(mockGoBackPrevStep).toHaveBeenCalled() }) + + it('should call commands in the correct order for the primaryOnClick callback', async () => { + const setRobotInMotionMock = vi.fn(() => Promise.resolve()) + const retryFailedCommandMock = vi.fn(() => Promise.resolve()) + const resumeRunMock = vi.fn() + + const mockRecoveryCommands = { + retryFailedCommand: retryFailedCommandMock, + resumeRun: resumeRunMock, + } as any + + const mockRouteUpdateActions = { + setRobotInMotion: setRobotInMotionMock, + } as any + + render({ + ...props, + recoveryCommands: mockRecoveryCommands, + routeUpdateActions: mockRouteUpdateActions, + }) + + const primaryBtn = screen.getByRole('button', { name: 'Confirm' }) + fireEvent.click(primaryBtn) + + await waitFor(() => { + expect(setRobotInMotionMock).toHaveBeenCalledTimes(1) + }) + await waitFor(() => { + expect(setRobotInMotionMock).toHaveBeenCalledWith( + true, + ROBOT_RETRYING_COMMAND.ROUTE + ) + }) + await waitFor(() => { + expect(retryFailedCommandMock).toHaveBeenCalledTimes(1) + }) + await waitFor(() => { + expect(resumeRunMock).toHaveBeenCalledTimes(1) + }) + + expect(setRobotInMotionMock.mock.invocationCallOrder[0]).toBeLessThan( + retryFailedCommandMock.mock.invocationCallOrder[0] + ) + + await waitFor(() => { + expect(retryFailedCommandMock.mock.invocationCallOrder[0]).toBeLessThan( + resumeRunMock.mock.invocationCallOrder[0] + ) + }) + }) }) diff --git a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SelectRecoveryOptions.test.tsx b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SelectRecoveryOptions.test.tsx index 81e47d85dac..71cc9224709 100644 --- a/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SelectRecoveryOptions.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/RecoveryOptions/__tests__/SelectRecoveryOptions.test.tsx @@ -42,7 +42,8 @@ describe('SelectRecoveryOption', () => { props = { isOnDevice: true, errorKind: ERROR_KINDS.GENERAL_ERROR, - onComplete: vi.fn(), + failedCommand: {} as any, + recoveryCommands: {} as any, routeUpdateActions: mockRouteUpdateActions, recoveryMap: { route: RESUME.ROUTE, diff --git a/app/src/organisms/ErrorRecoveryFlows/__tests__/BeforeBeginning.test.tsx b/app/src/organisms/ErrorRecoveryFlows/__tests__/BeforeBeginning.test.tsx index d9e8ed280c5..ceafea5c520 100644 --- a/app/src/organisms/ErrorRecoveryFlows/__tests__/BeforeBeginning.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/__tests__/BeforeBeginning.test.tsx @@ -28,8 +28,9 @@ describe('BeforeBeginning', () => { props = { isOnDevice: true, + recoveryCommands: {} as any, + failedCommand: {} as any, errorKind: ERROR_KINDS.GENERAL_ERROR, - onComplete: vi.fn(), routeUpdateActions: mockRouteUpdateActions, recoveryMap: { route: BEFORE_BEGINNING.ROUTE, diff --git a/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryFlows.test.tsx b/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryFlows.test.tsx index 93ca813ba37..782d0df6f16 100644 --- a/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryFlows.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryFlows.test.tsx @@ -1,114 +1,109 @@ import * as React from 'react' -import { vi, describe, it, beforeEach } from 'vitest' -import { screen } from '@testing-library/react' +import { vi, describe, expect, it, beforeEach } from 'vitest' +import { screen, renderHook, act } from '@testing-library/react' + +import { + RUN_STATUS_AWAITING_RECOVERY, + RUN_STATUS_RUNNING, +} from '@opentrons/api-client' import { renderWithProviders } from '../../../__testing-utils__' import { i18n } from '../../../i18n' -import { ErrorRecoveryContent } from '..' -import { ERROR_KINDS, RECOVERY_MAP } from '../constants' -import { BeforeBeginning } from '../BeforeBeginning' -import { SelectRecoveryOption, ResumeRun } from '../RecoveryOptions' -import { RecoveryInProgress } from '../RecoveryInProgress' - -import type { IRecoveryMap } from '../types' - -vi.mock('../BeforeBeginning') -vi.mock('../RecoveryOptions') -vi.mock('../RecoveryInProgress') +import { ErrorRecoveryFlows, useErrorRecoveryFlows } from '..' +import { ErrorRecoveryWizard } from '../ErrorRecoveryWizard' +import { useCurrentlyFailedRunCommand } from '../utils' -const render = (props: React.ComponentProps) => { - return renderWithProviders(, { - i18nInstance: i18n, - })[0] -} +import type { RunStatus } from '@opentrons/api-client' -describe('ErrorRecoveryContent', () => { - const { - OPTION_SELECTION, - BEFORE_BEGINNING, - RESUME, - ROBOT_RESUMING, - ROBOT_IN_MOTION, - } = RECOVERY_MAP - - let props: React.ComponentProps - const mockRecoveryMap: IRecoveryMap = { - route: OPTION_SELECTION.ROUTE, - step: OPTION_SELECTION.STEPS.SELECT, - } +vi.mock('../ErrorRecoveryWizard') +vi.mock('../utils') +describe('useErrorRecovery', () => { beforeEach(() => { - props = { - errorKind: ERROR_KINDS.GENERAL_ERROR, - routeUpdateActions: {} as any, - recoveryMap: mockRecoveryMap, - onComplete: vi.fn(), - isOnDevice: true, - } - - vi.mocked(SelectRecoveryOption).mockReturnValue( -
MOCK_SELECT_RECOVERY_OPTION
+ vi.mocked(useCurrentlyFailedRunCommand).mockReturnValue( + 'mockCommand' as any ) - vi.mocked(BeforeBeginning).mockReturnValue(
MOCK_BEFORE_BEGINNING
) - vi.mocked(ResumeRun).mockReturnValue(
MOCK_RESUME_RUN
) - vi.mocked(RecoveryInProgress).mockReturnValue(
MOCK_IN_PROGRESS
) }) - it(`returns SelectRecoveryOption when the route is ${OPTION_SELECTION.ROUTE}`, () => { - render(props) + it('should have initial state of isEREnabled as false', () => { + const { result } = renderHook(() => + useErrorRecoveryFlows('MOCK_ID', RUN_STATUS_RUNNING) + ) - screen.getByText('MOCK_SELECT_RECOVERY_OPTION') + expect(result.current.isEREnabled).toBe(false) }) - it(`returns BeforeBeginning when the route is ${BEFORE_BEGINNING.ROUTE}`, () => { - props = { - ...props, - recoveryMap: { - ...props.recoveryMap, - route: BEFORE_BEGINNING.ROUTE, - }, - } - render(props) + it('should toggle the value of isEREnabled properly', () => { + const { result } = renderHook(() => + useErrorRecoveryFlows('MOCK_ID', RUN_STATUS_AWAITING_RECOVERY) + ) + act(() => { + result.current.toggleER() + }) - screen.getByText('MOCK_BEFORE_BEGINNING') + expect(result.current.isEREnabled).toBe(true) + + act(() => { + result.current.toggleER() + }) + + expect(result.current.isEREnabled).toBe(false) }) - it(`returns ResumeRun when the route is ${RESUME.ROUTE}`, () => { - props = { - ...props, - recoveryMap: { - ...props.recoveryMap, - route: RESUME.ROUTE, - }, - } - render(props) + it('should disable error recovery when runStatus is not "awaiting-recovery"', () => { + const { result, rerender } = renderHook( + (runStatus: RunStatus) => useErrorRecoveryFlows('MOCK_ID', runStatus), + { + initialProps: RUN_STATUS_AWAITING_RECOVERY, + } + ) + + act(() => { + result.current.toggleER() + }) + + // @ts-expect-error "running" is a valid status here + rerender(RUN_STATUS_RUNNING) + + expect(result.current.isEREnabled).toBe(false) + + act(() => { + result.current.toggleER() + }) - screen.getByText('MOCK_RESUME_RUN') + rerender(RUN_STATUS_AWAITING_RECOVERY) + + expect(result.current.isEREnabled).toBe(false) }) - it(`returns RecoveryInProgressModal when the route is ${ROBOT_IN_MOTION.ROUTE}`, () => { - props = { - ...props, - recoveryMap: { - ...props.recoveryMap, - route: ROBOT_IN_MOTION.ROUTE, - }, - } - render(props) + it('should return the failed run command', () => { + const { result } = renderHook(() => + useErrorRecoveryFlows('MOCK_ID', RUN_STATUS_RUNNING) + ) - screen.getByText('MOCK_IN_PROGRESS') + expect(result.current.failedCommand).toEqual('mockCommand') }) +}) + +const render = (props: React.ComponentProps) => { + return renderWithProviders(, { + i18nInstance: i18n, + })[0] +} - it(`returns RecoveryInProgressModal when the route is ${ROBOT_RESUMING.ROUTE}`, () => { +describe('ErrorRecovery', () => { + let props: React.ComponentProps + + beforeEach(() => { props = { - ...props, - recoveryMap: { - ...props.recoveryMap, - route: ROBOT_IN_MOTION.ROUTE, - }, + failedCommand: {} as any, + runId: 'MOCK_RUN_ID', } - render(props) + vi.mocked(ErrorRecoveryWizard).mockReturnValue(
MOCK WIZARD
) + }) - screen.getByText('MOCK_IN_PROGRESS') + it(`renders the wizard`, () => { + render(props) + screen.getByText('MOCK WIZARD') }) }) diff --git a/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryWizard.test.tsx b/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryWizard.test.tsx new file mode 100644 index 00000000000..496fd676b9c --- /dev/null +++ b/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryWizard.test.tsx @@ -0,0 +1,187 @@ +import * as React from 'react' +import { vi, describe, it, expect, beforeEach } from 'vitest' +import { renderHook, screen, waitFor } from '@testing-library/react' + +import { renderWithProviders } from '../../../__testing-utils__' +import { i18n } from '../../../i18n' +import { + ErrorRecoveryContent, + useInitialPipetteHome, +} from '../ErrorRecoveryWizard' +import { ERROR_KINDS, RECOVERY_MAP } from '../constants' +import { BeforeBeginning } from '../BeforeBeginning' +import { SelectRecoveryOption, ResumeRun } from '../RecoveryOptions' +import { RecoveryInProgress } from '../RecoveryInProgress' + +import type { Mock } from 'vitest' +import type { IRecoveryMap } from '../types' + +vi.mock('../BeforeBeginning') +vi.mock('../RecoveryOptions') +vi.mock('../RecoveryInProgress') + +const render = (props: React.ComponentProps) => { + return renderWithProviders(, { + i18nInstance: i18n, + })[0] +} + +describe('ErrorRecoveryContent', () => { + const { + OPTION_SELECTION, + BEFORE_BEGINNING, + RESUME, + ROBOT_RESUMING, + ROBOT_IN_MOTION, + ROBOT_RETRYING_COMMAND, + } = RECOVERY_MAP + + let props: React.ComponentProps + const mockRecoveryMap: IRecoveryMap = { + route: OPTION_SELECTION.ROUTE, + step: OPTION_SELECTION.STEPS.SELECT, + } + + beforeEach(() => { + props = { + failedCommand: {} as any, + recoveryCommands: {} as any, + errorKind: ERROR_KINDS.GENERAL_ERROR, + routeUpdateActions: {} as any, + recoveryMap: mockRecoveryMap, + isOnDevice: true, + } + + vi.mocked(SelectRecoveryOption).mockReturnValue( +
MOCK_SELECT_RECOVERY_OPTION
+ ) + vi.mocked(BeforeBeginning).mockReturnValue(
MOCK_BEFORE_BEGINNING
) + vi.mocked(ResumeRun).mockReturnValue(
MOCK_RESUME_RUN
) + vi.mocked(RecoveryInProgress).mockReturnValue(
MOCK_IN_PROGRESS
) + }) + + it(`returns SelectRecoveryOption when the route is ${OPTION_SELECTION.ROUTE}`, () => { + render(props) + + screen.getByText('MOCK_SELECT_RECOVERY_OPTION') + }) + + it(`returns BeforeBeginning when the route is ${BEFORE_BEGINNING.ROUTE}`, () => { + props = { + ...props, + recoveryMap: { + ...props.recoveryMap, + route: BEFORE_BEGINNING.ROUTE, + }, + } + render(props) + + screen.getByText('MOCK_BEFORE_BEGINNING') + }) + + it(`returns ResumeRun when the route is ${RESUME.ROUTE}`, () => { + props = { + ...props, + recoveryMap: { + ...props.recoveryMap, + route: RESUME.ROUTE, + }, + } + render(props) + + screen.getByText('MOCK_RESUME_RUN') + }) + + it(`returns RecoveryInProgressModal when the route is ${ROBOT_IN_MOTION.ROUTE}`, () => { + props = { + ...props, + recoveryMap: { + ...props.recoveryMap, + route: ROBOT_IN_MOTION.ROUTE, + }, + } + render(props) + + screen.getByText('MOCK_IN_PROGRESS') + }) + + it(`returns RecoveryInProgressModal when the route is ${ROBOT_RESUMING.ROUTE}`, () => { + props = { + ...props, + recoveryMap: { + ...props.recoveryMap, + route: ROBOT_IN_MOTION.ROUTE, + }, + } + render(props) + + screen.getByText('MOCK_IN_PROGRESS') + }) + + it(`returns RecoveryInProgressModal when the route is ${ROBOT_RETRYING_COMMAND.ROUTE}`, () => { + props = { + ...props, + recoveryMap: { + ...props.recoveryMap, + route: ROBOT_IN_MOTION.ROUTE, + }, + } + render(props) + + screen.getByText('MOCK_IN_PROGRESS') + }) +}) + +describe('useInitialPipetteHome', () => { + let mockZHomePipetteZAxes: Mock + let mockSetRobotInMotion: Mock + let mockRecoveryCommands: any + let mockRouteUpdateActions: any + + beforeEach(() => { + mockZHomePipetteZAxes = vi.fn() + mockSetRobotInMotion = vi.fn() + + mockSetRobotInMotion.mockResolvedValue(() => mockZHomePipetteZAxes()) + mockZHomePipetteZAxes.mockResolvedValue(() => mockSetRobotInMotion()) + + mockRecoveryCommands = { + homePipetteZAxes: mockZHomePipetteZAxes, + } as any + mockRouteUpdateActions = { + setRobotInMotion: mockSetRobotInMotion, + } as any + }) + + it('sets the motion screen properly and z-homes all pipettes only on the initial render of Error Recovery', async () => { + const { rerender } = renderHook(() => + useInitialPipetteHome(mockRecoveryCommands, mockRouteUpdateActions) + ) + + await waitFor(() => { + expect(mockSetRobotInMotion).toHaveBeenCalledWith(true) + }) + await waitFor(() => { + expect(mockZHomePipetteZAxes).toHaveBeenCalledTimes(1) + }) + await waitFor(() => { + expect(mockSetRobotInMotion).toHaveBeenCalledWith(false) + }) + + expect(mockSetRobotInMotion.mock.invocationCallOrder[0]).toBeLessThan( + mockZHomePipetteZAxes.mock.invocationCallOrder[0] + ) + expect(mockZHomePipetteZAxes.mock.invocationCallOrder[0]).toBeLessThan( + mockSetRobotInMotion.mock.invocationCallOrder[1] + ) + + rerender() + + await waitFor(() => { + expect(mockSetRobotInMotion).toHaveBeenCalledTimes(2) + }) + await waitFor(() => { + expect(mockZHomePipetteZAxes).toHaveBeenCalledTimes(1) + }) + }) +}) diff --git a/app/src/organisms/ErrorRecoveryFlows/__tests__/RecoveryInProgress.test.tsx b/app/src/organisms/ErrorRecoveryFlows/__tests__/RecoveryInProgress.test.tsx index 2811c2dafad..a280a270970 100644 --- a/app/src/organisms/ErrorRecoveryFlows/__tests__/RecoveryInProgress.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/__tests__/RecoveryInProgress.test.tsx @@ -14,14 +14,19 @@ const render = (props: React.ComponentProps) => { } describe('RecoveryInProgress', () => { - const { ROBOT_IN_MOTION, ROBOT_RESUMING } = RECOVERY_MAP + const { + ROBOT_IN_MOTION, + ROBOT_RESUMING, + ROBOT_RETRYING_COMMAND, + } = RECOVERY_MAP let props: React.ComponentProps beforeEach(() => { props = { isOnDevice: true, errorKind: ERROR_KINDS.GENERAL_ERROR, - onComplete: vi.fn(), + failedCommand: {} as any, + recoveryCommands: {} as any, routeUpdateActions: vi.fn() as any, recoveryMap: { route: ROBOT_IN_MOTION.ROUTE, @@ -48,4 +53,17 @@ describe('RecoveryInProgress', () => { screen.getByText('Stand back, resuming current step') }) + + it(`renders appropriate copy when the route is ${ROBOT_RETRYING_COMMAND.ROUTE}`, () => { + props = { + ...props, + recoveryMap: { + route: ROBOT_RETRYING_COMMAND.ROUTE, + step: ROBOT_RETRYING_COMMAND.STEPS.RETRYING, + }, + } + render(props) + + screen.getByText('Stand back, retrying current command') + }) }) diff --git a/app/src/organisms/ErrorRecoveryFlows/__tests__/useRecoveryCommands.test.ts b/app/src/organisms/ErrorRecoveryFlows/__tests__/useRecoveryCommands.test.ts new file mode 100644 index 00000000000..801b5785b7e --- /dev/null +++ b/app/src/organisms/ErrorRecoveryFlows/__tests__/useRecoveryCommands.test.ts @@ -0,0 +1,106 @@ +import { vi, it, describe, expect, beforeEach } from 'vitest' +import { renderHook, act } from '@testing-library/react' + +import { useResumeRunFromRecoveryMutation } from '@opentrons/react-api-client' + +import { useChainRunCommands } from '../../../resources/runs' +import { + useRecoveryCommands, + HOME_PIPETTE_Z_AXES, +} from '../useRecoveryCommands' + +vi.mock('@opentrons/react-api-client') +vi.mock('../../../resources/runs') + +const mockFailedCommand = { + id: 'MOCK_ID', + commandType: 'mockCommandType', + params: { test: 'mock_param' }, +} as any +const mockRunId = '123' + +describe('useRecoveryCommands', () => { + const mockResumeRunFromRecovery = vi.fn() + const mockChainRunCommands = vi.fn().mockResolvedValue([]) + + beforeEach(() => { + vi.mocked(useResumeRunFromRecoveryMutation).mockReturnValue({ + resumeRunFromRecovery: mockResumeRunFromRecovery, + } as any) + vi.mocked(useChainRunCommands).mockReturnValue({ + chainRunCommands: mockChainRunCommands, + } as any) + }) + + it('should call chainRunRecoveryCommands with continuePastCommandFailure set to true', async () => { + const { result } = renderHook(() => + useRecoveryCommands({ + runId: mockRunId, + failedCommand: mockFailedCommand, + }) + ) + + await act(async () => { + await result.current.homePipetteZAxes() // can use any result returned command + }) + + expect(mockChainRunCommands).toHaveBeenCalledWith( + [HOME_PIPETTE_Z_AXES], + true + ) + }) + + it('should call retryFailedCommand with the failedCommand', async () => { + const expectedNewCommand = { + commandType: mockFailedCommand.commandType, + params: mockFailedCommand.params, + } + + const { result } = renderHook(() => + useRecoveryCommands({ + runId: mockRunId, + failedCommand: mockFailedCommand, + }) + ) + + await act(async () => { + await result.current.retryFailedCommand() + }) + + expect(mockChainRunCommands).toHaveBeenCalledWith( + [expectedNewCommand], + true + ) + }) + + it('should call resumeRun with runId', () => { + const { result } = renderHook(() => + useRecoveryCommands({ + runId: mockRunId, + failedCommand: mockFailedCommand, + }) + ) + + result.current.resumeRun() + + expect(mockResumeRunFromRecovery).toHaveBeenCalledWith(mockRunId) + }) + + it('should call homePipetteZAxes with the appropriate command', async () => { + const { result } = renderHook(() => + useRecoveryCommands({ + runId: mockRunId, + failedCommand: mockFailedCommand, + }) + ) + + await act(async () => { + await result.current.homePipetteZAxes() + }) + + expect(mockChainRunCommands).toHaveBeenCalledWith( + [HOME_PIPETTE_Z_AXES], + true + ) + }) +}) diff --git a/app/src/organisms/ErrorRecoveryFlows/__tests__/utils.test.ts b/app/src/organisms/ErrorRecoveryFlows/__tests__/utils.test.ts index 29da7e1a213..68f551119bc 100644 --- a/app/src/organisms/ErrorRecoveryFlows/__tests__/utils.test.ts +++ b/app/src/organisms/ErrorRecoveryFlows/__tests__/utils.test.ts @@ -1,16 +1,24 @@ import { vi, describe, it, expect, beforeEach } from 'vitest' -import { renderHook } from '@testing-library/react' +import { renderHook, act } from '@testing-library/react' import { ERROR_KINDS, INVALID, RECOVERY_MAP } from '../constants' import { getErrorKind, getRecoveryRouteNavigation, useRouteUpdateActions, + useCurrentlyFailedRunCommand, } from '../utils' +import { useAllCommandsQuery } from '@opentrons/react-api-client' +import { + RUN_STATUS_AWAITING_RECOVERY, + RUN_STATUS_RUNNING, +} from '@opentrons/api-client' import type { Mock } from 'vitest' import type { GetRouteUpdateActionsParams } from '../utils' +vi.mock('@opentrons/react-api-client') + describe('getErrorKind', () => { it(`returns ${ERROR_KINDS.GENERAL_ERROR} if the errorType isn't handled explicitly`, () => { const mockErrorType = 'NON_HANDLED_ERROR' @@ -135,3 +143,88 @@ describe('useRouteUpdateActions', () => { }) }) }) + +const MOCK_COMMANDS_QUERY = { + data: { + data: [ + { status: 'failed', intent: 'fixit', id: '0' }, + { status: 'failed', intent: 'protocol', id: '111' }, + { status: 'failed', intent: 'protocol', id: '123' }, + { status: 'success', intent: 'fixit', id: '1' }, + ], + }, +} as any + +const MOCK_RUN_ID = 'runId' + +describe('useCurrentlyFailedRunCommand', () => { + beforeEach(() => { + vi.mocked(useAllCommandsQuery).mockReturnValue(MOCK_COMMANDS_QUERY) + }) + + it('returns null on initial render when the run status is not "awaiting-recovery"', () => { + const { result } = renderHook(() => + useCurrentlyFailedRunCommand(MOCK_RUN_ID, RUN_STATUS_RUNNING) + ) + + expect(result.current).toBeNull() + }) + + it('sets recentFailedCommand correctly when runStatus is "awaiting-recovery" and there is no recent failed command', () => { + const { result, rerender } = renderHook( + // @ts-expect-error this works + props => useCurrentlyFailedRunCommand(...props), + { + initialProps: [MOCK_RUN_ID, RUN_STATUS_RUNNING], + } + ) + + act(() => { + rerender([MOCK_RUN_ID, RUN_STATUS_AWAITING_RECOVERY]) + }) + + expect(result.current).toEqual({ + status: 'failed', + intent: 'protocol', + id: '123', + }) + }) + + it('always returns the failed protocol run command that caused the run to enter "awaiting-recovery"', () => { + const { result, rerender } = renderHook( + // @ts-expect-error this works + props => useCurrentlyFailedRunCommand(...props), + { + initialProps: [MOCK_RUN_ID, RUN_STATUS_AWAITING_RECOVERY], + } + ) + + vi.mocked(useAllCommandsQuery).mockReturnValue({ + ...MOCK_COMMANDS_QUERY, + ...{ status: 'failed', intent: 'protocol', id: '124' }, + }) + rerender([MOCK_RUN_ID, RUN_STATUS_AWAITING_RECOVERY]) + + expect(result.current).toEqual({ + status: 'failed', + intent: 'protocol', + id: '123', + }) + }) + + it('sets recentFailedCommand to null when runStatus is not "awaiting-recovery"', () => { + const { result, rerender } = renderHook( + // @ts-expect-error this works + props => useCurrentlyFailedRunCommand(...props), + { + initialProps: ['runId', 'awaiting-recovery'], + } + ) + + act(() => { + rerender([MOCK_RUN_ID, RUN_STATUS_RUNNING]) + }) + + expect(result.current).toBeNull() + }) +}) diff --git a/app/src/organisms/ErrorRecoveryFlows/constants.ts b/app/src/organisms/ErrorRecoveryFlows/constants.ts index 4a27ea62db8..e0da763d076 100644 --- a/app/src/organisms/ErrorRecoveryFlows/constants.ts +++ b/app/src/organisms/ErrorRecoveryFlows/constants.ts @@ -8,6 +8,7 @@ export const ERROR_KINDS = { GENERAL_ERROR: 'GENERAL_ERROR', } as const +// TODO(jh, 05-09-24): Refactor to a directed graph. EXEC-430. // Valid recovery routes and steps. export const RECOVERY_MAP = { BEFORE_BEGINNING: { @@ -36,6 +37,12 @@ export const RECOVERY_MAP = { RESUMING: 'resuming', }, }, + ROBOT_RETRYING_COMMAND: { + ROUTE: 'robot-retrying-command', + STEPS: { + RETRYING: 'retrying', + }, + }, OPTION_SELECTION: { ROUTE: 'option-selection', STEPS: { SELECT: 'select' }, @@ -48,6 +55,7 @@ const { RESUME, ROBOT_RESUMING, ROBOT_IN_MOTION, + ROBOT_RETRYING_COMMAND, DROP_TIP, REFILL_AND_RESUME, IGNORE_AND_RESUME, @@ -61,6 +69,7 @@ export const STEP_ORDER: StepOrder = { [RESUME.ROUTE]: [RESUME.STEPS.CONFIRM_RESUME], [ROBOT_IN_MOTION.ROUTE]: [ROBOT_IN_MOTION.STEPS.IN_MOTION], [ROBOT_RESUMING.ROUTE]: [ROBOT_RESUMING.STEPS.RESUMING], + [ROBOT_RETRYING_COMMAND.ROUTE]: [ROBOT_RETRYING_COMMAND.STEPS.RETRYING], [DROP_TIP.ROUTE]: [], [REFILL_AND_RESUME.ROUTE]: [], [IGNORE_AND_RESUME.ROUTE]: [], diff --git a/app/src/organisms/ErrorRecoveryFlows/index.tsx b/app/src/organisms/ErrorRecoveryFlows/index.tsx index 2824b85fbe0..0db32dfa30a 100644 --- a/app/src/organisms/ErrorRecoveryFlows/index.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/index.tsx @@ -1,109 +1,51 @@ import * as React from 'react' -import { createPortal } from 'react-dom' -import { useSelector } from 'react-redux' -import { - BORDERS, - COLORS, - DIRECTION_COLUMN, - Flex, - POSITION_ABSOLUTE, -} from '@opentrons/components' +import { RUN_STATUS_AWAITING_RECOVERY } from '@opentrons/api-client' -import { getIsOnDevice } from '../../redux/config' -import { getTopPortalEl } from '../../App/portal' -import { BeforeBeginning } from './BeforeBeginning' -import { SelectRecoveryOption, ResumeRun } from './RecoveryOptions' -import { ErrorRecoveryHeader } from './ErrorRecoveryHeader' -import { RecoveryInProgress } from './RecoveryInProgress' -import { getErrorKind, useRouteUpdateActions } from './utils' -import { RECOVERY_MAP } from './constants' +import { ErrorRecoveryWizard } from './ErrorRecoveryWizard' +import { useCurrentlyFailedRunCommand } from './utils' -import type { IRecoveryMap, RecoveryContentProps } from './types' +import type { RunStatus } from '@opentrons/api-client' +import type { FailedCommand } from './types' -interface ErrorRecoveryProps { - onComplete: () => void - errorType?: string -} -export function ErrorRecoveryFlows({ - onComplete, - errorType, -}: ErrorRecoveryProps): JSX.Element { - /** - * Recovery Route: A logically-related collection of recovery steps or a single step if unrelated to any existing recovery route. - * Recovery Step: Analogous to a "step" in other wizard flows. - */ - const [recoveryMap, setRecoveryMap] = React.useState({ - route: RECOVERY_MAP.BEFORE_BEGINNING.ROUTE, - step: RECOVERY_MAP.BEFORE_BEGINNING.STEPS.RECOVERY_DESCRIPTION, - }) - - const errorKind = getErrorKind(errorType) - const isOnDevice = useSelector(getIsOnDevice) - - const routeUpdateActions = useRouteUpdateActions({ - recoveryMap, - setRecoveryMap, - }) - - return ( - - ) -} - -function ErrorRecoveryComponent(props: RecoveryContentProps): JSX.Element { - return createPortal( - - - - , - getTopPortalEl() - ) +interface UseErrorRecoveryResult { + isEREnabled: boolean + failedCommand: FailedCommand | null + toggleER: () => void } -export function ErrorRecoveryContent(props: RecoveryContentProps): JSX.Element { - const buildBeforeBeginning = (): JSX.Element => { - return - } - - const buildSelectRecoveryOption = (): JSX.Element => { - return - } +export function useErrorRecoveryFlows( + runId: string, + runStatus: RunStatus | null +): UseErrorRecoveryResult { + const [isEREnabled, setIsEREnabled] = React.useState(false) + const failedCommand = useCurrentlyFailedRunCommand(runId, runStatus) - const buildRecoveryInProgress = (): JSX.Element => { - return + const toggleER = (): void => { + setIsEREnabled(!isEREnabled) } - const buildResumeRun = (): JSX.Element => { - return + // Because multiple ER flows may occur per run, disable ER when the status is not "awaiting-recovery." + React.useEffect(() => { + if (isEREnabled && runStatus !== RUN_STATUS_AWAITING_RECOVERY) { + setIsEREnabled(false) + } + }, [isEREnabled, runStatus]) + + return { + isEREnabled, + failedCommand, + toggleER, } +} - switch (props.recoveryMap.route) { - case RECOVERY_MAP.BEFORE_BEGINNING.ROUTE: - return buildBeforeBeginning() - case RECOVERY_MAP.OPTION_SELECTION.ROUTE: - return buildSelectRecoveryOption() - case RECOVERY_MAP.RESUME.ROUTE: - return buildResumeRun() - case RECOVERY_MAP.ROBOT_IN_MOTION.ROUTE: - case RECOVERY_MAP.ROBOT_RESUMING.ROUTE: - return buildRecoveryInProgress() - default: - return buildSelectRecoveryOption() - } +interface ErrorRecoveryFlowsProps { + runId: string + failedCommand: FailedCommand | null +} +export function ErrorRecoveryFlows({ + runId, + failedCommand, +}: ErrorRecoveryFlowsProps): JSX.Element | null { + return } diff --git a/app/src/organisms/ErrorRecoveryFlows/types.ts b/app/src/organisms/ErrorRecoveryFlows/types.ts index 51a3f4deb28..794b36e679e 100644 --- a/app/src/organisms/ErrorRecoveryFlows/types.ts +++ b/app/src/organisms/ErrorRecoveryFlows/types.ts @@ -1,11 +1,15 @@ +import type { RunCommandSummary } from '@opentrons/api-client' import type { ERROR_KINDS, RECOVERY_MAP, INVALID } from './constants' import type { UseRouteUpdateActionsResult } from './utils' +import type { UseRecoveryCommandsResult } from './useRecoveryCommands' +export type FailedCommand = RunCommandSummary export type InvalidStep = typeof INVALID export type RecoveryRoute = typeof RECOVERY_MAP[keyof typeof RECOVERY_MAP]['ROUTE'] export type RobotMovingRoute = | typeof RECOVERY_MAP['ROBOT_IN_MOTION']['ROUTE'] | typeof RECOVERY_MAP['ROBOT_RESUMING']['ROUTE'] + | typeof RECOVERY_MAP['ROBOT_RETRYING_COMMAND']['ROUTE'] export type ErrorKind = keyof typeof ERROR_KINDS interface RecoveryMapDetails { @@ -25,6 +29,7 @@ type RecoveryStep< type RobotInMotionStep = RecoveryStep<'ROBOT_IN_MOTION'> type RobotResumingStep = RecoveryStep<'ROBOT_RESUMING'> +type RobotRetryingCommandStep = RecoveryStep<'ROBOT_RETRYING_COMMAND'> type BeforeBeginningStep = RecoveryStep<'BEFORE_BEGINNING'> type CancelRunStep = RecoveryStep<'CANCEL_RUN'> type DropTipStep = RecoveryStep<'DROP_TIP'> @@ -36,6 +41,7 @@ type OptionSelectionStep = RecoveryStep<'OPTION_SELECTION'> export type RouteStep = | RobotInMotionStep | RobotResumingStep + | RobotRetryingCommandStep | BeforeBeginningStep | CancelRunStep | DropTipStep @@ -50,9 +56,10 @@ export interface IRecoveryMap { } export interface RecoveryContentProps { + failedCommand: FailedCommand | null errorKind: ErrorKind isOnDevice: boolean recoveryMap: IRecoveryMap routeUpdateActions: UseRouteUpdateActionsResult - onComplete: () => void + recoveryCommands: UseRecoveryCommandsResult } diff --git a/app/src/organisms/ErrorRecoveryFlows/useRecoveryCommands.ts b/app/src/organisms/ErrorRecoveryFlows/useRecoveryCommands.ts new file mode 100644 index 00000000000..78cab04f2d4 --- /dev/null +++ b/app/src/organisms/ErrorRecoveryFlows/useRecoveryCommands.ts @@ -0,0 +1,68 @@ +import * as React from 'react' + +import { useResumeRunFromRecoveryMutation } from '@opentrons/react-api-client' + +import { useChainRunCommands } from '../../resources/runs' + +import type { CreateCommand } from '@opentrons/shared-data' +import type { CommandData } from '@opentrons/api-client' +import type { FailedCommand } from './types' + +interface UseRecoveryCommandsParams { + runId: string + failedCommand: FailedCommand | null +} +export interface UseRecoveryCommandsResult { + resumeRun: () => void + retryFailedCommand: () => Promise + homePipetteZAxes: () => Promise +} +// Returns recovery command functions. +export function useRecoveryCommands({ + runId, + failedCommand, +}: UseRecoveryCommandsParams): UseRecoveryCommandsResult { + const { chainRunCommands } = useChainRunCommands(runId, failedCommand?.id) + const { resumeRunFromRecovery } = useResumeRunFromRecoveryMutation() + + const chainRunRecoveryCommands = React.useCallback( + ( + commands: CreateCommand[], + continuePastFailure: boolean = true + ): Promise => + chainRunCommands(commands, continuePastFailure).catch(e => { + // the catch never occurs if continuePastCommandFailure is "true" + return Promise.reject(new Error(`placeholder error ${e}`)) + }), + [chainRunCommands] + ) + + const retryFailedCommand = React.useCallback((): Promise => { + const { commandType, params } = failedCommand as FailedCommand // Null case is handled before command could be issued. + + return chainRunRecoveryCommands([ + { commandType, params }, + ] as CreateCommand[]) // the created command is the same command that failed + }, [chainRunRecoveryCommands, failedCommand]) + + // Homes the Z-axis of all attached pipettes. + const homePipetteZAxes = React.useCallback((): Promise => { + return chainRunRecoveryCommands([HOME_PIPETTE_Z_AXES]) + }, [chainRunRecoveryCommands]) + + const resumeRun = React.useCallback((): void => { + resumeRunFromRecovery(runId) + }, [runId, resumeRunFromRecovery]) + + return { + resumeRun, + retryFailedCommand, + homePipetteZAxes, + } +} + +export const HOME_PIPETTE_Z_AXES: CreateCommand = { + commandType: 'home', + params: { axes: ['leftZ', 'rightZ'] }, + intent: 'fixit', +} diff --git a/app/src/organisms/ErrorRecoveryFlows/utils.ts b/app/src/organisms/ErrorRecoveryFlows/utils.ts index 3a6c23e73dd..ef42149410f 100644 --- a/app/src/organisms/ErrorRecoveryFlows/utils.ts +++ b/app/src/organisms/ErrorRecoveryFlows/utils.ts @@ -1,18 +1,66 @@ import * as React from 'react' import { useTranslation } from 'react-i18next' -import last from 'lodash/last' import head from 'lodash/head' +import last from 'lodash/last' +import findLast from 'lodash/findLast' + +import { useAllCommandsQuery } from '@opentrons/react-api-client' +import { RUN_STATUS_AWAITING_RECOVERY } from '@opentrons/api-client' import { RECOVERY_MAP, ERROR_KINDS, INVALID, STEP_ORDER } from './constants' +import type { RunStatus } from '@opentrons/api-client' import type { RouteStep, IRecoveryMap, RecoveryRoute, ErrorKind, RobotMovingRoute, + FailedCommand, } from './types' +// TODO(jh, 05-09-24): Migrate utils, useRecoveryCommands.ts, and respective tests to a utils dir, and make each util a separate file. + +// TODO(jh, 05-09-24): It is possible for a desktop app not to detect the most recently failed run command if there are +// too many FIXIT commands. See PR associated with TODO for explanation. + +// While the run is "awaiting-recovery", return the most recently failed run command with a protocol intent. +// Otherwise, returns null. +const ALL_COMMANDS_POLL_MS = 5000 + +export function useCurrentlyFailedRunCommand( + runId: string, + runStatus: RunStatus | null +): FailedCommand | null { + const [ + recentFailedCommand, + setRecentFailedCommand, + ] = React.useState(null) + // The most recently failed protocol command causes the run to enter "awaiting-recovery", therefore only check + // for a newly failed command when the run first enters "awaiting-recovery." + const isRunStatusAwaitingRecovery = runStatus === RUN_STATUS_AWAITING_RECOVERY + + const { data: allCommandsQueryData } = useAllCommandsQuery(runId, null, { + enabled: isRunStatusAwaitingRecovery && recentFailedCommand == null, + refetchInterval: ALL_COMMANDS_POLL_MS, + }) + + React.useEffect(() => { + if (isRunStatusAwaitingRecovery && recentFailedCommand == null) { + const failedCommand = + findLast( + allCommandsQueryData?.data, + command => command.status === 'failed' && command.intent !== 'fixit' + ) ?? null + setRecentFailedCommand(failedCommand) + } else if (!isRunStatusAwaitingRecovery && recentFailedCommand != null) { + setRecentFailedCommand(null) + } + }, [isRunStatusAwaitingRecovery, recentFailedCommand, allCommandsQueryData]) + + return recentFailedCommand +} + export function useErrorName(errorKind: ErrorKind): string { const { t } = useTranslation('error_recovery') @@ -44,10 +92,13 @@ export interface GetRouteUpdateActionsParams { setRecoveryMap: (recoveryMap: IRecoveryMap) => void } export interface UseRouteUpdateActionsResult { - goBackPrevStep: () => void - proceedNextStep: () => void - proceedToRoute: (route: RecoveryRoute) => void - setRobotInMotion: (inMotion: boolean, movingRoute?: RobotMovingRoute) => void + goBackPrevStep: () => Promise + proceedNextStep: () => Promise + proceedToRoute: (route: RecoveryRoute) => Promise + setRobotInMotion: ( + inMotion: boolean, + movingRoute?: RobotMovingRoute + ) => Promise } // Utilities related to routing within the error recovery flows. export function useRouteUpdateActions({ @@ -59,73 +110,92 @@ export function useRouteUpdateActions({ const { OPTION_SELECTION, ROBOT_IN_MOTION } = RECOVERY_MAP // Redirect to the previous step for the current route if it exists, otherwise redirects to the option selection route. - const goBackPrevStep = React.useCallback((): void => { - const { getPrevStep } = getRecoveryRouteNavigation(currentRoute) - const updatedStep = getPrevStep(currentStep) - - if (updatedStep === INVALID) { - setRecoveryMap({ - route: OPTION_SELECTION.ROUTE, - step: OPTION_SELECTION.STEPS.SELECT, - }) - } else { - setRecoveryMap({ route: currentRoute, step: updatedStep }) - } + const goBackPrevStep = React.useCallback((): Promise => { + return new Promise((resolve, reject) => { + const { getPrevStep } = getRecoveryRouteNavigation(currentRoute) + const updatedStep = getPrevStep(currentStep) + + if (updatedStep === INVALID) { + setRecoveryMap({ + route: OPTION_SELECTION.ROUTE, + step: OPTION_SELECTION.STEPS.SELECT, + }) + } else { + setRecoveryMap({ route: currentRoute, step: updatedStep }) + } + + resolve() + }) }, [currentStep, currentRoute]) // Redirect to the next step for the current route if it exists, otherwise redirects to the option selection route. - const proceedNextStep = React.useCallback((): void => { - const { getNextStep } = getRecoveryRouteNavigation(currentRoute) - const updatedStep = getNextStep(currentStep) - - if (updatedStep === INVALID) { - setRecoveryMap({ - route: OPTION_SELECTION.ROUTE, - step: OPTION_SELECTION.STEPS.SELECT, - }) - } else { - setRecoveryMap({ route: currentRoute, step: updatedStep }) - } - }, [currentStep, currentRoute]) + const proceedNextStep = React.useCallback((): Promise => { + return new Promise((resolve, reject) => { + const { getNextStep } = getRecoveryRouteNavigation(currentRoute) + const updatedStep = getNextStep(currentStep) - // Redirect to a specific route. - const proceedToRoute = React.useCallback((route: RecoveryRoute): void => { - const newFlowSteps = STEP_ORDER[route] + if (updatedStep === INVALID) { + setRecoveryMap({ + route: OPTION_SELECTION.ROUTE, + step: OPTION_SELECTION.STEPS.SELECT, + }) + } else { + setRecoveryMap({ route: currentRoute, step: updatedStep }) + } - setRecoveryMap({ - route, - step: head(newFlowSteps) as RouteStep, + resolve() }) - }, []) + }, [currentStep, currentRoute]) - // Stashes the current map then sets the current map to robot in motion. Restores the map after motion completes. - const setRobotInMotion = React.useCallback( - (inMotion: boolean, robotMovingRoute?: RobotMovingRoute): void => { - if (inMotion) { - if (stashedMap == null) { - setStashedMap({ route: currentRoute, step: currentStep }) - } - const route = robotMovingRoute ?? ROBOT_IN_MOTION.ROUTE - const step = - robotMovingRoute != null - ? (head(STEP_ORDER[robotMovingRoute]) as RouteStep) - : ROBOT_IN_MOTION.STEPS.IN_MOTION + // Redirect to a specific route. + const proceedToRoute = React.useCallback( + (route: RecoveryRoute): Promise => { + return new Promise((resolve, reject) => { + const newFlowSteps = STEP_ORDER[route] setRecoveryMap({ route, - step, + step: head(newFlowSteps) as RouteStep, }) - } else { - if (stashedMap != null) { - setRecoveryMap(stashedMap) - setStashedMap(null) - } else { + + resolve() + }) + }, + [] + ) + + // Stashes the current map then sets the current map to robot in motion. Restores the map after motion completes. + const setRobotInMotion = React.useCallback( + (inMotion: boolean, robotMovingRoute?: RobotMovingRoute): Promise => { + return new Promise((resolve, reject) => { + if (inMotion) { + if (stashedMap == null) { + setStashedMap({ route: currentRoute, step: currentStep }) + } + const route = robotMovingRoute ?? ROBOT_IN_MOTION.ROUTE + const step = + robotMovingRoute != null + ? (head(STEP_ORDER[robotMovingRoute]) as RouteStep) + : ROBOT_IN_MOTION.STEPS.IN_MOTION + setRecoveryMap({ - route: OPTION_SELECTION.ROUTE, - step: OPTION_SELECTION.STEPS.SELECT, + route, + step, }) + } else { + if (stashedMap != null) { + setRecoveryMap(stashedMap) + setStashedMap(null) + } else { + setRecoveryMap({ + route: OPTION_SELECTION.ROUTE, + step: OPTION_SELECTION.STEPS.SELECT, + }) + } } - } + + resolve() + }) }, [currentRoute, currentStep, stashedMap] ) diff --git a/app/src/organisms/RunTimeControl/__tests__/hooks.test.tsx b/app/src/organisms/RunTimeControl/__tests__/hooks.test.tsx index a46bc37d865..ed3e2311604 100644 --- a/app/src/organisms/RunTimeControl/__tests__/hooks.test.tsx +++ b/app/src/organisms/RunTimeControl/__tests__/hooks.test.tsx @@ -51,14 +51,17 @@ describe('useRunControls hook', () => { const mockPauseRun = vi.fn() const mockStopRun = vi.fn() const mockCloneRun = vi.fn() + const mockResumeRunFromRecovery = vi.fn() when(useRunActionMutations).calledWith(mockPausedRun.id).thenReturn({ playRun: mockPlayRun, pauseRun: mockPauseRun, stopRun: mockStopRun, + resumeRunFromRecovery: mockResumeRunFromRecovery, isPlayRunActionLoading: false, isPauseRunActionLoading: false, isStopRunActionLoading: false, + isResumeRunFromRecoveryActionLoading: false, }) when(useCloneRun) .calledWith(mockPausedRun.id, undefined, true) diff --git a/app/src/organisms/RunTimeControl/hooks.ts b/app/src/organisms/RunTimeControl/hooks.ts index d513fcbe118..076dc3f8a22 100644 --- a/app/src/organisms/RunTimeControl/hooks.ts +++ b/app/src/organisms/RunTimeControl/hooks.ts @@ -32,9 +32,11 @@ export interface RunControls { pause: () => void stop: () => void reset: () => void + resumeFromRecovery: () => void isPlayRunActionLoading: boolean isPauseRunActionLoading: boolean isStopRunActionLoading: boolean + isResumeRunFromRecoveryActionLoading: boolean isResetRunLoading: boolean } @@ -46,9 +48,11 @@ export function useRunControls( playRun, pauseRun, stopRun, + resumeRunFromRecovery, isPlayRunActionLoading, isPauseRunActionLoading, isStopRunActionLoading, + isResumeRunFromRecoveryActionLoading, } = useRunActionMutations(runId as string) const { cloneRun, isLoading: isResetRunLoading } = useCloneRun( @@ -62,9 +66,11 @@ export function useRunControls( pause: pauseRun, stop: stopRun, reset: cloneRun, + resumeFromRecovery: resumeRunFromRecovery, isPlayRunActionLoading, isPauseRunActionLoading, isStopRunActionLoading, + isResumeRunFromRecoveryActionLoading, isResetRunLoading, } } diff --git a/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx b/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx index 623761693f3..55daf6aa987 100644 --- a/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx +++ b/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx @@ -218,10 +218,12 @@ describe('ProtocolSetup', () => { pause: () => {}, stop: () => {}, reset: () => {}, + resumeFromRecovery: () => {}, isPlayRunActionLoading: false, isPauseRunActionLoading: false, isStopRunActionLoading: false, isResetRunLoading: false, + isResumeRunFromRecoveryActionLoading: false, }) when(vi.mocked(useRunStatus)).calledWith(RUN_ID).thenReturn(RUN_STATUS_IDLE) vi.mocked(useProtocolAnalysisAsDocumentQuery).mockReturnValue({ diff --git a/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx b/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx index acf08a15d77..8a56a637275 100644 --- a/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx +++ b/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx @@ -2,6 +2,7 @@ import * as React from 'react' import { Route, MemoryRouter } from 'react-router-dom' import { vi, it, describe, expect, beforeEach, afterEach } from 'vitest' import { when } from 'vitest-when' +import { screen } from '@testing-library/react' import { RUN_STATUS_BLOCKED_BY_OPEN_DOOR, @@ -39,6 +40,10 @@ import { useNotifyRunQuery, } from '../../../resources/runs' import { useFeatureFlag } from '../../../redux/config' +import { + ErrorRecoveryFlows, + useErrorRecoveryFlows, +} from '../../../organisms/ErrorRecoveryFlows' import type { UseQueryResult } from 'react-query' import type { ProtocolAnalyses, RunCommandSummary } from '@opentrons/api-client' @@ -58,6 +63,7 @@ vi.mock('../../../organisms/OnDeviceDisplay/RunningProtocol/CancelingRunModal') vi.mock('../../../organisms/OpenDoorAlertModal') vi.mock('../../../resources/runs') vi.mock('../../../redux/config') +vi.mock('../../../organisms/ErrorRecoveryFlows') const RUN_ID = 'run_id' const ROBOT_NAME = 'otie' @@ -71,6 +77,7 @@ const PROTOCOL_ANALYSIS = { const mockPlayRun = vi.fn() const mockPauseRun = vi.fn() const mockStopRun = vi.fn() +const mockResumeRunFromRecovery = vi.fn() const render = (path = '/') => { return renderWithProviders( @@ -127,9 +134,11 @@ describe('RunningProtocol', () => { playRun: mockPlayRun, pauseRun: mockPauseRun, stopRun: mockStopRun, + resumeRunFromRecovery: mockResumeRunFromRecovery, isPlayRunActionLoading: false, isPauseRunActionLoading: false, isStopRunActionLoading: false, + isResumeRunFromRecoveryActionLoading: false, }) when(vi.mocked(useMostRecentCompletedAnalysis)) .calledWith(RUN_ID) @@ -143,6 +152,14 @@ describe('RunningProtocol', () => { when(vi.mocked(useFeatureFlag)) .calledWith('enableRunNotes') .thenReturn(true) + vi.mocked(ErrorRecoveryFlows).mockReturnValue( +
MOCK ERROR RECOVERY
+ ) + vi.mocked(useErrorRecoveryFlows).mockReturnValue({ + isEREnabled: false, + toggleER: vi.fn(), + failedCommand: {} as any, + }) }) afterEach(() => { @@ -184,6 +201,19 @@ describe('RunningProtocol', () => { expect(vi.mocked(RunPausedSplash)).toHaveBeenCalled() }) + it('should render ErrorRecovery appropriately', () => { + render(`/runs/${RUN_ID}/run`) + expect(screen.queryByText('MOCK ERROR RECOVERY')).not.toBeInTheDocument() + + vi.mocked(useErrorRecoveryFlows).mockReturnValue({ + isEREnabled: true, + toggleER: vi.fn(), + failedCommand: {} as any, + }) + render(`/runs/${RUN_ID}/run`) + screen.getByText('MOCK ERROR RECOVERY') + }) + // ToDo (kj:04/04/2023) need to figure out the way to simulate swipe it.todo('should render RunningProtocolCommandList when swiping left') // const [{ getByText }] = render(`/runs/${RUN_ID}/run`) diff --git a/app/src/pages/RunningProtocol/index.tsx b/app/src/pages/RunningProtocol/index.tsx index 0bd793acc8f..463894ea775 100644 --- a/app/src/pages/RunningProtocol/index.tsx +++ b/app/src/pages/RunningProtocol/index.tsx @@ -55,7 +55,10 @@ import { ConfirmCancelRunModal } from '../../organisms/OnDeviceDisplay/RunningPr import { RunPausedSplash } from '../../organisms/OnDeviceDisplay/RunningProtocol/RunPausedSplash' import { getLocalRobot } from '../../redux/discovery' import { OpenDoorAlertModal } from '../../organisms/OpenDoorAlertModal' -import { ErrorRecoveryFlows } from '../../organisms/ErrorRecoveryFlows' +import { + useErrorRecoveryFlows, + ErrorRecoveryFlows, +} from '../../organisms/ErrorRecoveryFlows' import type { OnDeviceRouteParams } from '../../App/types' @@ -106,8 +109,6 @@ export function RunningProtocol(): JSX.Element { const runStatus = useRunStatus(runId, { refetchInterval: RUN_STATUS_REFETCH_INTERVAL, }) - const [enableSplash, setEnableSplash] = React.useState(true) - const [showErrorRecovery, setShowErrorRecovery] = React.useState(false) const { startedAt, stoppedAt, completedAt } = useRunTimestamps(runId) const { data: runRecord } = useNotifyRunQuery(runId, { staleTime: Infinity }) const protocolId = runRecord?.data.protocolId ?? null @@ -123,9 +124,12 @@ export function RunningProtocol(): JSX.Element { const { trackProtocolRunEvent } = useTrackProtocolRunEvent(runId, robotName) const robotAnalyticsData = useRobotAnalyticsData(robotName) const robotType = useRobotType(robotName) - const errorType = runRecord?.data.errors[0]?.errorType - const enableRunNotes = useFeatureFlag('enableRunNotes') + const { isEREnabled, failedCommand, toggleER } = useErrorRecoveryFlows( + runId, + runStatus + ) + const errorType = failedCommand?.error?.errorType React.useEffect(() => { if ( @@ -162,21 +166,14 @@ export function RunningProtocol(): JSX.Element { } }, [lastRunCommand, interventionModalCommandKey]) - const handleCompleteRecovery = (): void => { - setShowErrorRecovery(false) - setEnableSplash(false) - } - return ( <> - {showErrorRecovery ? ( - + {isEREnabled ? ( + ) : null} - {enableSplash && - runStatus === RUN_STATUS_AWAITING_RECOVERY && - enableRunNotes ? ( + {runStatus === RUN_STATUS_AWAITING_RECOVERY && enableRunNotes ? ( setShowErrorRecovery(true)} + onClick={toggleER} errorType={errorType} protocolName={protocolName} /> diff --git a/react-api-client/src/runs/__fixtures__/runActions.ts b/react-api-client/src/runs/__fixtures__/runActions.ts index 41c333dd459..5bb13a2d06f 100644 --- a/react-api-client/src/runs/__fixtures__/runActions.ts +++ b/react-api-client/src/runs/__fixtures__/runActions.ts @@ -3,6 +3,7 @@ import { RUN_ACTION_TYPE_PLAY, RUN_ACTION_TYPE_PAUSE, RUN_ACTION_TYPE_STOP, + RUN_ACTION_TYPE_RESUME_FROM_RECOVERY, } from '@opentrons/api-client' export const mockPlayRunAction: RunAction = { @@ -22,3 +23,9 @@ export const mockStopRunAction: RunAction = { createdAt: '2021-10-25T13:23:31.366581+00:00', actionType: RUN_ACTION_TYPE_STOP, } + +export const mockResumeFromRecoveryAction: RunAction = { + id: '4', + createdAt: '2021-10-25T13:23:31.366581+00:00', + actionType: RUN_ACTION_TYPE_RESUME_FROM_RECOVERY, +} diff --git a/react-api-client/src/runs/__tests__/useResumeRunFromRecoveryMutation.test.tsx b/react-api-client/src/runs/__tests__/useResumeRunFromRecoveryMutation.test.tsx new file mode 100644 index 00000000000..a3ebe42a73d --- /dev/null +++ b/react-api-client/src/runs/__tests__/useResumeRunFromRecoveryMutation.test.tsx @@ -0,0 +1,64 @@ +import * as React from 'react' +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { QueryClient, QueryClientProvider } from 'react-query' +import { act, renderHook, waitFor } from '@testing-library/react' +import { createRunAction } from '@opentrons/api-client' +import { useHost } from '../../api' +import { useResumeRunFromRecoveryMutation } from '..' + +import { RUN_ID_1, mockResumeFromRecoveryAction } from '../__fixtures__' + +import type { HostConfig, Response, RunAction } from '@opentrons/api-client' +import type { UsePlayRunMutationOptions } from '../usePlayRunMutation' + +vi.mock('@opentrons/api-client') +vi.mock('../../api/useHost') + +const HOST_CONFIG: HostConfig = { hostname: 'localhost' } + +describe('usePlayRunMutation hook', () => { + let wrapper: React.FunctionComponent< + { children: React.ReactNode } & UsePlayRunMutationOptions + > + + beforeEach(() => { + const queryClient = new QueryClient() + const clientProvider: React.FunctionComponent< + { children: React.ReactNode } & UsePlayRunMutationOptions + > = ({ children }) => ( + {children} + ) + wrapper = clientProvider + }) + + it('should return no data when calling resumeRunFromRecovery if the request fails', async () => { + vi.mocked(useHost).mockReturnValue(HOST_CONFIG) + vi.mocked(createRunAction).mockRejectedValue('oh no') + + const { result } = renderHook(useResumeRunFromRecoveryMutation, { + wrapper, + }) + + expect(result.current.data).toBeUndefined() + act(() => result.current.resumeRunFromRecovery(RUN_ID_1)) + await waitFor(() => { + expect(result.current.data).toBeUndefined() + }) + }) + + it('should create a resumeFromRecovery run action when calling the resumeRunFromRecovery callback', async () => { + vi.mocked(useHost).mockReturnValue(HOST_CONFIG) + vi.mocked(createRunAction).mockResolvedValue({ + data: mockResumeFromRecoveryAction, + } as Response) + + const { result } = renderHook(useResumeRunFromRecoveryMutation, { + wrapper, + }) + act(() => result.current.resumeRunFromRecovery(RUN_ID_1)) + + await waitFor(() => { + expect(result.current.data).toEqual(mockResumeFromRecoveryAction) + }) + }) +}) diff --git a/react-api-client/src/runs/__tests__/useRunActionMutations.test.tsx b/react-api-client/src/runs/__tests__/useRunActionMutations.test.tsx index 0a6390889a0..24aafe14ae2 100644 --- a/react-api-client/src/runs/__tests__/useRunActionMutations.test.tsx +++ b/react-api-client/src/runs/__tests__/useRunActionMutations.test.tsx @@ -9,14 +9,17 @@ import { UsePlayRunMutationResult, UsePauseRunMutationResult, UseStopRunMutationResult, + UseResumeRunFromRecoveryMutationResult, usePlayRunMutation, usePauseRunMutation, useStopRunMutation, + useResumeRunFromRecoveryMutation, } from '..' vi.mock('../usePlayRunMutation') vi.mock('../usePauseRunMutation') vi.mock('../useStopRunMutation') +vi.mock('../useResumeRunFromRecoveryMutation') describe('useRunActionMutations hook', () => { let wrapper: React.FunctionComponent<{ children: React.ReactNode }> @@ -38,6 +41,7 @@ describe('useRunActionMutations hook', () => { const mockPlayRun = vi.fn() const mockPauseRun = vi.fn() const mockStopRun = vi.fn() + const mockResumeRunFromRecovery = vi.fn() vi.mocked(usePlayRunMutation).mockReturnValue(({ playRun: mockPlayRun, @@ -51,6 +55,10 @@ describe('useRunActionMutations hook', () => { stopRun: mockStopRun, } as unknown) as UseStopRunMutationResult) + vi.mocked(useResumeRunFromRecoveryMutation).mockReturnValue(({ + resumeRunFromRecovery: mockResumeRunFromRecovery, + } as unknown) as UseResumeRunFromRecoveryMutationResult) + const { result } = renderHook(() => useRunActionMutations(RUN_ID_1), { wrapper, }) @@ -64,5 +72,8 @@ describe('useRunActionMutations hook', () => { act(() => result.current.stopRun()) expect(mockStopRun).toHaveBeenCalledTimes(1) expect(mockStopRun).toHaveBeenCalledWith(RUN_ID_1) + act(() => result.current.resumeRunFromRecovery()) + expect(mockResumeRunFromRecovery).toHaveBeenCalledTimes(1) + expect(mockResumeRunFromRecovery).toHaveBeenCalledWith(RUN_ID_1) }) }) diff --git a/react-api-client/src/runs/index.ts b/react-api-client/src/runs/index.ts index 5790abb860b..207950738e1 100644 --- a/react-api-client/src/runs/index.ts +++ b/react-api-client/src/runs/index.ts @@ -8,6 +8,7 @@ export { useDismissCurrentRunMutation } from './useDismissCurrentRunMutation' export { usePlayRunMutation } from './usePlayRunMutation' export { usePauseRunMutation } from './usePauseRunMutation' export { useStopRunMutation } from './useStopRunMutation' +export { useResumeRunFromRecoveryMutation } from './useResumeRunFromRecoveryMutation' export { useRunActionMutations } from './useRunActionMutations' export { useAllCommandsQuery } from './useAllCommandsQuery' export { useAllCommandsAsPreSerializedList } from './useAllCommandsAsPreSerializedList' @@ -18,3 +19,4 @@ export * from './useCreateLabwareDefinitionMutation' export type { UsePlayRunMutationResult } from './usePlayRunMutation' export type { UsePauseRunMutationResult } from './usePauseRunMutation' export type { UseStopRunMutationResult } from './useStopRunMutation' +export type { UseResumeRunFromRecoveryMutationResult } from './useResumeRunFromRecoveryMutation' diff --git a/react-api-client/src/runs/useResumeRunFromRecoveryMutation.ts b/react-api-client/src/runs/useResumeRunFromRecoveryMutation.ts new file mode 100644 index 00000000000..a7af94a99e7 --- /dev/null +++ b/react-api-client/src/runs/useResumeRunFromRecoveryMutation.ts @@ -0,0 +1,53 @@ +import { + UseMutationResult, + useMutation, + UseMutateFunction, + UseMutationOptions, +} from 'react-query' + +import { + HostConfig, + RunAction, + RUN_ACTION_TYPE_RESUME_FROM_RECOVERY, + createRunAction, +} from '@opentrons/api-client' + +import { useHost } from '../api' + +import type { AxiosError } from 'axios' + +export type UseResumeRunFromRecoveryMutationResult = UseMutationResult< + RunAction, + AxiosError, + string +> & { + resumeRunFromRecovery: UseMutateFunction +} + +export type UseResumeRunFromRecoveryMutationOptions = UseMutationOptions< + RunAction, + AxiosError, + string +> + +export const useResumeRunFromRecoveryMutation = ( + options: UseResumeRunFromRecoveryMutationOptions = {} +): UseResumeRunFromRecoveryMutationResult => { + const host = useHost() + const mutation = useMutation( + [host, 'runs', RUN_ACTION_TYPE_RESUME_FROM_RECOVERY], + (runId: string) => + createRunAction(host as HostConfig, runId, { + actionType: RUN_ACTION_TYPE_RESUME_FROM_RECOVERY, + }) + .then(response => response.data) + .catch(e => { + throw e + }), + options + ) + return { + ...mutation, + resumeRunFromRecovery: mutation.mutate, + } +} diff --git a/react-api-client/src/runs/useRunActionMutations.ts b/react-api-client/src/runs/useRunActionMutations.ts index b178af3d5d7..7d0eb6158ad 100644 --- a/react-api-client/src/runs/useRunActionMutations.ts +++ b/react-api-client/src/runs/useRunActionMutations.ts @@ -4,15 +4,18 @@ import { usePlayRunMutation, usePauseRunMutation, useStopRunMutation, + useResumeRunFromRecoveryMutation, } from '..' interface UseRunActionMutations { playRun: () => void pauseRun: () => void stopRun: () => void + resumeRunFromRecovery: () => void isPlayRunActionLoading: boolean isPauseRunActionLoading: boolean isStopRunActionLoading: boolean + isResumeRunFromRecoveryActionLoading: boolean } export function useRunActionMutations(runId: string): UseRunActionMutations { @@ -37,12 +40,19 @@ export function useRunActionMutations(runId: string): UseRunActionMutations { const { stopRun, isLoading: isStopRunActionLoading } = useStopRunMutation() + const { + resumeRunFromRecovery, + isLoading: isResumeRunFromRecoveryActionLoading, + } = useResumeRunFromRecoveryMutation() + return { playRun: () => playRun(runId), pauseRun: () => pauseRun(runId), stopRun: () => stopRun(runId), + resumeRunFromRecovery: () => resumeRunFromRecovery(runId), isPlayRunActionLoading, isPauseRunActionLoading, isStopRunActionLoading, + isResumeRunFromRecoveryActionLoading, } } From 7f119a135fba59ac79b4f71a930c4b8141a2a127 Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Mon, 20 May 2024 13:16:45 -0400 Subject: [PATCH 2/5] fix in motion routing --- .../ErrorRecoveryWizard.tsx | 7 +++-- .../__tests__/utils.test.ts | 4 +-- app/src/organisms/ErrorRecoveryFlows/utils.ts | 28 +++++++------------ 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx b/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx index 8f27a717f63..53923b314ad 100644 --- a/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx @@ -36,8 +36,8 @@ export function ErrorRecoveryWizard({ * Recovery Step: Analogous to a "step" in other wizard flows. */ const [recoveryMap, setRecoveryMap] = React.useState({ - route: RECOVERY_MAP.ROBOT_IN_MOTION.ROUTE, // Initialize the route to "in motion", since wizard always start with a home command. - step: RECOVERY_MAP.ROBOT_IN_MOTION.STEPS.IN_MOTION, + route: RECOVERY_MAP.OPTION_SELECTION.ROUTE, + step: RECOVERY_MAP.OPTION_SELECTION.STEPS.SELECT, }) const errorKind = getErrorKind(failedCommand?.error?.errorType) @@ -125,7 +125,8 @@ export function useInitialPipetteHome( const { homePipetteZAxes } = recoveryCommands const { setRobotInMotion } = routeUpdateActions - React.useEffect(() => { + // Synchronously set the recovery route to "robot in motion" before initial render to prevent screen flicker on ER launch. + React.useLayoutEffect(() => { void setRobotInMotion(true) .then(() => homePipetteZAxes()) .then(() => setRobotInMotion(false)) diff --git a/app/src/organisms/ErrorRecoveryFlows/__tests__/utils.test.ts b/app/src/organisms/ErrorRecoveryFlows/__tests__/utils.test.ts index dba68bfb84a..b91ad453591 100644 --- a/app/src/organisms/ErrorRecoveryFlows/__tests__/utils.test.ts +++ b/app/src/organisms/ErrorRecoveryFlows/__tests__/utils.test.ts @@ -139,8 +139,8 @@ describe('useRouteUpdateActions', () => { setRobotInMotion(false) expect(mockSetRecoveryMap).toHaveBeenCalledWith({ - route: RECOVERY_MAP.OPTION_SELECTION.ROUTE, - step: RECOVERY_MAP.OPTION_SELECTION.STEPS.SELECT, + route: RECOVERY_MAP.RESUME.ROUTE, + step: RECOVERY_MAP.RESUME.STEPS.CONFIRM_RESUME, }) }) }) diff --git a/app/src/organisms/ErrorRecoveryFlows/utils.ts b/app/src/organisms/ErrorRecoveryFlows/utils.ts index 3410878cb16..29c23d555c8 100644 --- a/app/src/organisms/ErrorRecoveryFlows/utils.ts +++ b/app/src/organisms/ErrorRecoveryFlows/utils.ts @@ -110,7 +110,7 @@ export function useRouteUpdateActions({ setRecoveryMap, }: GetRouteUpdateActionsParams): UseRouteUpdateActionsResult { const { route: currentRoute, step: currentStep } = recoveryMap - const [stashedMap, setStashedMap] = React.useState(null) + const stashedMapRef = React.useRef(null) const { OPTION_SELECTION, ROBOT_IN_MOTION } = RECOVERY_MAP // Redirect to the previous step for the current route if it exists, otherwise redirects to the option selection route. @@ -156,12 +156,7 @@ export function useRouteUpdateActions({ (route: RecoveryRoute): Promise => { return new Promise((resolve, reject) => { const newFlowSteps = STEP_ORDER[route] - - setRecoveryMap({ - route, - step: head(newFlowSteps) as RouteStep, - }) - + setRecoveryMap({ route, step: head(newFlowSteps) as RouteStep }) resolve() }) }, @@ -173,23 +168,20 @@ export function useRouteUpdateActions({ (inMotion: boolean, robotMovingRoute?: RobotMovingRoute): Promise => { return new Promise((resolve, reject) => { if (inMotion) { - if (stashedMap == null) { - setStashedMap({ route: currentRoute, step: currentStep }) + if (stashedMapRef.current == null) { + stashedMapRef.current = { route: currentRoute, step: currentStep } } + const route = robotMovingRoute ?? ROBOT_IN_MOTION.ROUTE const step = robotMovingRoute != null ? (head(STEP_ORDER[robotMovingRoute]) as RouteStep) : ROBOT_IN_MOTION.STEPS.IN_MOTION - - setRecoveryMap({ - route, - step, - }) + setRecoveryMap({ route, step }) } else { - if (stashedMap != null) { - setRecoveryMap(stashedMap) - setStashedMap(null) + if (stashedMapRef.current != null) { + setRecoveryMap(stashedMapRef.current) + stashedMapRef.current = null } else { setRecoveryMap({ route: OPTION_SELECTION.ROUTE, @@ -201,7 +193,7 @@ export function useRouteUpdateActions({ resolve() }) }, - [currentRoute, currentStep, stashedMap] + [currentRoute, currentStep] ) return { goBackPrevStep, proceedNextStep, proceedToRoute, setRobotInMotion } From a4f5c28564bb398151cabc42e4cc1f44ba136c15 Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Mon, 20 May 2024 13:26:11 -0400 Subject: [PATCH 3/5] feedback --- .../ErrorRecoveryFlows/ErrorRecoveryWizard.tsx | 2 +- .../__tests__/ErrorRecoveryFlows.test.tsx | 10 +++++----- app/src/organisms/ErrorRecoveryFlows/index.tsx | 14 +++++++------- app/src/organisms/ErrorRecoveryFlows/utils.ts | 4 +--- .../__tests__/RunningProtocol.test.tsx | 4 ++-- app/src/pages/RunningProtocol/index.tsx | 4 ++-- 6 files changed, 18 insertions(+), 20 deletions(-) diff --git a/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx b/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx index 53923b314ad..4c4c49deebd 100644 --- a/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx @@ -129,6 +129,6 @@ export function useInitialPipetteHome( React.useLayoutEffect(() => { void setRobotInMotion(true) .then(() => homePipetteZAxes()) - .then(() => setRobotInMotion(false)) + .finally(() => setRobotInMotion(false)) }, []) } diff --git a/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryFlows.test.tsx b/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryFlows.test.tsx index 782d0df6f16..d44e3d4b333 100644 --- a/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryFlows.test.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/__tests__/ErrorRecoveryFlows.test.tsx @@ -30,7 +30,7 @@ describe('useErrorRecovery', () => { useErrorRecoveryFlows('MOCK_ID', RUN_STATUS_RUNNING) ) - expect(result.current.isEREnabled).toBe(false) + expect(result.current.isERActive).toBe(false) }) it('should toggle the value of isEREnabled properly', () => { @@ -41,13 +41,13 @@ describe('useErrorRecovery', () => { result.current.toggleER() }) - expect(result.current.isEREnabled).toBe(true) + expect(result.current.isERActive).toBe(true) act(() => { result.current.toggleER() }) - expect(result.current.isEREnabled).toBe(false) + expect(result.current.isERActive).toBe(false) }) it('should disable error recovery when runStatus is not "awaiting-recovery"', () => { @@ -65,7 +65,7 @@ describe('useErrorRecovery', () => { // @ts-expect-error "running" is a valid status here rerender(RUN_STATUS_RUNNING) - expect(result.current.isEREnabled).toBe(false) + expect(result.current.isERActive).toBe(false) act(() => { result.current.toggleER() @@ -73,7 +73,7 @@ describe('useErrorRecovery', () => { rerender(RUN_STATUS_AWAITING_RECOVERY) - expect(result.current.isEREnabled).toBe(false) + expect(result.current.isERActive).toBe(false) }) it('should return the failed run command', () => { diff --git a/app/src/organisms/ErrorRecoveryFlows/index.tsx b/app/src/organisms/ErrorRecoveryFlows/index.tsx index 0db32dfa30a..b7da1f4caaf 100644 --- a/app/src/organisms/ErrorRecoveryFlows/index.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/index.tsx @@ -9,7 +9,7 @@ import type { RunStatus } from '@opentrons/api-client' import type { FailedCommand } from './types' interface UseErrorRecoveryResult { - isEREnabled: boolean + isERActive: boolean failedCommand: FailedCommand | null toggleER: () => void } @@ -18,22 +18,22 @@ export function useErrorRecoveryFlows( runId: string, runStatus: RunStatus | null ): UseErrorRecoveryResult { - const [isEREnabled, setIsEREnabled] = React.useState(false) + const [isERActive, setIsERActive] = React.useState(false) const failedCommand = useCurrentlyFailedRunCommand(runId, runStatus) const toggleER = (): void => { - setIsEREnabled(!isEREnabled) + setIsERActive(!isERActive) } // Because multiple ER flows may occur per run, disable ER when the status is not "awaiting-recovery." React.useEffect(() => { - if (isEREnabled && runStatus !== RUN_STATUS_AWAITING_RECOVERY) { - setIsEREnabled(false) + if (isERActive && runStatus !== RUN_STATUS_AWAITING_RECOVERY) { + setIsERActive(false) } - }, [isEREnabled, runStatus]) + }, [isERActive, runStatus]) return { - isEREnabled, + isERActive, failedCommand, toggleER, } diff --git a/app/src/organisms/ErrorRecoveryFlows/utils.ts b/app/src/organisms/ErrorRecoveryFlows/utils.ts index 29c23d555c8..749c314d7fb 100644 --- a/app/src/organisms/ErrorRecoveryFlows/utils.ts +++ b/app/src/organisms/ErrorRecoveryFlows/utils.ts @@ -21,13 +21,11 @@ import type { // TODO(jh, 05-09-24): Migrate utils, useRecoveryCommands.ts, and respective tests to a utils dir, and make each util a separate file. -// TODO(jh, 05-09-24): It is possible for a desktop app not to detect the most recently failed run command if there are -// too many FIXIT commands. See PR associated with TODO for explanation. - // While the run is "awaiting-recovery", return the most recently failed run command with a protocol intent. // Otherwise, returns null. const ALL_COMMANDS_POLL_MS = 5000 +// TODO(jh, 05-20-24): Update the logic for returning the failed run command once EXEC-458 merges. export function useCurrentlyFailedRunCommand( runId: string, runStatus: RunStatus | null diff --git a/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx b/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx index 4c8f1fa8ea9..77f5ae182ce 100644 --- a/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx +++ b/app/src/pages/RunningProtocol/__tests__/RunningProtocol.test.tsx @@ -157,7 +157,7 @@ describe('RunningProtocol', () => {
MOCK ERROR RECOVERY
) vi.mocked(useErrorRecoveryFlows).mockReturnValue({ - isEREnabled: false, + isERActive: false, toggleER: vi.fn(), failedCommand: {} as any, }) @@ -207,7 +207,7 @@ describe('RunningProtocol', () => { expect(screen.queryByText('MOCK ERROR RECOVERY')).not.toBeInTheDocument() vi.mocked(useErrorRecoveryFlows).mockReturnValue({ - isEREnabled: true, + isERActive: true, toggleER: vi.fn(), failedCommand: {} as any, }) diff --git a/app/src/pages/RunningProtocol/index.tsx b/app/src/pages/RunningProtocol/index.tsx index 2a9b2df84d2..8d1818b50a1 100644 --- a/app/src/pages/RunningProtocol/index.tsx +++ b/app/src/pages/RunningProtocol/index.tsx @@ -123,7 +123,7 @@ export function RunningProtocol(): JSX.Element { const robotAnalyticsData = useRobotAnalyticsData(robotName) const robotType = useRobotType(robotName) const enableRunNotes = useFeatureFlag('enableRunNotes') - const { isEREnabled, failedCommand, toggleER } = useErrorRecoveryFlows( + const { isERActive, failedCommand, toggleER } = useErrorRecoveryFlows( runId, runStatus ) @@ -166,7 +166,7 @@ export function RunningProtocol(): JSX.Element { return ( <> - {isEREnabled ? ( + {isERActive ? ( ) : null} {runStatus === RUN_STATUS_AWAITING_RECOVERY && enableRunNotes ? ( From 79bd847a0c5b9eed2fb8415ae97ba462bf93177d Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Mon, 20 May 2024 16:07:38 -0400 Subject: [PATCH 4/5] lint --- .../src/runs/useResumeRunFromRecoveryMutation.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/react-api-client/src/runs/useResumeRunFromRecoveryMutation.ts b/react-api-client/src/runs/useResumeRunFromRecoveryMutation.ts index a7af94a99e7..dac560f0e92 100644 --- a/react-api-client/src/runs/useResumeRunFromRecoveryMutation.ts +++ b/react-api-client/src/runs/useResumeRunFromRecoveryMutation.ts @@ -1,13 +1,6 @@ -import { - UseMutationResult, - useMutation, - UseMutateFunction, - UseMutationOptions, -} from 'react-query' +import { useMutation } from 'react-query' import { - HostConfig, - RunAction, RUN_ACTION_TYPE_RESUME_FROM_RECOVERY, createRunAction, } from '@opentrons/api-client' @@ -15,6 +8,12 @@ import { import { useHost } from '../api' import type { AxiosError } from 'axios' +import type { + UseMutateFunction, + UseMutationOptions, + UseMutationResult, +} from 'react-query' +import type { HostConfig, RunAction } from '@opentrons/api-client' export type UseResumeRunFromRecoveryMutationResult = UseMutationResult< RunAction, From c7a7fb01ad7abb0bf4ea238cda97545ecf11808e Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Tue, 21 May 2024 08:43:43 -0400 Subject: [PATCH 5/5] lower z-index for both desktop & odd --- app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx b/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx index 4c4c49deebd..55fbf5047b9 100644 --- a/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx +++ b/app/src/organisms/ErrorRecoveryFlows/ErrorRecoveryWizard.tsx @@ -11,7 +11,7 @@ import { } from '@opentrons/components' import { getIsOnDevice } from '../../redux/config' -import { getTopPortalEl } from '../../App/portal' +import { getModalPortalEl } from '../../App/portal' import { BeforeBeginning } from './BeforeBeginning' import { SelectRecoveryOption, ResumeRun } from './RecoveryOptions' import { ErrorRecoveryHeader } from './ErrorRecoveryHeader' @@ -80,7 +80,7 @@ function ErrorRecoveryComponent(props: RecoveryContentProps): JSX.Element { , - getTopPortalEl() + getModalPortalEl() ) }