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

refactor(app): improve top level query chaining #16179

Merged
merged 3 commits into from
Sep 3, 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
1 change: 1 addition & 0 deletions app/src/App/ODDTopLevelRedirects/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const CURRENT_RUN_POLL = 5000
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import { renderHook } from '@testing-library/react'
import { describe, it, expect, vi } from 'vitest'
import { useCurrentRunRoute } from '../useCurrentRunRoute'
import { useNotifyRunQuery } from '../../../../resources/runs'
import {
RUN_STATUS_BLOCKED_BY_OPEN_DOOR,
RUN_STATUS_FAILED,
RUN_STATUS_IDLE,
RUN_STATUS_STOPPED,
RUN_STATUS_SUCCEEDED,
} from '@opentrons/api-client'

vi.mock('../../../../resources/runs')

const MOCK_RUN_ID = 'MOCK_RUN_ID'

describe('useCurrentRunRoute', () => {
it('returns null when the run record is null', () => {
vi.mocked(useNotifyRunQuery).mockReturnValue({
data: null,
isFetching: false,
} as any)

const { result } = renderHook(() => useCurrentRunRoute(MOCK_RUN_ID))

expect(result.current).toBeNull()
})

it('returns null when isFetching is true', () => {
vi.mocked(useNotifyRunQuery).mockReturnValue({
data: { data: { startedAt: '123' } },
isFetching: true,
} as any)

const { result } = renderHook(() => useCurrentRunRoute(MOCK_RUN_ID))

expect(result.current).toBeNull()
})

it('returns the summary route for a run with succeeded status', () => {
vi.mocked(useNotifyRunQuery).mockReturnValue({
data: {
data: {
id: MOCK_RUN_ID,
status: RUN_STATUS_SUCCEEDED,
startedAt: '123',
},
},
isFetching: false,
} as any)

const { result } = renderHook(() => useCurrentRunRoute(MOCK_RUN_ID))

expect(result.current).toBe(`/runs/${MOCK_RUN_ID}/summary`)
})

it('returns the summary route for a started run that has a stopped status', () => {
vi.mocked(useNotifyRunQuery).mockReturnValue({
data: {
data: {
id: MOCK_RUN_ID,
status: RUN_STATUS_STOPPED,
startedAt: '123',
},
},
isFetching: false,
} as any)

const { result } = renderHook(() => useCurrentRunRoute(MOCK_RUN_ID))

expect(result.current).toBe(`/runs/${MOCK_RUN_ID}/summary`)
})

it('returns summary route for a run with failed status', () => {
vi.mocked(useNotifyRunQuery).mockReturnValue({
data: {
data: { id: MOCK_RUN_ID, status: RUN_STATUS_FAILED, startedAt: '123' },
},
isFetching: false,
} as any)

const { result } = renderHook(() => useCurrentRunRoute(MOCK_RUN_ID))

expect(result.current).toBe(`/runs/${MOCK_RUN_ID}/summary`)
})

it('returns the setup route for a run with an idle status', () => {
vi.mocked(useNotifyRunQuery).mockReturnValue({
data: {
data: { id: MOCK_RUN_ID, status: RUN_STATUS_IDLE, startedAt: null },
},
isFetching: false,
} as any)

const { result } = renderHook(() => useCurrentRunRoute(MOCK_RUN_ID))

expect(result.current).toBe(`/runs/${MOCK_RUN_ID}/setup`)
})

it('returns the setup route for a "blocked by open door" run that has not been started yet', () => {
vi.mocked(useNotifyRunQuery).mockReturnValue({
data: {
data: {
id: MOCK_RUN_ID,
status: RUN_STATUS_BLOCKED_BY_OPEN_DOOR,
startedAt: null,
},
},
isFetching: false,
} as any)

const { result } = renderHook(() => useCurrentRunRoute(MOCK_RUN_ID))

expect(result.current).toBe(`/runs/${MOCK_RUN_ID}/setup`)
})

it('returns run route for a started run', () => {
vi.mocked(useNotifyRunQuery).mockReturnValue({
data: {
data: {
id: MOCK_RUN_ID,
status: RUN_STATUS_BLOCKED_BY_OPEN_DOOR,
startedAt: '123',
},
},
isFetching: false,
} as any)

const { result } = renderHook(() => useCurrentRunRoute(MOCK_RUN_ID))

expect(result.current).toBe(`/runs/${MOCK_RUN_ID}/run`)
})

it('returns null for cancelled run before starting', () => {
vi.mocked(useNotifyRunQuery).mockReturnValue({
data: {
data: { id: MOCK_RUN_ID, status: RUN_STATUS_STOPPED, startedAt: null },
},
isFetching: false,
} as any)

const { result } = renderHook(() => useCurrentRunRoute(MOCK_RUN_ID))

expect(result.current).toBeNull()
})
})
1 change: 1 addition & 0 deletions app/src/App/ODDTopLevelRedirects/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { useCurrentRunRoute } from './useCurrentRunRoute'
42 changes: 42 additions & 0 deletions app/src/App/ODDTopLevelRedirects/hooks/useCurrentRunRoute.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import {
RUN_STATUS_BLOCKED_BY_OPEN_DOOR,
RUN_STATUS_FAILED,
RUN_STATUS_IDLE,
RUN_STATUS_STOPPED,
RUN_STATUS_SUCCEEDED,
} from '@opentrons/api-client'

