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 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
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
2 changes: 2 additions & 0 deletions app/src/assets/localization/en/protocol_setup.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
"confirm_values": "Confirm values",
"connect_all_hardware": "Connect and calibrate all hardware first",
"connect_all_mod": "Connect all modules first",
"connect_modules_for_controls": "Connect modules to see controls",
"connection_info_not_available": "Connection info not available once run has started",
"connection_status": "Connection Status",
"currently_configured": "Currently configured",
Expand Down Expand Up @@ -236,6 +237,7 @@
"run_disabled_modules_and_calibration_not_complete": "Make sure robot calibration is complete and all modules are connected before proceeding to run",
"run_disabled_modules_not_connected": "Make sure all modules are connected before proceeding to run",
"run_labware_position_check": "run labware position check",
"run_never_started": "Run was never started",
"run": "Run",
"secure_labware_instructions": "Secure labware instructions",
"secure_labware_modal": "Securing labware to the {{name}}",
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,11 @@
import * as React from 'react'
import { useTranslation } from 'react-i18next'
import styled, { css } from 'styled-components'
import {
RUN_ACTION_TYPE_PLAY,
RUN_STATUS_STOPPED,
RUN_STATUSES_TERMINAL,
} from '@opentrons/api-client'
import { formatRunTimeParameterValue } from '@opentrons/shared-data'
import {
ALIGN_CENTER,
Expand All @@ -23,8 +28,11 @@ import { Banner } from '../../../atoms/Banner'
import { Divider } from '../../../atoms/structure'
import { Tooltip } from '../../../atoms/Tooltip'
import { useMostRecentCompletedAnalysis } from '../../LabwarePositionCheck/useMostRecentCompletedAnalysis'
import { useRunStatus } from '../../RunTimeControl/hooks'
import { useNotifyRunQuery } from '../../../resources/runs'
ncdiehl11 marked this conversation as resolved.
Show resolved Hide resolved

import type { RunTimeParameter } from '@opentrons/shared-data'
import type { RunStatus } from '@opentrons/api-client'

interface ProtocolRunRuntimeParametersProps {
runId: string
Expand All @@ -34,13 +42,31 @@ export function ProtocolRunRuntimeParameters({
}: ProtocolRunRuntimeParametersProps): JSX.Element {
const { t } = useTranslation('protocol_setup')
const mostRecentAnalysis = useMostRecentCompletedAnalysis(runId)
const runTimeParameters = mostRecentAnalysis?.runTimeParameters ?? []
const hasParameter = runTimeParameters.length > 0

const hasCustomValues = runTimeParameters.some(
const runStatus = useRunStatus(runId)
const isRunTerminal =
runStatus == null
? false
: (RUN_STATUSES_TERMINAL as RunStatus[]).includes(runStatus)
// we access runTimeParameters from the run record rather than the most recent analysis
// because the most recent analysis may not reflect the selected run (e.g. cloning a run
// from a historical protocol run from the device details page)
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 hasRunTimeParameters = runTimeParameters.length > 0
const hasCustomRunTimeParameterValues = runTimeParameters.some(
parameter => parameter.value !== parameter.default
)

const runActions = run?.data.actions
const hasRunStarted = runActions?.some(
action => action.actionType === RUN_ACTION_TYPE_PLAY
)
const isRunCancelledWithoutStarting =
!hasRunStarted && runStatus === RUN_STATUS_STOPPED

return (
<>
<Flex
Expand All @@ -56,13 +82,15 @@ export function ProtocolRunRuntimeParameters({
<StyledText as="h3" fontWeight={TYPOGRAPHY.fontWeightSemiBold}>
{t('parameters')}
</StyledText>
{hasParameter ? (
{hasRunTimeParameters ? (
<StyledText as="label" color={COLORS.grey60}>
{hasCustomValues ? t('custom_values') : t('default_values')}
{hasCustomRunTimeParameterValues
? t('custom_values')
: t('default_values')}
</StyledText>
) : null}
</Flex>
{hasParameter ? (
{hasRunTimeParameters ? (
<Banner
type="informing"
width="100%"
Expand All @@ -77,9 +105,14 @@ export function ProtocolRunRuntimeParameters({
</Banner>
) : null}
</Flex>
{!hasParameter ? (
{!hasRunTimeParameters ? (
<Flex padding={SPACING.spacing16}>
<InfoScreen contentType="parameters" />
<InfoScreen
contentType={
isRunCancelledWithoutStarting ? 'runNotStarted' : 'parameters'
}
t={t}
/>
</Flex>
) : (
<>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
import * as React from 'react'
import { UseQueryResult } from 'react-query'
import { describe, it, vi, beforeEach, afterEach, expect } from 'vitest'
import { screen } from '@testing-library/react'
import { when } from 'vitest-when'

import { Run } from '@opentrons/api-client'
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'

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
12 changes: 6 additions & 6 deletions app/src/organisms/Devices/hooks/useRunStatuses.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
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'

import type { RunStatus } from '@opentrons/api-client'

interface RunStatusesInfo {
isRunStill: boolean
isRunTerminal: boolean
Expand All @@ -29,9 +29,9 @@ 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
runStatus != null
? (RUN_STATUSES_TERMINAL as RunStatus[]).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: {},
})
})
})
Loading
Loading