🐛 Bugfixes + Refactoring relating to User Inputs #1155
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Part of the initiative described in e-mission/e-mission-docs#1045 This implements the "proposed" structure on the phone for Enketo user inputs. When unprocessed user inputs are read and stored into unprocessedLabels in timelineHelper, responses will now be sorted by survey name instead of all being kept in one slot 'SURVEY'. Also implement a new function, enketoHelper > resolveSurveyButtonConfig, which reads the config and determines what survey(s) are available for a button (like 'trip label', 'trip addition', 'place label', 'place addition'). Includes backwards compat so old configs (which don't have those fields) will just use the default 'TripConfirmSurvey'. Including the translations for TripConfirmSurvey's 'not-filled-in-label' directly in the backwards compat to be consistent with new-style surveys where the translations are directly in the config. This new function is used by <UserInputButton>, simplifying the code that was there previously. It now stores the returned SurveyButtonConfig object as 'survey' rather than keeping 'surveyName' and 'notFilledInLabel' separately. This allows conditionalSurveys > getSurveyForTimelineEntry to be simplified too. The new function is also used in timelineHelper > updateUnprocessedInputs. Instead of calling filterByNameAndVersion only for 'TripConfirmSurvey' and then storing those under the key of 'SURVEY', it first calls `resolveSurveyButtonConfig` to get *all* possible 'trip-label' surveys, iterates over those surveys calling filterByNameAndVersion for each, and stores unprocessed responses using the name of the survey as the key. See types updated in LabelTabContext.ts. appConfigTypes.ts updated to reflect that showsIf is optional (if not present, that survey shows unconditionally)
I found an edge case where if an old addition (ie one that was already processed) was edited, there would be duplicates; both the old and new version would show up (until the new one got processed). This is because we were first applying the 'not deleted' filter to unprocessed entries. Then after they are merged with processed entreis we applied it to the merged copy. This didn't work for the edge case because the DELETED entry was already filtered out the first time, so the processed original entry just sticks around. This fix ensures that the 'not deleted' filter ONLY happens once processed+unprocessed are merged together, this way the filter considers all entries at once
02655ba changed from using 'SURVEY' as the key for unprocessed user inputs to using the name of the survey These tests still expected 'SURVEY'; replaced with the survey name. The existing test only had 'TripConfirmSurvey'; I also changed one to 'MyCustomSurvey' to highlight that they will be stored under separate fields now. inputMatcher.test.ts tests on both MULTILABEL and ENKETO configurations. It is necessary to call updateUnprocessedInputs after testing MULTILABEL and before testing ENKETO so that unprocessed MULTILABEL inputs are not kept it memory. It is also necessary for updateUnprocessedInputs to clear unprocessedLabels before re-filling it; otherwise survey responses could linger from a previous time the function was called. Previously all responses were kept in SURVEY, so SURVEY would always get reassigned. but now each survey's responses are kept under the survey name as a key
Trip and place surveys give their own button text in the config now, so this text + translations relocated to the backwards-compat in enketoHelper > resolveSurveyButtonConfig
|
This fixes 2 bugs relating to unprocessed inputs:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1155 +/- ##
==========================================
+ Coverage 26.40% 26.57% +0.16%
==========================================
Files 114 114
Lines 4983 4993 +10
Branches 1064 1033 -31
==========================================
+ Hits 1316 1327 +11
- Misses 3665 3666 +1
+ Partials 2 0 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
shankari
approved these changes
May 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refactor ENKETO userinputs to use survey name as key instead of 'SURVEY'
Part of the initiative described in e-mission/e-mission-docs#1045
This implements the "proposed" structure on the phone for Enketo user inputs.
When unprocessed user inputs are read and stored into unprocessedLabels in timelineHelper, responses will now be sorted by survey name instead of all being kept in one slot 'SURVEY'.
Also implement a new function, enketoHelper > resolveSurveyButtonConfig, which reads the config and determines what survey(s) are available for a button (like 'trip label', 'trip addition', 'place label', 'place addition').
Includes backwards compat so old configs (which don't have those fields) will just use the default 'TripConfirmSurvey'.
Including the translations for TripConfirmSurvey's 'not-filled-in-label' directly in the backwards compat to be consistent with new-style surveys where the translations are directly in the config.
This new function is used by , simplifying the code that was there previously. It now stores the returned SurveyButtonConfig object as 'survey' rather than keeping 'surveyName' and 'notFilledInLabel' separately.
This allows conditionalSurveys > getSurveyForTimelineEntry to be simplified too.
The new function is also used in timelineHelper > updateUnprocessedInputs. Instead of calling filterByNameAndVersion only for 'TripConfirmSurvey' and then storing those under the key of 'SURVEY', it first calls
resolveSurveyButtonConfigto get all possible 'trip-label' surveys, iterates over those surveys calling filterByNameAndVersion for each, and stores unprocessed responses using the name of the survey as the key.See types updated in LabelTabContext.ts.
appConfigTypes.ts updated to reflect that showsIf is optional (if not present, that survey shows unconditionally)
don't filter deleted additions twice
I found an edge case where if an old addition (ie one that was already processed) was edited, there would be duplicates; both the old and new version would show up (until the new one got processed).
This is because we were first applying the 'not deleted' filter to unprocessed entries. Then after they are merged with processed entreis we applied it to the merged copy. This didn't work for the edge case because the DELETED entry was already filtered out the first time, so the processed original entry just sticks around.
This fix ensures that the 'not deleted' filter ONLY happens once processed+unprocessed are merged together, this way the filter considers all entries at once