Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(app): Update ignore error route in Error Recovery #16511

Merged
merged 2 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion app/src/assets/localization/en/error_recovery.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,13 @@
"homing_pipette_dangerous": "Homing the <bold>{{mount}} pipette</bold> with liquid in the tips may damage it. You must remove all tips before using the pipette again.",
"if_issue_persists_gripper_error": " If the issue persists, cancel the run and rerun gripper calibration",
"if_issue_persists_overpressure": " If the issue persists, cancel the run and make the necessary changes to the protocol",
"if_issue_persists_tip_not_detected": " If the issue persists, cancel the run and initiate Labware Position Check",
"if_issue_persists_tip_not_detected": " If the issue persists, cancel the run and perform Labware Position Check",
"if_tips_are_attached": "If tips are attached, you can choose to blow out any aspirated liquid and drop tips before the run is terminated.",
"ignore_all_errors_of_this_type": "Ignore all errors of this type",
"ignore_error_and_skip": "Ignore error and skip to next step",
"ignore_only_this_error": "Ignore only this error",
"ignore_similar_errors_later_in_run": "Ignore similar errors later in the run?",
"inspect_the_robot": "<block>First, inspect the robot to ensure it's prepared to continue the run from the next step.</block><block>Then, close the robot door before proceeding.</block>",
"labware_released_from_current_height": "The labware will be released from its current height.",
"launch_recovery_mode": "Launch Recovery Mode",
"manually_fill_liquid_in_well": "Manually fill liquid in well {{well}}",
Expand Down Expand Up @@ -70,6 +71,7 @@
"resume": "Resume",
"retry_dropping_tip": "Retry dropping tip",
"retry_now": "Retry now",
"retry_picking_up_tip": "Retry picking up tip",
"retry_step": "Retry step",
"retry_with_new_tips": "Retry with new tips",
"retry_with_same_tips": "Retry with same tips",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ export function ErrorRecoveryContent(props: RecoveryContentProps): JSX.Element {
return <RecoveryInProgress {...props} />
}

