From 91a3d4d820e6a87b819334bd72709c1de1a777f5 Mon Sep 17 00:00:00 2001 From: Kai Vandivier <49666798+KaiVandivier@users.noreply.github.com> Date: Tue, 16 May 2023 11:29:41 +0200 Subject: [PATCH] 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 --- .../offline/useDhis2ConnectionStatus.md | 2 +- .../dhis2-connection-status.test.tsx | 27 +++++++++++++++++++ .../dhis2-connection-status.tsx | 19 ++++++++----- .../is-ping-available.test.ts | 2 +- .../is-ping-available.ts | 4 +-- 5 files changed, 44 insertions(+), 10 deletions(-) diff --git a/docs/advanced/offline/useDhis2ConnectionStatus.md b/docs/advanced/offline/useDhis2ConnectionStatus.md index 7ab98e51a..6d7ede27f 100644 --- a/docs/advanced/offline/useDhis2ConnectionStatus.md +++ b/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. diff --git a/services/offline/src/lib/dhis2-connection-status/dhis2-connection-status.test.tsx b/services/offline/src/lib/dhis2-connection-status/dhis2-connection-status.test.tsx index c14c33d51..71f44328e 100644 --- a/services/offline/src/lib/dhis2-connection-status/dhis2-connection-status.test.tsx +++ b/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(), { diff --git a/services/offline/src/lib/dhis2-connection-status/dhis2-connection-status.tsx b/services/offline/src/lib/dhis2-connection-status/dhis2-connection-status.tsx index 83b93f899..accf3ac69 100644 --- a/services/offline/src/lib/dhis2-connection-status/dhis2-connection-status.tsx +++ b/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]) diff --git a/services/offline/src/lib/dhis2-connection-status/is-ping-available.test.ts b/services/offline/src/lib/dhis2-connection-status/is-ping-available.test.ts index 095ba3965..74e1ea765 100644 --- a/services/offline/src/lib/dhis2-connection-status/is-ping-available.test.ts +++ b/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 }, diff --git a/services/offline/src/lib/dhis2-connection-status/is-ping-available.ts b/services/offline/src/lib/dhis2-connection-status/is-ping-available.ts index a8869ff25..387ccfa8b 100644 --- a/services/offline/src/lib/dhis2-connection-status/is-ping-available.ts +++ b/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: