Skip to content

Commit

Permalink
Merge pull request #1155 from JGreenlee/fix-userinput-may9
Browse files Browse the repository at this point in the history
🐛 Bugfixes + Refactoring relating to User Inputs
  • Loading branch information
shankari authored May 11, 2024
2 parents 681d2f1 + 2761e19 commit 4ea0f35
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 65 deletions.
22 changes: 14 additions & 8 deletions www/__tests__/inputMatcher.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { mockBEMUserCache } from '../__mocks__/cordovaMocks';
import { mockLogger } from '../__mocks__/globalMocks';
import { unprocessedLabels, updateLocalUnprocessedInputs } from '../js/diary/timelineHelper';
import { updateLocalUnprocessedInputs } from '../js/diary/timelineHelper';
import * as logger from '../js/plugin/logger';
import { EnketoUserInputEntry } from '../js/survey/enketo/enketoHelper';
import {
Expand Down Expand Up @@ -376,9 +376,9 @@ describe('mapInputsToTimelineEntries on an ENKETO configuration', () => {
user_input: {
trip_user_input: {
data: {
name: 'TripConfirmSurvey',
name: 'MyCustomSurvey',
version: 1,
xmlResponse: '<processed TripConfirmSurvey response>',
xmlResponse: '<processed MyCustomSurvey response>',
start_ts: 1000,
end_ts: 3000,
},
Expand Down Expand Up @@ -417,15 +417,21 @@ describe('mapInputsToTimelineEntries on an ENKETO configuration', () => {
],
},
] as any as TimelineEntry[];

// reset local unprocessed inputs to ensure MUTLILABEL inputs don't leak into ENKETO tests
beforeAll(async () => {
await updateLocalUnprocessedInputs({ start_ts: 1000, end_ts: 5000 }, fakeConfigEnketo);
});

it('creates a map that has the processed responses and notes', () => {
const [labelMap, notesMap] = mapInputsToTimelineEntries(
timelineEntriesEnketo,
fakeConfigEnketo,
);
expect(labelMap).toMatchObject({
trip1: {
SURVEY: {
data: { xmlResponse: '<processed TripConfirmSurvey response>' },
MyCustomSurvey: {
data: { xmlResponse: '<processed MyCustomSurvey response>' },
},
},
});
Expand Down Expand Up @@ -460,12 +466,12 @@ describe('mapInputsToTimelineEntries on an ENKETO configuration', () => {

expect(labelMap).toMatchObject({
trip1: {
SURVEY: {
data: { xmlResponse: '<processed TripConfirmSurvey response>' },
MyCustomSurvey: {
data: { xmlResponse: '<processed MyCustomSurvey response>' },
},
},
trip2: {
SURVEY: {
TripConfirmSurvey: {
data: { xmlResponse: '<unprocessed TripConfirmSurvey response>' },
},
},
Expand Down
4 changes: 2 additions & 2 deletions www/__tests__/timelineHelper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,10 @@ describe('unprocessedLabels, unprocessedNotes', () => {

// update unprocessed inputs and check that the trip survey response shows up in unprocessedLabels
await updateAllUnprocessedInputs({ start_ts: 4, end_ts: 6 }, mockTLH.mockConfigEnketo);
expect(unprocessedLabels['SURVEY'][0].data).toEqual(tripSurveyResponse);
expect(unprocessedLabels['TripConfirmSurvey'][0].data).toEqual(tripSurveyResponse);
// the second response is ignored for now because we haven't enabled place_user_input yet
// so the length is only 1
expect(unprocessedLabels['SURVEY'].length).toEqual(1);
expect(unprocessedLabels['TripConfirmSurvey'].length).toEqual(1);
});

it('has some trip- and place- level additions after they were just recorded', async () => {
Expand Down
1 change: 0 additions & 1 deletion www/i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@
"choose-mode": "Mode",
"choose-replaced-mode": "Replaces",
"choose-purpose": "Purpose",
"choose-survey": "Add Trip Details",
"select-mode-scroll": "Mode (scroll for more)",
"select-replaced-mode-scroll": "Replaces (scroll for more)",
"select-purpose-scroll": "Purpose (scroll for more)",
Expand Down
12 changes: 6 additions & 6 deletions www/js/diary/LabelTabContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import { EnketoUserInputEntry } from '../survey/enketo/enketoHelper';
import { VehicleIdentity } from '../types/appConfigTypes';

export type UserInputMap = {
/* if the key here is 'SURVEY', we are in the ENKETO configuration, meaning the user input
value will have the raw 'xmlResponse' string */
SURVEY?: EnketoUserInputEntry;
} & {
/* all other keys, (e.g. 'MODE', 'PURPOSE') are from the MULTILABEL configuration
and will have the 'label' string but no 'xmlResponse' string */
/* If keys are 'MODE', 'PURPOSE', 'REPLACED_MODE', this is the MULTILABEL configuration.
Values are entries that have a 'label' value in their 'data' */
[k in MultilabelKey]?: UserInputEntry;
} & {
/* Otherwise we are in the ENKETO configuration, and keys are names of surveys.
Values are entries that have an 'xmlResponse' value in their 'data' */
[k: string]: EnketoUserInputEntry | undefined;
};

export type TimelineMap = Map<string, TimelineEntry>; // Todo: update to reflect unpacked trips (origin_Key, etc)
Expand Down
17 changes: 13 additions & 4 deletions www/js/diary/timelineHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ import {
} from '../types/diaryTypes';
import { getLabelInputDetails, getLabelInputs } from '../survey/multilabel/confirmHelper';
import { LabelOptions } from '../types/labelTypes';
import { EnketoUserInputEntry, filterByNameAndVersion } from '../survey/enketo/enketoHelper';
import {
EnketoUserInputEntry,
filterByNameAndVersion,
resolveSurveyButtonConfig,
} from '../survey/enketo/enketoHelper';
import { AppConfig } from '../types/appConfigTypes';
import { Point, Feature } from 'geojson';
import { ble_matching } from 'e-mission-common';
Expand Down Expand Up @@ -91,7 +95,8 @@ export function compositeTrips2TimelineMap(ctList: Array<any>, unpackPlaces?: bo
}

/* 'LABELS' are 1:1 - each trip or place has a single label for each label type
(e.g. 'MODE' and 'PURPOSE' for MULTILABEL configuration, or 'SURVEY' for ENKETO configuration) */
(e.g. 'MODE' and 'PURPOSE' for MULTILABEL configuration, or the name of the survey
for ENKETO configuration) */
export let unprocessedLabels: { [key: string]: UserInputEntry[] } = {};
/* 'NOTES' are 1:n - each trip or place can have any number of notes */
export let unprocessedNotes: EnketoUserInputEntry[] = [];
Expand All @@ -115,10 +120,14 @@ function updateUnprocessedInputs(
const labelResults = comboResults.slice(0, labelsPromises.length);
const notesResults = comboResults.slice(labelsPromises.length).flat(2);
// fill in the unprocessedLabels object with the labels we just read
unprocessedLabels = {};
labelResults.forEach((r, i) => {
if (appConfig.survey_info?.['trip-labels'] == 'ENKETO') {
const filtered = filterByNameAndVersion('TripConfirmSurvey', r, appConfig);
unprocessedLabels['SURVEY'] = filtered as UserInputEntry[];
const tripSurveys = resolveSurveyButtonConfig(appConfig, 'trip-label');
tripSurveys.forEach((survey) => {
const filtered = filterByNameAndVersion(survey.surveyName, r, appConfig);
unprocessedLabels[survey.surveyName] = filtered as UserInputEntry[];
});
} else {
unprocessedLabels[getLabelInputs()[i]] = r;
}
Expand Down
41 changes: 20 additions & 21 deletions www/js/survey/enketo/UserInputButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import LabelTabContext from '../../diary/LabelTabContext';
import useAppConfig from '../../useAppConfig';
import { getSurveyForTimelineEntry } from './conditionalSurveys';
import useDerivedProperties from '../../diary/useDerivedProperties';
import { resolveSurveyButtonConfig } from './enketoHelper';
import { SurveyButtonConfig } from '../../types/appConfigTypes';

type Props = {
timelineEntry: any;
Expand All @@ -33,28 +35,25 @@ const UserInputButton = ({ timelineEntry }: Props) => {
const derivedTripProps = useDerivedProperties(timelineEntry);

// which survey will this button launch?
const [surveyName, notFilledInLabel] = useMemo(() => {
if (!appConfig) return []; // no config loaded yet; show blank for now
const tripLabelConfig = appConfig?.survey_info?.buttons?.['trip-label'];
if (!tripLabelConfig) {
// config doesn't specify; use default
return ['TripConfirmSurvey', t('diary.choose-survey')];
}
// config lists one or more surveys; find which one to use
const s = getSurveyForTimelineEntry(tripLabelConfig, timelineEntry, derivedTripProps);
const lang = i18n.resolvedLanguage || 'en';
return [s?.surveyName, s?.['not-filled-in-label'][lang]];
const survey = useMemo<SurveyButtonConfig | null>(() => {
if (!appConfig) return null; // no config loaded yet; show blank for now
const possibleSurveysForButton = resolveSurveyButtonConfig(appConfig, 'trip-label');
// if there is only one survey, no need to check further
if (possibleSurveysForButton.length == 1) return possibleSurveysForButton[0];
// config lists one or more surveys; find which one to use for this timeline entry
return getSurveyForTimelineEntry(possibleSurveysForButton, timelineEntry, derivedTripProps);
}, [appConfig, timelineEntry, i18n.resolvedLanguage]);

// the label resolved from the survey response, or null if there is no response yet
const responseLabel = useMemo<string | undefined>(
() => userInputFor(timelineEntry)?.['SURVEY']?.data.label || undefined,
[userInputFor(timelineEntry)?.['SURVEY']?.data.label],
);
// the label resolved from the survey response, or undefined if there is no response yet
const responseLabel = useMemo<string | undefined>(() => {
if (!survey) return undefined;
return userInputFor(timelineEntry)?.[survey.surveyName]?.data.label || undefined;
}, [survey, userInputFor(timelineEntry)?.[survey?.surveyName || '']?.data.label]);

function launchUserInputSurvey() {
if (!survey) return displayErrorMsg('UserInputButton: no survey to launch');
logDebug('UserInputButton: About to launch survey');
const prevResponse = userInputFor(timelineEntry)?.['SURVEY'];
const prevResponse = userInputFor(timelineEntry)?.[survey.surveyName];
if (prevResponse?.data?.xmlResponse) {
setPrevSurveyResponse(prevResponse.data.xmlResponse);
}
Expand All @@ -65,27 +64,27 @@ const UserInputButton = ({ timelineEntry }: Props) => {
if (result) {
logDebug(`UserInputButton: response was saved, about to addUserInputToEntry;
result = ${JSON.stringify(result)}`);
addUserInputToEntry(timelineEntry._id.$oid, { SURVEY: result }, 'label');
addUserInputToEntry(timelineEntry._id.$oid, { [result.name]: result }, 'label');
} else {
displayErrorMsg('UserInputButton: response was not saved, result=', result);
}
}

if (!surveyName) return <></>; // no survey to launch
if (!survey) return <></>; // no survey to launch
return (
<>
<DiaryButton
// if a response has been been recorded, the button is 'filled in'
fillColor={responseLabel && colors.primary}
onPress={() => launchUserInputSurvey()}>
{responseLabel || notFilledInLabel}
{responseLabel || survey['not-filled-in-label'][i18n.resolvedLanguage || 'en']}
</DiaryButton>

<EnketoModal
visible={modalVisible}
onDismiss={() => setModalVisible(false)}
onResponseSaved={onResponseSaved}
surveyName={surveyName}
surveyName={survey.surveyName}
opts={{ timelineEntry, prefilledSurveyResponse: prevSurveyResponse }}
/>
</>
Expand Down
16 changes: 6 additions & 10 deletions www/js/survey/enketo/conditionalSurveys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,26 +29,22 @@ const scopedEval = (script: string, scope: { [k: string]: any }) =>

// the first survey in the list that passes its condition will be returned
export function getSurveyForTimelineEntry(
tripLabelConfig: SurveyButtonConfig | SurveyButtonConfig[],
possibleSurveys: SurveyButtonConfig[],
tlEntry: TimelineEntry,
derivedProperties: DerivedProperties,
) {
// if only one survey is given, just return it
if (!(tripLabelConfig instanceof Array)) return tripLabelConfig;
if (tripLabelConfig.length == 1) return tripLabelConfig[0];
// else we have an array of possible surveys, we need to find which one to use for this entry
for (let surveyConfig of tripLabelConfig) {
if (!surveyConfig.showsIf) return surveyConfig; // survey shows unconditionally
for (let survey of possibleSurveys) {
if (!survey.showsIf) return survey; // survey shows unconditionally
const scope = {
...tlEntry,
...derivedProperties,
...conditionalSurveyFunctions,
};
try {
const evalResult = scopedEval(surveyConfig.showsIf, scope);
if (evalResult) return surveyConfig;
const evalResult = scopedEval(survey.showsIf, scope);
if (evalResult) return survey;
} catch (e) {
displayError(e, `Error evaluating survey condition "${surveyConfig.showsIf}"`);
displayError(e, `Error evaluating survey condition "${survey.showsIf}"`);
}
}
// TODO if none of the surveys passed conditions?? should we return null, throw error, or return a default?
Expand Down
28 changes: 27 additions & 1 deletion www/js/survey/enketo/enketoHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { getConfig } from '../../config/dynamicConfig';
import { DateTime } from 'luxon';
import { fetchUrlCached } from '../../services/commHelper';
import { getUnifiedDataForInterval } from '../../services/unifiedDataLoader';
import { AppConfig, EnketoSurveyConfig } from '../../types/appConfigTypes';
import { AppConfig, EnketoSurveyConfig, SurveyButtonConfig } from '../../types/appConfigTypes';
import {
CompositeTrip,
ConfirmedPlace,
Expand Down Expand Up @@ -315,6 +315,32 @@ export function loadPreviousResponseForSurvey(dataKey: string) {
);
}

/**
* @description Returns an array of surveys that could be prompted for one button in the UI (trip label, trip notes, place label, or place notes)
* (If multiple are returned, they will show conditionally in the UI based on their `showsIf` field)
* Includes backwards compats for app config fields that didn't use to exist
*/
export function resolveSurveyButtonConfig(
config: AppConfig,
button: 'trip-label' | 'trip-notes' | 'place-label' | 'place-notes',
): SurveyButtonConfig[] {
const buttonConfig = config.survey_info.buttons?.[button];
// backwards compat: default to the trip confirm survey if this button isn't configured
if (!buttonConfig) {
return [
{
surveyName: 'TripConfirmSurvey',
'not-filled-in-label': {
en: 'Add Trip Details',
es: 'Agregar detalles del viaje',
lo: 'ເພີ່ມລາຍລະອຽດການເດີນທາງ',
},
},
];
}
return buttonConfig instanceof Array ? buttonConfig : [buttonConfig];
}

export async function fetchSurvey(url: string) {
const responseText = await fetchUrlCached(url);
if (!responseText) return;
Expand Down
3 changes: 2 additions & 1 deletion www/js/survey/enketo/infinite_scroll_filters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@

import i18next from 'i18next';

const unlabeledCheck = (trip, userInputForTrip) => !userInputForTrip?.['SURVEY'];
const unlabeledCheck = (trip, userInputForTrip) =>
!userInputForTrip || !Object.values(userInputForTrip).some((input) => input);

const TO_LABEL = {
key: 'to_label',
Expand Down
23 changes: 13 additions & 10 deletions www/js/survey/inputMatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
inputType2retKey,
removeManualPrefix,
} from './multilabel/confirmHelper';
import { TimelineLabelMap, TimelineNotesMap } from '../diary/LabelTabContext';
import { TimelineLabelMap, TimelineNotesMap, UserInputMap } from '../diary/LabelTabContext';
import { MultilabelKey } from '../types/labelTypes';
import { EnketoUserInputEntry } from './enketo/enketoHelper';
import { AppConfig } from '../types/appConfigTypes';
Expand Down Expand Up @@ -216,9 +216,8 @@ export function getAdditionsForTimelineEntry(
return [];
}

// get additions that have not been deleted and filter out additions that do not start within the bounds of the timeline entry
const notDeleted = getNotDeletedCandidates(additionsList);
const matchingAdditions = notDeleted.filter((ui) =>
// filter out additions that do not start within the bounds of the timeline entry
const matchingAdditions = additionsList.filter((ui) =>
validUserInputForTimelineEntry(entry, nextEntry, ui, logsEnabled),
);

Expand Down Expand Up @@ -280,29 +279,33 @@ export function mapInputsToTimelineEntries(
allEntries.forEach((tlEntry, i) => {
const nextEntry = i + 1 < allEntries.length ? allEntries[i + 1] : null;
if (appConfig?.survey_info?.['trip-labels'] == 'ENKETO') {
// ENKETO configuration: just look for the 'SURVEY' key in the unprocessedInputs
// ENKETO configuration: consider reponses from all surveys in unprocessedLabels
const userInputForTrip = getUserInputForTimelineEntry(
tlEntry,
nextEntry,
unprocessedLabels['SURVEY'],
Object.values(unprocessedLabels).flat(1),
) as EnketoUserInputEntry;
if (userInputForTrip) {
timelineLabelMap[tlEntry._id.$oid] = { SURVEY: userInputForTrip };
timelineLabelMap[tlEntry._id.$oid] = { [userInputForTrip.data.name]: userInputForTrip };
} else {
let processedSurveyResponse;
let processedSurveyResponse: EnketoUserInputEntry | undefined;
for (const dataKey of keysForLabelInputs(appConfig)) {
const key = removeManualPrefix(dataKey);
if (tlEntry.user_input?.[key]) {
processedSurveyResponse = tlEntry.user_input[key];
break;
}
}
timelineLabelMap[tlEntry._id.$oid] = { SURVEY: processedSurveyResponse };
if (processedSurveyResponse) {
timelineLabelMap[tlEntry._id.$oid] = {
[processedSurveyResponse.data.name]: processedSurveyResponse,
};
}
}
} else {
// MULTILABEL configuration: use the label inputs from the labelOptions to determine which
// keys to look for in the unprocessedInputs
const labelsForTrip: { [k: string]: UserInputEntry | undefined } = {};
const labelsForTrip: UserInputMap = {};
Object.keys(getLabelInputDetails(appConfig)).forEach((label: MultilabelKey) => {
// Check unprocessed labels first since they are more recent
const userInputForTrip = getUserInputForTimelineEntry(
Expand Down
2 changes: 1 addition & 1 deletion www/js/types/appConfigTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export type SurveyButtonConfig = {
'not-filled-in-label': {
[lang: string]: string;
};
showsIf: string; // a JS expression that evaluates to a boolean
showsIf?: string; // a JS expression that evaluates to a boolean
};
export type SurveyButtonsConfig = {
[k in 'trip-label' | 'trip-notes' | 'place-label' | 'place-notes']:
Expand Down

0 comments on commit 4ea0f35

Please sign in to comment.