Skip to content

Commit cc4d520

Browse files
labkey-nickalabkey-nicka
authored
EditableDetailPanel: compare form values against model with update columns (#1959)
1 parent d209e97 commit cc4d520

8 files changed

Lines changed: 246 additions & 133 deletions

File tree

‎packages/components/package-lock.json‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/components/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@labkey/components",
3-
"version": "7.24.0",
3+
"version": "7.24.1",
44
"description": "Components, models, actions, and utility functions for LabKey applications and pages",
55
"sideEffects": false,
66
"files": [

‎packages/components/releaseNotes/components.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
# @labkey/components
22
Components, models, actions, and utility functions for LabKey applications and pages
33

4+
### version 7.24.1
5+
*Released*: 25 March 2026
6+
- Factor `EditingForm` out of `EditableDetailPanel` and load model with update columns
7+
- Update `extractChanges()` to account for `column.jsonType === 'array'`
8+
- Refactor `arrayEquals` to fix edge cases of array mutation and delimiter collisions
9+
410
### version 7.24.0
511
*Released*: 24 March 2026
612
- SchemaQuery.isEqual: add optional includeViewName argument, defaults to true

‎packages/components/src/internal/components/forms/detail/utils.test.ts‎

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { fromJS } from 'immutable';
1+
import { fromJS, List } from 'immutable';
22

33
import { QueryColumn } from '../../../../public/QueryColumn';
44
import { QueryInfo } from '../../../../public/QueryInfo';
@@ -26,47 +26,55 @@ const COLUMN_FILE_INPUT = new QueryColumn({
2626
inputType: 'file',
2727
jsonType: 'string',
2828
});
29+
const COLUMN_ARRAY_INPUT = new QueryColumn({
30+
fieldKey: 'arrInput',
31+
name: 'arrInput',
32+
fieldKeyArray: ['arrInput'],
33+
inputType: 'text',
34+
jsonType: 'array',
35+
});
2936
const QUERY_INFO = QueryInfo.fromJsonForTests({
3037
name: 'test',
3138
schemaName: 'schema',
3239
columns: {
33-
strInput: COLUMN_STRING_INPUT,
40+
arrInput: COLUMN_ARRAY_INPUT,
3441
dtInput: COLUMN_DATE_INPUT,
3542
fileInput: COLUMN_FILE_INPUT,
43+
strInput: COLUMN_STRING_INPUT,
3644
},
3745
});
3846

3947
describe('extractChanges', () => {
4048
test('file input', () => {
4149
const FILE = new File([], 'file');
4250
const currentData = fromJS({ fileInput: { value: FILE } });
43-
expect(extractChanges(QUERY_INFO, currentData, {}).fileInput).toBe(undefined);
51+
expect(extractChanges(QUERY_INFO, currentData, {}).fileInput).toBeUndefined();
4452
expect(extractChanges(QUERY_INFO, currentData, { fileInput: undefined }).fileInput).toBeUndefined();
4553
expect(extractChanges(QUERY_INFO, currentData, { fileInput: FILE }).fileInput).toBeUndefined();
46-
expect(extractChanges(QUERY_INFO, currentData, { fileInput: null }).fileInput).toBe(null);
54+
expect(extractChanges(QUERY_INFO, currentData, { fileInput: null }).fileInput).toBeNull();
4755
expect(
4856
extractChanges(QUERY_INFO, currentData, { fileInput: new File([], 'fileEdit') }).fileInput
4957
).toBeDefined();
5058
});
5159

5260
test('string input', () => {
5361
const currentData = fromJS({ strInput: { value: 'abc' } });
54-
expect(extractChanges(QUERY_INFO, currentData, {}).strInput).toBe(undefined);
55-
expect(extractChanges(QUERY_INFO, currentData, { strInput: undefined }).strInput).toBe(null);
56-
expect(extractChanges(QUERY_INFO, currentData, { strInput: null }).strInput).toBe(null);
62+
expect(extractChanges(QUERY_INFO, currentData, {}).strInput).toBeUndefined();
63+
expect(extractChanges(QUERY_INFO, currentData, { strInput: undefined }).strInput).toBeNull();
64+
expect(extractChanges(QUERY_INFO, currentData, { strInput: null }).strInput).toBeNull();
5765
expect(extractChanges(QUERY_INFO, currentData, { strInput: '' }).strInput).toBe('');
5866
expect(extractChanges(QUERY_INFO, currentData, { strInput: [] }).strInput).toStrictEqual([]);
59-
expect(extractChanges(QUERY_INFO, currentData, { strInput: 'abc' }).strInput).toBe(undefined);
60-
expect(extractChanges(QUERY_INFO, currentData, { strInput: ' abc ' }).strInput).toBe(undefined);
67+
expect(extractChanges(QUERY_INFO, currentData, { strInput: 'abc' }).strInput).toBeUndefined();
68+
expect(extractChanges(QUERY_INFO, currentData, { strInput: ' abc ' }).strInput).toBeUndefined();
6169
expect(extractChanges(QUERY_INFO, currentData, { strInput: ' abcd ' }).strInput).toBe('abcd');
6270
});
6371

6472
test('date input', () => {
6573
let currentData = fromJS({ dtInput: { value: '2022-08-30 01:02:03' } });
66-
expect(extractChanges(QUERY_INFO, currentData, {}).dtInput).toBe(undefined);
67-
expect(extractChanges(QUERY_INFO, currentData, { dtInput: undefined }).dtInput).toBe(null);
68-
expect(extractChanges(QUERY_INFO, currentData, { dtInput: null }).dtInput).toBe(null);
69-
expect(extractChanges(QUERY_INFO, currentData, { dtInput: '2022-08-30 01:02:03' }).dtInput).toBe(undefined);
74+
expect(extractChanges(QUERY_INFO, currentData, {}).dtInput).toBeUndefined();
75+
expect(extractChanges(QUERY_INFO, currentData, { dtInput: undefined }).dtInput).toBeNull();
76+
expect(extractChanges(QUERY_INFO, currentData, { dtInput: null }).dtInput).toBeNull();
77+
expect(extractChanges(QUERY_INFO, currentData, { dtInput: '2022-08-30 01:02:03' }).dtInput).toBeUndefined();
7078
expect(extractChanges(QUERY_INFO, currentData, { dtInput: '2022-08-30 01:02:04' }).dtInput).toBe(
7179
'2022-08-30 01:02:04'
7280
); // Issue 40139, 52536: date comparison only down to minute precision
@@ -81,10 +89,27 @@ describe('extractChanges', () => {
8189
);
8290

8391
currentData = fromJS({ dtInput: { value: '2022-08-30' } });
84-
expect(extractChanges(QUERY_INFO, currentData, { dtInput: '2022-08-30' }).dtInput).toBe(undefined);
92+
expect(extractChanges(QUERY_INFO, currentData, { dtInput: '2022-08-30' }).dtInput).toBeUndefined();
8593
expect(extractChanges(QUERY_INFO, currentData, { dtInput: '2022-08-31' }).dtInput).toBe('2022-08-31');
8694
expect(extractChanges(QUERY_INFO, currentData, { dtInput: '2022-08-30 01:02:03' }).dtInput).toBe(
8795
'2022-08-30 01:02:03'
8896
);
8997
});
98+
99+
test('array input', () => {
100+
// The existing value is an Immutable List
101+
const currentDataList = fromJS({ arrInput: { value: List([1, 2, 3]) } });
102+
expect(extractChanges(QUERY_INFO, currentDataList, { arrInput: [1, 2, 3] }).arrInput).toBeUndefined();
103+
expect(extractChanges(QUERY_INFO, currentDataList, { arrInput: [1, 2, 4] }).arrInput).toEqual([1, 2, 4]);
104+
expect(extractChanges(QUERY_INFO, currentDataList, { arrInput: [1, 2] }).arrInput).toEqual([1, 2]);
105+
106+
// Existing value is a raw JavaScript array
107+
const currentDataRaw = fromJS({ arrInput: { value: [10, 20] } });
108+
expect(extractChanges(QUERY_INFO, currentDataRaw, { arrInput: [10, 20] }).arrInput).toBeUndefined();
109+
expect(extractChanges(QUERY_INFO, currentDataRaw, { arrInput: [10, 20, 30] }).arrInput).toEqual([10, 20, 30]);
110+
111+
// Nulls and Undefined
112+
expect(extractChanges(QUERY_INFO, currentDataList, { arrInput: null }).arrInput).toBeNull();
113+
expect(extractChanges(QUERY_INFO, currentDataList, { arrInput: undefined }).arrInput).toBeNull();
114+
});
90115
});

‎packages/components/src/internal/components/forms/detail/utils.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { List, Map } from 'immutable';
22
import { Utils } from '@labkey/api';
33

44
import { QueryInfo } from '../../../../public/QueryInfo';
5+
import { isSetEqual } from '../../../util/utils';
56

67
function arrayListIsEqual(valueArr: Array<string | number>, nestedModelList: List<Map<string, any>>): boolean {
78
let matched = 0;
@@ -92,6 +93,14 @@ export function extractChanges(
9293
if (existingValue === newValue) {
9394
return false;
9495
}
96+
} else if (column?.jsonType === 'array') {
97+
if (Array.isArray(newValue)) {
98+
const existingArray = List.isList(existingValue) ? existingValue.toJS() : existingValue;
99+
100+
if (Array.isArray(existingArray) && isSetEqual(newValue, existingArray)) {
101+
return false;
102+
}
103+
}
95104
}
96105

97106
changedValues[col.name] = newValue === undefined ? null : newValue;

‎packages/components/src/internal/util/utils.test.ts‎

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1588,6 +1588,7 @@ describe('arrayEquals', () => {
15881588
test('ignore order, case sensitive', () => {
15891589
expect(arrayEquals(undefined, undefined)).toBeTruthy();
15901590
expect(arrayEquals(undefined, null)).toBeTruthy();
1591+
expect(arrayEquals(null, undefined)).toBeTruthy();
15911592
expect(arrayEquals([], [])).toBeTruthy();
15921593
expect(arrayEquals(null, [])).toBeFalsy();
15931594
expect(arrayEquals(['a'], null)).toBeFalsy();
@@ -1621,38 +1622,61 @@ describe('arrayEquals', () => {
16211622
expect(arrayEquals(['a', 'b'], ['A', 'b'], false)).toBeFalsy();
16221623
expect(arrayEquals(['a', 'b'], ['B', 'A'], false)).toBeFalsy();
16231624
});
1625+
1626+
test('does not mutate original arrays', () => {
1627+
const arrA = ['b', 'a'];
1628+
const arrB = ['a', 'b'];
1629+
arrayEquals(arrA, arrB, true);
1630+
expect(arrA[0]).toBe('b');
1631+
expect(arrB[0]).toBe('a');
1632+
});
1633+
1634+
test('handles delimiter collision', () => {
1635+
expect(arrayEquals(['a;b', 'c'], ['a', 'b;c'])).toBeFalsy();
1636+
});
1637+
1638+
test('handles numeric-string collisions', () => {
1639+
expect(arrayEquals(['1', '23'], ['12', '3'])).toBeFalsy();
1640+
});
1641+
1642+
test('handles duplicate elements correctly with ignoreOrder', () => {
1643+
expect(arrayEquals(['a', 'a', 'b'], ['a', 'b', 'b'], true)).toBeFalsy();
1644+
expect(arrayEquals(['a', 'a', 'b'], ['a', 'b', 'b'], false)).toBeFalsy();
1645+
expect(arrayEquals(['a', 'a', 'b'], ['a', 'b', 'a'], true)).toBeTruthy();
1646+
expect(arrayEquals(['a', 'a', 'b'], ['a', 'b', 'a'], false)).toBeFalsy();
1647+
});
16241648
});
16251649

16261650
describe('getValueFromRow', () => {
16271651
test('no row', () => {
1628-
expect(getValueFromRow(undefined, 'Name')).toEqual(undefined);
1629-
expect(getValueFromRow({}, 'Name')).toEqual(undefined);
1652+
expect(getValueFromRow(undefined, 'Name')).toBeUndefined();
1653+
expect(getValueFromRow({}, 'Name')).toBeUndefined();
16301654
});
16311655

16321656
test('returns value', () => {
16331657
const row = { Name: 'test' };
16341658
expect(getValueFromRow(row, 'Name')).toEqual('test');
16351659
expect(getValueFromRow(row, 'name')).toEqual('test');
1636-
expect(getValueFromRow(row, 'bogus')).toEqual(undefined);
1660+
expect(getValueFromRow(row, 'bogus')).toBeUndefined();
16371661
});
16381662

16391663
test('returns value from object', () => {
16401664
const row = { Name: { value: 'test' } };
16411665
expect(getValueFromRow(row, 'Name')).toEqual('test');
16421666
expect(getValueFromRow(row, 'name')).toEqual('test');
1643-
expect(getValueFromRow(row, 'bogus')).toEqual(undefined);
1667+
expect(getValueFromRow(row, 'bogus')).toBeUndefined();
16441668
});
16451669

16461670
test('returns value from array', () => {
16471671
const flatRow = { Name: ['test1', 'test2'] };
1648-
expect(getValueFromRow(flatRow, 'Name')).toEqual(undefined);
1649-
expect(getValueFromRow(flatRow, 'name')).toEqual(undefined);
1650-
expect(getValueFromRow(flatRow, 'bogus')).toEqual(undefined);
1672+
expect(getValueFromRow(flatRow, 'Name')).toBeUndefined();
1673+
expect(getValueFromRow(flatRow, 'name')).toBeUndefined();
1674+
expect(getValueFromRow(flatRow, 'bogus')).toBeUndefined();
16511675

16521676
const nestedRow = { Name: [{ value: 'test1' }, { value: 'test2' }] };
16531677
expect(getValueFromRow(nestedRow, 'Name')).toEqual('test1');
16541678
expect(getValueFromRow(nestedRow, 'name')).toEqual('test1');
1655-
expect(getValueFromRow(nestedRow, 'bogus')).toEqual(undefined);
1679+
expect(getValueFromRow(nestedRow, 'bogus')).toBeUndefined();
16561680
});
16571681
});
16581682

‎packages/components/src/internal/util/utils.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -748,16 +748,27 @@ export function isQuotedWithDelimiters(value: any, delimiter: string): boolean {
748748
return strVal.startsWith('"') && strVal.endsWith('"');
749749
}
750750

751-
export function arrayEquals(a: string[], b: string[], ignoreOrder = true, caseInsensitive?: boolean): boolean {
751+
export function arrayEquals(
752+
a: null | string[] | undefined,
753+
b: null | string[] | undefined,
754+
ignoreOrder = true,
755+
caseInsensitive = false
756+
): boolean {
752757
if (a === b) return true;
753758
if (a == null && b == null) return true;
754759
if (a == null || b == null) return false;
755760
if (a.length !== b.length) return false;
756761

757-
const aStr = ignoreOrder ? a.sort().join(';') : a.join(';');
758-
const bStr = ignoreOrder ? b.sort().join(';') : b.join(';');
762+
const normalize = (s: string) => (caseInsensitive ? s.toLowerCase() : s);
759763

760-
return caseInsensitive ? aStr.toLowerCase() === bStr.toLowerCase() : aStr === bStr;
764+
if (ignoreOrder) {
765+
// Use a copy to avoid mutating the original arrays
766+
const aSorted = [...a].map(normalize).sort();
767+
const bSorted = [...b].map(normalize).sort();
768+
return aSorted.every((val, index) => val === bSorted[index]);
769+
}
770+
771+
return a.every((val, index) => normalize(val) === normalize(b[index]));
761772
}
762773

763774
export function getValueFromRow(row: Record<string, any>, col: string): number | string {

0 commit comments

Comments
 (0)