Skip to content

Commit

Permalink
refactor(app): improve top level query chaining
Browse files Browse the repository at this point in the history
  • Loading branch information
mjhuff committed Sep 3, 2024
1 parent ee740c0 commit 738becd
Show file tree
Hide file tree
Showing 8 changed files with 237 additions and 69 deletions.
20 changes: 4 additions & 16 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 { TopLevelRedirects } from './TopLevelRedirects'

import { OnDeviceDisplayAppFallback } from './OnDeviceDisplayAppFallback'

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
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import { renderHook } from '@testing-library/react'
import { describe, it, expect, vi } from 'vitest'
import { useCurrentRunRoute } from '../useCurrentRunRoute'
import { useNotifyRunQuery } from '../../../../resources/runs'
import {
RUN_ACTION_TYPE_PLAY,
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: {} },
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, actions: [] },
},
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,
actions: [{ actionType: RUN_ACTION_TYPE_PLAY }],
},
},
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, actions: [] },
},
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, actions: [] } },
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,
actions: [],
},
},
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,
actions: [{ actionType: RUN_ACTION_TYPE_PLAY }],
},
},
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, actions: [] },
},
isFetching: false,
} as any)

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

expect(result.current).toBeNull()
})
})
1 change: 1 addition & 0 deletions app/src/App/TopLevelRedirects/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { useCurrentRunRoute } from './useCurrentRunRoute'
50 changes: 50 additions & 0 deletions app/src/App/TopLevelRedirects/hooks/useCurrentRunRoute.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import {
RUN_ACTION_TYPE_PLAY,
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'

export function useCurrentRunRoute(currentRunId: string): string | null {
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
}
}
28 changes: 28 additions & 0 deletions app/src/App/TopLevelRedirects/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 TopLevelRedirects(): 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
}
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 { TopLevelRedirects } from '../TopLevelRedirects'

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('../TopLevelRedirects')

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(TopLevelRedirects).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(TopLevelRedirects).mockReturnValue(<div>MOCK_REDIRECTS</div>)
render('/')
screen.getByText('MOCK_REDIRECTS')
})
})
3 changes: 3 additions & 0 deletions app/src/App/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,6 @@ export const SLEEP_NEVER_MS = 604800000

// What is the maximum number of protocols one can pin? This many.
export const MAXIMUM_PINNED_PROTOCOLS = 8

// Used for top level redirects.
export const CURRENT_RUN_POLL = 5000
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
}
}

0 comments on commit 738becd

Please sign in to comment.