Fix dangling nulls in list when removing Observation from DiagnosticReport/MultiObservation #324
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.
Fixes the issue in #323 where removing an Observation will leave a null in the list.
The short version of the root cause here is that there is some magic behind the scenes to filter out these nulls, but the function wasn't being invoked properly to trigger that.
Full details for posterity in case this happens again:
The
this.props.onChange
function invoked on the changed line points to this:module-builder/src/containers/Editor.js
Lines 106 to 111 in 270208e
which is a recursive closure, where every time it gets called with a string it creates the next level of the function with a path element appended to it. Path is an array containing the sequence of fields in the JSON to get to a given location. For example
["covid19/measurements_daily", "states.CBC_Differential", "conditional_transition", "[0]"]
(Don't ask whystates.CBC_Differential
is one entry instead of two, I didn't dig that far)Anyway once this function gets called with an object, that object is used to set the value at the given path. If the object has a null then that means "delete".
Ultimately it works its way to the reducer:
module-builder/src/reducers/editor.js
Line 257 in 270208e
And more specifically this section will pick up the parent field, check if it's an array, and if so filter out any "falsy" values such as null.
module-builder/src/reducers/editor.js
Lines 293 to 295 in 270208e
Because the
onChange
function up above was previously being called withobservation.[{i}]
, the "parent" field (at the second-to-last index in the path array) wasn't correctly identified and so it wasn't looking at the observations array to filter out nulls.The fix is to change the call to onChange to call it once for each level of the path, ie,
onChange('observations')(`[${i}]`)
instead ofonChange(`observations.[${i}]`)