Skip to content

Commit

Permalink
fix(app): fix desktop post-run drop tip wiz crash after tip removal (#…
Browse files Browse the repository at this point in the history
…15887)

Closes RQA-2902

Refactor useTipAttachementStatus to provide only one pipette with tip at a time.
  • Loading branch information
mjhuff authored Aug 5, 2024
1 parent d84b349 commit f93ac41
Show file tree
Hide file tree
Showing 9 changed files with 83 additions and 77 deletions.
10 changes: 5 additions & 5 deletions app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ export function ProtocolRunHeader({
determineTipStatus,
resetTipStatus,
setTipStatusResolved,
pipettesWithTip,
aPipetteWithTip,
} = useTipAttachmentStatus({
runId,
runRecord,
Expand Down Expand Up @@ -421,7 +421,7 @@ export function ProtocolRunHeader({
<ProtocolDropTipModal
onSkip={onDTModalSkip}
onBeginRemoval={onDTModalRemoval}
mount={pipettesWithTip[0]?.mount}
mount={aPipetteWithTip?.mount}
/>
) : null}
<Box display="grid" gridTemplateColumns="4fr 3fr 3fr 4fr">
Expand Down Expand Up @@ -496,11 +496,11 @@ export function ProtocolRunHeader({
robotName={robotName}
/>
) : null}
{showDTWiz && mostRecentRunId === runId ? (
{showDTWiz && aPipetteWithTip != null ? (
<DropTipWizardFlows
robotType={isFlex ? FLEX_ROBOT_TYPE : OT2_ROBOT_TYPE}
mount={pipettesWithTip[0]?.mount}
instrumentModelSpecs={pipettesWithTip[0].specs}
mount={aPipetteWithTip.mount}
instrumentModelSpecs={aPipetteWithTip.specs}
closeFlow={() => setTipStatusResolved().then(toggleDTWiz)}
/>
) : null}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
RUN_STATUS_SUCCEEDED,
RUN_STATUS_BLOCKED_BY_OPEN_DOOR,
instrumentsResponseLeftPipetteFixture,
instrumentsResponseRightPipetteFixture,
} from '@opentrons/api-client'
import {
useHost,
Expand Down Expand Up @@ -88,6 +87,7 @@ import { useNotifyRunQuery, useCurrentRunId } from '../../../../resources/runs'
import {
useDropTipWizardFlows,
useTipAttachmentStatus,
DropTipWizardFlows,
} from '../../../DropTipWizardFlows'
import {
useErrorRecoveryFlows,
Expand Down Expand Up @@ -340,10 +340,7 @@ describe('ProtocolRunHeader', () => {
vi.mocked(useInstrumentsQuery).mockReturnValue({ data: {} } as any)
vi.mocked(useHost).mockReturnValue({} as any)
vi.mocked(useTipAttachmentStatus).mockReturnValue({
pipettesWithTip: [
instrumentsResponseLeftPipetteFixture,
instrumentsResponseRightPipetteFixture,
],
aPipetteWithTip: instrumentsResponseLeftPipetteFixture,
areTipsAttached: true,
determineTipStatus: mockDetermineTipStatus,
resetTipStatus: vi.fn(),
Expand Down Expand Up @@ -384,6 +381,9 @@ describe('ProtocolRunHeader', () => {
vi.mocked(ProtocolDropTipModal).mockReturnValue(
<div>MOCK_DROP_TIP_MODAL</div>
)
vi.mocked(DropTipWizardFlows).mockReturnValue(
<div>MOCK_DROP_TIP_WIZARD_FLOWS</div>
)
})

afterEach(() => {
Expand Down Expand Up @@ -1076,4 +1076,14 @@ describe('ProtocolRunHeader', () => {
render()
screen.getByText('MOCK_ERROR_RECOVERY')
})

it('renders DropTipWizardFlows when conditions are met', () => {
vi.mocked(useDropTipWizardFlows).mockReturnValue({
showDTWiz: true,
toggleDTWiz: vi.fn(),
})

render()
screen.getByText('MOCK_DROP_TIP_WIZARD_FLOWS')
})
})
11 changes: 6 additions & 5 deletions app/src/organisms/DropTipWizardFlows/TipsAttachedModal.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as React from 'react'
import capitalize from 'lodash/capitalize'
import head from 'lodash/head'
import NiceModal, { useModal } from '@ebay/nice-modal-react'
import { Trans, useTranslation } from 'react-i18next'

Expand All @@ -23,7 +22,7 @@ import type { ModalHeaderBaseProps } from '../../molecules/Modal/types'
import type { PipetteWithTip } from '.'

interface TipsAttachedModalProps {
pipettesWithTip: PipetteWithTip[]
aPipetteWithTip: PipetteWithTip
host: HostConfig | null
setTipStatusResolved: (onEmpty?: () => void) => Promise<void>
}
Expand All @@ -38,11 +37,11 @@ export const handleTipsAttachedModal = (

const TipsAttachedModal = NiceModal.create(
(props: TipsAttachedModalProps): JSX.Element => {
const { pipettesWithTip, host, setTipStatusResolved } = props
const { aPipetteWithTip, host, setTipStatusResolved } = props
const { t } = useTranslation(['drop_tip_wizard'])
const modal = useModal()

const { mount, specs } = head(pipettesWithTip) as PipetteWithTip
const { mount, specs } = aPipetteWithTip
const { showDTWiz, toggleDTWiz } = useDropTipWizardFlows()

const tipsAttachedHeader: ModalHeaderBaseProps = {
Expand All @@ -57,7 +56,9 @@ const TipsAttachedModal = NiceModal.create(
}

const is96Channel = specs.channels === 96
const displayMountText = is96Channel ? '96-Channel' : capitalize(mount)
const displayMountText = is96Channel
? '96-Channel'
: capitalize(mount as string)

return (
<ApiHostProvider {...host} hostname={host?.hostname ?? null}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,29 @@ const MOCK_ACTUAL_PIPETTE = {
},
} as PipetteModelSpecs

const mockPipetteWithTip: PipetteWithTip = {
mount: 'left',
specs: MOCK_ACTUAL_PIPETTE,
}

const mockSecondPipetteWithTip: PipetteWithTip = {
mount: 'right',
specs: MOCK_ACTUAL_PIPETTE,
}

const mockPipettesWithTip: PipetteWithTip[] = [
mockPipetteWithTip,
mockSecondPipetteWithTip,
]

describe('useTipAttachmentStatus', () => {
let mockGetPipettesWithTipAttached: Mock

beforeEach(() => {
mockGetPipettesWithTipAttached = vi.mocked(getPipettesWithTipAttached)
vi.mocked(getPipetteModelSpecs).mockReturnValue(MOCK_ACTUAL_PIPETTE)
vi.mocked(DropTipWizard).mockReturnValue(<div>MOCK DROP TIP WIZ</div>)
mockGetPipettesWithTipAttached.mockResolvedValue(mockPipettesWithTip)
})

afterEach(() => {
Expand All @@ -54,32 +70,21 @@ describe('useTipAttachmentStatus', () => {
const { result } = renderHook(() => useTipAttachmentStatus({} as any))

expect(result.current.areTipsAttached).toBe(false)
expect(result.current.pipettesWithTip).toEqual([])
expect(result.current.aPipetteWithTip).toEqual(null)
})

it('should determine tip status and update state accordingly', async () => {
const mockPipettesWithTip: PipetteWithTip[] = [
{ mount: 'left', specs: MOCK_ACTUAL_PIPETTE },
{ mount: 'right', specs: MOCK_ACTUAL_PIPETTE },
]
mockGetPipettesWithTipAttached.mockResolvedValueOnce(mockPipettesWithTip)

const { result } = renderHook(() => useTipAttachmentStatus({} as any))

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

expect(result.current.areTipsAttached).toBe(true)
expect(result.current.pipettesWithTip).toEqual(mockPipettesWithTip)
expect(result.current.aPipetteWithTip).toEqual(mockPipetteWithTip)
})

it('should reset tip status', async () => {
const mockPipettesWithTip: PipetteWithTip[] = [
{ mount: 'left', specs: MOCK_ACTUAL_PIPETTE },
]
mockGetPipettesWithTipAttached.mockResolvedValueOnce(mockPipettesWithTip)

const { result } = renderHook(() => useTipAttachmentStatus({} as any))

await act(async () => {
Expand All @@ -88,31 +93,22 @@ describe('useTipAttachmentStatus', () => {
})

expect(result.current.areTipsAttached).toBe(false)
expect(result.current.pipettesWithTip).toEqual([])
expect(result.current.aPipetteWithTip).toEqual(null)
})

it('should set tip status resolved and update state', async () => {
const mockPipettesWithTip: PipetteWithTip[] = [
{ mount: 'left', specs: MOCK_ACTUAL_PIPETTE },
{ mount: 'right', specs: MOCK_ACTUAL_PIPETTE },
]
mockGetPipettesWithTipAttached.mockResolvedValueOnce(mockPipettesWithTip)

const { result } = renderHook(() => useTipAttachmentStatus({} as any))

await act(async () => {
await result.current.determineTipStatus()
result.current.setTipStatusResolved()
})

expect(result.current.pipettesWithTip).toEqual([mockPipettesWithTip[1]])
expect(result.current.aPipetteWithTip).toEqual(mockSecondPipetteWithTip)
})

it('should call onEmptyCache callback when cache becomes empty', async () => {
const mockPipettesWithTip: PipetteWithTip[] = [
{ mount: 'left', specs: MOCK_ACTUAL_PIPETTE },
]
mockGetPipettesWithTipAttached.mockResolvedValueOnce(mockPipettesWithTip)
mockGetPipettesWithTipAttached.mockResolvedValueOnce([mockPipetteWithTip])

const onEmptyCacheMock = vi.fn()
const { result } = renderHook(() => useTipAttachmentStatus({} as any))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,24 +33,24 @@ const ninetySixSpecs = {
channels: 96,
} as PipetteModelSpecs

const MOCK_PIPETTES_WITH_TIP: PipetteWithTip[] = [
{ mount: LEFT, specs: MOCK_ACTUAL_PIPETTE },
]
const MOCK_96_WITH_TIP: PipetteWithTip[] = [
{ mount: LEFT, specs: ninetySixSpecs },
]
const MOCK_A_PIPETTE_WITH_TIP: PipetteWithTip = {
mount: LEFT,
specs: MOCK_ACTUAL_PIPETTE,
}

const MOCK_96_WITH_TIP: PipetteWithTip = { mount: LEFT, specs: ninetySixSpecs }

const mockSetTipStatusResolved = vi.fn()
const MOCK_HOST: HostConfig = { hostname: 'MOCK_HOST' }

const render = (pipettesWithTips: PipetteWithTip[]) => {
const render = (aPipetteWithTip: PipetteWithTip) => {
return renderWithProviders(
<NiceModal.Provider>
<button
onClick={() =>
handleTipsAttachedModal({
host: MOCK_HOST,
pipettesWithTip: pipettesWithTips,
aPipetteWithTip,
setTipStatusResolved: mockSetTipStatusResolved,
})
}
Expand Down Expand Up @@ -79,7 +79,7 @@ describe('TipsAttachedModal', () => {
})

it('renders appropriate warning given the pipette mount', () => {
render(MOCK_PIPETTES_WITH_TIP)
render(MOCK_A_PIPETTE_WITH_TIP)
const btn = screen.getByTestId('testButton')
fireEvent.click(btn)

Expand All @@ -89,7 +89,7 @@ describe('TipsAttachedModal', () => {
)
})
it('clicking the skip button properly closes the modal', () => {
render(MOCK_PIPETTES_WITH_TIP)
render(MOCK_A_PIPETTE_WITH_TIP)
const btn = screen.getByTestId('testButton')
fireEvent.click(btn)

Expand All @@ -98,7 +98,7 @@ describe('TipsAttachedModal', () => {
expect(mockSetTipStatusResolved).toHaveBeenCalled()
})
it('clicking the launch wizard button properly launches the wizard', () => {
render(MOCK_PIPETTES_WITH_TIP)
render(MOCK_A_PIPETTE_WITH_TIP)
const btn = screen.getByTestId('testButton')
fireEvent.click(btn)

Expand Down
20 changes: 11 additions & 9 deletions app/src/organisms/DropTipWizardFlows/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ export interface TipAttachmentStatusResult {
* NOTE: Use responsibly! This function can potentially (but not likely) iterate over the entire length of a protocol run.
* */
determineTipStatus: () => Promise<PipetteWithTip[]>
/** Whether tips are likely attached on *any* pipette. Typically called after determineTipStatus() */
/* Whether tips are likely attached on *any* pipette. Typically called after determineTipStatus() */
areTipsAttached: boolean
/** Resets the cached pipettes with tip statuses to null. */
/* Resets the cached pipettes with tip statuses to null. */
resetTipStatus: () => void
/** Removes the first element from the tip attached cache if present.
* @param {Function} onEmptyCache After removing the pipette from the cache, if the attached tip cache is empty, invoke this callback.
Expand All @@ -86,9 +86,9 @@ export interface TipAttachmentStatusResult {
setTipStatusResolved: (
onEmptyCache?: () => void,
onTipsDetected?: () => void
) => Promise<PipetteWithTip[]>
/** Relevant pipette information for those pipettes with tips attached. */
pipettesWithTip: PipetteWithTip[]
) => Promise<PipetteWithTip>
/* Relevant pipette information for a pipette with a tip attached. If both pipettes have tips attached, return the left pipette. */
aPipetteWithTip: PipetteWithTip | null
}

// Returns various utilities for interacting with the cache of pipettes with tips attached.
Expand All @@ -99,6 +99,8 @@ export function useTipAttachmentStatus(
PipetteWithTip[]
>([])

const aPipetteWithTip = head(pipettesWithTip) ?? null

const areTipsAttached =
pipettesWithTip.length != null && head(pipettesWithTip)?.specs != null

Expand Down Expand Up @@ -130,8 +132,8 @@ export function useTipAttachmentStatus(
const setTipStatusResolved = (
onEmptyCache?: () => void,
onTipsDetected?: () => void
): Promise<PipetteWithTip[]> => {
return new Promise<PipetteWithTip[]>(resolve => {
): Promise<PipetteWithTip> => {
return new Promise<PipetteWithTip>(resolve => {
setPipettesWithTip(prevPipettesWithTip => {
const newState = [...prevPipettesWithTip.slice(1)]
if (newState.length === 0) {
Expand All @@ -140,7 +142,7 @@ export function useTipAttachmentStatus(
onTipsDetected?.()
}

resolve(newState)
resolve(newState[0])
return newState
})
})
Expand All @@ -150,7 +152,7 @@ export function useTipAttachmentStatus(
areTipsAttached,
determineTipStatus,
resetTipStatus,
pipettesWithTip,
aPipetteWithTip,
setTipStatusResolved,
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as React from 'react'
import { Trans, useTranslation } from 'react-i18next'
import head from 'lodash/head'

import {
DIRECTION_COLUMN,
Expand Down Expand Up @@ -60,7 +59,7 @@ export function BeginRemoval({
currentRecoveryOptionUtils,
}: RecoveryContentProps): JSX.Element | null {
const { t } = useTranslation('error_recovery')
const { pipettesWithTip } = tipStatusUtils
const { aPipetteWithTip } = tipStatusUtils
const {
proceedNextStep,
setRobotInMotion,
Expand All @@ -69,7 +68,7 @@ export function BeginRemoval({
const { cancelRun } = recoveryCommands
const { selectedRecoveryOption } = currentRecoveryOptionUtils
const { ROBOT_CANCELING, RETRY_NEW_TIPS } = RECOVERY_MAP
const mount = head(pipettesWithTip)?.mount
const mount = aPipetteWithTip?.mount

const primaryOnClick = (): void => {
void proceedNextStep()
Expand Down Expand Up @@ -155,9 +154,7 @@ function DropTipFlowsContainer(
const { setTipStatusResolved } = tipStatusUtils
const { cancelRun } = recoveryCommands

const { mount, specs } = head(
tipStatusUtils.pipettesWithTip
) as PipetteWithTip // Safe as we have to have tips to get to this point in the flow.
const { mount, specs } = tipStatusUtils.aPipetteWithTip as PipetteWithTip // Safe as we have to have tips to get to this point in the flow.

const onCloseFlow = (): void => {
if (selectedRecoveryOption === RETRY_NEW_TIPS.ROUTE) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe('ManageTips', () => {
step: DROP_TIP_FLOWS.STEPS.BEGIN_REMOVAL,
},
tipStatusUtils: {
pipettesWithTip: [{ mount: 'left', specs: MOCK_ACTUAL_PIPETTE }],
aPipetteWithTip: { mount: 'left', specs: MOCK_ACTUAL_PIPETTE },
} as any,
routeUpdateActions: {
proceedNextStep: mockProceedNextStep,
Expand Down
Loading

0 comments on commit f93ac41

Please sign in to comment.