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

fix(app): clone run with RTPs from HistoricalProtocolRun #14959

Merged
merged 8 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
11 changes: 11 additions & 0 deletions api-client/src/runs/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import {
RUN_STATUS_FAILED,
RUN_STATUS_STOPPED,
RUN_STATUS_SUCCEEDED,
} from './types'

export const RUN_STATUSES_TERMINAL = [
RUN_STATUS_SUCCEEDED,
RUN_STATUS_FAILED,
RUN_STATUS_STOPPED,
]
2 changes: 1 addition & 1 deletion api-client/src/runs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ export { getCommands } from './commands/getCommands'
export { createRunAction } from './createRunAction'
export * from './createLabwareOffset'
export * from './createLabwareDefinition'

export * from './constants'
export * from './types'
export type { CreateRunData } from './createRun'
3 changes: 2 additions & 1 deletion api-client/src/runs/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
LoadedPipette,
ModuleModel,
RunTimeCommand,
RunTimeParameter,
} from '@opentrons/shared-data'
import type { ResourceLink, ErrorDetails } from '../types'
export * from './commands/types'
Expand Down Expand Up @@ -47,7 +48,7 @@ export interface LegacyGoodRunData {
modules: LoadedModule[]
protocolId?: string
labwareOffsets?: LabwareOffset[]
runTimeParameterValues?: RunTimeParameterCreateData
runTimeParameters: RunTimeParameter[]
}

