fix(connection-status): responsiveness to online events [LIBS-497] (#…

…1348)

* fix(connection-status): supported versions

* fix(connection-status): ping on online event, but throttled

* docs: fix typos

* fix: clear throttled function on unmount

* test: ping on online events

* chore: fix comment

docs/advanced/offline/useDhis2ConnectionStatus.md

@@ -45,4 +45,4 @@ During periods when there’s no network traffic from the app, “pings” will
 
 ### Supported versions
 
-The pings are only sent for server versions that support them, meaning patch versions 2.40.0, 2.39.2, 2.38.3, and 2.37.10. and after. For unsupported versions, the hook will still use the incidental network traffic to determind a connections status value.
+The pings are only sent for server versions that support them, meaning patch versions 2.40.0, 2.39.2, 2.38.4, and 2.37.10 and after. For unsupported versions, the hook will still use the incidental network traffic to determine a connection status value.

services/offline/src/lib/dhis2-connection-status/dhis2-connection-status.test.tsx

@@ -559,6 +559,33 @@ describe('it pings when an offline event is detected', () => {
     })
 })
 
+describe('it pings when an online event is detected', () => {
+    test('if the app is focused, it pings immediately', () => {
+        renderHook(() => useDhis2ConnectionStatus(), { wrapper })
+
+        window.dispatchEvent(new Event('offline'))
+        expect(mockPing).toHaveBeenCalledTimes(1)
+
+        window.dispatchEvent(new Event('online'))
+        expect(mockPing).toHaveBeenCalledTimes(2)
+    })
+
+    test('pings are throttled', () => {
+        renderHook(() => useDhis2ConnectionStatus(), { wrapper })
+
+        window.dispatchEvent(new Event('offline'))
+        window.dispatchEvent(new Event('online'))
+        window.dispatchEvent(new Event('offline'))
+        expect(mockPing).toHaveBeenCalledTimes(3)
+
+        window.dispatchEvent(new Event('online'))
+        // Another ping should NOT be sent immediately after this latest
+        // online event
+        expect(mockPing).toHaveBeenCalledTimes(3)
+        // (Not testing throttle time here because further pings may interfere)
+    })
+})
+
 describe('lastConnected status', () => {
     test('it sets lastConnected in localStorage when it becomes disconnected', async () => {
         const { result } = renderHook(() => useDhis2ConnectionStatus(), {

services/offline/src/lib/dhis2-connection-status/dhis2-connection-status.tsx

@@ -1,4 +1,5 @@
 import { useConfig } from '@dhis2/app-service-config'
+import { throttle } from 'lodash'
 import PropTypes from 'prop-types'
 import React, {
     useCallback,
@@ -152,24 +153,30 @@ export const Dhis2ConnectionStatusProvider = ({
 
         const handleBlur = () => smartInterval.pause()
         const handleFocus = () => smartInterval.resume()
-        // On offline event, ping immediately to test server connection.
-        // Only do this when going offline -- it's theoretically no-cost
-        // for both online and offline servers. Pinging when going online
-        // can be costly for clients connecting over the internet to online
-        // servers.
+        // Pinging when going offline should be low/no-cost in both online and
+        // local servers
         const handleOffline = () => smartInterval.invokeCallbackImmediately()
+        // Pinging when going online has a cost but improves responsiveness of
+        // the connection status -- only do it once every 15 seconds at most
+        const handleOnline = throttle(
+            () => smartInterval.invokeCallbackImmediately(),
+            15000
+        )
 
         window.addEventListener('blur', handleBlur)
         window.addEventListener('focus', handleFocus)
         window.addEventListener('offline', handleOffline)
+        window.addEventListener('online', handleOnline)
 
         return () => {
             window.removeEventListener('blur', handleBlur)
             window.removeEventListener('focus', handleFocus)
             window.removeEventListener('offline', handleOffline)
+            window.removeEventListener('online', handleOnline)
 
-            // clean up smart interval
+            // clean up smart interval and throttled function
             smartInterval.clear()
+            handleOnline.cancel()
         }
     }, [pingAndHandleStatus, serverVersion])
 

services/offline/src/lib/dhis2-connection-status/is-ping-available.test.ts

@@ -4,7 +4,7 @@ import { isPingAvailable } from './is-ping-available'
 const fixedVersions: Version[] = [
     { full: 'unimportant', major: 2, minor: 40, patch: 0 },
     { full: 'unimportant', major: 2, minor: 39, patch: 2 },
-    { full: 'unimportant', major: 2, minor: 38, patch: 3 },
+    { full: 'unimportant', major: 2, minor: 38, patch: 4 },
     { full: 'unimportant', major: 2, minor: 37, patch: 10 },
     { full: 'unimportant', major: 2, minor: 40, tag: 'SNAPSHOT' },
     { full: 'unimportant', major: 2, minor: 3291, patch: 0 },

services/offline/src/lib/dhis2-connection-status/is-ping-available.ts

@@ -4,7 +4,7 @@ import { Version } from '@dhis2/app-service-config'
  * Checks the server version to see if the /api/ping endpoint is available for
  * this version.
  *
- * The endpoint was released for versions 2.37.10, 2.38.3, 2.39.2, and 2.40.0
+ * The endpoint was released for versions 2.37.10, 2.38.4, 2.39.2, and 2.40.0
  * (see DHIS2-14531). All versions below that are considered unsupported
  *
  * If the patchVersion is undefined, it's assumed to be a dev server that's
@@ -22,7 +22,7 @@ export function isPingAvailable(serverVersion: Version) {
         case 39:
             return patch === undefined || patch >= 2
         case 38:
-            return patch === undefined || patch >= 3
+            return patch === undefined || patch >= 4
         case 37:
             return patch === undefined || patch >= 10
         default: