ERA-13853: Choice list fields ignore legacy {name, value} event detail values - #1702
ERA-13853: Choice list fields ignore legacy {name, value} event detail values#1702luixlive wants to merge 5 commits into
Conversation
…ist values - Introduced `normalizeChoiceListValues` utility to handle legacy choice list formats in event details. - Updated `EventFormSummary` to utilize normalized form data for version 2 schema. - Modified `DetailsSection` and `ReportDetailView` to pass normalized event details. - Added tests for `normalizeChoiceListValues` to ensure correct transformation of legacy data structures.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a normalization layer for v2 choice-list fields so legacy { name, value } objects (and other historical shapes) are converted into the string/array values expected by current schema-driven forms and save payloads.
Changes:
- Introduces
normalizeChoiceListValuesutility + unit tests to repair legacy choice-list shapes based on JSON Schema. - Uses normalized
eventDetailswhen rendering v2 schema forms and when building save payloads. - Updates/extends UI tests to cover rendering + saving of legacy v2 choice list values.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/form-schemas/normalizeChoiceListValues/index.js | Adds the normalization logic for legacy choice-list values based on schema shape. |
| src/utils/form-schemas/normalizeChoiceListValues/index.test.js | Adds unit tests covering multiple/single choice lists, conditional sections, collections, and no-op behavior. |
| src/ReportManager/ReportDetailView/index.js | Normalizes event_details before passing to v2 SchemaForm and before saving. |
| src/ReportManager/ReportDetailView/index.test.js | Adds an integration test verifying legacy { name, value } choice list values render checked and save as strings. |
| src/ReportManager/DetailsSection/index.js | Accepts eventDetails prop and passes it to the v2 SchemaForm. |
| src/ReportManager/DetailsSection/index.test.js | Updates test setup to pass eventDetails into DetailsSection. |
| src/ReportFormSummary/index.js | Normalizes v2 summary formData before rendering the v2 summary component. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…r components - Updated `normalizeChoiceListValues` to accommodate both legacy and current choice list formats. - Modified `EventFormSummary` and `V1SchemaFormSummary` to utilize normalized form data. - Adjusted `DetailsSection` and `ReportDetailView` to ensure consistent event details processing. - Enhanced tests for `normalizeChoiceListValues` to validate handling of various choice list scenarios.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/utils/form-schemas/normalizeChoiceListValues/index.js:35
- For multiple-choice lists, a cleared value can plausibly show up as an empty string (
'') or a legacy{ name, value: '' }object. With the current logic,normalizedValue === ''causes this branch to return the originalvalue(which can be a legacy object or the wrong type for an array field), leaving the data un-normalized. Consider explicitly handlingtypeof normalizedValue === 'string'and returning[]when it’s empty (or otherwise ensuring the output matches the schema’s array type).
const normalizedValue = normalizeChoiceListValue(value);
return normalizedValue && typeof normalizedValue === 'string' ? [normalizedValue] : value;
src/utils/form-schemas/normalizeChoiceListValues/index.js:57
- This allocates an intermediate array via
.map(...)even though you only need the first match. Consider switching to a simple loop (or.find(...)directly overallOf) to short-circuit without creating an extra array; it keeps the intent the same while reducing allocations on large schemas.
const getFieldJSONSubschema = (jsonSchema, fieldName) => jsonSchema.properties?.[fieldName] ?? (jsonSchema.allOf ?? [])
.map((conditionalSectionJSONSubschema) => conditionalSectionJSONSubschema.then?.properties?.[fieldName])
.find(Boolean);
src/ReportManager/ReportDetailView/index.js:178
- The memo dependency uses the entire
eventSchemaobject, which can cause unnecessary recomputation if unrelated parts of the selected schema wrapper change identity. Since normalization only depends oneventSchema?.json, consider depending oneventSchema?.json(or a stable identifier for it) rather thaneventSchemato make the memo more effective.
const eventDetails = useMemo(
() => normalizeChoiceListValues(reportForm?.event_details, eventSchema?.json),
[eventSchema, reportForm?.event_details]
);
…r components - Simplified the normalization of choice list values across various components. - Updated `EventFormSummary`, `V1SchemaFormSummary`, and `V2SchemaFormSummary` to utilize normalized form data. - Adjusted `DetailsSection` and `ReportDetailView` to consistently process event details. - Enhanced tests for `normalizeChoiceListValues` to validate handling of legacy and current choice list formats.
…d components - Updated `SchemaForm` to consistently use normalized form data for validation and state management. - Simplified handling of legacy choice list formats across `ReportDetailView` and `V2SchemaFormSummary`. - Enhanced `normalizeChoiceListValues` utility to improve data integrity and processing efficiency. - Adjusted related components to ensure seamless integration of normalized data throughout the application.
- Updated `normalizeChoiceListValues` to handle both legacy and current choice list formats more effectively. - Modified `V2SchemaFormSummary`, `ReportDetailView`, and `SchemaForm` to utilize the enhanced normalization logic. - Improved integration of normalized data in event details processing throughout the application. - Expanded tests for `normalizeChoiceListValues` to ensure comprehensive coverage of various scenarios.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/utils/form-schemas/normalizeChoiceListValues/index.js:20
- The change-detection logic uses !== to decide whether to preserve references. This breaks for values like NaN (since NaN !== NaN is always true), causing the function to treat unchanged data as changed and unnecessarily allocate new arrays/objects. To keep the intended “preserve reference when unchanged” behavior, use Object.is(...) for comparisons in both normalizeArrayItems and normalizeObjectValues.
const normalizeArrayItems = (array, normalizeItem) => {
const normalizedArray = array.map(normalizeItem);
return normalizedArray.some((item, index) => item !== array[index]) ? normalizedArray : array;
};
const normalizeObjectValues = (object, normalizeValue) => {
const normalizedEntries = Object.entries(object).map(([key, value]) => [key, normalizeValue(value, key)]);
return normalizedEntries.some(([key, value]) => value !== object[key])
? Object.fromEntries(normalizedEntries)
: object;
};
src/utils/form-schemas/normalizeChoiceListValues/index.js:49
- The comment understates current behavior: the implementation also normalizes arrays (multi-select) and recursively normalizes choice lists inside COLLECTION items, while preserving references when no changes are needed. Please update this comment to reflect the actual supported shapes and recursion so callers understand the scope/guarantees.
// Replaces the values of the choice list fields described by formElements from
// { name, value } format to their value.
const normalizeChoiceListValues = (formData, formElements) => isPlainObject(formElements)
? normalizeFieldValues(formData, formElements)
: formData;
🚀 PR Environment Deployed
Access: https://fix-normalize-legacy-choice-lists.dev.pamdas.org |
No description provided.