Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Bugfixes + Refactoring relating to User Inputs #1155

Merged
merged 4 commits into from
May 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 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 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>(() => {

Check warning on line 38 in www/js/survey/enketo/UserInputButton.tsx

View check run for this annotation

Codecov / codecov/patch

www/js/survey/enketo/UserInputButton.tsx#L38

Added line #L38 was not covered by tests
if (!appConfig) return null; // no config loaded yet; show blank for now
const possibleSurveysForButton = resolveSurveyButtonConfig(appConfig, 'trip-label');

Check warning on line 40 in www/js/survey/enketo/UserInputButton.tsx

View check run for this annotation

Codecov / codecov/patch

www/js/survey/enketo/UserInputButton.tsx#L40

Added line #L40 was not covered by tests
// 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);

Check warning on line 44 in www/js/survey/enketo/UserInputButton.tsx

View check run for this annotation

Codecov / codecov/patch

www/js/survey/enketo/UserInputButton.tsx#L44

Added line #L44 was not covered by tests
}, [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>(() => {

Check warning on line 48 in www/js/survey/enketo/UserInputButton.tsx

View check run for this annotation

Codecov / codecov/patch

www/js/survey/enketo/UserInputButton.tsx#L48

Added line #L48 was not covered by tests
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];

Check warning on line 56 in www/js/survey/enketo/UserInputButton.tsx

View check run for this annotation

Codecov / codecov/patch

www/js/survey/enketo/UserInputButton.tsx#L56

Added line #L56 was not covered by tests
if (prevResponse?.data?.xmlResponse) {
setPrevSurveyResponse(prevResponse.data.xmlResponse);
}
Expand All @@ -65,27 +64,27 @@
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');

Check warning on line 67 in www/js/survey/enketo/UserInputButton.tsx

View check run for this annotation

Codecov / codecov/patch

www/js/survey/enketo/UserInputButton.tsx#L67

Added line #L67 was not covered by tests
} 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 @@

// 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) {

Check warning on line 36 in www/js/survey/enketo/conditionalSurveys.ts

View check run for this annotation

Codecov / codecov/patch

www/js/survey/enketo/conditionalSurveys.ts#L36

Added line #L36 was not covered by tests
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);

Check warning on line 44 in www/js/survey/enketo/conditionalSurveys.ts

View check run for this annotation

Codecov / codecov/patch

www/js/survey/enketo/conditionalSurveys.ts#L44

Added line #L44 was not covered by tests
if (evalResult) return survey;
} catch (e) {
displayError(e, `Error evaluating survey condition "${surveyConfig.showsIf}"`);
displayError(e, `Error evaluating survey condition "${survey.showsIf}"`);

Check warning on line 47 in www/js/survey/enketo/conditionalSurveys.ts

View check run for this annotation

Codecov / codecov/patch

www/js/survey/enketo/conditionalSurveys.ts#L47

Added line #L47 was not covered by tests
}
}
// 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) =>

Check warning on line 11 in www/js/survey/enketo/infinite_scroll_filters.ts

View check run for this annotation

Codecov / codecov/patch

www/js/survey/enketo/infinite_scroll_filters.ts#L11

Added line #L11 was not covered by tests
!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
Loading