diff --git a/app/src/organisms/DropTipWizard/__tests__/TipsAttachedModal.test.tsx b/app/src/organisms/DropTipWizard/__tests__/TipsAttachedModal.test.tsx index 34540b1c516..dfd82df19c0 100644 --- a/app/src/organisms/DropTipWizard/__tests__/TipsAttachedModal.test.tsx +++ b/app/src/organisms/DropTipWizard/__tests__/TipsAttachedModal.test.tsx @@ -10,13 +10,12 @@ import { handleTipsAttachedModal } from '../TipsAttachedModal' import { LEFT } from '@opentrons/shared-data' import { mockPipetteInfo } from '../../../redux/pipettes/__fixtures__' import { ROBOT_MODEL_OT3 } from '../../../redux/discovery' -import { useNotifyCurrentMaintenanceRun } from '../../../resources/maintenance_runs' +import { useCloseCurrentRun } from '../../ProtocolUpload/hooks' import type { PipetteModelSpecs } from '@opentrons/shared-data' import type { HostConfig } from '@opentrons/api-client' -vi.mock('../../../resources/maintenance_runs') -vi.mock('../../../resources/useNotifyService') +vi.mock('../../ProtocolUpload/hooks') const MOCK_ACTUAL_PIPETTE = { ...mockPipetteInfo.pipetteSpecs, @@ -53,12 +52,8 @@ const render = (pipetteSpecs: PipetteModelSpecs) => { describe('TipsAttachedModal', () => { beforeEach(() => { - vi.mocked(useNotifyCurrentMaintenanceRun).mockReturnValue({ - data: { - data: { - id: 'test', - }, - }, + vi.mocked(useCloseCurrentRun).mockReturnValue({ + closeCurrentRun: vi.fn(), } as any) }) diff --git a/app/src/resources/__tests__/useNotifyService.test.ts b/app/src/resources/__tests__/useNotifyService.test.ts index f2bf6bb516f..b11db79ee43 100644 --- a/app/src/resources/__tests__/useNotifyService.test.ts +++ b/app/src/resources/__tests__/useNotifyService.test.ts @@ -34,7 +34,6 @@ describe('useNotifyService', () => { beforeEach(() => { mockDispatch = vi.fn() - mockHTTPRefetch = vi.fn() mockTrackEvent = vi.fn() vi.mocked(useTrackEvent).mockReturnValue(mockTrackEvent) vi.mocked(useDispatch).mockReturnValue(mockDispatch) @@ -48,14 +47,13 @@ describe('useNotifyService', () => { }) it('should trigger an HTTP refetch and subscribe action on a successful initial mount', () => { - renderHook(() => + const { result } = renderHook(() => useNotifyService({ topic: MOCK_TOPIC, - setRefetch: mockHTTPRefetch, options: MOCK_OPTIONS, } as any) ) - expect(mockHTTPRefetch).toHaveBeenCalledWith('once') + expect(result.current.isNotifyEnabled).toEqual(true) expect(mockDispatch).toHaveBeenCalledWith( notifySubscribeAction(MOCK_HOST_CONFIG.hostname, MOCK_TOPIC) ) @@ -63,40 +61,37 @@ describe('useNotifyService', () => { }) it('should not subscribe to notifications if forceHttpPolling is true', () => { - renderHook(() => + const { result } = renderHook(() => useNotifyService({ topic: MOCK_TOPIC, - setRefetch: mockHTTPRefetch, options: { ...MOCK_OPTIONS, forceHttpPolling: true }, } as any) ) - expect(mockHTTPRefetch).toHaveBeenCalled() + expect(result.current.isNotifyEnabled).toEqual(true) expect(appShellListener).not.toHaveBeenCalled() expect(mockDispatch).not.toHaveBeenCalled() }) it('should not subscribe to notifications if enabled is false', () => { - renderHook(() => + const { result } = renderHook(() => useNotifyService({ topic: MOCK_TOPIC, - setRefetch: mockHTTPRefetch, options: { ...MOCK_OPTIONS, enabled: false }, } as any) ) - expect(mockHTTPRefetch).toHaveBeenCalled() + expect(result.current.isNotifyEnabled).toEqual(true) expect(appShellListener).not.toHaveBeenCalled() expect(mockDispatch).not.toHaveBeenCalled() }) it('should not subscribe to notifications if staleTime is Infinity', () => { - renderHook(() => + const { result } = renderHook(() => useNotifyService({ topic: MOCK_TOPIC, - setRefetch: mockHTTPRefetch, options: { ...MOCK_OPTIONS, staleTime: Infinity }, } as any) ) - expect(mockHTTPRefetch).toHaveBeenCalled() + expect(result.current.isNotifyEnabled).toEqual(true) expect(appShellListener).not.toHaveBeenCalled() expect(mockDispatch).not.toHaveBeenCalled() }) @@ -106,14 +101,15 @@ describe('useNotifyService', () => { const errorSpy = vi.spyOn(console, 'error') errorSpy.mockImplementation(() => {}) - renderHook(() => + const { result } = renderHook(() => useNotifyService({ topic: MOCK_TOPIC, setRefetch: mockHTTPRefetch, options: MOCK_OPTIONS, } as any) ) - expect(mockHTTPRefetch).toHaveBeenCalledWith('always') + + expect(result.current.isNotifyEnabled).toEqual(true) }) it('should return set HTTP refetch to always and fire an analytics reporting event if the connection was refused', () => { @@ -123,7 +119,7 @@ describe('useNotifyService', () => { // eslint-disable-next-line n/no-callback-literal callback('ECONNREFUSED') }) - const { rerender } = renderHook(() => + const { rerender, result } = renderHook(() => useNotifyService({ topic: MOCK_TOPIC, setRefetch: mockHTTPRefetch, @@ -132,7 +128,7 @@ describe('useNotifyService', () => { ) expect(mockTrackEvent).toHaveBeenCalled() rerender() - expect(mockHTTPRefetch).toHaveBeenCalledWith('always') + expect(result.current.isNotifyEnabled).toEqual(true) }) it('should trigger a single HTTP refetch if the refetch flag was returned', () => { @@ -142,7 +138,7 @@ describe('useNotifyService', () => { // eslint-disable-next-line n/no-callback-literal callback({ refetch: true }) }) - const { rerender } = renderHook(() => + const { rerender, result } = renderHook(() => useNotifyService({ topic: MOCK_TOPIC, setRefetch: mockHTTPRefetch, @@ -150,7 +146,7 @@ describe('useNotifyService', () => { } as any) ) rerender() - expect(mockHTTPRefetch).toHaveBeenCalledWith('once') + expect(result.current.isNotifyEnabled).toEqual(true) }) it('should trigger a single HTTP refetch if the unsubscribe flag was returned', () => { @@ -160,22 +156,20 @@ describe('useNotifyService', () => { // eslint-disable-next-line n/no-callback-literal callback({ unsubscribe: true }) }) - const { rerender } = renderHook(() => + const { rerender, result } = renderHook(() => useNotifyService({ topic: MOCK_TOPIC, - setRefetch: mockHTTPRefetch, options: MOCK_OPTIONS, } as any) ) rerender() - expect(mockHTTPRefetch).toHaveBeenCalledWith('once') + expect(result.current.isNotifyEnabled).toEqual(true) }) it('should clean up the listener on dismount', () => { const { unmount } = renderHook(() => useNotifyService({ topic: MOCK_TOPIC, - setRefetch: mockHTTPRefetch, options: MOCK_OPTIONS, }) ) @@ -188,7 +182,6 @@ describe('useNotifyService', () => { useNotifyService({ hostOverride: MOCK_HOST_CONFIG, topic: MOCK_TOPIC, - setRefetch: mockHTTPRefetch, options: MOCK_OPTIONS, }) ) diff --git a/app/src/resources/deck_configuration/useNotifyDeckConfigurationQuery.ts b/app/src/resources/deck_configuration/useNotifyDeckConfigurationQuery.ts index 3ccfd9feca5..827350bd47e 100644 --- a/app/src/resources/deck_configuration/useNotifyDeckConfigurationQuery.ts +++ b/app/src/resources/deck_configuration/useNotifyDeckConfigurationQuery.ts @@ -1,31 +1,23 @@ -import * as React from 'react' - import { useDeckConfigurationQuery } from '@opentrons/react-api-client' import { useNotifyService } from '../useNotifyService' import type { UseQueryResult } from 'react-query' import type { DeckConfiguration } from '@opentrons/shared-data' -import type { - QueryOptionsWithPolling, - HTTPRefetchFrequency, -} from '../useNotifyService' +import type { QueryOptionsWithPolling } from '../useNotifyService' export function useNotifyDeckConfigurationQuery( options: QueryOptionsWithPolling = {} ): UseQueryResult { - const [refetch, setRefetch] = React.useState(null) - - useNotifyService({ + const { notifyOnSettled, isNotifyEnabled } = useNotifyService({ topic: 'robot-server/deck_configuration', - setRefetch, options, }) const httpQueryResult = useDeckConfigurationQuery({ ...options, - enabled: options?.enabled !== false && refetch != null, - onSettled: refetch === 'once' ? () => setRefetch(null) : () => null, + enabled: options?.enabled !== false && isNotifyEnabled, + onSettled: notifyOnSettled, }) return httpQueryResult diff --git a/app/src/resources/maintenance_runs/useNotifyCurrentMaintenanceRun.ts b/app/src/resources/maintenance_runs/useNotifyCurrentMaintenanceRun.ts index 28859afe393..1f9bbf2ff3b 100644 --- a/app/src/resources/maintenance_runs/useNotifyCurrentMaintenanceRun.ts +++ b/app/src/resources/maintenance_runs/useNotifyCurrentMaintenanceRun.ts @@ -1,31 +1,23 @@ -import * as React from 'react' - import { useCurrentMaintenanceRun } from '@opentrons/react-api-client' import { useNotifyService } from '../useNotifyService' import type { UseQueryResult } from 'react-query' import type { MaintenanceRun } from '@opentrons/api-client' -import type { - QueryOptionsWithPolling, - HTTPRefetchFrequency, -} from '../useNotifyService' +import type { QueryOptionsWithPolling } from '../useNotifyService' export function useNotifyCurrentMaintenanceRun( options: QueryOptionsWithPolling = {} ): UseQueryResult | UseQueryResult { - const [refetch, setRefetch] = React.useState(null) - - useNotifyService({ + const { notifyOnSettled, isNotifyEnabled } = useNotifyService({ topic: 'robot-server/maintenance_runs/current_run', - setRefetch, options, }) const httpQueryResult = useCurrentMaintenanceRun({ ...options, - enabled: options?.enabled !== false && refetch != null, - onSettled: refetch === 'once' ? () => setRefetch(null) : () => null, + enabled: options?.enabled !== false && isNotifyEnabled, + onSettled: notifyOnSettled, }) return httpQueryResult diff --git a/app/src/resources/runs/useNotifyAllCommandsAsPreSerializedList.ts b/app/src/resources/runs/useNotifyAllCommandsAsPreSerializedList.ts index 1410c23cef0..67922a9e3ec 100644 --- a/app/src/resources/runs/useNotifyAllCommandsAsPreSerializedList.ts +++ b/app/src/resources/runs/useNotifyAllCommandsAsPreSerializedList.ts @@ -1,5 +1,3 @@ -import * as React from 'react' - import { useAllCommandsAsPreSerializedList } from '@opentrons/react-api-client' import { useNotifyService } from '../useNotifyService' @@ -7,28 +5,22 @@ import { useNotifyService } from '../useNotifyService' import type { UseQueryResult } from 'react-query' import type { AxiosError } from 'axios' import type { CommandsData, GetCommandsParams } from '@opentrons/api-client' -import type { - QueryOptionsWithPolling, - HTTPRefetchFrequency, -} from '../useNotifyService' +import type { QueryOptionsWithPolling } from '../useNotifyService' export function useNotifyAllCommandsAsPreSerializedList( runId: string | null, params?: GetCommandsParams | null, options: QueryOptionsWithPolling = {} ): UseQueryResult { - const [refetch, setRefetch] = React.useState(null) - - useNotifyService({ + const { notifyOnSettled, isNotifyEnabled } = useNotifyService({ topic: `robot-server/runs/pre_serialized_commands/${runId}`, - setRefetch, options, }) const httpResponse = useAllCommandsAsPreSerializedList(runId, params, { ...options, - enabled: options?.enabled !== false && refetch != null, - onSettled: refetch === 'once' ? () => setRefetch(null) : () => null, + enabled: options?.enabled !== false && isNotifyEnabled, + onSettled: notifyOnSettled, }) return httpResponse diff --git a/app/src/resources/runs/useNotifyAllRunsQuery.ts b/app/src/resources/runs/useNotifyAllRunsQuery.ts index 1ae93ffc713..ab340039cfd 100644 --- a/app/src/resources/runs/useNotifyAllRunsQuery.ts +++ b/app/src/resources/runs/useNotifyAllRunsQuery.ts @@ -1,5 +1,3 @@ -import * as React from 'react' - import { useAllRunsQuery } from '@opentrons/react-api-client' import { useNotifyService } from '../useNotifyService' @@ -8,21 +6,15 @@ import type { UseQueryResult } from 'react-query' import type { AxiosError } from 'axios' import type { HostConfig, GetRunsParams, Runs } from '@opentrons/api-client' import type { UseAllRunsQueryOptions } from '@opentrons/react-api-client/src/runs/useAllRunsQuery' -import type { - QueryOptionsWithPolling, - HTTPRefetchFrequency, -} from '../useNotifyService' +import type { QueryOptionsWithPolling } from '../useNotifyService' export function useNotifyAllRunsQuery( params: GetRunsParams = {}, options: QueryOptionsWithPolling = {}, hostOverride?: HostConfig | null ): UseQueryResult { - const [refetch, setRefetch] = React.useState(null) - - useNotifyService({ + const { notifyOnSettled, isNotifyEnabled } = useNotifyService({ topic: 'robot-server/runs', - setRefetch, options, hostOverride, }) @@ -31,8 +23,8 @@ export function useNotifyAllRunsQuery( params, { ...(options as UseAllRunsQueryOptions), - enabled: options?.enabled !== false && refetch != null, - onSettled: refetch === 'once' ? () => setRefetch(null) : () => null, + enabled: options?.enabled !== false && isNotifyEnabled, + onSettled: notifyOnSettled, }, hostOverride ) diff --git a/app/src/resources/runs/useNotifyLastRunCommand.ts b/app/src/resources/runs/useNotifyLastRunCommand.ts index 14c68944e43..b7c2289ffc8 100644 --- a/app/src/resources/runs/useNotifyLastRunCommand.ts +++ b/app/src/resources/runs/useNotifyLastRunCommand.ts @@ -1,30 +1,22 @@ -import * as React from 'react' - import { useNotifyService } from '../useNotifyService' import { useLastRunCommand } from '../../organisms/Devices/hooks/useLastRunCommand' import type { CommandsData, RunCommandSummary } from '@opentrons/api-client' -import type { - QueryOptionsWithPolling, - HTTPRefetchFrequency, -} from '../useNotifyService' +import type { QueryOptionsWithPolling } from '../useNotifyService' export function useNotifyLastRunCommand( runId: string, options: QueryOptionsWithPolling = {} ): RunCommandSummary | null { - const [refetch, setRefetch] = React.useState(null) - - useNotifyService({ + const { notifyOnSettled, isNotifyEnabled } = useNotifyService({ topic: 'robot-server/runs/current_command', - setRefetch, options, }) const httpResponse = useLastRunCommand(runId, { ...options, - enabled: options?.enabled !== false && refetch != null, - onSettled: refetch === 'once' ? () => setRefetch(null) : () => null, + enabled: options?.enabled !== false && isNotifyEnabled, + onSettled: notifyOnSettled, }) return httpResponse diff --git a/app/src/resources/runs/useNotifyRunQuery.ts b/app/src/resources/runs/useNotifyRunQuery.ts index 2ca72687341..ae6b1b1bdcc 100644 --- a/app/src/resources/runs/useNotifyRunQuery.ts +++ b/app/src/resources/runs/useNotifyRunQuery.ts @@ -1,35 +1,27 @@ -import * as React from 'react' - import { useRunQuery } from '@opentrons/react-api-client' import { useNotifyService } from '../useNotifyService' import type { UseQueryResult } from 'react-query' import type { Run } from '@opentrons/api-client' -import type { - QueryOptionsWithPolling, - HTTPRefetchFrequency, -} from '../useNotifyService' +import type { QueryOptionsWithPolling } from '../useNotifyService' import type { NotifyTopic } from '../../redux/shell/types' export function useNotifyRunQuery( runId: string | null, options: QueryOptionsWithPolling = {} ): UseQueryResult { - const [refetch, setRefetch] = React.useState(null) - const isEnabled = options.enabled !== false && runId != null - useNotifyService({ + const { notifyOnSettled, isNotifyEnabled } = useNotifyService({ topic: `robot-server/runs/${runId}` as NotifyTopic, - setRefetch, options: { ...options, enabled: options.enabled != null && runId != null }, }) const httpResponse = useRunQuery(runId, { ...options, - enabled: isEnabled && refetch != null, - onSettled: refetch === 'once' ? () => setRefetch(null) : () => null, + enabled: isEnabled && isNotifyEnabled, + onSettled: notifyOnSettled, }) return httpResponse diff --git a/app/src/resources/useNotifyService.ts b/app/src/resources/useNotifyService.ts index d8422ba786f..1cf1d1dc3fa 100644 --- a/app/src/resources/useNotifyService.ts +++ b/app/src/resources/useNotifyService.ts @@ -24,23 +24,28 @@ export interface QueryOptionsWithPolling interface UseNotifyServiceProps { topic: NotifyTopic - setRefetch: (refetch: HTTPRefetchFrequency) => void options: QueryOptionsWithPolling hostOverride?: HostConfig | null } +interface UseNotifyServiceResults { + notifyOnSettled: () => void + isNotifyEnabled: boolean +} + export function useNotifyService({ topic, - setRefetch, options, hostOverride, -}: UseNotifyServiceProps): void { +}: UseNotifyServiceProps): UseNotifyServiceResults { const dispatch = useDispatch() const hostFromProvider = useHost() const host = hostOverride ?? hostFromProvider const hostname = host?.hostname ?? null const doTrackEvent = useTrackEvent() const seenHostname = React.useRef(null) + const [refetch, setRefetch] = React.useState(null) + const { enabled, staleTime, forceHttpPolling } = options const shouldUseNotifications = @@ -76,7 +81,7 @@ export function useNotifyService({ } }, [topic, hostname, shouldUseNotifications]) - function onDataEvent(data: NotifyResponseData): void { + const onDataEvent = React.useCallback((data: NotifyResponseData): void => { if (data === 'ECONNFAILED' || data === 'ECONNREFUSED') { setRefetch('always') if (data === 'ECONNREFUSED') { @@ -88,5 +93,13 @@ export function useNotifyService({ } else if ('refetch' in data || 'unsubscribe' in data) { setRefetch('once') } - } + }, []) + + const notifyOnSettled = React.useCallback(() => { + if (refetch === 'once') { + setRefetch(null) + } + }, [refetch]) + + return { notifyOnSettled, isNotifyEnabled: refetch != null } } diff --git a/setup-vitest.ts b/setup-vitest.ts index eb30f021428..4488cc17bf6 100644 --- a/setup-vitest.ts +++ b/setup-vitest.ts @@ -7,7 +7,16 @@ vi.mock('electron-store') vi.mock('electron-updater') vi.mock('electron') vi.mock('./app/src/redux/shell/remote') -vi.mock('./app/src/resources/useNotifyService') +vi.mock('./app/src/resources/useNotifyService', async () => { + const actual = await vi.importActual('./app/src/resources/useNotifyService') + return { + ...actual, + useNotifyService: () => ({ + notifyOnSettled: vi.fn(), + isNotifyEnabled: true, + }), + } +}) process.env.OT_PD_VERSION = 'fake_PD_version' global._PKG_VERSION_ = 'test environment'