import { useNotifyRunQuery } from '../../../resources/runs'
import { CURRENT_RUN_POLL } from '../constants'

// Returns the route to which React Router should navigate, if any.
export function useCurrentRunRoute(currentRunId: string): string | null {
const { data: runRecord, isFetching } = useNotifyRunQuery(currentRunId, {
refetchInterval: CURRENT_RUN_POLL,
})

// grabbing run id off of the run query to have all routing info come from one source of truth
const runId = runRecord?.data.id
const hasRunStarted = runRecord?.data.startedAt != null
const runStatus = runRecord?.data.status

if (isFetching) {
return null
} else if (
runStatus === RUN_STATUS_SUCCEEDED ||
(runStatus === RUN_STATUS_STOPPED && hasRunStarted) ||
runStatus === RUN_STATUS_FAILED
) {
return `/runs/${runId}/summary`
} else if (
runStatus === RUN_STATUS_IDLE ||
(!hasRunStarted && runStatus === RUN_STATUS_BLOCKED_BY_OPEN_DOOR)
) {
return `/runs/${runId}/setup`
} else if (hasRunStarted) {
return `/runs/${runId}/run`
} else {
// includes runs cancelled before starting and runs not yet started
return null
}
}
28 changes: 28 additions & 0 deletions app/src/App/ODDTopLevelRedirects/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import * as React from 'react'
import { Navigate, Route, Routes } from 'react-router-dom'

import { useCurrentRunId } from '../../resources/runs'
import { CURRENT_RUN_POLL } from './constants'
import { useCurrentRunRoute } from './hooks'

export function ODDTopLevelRedirects(): JSX.Element | null {
const currentRunId = useCurrentRunId({ refetchInterval: CURRENT_RUN_POLL })

return currentRunId != null ? (
<CurrentRunRoute currentRunId={currentRunId} />
) : null
}

function CurrentRunRoute({
currentRunId,
}: {
currentRunId: string
}): JSX.Element | null {
const currentRunRoute = useCurrentRunRoute(currentRunId)

return currentRunRoute != null ? (
<Routes>
<Route path="*" element={<Navigate to={currentRunRoute} />} />
</Routes>
) : null
}
22 changes: 5 additions & 17 deletions app/src/App/OnDeviceDisplayApp.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import * as React from 'react'
import { useDispatch, useSelector } from 'react-redux'
import { Routes, Route, Navigate } from 'react-router-dom'
import { Navigate, Route, Routes } from 'react-router-dom'
import { css } from 'styled-components'
import { ErrorBoundary } from 'react-error-boundary'

import {
Box,
POSITION_RELATIVE,
COLORS,
OVERFLOW_AUTO,
POSITION_RELATIVE,
useIdle,
useScrolling,
} from '@opentrons/components'
Expand Down Expand Up @@ -49,11 +49,8 @@ import { PortalRoot as ModalPortalRoot } from './portal'
import { getOnDeviceDisplaySettings, updateConfigValue } from '../redux/config'
import { updateBrightness } from '../redux/shell'
import { SLEEP_NEVER_MS } from './constants'
import {
useCurrentRunRoute,
useProtocolReceiptToast,
useSoftwareUpdatePoll,
} from './hooks'
import { useProtocolReceiptToast, useSoftwareUpdatePoll } from './hooks'
import { ODDTopLevelRedirects } from './ODDTopLevelRedirects'

import { OnDeviceDisplayAppFallback } from './OnDeviceDisplayAppFallback'

Expand Down Expand Up @@ -201,7 +198,7 @@ export const OnDeviceDisplayApp = (): JSX.Element => {
</>
)}
</Box>
<TopLevelRedirects />
<ODDTopLevelRedirects />
</ErrorBoundary>
</OnDeviceLocalizationProvider>
</InitialLoadingScreen>
Expand Down Expand Up @@ -272,15 +269,6 @@ export function OnDeviceDisplayAppRoutes(): JSX.Element {
)
}

function TopLevelRedirects(): JSX.Element | null {
const currentRunRoute = useCurrentRunRoute()
return currentRunRoute != null ? (
<Routes>
<Route path="*" element={<Navigate to={currentRunRoute} />} />
</Routes>
) : null
}

