Instrument record PATCH silently discards arrays supplied for existing array fields #1639

PATCH /v1/instrument-records/:id silently discards any array the request supplies for a field that already holds an array. The mergeWith customizer in updateById returns the existing (destination) value whenever the source value is an array, so the stored array never changes. The request still returns 200 with the record, so the client is told the edit succeeded when it was not applied. This affects record-array fields and any other array-valued data. An array for a field that is currently absent is written, because the customizer then returns undefined and lodash falls back to its default merge.

Where

apps/api/src/instrument-records/instrument-records.service.ts:381-386

const updatedData = mergeWith(instrumentRecord.data, data, (updatedValue: unknown, sourceValue: unknown) => {
  if (Array.isArray(sourceValue)) {
    return updatedValue; // the existing value, not the submitted one
  }
  return undefined;
});

Reproduce

Using the same customizer with lodash-es 4.18.1:

• existing { meds: [{ name: 'a' }, { name: 'b' }], x: 1 }, update { meds: [{ name: 'c' }], x: 2 } → { meds: [{ name: 'a' }, { name: 'b' }], x: 2 }
• Expected: { meds: [{ name: 'c' }], x: 2 }

Through the API: PATCH a record whose data has answers: [1, 2, 3] with { "answers": [9] }. The response is 200 and the stored value is still [1, 2, 3].

Tests

apps/api/src/instrument-records/__tests__/instrument-records.service.spec.ts, test should ignore an array the update supplies for an existing array field (line 1150, with a "Pins a reported defect" comment). When fixing, invert it: expect data: { answers: [9] } and rename it (for example, "should replace an existing array field with the array the update supplies"). The neighbouring test should merge an array update element by element into array data covers records whose root data is an array. The customizer is not called for the root, so that behaviour is unaffected.

Suggested fix

Return sourceValue instead of updatedValue from the customizer, so a submitted array replaces the stored one rather than being merged index by index or dropped.

Found while raising unit test coverage in #1632.