module-builder
module-builder copied to clipboard
Fix dangling nulls in list when removing Observation from DiagnosticReport/MultiObservation
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:
https://github.com/synthetichealth/module-builder/blob/270208ea7f99c3edc8a785d1163565b24bc44f14/src/containers/Editor.js#L106-L111
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 why states.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: https://github.com/synthetichealth/module-builder/blob/270208ea7f99c3edc8a785d1163565b24bc44f14/src/reducers/editor.js#L257
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. https://github.com/synthetichealth/module-builder/blob/270208ea7f99c3edc8a785d1163565b24bc44f14/src/reducers/editor.js#L293-L295
Because the onChange
function up above was previously being called with observation.[{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 of onChange(`observations.[${i}]`)