From 79656d8bc1fe74fb06ebe8b4fdd0680d217f90ae Mon Sep 17 00:00:00 2001 From: "K. Shankari" Date: Sun, 5 May 2024 02:47:12 -0700 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=9A=91=EF=B8=8F=20=F0=9F=90=9B=20Ensu?= =?UTF-8?q?re=20that=20we=20do=20show=20the=20replaced=20mode=20when=20the?= =?UTF-8?q?=20`mode=5Fof=5Finterest`=20matches?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At some point, we changed the mode and purpose user inputs as objects that were stored to be full objects, with start and end timestamps, instead of just the labels. We changed all uses of the MODE and PURPOSE to match it, but apparently forgot to change this location, where the replaced mode button is conditionally displayed. This is a quick change to make the usage here consistent with the rest of the code so that we can push this out ASAP. @JGreenlee, please let me know if there is a more principled fix, it is late and I don't want to experiment any further. Testing done: Please see PR --- www/js/survey/multilabel/confirmHelper.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/www/js/survey/multilabel/confirmHelper.ts b/www/js/survey/multilabel/confirmHelper.ts index 58980f3c0..cf542ec62 100644 --- a/www/js/survey/multilabel/confirmHelper.ts +++ b/www/js/survey/multilabel/confirmHelper.ts @@ -91,12 +91,12 @@ export function getLabelInputDetails(appConfigParam?) { export function labelInputDetailsForTrip(userInputForTrip, appConfigParam?) { if (appConfigParam) appConfig = appConfigParam; if (appConfig.intro.mode_studied) { - if (userInputForTrip?.['MODE']?.value == appConfig.intro.mode_studied) { - logDebug(`Found trip labeled with mode of study, ${appConfig.intro.mode_studied}. + if (userInputForTrip?.['MODE']?.data.label == appConfig.intro.mode_studied) { + logDebug(`Found trip labeled with ${userInputForTrip?.['MODE']?.data.label}, mode of study = ${appConfig.intro.mode_studied}. Needs REPLACED_MODE`); return getLabelInputDetails(); } else { - logDebug(`Found trip not labeled with mode of study, ${appConfig.intro.mode_studied}. + logDebug(`Found trip labeled with ${userInputForTrip?.['MODE']?.data.label}, not labeled with mode of study = ${appConfig.intro.mode_studied}. Doesn't need REPLACED_MODE`); return baseLabelInputDetails; } From ed61b8d4da5c442e6acd5d2c40a50b9a455e9e10 Mon Sep 17 00:00:00 2001 From: "K. Shankari" Date: Sun, 5 May 2024 10:18:20 -0700 Subject: [PATCH 2/4] Handle case where there is no input through the conditional access --- www/js/survey/multilabel/confirmHelper.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/www/js/survey/multilabel/confirmHelper.ts b/www/js/survey/multilabel/confirmHelper.ts index cf542ec62..f94032f3e 100644 --- a/www/js/survey/multilabel/confirmHelper.ts +++ b/www/js/survey/multilabel/confirmHelper.ts @@ -91,12 +91,12 @@ export function getLabelInputDetails(appConfigParam?) { export function labelInputDetailsForTrip(userInputForTrip, appConfigParam?) { if (appConfigParam) appConfig = appConfigParam; if (appConfig.intro.mode_studied) { - if (userInputForTrip?.['MODE']?.data.label == appConfig.intro.mode_studied) { - logDebug(`Found trip labeled with ${userInputForTrip?.['MODE']?.data.label}, mode of study = ${appConfig.intro.mode_studied}. + if (userInputForTrip?.['MODE']?.data?.label == appConfig.intro.mode_studied) { + logDebug(`Found trip labeled with ${userInputForTrip?.['MODE']?.data?.label}, mode of study = ${appConfig.intro.mode_studied}. Needs REPLACED_MODE`); return getLabelInputDetails(); } else { - logDebug(`Found trip labeled with ${userInputForTrip?.['MODE']?.data.label}, not labeled with mode of study = ${appConfig.intro.mode_studied}. + logDebug(`Found trip labeled with ${userInputForTrip?.['MODE']?.data?.label}, not labeled with mode of study = ${appConfig.intro.mode_studied}. Doesn't need REPLACED_MODE`); return baseLabelInputDetails; } From 8b26d2d90b26a60265e7e1f1252a9944555993a7 Mon Sep 17 00:00:00 2001 From: "K. Shankari" Date: Sun, 5 May 2024 11:09:14 -0700 Subject: [PATCH 3/4] =?UTF-8?q?=E2=9C=85=20=F0=9F=90=9B=20Improve=20user?= =?UTF-8?q?=20input=20matching=20mocks=20and=20use=20them?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous unit tests for the input matching assumed that the user input would be of the form of the label options, so reused the label options as mock options. However, they actually are of the form of user input objects with data and metadata and the input as a label. We create new mock objects with the correct format, and use them in the tests. This is the reason why https://github.com/e-mission/e-mission-phone/pull/1150 was not caught for so long. And when we fixed the code, the test broke. https://github.com/e-mission/e-mission-phone/pull/1150#issuecomment-2094892441 https://github.com/e-mission/e-mission-phone/pull/1150#issuecomment-2094895504 Testing done: ``` npx jest www/__tests__/confirmHelper.test.ts Test Suites: 1 passed, 1 total Tests: 11 passed, 11 total ``` --- www/__tests__/confirmHelper.test.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/www/__tests__/confirmHelper.test.ts b/www/__tests__/confirmHelper.test.ts index 52cb9c0e8..1e24801f1 100644 --- a/www/__tests__/confirmHelper.test.ts +++ b/www/__tests__/confirmHelper.test.ts @@ -45,6 +45,16 @@ const fakeDefaultLabelOptions = { }, }, }; +const fakeInputs = { + MODE: [ + { data: { label: 'walk', start_ts: 1245, end_ts: 5678}}, + { data: { label: 'bike', start_ts: 1245, end_ts: 5678}}, + ], + PURPOSE: [ + { data: {label: 'home', start_ts: 1245, end_ts: 5678 }}, + { data: {label: 'work', start_ts: 1245, end_ts: 5678 }} + ], +}; jest.mock('../js/services/commHelper', () => ({ ...jest.requireActual('../js/services/commHelper'), @@ -62,8 +72,8 @@ describe('confirmHelper', () => { it('returns base labelInputDetails for a labelUserInput which does not have mode of study', () => { const fakeLabelUserInput = { - MODE: fakeDefaultLabelOptions.MODE[1], - PURPOSE: fakeDefaultLabelOptions.PURPOSE[0], + MODE: fakeInputs.MODE[1], + PURPOSE: fakeInputs.PURPOSE[0], }; const labelInputDetails = labelInputDetailsForTrip( fakeLabelUserInput, @@ -74,8 +84,8 @@ describe('confirmHelper', () => { it('returns full labelInputDetails for a labelUserInput which has the mode of study', () => { const fakeLabelUserInput = { - MODE: fakeDefaultLabelOptions.MODE[0], // 'walk' is mode of study - PURPOSE: fakeDefaultLabelOptions.PURPOSE[0], + MODE: fakeInputs.MODE[0], // 'walk' is mode of study + PURPOSE: fakeInputs.PURPOSE[0], }; const labelInputDetails = labelInputDetailsForTrip( fakeLabelUserInput, From 20b9e1aa5c0fe99a8d57cfea4b3d0fa6d7c6881b Mon Sep 17 00:00:00 2001 From: "K. Shankari" Date: Sun, 5 May 2024 11:19:51 -0700 Subject: [PATCH 4/4] Fix formatting --- www/__tests__/confirmHelper.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/www/__tests__/confirmHelper.test.ts b/www/__tests__/confirmHelper.test.ts index 1e24801f1..b95b10372 100644 --- a/www/__tests__/confirmHelper.test.ts +++ b/www/__tests__/confirmHelper.test.ts @@ -47,12 +47,12 @@ const fakeDefaultLabelOptions = { }; const fakeInputs = { MODE: [ - { data: { label: 'walk', start_ts: 1245, end_ts: 5678}}, - { data: { label: 'bike', start_ts: 1245, end_ts: 5678}}, + { data: { label: 'walk', start_ts: 1245, end_ts: 5678 } }, + { data: { label: 'bike', start_ts: 1245, end_ts: 5678 } }, ], PURPOSE: [ - { data: {label: 'home', start_ts: 1245, end_ts: 5678 }}, - { data: {label: 'work', start_ts: 1245, end_ts: 5678 }} + { data: { label: 'home', start_ts: 1245, end_ts: 5678 } }, + { data: { label: 'work', start_ts: 1245, end_ts: 5678 } }, ], };