fix(app): fix fallback polling not working on select notifications (#…

…16162) Closes RQA-3110 and RQA-3108
Opentrons · Aug 29, 2024 · f401e94 · f401e94
1 parent 7459249
commit f401e94
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 18 deletions.
diff --git a/...resources/__tests__/useNotifyDataReady.ts → ...rces/__tests__/useNotifyDataReady.test.ts b/...resources/__tests__/useNotifyDataReady.ts → ...rces/__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
@@ -14,7 +14,11 @@ export function useNotifyClientDataRecovery(
     AxiosError
   > = {}
 ): UseQueryResult<ClientDataResponse<ClientDataRecovery>, 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
@@ -9,14 +9,18 @@ import type { QueryOptionsWithPolling } from '../useNotifyDataReady'
 export function useNotifyDeckConfigurationQuery(
   options: QueryOptionsWithPolling<DeckConfiguration, unknown> = {}
 ): UseQueryResult<DeckConfiguration> {
-  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
@@ -9,14 +9,18 @@ import type { QueryOptionsWithPolling } from '../useNotifyDataReady'
 export function useNotifyCurrentMaintenanceRun(
   options: QueryOptionsWithPolling<MaintenanceRun, Error> = {}
 ): UseQueryResult<MaintenanceRun> | UseQueryResult<MaintenanceRun, Error> {
-  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
@@ -12,14 +12,18 @@ export function useNotifyAllCommandsAsPreSerializedList(
   params?: GetCommandsParams | null,
   options: QueryOptionsWithPolling<CommandsData, AxiosError> = {}
 ): UseQueryResult<CommandsData, AxiosError> {
-  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
@@ -17,14 +17,18 @@ export function useNotifyAllCommandsQuery<TError = Error>(
   // 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,
   })