export interface KnownGoodRunData extends LegacyGoodRunData {
Expand Down
12 changes: 6 additions & 6 deletions app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
RUN_STATUS_SUCCEEDED,
RUN_STATUS_BLOCKED_BY_OPEN_DOOR,
RUN_STATUS_AWAITING_RECOVERY,
RUN_STATUSES_TERMINAL,
} from '@opentrons/api-client'
import {
useModulesQuery,
Expand Down Expand Up @@ -128,11 +129,6 @@ const CANCELLABLE_STATUSES = [
RUN_STATUS_IDLE,
RUN_STATUS_AWAITING_RECOVERY,
]
const RUN_OVER_STATUSES: RunStatus[] = [
RUN_STATUS_FAILED,
RUN_STATUS_STOPPED,
RUN_STATUS_SUCCEEDED,
]

interface ProtocolRunHeaderProps {
protocolRunHeaderRef: React.RefObject<HTMLDivElement> | null
Expand Down Expand Up @@ -215,7 +211,11 @@ export function ProtocolRunHeader({
if (runStatus === RUN_STATUS_IDLE) {
setShowDropTipBanner(true)
setPipettesWithTip([])
} else if (runStatus != null && RUN_OVER_STATUSES.includes(runStatus)) {
} else if (
runStatus != null &&
// @ts-expect-error runStatus expected to possibly not be terminal
RUN_STATUSES_TERMINAL.includes(runStatus)
) {
getPipettesWithTipAttached({
host,
runId,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import * as React from 'react'
import { useTranslation } from 'react-i18next'
import styled, { css } from 'styled-components'
import {
RUN_STATUS_STOPPED,
RUN_STATUSES_TERMINAL,
} from '@opentrons/api-client'
import { formatRunTimeParameterValue } from '@opentrons/shared-data'
import {
ALIGN_CENTER,
Expand All @@ -25,6 +29,8 @@ import { Tooltip } from '../../../atoms/Tooltip'
import { useMostRecentCompletedAnalysis } from '../../LabwarePositionCheck/useMostRecentCompletedAnalysis'

import type { RunTimeParameter } from '@opentrons/shared-data'
import { useRunStatus } from '../../RunTimeControl/hooks'
import { useNotifyRunQuery } from '../../../resources/runs'
ncdiehl11 marked this conversation as resolved.
Show resolved Hide resolved

interface ProtocolRunRuntimeParametersProps {
runId: string
Expand All @@ -34,7 +40,15 @@ export function ProtocolRunRuntimeParameters({
}: ProtocolRunRuntimeParametersProps): JSX.Element {
const { t } = useTranslation('protocol_setup')
const mostRecentAnalysis = useMostRecentCompletedAnalysis(runId)
const runTimeParameters = mostRecentAnalysis?.runTimeParameters ?? []
const runStatus = useRunStatus(runId)
const isRunTerminal =
// @ts-expect-error: expected that runStatus may not be a terminal status
runStatus == null ? false : RUN_STATUSES_TERMINAL.includes(runStatus)
ncdiehl11 marked this conversation as resolved.
Show resolved Hide resolved
const run = useNotifyRunQuery(runId).data
const runTimeParameters =
(isRunTerminal
? run?.data?.runTimeParameters
: mostRecentAnalysis?.runTimeParameters) ?? []
Comment on lines +53 to +57
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd add a little comment above here explaining why we're doing this — i can see myself forgetting a year from now 😛

const hasParameter = runTimeParameters.length > 0

const hasCustomValues = runTimeParameters.some(
Expand Down Expand Up @@ -79,7 +93,11 @@ export function ProtocolRunRuntimeParameters({
</Flex>
{!hasParameter ? (
<Flex padding={SPACING.spacing16}>
<InfoScreen contentType="parameters" />
<InfoScreen
contentType={
runStatus === RUN_STATUS_STOPPED ? 'runNotStarted' : 'parameters'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't quite right — a run could be stopped half way through a run. i think what we want to check here is whether a play action has been dispatched. so we'd do something like this:

  const runActions = runRecord?.data.actions
  const hasRunStarted = runActions?.some(
    action => action.actionType === RUN_ACTION_TYPE_PLAY
  )

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes more sense— thanks for the catch

}
/>
</Flex>
) : (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,18 @@ import { InfoScreen } from '@opentrons/components'
import { renderWithProviders } from '../../../../__testing-utils__'
import { i18n } from '../../../../i18n'
import { useMostRecentCompletedAnalysis } from '../../../LabwarePositionCheck/useMostRecentCompletedAnalysis'
import { useRunStatus } from '../../../RunTimeControl/hooks'
import { useNotifyRunQuery } from '../../../../resources/runs'
import { mockSucceededRun } from '../../../RunTimeControl/__fixtures__'

import { ProtocolRunRuntimeParameters } from '../ProtocolRunRunTimeParameters'

import type {
CompletedProtocolAnalysis,
RunTimeParameter,
} from '@opentrons/shared-data'
import { Run } from '@opentrons/api-client'
import { UseQueryResult } from 'react-query'
ncdiehl11 marked this conversation as resolved.
Show resolved Hide resolved

vi.mock('@opentrons/components', async importOriginal => {
const actual = await importOriginal<typeof InfoScreen>()
Expand All @@ -23,6 +28,8 @@ vi.mock('@opentrons/components', async importOriginal => {
}
})
vi.mock('../../../LabwarePositionCheck/useMostRecentCompletedAnalysis')
vi.mock('../../../RunTimeControl/hooks')
vi.mock('../../../../resources/runs')

const RUN_ID = 'mockId'

Expand Down Expand Up @@ -100,13 +107,17 @@ describe('ProtocolRunRuntimeParameters', () => {
.thenReturn({
runTimeParameters: mockRunTimeParameterData,
} as CompletedProtocolAnalysis)
vi.mocked(useRunStatus).mockReturnValue('running')
vi.mocked(useNotifyRunQuery).mockReturnValue(({
data: { data: mockSucceededRun },
} as unknown) as UseQueryResult<Run>)
})

afterEach(() => {
vi.resetAllMocks()
})

it('should render title, and banner when RunTimeParameters are note empty and all values are default', () => {
it('should render title, and banner when RunTimeParameters are not empty and all values are default', () => {
render(props)
screen.getByText('Parameters')
screen.getByText('Default values')
Expand All @@ -116,7 +127,7 @@ describe('ProtocolRunRuntimeParameters', () => {
screen.getByText('Value')
})

it('should render title, and banner when RunTimeParameters are note empty and some value is changed', () => {
it('should render title, and banner when RunTimeParameters are not empty and some value is changed', () => {
vi.mocked(useMostRecentCompletedAnalysis).mockReturnValue({
runTimeParameters: [
...mockRunTimeParameterData,
Expand All @@ -139,7 +150,7 @@ describe('ProtocolRunRuntimeParameters', () => {
screen.getByText('Value')
})

it('should render RunTimeParameters when RunTimeParameters are note empty', () => {
it('should render RunTimeParameters when RunTimeParameters are not empty', () => {
render(props)
screen.getByText('Dry Run')
screen.getByText('Off')
Expand Down
9 changes: 3 additions & 6 deletions app/src/organisms/Devices/hooks/useRunStatuses.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import {
RUN_STATUSES_TERMINAL,
RUN_STATUS_AWAITING_RECOVERY,
RUN_STATUS_FAILED,
RUN_STATUS_IDLE,
RUN_STATUS_PAUSED,
RUN_STATUS_RUNNING,
RUN_STATUS_STOPPED,
RUN_STATUS_SUCCEEDED,
} from '@opentrons/api-client'
import { useCurrentRunId } from '../../ProtocolUpload/hooks'
import { useRunStatus } from '../../RunTimeControl/hooks'
Expand All @@ -29,9 +27,8 @@ export function useRunStatuses(): RunStatusesInfo {
runStatus === RUN_STATUS_RUNNING ||
runStatus === RUN_STATUS_AWAITING_RECOVERY
const isRunTerminal =
runStatus === RUN_STATUS_SUCCEEDED ||
runStatus === RUN_STATUS_STOPPED ||
runStatus === RUN_STATUS_FAILED
// @ts-expect-error: runStatus expected to not necessarily be in RUN_STATUSES_TERMINAL
ncdiehl11 marked this conversation as resolved.
Show resolved Hide resolved
runStatus != null ? RUN_STATUSES_TERMINAL.includes(runStatus) : false
const isRunStill = isRunTerminal || isRunIdle

return { isRunStill, isRunTerminal, isRunIdle, isRunRunning }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ export const mockRunData: RunData = {
pipettes: [],
labware: [mockLabwareOnModule, mockLabwareOnSlot, mockLabwareOffDeck],
modules: [mockModule],
runTimeParameters: [],
}

export const mockLabwareRenderInfo = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const mockRun = {
pipettes: [],
protocolId: 'mockSortedProtocolID',
status: 'stopped',
runTimeParameters: [],
}

const render = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import { renderHook } from '@testing-library/react'
import { QueryClient, QueryClientProvider } from 'react-query'
import { describe, it, beforeEach, afterEach, vi, expect } from 'vitest'

import { useHost, useCreateRunMutation } from '@opentrons/react-api-client'
import {
useHost,
useCreateRunMutation,
useCreateProtocolAnalysisMutation,
} from '@opentrons/react-api-client'

import { useCloneRun } from '../useCloneRun'
import { useNotifyRunQuery } from '../../../../resources/runs'
Expand All @@ -30,13 +34,16 @@ describe('useCloneRun hook', () => {
id: RUN_ID,
protocolId: 'protocolId',
labwareOffsets: 'someOffset',
runTimeParameterValues: 'someRtp',
runTimeParameters: [],
},
},
} as any)
when(vi.mocked(useCreateRunMutation))
.calledWith(expect.anything())
.thenReturn({ createRun: vi.fn() } as any)
vi.mocked(useCreateProtocolAnalysisMutation).mockReturnValue({
createProtocolAnalysis: vi.fn(),
} as any)

const queryClient = new QueryClient()
const clientProvider: React.FunctionComponent<{
Expand All @@ -61,7 +68,7 @@ describe('useCloneRun hook', () => {
expect(mockCreateRun).toHaveBeenCalledWith({
protocolId: 'protocolId',
labwareOffsets: 'someOffset',
runTimeParameterValues: 'someRtp',
runTimeParameterValues: {},
})
})
})
48 changes: 34 additions & 14 deletions app/src/organisms/ProtocolUpload/hooks/useCloneRun.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import { useQueryClient } from 'react-query'

import { useHost, useCreateRunMutation } from '@opentrons/react-api-client'

import {
useHost,
useCreateRunMutation,
useCreateProtocolAnalysisMutation,
} from '@opentrons/react-api-client'
import { useNotifyRunQuery } from '../../../resources/runs'

import type { Run } from '@opentrons/api-client'
import type { Run, RunTimeParameterCreateData } from '@opentrons/api-client'

interface UseCloneRunResult {
cloneRun: () => void
Expand All @@ -13,28 +16,45 @@ interface UseCloneRunResult {

export function useCloneRun(
runId: string | null,
onSuccessCallback?: (createRunResponse: Run) => unknown
onSuccessCallback?: (createRunResponse: Run) => unknown,
triggerAnalysis: boolean = false
): UseCloneRunResult {
const host = useHost()
const queryClient = useQueryClient()
const { data: runRecord } = useNotifyRunQuery(runId)
const protocolKey = runRecord?.data.protocolId ?? null

const { createRun, isLoading } = useCreateRunMutation({
onSuccess: response => {
queryClient
.invalidateQueries([host, 'runs'])
.catch((e: Error) =>
console.error(`error invalidating runs query: ${e.message}`)
)
const invalidateRuns = queryClient.invalidateQueries([host, 'runs'])
const invalidateProtocols = queryClient.invalidateQueries([
host,
'protocols',
protocolKey,
])
Promise.all([invalidateRuns, invalidateProtocols]).catch((e: Error) =>
console.error(`error invalidating runs query: ${e.message}`)
)
if (onSuccessCallback != null) onSuccessCallback(response)
},
})
const { createProtocolAnalysis } = useCreateProtocolAnalysisMutation(
protocolKey,
host
)
const cloneRun = (): void => {
if (runRecord != null) {
const {
protocolId,
labwareOffsets,
runTimeParameterValues,
} = runRecord.data
const { protocolId, labwareOffsets, runTimeParameters } = runRecord.data
const runTimeParameterValues = runTimeParameters.reduce<RunTimeParameterCreateData>(
(acc, param) =>
param.value !== param.default
? { ...acc, [param.variableName]: param.value }
: acc,
{}
)
if (triggerAnalysis && protocolKey != null) {
createProtocolAnalysis({ protocolKey, runTimeParameterValues })
}
createRun({ protocolId, labwareOffsets, runTimeParameterValues })
} else {
console.info('failed to clone run record, source run record not found')
Expand Down
Loading
Loading