From f401e94565530074532ee7a922ace75ead75c7f0 Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Thu, 29 Aug 2024 15:00:36 -0400 Subject: [PATCH] fix(app): fix fallback polling not working on select notifications (#16162) Closes RQA-3110 and RQA-3108 --- ...ataReady.ts => useNotifyDataReady.test.ts} | 23 ++++++++++++------- .../recovery/useNotifyClientDataRecovery.ts | 9 ++++++-- .../useNotifyDeckConfigurationQuery.ts | 8 +++++-- .../useNotifyCurrentMaintenanceRun.ts | 8 +++++-- ...useNotifyAllCommandsAsPreSerializedList.ts | 8 +++++-- .../runs/useNotifyAllCommandsQuery.ts | 8 +++++-- 6 files changed, 46 insertions(+), 18 deletions(-) rename app/src/resources/__tests__/{useNotifyDataReady.ts => useNotifyDataReady.test.ts} (86%) diff --git a/app/src/resources/__tests__/useNotifyDataReady.ts b/app/src/resources/__tests__/useNotifyDataReady.test.ts similarity index 86% rename from app/src/resources/__tests__/useNotifyDataReady.ts rename to app/src/resources/__tests__/useNotifyDataReady.test.ts index 3fe27de0d77..7dd6ab5c83f 100644 --- a/app/src/resources/__tests__/useNotifyDataReady.ts +++ b/app/src/resources/__tests__/useNotifyDataReady.test.ts @@ -73,7 +73,8 @@ describe('useNotifyDataReady', () => { options: { ...MOCK_OPTIONS, forceHttpPolling: true }, } as any) ) - expect(result.current.shouldRefetch).toEqual(true) + expect(result.current.shouldRefetch).toEqual(false) + expect(result.current.isNotifyEnabled).toEqual(false) expect(appShellListener).not.toHaveBeenCalled() expect(mockDispatch).not.toHaveBeenCalled() }) @@ -85,7 +86,8 @@ describe('useNotifyDataReady', () => { options: { ...MOCK_OPTIONS, enabled: false }, } as any) ) - expect(result.current.shouldRefetch).toEqual(true) + expect(result.current.shouldRefetch).toEqual(false) + expect(result.current.isNotifyEnabled).toEqual(false) expect(appShellListener).not.toHaveBeenCalled() expect(mockDispatch).not.toHaveBeenCalled() }) @@ -97,12 +99,13 @@ describe('useNotifyDataReady', () => { options: { ...MOCK_OPTIONS, staleTime: Infinity }, } as any) ) - expect(result.current.shouldRefetch).toEqual(true) + expect(result.current.shouldRefetch).toEqual(false) + expect(result.current.isNotifyEnabled).toEqual(false) expect(appShellListener).not.toHaveBeenCalled() expect(mockDispatch).not.toHaveBeenCalled() }) - it('should set HTTP refetch to always if there is an error', () => { + it('should set isNotifyEnabled to false if there is an error', () => { vi.mocked(useHost).mockReturnValue({ hostname: null } as any) const errorSpy = vi.spyOn(console, 'error') errorSpy.mockImplementation(() => {}) @@ -115,10 +118,11 @@ describe('useNotifyDataReady', () => { } as any) ) - expect(result.current.shouldRefetch).toEqual(true) + expect(result.current.shouldRefetch).toEqual(false) + expect(result.current.isNotifyEnabled).toEqual(false) }) - it('should return set HTTP refetch to always and fire an analytics reporting event if the connection was refused', () => { + it('should return set isNotifyEnabled to false and fire an analytics reporting event if the connection was refused', () => { vi.mocked(appShellListener).mockImplementation(function ({ callback, }): any { @@ -134,7 +138,7 @@ describe('useNotifyDataReady', () => { ) expect(mockTrackEvent).toHaveBeenCalled() rerender() - expect(result.current.shouldRefetch).toEqual(true) + expect(result.current.isNotifyEnabled).toEqual(false) }) it('should trigger a single HTTP refetch if the refetch flag was returned', () => { @@ -153,6 +157,7 @@ describe('useNotifyDataReady', () => { ) rerender() expect(result.current.shouldRefetch).toEqual(true) + expect(result.current.isNotifyEnabled).toEqual(true) }) it('should trigger a single HTTP refetch if the unsubscribe flag was returned', () => { @@ -170,6 +175,7 @@ describe('useNotifyDataReady', () => { ) rerender() expect(result.current.shouldRefetch).toEqual(true) + expect(result.current.isNotifyEnabled).toEqual(true) }) it('should clean up the listener on dismount', () => { @@ -210,7 +216,8 @@ describe('useNotifyDataReady', () => { } as any) ) - expect(result.current.shouldRefetch).toEqual(true) + expect(result.current.shouldRefetch).toEqual(false) + expect(result.current.isNotifyEnabled).toEqual(false) expect(appShellListener).not.toHaveBeenCalled() }) }) diff --git a/app/src/resources/client_data/recovery/useNotifyClientDataRecovery.ts b/app/src/resources/client_data/recovery/useNotifyClientDataRecovery.ts index a93553fa3fa..2beb536a365 100644 --- a/app/src/resources/client_data/recovery/useNotifyClientDataRecovery.ts +++ b/app/src/resources/client_data/recovery/useNotifyClientDataRecovery.ts @@ -14,7 +14,11 @@ export function useNotifyClientDataRecovery( AxiosError > = {} ): UseQueryResult, AxiosError> { - const { notifyOnSettled, shouldRefetch } = useNotifyDataReady({ + const { + notifyOnSettled, + shouldRefetch, + isNotifyEnabled, + } = useNotifyDataReady({ topic: `robot-server/clientData/${KEYS.ERROR_RECOVERY}`, options, }) @@ -23,7 +27,8 @@ export function useNotifyClientDataRecovery( KEYS.ERROR_RECOVERY, { ...options, - enabled: options?.enabled !== false && shouldRefetch, + enabled: + options?.enabled !== false && (shouldRefetch || !isNotifyEnabled), onSettled: notifyOnSettled, } ) diff --git a/app/src/resources/deck_configuration/useNotifyDeckConfigurationQuery.ts b/app/src/resources/deck_configuration/useNotifyDeckConfigurationQuery.ts index a4e999bfbaa..2627f701060 100644 --- a/app/src/resources/deck_configuration/useNotifyDeckConfigurationQuery.ts +++ b/app/src/resources/deck_configuration/useNotifyDeckConfigurationQuery.ts @@ -9,14 +9,18 @@ import type { QueryOptionsWithPolling } from '../useNotifyDataReady' export function useNotifyDeckConfigurationQuery( options: QueryOptionsWithPolling = {} ): UseQueryResult { - const { notifyOnSettled, shouldRefetch } = useNotifyDataReady({ + const { + notifyOnSettled, + shouldRefetch, + isNotifyEnabled, + } = useNotifyDataReady({ topic: 'robot-server/deck_configuration', options, }) const httpQueryResult = useDeckConfigurationQuery({ ...options, - enabled: options?.enabled !== false && shouldRefetch, + enabled: options?.enabled !== false && (shouldRefetch || !isNotifyEnabled), onSettled: notifyOnSettled, }) diff --git a/app/src/resources/maintenance_runs/useNotifyCurrentMaintenanceRun.ts b/app/src/resources/maintenance_runs/useNotifyCurrentMaintenanceRun.ts index 1d19084ba59..669a7ae09e3 100644 --- a/app/src/resources/maintenance_runs/useNotifyCurrentMaintenanceRun.ts +++ b/app/src/resources/maintenance_runs/useNotifyCurrentMaintenanceRun.ts @@ -9,14 +9,18 @@ import type { QueryOptionsWithPolling } from '../useNotifyDataReady' export function useNotifyCurrentMaintenanceRun( options: QueryOptionsWithPolling = {} ): UseQueryResult | UseQueryResult { - const { notifyOnSettled, shouldRefetch } = useNotifyDataReady({ + const { + notifyOnSettled, + shouldRefetch, + isNotifyEnabled, + } = useNotifyDataReady({ topic: 'robot-server/maintenance_runs/current_run', options, }) const httpQueryResult = useCurrentMaintenanceRun({ ...options, - enabled: options?.enabled !== false && shouldRefetch, + enabled: options?.enabled !== false && (shouldRefetch || !isNotifyEnabled), onSettled: notifyOnSettled, }) diff --git a/app/src/resources/runs/useNotifyAllCommandsAsPreSerializedList.ts b/app/src/resources/runs/useNotifyAllCommandsAsPreSerializedList.ts index 25d45392185..dcf9a8b237d 100644 --- a/app/src/resources/runs/useNotifyAllCommandsAsPreSerializedList.ts +++ b/app/src/resources/runs/useNotifyAllCommandsAsPreSerializedList.ts @@ -12,14 +12,18 @@ export function useNotifyAllCommandsAsPreSerializedList( params?: GetCommandsParams | null, options: QueryOptionsWithPolling = {} ): UseQueryResult { - const { notifyOnSettled, shouldRefetch } = useNotifyDataReady({ + const { + notifyOnSettled, + shouldRefetch, + isNotifyEnabled, + } = useNotifyDataReady({ topic: `robot-server/runs/pre_serialized_commands/${runId}`, options, }) const httpResponse = useAllCommandsAsPreSerializedList(runId, params, { ...options, - enabled: options?.enabled !== false && shouldRefetch, + enabled: options?.enabled !== false && (shouldRefetch || !isNotifyEnabled), onSettled: notifyOnSettled, }) diff --git a/app/src/resources/runs/useNotifyAllCommandsQuery.ts b/app/src/resources/runs/useNotifyAllCommandsQuery.ts index ec6fc65a38f..214dd7aee16 100644 --- a/app/src/resources/runs/useNotifyAllCommandsQuery.ts +++ b/app/src/resources/runs/useNotifyAllCommandsQuery.ts @@ -17,14 +17,18 @@ export function useNotifyAllCommandsQuery( // running to succeeded, that may change the useAllCommandsQuery response, but it // will not necessarily change the command links. We might need an MQTT topic // covering "any change in `GET /runs/{id}/commands`". - const { notifyOnSettled, shouldRefetch } = useNotifyDataReady({ + const { + notifyOnSettled, + shouldRefetch, + isNotifyEnabled, + } = useNotifyDataReady({ topic: 'robot-server/runs/commands_links', options, }) const httpResponse = useAllCommandsQuery(runId, params, { ...options, - enabled: options?.enabled !== false && shouldRefetch, + enabled: options?.enabled !== false && (shouldRefetch || !isNotifyEnabled), onSettled: notifyOnSettled, })