From 690a5165e2506eb93333ec89382fd1fc8552baa5 Mon Sep 17 00:00:00 2001 From: Jack Greenlee Date: Fri, 13 Sep 2024 14:39:30 -0400 Subject: [PATCH 1/4] cleanup appstate hook, refreshTimeline on app resume, revamp clientstats TimelineContext When appstate resumes from background, refresh the timeline to avoid stale state useAppStateChange: Added stat reading and removed log statement (because the stat reading itself performs a log) Clean up the subscription on teardown (the return of useEffect) clientStats: Updated the stat keys to be typed strings, eliminating the need for statKeys. `'my_stat_key'` is less verbose than importing statKeys and calling`statKeys.MY_STAT_KEY`, and this approach utilizes Typescript. Updated existing clientstats readings and removed unused clientstats keys. Added new clientstats readings in useAppStateChange MultiLabelButtonGroup. Unified CLIENT_NAV_EVENT and CLIENT_TIME since they are fundamentally the same and only differ by whether the 'reading' field has a value or is null --- www/__tests__/clientStats.test.ts | 22 ++++------ www/__tests__/remoteNotifyHandler.test.ts | 8 ++-- www/js/Main.tsx | 13 +++++- www/js/TimelineContext.ts | 2 + www/js/control/ControlSyncHelper.tsx | 6 +-- www/js/diary/cards/TripCard.tsx | 2 + www/js/plugin/clientStats.ts | 42 +++++++------------ www/js/plugin/storage.ts | 6 +-- www/js/splash/notifScheduler.ts | 4 +- www/js/splash/remoteNotifyHandler.ts | 4 +- .../multilabel/MultiLabelButtonGroup.tsx | 9 +++- www/js/useAppStateChange.ts | 5 ++- 12 files changed, 62 insertions(+), 61 deletions(-) diff --git a/www/__tests__/clientStats.test.ts b/www/__tests__/clientStats.test.ts index a3a953582..632eafddf 100644 --- a/www/__tests__/clientStats.test.ts +++ b/www/__tests__/clientStats.test.ts @@ -1,11 +1,5 @@ import { mockBEMUserCache, mockDevice, mockGetAppVersion } from '../__mocks__/cordovaMocks'; -import { - addStatError, - addStatEvent, - addStatReading, - getAppVersion, - statKeys, -} from '../js/plugin/clientStats'; +import { addStatError, addStatReading, getAppVersion } from '../js/plugin/clientStats'; mockDevice(); // this mocks cordova-plugin-app-version, generating a "Mock App", version "1.2.3" @@ -21,10 +15,10 @@ it('gets the app version', async () => { it('stores a client stats reading', async () => { const reading = { a: 1, b: 2 }; - await addStatReading(statKeys.REMINDER_PREFS, reading); + await addStatReading('set_reminder_prefs', reading); const storedMessages = await db.getAllMessages('stats/client_time', false); expect(storedMessages).toContainEqual({ - name: statKeys.REMINDER_PREFS, + name: 'set_reminder_prefs', ts: expect.any(Number), reading, client_app_version: '1.2.3', @@ -33,10 +27,10 @@ it('stores a client stats reading', async () => { }); it('stores a client stats event', async () => { - await addStatEvent(statKeys.BUTTON_FORCE_SYNC); - const storedMessages = await db.getAllMessages('stats/client_nav_event', false); + await addStatReading('force_sync'); + const storedMessages = await db.getAllMessages('stats/client_time', false); expect(storedMessages).toContainEqual({ - name: statKeys.BUTTON_FORCE_SYNC, + name: 'force_sync', ts: expect.any(Number), reading: null, client_app_version: '1.2.3', @@ -46,10 +40,10 @@ it('stores a client stats event', async () => { it('stores a client stats error', async () => { const errorStr = 'test error'; - await addStatError(statKeys.MISSING_KEYS, errorStr); + await addStatError('missing_keys', errorStr); const storedMessages = await db.getAllMessages('stats/client_error', false); expect(storedMessages).toContainEqual({ - name: statKeys.MISSING_KEYS, + name: 'missing_keys', ts: expect.any(Number), reading: errorStr, client_app_version: '1.2.3', diff --git a/www/__tests__/remoteNotifyHandler.test.ts b/www/__tests__/remoteNotifyHandler.test.ts index 320877c6b..4d3761547 100644 --- a/www/__tests__/remoteNotifyHandler.test.ts +++ b/www/__tests__/remoteNotifyHandler.test.ts @@ -26,7 +26,7 @@ beforeEach(() => { it('does not adds a statEvent if not subscribed', async () => { publish(EVENTS.CLOUD_NOTIFICATION_EVENT, 'test data'); - const storedMessages = await db.getAllMessages('stats/client_nav_event', false); + const storedMessages = await db.getAllMessages('stats/client_time', false); expect(storedMessages).toEqual([]); }); @@ -35,11 +35,11 @@ it('adds a statEvent if subscribed', async () => { await new Promise((r) => setTimeout(r, 500)); //wait for subscription publish(EVENTS.CLOUD_NOTIFICATION_EVENT, 'test data'); await new Promise((r) => setTimeout(r, 500)); //wait for event handling - const storedMessages = await db.getAllMessages('stats/client_nav_event', false); + const storedMessages = await db.getAllMessages('stats/client_time', false); expect(storedMessages).toContainEqual({ - name: 'notification_open', + name: 'open_notification', ts: expect.any(Number), - reading: null, + reading: 'test data', client_app_version: '1.2.3', client_os_version: '14.0.0', }); diff --git a/www/js/Main.tsx b/www/js/Main.tsx index 650ed4044..cb232535d 100644 --- a/www/js/Main.tsx +++ b/www/js/Main.tsx @@ -1,7 +1,7 @@ /* Once onboarding is done, this is the main app content. Includes the bottom navigation bar and each of the tabs. */ -import React, { useEffect } from 'react'; +import React, { useCallback, useEffect } from 'react'; import { useContext, useMemo, useState } from 'react'; import { BottomNavigation, useTheme } from 'react-native-paper'; import { AppContext } from './App'; @@ -11,6 +11,7 @@ import LabelTab from './diary/LabelTab'; import MetricsTab from './metrics/MetricsTab'; import ProfileSettings from './control/ProfileSettings'; import TimelineContext, { useTimelineContext } from './TimelineContext'; +import { addStatReading } from './plugin/clientStats'; const defaultRoutes = (t) => [ { @@ -57,6 +58,14 @@ const Main = () => { return showMetrics ? defaultRoutes(t) : defaultRoutes(t).filter((r) => r.key != 'metrics'); }, [appConfig, t]); + const onIndexChange = useCallback( + (i: number) => { + addStatReading('nav_tab_change', routes[i].key); + setIndex(i); + }, + [routes], + ); + useEffect(() => { const { setShouldUpdateTimeline } = timelineContext; // update TimelineScrollList component only when the active tab is 'label' to fix leaflet map issue @@ -67,7 +76,7 @@ const Main = () => { { const { t } = useTranslation(); const appConfig = useAppConfig(); + useAppStateChange(() => refreshTimeline()); const [labelOptions, setLabelOptions] = useState(null); // timestamp range that has been processed by the pipeline on the server diff --git a/www/js/control/ControlSyncHelper.tsx b/www/js/control/ControlSyncHelper.tsx index bdb565f61..30ba7a87c 100644 --- a/www/js/control/ControlSyncHelper.tsx +++ b/www/js/control/ControlSyncHelper.tsx @@ -6,7 +6,7 @@ import { settingStyles } from './ProfileSettings'; import ActionMenu from '../components/ActionMenu'; import SettingRow from './SettingRow'; import { AlertManager } from '../components/AlertBar'; -import { addStatEvent, statKeys } from '../plugin/clientStats'; +import { addStatReading } from '../plugin/clientStats'; import { updateUser } from '../services/commHelper'; import { displayError, logDebug, logWarn } from '../plugin/logger'; import { DateTime } from 'luxon'; @@ -42,8 +42,8 @@ export const ForceSyncRow = ({ getState }) => { async function forceSync() { try { - let addedEvent = await addStatEvent(statKeys.BUTTON_FORCE_SYNC); - let sync = await forcePluginSync(); + await addStatReading('force_sync'); + await forcePluginSync(); /* * Change to sensorKey to "background/location" after fixing issues * with getLastSensorData and getLastMessages in the usercache diff --git a/www/js/diary/cards/TripCard.tsx b/www/js/diary/cards/TripCard.tsx index a309d63aa..23d7db224 100644 --- a/www/js/diary/cards/TripCard.tsx +++ b/www/js/diary/cards/TripCard.tsx @@ -25,6 +25,7 @@ import ModesIndicator from './ModesIndicator'; import { useGeojsonForTrip } from '../timelineHelper'; import { CompositeTrip } from '../../types/diaryTypes'; import { EnketoUserInputEntry } from '../../survey/enketo/enketoHelper'; +import { addStatReading } from '../../plugin/clientStats'; type Props = { trip: CompositeTrip; isFirstInList?: boolean }; const TripCard = ({ trip, isFirstInList }: Props) => { @@ -51,6 +52,7 @@ const TripCard = ({ trip, isFirstInList }: Props) => { function showDetail() { const tripId = trip._id.$oid; + addStatReading('view_trip_details', { tripId }); navigation.navigate('label.details', { tripId, flavoredTheme }); } diff --git a/www/js/plugin/clientStats.ts b/www/js/plugin/clientStats.ts index bdf4c1888..e106dd7cf 100644 --- a/www/js/plugin/clientStats.ts +++ b/www/js/plugin/clientStats.ts @@ -2,24 +2,17 @@ import { displayErrorMsg, logDebug } from './logger'; const CLIENT_TIME = 'stats/client_time'; const CLIENT_ERROR = 'stats/client_error'; -const CLIENT_NAV_EVENT = 'stats/client_nav_event'; -export const statKeys = { - STATE_CHANGED: 'state_changed', - BUTTON_FORCE_SYNC: 'button_sync_forced', - CHECKED_DIARY: 'checked_diary', - DIARY_TIME: 'diary_time', - METRICS_TIME: 'metrics_time', - CHECKED_INF_SCROLL: 'checked_inf_scroll', - INF_SCROLL_TIME: 'inf_scroll_time', - VERIFY_TRIP: 'verify_trip', - LABEL_TAB_SWITCH: 'label_tab_switch', - SELECT_LABEL: 'select_label', - EXPANDED_TRIP: 'expanded_trip', - NOTIFICATION_OPEN: 'notification_open', - REMINDER_PREFS: 'reminder_time_prefs', - MISSING_KEYS: 'missing_keys', -}; +type StatKey = + | 'app_state_change' + | 'nav_tab_change' + | 'view_trip_details' + | 'multilabel_open' + | 'multilabel_choose' + | 'set_reminder_prefs' + | 'force_sync' + | 'open_notification' + | 'missing_keys'; let appVersion; export function getAppVersion() { @@ -30,14 +23,15 @@ export function getAppVersion() { }); } -async function getStatsEvent(name: string, reading: any) { +async function getStatsEvent(name: StatKey, reading?: any) { const ts = Date.now() / 1000; const client_app_version = await getAppVersion(); const client_os_version = window['device'].version; + reading = reading || null; return { name, ts, reading, client_app_version, client_os_version }; } -export async function addStatReading(name: string, reading: any) { +export async function addStatReading(name: StatKey, reading?: any) { const db = window['cordova']?.plugins?.BEMUserCache; const event = await getStatsEvent(name, reading); logDebug('addStatReading: adding CLIENT_TIME event: ' + JSON.stringify(event)); @@ -45,15 +39,7 @@ export async function addStatReading(name: string, reading: any) { displayErrorMsg('addStatReading: db is not defined'); } -export async function addStatEvent(name: string) { - const db = window['cordova']?.plugins?.BEMUserCache; - const event = await getStatsEvent(name, null); - logDebug('addStatEvent: adding CLIENT_NAV_EVENT event: ' + JSON.stringify(event)); - if (db) return db.putMessage(CLIENT_NAV_EVENT, event); - displayErrorMsg('addStatEvent: db is not defined'); -} - -export async function addStatError(name: string, errorStr: string) { +export async function addStatError(name: StatKey, errorStr: string) { const db = window['cordova']?.plugins?.BEMUserCache; const event = await getStatsEvent(name, errorStr); logDebug('addStatError: adding CLIENT_ERROR event: ' + JSON.stringify(event)); diff --git a/www/js/plugin/storage.ts b/www/js/plugin/storage.ts index e22bb4669..51701730e 100644 --- a/www/js/plugin/storage.ts +++ b/www/js/plugin/storage.ts @@ -1,4 +1,4 @@ -import { addStatReading, statKeys } from './clientStats'; +import { addStatReading } from './clientStats'; import { logDebug, logWarn } from './logger'; function mungeValue(key, value) { @@ -162,7 +162,7 @@ export function storageSyncLocalAndNative() { logDebug('STORAGE_PLUGIN: Syncing all missing keys ' + allMissing); allMissing.forEach(getUnifiedValue); if (allMissing.length != 0) { - addStatReading(statKeys.MISSING_KEYS, { + addStatReading('missing_keys', { type: 'local_storage_mismatch', allMissingLength: allMissing.length, missingWebLength: missingWeb.length, @@ -178,7 +178,7 @@ export function storageSyncLocalAndNative() { (nativeKeys) => { logDebug('STORAGE_PLUGIN: For the record, all unique native keys are ' + nativeKeys); if (nativeKeys.length == 0) { - addStatReading(statKeys.MISSING_KEYS, { + addStatReading('missing_keys', { type: 'all_native', }).then(logDebug('Logged all missing native keys to client stats')); } diff --git a/www/js/splash/notifScheduler.ts b/www/js/splash/notifScheduler.ts index 10d20b18b..d882c5cb6 100644 --- a/www/js/splash/notifScheduler.ts +++ b/www/js/splash/notifScheduler.ts @@ -1,4 +1,4 @@ -import { addStatReading, statKeys } from '../plugin/clientStats'; +import { addStatReading } from '../plugin/clientStats'; import { getUser, updateUser } from '../services/commHelper'; import { displayErrorMsg, logDebug } from '../plugin/logger'; import { DateTime } from 'luxon'; @@ -286,7 +286,7 @@ export async function setReminderPrefs( // extract only the relevant fields from the prefs, // and add as a reading to client stats const { reminder_assignment, reminder_join_date, reminder_time_of_day } = prefs; - addStatReading(statKeys.REMINDER_PREFS, { + addStatReading('set_reminder_prefs', { reminder_assignment, reminder_join_date, reminder_time_of_day, diff --git a/www/js/splash/remoteNotifyHandler.ts b/www/js/splash/remoteNotifyHandler.ts index 098625ee2..513b3719e 100644 --- a/www/js/splash/remoteNotifyHandler.ts +++ b/www/js/splash/remoteNotifyHandler.ts @@ -11,7 +11,7 @@ * notification handling gets more complex, we should consider decoupling it as well. */ import { EVENTS, subscribe } from '../customEventHandler'; -import { addStatEvent, statKeys } from '../plugin/clientStats'; +import { addStatReading } from '../plugin/clientStats'; import { displayErrorMsg, logDebug } from '../plugin/logger'; const options = 'location=yes,clearcache=no,toolbar=yes,hideurlbar=yes'; @@ -31,7 +31,7 @@ const launchWebpage = (url) => window['cordova'].InAppBrowser.open(url, '_blank' */ function onCloudNotifEvent(event) { const data = event.detail; - addStatEvent(statKeys.NOTIFICATION_OPEN); + addStatReading('open_notification', data); logDebug('data = ' + JSON.stringify(data)); if ( data.additionalData && diff --git a/www/js/survey/multilabel/MultiLabelButtonGroup.tsx b/www/js/survey/multilabel/MultiLabelButtonGroup.tsx index 41257b5d0..5cc1de3d7 100644 --- a/www/js/survey/multilabel/MultiLabelButtonGroup.tsx +++ b/www/js/survey/multilabel/MultiLabelButtonGroup.tsx @@ -32,6 +32,7 @@ import useAppConfig from '../../useAppConfig'; import { MultilabelKey } from '../../types/labelTypes'; // import { updateUserCustomLabel } from '../../services/commHelper'; import { AppContext } from '../../App'; +import { addStatReading } from '../../plugin/clientStats'; const MultilabelButtonGroup = ({ trip, buttonsInline = false }) => { const { colors } = useTheme(); @@ -72,6 +73,11 @@ const MultilabelButtonGroup = ({ trip, buttonsInline = false }) => { } } + function openModalFor(inputType: MultilabelKey) { + addStatReading('multilabel_open', inputType); + setModalVisibleFor(inputType); + } + function dismiss() { setModalVisibleFor(null); setOtherLabel(null); @@ -123,6 +129,7 @@ const MultilabelButtonGroup = ({ trip, buttonsInline = false }) => { } Promise.all(storePromises).then(() => { logDebug('Successfully stored input data ' + JSON.stringify(inputsToStore)); + addStatReading('multilabel_choose', inputsToStore); dismiss(); addUserInputToEntry(trip._id.$oid, inputsToStore, 'label'); }); @@ -156,7 +163,7 @@ const MultilabelButtonGroup = ({ trip, buttonsInline = false }) => { fillColor={fillColor} borderColor={borderColor} textColor={textColor} - onPress={(e) => setModalVisibleFor(input.name)}> + onPress={(e) => openModalFor(input.name)}> {t(btnText)} diff --git a/www/js/useAppStateChange.ts b/www/js/useAppStateChange.ts index 9a77909a0..760cbd8fa 100644 --- a/www/js/useAppStateChange.ts +++ b/www/js/useAppStateChange.ts @@ -5,7 +5,7 @@ import { useEffect, useRef } from 'react'; import { AppState } from 'react-native'; -import { logDebug } from './plugin/logger'; +import { addStatReading } from './plugin/clientStats'; const useAppStateChange = (onResume) => { const appState = useRef(AppState.currentState); @@ -17,8 +17,9 @@ const useAppStateChange = (onResume) => { } appState.current = nextAppState; - logDebug('new AppState: ' + appState.current); + addStatReading('app_state_change', appState.current); }); + return () => subscription.remove(); }, []); return {}; From f8fcf108204ef6aae8a546bcec75cacfd0533a3d Mon Sep 17 00:00:00 2001 From: Jack Greenlee Date: Fri, 13 Sep 2024 14:46:26 -0400 Subject: [PATCH 2/4] add `ui_error`s to clientStats Whenever the user sees an error, it will be recorded in clientStats as a `stats/client_error` --- www/__tests__/clientStats.test.ts | 8 ++++++-- www/js/plugin/clientStats.ts | 7 ++++--- www/js/plugin/logger.ts | 3 +++ 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/www/__tests__/clientStats.test.ts b/www/__tests__/clientStats.test.ts index 632eafddf..2795ff3fd 100644 --- a/www/__tests__/clientStats.test.ts +++ b/www/__tests__/clientStats.test.ts @@ -40,10 +40,14 @@ it('stores a client stats event', async () => { it('stores a client stats error', async () => { const errorStr = 'test error'; - await addStatError('missing_keys', errorStr); + try { + throw new Error(errorStr); + } catch (error) { + await addStatError(error.message); + } const storedMessages = await db.getAllMessages('stats/client_error', false); expect(storedMessages).toContainEqual({ - name: 'missing_keys', + name: 'ui_error', ts: expect.any(Number), reading: errorStr, client_app_version: '1.2.3', diff --git a/www/js/plugin/clientStats.ts b/www/js/plugin/clientStats.ts index e106dd7cf..7c512f2c2 100644 --- a/www/js/plugin/clientStats.ts +++ b/www/js/plugin/clientStats.ts @@ -12,7 +12,8 @@ type StatKey = | 'set_reminder_prefs' | 'force_sync' | 'open_notification' - | 'missing_keys'; + | 'missing_keys' + | 'ui_error'; let appVersion; export function getAppVersion() { @@ -39,9 +40,9 @@ export async function addStatReading(name: StatKey, reading?: any) { displayErrorMsg('addStatReading: db is not defined'); } -export async function addStatError(name: StatKey, errorStr: string) { +export async function addStatError(errorMsg: string) { const db = window['cordova']?.plugins?.BEMUserCache; - const event = await getStatsEvent(name, errorStr); + const event = await getStatsEvent('ui_error', errorMsg); logDebug('addStatError: adding CLIENT_ERROR event: ' + JSON.stringify(event)); if (db) return db.putMessage(CLIENT_ERROR, event); displayErrorMsg('addStatError: db is not defined'); diff --git a/www/js/plugin/logger.ts b/www/js/plugin/logger.ts index 98e852978..12664bbed 100644 --- a/www/js/plugin/logger.ts +++ b/www/js/plugin/logger.ts @@ -1,3 +1,5 @@ +import { addStatError } from './clientStats'; + export const logDebug = (message: string) => window['Logger']?.log(window['Logger'].LEVEL_DEBUG, message); @@ -19,6 +21,7 @@ export function displayErrorMsg(errorMsg: string, title?: string) { } const displayMsg = `━━━━\n${title}\n━━━━\n` + errorMsg; window.alert(displayMsg); + addStatError(title ? `${title}: ${errorMsg}` : errorMsg); console.error(displayMsg); window['Logger']?.log(window['Logger'].LEVEL_ERROR, displayMsg); } From 338f7f1071c4eb88642d97efe34165a6e1e2036d Mon Sep 17 00:00:00 2001 From: Jack Greenlee Date: Sat, 14 Sep 2024 01:18:54 -0400 Subject: [PATCH 3/4] fix tests, record client stats for missing native consent in dynamicConfig.test.ts, there are some tests where an error is expected to be thrown and error popup is triggered. clientstats is invoked because we are now recording a ui_error stat on all error popups. Client stats needs mockDevice and mockGetAppVersion to work in startprefs.test.ts, a similar error popup occurs so the mocks are needed there too. However, while inspecting startprefs.ts, I noticied the popup occurs when consent document exists in localstorage but not in native storage. I do not think we need an error popup for this; instead let's do what we do in storage.ts, which is record a 'missing_keys" client stat, and give it a type of 'document_mismatch'. --- www/__tests__/dynamicConfig.test.ts | 4 +++- www/__tests__/startprefs.test.ts | 9 ++++++++- www/js/splash/startprefs.ts | 8 +++++++- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/www/__tests__/dynamicConfig.test.ts b/www/__tests__/dynamicConfig.test.ts index a5b1d927d..e723e8308 100644 --- a/www/__tests__/dynamicConfig.test.ts +++ b/www/__tests__/dynamicConfig.test.ts @@ -1,4 +1,4 @@ -import { mockBEMUserCache } from '../__mocks__/cordovaMocks'; +import { mockBEMUserCache, mockDevice, mockGetAppVersion } from '../__mocks__/cordovaMocks'; import { mockAlert, mockLogger } from '../__mocks__/globalMocks'; import { getConfig, initByUser } from '../js/config/dynamicConfig'; @@ -8,6 +8,8 @@ window['i18next'] = initializedI18next; mockLogger(); mockAlert(); +mockDevice(); +mockGetAppVersion(); mockBEMUserCache(); beforeEach(() => { diff --git a/www/__tests__/startprefs.test.ts b/www/__tests__/startprefs.test.ts index 75ed707dc..4eba31c09 100644 --- a/www/__tests__/startprefs.test.ts +++ b/www/__tests__/startprefs.test.ts @@ -5,9 +5,16 @@ import { getConsentDocument, } from '../js/splash/startprefs'; -import { mockBEMUserCache, mockBEMDataCollection } from '../__mocks__/cordovaMocks'; +import { + mockBEMUserCache, + mockBEMDataCollection, + mockDevice, + mockGetAppVersion, +} from '../__mocks__/cordovaMocks'; import { mockLogger } from '../__mocks__/globalMocks'; +mockDevice(); +mockGetAppVersion(); mockBEMUserCache(); mockBEMDataCollection(); mockLogger(); diff --git a/www/js/splash/startprefs.ts b/www/js/splash/startprefs.ts index 5e1edd188..751ba664a 100644 --- a/www/js/splash/startprefs.ts +++ b/www/js/splash/startprefs.ts @@ -1,6 +1,7 @@ import { storageGet, storageSet } from '../plugin/storage'; import { logInfo, logDebug, displayErrorMsg } from '../plugin/logger'; import { EVENTS, publish } from '../customEventHandler'; +import { addStatReading } from '../plugin/clientStats'; // data collection consented protocol: string, represents the date on // which the consented protocol was approved by the IRB @@ -106,7 +107,12 @@ function checkNativeConsent() { if (resultDoc == null) { if (isConsented()) { logDebug('Local consent found, native consent missing, writing consent to native'); - displayErrorMsg('Local consent found, native consent missing, writing consent to native'); + addStatReading('missing_keys', { + type: 'document_mismatch', + document: 'config/consent', + localConsent: _curr_consented, + nativeConsent: resultDoc, + }).then(logDebug('Logged missing_keys event to client stats for missing native consent')); return writeConsentToNative(); } else { logDebug('Both local and native consent not found, nothing to sync'); From 4596d0c0b5930087a6ff09dace399d764a2b6d48 Mon Sep 17 00:00:00 2001 From: Jack Greenlee Date: Sat, 14 Sep 2024 01:28:29 -0400 Subject: [PATCH 4/4] simplify mocking by specifying jest setup file Instead of having each test file call the mocks it uses, we can have jest set up all the mocks beforehand. in jest.config.js, setupFiles points to setupJestEnv.js where all the mocks are called. This is way less verbose, and may also speed up the execution of the test suites because the mocks are applied once rather than duplicated for every individual test. We still have flexibility for individual tests to call mocks on their own; for example in enketoHelper.test.ts there are several calls to mockBEMUserCache() because it is testing out various configs --- jest.config.js | 1 + www/__mocks__/cordovaMocks.ts | 13 +++++++++ www/__mocks__/globalMocks.ts | 25 ------------------ www/__mocks__/setupJestEnv.js | 32 +++++++++++++++++++++++ www/__tests__/TimelineContext.test.tsx | 5 ---- www/__tests__/clientStats.test.ts | 6 ----- www/__tests__/commHelper.test.ts | 3 --- www/__tests__/confirmHelper.test.ts | 3 --- www/__tests__/customEventHandler.test.ts | 3 --- www/__tests__/customMetricsHelper.test.ts | 2 -- www/__tests__/dynamicConfig.test.ts | 8 ------ www/__tests__/enketoHelper.test.ts | 2 -- www/__tests__/footprintHelper.test.ts | 2 -- www/__tests__/inputMatcher.test.ts | 5 ---- www/__tests__/metHelper.test.ts | 2 -- www/__tests__/notifScheduler.test.ts | 6 ----- www/__tests__/pushNotifySettings.test.ts | 15 +---------- www/__tests__/remoteNotifyHandler.test.ts | 23 +++------------- www/__tests__/startprefs.test.ts | 14 ---------- www/__tests__/storage.test.ts | 7 ----- www/__tests__/storeDeviceSettings.test.ts | 17 ------------ www/__tests__/timelineHelper.test.ts | 14 ---------- www/__tests__/unifiedDataLoader.test.ts | 3 --- www/__tests__/uploadService.test.ts | 11 -------- 24 files changed, 51 insertions(+), 171 deletions(-) delete mode 100644 www/__mocks__/globalMocks.ts create mode 100644 www/__mocks__/setupJestEnv.js diff --git a/jest.config.js b/jest.config.js index c47992ee5..79b629488 100644 --- a/jest.config.js +++ b/jest.config.js @@ -17,6 +17,7 @@ module.exports = { moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx'], moduleDirectories: ["node_modules", "src"], globals: {"__DEV__": false}, + setupFiles: ["/www/__mocks__/setupJestEnv.js"], collectCoverage: true, collectCoverageFrom: [ "www/js/**/*.{ts,tsx,js,jsx}", diff --git a/www/__mocks__/cordovaMocks.ts b/www/__mocks__/cordovaMocks.ts index eac078058..b8c6d4361 100644 --- a/www/__mocks__/cordovaMocks.ts +++ b/www/__mocks__/cordovaMocks.ts @@ -1,5 +1,18 @@ import packageJsonBuild from '../../package.cordovabuild.json'; +export let alerts: string[] = []; + +export const mockLogger = () => { + window['Logger'] = { log: console.log }; + window['alert'] = (message) => { + console.log(message); + alerts.push(message); + }; + console.error = (msg) => { + console.log(msg); + }; +}; + export const mockCordova = () => { window['cordova'] ||= {}; window['cordova'].platformId ||= 'ios'; diff --git a/www/__mocks__/globalMocks.ts b/www/__mocks__/globalMocks.ts deleted file mode 100644 index 4d591f55f..000000000 --- a/www/__mocks__/globalMocks.ts +++ /dev/null @@ -1,25 +0,0 @@ -export const mockLogger = () => { - window['Logger'] = { log: console.log }; - window.alert = (msg) => { - console.log(msg); - }; - console.error = (msg) => { - console.log(msg); - }; -}; - -let alerts: string[] = []; - -export const mockAlert = () => { - window['alert'] = (message) => { - alerts.push(message); - }; -}; - -export const clearAlerts = () => { - alerts = []; -}; - -export const getAlerts = () => { - return alerts; -}; diff --git a/www/__mocks__/setupJestEnv.js b/www/__mocks__/setupJestEnv.js new file mode 100644 index 000000000..f20b1fa1e --- /dev/null +++ b/www/__mocks__/setupJestEnv.js @@ -0,0 +1,32 @@ +/* + Applies mocks to the global (window) object for use in tests. + This is run before all of the tests are run, so these mocks are available in all tests. +*/ + +import { + mockBEMDataCollection, + mockBEMServerCom, + mockBEMUserCache, + mockCordova, + mockDevice, + mockFile, + mockGetAppVersion, + mockInAppBrowser, + mockLogger, + mockReminders, +} from './cordovaMocks'; +import { mockFileSystem } from './fileSystemMocks'; +import { mockPushNotification } from './pushNotificationMocks'; + +mockLogger(); +mockCordova(); +mockDevice(); +mockGetAppVersion(); +mockBEMUserCache(); +mockBEMDataCollection(); +mockBEMServerCom(); +mockFile(); +mockFileSystem(); +mockInAppBrowser(); +mockPushNotification(); +mockReminders(); diff --git a/www/__tests__/TimelineContext.test.tsx b/www/__tests__/TimelineContext.test.tsx index 49b62315f..4aa670437 100644 --- a/www/__tests__/TimelineContext.test.tsx +++ b/www/__tests__/TimelineContext.test.tsx @@ -2,11 +2,6 @@ import React, { useEffect } from 'react'; import { View, Text } from 'react-native'; import { act, render, screen, waitFor } from '@testing-library/react-native'; import { useTimelineContext } from '../js/TimelineContext'; -import { mockLogger } from '../__mocks__/globalMocks'; -import { mockBEMServerCom, mockBEMUserCache } from '../__mocks__/cordovaMocks'; - -mockLogger(); -mockBEMUserCache(); jest.mock('../js/services/commHelper', () => ({ getPipelineRangeTs: jest.fn(() => Promise.resolve({ start_ts: 1, end_ts: 10 })), diff --git a/www/__tests__/clientStats.test.ts b/www/__tests__/clientStats.test.ts index 2795ff3fd..5bd906a69 100644 --- a/www/__tests__/clientStats.test.ts +++ b/www/__tests__/clientStats.test.ts @@ -1,11 +1,5 @@ -import { mockBEMUserCache, mockDevice, mockGetAppVersion } from '../__mocks__/cordovaMocks'; import { addStatError, addStatReading, getAppVersion } from '../js/plugin/clientStats'; -mockDevice(); -// this mocks cordova-plugin-app-version, generating a "Mock App", version "1.2.3" -mockGetAppVersion(); -// clientStats.ts uses BEMUserCache to store the stats, so we need to mock that too -mockBEMUserCache(); const db = window['cordova']?.plugins?.BEMUserCache; it('gets the app version', async () => { diff --git a/www/__tests__/commHelper.test.ts b/www/__tests__/commHelper.test.ts index c66fb5f36..1bc52c58f 100644 --- a/www/__tests__/commHelper.test.ts +++ b/www/__tests__/commHelper.test.ts @@ -1,8 +1,5 @@ -import { mockLogger } from '../__mocks__/globalMocks'; import { fetchUrlCached } from '../js/services/commHelper'; -mockLogger(); - // mock for JavaScript 'fetch' // we emulate a 100ms delay when i) fetching data and ii) parsing it as text global.fetch = (url: string) => diff --git a/www/__tests__/confirmHelper.test.ts b/www/__tests__/confirmHelper.test.ts index 6642b6ed4..72cdfb773 100644 --- a/www/__tests__/confirmHelper.test.ts +++ b/www/__tests__/confirmHelper.test.ts @@ -1,5 +1,3 @@ -import { mockLogger } from '../__mocks__/globalMocks'; -import * as CommHelper from '../js/services/commHelper'; import { baseLabelInputDetails, getLabelInputDetails, @@ -17,7 +15,6 @@ import initializedI18next from '../js/i18nextInit'; import { CompositeTrip, UserInputEntry } from '../js/types/diaryTypes'; import { UserInputMap } from '../js/TimelineContext'; window['i18next'] = initializedI18next; -mockLogger(); const fakeAppConfig = { label_options: 'json/label-options.json.sample', diff --git a/www/__tests__/customEventHandler.test.ts b/www/__tests__/customEventHandler.test.ts index 7a5c09539..f3476536b 100644 --- a/www/__tests__/customEventHandler.test.ts +++ b/www/__tests__/customEventHandler.test.ts @@ -1,7 +1,4 @@ import { publish, subscribe, unsubscribe } from '../js/customEventHandler'; -import { mockLogger } from '../__mocks__/globalMocks'; - -mockLogger(); it('subscribes and publishes to an event', () => { const listener = jest.fn(); diff --git a/www/__tests__/customMetricsHelper.test.ts b/www/__tests__/customMetricsHelper.test.ts index ef840283f..77bb009b0 100644 --- a/www/__tests__/customMetricsHelper.test.ts +++ b/www/__tests__/customMetricsHelper.test.ts @@ -6,12 +6,10 @@ import { initCustomDatasetHelper, } from '../js/metrics/customMetricsHelper'; import { mockBEMUserCache } from '../__mocks__/cordovaMocks'; -import { mockLogger } from '../__mocks__/globalMocks'; import fakeLabels from '../__mocks__/fakeLabels.json'; import fakeConfig from '../__mocks__/fakeConfig.json'; mockBEMUserCache(fakeConfig); -mockLogger(); beforeEach(() => { _test_clearCustomMetrics(); diff --git a/www/__tests__/dynamicConfig.test.ts b/www/__tests__/dynamicConfig.test.ts index e723e8308..5693ab3fb 100644 --- a/www/__tests__/dynamicConfig.test.ts +++ b/www/__tests__/dynamicConfig.test.ts @@ -1,17 +1,9 @@ -import { mockBEMUserCache, mockDevice, mockGetAppVersion } from '../__mocks__/cordovaMocks'; -import { mockAlert, mockLogger } from '../__mocks__/globalMocks'; import { getConfig, initByUser } from '../js/config/dynamicConfig'; import initializedI18next from '../js/i18nextInit'; import { storageClear } from '../js/plugin/storage'; window['i18next'] = initializedI18next; -mockLogger(); -mockAlert(); -mockDevice(); -mockGetAppVersion(); -mockBEMUserCache(); - beforeEach(() => { // clear all storage and the config document storageClear({ local: true, native: true }); diff --git a/www/__tests__/enketoHelper.test.ts b/www/__tests__/enketoHelper.test.ts index 5728caf0d..58347fde2 100644 --- a/www/__tests__/enketoHelper.test.ts +++ b/www/__tests__/enketoHelper.test.ts @@ -8,7 +8,6 @@ import { EnketoUserInputEntry, } from '../js/survey/enketo/enketoHelper'; import { mockBEMUserCache } from '../__mocks__/cordovaMocks'; -import { mockLogger } from '../__mocks__/globalMocks'; import { getConfig, _test_resetPromisedConfig } from '../../www/js/config/dynamicConfig'; import fakeConfig from '../__mocks__/fakeConfig.json'; @@ -18,7 +17,6 @@ import { AppConfig } from '../js/types/appConfigTypes'; window['i18next'] = initializedI18next; mockBEMUserCache(fakeConfig); -mockLogger(); global.URL = require('url').URL; global.Blob = require('node:buffer').Blob; diff --git a/www/__tests__/footprintHelper.test.ts b/www/__tests__/footprintHelper.test.ts index ba49e6645..666e36c92 100644 --- a/www/__tests__/footprintHelper.test.ts +++ b/www/__tests__/footprintHelper.test.ts @@ -10,12 +10,10 @@ import { } from '../js/metrics/footprintHelper'; import { getConfig } from '../js/config/dynamicConfig'; import { mockBEMUserCache } from '../__mocks__/cordovaMocks'; -import { mockLogger } from '../__mocks__/globalMocks'; import fakeLabels from '../__mocks__/fakeLabels.json'; import fakeConfig from '../__mocks__/fakeConfig.json'; mockBEMUserCache(fakeConfig); -mockLogger(); global.fetch = (url: string) => new Promise((rs, rj) => { diff --git a/www/__tests__/inputMatcher.test.ts b/www/__tests__/inputMatcher.test.ts index 9ea4d9b02..c957a6f86 100644 --- a/www/__tests__/inputMatcher.test.ts +++ b/www/__tests__/inputMatcher.test.ts @@ -1,5 +1,3 @@ -import { mockBEMUserCache } from '../__mocks__/cordovaMocks'; -import { mockLogger } from '../__mocks__/globalMocks'; import { updateLocalUnprocessedInputs } from '../js/diary/timelineHelper'; import * as logger from '../js/plugin/logger'; import { EnketoUserInputEntry } from '../js/survey/enketo/enketoHelper'; @@ -17,9 +15,6 @@ import { import { AppConfig } from '../js/types/appConfigTypes'; import { CompositeTrip, TimelineEntry, UserInputEntry } from '../js/types/diaryTypes'; -mockLogger(); -mockBEMUserCache(); - describe('input-matcher', () => { let userTrip: UserInputEntry; let trip: TimelineEntry; diff --git a/www/__tests__/metHelper.test.ts b/www/__tests__/metHelper.test.ts index 72aa09488..d7478d6c0 100644 --- a/www/__tests__/metHelper.test.ts +++ b/www/__tests__/metHelper.test.ts @@ -1,13 +1,11 @@ import { getMet } from '../js/metrics/metHelper'; import { mockBEMUserCache } from '../__mocks__/cordovaMocks'; -import { mockLogger } from '../__mocks__/globalMocks'; import fakeLabels from '../__mocks__/fakeLabels.json'; import { getConfig } from '../js/config/dynamicConfig'; import { initCustomDatasetHelper } from '../js/metrics/customMetricsHelper'; import fakeConfig from '../__mocks__/fakeConfig.json'; mockBEMUserCache(fakeConfig); -mockLogger(); global.fetch = (url: string) => new Promise((rs, rj) => { diff --git a/www/__tests__/notifScheduler.test.ts b/www/__tests__/notifScheduler.test.ts index ac6408204..d1a95c837 100644 --- a/www/__tests__/notifScheduler.test.ts +++ b/www/__tests__/notifScheduler.test.ts @@ -1,6 +1,3 @@ -import { mockReminders } from '../__mocks__/cordovaMocks'; -import { mockLogger } from '../__mocks__/globalMocks'; -import i18next from 'i18next'; import { logDebug } from '../js/plugin/logger'; import { DateTime } from 'luxon'; import { getUser, updateUser } from '../js/services/commHelper'; @@ -60,9 +57,6 @@ const exampleReminderSchemes = { }, }; -mockLogger(); -mockReminders(); - jest.mock('i18next', () => ({ resolvedLanguage: 'en', })); diff --git a/www/__tests__/pushNotifySettings.test.ts b/www/__tests__/pushNotifySettings.test.ts index d452aa819..179f56734 100644 --- a/www/__tests__/pushNotifySettings.test.ts +++ b/www/__tests__/pushNotifySettings.test.ts @@ -3,20 +3,7 @@ import { EVENTS, publish } from '../js/customEventHandler'; import { INTRO_DONE_KEY, readIntroDone } from '../js/onboarding/onboardingHelper'; import { storageSet } from '../js/plugin/storage'; import { initPushNotify } from '../js/splash/pushNotifySettings'; -import { mockCordova, mockBEMUserCache, mockBEMDataCollection } from '../__mocks__/cordovaMocks'; -import { mockLogger } from '../__mocks__/globalMocks'; -import { - clearNotifMock, - getOnList, - mockPushNotification, - getCalled, -} from '../__mocks__/pushNotificationMocks'; - -mockCordova(); -mockLogger(); -mockPushNotification(); -mockBEMUserCache(); -mockBEMDataCollection(); +import { clearNotifMock, getOnList, getCalled } from '../__mocks__/pushNotificationMocks'; global.fetch = (url: string) => new Promise((rs, rj) => { diff --git a/www/__tests__/remoteNotifyHandler.test.ts b/www/__tests__/remoteNotifyHandler.test.ts index 4d3761547..19d14b7a3 100644 --- a/www/__tests__/remoteNotifyHandler.test.ts +++ b/www/__tests__/remoteNotifyHandler.test.ts @@ -1,27 +1,12 @@ import { EVENTS, publish } from '../js/customEventHandler'; import { initRemoteNotifyHandler } from '../js/splash/remoteNotifyHandler'; -import { - clearURL, - getURL, - mockBEMUserCache, - mockDevice, - mockGetAppVersion, - mockInAppBrowser, -} from '../__mocks__/cordovaMocks'; -import { clearAlerts, getAlerts, mockAlert, mockLogger } from '../__mocks__/globalMocks'; - -mockLogger(); -mockDevice(); -mockBEMUserCache(); -mockGetAppVersion(); -mockInAppBrowser(); -mockAlert(); +import { alerts, clearURL, getURL } from '../__mocks__/cordovaMocks'; const db = window['cordova']?.plugins?.BEMUserCache; beforeEach(() => { clearURL(); - clearAlerts(); + alerts.length = 0; }); it('does not adds a statEvent if not subscribed', async () => { @@ -65,12 +50,12 @@ it('handles the popup if subscribed', () => { }, }, }); - expect(getAlerts()).toEqual(expect.arrayContaining(['Hello World'])); + expect(alerts).toEqual(expect.arrayContaining(['Hello World'])); }); it('does nothing if subscribed and no data', () => { initRemoteNotifyHandler(); publish(EVENTS.CLOUD_NOTIFICATION_EVENT, {}); expect(getURL()).toEqual(''); - expect(getAlerts()).toEqual([]); + expect(alerts).toEqual([]); }); diff --git a/www/__tests__/startprefs.test.ts b/www/__tests__/startprefs.test.ts index 4eba31c09..43828fa8b 100644 --- a/www/__tests__/startprefs.test.ts +++ b/www/__tests__/startprefs.test.ts @@ -5,20 +5,6 @@ import { getConsentDocument, } from '../js/splash/startprefs'; -import { - mockBEMUserCache, - mockBEMDataCollection, - mockDevice, - mockGetAppVersion, -} from '../__mocks__/cordovaMocks'; -import { mockLogger } from '../__mocks__/globalMocks'; - -mockDevice(); -mockGetAppVersion(); -mockBEMUserCache(); -mockBEMDataCollection(); -mockLogger(); - global.fetch = (url: string) => new Promise((rs, rj) => { setTimeout(() => diff --git a/www/__tests__/storage.test.ts b/www/__tests__/storage.test.ts index ca6d71dec..dbdb96efe 100644 --- a/www/__tests__/storage.test.ts +++ b/www/__tests__/storage.test.ts @@ -1,12 +1,5 @@ -import { mockBEMUserCache } from '../__mocks__/cordovaMocks'; -import { mockLogger } from '../__mocks__/globalMocks'; import { storageClear, storageGet, storageRemove, storageSet } from '../js/plugin/storage'; -// mocks used - storage.ts uses BEMUserCache and logging. -// localStorage is already mocked for us by Jest :) -mockLogger(); -mockBEMUserCache(); - it('stores a value and retrieves it back', async () => { await storageSet('test1', 'test value 1'); const retVal = await storageGet('test1'); diff --git a/www/__tests__/storeDeviceSettings.test.ts b/www/__tests__/storeDeviceSettings.test.ts index 4bccbc0af..f3d55c204 100644 --- a/www/__tests__/storeDeviceSettings.test.ts +++ b/www/__tests__/storeDeviceSettings.test.ts @@ -2,26 +2,9 @@ import { readConsentState, markConsented } from '../js/splash/startprefs'; import { storageClear } from '../js/plugin/storage'; import { getUser } from '../js/services/commHelper'; import { initStoreDeviceSettings, teardownDeviceSettings } from '../js/splash/storeDeviceSettings'; -import { - mockBEMDataCollection, - mockBEMServerCom, - mockBEMUserCache, - mockCordova, - mockDevice, - mockGetAppVersion, -} from '../__mocks__/cordovaMocks'; -import { mockLogger } from '../__mocks__/globalMocks'; import { EVENTS, publish } from '../js/customEventHandler'; import { markIntroDone } from '../js/onboarding/onboardingHelper'; -mockBEMUserCache(); -mockDevice(); -mockCordova(); -mockLogger(); -mockGetAppVersion(); -mockBEMServerCom(); -mockBEMDataCollection(); - global.fetch = (url: string) => new Promise((rs, rj) => { setTimeout(() => diff --git a/www/__tests__/timelineHelper.test.ts b/www/__tests__/timelineHelper.test.ts index c1c130272..93d68289d 100644 --- a/www/__tests__/timelineHelper.test.ts +++ b/www/__tests__/timelineHelper.test.ts @@ -1,4 +1,3 @@ -import { clearAlerts, mockAlert, mockLogger } from '../__mocks__/globalMocks'; import { useGeojsonForTrip, readAllCompositeTrips, @@ -10,22 +9,9 @@ import { unprocessedLabels, unprocessedNotes, } from '../js/diary/timelineHelper'; -import { mockBEMUserCache } from '../__mocks__/cordovaMocks'; import * as mockTLH from '../__mocks__/timelineHelperMocks'; import { GeoJSONData, GeoJSONStyledFeature } from '../js/types/diaryTypes'; -mockLogger(); -mockAlert(); -mockBEMUserCache(); - -beforeEach(() => { - clearAlerts(); -}); - -afterAll(() => { - jest.restoreAllMocks(); -}); - describe('useGeojsonForTrip', () => { it('work with an empty input', () => { const testVal = useGeojsonForTrip({} as any); diff --git a/www/__tests__/unifiedDataLoader.test.ts b/www/__tests__/unifiedDataLoader.test.ts index 7916cfde1..700035684 100644 --- a/www/__tests__/unifiedDataLoader.test.ts +++ b/www/__tests__/unifiedDataLoader.test.ts @@ -1,9 +1,6 @@ -import { mockLogger } from '../__mocks__/globalMocks'; import { removeDup, combinedPromises } from '../js/services/unifiedDataLoader'; import { LocalDt, BEMData } from '../js/types/serverData'; -mockLogger(); - const testOne: BEMData = { data: '', metadata: { diff --git a/www/__tests__/uploadService.test.ts b/www/__tests__/uploadService.test.ts index b9bede9fd..feb1cd48e 100644 --- a/www/__tests__/uploadService.test.ts +++ b/www/__tests__/uploadService.test.ts @@ -3,17 +3,6 @@ //at some point we hope to restore this functionality import { uploadFile } from '../js/control/uploadService'; -import { mockLogger } from '../__mocks__/globalMocks'; -import { mockDevice, mockGetAppVersion, mockCordova, mockFile } from '../__mocks__/cordovaMocks'; -import { mockFileSystem } from '../__mocks__/fileSystemMocks'; - -mockDevice(); -mockGetAppVersion(); -mockCordova(); - -mockLogger(); -mockFile(); //mocks the base directory -mockFileSystem(); //comnplex mock, allows the readDBFile to work in testing //use this message to verify that the post went through let message = '';