From 4ccd0527a9b6e9dbc72f66c189ce84c51de57797 Mon Sep 17 00:00:00 2001 From: ncdiehl11 Date: Mon, 6 May 2024 14:50:19 -0400 Subject: [PATCH 1/6] fix(app, react-api-client): handle failed analysis for RTP protocol on ODD RTP protocols introduce a new scenario where an RTP protocol can be sent to a Flex, analyze successfully with default values, and then have parameters input that will cause a failed analysis. To handle this, the ProtocolSetup on the ODD should promp the user with a failed analysis modal with call to action return to protocol setup to reselect parameters and start a new run. Keeping consistent with our current behavior for protocols that fail analysis, the user will still have the option to dismiss the re-parameterization CTA and proceed with the run. In addition, we need to modify the ProtocolCard on ODD to search not only for the most recent analysis, but the most recent successful analysis, should one exist. --- .../AnalysisFailedModal.tsx | 4 +- .../pages/ProtocolDashboard/ProtocolCard.tsx | 29 +++++++--- .../__tests__/ProtocolCard.test.tsx | 17 +++++- .../__tests__/ProtocolSetup.test.tsx | 4 ++ app/src/pages/ProtocolSetup/index.tsx | 24 ++++++++- react-api-client/src/protocols/index.ts | 1 + ...RecentSuccessfulAnalysisAsDocumentQuery.ts | 54 +++++++++++++++++++ 7 files changed, 122 insertions(+), 11 deletions(-) create mode 100644 react-api-client/src/protocols/useMostRecentSuccessfulAnalysisAsDocumentQuery.ts diff --git a/app/src/organisms/ProtocolSetupParameters/AnalysisFailedModal.tsx b/app/src/organisms/ProtocolSetupParameters/AnalysisFailedModal.tsx index 9c38db307dc..f52aa8a2265 100644 --- a/app/src/organisms/ProtocolSetupParameters/AnalysisFailedModal.tsx +++ b/app/src/organisms/ProtocolSetupParameters/AnalysisFailedModal.tsx @@ -17,7 +17,7 @@ import type { ModalHeaderBaseProps } from '../../molecules/Modal/types' interface AnalysisFailedModalProps { errors: string[] - protocolId: string + protocolId: string | null setShowAnalysisFailedModal: (showAnalysisFailedModal: boolean) => void } @@ -36,7 +36,7 @@ export function AnalysisFailedModal({ } const handleRestartSetup = (): void => { - history.push(`/protocols/${protocolId}`) + history.push(protocolId != null ? `/protocols/${protocolId}` : '/protocols') } return ( diff --git a/app/src/pages/ProtocolDashboard/ProtocolCard.tsx b/app/src/pages/ProtocolDashboard/ProtocolCard.tsx index 305ca99c7bc..1ddb0ac6099 100644 --- a/app/src/pages/ProtocolDashboard/ProtocolCard.tsx +++ b/app/src/pages/ProtocolDashboard/ProtocolCard.tsx @@ -25,6 +25,7 @@ import { } from '@opentrons/components' import { useHost, + useMostRecentSuccessfulAnalysisAsDocumentQuery, useProtocolAnalysisAsDocumentQuery, } from '@opentrons/react-api-client' import { deleteProtocol, deleteRun, getProtocol } from '@opentrons/api-client' @@ -66,8 +67,20 @@ export function ProtocolCard(props: { const queryClient = useQueryClient() const host = useHost() + const { id: protocolId, analysisSummaries } = protocol + const { + data: mostRecentSuccessfulAnalysis, + } = useMostRecentSuccessfulAnalysisAsDocumentQuery( + protocolId, + analysisSummaries, + { + enabled: protocol != null, + refetchInterval: analysisData => + analysisData == null ? REFETCH_INTERVAL : false, + } + ) const { data: mostRecentAnalysis } = useProtocolAnalysisAsDocumentQuery( - protocol.id, + protocolId, last(protocol.analysisSummaries)?.id ?? null, { enabled: protocol != null, @@ -76,14 +89,18 @@ export function ProtocolCard(props: { } ) + const analysisForProtocolCard = + mostRecentSuccessfulAnalysis == null + ? mostRecentAnalysis + : mostRecentSuccessfulAnalysis const isFailedAnalysis = - (mostRecentAnalysis != null && - 'result' in mostRecentAnalysis && - (mostRecentAnalysis.result === 'error' || - mostRecentAnalysis.result === 'not-ok')) ?? + (analysisForProtocolCard != null && + 'result' in analysisForProtocolCard && + (analysisForProtocolCard.result === 'error' || + analysisForProtocolCard.result === 'not-ok')) ?? false - const isPendingAnalysis = mostRecentAnalysis == null + const isPendingAnalysis = analysisForProtocolCard == null const handleProtocolClick = ( longpress: UseLongPressResult, diff --git a/app/src/pages/ProtocolDashboard/__tests__/ProtocolCard.test.tsx b/app/src/pages/ProtocolDashboard/__tests__/ProtocolCard.test.tsx index 26ec86337fc..040e00d640c 100644 --- a/app/src/pages/ProtocolDashboard/__tests__/ProtocolCard.test.tsx +++ b/app/src/pages/ProtocolDashboard/__tests__/ProtocolCard.test.tsx @@ -3,7 +3,10 @@ import { vi, it, describe, expect, beforeEach } from 'vitest' import { act, fireEvent, screen } from '@testing-library/react' import { MemoryRouter } from 'react-router-dom' import { UseQueryResult } from 'react-query' -import { useProtocolAnalysisAsDocumentQuery } from '@opentrons/react-api-client' +import { + useMostRecentSuccessfulAnalysisAsDocumentQuery, + useProtocolAnalysisAsDocumentQuery, +} from '@opentrons/react-api-client' import { renderWithProviders } from '../../../__testing-utils__' import { i18n } from '../../../i18n' @@ -69,6 +72,9 @@ describe('ProtocolCard', () => { vi.mocked(useProtocolAnalysisAsDocumentQuery).mockReturnValue({ data: { result: 'ok' } as any, } as UseQueryResult) + vi.mocked(useMostRecentSuccessfulAnalysisAsDocumentQuery).mockReturnValue({ + data: { result: 'ok' } as any, + } as UseQueryResult) }) it('should redirect to protocol details after short click', () => { render() @@ -81,6 +87,9 @@ describe('ProtocolCard', () => { vi.mocked(useProtocolAnalysisAsDocumentQuery).mockReturnValue({ data: { result: 'error' } as any, } as UseQueryResult) + vi.mocked(useMostRecentSuccessfulAnalysisAsDocumentQuery).mockReturnValue({ + data: { result: 'not-ok', errors: ['some analysis error'] } as any, + } as UseQueryResult) render() screen.getByLabelText('failedAnalysis_icon') screen.getByText('Failed analysis') @@ -115,6 +124,9 @@ describe('ProtocolCard', () => { vi.mocked(useProtocolAnalysisAsDocumentQuery).mockReturnValue({ data: { result: 'error' } as any, } as UseQueryResult) + vi.mocked(useMostRecentSuccessfulAnalysisAsDocumentQuery).mockReturnValue({ + data: { result: 'not-ok', errors: ['some analysis error'] } as any, + } as UseQueryResult) render() const name = screen.getByText('yay mock protocol') fireEvent.mouseDown(name) @@ -136,6 +148,9 @@ describe('ProtocolCard', () => { vi.mocked(useProtocolAnalysisAsDocumentQuery).mockReturnValue({ data: null as any, } as UseQueryResult) + vi.mocked(useMostRecentSuccessfulAnalysisAsDocumentQuery).mockReturnValue({ + data: null as any, + } as UseQueryResult) render() const name = screen.getByText('yay mock protocol') fireEvent.mouseDown(name) diff --git a/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx b/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx index 32bc9963d0a..623761693f3 100644 --- a/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx +++ b/app/src/pages/ProtocolSetup/__tests__/ProtocolSetup.test.tsx @@ -29,6 +29,7 @@ import { useAttachedModules, useLPCDisabledReason, useModuleCalibrationStatus, + useProtocolAnalysisErrors, useRobotType, useRunCreatedAtTimestamp, useTrackProtocolRunEvent, @@ -248,6 +249,9 @@ describe('ProtocolSetup', () => { }, }, } as any) + when(vi.mocked(useProtocolAnalysisErrors)) + .calledWith(RUN_ID) + .thenReturn({ analysisErrors: null }) when(vi.mocked(useProtocolQuery)) .calledWith(PROTOCOL_ID, { staleTime: Infinity }) .thenReturn({ diff --git a/app/src/pages/ProtocolSetup/index.tsx b/app/src/pages/ProtocolSetup/index.tsx index 0e60ab60328..093deb6879a 100644 --- a/app/src/pages/ProtocolSetup/index.tsx +++ b/app/src/pages/ProtocolSetup/index.tsx @@ -6,7 +6,7 @@ import { useHistory, useParams } from 'react-router-dom' import first from 'lodash/first' import { css } from 'styled-components' -import { RUN_STATUS_IDLE, RUN_STATUS_STOPPED } from '@opentrons/api-client' +import { RUN_STATUS_IDLE, RUN_STATUS_STOPPED, Run } from '@opentrons/api-client' import { ALIGN_CENTER, BORDERS, @@ -47,6 +47,7 @@ import { useAttachedModules, useLPCDisabledReason, useModuleCalibrationStatus, + useProtocolAnalysisErrors, useRobotAnalyticsData, useRobotType, useTrackProtocolRunEvent, @@ -64,6 +65,7 @@ import { ProtocolSetupDeckConfiguration } from '../../organisms/ProtocolSetupDec import { useLaunchLPC } from '../../organisms/LabwarePositionCheck/useLaunchLPC' import { getUnmatchedModulesForProtocol } from '../../organisms/ProtocolSetupModulesAndDeck/utils' import { ConfirmCancelRunModal } from '../../organisms/OnDeviceDisplay/RunningProtocol' +import { AnalysisFailedModal } from '../../organisms/ProtocolSetupParameters/AnalysisFailedModal' import { getIncompleteInstrumentCount, getProtocolUsesGripper, @@ -251,6 +253,7 @@ interface PrepareToRunProps { confirmAttachment: () => void play: () => void robotName: string + runRecord: Run | null } function PrepareToRun({ @@ -259,6 +262,7 @@ function PrepareToRun({ confirmAttachment, play, robotName, + runRecord, }: PrepareToRunProps): JSX.Element { const { t, i18n } = useTranslation(['protocol_setup', 'shared']) const history = useHistory() @@ -272,7 +276,6 @@ function PrepareToRun({ observer.observe(scrollRef.current) } - const { data: runRecord } = useNotifyRunQuery(runId, { staleTime: Infinity }) const protocolId = runRecord?.data?.protocolId ?? null const { data: protocolRecord } = useProtocolQuery(protocolId, { staleTime: Infinity, @@ -797,11 +800,18 @@ export type SetupScreens = export function ProtocolSetup(): JSX.Element { const { runId } = useParams() + console.log(runId) + const { data: runRecord } = useNotifyRunQuery(runId, { staleTime: Infinity }) + const { analysisErrors } = useProtocolAnalysisErrors(runId) const localRobot = useSelector(getLocalRobot) const robotSerialNumber = localRobot?.status != null ? getRobotSerialNumber(localRobot) : null const trackEvent = useTrackEvent() const { play } = useRunControls(runId) + const [ + showAnalysisFailedModal, + setShowAnalysisFailedModal, + ] = React.useState(true) const handleProceedToRunClick = (): void => { trackEvent({ @@ -838,6 +848,7 @@ export function ProtocolSetup(): JSX.Element { confirmAttachment={confirmAttachment} play={play} robotName={localRobot?.name != null ? localRobot.name : 'no name'} + runRecord={runRecord ?? null} /> ), instruments: ( @@ -872,6 +883,15 @@ export function ProtocolSetup(): JSX.Element { return ( <> + {showAnalysisFailedModal && + analysisErrors != null && + analysisErrors?.length > 0 && ( + error.detail)} + /> + )} {showConfirmationModal ? ( => { + for (const analysisId of analysisSummaryIds) { + const { data: analysis } = await getProtocolAnalysisAsDocument( + host as HostConfig, + protocolId, + analysisId + ) + if (analysis.errors.length === 0) { + return analysis + } + } + return null +} + +export function useMostRecentSuccessfulAnalysisAsDocumentQuery( + protocolId: string, + analysisSummaries: ProtocolAnalysisSummary[], + options: UseQueryOptions = {} +): UseQueryResult { + const host = useHost() + + const query = useQuery( + [host, 'protocols', protocolId, 'analyses', 'mostRecentSuccessful'], + async () => { + const analysisIds = analysisSummaries.map(summary => summary.id) + + const mostRecentSuccessfulAnalysis = await getMostRecentSuccessfulAnalysisId( + analysisIds, + host, + protocolId + ) + + return mostRecentSuccessfulAnalysis + }, + options + ) + + return query +} From 53952912181a1f74d7cffdff4005576fb323a1a5 Mon Sep 17 00:00:00 2001 From: ncdiehl11 Date: Mon, 6 May 2024 14:54:00 -0400 Subject: [PATCH 2/6] remove console log --- app/src/pages/ProtocolSetup/index.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/pages/ProtocolSetup/index.tsx b/app/src/pages/ProtocolSetup/index.tsx index 093deb6879a..e69f8bb26fa 100644 --- a/app/src/pages/ProtocolSetup/index.tsx +++ b/app/src/pages/ProtocolSetup/index.tsx @@ -800,7 +800,6 @@ export type SetupScreens = export function ProtocolSetup(): JSX.Element { const { runId } = useParams() - console.log(runId) const { data: runRecord } = useNotifyRunQuery(runId, { staleTime: Infinity }) const { analysisErrors } = useProtocolAnalysisErrors(runId) const localRobot = useSelector(getLocalRobot) From ba0a269aaaa2e0127fafc3eea82f5297db0ede03 Mon Sep 17 00:00:00 2001 From: ncdiehl11 Date: Mon, 6 May 2024 16:27:57 -0400 Subject: [PATCH 3/6] add test for null protocolId --- .../__tests__/AnalysisFailedModal.test.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/src/organisms/ProtocolSetupParameters/__tests__/AnalysisFailedModal.test.tsx b/app/src/organisms/ProtocolSetupParameters/__tests__/AnalysisFailedModal.test.tsx index 9d6436a11bf..143f6d92feb 100644 --- a/app/src/organisms/ProtocolSetupParameters/__tests__/AnalysisFailedModal.test.tsx +++ b/app/src/organisms/ProtocolSetupParameters/__tests__/AnalysisFailedModal.test.tsx @@ -60,4 +60,10 @@ describe('AnalysisFailedModal', () => { fireEvent.click(screen.getByText('Restart setup')) expect(mockPush).toHaveBeenCalledWith(`/protocols/${PROTOCOL_ID}`) }) + + it('should push to protocols dashboard when tapping restart setup button and protocol ID is null', () => { + render({ ...props, protocolId: null }) + fireEvent.click(screen.getByText('Restart setup')) + expect(mockPush).toHaveBeenCalledWith('/protocols') + }) }) From dc6496b6f68d59256fe8bd9111789f03a559fbc6 Mon Sep 17 00:00:00 2001 From: ncdiehl11 Date: Mon, 6 May 2024 16:28:20 -0400 Subject: [PATCH 4/6] refactor imports and ternary --- app/src/pages/ProtocolSetup/index.tsx | 16 ++++++++-------- ...ostRecentSuccessfulAnalysisAsDocumentQuery.ts | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/src/pages/ProtocolSetup/index.tsx b/app/src/pages/ProtocolSetup/index.tsx index e69f8bb26fa..1e85d8ae63f 100644 --- a/app/src/pages/ProtocolSetup/index.tsx +++ b/app/src/pages/ProtocolSetup/index.tsx @@ -883,14 +883,14 @@ export function ProtocolSetup(): JSX.Element { return ( <> {showAnalysisFailedModal && - analysisErrors != null && - analysisErrors?.length > 0 && ( - error.detail)} - /> - )} + analysisErrors != null && + analysisErrors?.length > 0 ? ( + error.detail)} + /> + ) : null} {showConfirmationModal ? ( Date: Mon, 6 May 2024 16:53:01 -0400 Subject: [PATCH 5/6] fix type import --- app/src/pages/ProtocolSetup/index.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/pages/ProtocolSetup/index.tsx b/app/src/pages/ProtocolSetup/index.tsx index 1e85d8ae63f..3d103a89df6 100644 --- a/app/src/pages/ProtocolSetup/index.tsx +++ b/app/src/pages/ProtocolSetup/index.tsx @@ -6,7 +6,7 @@ import { useHistory, useParams } from 'react-router-dom' import first from 'lodash/first' import { css } from 'styled-components' -import { RUN_STATUS_IDLE, RUN_STATUS_STOPPED, Run } from '@opentrons/api-client' +import { RUN_STATUS_IDLE, RUN_STATUS_STOPPED } from '@opentrons/api-client' import { ALIGN_CENTER, BORDERS, @@ -92,6 +92,7 @@ import { getRequiredDeckConfig } from '../../resources/deck_configuration/utils' import { useNotifyRunQuery } from '../../resources/runs' import { ViewOnlyParameters } from '../../organisms/ProtocolSetupParameters/ViewOnlyParameters' +import type { Run } from '@opentrons/api-client' import type { CutoutFixtureId, CutoutId } from '@opentrons/shared-data' import type { OnDeviceRouteParams } from '../../App/types' import type { From 8f5e3ae3e8011b962d5d67a4c5690f88d4ffb2d9 Mon Sep 17 00:00:00 2001 From: ncdiehl11 Date: Mon, 6 May 2024 17:27:36 -0400 Subject: [PATCH 6/6] disable parameters after values are confirmed --- app/src/organisms/ProtocolSetupParameters/index.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/organisms/ProtocolSetupParameters/index.tsx b/app/src/organisms/ProtocolSetupParameters/index.tsx index e2bdde62f97..c99e38c39b5 100644 --- a/app/src/organisms/ProtocolSetupParameters/index.tsx +++ b/app/src/organisms/ProtocolSetupParameters/index.tsx @@ -174,6 +174,7 @@ export function ProtocolSetupParameters({ detail={formatRunTimeParameterValue(parameter, t)} description={parameter.description} fontSize="h4" + disabled={startSetup} /> )