Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
mjhuff committed Sep 3, 2024
1 parent 1c4126c commit ce30d89
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 29 deletions.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('useCurrentRunRoute', () => {

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

Expand All @@ -41,7 +41,11 @@ describe('useCurrentRunRoute', () => {
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: [] },
data: {
id: MOCK_RUN_ID,
status: RUN_STATUS_SUCCEEDED,
startedAt: '123',
},
},
isFetching: false,
} as any)
Expand All @@ -57,7 +61,7 @@ describe('useCurrentRunRoute', () => {
data: {
id: MOCK_RUN_ID,
status: RUN_STATUS_STOPPED,
actions: [{ actionType: RUN_ACTION_TYPE_PLAY }],
startedAt: '123',
},
},
isFetching: false,
Expand All @@ -71,7 +75,7 @@ describe('useCurrentRunRoute', () => {
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: [] },
data: { id: MOCK_RUN_ID, status: RUN_STATUS_FAILED, startedAt: '123' },
},
isFetching: false,
} as any)
Expand All @@ -83,7 +87,9 @@ describe('useCurrentRunRoute', () => {

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: [] } },
data: {
data: { id: MOCK_RUN_ID, status: RUN_STATUS_IDLE, startedAt: null },
},
isFetching: false,
} as any)

Expand All @@ -98,7 +104,7 @@ describe('useCurrentRunRoute', () => {
data: {
id: MOCK_RUN_ID,
status: RUN_STATUS_BLOCKED_BY_OPEN_DOOR,
actions: [],
startedAt: null,
},
},
isFetching: false,
Expand All @@ -115,7 +121,7 @@ describe('useCurrentRunRoute', () => {
data: {
id: MOCK_RUN_ID,
status: RUN_STATUS_BLOCKED_BY_OPEN_DOOR,
actions: [{ actionType: RUN_ACTION_TYPE_PLAY }],
startedAt: '123',
},
},
isFetching: false,
Expand All @@ -129,7 +135,7 @@ describe('useCurrentRunRoute', () => {
it('returns null for cancelled run before starting', () => {
vi.mocked(useNotifyRunQuery).mockReturnValue({
data: {
data: { id: MOCK_RUN_ID, status: RUN_STATUS_STOPPED, actions: [] },
data: { id: MOCK_RUN_ID, status: RUN_STATUS_STOPPED, startedAt: null },
},
isFetching: false,
} as any)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {
RUN_ACTION_TYPE_PLAY,
RUN_STATUS_BLOCKED_BY_OPEN_DOOR,
RUN_STATUS_FAILED,
RUN_STATUS_IDLE,
Expand All @@ -10,27 +9,20 @@ import {
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
const runActions = runRecord?.data.actions
if (
runRecord == null ||
runStatus == null ||
runActions == null ||
isFetching
) {

if (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 (
} else if (
runStatus === RUN_STATUS_SUCCEEDED ||
(runStatus === RUN_STATUS_STOPPED && hasRunStarted) ||
runStatus === RUN_STATUS_FAILED
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { useCurrentRunId } from '../../resources/runs'
import { CURRENT_RUN_POLL } from './constants'
import { useCurrentRunRoute } from './hooks'

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

return currentRunId != null ? (
Expand Down
4 changes: 2 additions & 2 deletions app/src/App/OnDeviceDisplayApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import { getOnDeviceDisplaySettings, updateConfigValue } from '../redux/config'
import { updateBrightness } from '../redux/shell'
import { SLEEP_NEVER_MS } from './constants'
import { useProtocolReceiptToast, useSoftwareUpdatePoll } from './hooks'
import { TopLevelRedirects } from './TopLevelRedirects'
import { ODDTopLevelRedirects } from './ODDTopLevelRedirects'

import { OnDeviceDisplayAppFallback } from './OnDeviceDisplayAppFallback'

Expand Down Expand Up @@ -198,7 +198,7 @@ export const OnDeviceDisplayApp = (): JSX.Element => {
</>
)}
</Box>
<TopLevelRedirects />
<ODDTopLevelRedirects />
</ErrorBoundary>
</OnDeviceLocalizationProvider>
</InitialLoadingScreen>
Expand Down
6 changes: 3 additions & 3 deletions app/src/App/__tests__/OnDeviceDisplayApp.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { getLocalRobot } from '../../redux/discovery'
import { mockConnectedRobot } from '../../redux/discovery/__fixtures__'
import { useProtocolReceiptToast } from '../hooks'
import { useNotifyCurrentMaintenanceRun } from '../../resources/maintenance_runs'
import { TopLevelRedirects } from '../TopLevelRedirects'
import { ODDTopLevelRedirects } from '../ODDTopLevelRedirects'

import type { UseQueryResult } from 'react-query'
import type { RobotSettingsResponse } from '@opentrons/api-client'
Expand Down Expand Up @@ -90,7 +90,7 @@ describe('OnDeviceDisplayApp', () => {
beforeEach(() => {
vi.mocked(getOnDeviceDisplaySettings).mockReturnValue(mockSettings as any)
vi.mocked(getIsShellReady).mockReturnValue(true)
vi.mocked(TopLevelRedirects).mockReturnValue(null)
vi.mocked(ODDTopLevelRedirects).mockReturnValue(null)
vi.mocked(getLocalRobot).mockReturnValue(mockConnectedRobot)
vi.mocked(useNotifyCurrentMaintenanceRun).mockReturnValue({
data: {
Expand Down Expand Up @@ -190,7 +190,7 @@ describe('OnDeviceDisplayApp', () => {
expect(vi.mocked(useProtocolReceiptToast)).toHaveBeenCalled()
})
it('renders TopLevelRedirects when it should conditionally render', () => {
vi.mocked(TopLevelRedirects).mockReturnValue(<div>MOCK_REDIRECTS</div>)
vi.mocked(ODDTopLevelRedirects).mockReturnValue(<div>MOCK_REDIRECTS</div>)
render('/')
screen.getByText('MOCK_REDIRECTS')
})
Expand Down

0 comments on commit ce30d89

Please sign in to comment.