Skip to content

Commit

Permalink
cleanup appstate hook, refreshTimeline on app resume, revamp clientstats
Browse files Browse the repository at this point in the history
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
  • Loading branch information
JGreenlee committed Sep 13, 2024
1 parent b426c9a commit b6a1b0b
Show file tree
Hide file tree
Showing 12 changed files with 64 additions and 61 deletions.
22 changes: 8 additions & 14 deletions www/__tests__/clientStats.test.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand Down
8 changes: 4 additions & 4 deletions www/__tests__/remoteNotifyHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([]);
});

Expand All @@ -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',
});
Expand Down
13 changes: 11 additions & 2 deletions www/js/Main.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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) => [
{
Expand Down Expand Up @@ -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
Expand All @@ -67,7 +76,7 @@ const Main = () => {
<TimelineContext.Provider value={timelineContext}>
<BottomNavigation
navigationState={{ index, routes }}
onIndexChange={setIndex}
onIndexChange={onIndexChange}
renderScene={renderScene}
// Place at bottom, color of 'surface' (white) by default, and 68px tall (default was 80)
safeAreaInsets={{ bottom: 0 }}
Expand Down
2 changes: 2 additions & 0 deletions www/js/TimelineContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { getNotDeletedCandidates, mapInputsToTimelineEntries } from './survey/in
import { EnketoUserInputEntry } from './survey/enketo/enketoHelper';
import { VehicleIdentity } from './types/appConfigTypes';
import { primarySectionForTrip } from './diary/diaryHelper';
import useAppStateChange from './useAppStateChange';

const TODAY_DATE = DateTime.now().toISODate();

Expand Down Expand Up @@ -53,6 +54,7 @@ type ContextProps = {
export const useTimelineContext = (): ContextProps => {
const { t } = useTranslation();
const appConfig = useAppConfig();
useAppStateChange(() => refreshTimeline());

const [labelOptions, setLabelOptions] = useState<LabelOptions | null>(null);
// timestamp range that has been processed by the pipeline on the server
Expand Down
6 changes: 3 additions & 3 deletions www/js/control/ControlSyncHelper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions www/js/diary/cards/TripCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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 });
}

Expand Down
42 changes: 14 additions & 28 deletions www/js/plugin/clientStats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -30,30 +23,23 @@ 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));
if (db) return db.putMessage(CLIENT_TIME, event);
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));
Expand Down
6 changes: 3 additions & 3 deletions www/js/plugin/storage.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { addStatReading, statKeys } from './clientStats';
import { addStatReading } from './clientStats';
import { logDebug, logWarn } from './logger';

function mungeValue(key, value) {
Expand Down Expand Up @@ -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,
Expand All @@ -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'));
}
Expand Down
4 changes: 2 additions & 2 deletions www/js/splash/notifScheduler.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions www/js/splash/remoteNotifyHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 &&
Expand Down
9 changes: 8 additions & 1 deletion www/js/survey/multilabel/MultiLabelButtonGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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');
});
Expand Down Expand Up @@ -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)}
</DiaryButton>
</View>
Expand Down
7 changes: 5 additions & 2 deletions www/js/useAppStateChange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -17,8 +17,11 @@ const useAppStateChange = (onResume) => {
}

appState.current = nextAppState;
logDebug('new AppState: ' + appState.current);
addStatReading('app_state_change', appState.current);
});
return () => {
subscription.remove();
};
}, []);

return {};
Expand Down

0 comments on commit b6a1b0b

Please sign in to comment.