const buildResumeRun = (): JSX.Element => {
const buildRetryStep = (): JSX.Element => {
return <RetryStep {...props} />
}

Expand Down Expand Up @@ -246,7 +246,7 @@ export function ErrorRecoveryContent(props: RecoveryContentProps): JSX.Element {
case RECOVERY_MAP.ERROR_WHILE_RECOVERING.ROUTE:
return buildRecoveryError()
case RECOVERY_MAP.RETRY_STEP.ROUTE:
return buildResumeRun()
return buildRetryStep()
case RECOVERY_MAP.CANCEL_RUN.ROUTE:
return buildCancelRun()
case RECOVERY_MAP.DROP_TIP_FLOWS.ROUTE:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ export function SkipToNextStep(
const { ROBOT_SKIPPING_STEP, IGNORE_AND_SKIP } = RECOVERY_MAP
const { t } = useTranslation('error_recovery')

// TODO(jh, 06-18-24): EXEC-569
const secondaryBtnOnClick = (): void => {
if (selectedRecoveryOption === IGNORE_AND_SKIP.ROUTE) {
void proceedToRouteAndStep(IGNORE_AND_SKIP.ROUTE)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as React from 'react'
import { useState, useEffect } from 'react'
import head from 'lodash/head'
import { css } from 'styled-components'
import { useTranslation } from 'react-i18next'
Expand All @@ -17,12 +17,14 @@ import {
RECOVERY_MAP,
ODD_ONLY,
DESKTOP_ONLY,
ERROR_KINDS,
} from '../constants'
import { SelectRecoveryOption } from './SelectRecoveryOption'
import {
RecoveryFooterButtons,
RecoverySingleColumnContentWrapper,
RecoveryRadioGroup,
SkipStepInfo,
} from '../shared'

import type { RecoveryContentProps } from '../types'
Expand All @@ -36,6 +38,8 @@ export function IgnoreErrorSkipStep(props: RecoveryContentProps): JSX.Element {
switch (step) {
case IGNORE_AND_SKIP.STEPS.SELECT_IGNORE_KIND:
return <IgnoreErrorStepHome {...props} />
case IGNORE_AND_SKIP.STEPS.SKIP_STEP:
return <SkipStepInfo {...props} />
default:
console.warn(`${step} in ${route} not explicitly handled. Rerouting.`)
return <SelectRecoveryOption {...props} />
Expand All @@ -48,34 +52,56 @@ export function IgnoreErrorSkipStep(props: RecoveryContentProps): JSX.Element {
export function IgnoreErrorStepHome({
recoveryCommands,
routeUpdateActions,
errorKind,
}: RecoveryContentProps): JSX.Element | null {
const { t } = useTranslation('error_recovery')
const { MANUAL_FILL_AND_SKIP } = RECOVERY_MAP
const { ignoreErrorKindThisRun } = recoveryCommands
const { proceedToRouteAndStep, goBackPrevStep } = routeUpdateActions
const {
proceedNextStep,
proceedToRouteAndStep,
goBackPrevStep,
} = routeUpdateActions

const [selectedOption, setSelectedOption] = React.useState<IgnoreOption>(
const [selectedOption, setSelectedOption] = useState<IgnoreOption>(
head(IGNORE_OPTIONS_IN_ORDER) as IgnoreOption
)

// It's safe to hard code the routing here, since only one route currently
// utilizes ignoring. In the future, we may have to check the selectedRecoveryOption
// and route appropriately.
// Reset client choice to ignore all errors whenever navigating back to this view. This prevents unexpected
// behavior after pressing "go back" and ending up on this screen.
useEffect(() => {
void ignoreErrorKindThisRun(false)
}, [])

// In order to keep routing linear, all extended "skip" flows should be kept as separate recovery options with
// go back functionality that routes to this view. Those "skip" views encapsulate the generic "skip" view.
// See the "manually fill well and skip" recovery option for an example.
const ignoreOnce = (): void => {
void proceedToRouteAndStep(
MANUAL_FILL_AND_SKIP.ROUTE,
MANUAL_FILL_AND_SKIP.STEPS.SKIP
)
switch (errorKind) {
case ERROR_KINDS.NO_LIQUID_DETECTED:
void proceedToRouteAndStep(
RECOVERY_MAP.MANUAL_FILL_AND_SKIP.ROUTE,
RECOVERY_MAP.MANUAL_FILL_AND_SKIP.STEPS.SKIP
)
break
default:
void proceedNextStep()
}
}

// See ignoreOnce comment.
const ignoreAlways = (): void => {
void ignoreErrorKindThisRun().then(() =>
proceedToRouteAndStep(
MANUAL_FILL_AND_SKIP.ROUTE,
MANUAL_FILL_AND_SKIP.STEPS.SKIP
)
)
void ignoreErrorKindThisRun(true).then(() => {
switch (errorKind) {
case ERROR_KINDS.NO_LIQUID_DETECTED:
void proceedToRouteAndStep(
RECOVERY_MAP.MANUAL_FILL_AND_SKIP.ROUTE,
RECOVERY_MAP.MANUAL_FILL_AND_SKIP.STEPS.SKIP
)
break
default:
void proceedNextStep()
}
})
}

const primaryOnClick = (): void => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,23 @@ import {
IgnoreErrorStepHome,
IgnoreOptions,
} from '../IgnoreErrorSkipStep'
import { RECOVERY_MAP } from '../../constants'
import { ERROR_KINDS, RECOVERY_MAP } from '../../constants'
import { SelectRecoveryOption } from '../SelectRecoveryOption'
import { clickButtonLabeled } from '../../__tests__/util'
import { SkipStepInfo } from '/app/organisms/ErrorRecoveryFlows/shared'

import type { Mock } from 'vitest'

vi.mock('../shared', async () => {
const actual = await vi.importActual('../shared')
vi.mock('/app/organisms/ErrorRecoveryFlows/shared', async () => {
const actual = await vi.importActual(
'/app/organisms/ErrorRecoveryFlows/shared'
)
return {
...actual,
RecoverySingleColumnContentWrapper: vi.fn(({ children }) => (
<div>{children}</div>
)),
SkipStepInfo: vi.fn(),
}
})
vi.mock('../SelectRecoveryOption')
Expand All @@ -47,11 +51,13 @@ describe('IgnoreErrorSkipStep', () => {
beforeEach(() => {
props = {
...mockRecoveryContentProps,
recoveryCommands: { ignoreErrorKindThisRun: vi.fn() } as any,
}

vi.mocked(SelectRecoveryOption).mockReturnValue(
<div>MOCK_SELECT_RECOVERY_OPTION</div>
)
vi.mocked(SkipStepInfo).mockReturnValue(<div>MOCK_SKIP_STEP_INFO</div>)
})

it(`renders IgnoreErrorStepHome when step is ${RECOVERY_MAP.IGNORE_AND_SKIP.STEPS.SELECT_IGNORE_KIND}`, () => {
Expand All @@ -66,6 +72,18 @@ describe('IgnoreErrorSkipStep', () => {
screen.getByText('Ignore similar errors later in the run?')
})

it(`renders SkipStepInfo when step is ${RECOVERY_MAP.IGNORE_AND_SKIP.STEPS.SKIP_STEP}`, () => {
props = {
...props,
recoveryMap: {
...props.recoveryMap,
step: RECOVERY_MAP.IGNORE_AND_SKIP.STEPS.SKIP_STEP,
},
}
render(props)
screen.getByText('MOCK_SKIP_STEP_INFO')
})

it('renders SelectRecoveryOption as a fallback', () => {
props = {
...props,
Expand All @@ -84,26 +102,30 @@ describe('IgnoreErrorStepHome', () => {
let mockIgnoreErrorKindThisRun: Mock
let mockProceedToRouteAndStep: Mock
let mockGoBackPrevStep: Mock
let mockProceedNextStep: Mock

beforeEach(() => {
mockIgnoreErrorKindThisRun = vi.fn(() => Promise.resolve())
mockProceedToRouteAndStep = vi.fn()
mockGoBackPrevStep = vi.fn()
mockProceedNextStep = vi.fn()

props = {
...mockRecoveryContentProps,
isOnDevice: true,
errorKind: ERROR_KINDS.NO_LIQUID_DETECTED,
recoveryCommands: {
ignoreErrorKindThisRun: mockIgnoreErrorKindThisRun,
} as any,
routeUpdateActions: {
proceedToRouteAndStep: mockProceedToRouteAndStep,
goBackPrevStep: mockGoBackPrevStep,
proceedNextStep: mockProceedNextStep,
} as any,
}
})

it('calls ignoreOnce when "ignore_only_this_error" is selected and primary button is clicked', async () => {
it(`ignoreOnce correctly routes "ignore_only_this_error" is clicked and the errorKind is ${ERROR_KINDS.NO_LIQUID_DETECTED}`, async () => {
renderIgnoreErrorStepHome(props)
fireEvent.click(screen.queryAllByText('Ignore only this error')[0])
clickButtonLabeled('Continue')
Expand All @@ -115,7 +137,19 @@ describe('IgnoreErrorStepHome', () => {
})
})

it('calls ignoreAlways when "ignore_all_errors_of_this_type" is selected and primary button is clicked', async () => {
it(`ignoreOnce correctly routes "ignore_only_this_error" is clicked and the errorKind not explicitly handled`, async () => {
renderIgnoreErrorStepHome({
...props,
errorKind: ERROR_KINDS.GENERAL_ERROR,
})
fireEvent.click(screen.queryAllByText('Ignore only this error')[0])
clickButtonLabeled('Continue')
await waitFor(() => {
expect(mockProceedNextStep).toHaveBeenCalled()
})
})

it(`ignoreAlways correctly routes when "ignore_all_errors_of_this_type" is clicked and the errorKind is ${ERROR_KINDS.NO_LIQUID_DETECTED}`, async () => {
renderIgnoreErrorStepHome(props)
fireEvent.click(screen.queryAllByText('Ignore all errors of this type')[0])
clickButtonLabeled('Continue')
Expand All @@ -130,6 +164,18 @@ describe('IgnoreErrorStepHome', () => {
})
})

it(`ignoreAlways correctly routes "ignore_all_errors_of_this_type" is clicked and the errorKind not explicitly handled`, async () => {
renderIgnoreErrorStepHome({
...props,
errorKind: ERROR_KINDS.GENERAL_ERROR,
})
fireEvent.click(screen.queryAllByText('Ignore all errors of this type')[0])
clickButtonLabeled('Continue')
await waitFor(() => {
expect(mockProceedNextStep).toHaveBeenCalled()
})
})

it('calls goBackPrevStep when secondary button is clicked', () => {
renderIgnoreErrorStepHome(props)
clickButtonLabeled('Go back')
Expand Down
8 changes: 6 additions & 2 deletions app/src/organisms/ErrorRecoveryFlows/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export const RECOVERY_MAP = {
},
IGNORE_AND_SKIP: {
ROUTE: 'ignore-and-skip-step',
STEPS: { SELECT_IGNORE_KIND: 'select-ignore' },
STEPS: { SELECT_IGNORE_KIND: 'select-ignore', SKIP_STEP: 'skip-step' },
},
MANUAL_FILL_AND_SKIP: {
ROUTE: 'manual-fill-well-and-skip',
Expand Down Expand Up @@ -248,7 +248,10 @@ export const STEP_ORDER: StepOrder = {
DROP_TIP_FLOWS.STEPS.CHOOSE_TIP_DROP,
],
[REFILL_AND_RESUME.ROUTE]: [],
[IGNORE_AND_SKIP.ROUTE]: [IGNORE_AND_SKIP.STEPS.SELECT_IGNORE_KIND],
[IGNORE_AND_SKIP.ROUTE]: [
IGNORE_AND_SKIP.STEPS.SELECT_IGNORE_KIND,
IGNORE_AND_SKIP.STEPS.SKIP_STEP,
],
[CANCEL_RUN.ROUTE]: [CANCEL_RUN.STEPS.CONFIRM_CANCEL],
[MANUAL_FILL_AND_SKIP.ROUTE]: [
MANUAL_FILL_AND_SKIP.STEPS.MANUAL_FILL,
Expand Down Expand Up @@ -343,6 +346,7 @@ export const RECOVERY_MAP_METADATA: RecoveryRouteStepMetadata = {
[IGNORE_AND_SKIP.STEPS.SELECT_IGNORE_KIND]: {
allowDoorOpen: false,
},
[IGNORE_AND_SKIP.STEPS.SKIP_STEP]: { allowDoorOpen: false },
},
[MANUAL_FILL_AND_SKIP.ROUTE]: {
[MANUAL_FILL_AND_SKIP.STEPS.MANUAL_FILL]: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe('useRecoveryCommands', () => {
const mockChainRunCommands = vi.fn().mockResolvedValue([])
const mockReportActionSelectedResult = vi.fn()
const mockReportRecoveredRunResult = vi.fn()
const mockUpdateErrorRecoveryPolicy = vi.fn()
const mockUpdateErrorRecoveryPolicy = vi.fn(() => Promise.resolve())

const props = {
runId: mockRunId,
Expand All @@ -70,7 +70,7 @@ describe('useRecoveryCommands', () => {
chainRunCommands: mockChainRunCommands,
} as any)
vi.mocked(useUpdateErrorRecoveryPolicy).mockReturnValue({
updateErrorRecoveryPolicy: mockUpdateErrorRecoveryPolicy,
mutateAsync: mockUpdateErrorRecoveryPolicy,
} as any)
})

Expand Down Expand Up @@ -302,12 +302,18 @@ describe('useRecoveryCommands', () => {
failedCommandByRunRecord: mockFailedCommandWithError,
}

const { result } = renderHook(() => useRecoveryCommands(testProps))
const { result, rerender } = renderHook(() =>
useRecoveryCommands(testProps)
)

await act(async () => {
await result.current.ignoreErrorKindThisRun()
await result.current.ignoreErrorKindThisRun(true)
})

rerender()

result.current.skipFailedCommand()

const expectedPolicyRules = buildIgnorePolicyRules(
'aspirateInPlace',
'mockErrorType'
Expand All @@ -318,16 +324,33 @@ describe('useRecoveryCommands', () => {
)
})

it('should reject with an error when failedCommand or error is null', async () => {
it('should call proceedToRouteAndStep with ERROR_WHILE_RECOVERING route when updateErrorRecoveryPolicy rejects', async () => {
const mockFailedCommandWithError = {
...mockFailedCommand,
commandType: 'aspirateInPlace',
error: {
errorType: 'mockErrorType',
},
}

const testProps = {
...props,
failedCommand: null,
failedCommandByRunRecord: mockFailedCommandWithError,
}

mockUpdateErrorRecoveryPolicy.mockRejectedValueOnce(
new Error('Update policy failed')
)

const { result } = renderHook(() => useRecoveryCommands(testProps))

await expect(result.current.ignoreErrorKindThisRun()).rejects.toThrow(
'Could not execute command. No failed command.'
await act(async () => {
await result.current.ignoreErrorKindThisRun(true)
})

expect(mockUpdateErrorRecoveryPolicy).toHaveBeenCalled()
expect(mockProceedToRouteAndStep).toHaveBeenCalledWith(
RECOVERY_MAP.ERROR_WHILE_RECOVERING.ROUTE
)
})
})
Loading
Loading