-
Notifications
You must be signed in to change notification settings - Fork 178
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
feat(app, api-client, react-api-client): add api-client method for protocol reanalysis #14878
Changes from 12 commits
78876d3
b6edfbb
fd8eabb
81f5546
e7c9f5a
54958d2
fe6beef
3938a97
bc55d0b
7d5c8bb
b008743
0ceef3f
7fa3c9f
2bffac6
931d8bb
5610f9f
45c27b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import { POST, request } from '../request' | ||
|
||
import type { ResponsePromise } from '../request' | ||
import type { HostConfig } from '../types' | ||
import type { ProtocolAnalysisSummary } from '@opentrons/shared-data' | ||
import type { RunTimeParameterCreateData } from '../runs' | ||
|
||
interface CreateProtocolAnalysisData { | ||
runTimeParameterValues: RunTimeParameterCreateData | ||
forceReAnalyze: boolean | ||
} | ||
|
||
export function createProtocolAnalysis( | ||
config: HostConfig, | ||
protocolKey: string, | ||
runTimeParameterValues?: RunTimeParameterCreateData, | ||
forceReAnalyze?: boolean | ||
): ResponsePromise<ProtocolAnalysisSummary[]> { | ||
const data = { | ||
runTimeParameterValues: runTimeParameterValues ?? {}, | ||
forceReAnalyze: forceReAnalyze ?? false, | ||
} | ||
const response = request< | ||
ProtocolAnalysisSummary[], | ||
{ data: CreateProtocolAnalysisData } | ||
>(POST, `/protocols/${protocolKey}/analyses`, { data }, config) | ||
return response | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,6 +75,7 @@ export interface InputFieldProps { | |
| typeof TYPOGRAPHY.textAlignCenter | ||
/** small or medium input field height, relevant only */ | ||
size?: 'medium' | 'small' | ||
ref?: React.MutableRefObject<null> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mistake from merging in edge. I will fix |
||
} | ||
|
||
export function InputField(props: InputFieldProps): JSX.Element { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import * as React from 'react' | ||
import { KeyboardReact as Keyboard } from 'react-simple-keyboard' | ||
import Keyboard from 'react-simple-keyboard' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Other keyboards use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above. reverting... |
||
import { numericalKeyboardLayout, numericalCustom } from '../constants' | ||
import '../index.css' | ||
import './index.css' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,11 @@ import * as React from 'react' | |
import { when } from 'vitest-when' | ||
import { it, describe, beforeEach, vi, expect } from 'vitest' | ||
import { fireEvent, screen } from '@testing-library/react' | ||
import { useCreateRunMutation, useHost } from '@opentrons/react-api-client' | ||
import { | ||
useCreateProtocolAnalysisMutation, | ||
useCreateRunMutation, | ||
useHost, | ||
} from '@opentrons/react-api-client' | ||
import { i18n } from '../../../i18n' | ||
import { renderWithProviders } from '../../../__testing-utils__' | ||
import { ProtocolSetupParameters } from '..' | ||
|
@@ -24,6 +28,7 @@ vi.mock('react-router-dom', async importOriginal => { | |
} | ||
}) | ||
const MOCK_HOST_CONFIG: HostConfig = { hostname: 'MOCK_HOST' } | ||
const mockCreateProtocolAnalysis = vi.fn() | ||
const mockCreateRun = vi.fn() | ||
const render = ( | ||
props: React.ComponentProps<typeof ProtocolSetupParameters> | ||
|
@@ -43,6 +48,9 @@ describe('ProtocolSetupParameters', () => { | |
} | ||
vi.mocked(ChooseEnum).mockReturnValue(<div>mock ChooseEnum</div>) | ||
vi.mocked(useHost).mockReturnValue(MOCK_HOST_CONFIG) | ||
when(vi.mocked(useCreateProtocolAnalysisMutation)) | ||
.calledWith(expect.anything(), expect.anything(), expect.anything()) | ||
.thenReturn({ createProtocolAnalysis: mockCreateProtocolAnalysis } as any) | ||
when(vi.mocked(useCreateRunMutation)) | ||
.calledWith(expect.anything()) | ||
.thenReturn({ createRun: mockCreateRun } as any) | ||
|
@@ -62,10 +70,10 @@ describe('ProtocolSetupParameters', () => { | |
}) | ||
it('renders the other setting when boolean param is selected', () => { | ||
render(props) | ||
screen.getByText('Off') | ||
expect(screen.getAllByText('On')).toHaveLength(3) | ||
screen.debug() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can be removed if there is no specific reason to keep this debug line. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just an oversight, thanks |
||
expect(screen.getAllByText('On')).toHaveLength(2) | ||
fireEvent.click(screen.getByText('Dry Run')) | ||
expect(screen.getAllByText('On')).toHaveLength(4) | ||
expect(screen.getAllByText('On')).toHaveLength(3) | ||
}) | ||
it('renders the back icon and calls useHistory', () => { | ||
render(props) | ||
|
@@ -88,6 +96,5 @@ describe('ProtocolSetupParameters', () => { | |
const title = screen.getByText('Reset parameter values?') | ||
fireEvent.click(screen.getByRole('button', { name: 'Go back' })) | ||
expect(title).not.toBeInTheDocument() | ||
// TODO(jr, 3/19/24): wire up the confirm button | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,7 +1,11 @@ | ||||||||||||||||||||
import * as React from 'react' | ||||||||||||||||||||
import { useTranslation } from 'react-i18next' | ||||||||||||||||||||
import { useHistory } from 'react-router-dom' | ||||||||||||||||||||
import { useCreateRunMutation, useHost } from '@opentrons/react-api-client' | ||||||||||||||||||||
import { | ||||||||||||||||||||
useCreateProtocolAnalysisMutation, | ||||||||||||||||||||
useCreateRunMutation, | ||||||||||||||||||||
useHost, | ||||||||||||||||||||
} from '@opentrons/react-api-client' | ||||||||||||||||||||
import { useQueryClient } from 'react-query' | ||||||||||||||||||||
import { | ||||||||||||||||||||
ALIGN_CENTER, | ||||||||||||||||||||
|
@@ -51,7 +55,12 @@ export function ProtocolSetupParameters({ | |||||||||||||||||||
const [ | ||||||||||||||||||||
runTimeParametersOverrides, | ||||||||||||||||||||
setRunTimeParametersOverrides, | ||||||||||||||||||||
] = React.useState<RunTimeParameter[]>(runTimeParameters) | ||||||||||||||||||||
] = React.useState<RunTimeParameter[]>( | ||||||||||||||||||||
// present defaults rather than last-set value | ||||||||||||||||||||
runTimeParameters.map(param => { | ||||||||||||||||||||
return { ...param, value: param.default } | ||||||||||||||||||||
}) | ||||||||||||||||||||
) | ||||||||||||||||||||
|
||||||||||||||||||||
const updateParameters = ( | ||||||||||||||||||||
value: boolean | string | number, | ||||||||||||||||||||
|
@@ -85,6 +94,15 @@ export function ProtocolSetupParameters({ | |||||||||||||||||||
} | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
const runTimeParameterValues = getRunTimeParameterValuesForRun( | ||||||||||||||||||||
runTimeParametersOverrides | ||||||||||||||||||||
) | ||||||||||||||||||||
const { createProtocolAnalysis } = useCreateProtocolAnalysisMutation( | ||||||||||||||||||||
{}, | ||||||||||||||||||||
protocolId, | ||||||||||||||||||||
host | ||||||||||||||||||||
) | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we change like below export function useCreateProtocolAnalysisMutation(
protocolId: string | null,
hostOverride?: HostConfig | null,
options: UseCreateProtocolAnalysisMutationOptions | undefined = {}
): UseCreateProtocolMutationResult { We don't need to pass
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. smart. fixing that. |
||||||||||||||||||||
|
||||||||||||||||||||
const { createRun, isLoading } = useCreateRunMutation({ | ||||||||||||||||||||
onSuccess: data => { | ||||||||||||||||||||
queryClient | ||||||||||||||||||||
|
@@ -96,6 +114,10 @@ export function ProtocolSetupParameters({ | |||||||||||||||||||
}) | ||||||||||||||||||||
const handleConfirmValues = (): void => { | ||||||||||||||||||||
setStartSetup(true) | ||||||||||||||||||||
createProtocolAnalysis({ | ||||||||||||||||||||
protocolKey: protocolId, | ||||||||||||||||||||
runTimeParameterValues: runTimeParameterValues, | ||||||||||||||||||||
}) | ||||||||||||||||||||
createRun({ | ||||||||||||||||||||
protocolId, | ||||||||||||||||||||
labwareOffsets, | ||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
import * as React from 'react' | ||
import { describe, it, expect, beforeEach, vi } from 'vitest' | ||
import { QueryClient, QueryClientProvider } from 'react-query' | ||
import { act, renderHook, waitFor } from '@testing-library/react' | ||
import { createProtocolAnalysis } from '@opentrons/api-client' | ||
import { useHost } from '../../api' | ||
import { useCreateProtocolAnalysisMutation } from '..' | ||
import type { HostConfig, Response } from '@opentrons/api-client' | ||
import type { ProtocolAnalysisSummary } from '@opentrons/shared-data' | ||
|
||
vi.mock('@opentrons/api-client') | ||
vi.mock('../../api/useHost') | ||
|
||
const HOST_CONFIG: HostConfig = { hostname: 'localhost' } | ||
const ANALYSIS_SUMMARY_RESPONSE = [ | ||
{ id: 'fakeAnalysis1', status: 'completed' }, | ||
{ id: 'fakeAnalysis2', status: 'pending' }, | ||
] as ProtocolAnalysisSummary[] | ||
|
||
describe('useCreateProtocolAnalysisMutation hook', () => { | ||
let wrapper: React.FunctionComponent<{ children: React.ReactNode }> | ||
|
||
beforeEach(() => { | ||
const queryClient = new QueryClient() | ||
const clientProvider: React.FunctionComponent<{ | ||
children: React.ReactNode | ||
}> = ({ children }) => ( | ||
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider> | ||
) | ||
wrapper = clientProvider | ||
}) | ||
|
||
it('should return no data when calling createProtocolAnalysis if the request fails', async () => { | ||
vi.mocked(useHost).mockReturnValue(HOST_CONFIG) | ||
vi.mocked(createProtocolAnalysis).mockRejectedValue('oh no') | ||
|
||
const { result } = renderHook( | ||
() => useCreateProtocolAnalysisMutation({}, 'fake-protocol-key'), | ||
{ | ||
wrapper, | ||
} | ||
) | ||
|
||
expect(result.current.data).toBeUndefined() | ||
result.current.createProtocolAnalysis({ | ||
protocolKey: 'fake-protocol-key', | ||
runTimeParameterValues: {}, | ||
}) | ||
await waitFor(() => { | ||
expect(result.current.data).toBeUndefined() | ||
}) | ||
}) | ||
|
||
it('should create an array of ProtocolAnalysisSummaries when calling the createProtocolAnalysis callback', async () => { | ||
vi.mocked(useHost).mockReturnValue(HOST_CONFIG) | ||
vi.mocked(createProtocolAnalysis).mockResolvedValue({ | ||
data: ANALYSIS_SUMMARY_RESPONSE, | ||
} as Response<ProtocolAnalysisSummary[]>) | ||
|
||
const { result } = renderHook( | ||
() => useCreateProtocolAnalysisMutation({}, 'fake-protocol-key'), | ||
{ | ||
wrapper, | ||
} | ||
) | ||
act(() => | ||
result.current.createProtocolAnalysis({ | ||
protocolKey: 'fake-protocol-key', | ||
runTimeParameterValues: {}, | ||
}) | ||
) | ||
|
||
await waitFor(() => { | ||
expect(result.current.data).toEqual(ANALYSIS_SUMMARY_RESPONSE) | ||
}) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,87 @@ | ||||||||||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||||||||||
UseMutationResult, | ||||||||||||||||||||||||||||||||||||
UseMutationOptions, | ||||||||||||||||||||||||||||||||||||
useMutation, | ||||||||||||||||||||||||||||||||||||
UseMutateFunction, | ||||||||||||||||||||||||||||||||||||
useQueryClient, | ||||||||||||||||||||||||||||||||||||
} from 'react-query' | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||
import { createProtocolAnalysis } from '@opentrons/api-client' | ||||||||||||||||||||||||||||||||||||
import { useHost } from '../api' | ||||||||||||||||||||||||||||||||||||
import type { AxiosError } from 'axios' | ||||||||||||||||||||||||||||||||||||
import type { | ||||||||||||||||||||||||||||||||||||
ErrorResponse, | ||||||||||||||||||||||||||||||||||||
HostConfig, | ||||||||||||||||||||||||||||||||||||
RunTimeParameterCreateData, | ||||||||||||||||||||||||||||||||||||
} from '@opentrons/api-client' | ||||||||||||||||||||||||||||||||||||
import { ProtocolAnalysisSummary } from '@opentrons/shared-data' | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
export interface CreateProtocolAnalysisVariables { | ||||||||||||||||||||||||||||||||||||
protocolKey: string | ||||||||||||||||||||||||||||||||||||
runTimeParameterValues?: RunTimeParameterCreateData | ||||||||||||||||||||||||||||||||||||
forceReAnalyze?: boolean | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
export type UseCreateProtocolMutationResult = UseMutationResult< | ||||||||||||||||||||||||||||||||||||
ProtocolAnalysisSummary[], | ||||||||||||||||||||||||||||||||||||
AxiosError<ErrorResponse>, | ||||||||||||||||||||||||||||||||||||
CreateProtocolAnalysisVariables | ||||||||||||||||||||||||||||||||||||
> & { | ||||||||||||||||||||||||||||||||||||
createProtocolAnalysis: UseMutateFunction< | ||||||||||||||||||||||||||||||||||||
ProtocolAnalysisSummary[], | ||||||||||||||||||||||||||||||||||||
AxiosError<ErrorResponse>, | ||||||||||||||||||||||||||||||||||||
CreateProtocolAnalysisVariables | ||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
export type UseCreateProtocolAnalysisMutationOptions = UseMutationOptions< | ||||||||||||||||||||||||||||||||||||
ProtocolAnalysisSummary[], | ||||||||||||||||||||||||||||||||||||
AxiosError<ErrorResponse>, | ||||||||||||||||||||||||||||||||||||
CreateProtocolAnalysisVariables | ||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
export function useCreateProtocolAnalysisMutation( | ||||||||||||||||||||||||||||||||||||
options: UseCreateProtocolAnalysisMutationOptions = {}, | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please see the above comment |
||||||||||||||||||||||||||||||||||||
protocolId: string | null, | ||||||||||||||||||||||||||||||||||||
hostOverride?: HostConfig | null | ||||||||||||||||||||||||||||||||||||
): UseCreateProtocolMutationResult { | ||||||||||||||||||||||||||||||||||||
const contextHost = useHost() | ||||||||||||||||||||||||||||||||||||
const host = | ||||||||||||||||||||||||||||||||||||
hostOverride != null ? { ...contextHost, ...hostOverride } : contextHost | ||||||||||||||||||||||||||||||||||||
const queryClient = useQueryClient() | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const mutation = useMutation< | ||||||||||||||||||||||||||||||||||||
ProtocolAnalysisSummary[], | ||||||||||||||||||||||||||||||||||||
AxiosError<ErrorResponse>, | ||||||||||||||||||||||||||||||||||||
CreateProtocolAnalysisVariables | ||||||||||||||||||||||||||||||||||||
>( | ||||||||||||||||||||||||||||||||||||
[host, 'protocols', protocolId, 'analyses'], | ||||||||||||||||||||||||||||||||||||
({ protocolKey, runTimeParameterValues, forceReAnalyze }) => | ||||||||||||||||||||||||||||||||||||
createProtocolAnalysis( | ||||||||||||||||||||||||||||||||||||
host as HostConfig, | ||||||||||||||||||||||||||||||||||||
protocolKey, | ||||||||||||||||||||||||||||||||||||
runTimeParameterValues, | ||||||||||||||||||||||||||||||||||||
forceReAnalyze | ||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||
.then(response => { | ||||||||||||||||||||||||||||||||||||
queryClient | ||||||||||||||||||||||||||||||||||||
.invalidateQueries([host, 'protocols', protocolId, 'analyses']) | ||||||||||||||||||||||||||||||||||||
.then(() => | ||||||||||||||||||||||||||||||||||||
queryClient.setQueryData( | ||||||||||||||||||||||||||||||||||||
[host, 'protocols', protocolId], | ||||||||||||||||||||||||||||||||||||
response.data | ||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||
.catch(e => { | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit
Suggested change
|
||||||||||||||||||||||||||||||||||||
throw e | ||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||
return response.data | ||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||
.catch(e => { | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit
Suggested change
|
||||||||||||||||||||||||||||||||||||
throw e | ||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||
options | ||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||||||
...mutation, | ||||||||||||||||||||||||||||||||||||
createProtocolAnalysis: mutation.mutate, | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be higher