function ProtocolReceiptToasts(): null {
useProtocolReceiptToast()
return null
Expand Down
11 changes: 9 additions & 2 deletions app/src/App/__tests__/OnDeviceDisplayApp.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ import { getOnDeviceDisplaySettings } from '../../redux/config'
import { getIsShellReady } from '../../redux/shell'
import { getLocalRobot } from '../../redux/discovery'
import { mockConnectedRobot } from '../../redux/discovery/__fixtures__'
import { useCurrentRunRoute, useProtocolReceiptToast } from '../hooks'
import { useProtocolReceiptToast } from '../hooks'
import { useNotifyCurrentMaintenanceRun } from '../../resources/maintenance_runs'
import { ODDTopLevelRedirects } from '../ODDTopLevelRedirects'

import type { UseQueryResult } from 'react-query'
import type { RobotSettingsResponse } from '@opentrons/api-client'
Expand Down Expand Up @@ -67,6 +68,7 @@ vi.mock('../../redux/shell')
vi.mock('../../redux/discovery')
vi.mock('../../resources/maintenance_runs')
vi.mock('../hooks')
vi.mock('../ODDTopLevelRedirects')

const mockSettings = {
sleepMs: 60 * 1000 * 60 * 24 * 7,
Expand All @@ -88,7 +90,7 @@ describe('OnDeviceDisplayApp', () => {
beforeEach(() => {
vi.mocked(getOnDeviceDisplaySettings).mockReturnValue(mockSettings as any)
vi.mocked(getIsShellReady).mockReturnValue(true)
vi.mocked(useCurrentRunRoute).mockReturnValue(null)
vi.mocked(ODDTopLevelRedirects).mockReturnValue(null)
vi.mocked(getLocalRobot).mockReturnValue(mockConnectedRobot)
vi.mocked(useNotifyCurrentMaintenanceRun).mockReturnValue({
data: {
Expand Down Expand Up @@ -187,4 +189,9 @@ describe('OnDeviceDisplayApp', () => {
render('/')
expect(vi.mocked(useProtocolReceiptToast)).toHaveBeenCalled()
})
it('renders TopLevelRedirects when it should conditionally render', () => {
vi.mocked(ODDTopLevelRedirects).mockReturnValue(<div>MOCK_REDIRECTS</div>)
render('/')
screen.getByText('MOCK_REDIRECTS')
})
})
52 changes: 1 addition & 51 deletions app/src/App/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,14 @@ import {
useHost,
useCreateLiveCommandMutation,
} from '@opentrons/react-api-client'
import {
getProtocol,
RUN_ACTION_TYPE_PLAY,
RUN_STATUS_BLOCKED_BY_OPEN_DOOR,
RUN_STATUS_IDLE,
RUN_STATUS_STOPPED,
RUN_STATUS_FAILED,
RUN_STATUS_SUCCEEDED,
} from '@opentrons/api-client'
import { getProtocol } from '@opentrons/api-client'

import { checkShellUpdate } from '../redux/shell'
import { useToaster } from '../organisms/ToasterOven'
import { useCurrentRunId, useNotifyRunQuery } from '../resources/runs'

import type { SetStatusBarCreateCommand } from '@opentrons/shared-data'
import type { Dispatch } from '../redux/types'

const CURRENT_RUN_POLL = 5000
const UPDATE_RECHECK_INTERVAL_MS = 60000
const PROTOCOL_IDS_RECHECK_INTERVAL_MS = 3000

Expand Down Expand Up @@ -123,43 +113,3 @@ export function useProtocolReceiptToast(): void {
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [protocolIds])
}

export function useCurrentRunRoute(): string | null {
const currentRunId = useCurrentRunId({ refetchInterval: CURRENT_RUN_POLL })
const { data: runRecord, isFetching } = useNotifyRunQuery(currentRunId, {
refetchInterval: CURRENT_RUN_POLL,
})

const runStatus = runRecord?.data.status
const runActions = runRecord?.data.actions
if (
runRecord == null ||
runStatus == null ||
runActions == null ||
isFetching
) {
return null
}
// grabbing run id off of the run query to have all routing info come from one source of truth
const runId = runRecord.data.id
const hasRunStarted = runActions?.some(
action => action.actionType === RUN_ACTION_TYPE_PLAY
)
if (
runStatus === RUN_STATUS_SUCCEEDED ||
(runStatus === RUN_STATUS_STOPPED && hasRunStarted) ||
runStatus === RUN_STATUS_FAILED
) {
return `/runs/${runId}/summary`
} else if (
runStatus === RUN_STATUS_IDLE ||
(!hasRunStarted && runStatus === RUN_STATUS_BLOCKED_BY_OPEN_DOOR)
) {
return `/runs/${runId}/setup`
} else if (hasRunStarted) {
return `/runs/${runId}/run`
} else {
// includes runs cancelled before starting and runs not yet started
return null
}
}
Loading