Skip to content

Commit

Permalink
fix(app): includeFixitCommands in GET run commands (#16253)
Browse files Browse the repository at this point in the history
# Overview

closes [EXEC-704](https://opentrons.atlassian.net/browse/EXEC-704).
revert do not render fixit commands and add query string to req.

## Test Plan and Hands on Testing

- load an ER protocol.
- enter ER.
- dispatch a few fixit commands.
- finish run.
- make sure there are no fixit commands in the run preview. 

## Changelog

revert commend intent logic from UI.
add `includeFixitCommands` in query string to GET run commands.

## Review requests

am I missing anyhting? 

## Risk assessment

low. no change in appearance.


[EXEC-704]:
https://opentrons.atlassian.net/browse/EXEC-704?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

---------

Co-authored-by: Jamey Huffnagle <[email protected]>
  • Loading branch information
TamarZanzouri and mjhuff authored Sep 16, 2024
1 parent 0cfab82 commit a0ea43d
Show file tree
Hide file tree
Showing 14 changed files with 86 additions and 33 deletions.
4 changes: 2 additions & 2 deletions api-client/src/runs/commands/getCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import { GET, request } from '../../request'
import type { ResponsePromise } from '../../request'
import type { HostConfig } from '../../types'
import type { CommandsData } from '..'
import type { GetCommandsParams } from './types'
import type { GetRunCommandsParamsRequest } from './types'

export function getCommands(
config: HostConfig,
runId: string,
params: GetCommandsParams
params: GetRunCommandsParamsRequest
): ResponsePromise<CommandsData> {
return request<CommandsData>(
GET,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import type { ResponsePromise } from '../../request'
import type { HostConfig } from '../../types'
import type {
CommandsAsPreSerializedListData,
GetCommandsParams,
GetRunCommandsParamsRequest,
} from './types'

export function getCommandsAsPreSerializedList(
config: HostConfig,
runId: string,
params: GetCommandsParams
params: GetRunCommandsParamsRequest
): ResponsePromise<CommandsAsPreSerializedListData> {
return request<CommandsAsPreSerializedListData>(
GET,
Expand Down
8 changes: 8 additions & 0 deletions api-client/src/runs/commands/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ export interface GetCommandsParams {
pageLength: number // the number of items to include
}

export interface GetRunCommandsParams extends GetCommandsParams {
includeFixitCommands?: boolean
}

export interface GetRunCommandsParamsRequest extends GetCommandsParams {
includeFixitCommands: boolean | null
}

export interface RunCommandErrors {
data: RunCommandError[]
meta: GetCommandsParams & { totalLength: number }
Expand Down
2 changes: 2 additions & 0 deletions app/src/organisms/Devices/hooks/useDownloadRunLog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@ export function useDownloadRunLog(
getCommands(host, runId, {
cursor: null,
pageLength: 0,
includeFixitCommands: true,
})
.then(response => {
const { totalLength } = response.data.meta
getCommands(host, runId, {
cursor: 0,
pageLength: totalLength,
includeFixitCommands: true,
})
.then(response => {
const commands = response.data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,13 @@ function getCommandsExecutedDuringRun(
return getCommands(host, runId, {
cursor: null,
pageLength: 0,
includeFixitCommands: true,
}).then(response => {
const { totalLength } = response.data.meta
return getCommands(host, runId, {
cursor: 0,
pageLength: totalLength,
includeFixitCommands: null,
}).then(response => response.data)
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import type { UseQueryOptions } from 'react-query'
import type {
CommandsData,
RunCommandSummary,
GetCommandsParams,
GetRunCommandsParams,
} from '@opentrons/api-client'

export function useCurrentRunCommands(
params?: GetCommandsParams,
params?: GetRunCommandsParams,
options?: UseQueryOptions<CommandsData>
): RunCommandSummary[] | null {
const currentRunId = useCurrentRunId()
Expand Down
4 changes: 2 additions & 2 deletions app/src/organisms/ProtocolUpload/hooks/useRunCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ import type { UseQueryOptions } from 'react-query'
import type {
CommandsData,
RunCommandSummary,
GetCommandsParams,
GetRunCommandsParams,
} from '@opentrons/api-client'

const REFETCH_INTERVAL = 3000

export function useRunCommands(
runId: string | null,
params?: GetCommandsParams,
params?: GetRunCommandsParams,
options?: UseQueryOptions<CommandsData>
): RunCommandSummary[] | null {
const { data: commandsData } = useNotifyAllCommandsQuery(runId, params, {
Expand Down
11 changes: 2 additions & 9 deletions app/src/organisms/RunPreview/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export const RunPreviewComponent = (
isLoading: isRunCommandDataLoading,
} = useNotifyAllCommandsAsPreSerializedList(
runId,
{ cursor: 0, pageLength: MAX_COMMANDS },
{ cursor: 0, pageLength: MAX_COMMANDS, includeFixitCommands: false },
{
enabled: isRunTerminal,
}
Expand All @@ -78,20 +78,13 @@ export const RunPreviewComponent = (
isCurrentCommandVisible,
setIsCurrentCommandVisible,
] = React.useState<boolean>(true)
const filteredCommandsFromQuery = React.useMemo(
() =>
commandsFromQuery?.filter(
command => !('intent' in command) || command.intent !== 'fixit'
),
[commandsFromQuery == null]
)

if (robotSideAnalysis == null) {
return null
}

const commands = isRunTerminal
? filteredCommandsFromQuery
? commandsFromQuery
: robotSideAnalysis.commands

// pass relevant data from run rather than analysis so that CommandText utilities can properly hash the entities' IDs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,10 @@ describe('RunProgressMeter', () => {
.calledWith(NON_DETERMINISTIC_RUN_ID)
.thenReturn(null)
when(useNotifyAllCommandsQuery)
.calledWith(NON_DETERMINISTIC_RUN_ID, { cursor: null, pageLength: 1 })
.calledWith(NON_DETERMINISTIC_RUN_ID, {
cursor: null,
pageLength: 1,
})
.thenReturn(mockUseAllCommandsResponseNonDeterministic)
when(useCommandQuery)
.calledWith(NON_DETERMINISTIC_RUN_ID, NON_DETERMINISTIC_COMMAND_KEY)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,10 @@ describe('RunningProtocol', () => {
.calledWith(RUN_ID)
.thenReturn(mockRobotSideAnalysis)
when(vi.mocked(useNotifyAllCommandsQuery))
.calledWith(RUN_ID, { cursor: null, pageLength: 1 })
.calledWith(RUN_ID, {
cursor: null,
pageLength: 1,
})
.thenReturn(mockUseAllCommandsResponseNonDeterministic)
vi.mocked(useLastRunCommand).mockReturnValue({
key: 'FAKE_COMMAND_KEY',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import { useNotifyDataReady } from '../useNotifyDataReady'

import type { UseQueryResult } from 'react-query'
import type { AxiosError } from 'axios'
import type { CommandsData, GetCommandsParams } from '@opentrons/api-client'
import type { CommandsData, GetRunCommandsParams } from '@opentrons/api-client'
import type { QueryOptionsWithPolling } from '../useNotifyDataReady'

export function useNotifyAllCommandsAsPreSerializedList(
runId: string | null,
params?: GetCommandsParams | null,
params?: GetRunCommandsParams | null,
options: QueryOptionsWithPolling<CommandsData, AxiosError> = {}
): UseQueryResult<CommandsData, AxiosError> {
const { shouldRefetch, queryOptionsNotify } = useNotifyDataReady({
Expand Down
28 changes: 25 additions & 3 deletions app/src/resources/runs/useNotifyAllCommandsQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,23 @@ import { useAllCommandsQuery } from '@opentrons/react-api-client'
import { useNotifyDataReady } from '../useNotifyDataReady'

import type { UseQueryResult } from 'react-query'
import type { CommandsData, GetCommandsParams } from '@opentrons/api-client'
import type {
CommandsData,
GetRunCommandsParams,
GetCommandsParams,
} from '@opentrons/api-client'
import type { QueryOptionsWithPolling } from '../useNotifyDataReady'

const DEFAULT_PAGE_LENGTH = 30

export const DEFAULT_PARAMS: GetCommandsParams = {
cursor: null,
pageLength: DEFAULT_PAGE_LENGTH,
}

export function useNotifyAllCommandsQuery<TError = Error>(
runId: string | null,
params?: GetCommandsParams | null,
params?: GetRunCommandsParams | null,
options: QueryOptionsWithPolling<CommandsData, TError> = {}
): UseQueryResult<CommandsData, TError> {
// Assume the useAllCommandsQuery() response can only change when the command links change.
Expand All @@ -21,8 +32,19 @@ export function useNotifyAllCommandsQuery<TError = Error>(
topic: 'robot-server/runs/commands_links',
options,
})
const nullCheckedParams = params ?? DEFAULT_PARAMS

const nullCheckedFixitCommands = params?.includeFixitCommands ?? null
const finalizedNullCheckParams = {
...nullCheckedParams,
includeFixitCommands: nullCheckedFixitCommands,
}

const httpQueryResult = useAllCommandsQuery(runId, params, queryOptionsNotify)
const httpQueryResult = useAllCommandsQuery(
runId,
finalizedNullCheckParams,
queryOptionsNotify
)

if (shouldRefetch) {
void httpQueryResult.refetch()
Expand Down
14 changes: 10 additions & 4 deletions react-api-client/src/runs/useAllCommandsAsPreSerializedList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,21 @@ import { useHost } from '../api'

import type { UseQueryOptions, UseQueryResult } from 'react-query'
import type {
GetCommandsParams,
GetRunCommandsParams,
HostConfig,
CommandsData,
RunCommandSummary,
} from '@opentrons/api-client'

const DEFAULT_PAGE_LENGTH = 30
export const DEFAULT_PARAMS: GetCommandsParams = {
export const DEFAULT_PARAMS: GetRunCommandsParams = {
cursor: null,
pageLength: DEFAULT_PAGE_LENGTH,
}

export function useAllCommandsAsPreSerializedList<TError = Error>(
runId: string | null,
params?: GetCommandsParams | null,
params?: GetRunCommandsParams | null,
options: UseQueryOptions<CommandsData, TError> = {}
): UseQueryResult<CommandsData, TError> {
const host = useHost()
Expand All @@ -32,6 +32,11 @@ export function useAllCommandsAsPreSerializedList<TError = Error>(
enabled: host !== null && runId != null && options.enabled !== false,
}
const { cursor, pageLength } = nullCheckedParams
const nullCheckedFixitCommands = params?.includeFixitCommands ?? null
const finalizedNullCheckParams = {
...nullCheckedParams,
includeFixitCommands: nullCheckedFixitCommands,
}

// map undefined values to null to agree with react query caching
// TODO (nd: 05/15/2024) create sanitizer for react query key objects
Expand All @@ -45,12 +50,13 @@ export function useAllCommandsAsPreSerializedList<TError = Error>(
'getCommandsAsPreSerializedList',
cursor,
pageLength,
nullCheckedFixitCommands,
],
() => {
return getCommandsAsPreSerializedList(
host as HostConfig,
runId as string,
nullCheckedParams
finalizedNullCheckParams
).then(response => {
const responseData = response.data
return {
Expand Down
24 changes: 19 additions & 5 deletions react-api-client/src/runs/useAllCommandsQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,21 @@ import { getCommands } from '@opentrons/api-client'
import { useHost } from '../api'
import type { UseQueryOptions, UseQueryResult } from 'react-query'
import type {
GetCommandsParams,
GetRunCommandsParamsRequest,
HostConfig,
CommandsData,
} from '@opentrons/api-client'

const DEFAULT_PAGE_LENGTH = 30
export const DEFAULT_PARAMS: GetCommandsParams = {
export const DEFAULT_PARAMS: GetRunCommandsParamsRequest = {
cursor: null,
pageLength: DEFAULT_PAGE_LENGTH,
includeFixitCommands: null,
}

export function useAllCommandsQuery<TError = Error>(
runId: string | null,
params?: GetCommandsParams | null,
params?: GetRunCommandsParamsRequest | null,
options: UseQueryOptions<CommandsData, TError> = {}
): UseQueryResult<CommandsData, TError> {
const host = useHost()
Expand All @@ -27,13 +28,26 @@ export function useAllCommandsQuery<TError = Error>(
enabled: host !== null && runId != null && options.enabled !== false,
}
const { cursor, pageLength } = nullCheckedParams
const nullCheckedFixitCommands = params?.includeFixitCommands ?? null
const finalizedNullCheckParams = {
...nullCheckedParams,
includeFixitCommands: nullCheckedFixitCommands,
}
const query = useQuery<CommandsData, TError>(
[host, 'runs', runId, 'commands', cursor, pageLength],
[
host,
'runs',
runId,
'commands',
cursor,
pageLength,
finalizedNullCheckParams,
],
() => {
return getCommands(
host as HostConfig,
runId as string,
nullCheckedParams
finalizedNullCheckParams
).then(response => response.data)
},
allOptions
Expand Down

0 comments on commit a0ea43d

Please sign in to comment.