From a05726e95c0aab169f5438b1f0b53609157ebfcd Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 31 Mar 2026 20:09:01 -0700 Subject: [PATCH 1/8] GitHub Issue 928, 951, 970 & 987 --- packages/components/package-lock.json | 4 ++-- packages/components/package.json | 2 +- packages/components/releaseNotes/components.md | 7 +++++++ packages/components/src/index.ts | 2 ++ .../components/domainproperties/models.tsx | 4 +++- .../components/editable/EditableGrid.tsx | 4 +++- .../src/internal/components/editable/actions.ts | 12 ++++++++++-- .../src/internal/components/editable/utils.ts | 16 ++++++++++++++++ .../src/internal/components/entities/actions.ts | 11 ++++++++++- .../internal/components/forms/QuerySelect.tsx | 5 ++++- .../src/public/QueryModel/GridFilterModal.tsx | 15 ++++++++++++++- 11 files changed, 72 insertions(+), 10 deletions(-) diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index a7e7074896..8a0fb6062e 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,12 +1,12 @@ { "name": "@labkey/components", - "version": "7.26.4", + "version": "7.26.5-fb-mvtcBatch3.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "7.26.4", + "version": "7.26.5-fb-mvtcBatch3.0", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/packages/components/package.json b/packages/components/package.json index f7f16117ca..069062f59b 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "7.26.4", + "version": "7.26.5-fb-mvtcBatch3.0", "description": "Components, models, actions, and utility functions for LabKey applications and pages", "sideEffects": false, "files": [ diff --git a/packages/components/releaseNotes/components.md b/packages/components/releaseNotes/components.md index c5c21c5727..80cee6fa1f 100644 --- a/packages/components/releaseNotes/components.md +++ b/packages/components/releaseNotes/components.md @@ -1,6 +1,13 @@ # @labkey/components Components, models, actions, and utility functions for LabKey applications and pages +### version 7.X +*Released*: X 2026 +- GitHub Issue 928: Spaces not shown between text choices in identifying fields in editable grid +- GitHub Issue 951: Multi-line values converted to text choices lose multi-line editability +- GitHub Issue 970: Warn about more than 10 items for multi choice fields in editable grid. +- GitHub Issue 987: Multi value filter dialog lets you edit and save without any selected values + ### version 7.26.4 *Released*: 31 March 2026 - Update `@labkey/api` dependency diff --git a/packages/components/src/index.ts b/packages/components/src/index.ts index 0b5e09369d..6cd5370e14 100644 --- a/packages/components/src/index.ts +++ b/packages/components/src/index.ts @@ -491,6 +491,7 @@ import { getDataDeleteConfirmationData, getEntityTypeOptions, getExcludedDataTypeNames, + getIdentifyingFieldDisplayValue, getOrderedSelectedMappedKeysFromQueryModel, getParentTypeDataForLineage, getSampleIdentifyingFieldGridData, @@ -1352,6 +1353,7 @@ export { getEntityTypeOptions, getEventDataValueDisplay, getExcludedDataTypeNames, + getIdentifyingFieldDisplayValue, getExpandQueryInfo, getFieldFiltersValidationResult, getFilterForSampleOperation, diff --git a/packages/components/src/internal/components/domainproperties/models.tsx b/packages/components/src/internal/components/domainproperties/models.tsx index cce6f2f54b..4315be89f4 100644 --- a/packages/components/src/internal/components/domainproperties/models.tsx +++ b/packages/components/src/internal/components/domainproperties/models.tsx @@ -57,6 +57,7 @@ import { LOOKUP_VALIDATOR_VALUES, MAX_TEXT_LENGTH, MULTI_CHOICE_RANGE_URI, + MULTILINE_RANGE_URI, NUMBER_CONVERT_URIS, PHILEVEL_NOT_PHI, SAMPLE_TYPE_CONCEPT_URI, @@ -1778,7 +1779,8 @@ export function acceptablePropertyType(newType: PropDescType, originalRangeURI: // Original field is a uniqueId, text, or multi-line text, can convert to a string type if (newType.isString() && PropDescType.isString(originalRangeURI)) { - return true; + // GitHub Issue 951: Multi-line values converted to text choices lose multi-line editability + return !(MULTILINE_RANGE_URI === originalRangeURI && newType.isTextChoice()); } // Original field is a datetime, can convert to a date or time diff --git a/packages/components/src/internal/components/editable/EditableGrid.tsx b/packages/components/src/internal/components/editable/EditableGrid.tsx index b72759639b..241a67c06b 100644 --- a/packages/components/src/internal/components/editable/EditableGrid.tsx +++ b/packages/components/src/internal/components/editable/EditableGrid.tsx @@ -703,7 +703,9 @@ export class EditableGrid extends PureComponent = editorModel.getIn(keyPath); diff --git a/packages/components/src/internal/components/editable/actions.ts b/packages/components/src/internal/components/editable/actions.ts index 581c245866..839a478d09 100644 --- a/packages/components/src/internal/components/editable/actions.ts +++ b/packages/components/src/internal/components/editable/actions.ts @@ -504,6 +504,11 @@ async function convertRowToEditorModelData( } } else if (col.isMultiChoice && Array.isArray(data)) { const values = data.filter(item => !!item).map(item => ({ raw: item, display: item })); + if (values.length > 10) { + message = { + message: 'Too many values. Maximum allowed is 10.', + } + } valueDescriptors.push(...values); } else { let display = data; @@ -531,8 +536,7 @@ async function prepareInsertRowDataFromBulkForm( const col = insertColumns[colIdx]; const { message, valueDescriptors } = await convertRowToEditorModelData(data, col, containerPath); values = values.push(valueDescriptors); - - if (message) messages = messages.push(message); + messages = messages.push(message); } return { @@ -1564,6 +1568,10 @@ async function insertPastedData( .join(', '); msg = { message: `Duplicate values not allowed: ${valueStr}.` }; } + if (values.length > 10) { + // GitHub Issue 970 + msg = { message: 'Too many values. Maximum allowed is 10.' }; + } cv = List(values); } else { const { message, value } = getValidatedEditableGridValue(val, col); diff --git a/packages/components/src/internal/components/editable/utils.ts b/packages/components/src/internal/components/editable/utils.ts index 1decc9321e..8d7a32f07c 100644 --- a/packages/components/src/internal/components/editable/utils.ts +++ b/packages/components/src/internal/components/editable/utils.ts @@ -30,6 +30,7 @@ interface ValidatedValue { export const getValidatedEditableGridValue = (origValue: any, col: QueryColumn): ValidatedValue => { // col ?? {} so it's safe to destructure const { caption, isDateOnlyColumn, jsonType, required, scale, validValues } = col ?? {}; + const isMultiChoice = col.isMultiChoice; const isDateTimeType = jsonType === 'date'; const isDateType = isDateTimeType && isDateOnlyColumn; let message; @@ -50,6 +51,21 @@ export const getValidatedEditableGridValue = (origValue: any, col: QueryColumn): message = `Invalid ${noun}, use format ${dateFormat}`; } value = dateStrVal ?? origValue; + } else if (isMultiChoice && Array.isArray(origValue)) { + if (origValue.length > 10) { + // GitHub Issue 970 + message = 'Too many values. Maximum allowed is 10.'; + } + else if (validValues) { + origValue.forEach(val => { + const trimmed = val.display?.toString().trim(); + if (validValues.indexOf(trimmed) === -1) { + message = `'${trimmed}' is not a valid choice`; + return false; + } + }) + } + } else if (value != null && value !== '' && !col?.isPublicLookup()) { if (validValues) { const trimmed = origValue?.toString().trim(); diff --git a/packages/components/src/internal/components/entities/actions.ts b/packages/components/src/internal/components/entities/actions.ts index 3fddfead08..5c3606635a 100644 --- a/packages/components/src/internal/components/entities/actions.ts +++ b/packages/components/src/internal/components/entities/actions.ts @@ -1363,6 +1363,15 @@ export function getSingleSampleTypeQueryInfo(sampleIds: number[] | string[]): Pr }); }); } + +// GitHub Issue 928: Spaces not shown between text choices in identifying fields in editable grid +export function getIdentifyingFieldDisplayValue(fieldData: any): string { + const val = fieldData.formattedValue ?? fieldData.displayValue ?? fieldData.value; + if (Array.isArray(val)) + return val.join(', '); + return val; +} + export function getSampleIdentifyingFieldGridData( sampleIds: number[] | string[], sampleQueryInfo?: QueryInfo, @@ -1396,7 +1405,7 @@ export function getSampleIdentifyingFieldGridData( // Issue 52038: the Row has data keyed by name so make sure we do the same here (see QueryColumn index() comments) const colData = caseInsensitive(row, c.name); if (colData?.value != null) { - d[c.index] = colData?.formattedValue ?? colData?.displayValue ?? colData?.value; + d[c.index] = getIdentifyingFieldDisplayValue(colData); } }); samplesData[rowId] = d; diff --git a/packages/components/src/internal/components/forms/QuerySelect.tsx b/packages/components/src/internal/components/forms/QuerySelect.tsx index df586458e0..e11dde8059 100644 --- a/packages/components/src/internal/components/forms/QuerySelect.tsx +++ b/packages/components/src/internal/components/forms/QuerySelect.tsx @@ -108,7 +108,10 @@ const OptionRenderer: FC = props => { if (item !== undefined) { let text = resolveDetailFieldLabel(item.get(column.name)); if (!Utils.isString(text)) { - text = text != null ? text.toString() : ''; + if (text == null) + text = ''; + else if (Array.isArray(text)) + text = text.join(', '); } return ( diff --git a/packages/components/src/public/QueryModel/GridFilterModal.tsx b/packages/components/src/public/QueryModel/GridFilterModal.tsx index dcc20b4781..8eb6ac309f 100644 --- a/packages/components/src/public/QueryModel/GridFilterModal.tsx +++ b/packages/components/src/public/QueryModel/GridFilterModal.tsx @@ -78,7 +78,20 @@ export const GridFilterModal: FC = memo(props => { if (newFilters) { newFilters - ?.filter(newFilter => newFilter !== null) + ?.filter(newFilter => { + if (newFilter === null) + return false; + + const filterType = newFilter.getFilterType(); + if (filterType.getURLSuffix().toLowerCase().startsWith('array') && filterType.isMultiValued()) { + // GitHub Issue 987: Multi value filter dialog lets you edit and save without any selected values + if (!newFilter.getValue() || (Array.isArray(newFilter.getValue()) && newFilter.getValue().length === 0)) { + return false; + } + } + return true; + + }) .forEach(newFilter => { updatedFilters.push({ fieldKey: activeFieldKey, From 5ac7f0121366563bf1d1b8e469f173e769730fc0 Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 1 Apr 2026 06:51:42 -0700 Subject: [PATCH 2/8] GitHub Issue 966: Grid filter UI parses URL parameter incorrectly for array data --- packages/components/package-lock.json | 12 ++++++------ packages/components/package.json | 4 ++-- .../QueryModel/grid/actions/Filter.test.ts | 19 +++++++++++++++++++ 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index 8a0fb6062e..fed4d4caab 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,16 +1,16 @@ { "name": "@labkey/components", - "version": "7.26.5-fb-mvtcBatch3.0", + "version": "7.26.5-fb-mvtcBatch3.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "7.26.5-fb-mvtcBatch3.0", + "version": "7.26.5-fb-mvtcBatch3.1", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", - "@labkey/api": "1.51.0", + "@labkey/api": "1.51.1-mvtcBatch3.1", "@testing-library/dom": "~10.4.1", "@testing-library/jest-dom": "~6.9.1", "@testing-library/react": "~16.3.2", @@ -3746,9 +3746,9 @@ } }, "node_modules/@labkey/api": { - "version": "1.51.0", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.51.0.tgz", - "integrity": "sha512-pyYXCNbFF3HZQ8KVapcwBl/7cHlVDz0ysPlCRBUQ9czX6s2yLwiho0OWkBd9MpbGVkqGFihbTUsUAU+Y5rz5Pw==", + "version": "1.51.1-mvtcBatch3.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.51.1-mvtcBatch3.1.tgz", + "integrity": "sha512-HB/2tcDrlYiIxKewtqoWsHkFOBdXdndbabq8GkIJ8yzv0z08PNfzT50r34QTCHPNQM//0Sd4WIymYYSCEfOf4g==", "license": "Apache-2.0" }, "node_modules/@labkey/build": { diff --git a/packages/components/package.json b/packages/components/package.json index 069062f59b..3761d754dc 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "7.26.5-fb-mvtcBatch3.0", + "version": "7.26.5-fb-mvtcBatch3.1", "description": "Components, models, actions, and utility functions for LabKey applications and pages", "sideEffects": false, "files": [ @@ -53,7 +53,7 @@ "homepage": "https://github.com/LabKey/labkey-ui-components#readme", "dependencies": { "@hello-pangea/dnd": "18.0.1", - "@labkey/api": "1.51.0", + "@labkey/api": "1.51.1-mvtcBatch3.1", "@testing-library/dom": "~10.4.1", "@testing-library/jest-dom": "~6.9.1", "@testing-library/react": "~16.3.2", diff --git a/packages/components/src/public/QueryModel/grid/actions/Filter.test.ts b/packages/components/src/public/QueryModel/grid/actions/Filter.test.ts index 2e259e6bd9..8ebad19503 100644 --- a/packages/components/src/public/QueryModel/grid/actions/Filter.test.ts +++ b/packages/components/src/public/QueryModel/grid/actions/Filter.test.ts @@ -83,6 +83,25 @@ describe('FilterAction::actionValueFromFilter', () => { expect(value.displayValue).toBe('TimeCol = 01:02:00'); }); + test('multi-value IN filter with separator in value uses JSON encoding', () => { + // Value 'a}b;c' contains both '}' and ';' (the IN separator), triggering {json:...} encoding + const filter = Filter.create('col', ['a}b;c', 'normal'], Filter.Types.IN); + const value: ActionValue = action.actionValueFromFilter(filter); + expect(value.displayValue).toBe('col Equals One Of a}b;c, normal'); + }); + + test('multi-value IN filter round-trips through URL parameter encoding', () => { + // Simulate what happens when a filter is encoded to URL and parsed back + const original = ['a}b;c', 'x;y}z']; + const filter = Filter.create('col', original, Filter.Types.IN); + // getURLParameterValue produces {json:["a}b;c","x;y}z"]} + const encoded = filter.getFilterType().getURLParameterValue(filter.getValue()); + // Re-create from the encoded URL value, as if parsed from URL + const roundTripped = Filter.create('col', encoded, Filter.Types.IN); + const value: ActionValue = action.actionValueFromFilter(roundTripped); + expect(value.displayValue).toBe('col Equals One Of a}b;c, x;y}z'); + }); + test('decodePart label vs column name', () => { const col = new QueryColumn({ shortCaption: '$Bool Field./,$&' }); const filter = Filter.create('$DBoolField$P$S$C$D$A', true, Filter.Types.EQUAL); From 540317814ae515291e86c4e69e19c05e9e01c7af Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 1 Apr 2026 13:19:16 -0700 Subject: [PATCH 3/8] jest tests --- .../domainproperties/models.test.ts | 6 ++ .../components/editable/utils.test.ts | 73 ++++++++++++++++++- .../src/internal/components/editable/utils.ts | 8 +- .../components/entities/actions.test.ts | 32 +++++++- 4 files changed, 116 insertions(+), 3 deletions(-) diff --git a/packages/components/src/internal/components/domainproperties/models.test.ts b/packages/components/src/internal/components/domainproperties/models.test.ts index 3aa52bef86..cedf91609e 100644 --- a/packages/components/src/internal/components/domainproperties/models.test.ts +++ b/packages/components/src/internal/components/domainproperties/models.test.ts @@ -564,6 +564,12 @@ describe('PropDescType', () => { expect(acceptablePropertyType(DATETIME_TYPE, TIME_RANGE_URI)).toBeFalsy(); expect(acceptablePropertyType(DATE_TYPE, TIME_RANGE_URI)).toBeFalsy(); expect(acceptablePropertyType(TIME_TYPE, DATE_RANGE_URI)).toBeFalsy(); + + // GitHub Issue 951: multiline text cannot convert to text choice + expect(acceptablePropertyType(TEXT_CHOICE_TYPE, MULTILINE_RANGE_URI)).toBeFalsy(); + // but multiline text can still convert to other string types + expect(acceptablePropertyType(TEXT_TYPE, MULTILINE_RANGE_URI)).toBeTruthy(); + expect(acceptablePropertyType(MULTILINE_TYPE, MULTILINE_RANGE_URI)).toBeTruthy(); }); }); diff --git a/packages/components/src/internal/components/editable/utils.test.ts b/packages/components/src/internal/components/editable/utils.test.ts index 8895584578..3dd74f07a2 100644 --- a/packages/components/src/internal/components/editable/utils.test.ts +++ b/packages/components/src/internal/components/editable/utils.test.ts @@ -3,7 +3,7 @@ import { List, Map } from 'immutable'; import { QueryInfo } from '../../../public/QueryInfo'; import { QueryColumn, QueryLookup } from '../../../public/QueryColumn'; -import { DATE_RANGE_URI, NON_NEGATIVE_NUMBER_CONCEPT_URI } from '../domainproperties/constants'; +import { DATE_RANGE_URI, MULTI_CHOICE_RANGE_URI, NON_NEGATIVE_NUMBER_CONCEPT_URI } from '../domainproperties/constants'; import sampleSetQueryInfoJSON from '../../../test/data/sampleSetAllFieldTypes-getQueryDetails.json'; @@ -390,6 +390,77 @@ describe('getValidatedEditableGridValue', () => { }); }); + test('multi-choice column with valid values', () => { + const multiChoiceCol = new QueryColumn({ + jsonType: 'string', + rangeURI: MULTI_CHOICE_RANGE_URI, + validValues: ['alpha', 'beta', 'gamma'], + }); + + const validArray = [{ display: 'alpha' }, { display: 'beta' }]; + expect(getValidatedEditableGridValue(validArray, multiChoiceCol)).toStrictEqual({ + message: undefined, + value: validArray, + }); + }); + + test('multi-choice column with invalid choice', () => { + const multiChoiceCol = new QueryColumn({ + jsonType: 'string', + rangeURI: MULTI_CHOICE_RANGE_URI, + validValues: ['alpha', 'beta', 'gamma'], + }); + + const invalidArray = [{ display: 'alpha' }, { display: 'invalid' }]; + expect(getValidatedEditableGridValue(invalidArray, multiChoiceCol)).toStrictEqual({ + message: { message: "'invalid' is not a valid choice" }, + value: invalidArray, + }); + }); + + test('multi-choice column with duplicate choice', () => { + const multiChoiceCol = new QueryColumn({ + jsonType: 'string', + rangeURI: MULTI_CHOICE_RANGE_URI, + validValues: ['alpha', 'beta', 'gamma'], + }); + + const invalidArray = [{ display: 'alpha' }, { display: 'alpha' }]; + expect(getValidatedEditableGridValue(invalidArray, multiChoiceCol)).toStrictEqual({ + message: { message: "Duplicate values not allowed: alpha." }, + value: invalidArray, + }); + }); + + test('multi-choice column exceeding 10 items', () => { + const multiChoiceCol = new QueryColumn({ + jsonType: 'string', + rangeURI: MULTI_CHOICE_RANGE_URI, + validValues: ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k'], + }); + + const tooMany = Array.from({ length: 11 }, (_, i) => ({ display: String.fromCharCode(97 + i) })); + expect(getValidatedEditableGridValue(tooMany, multiChoiceCol)).toStrictEqual({ + message: { message: 'Too many values. Maximum allowed is 10.' }, + value: tooMany, + }); + }); + + test('multi-choice column with exactly 10 items is valid', () => { + const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j']; + const multiChoiceCol = new QueryColumn({ + jsonType: 'string', + rangeURI: MULTI_CHOICE_RANGE_URI, + validValues: values, + }); + + const tenItems = values.map(v => ({ display: v })); + expect(getValidatedEditableGridValue(tenItems, multiChoiceCol)).toStrictEqual({ + message: undefined, + value: tenItems, + }); + }); + test('required column', () => { const requiredCol = new QueryColumn({ jsonType: 'string', required: true, caption: 'ReqCol' }); diff --git a/packages/components/src/internal/components/editable/utils.ts b/packages/components/src/internal/components/editable/utils.ts index 8d7a32f07c..380c2a1418 100644 --- a/packages/components/src/internal/components/editable/utils.ts +++ b/packages/components/src/internal/components/editable/utils.ts @@ -30,7 +30,7 @@ interface ValidatedValue { export const getValidatedEditableGridValue = (origValue: any, col: QueryColumn): ValidatedValue => { // col ?? {} so it's safe to destructure const { caption, isDateOnlyColumn, jsonType, required, scale, validValues } = col ?? {}; - const isMultiChoice = col.isMultiChoice; + const isMultiChoice = col?.isMultiChoice; const isDateTimeType = jsonType === 'date'; const isDateType = isDateTimeType && isDateOnlyColumn; let message; @@ -57,8 +57,14 @@ export const getValidatedEditableGridValue = (origValue: any, col: QueryColumn): message = 'Too many values. Maximum allowed is 10.'; } else if (validValues) { + const seen = new Set(); origValue.forEach(val => { const trimmed = val.display?.toString().trim(); + if (seen.has(trimmed)) { + message = `Duplicate values not allowed: ${trimmed}.` + return false; + } + seen.add(trimmed); if (validValues.indexOf(trimmed) === -1) { message = `'${trimmed}' is not a valid choice`; return false; diff --git a/packages/components/src/internal/components/entities/actions.test.ts b/packages/components/src/internal/components/entities/actions.test.ts index 2ebb0d37f8..11f7655c14 100644 --- a/packages/components/src/internal/components/entities/actions.test.ts +++ b/packages/components/src/internal/components/entities/actions.test.ts @@ -1,6 +1,6 @@ import { Map } from 'immutable'; -import { extractEntityTypeOptionFromRow, getChosenParentData, sampleGenCellKey } from './actions'; +import { extractEntityTypeOptionFromRow, getChosenParentData, getIdentifyingFieldDisplayValue, sampleGenCellKey } from './actions'; import { EntityDataType, EntityIdCreationModel } from './models'; import { DataClassDataType, SampleTypeDataType } from './constants'; @@ -86,3 +86,33 @@ describe('sampleGenCellKey', () => { expect(sampleGenCellKey(null, 'Ancestors/Sources/Study', 1)).toBe('ancestors/sources/study&&1'); }); }); + +describe('getIdentifyingFieldDisplayValue', () => { + test('returns formattedValue when available', () => { + expect(getIdentifyingFieldDisplayValue({ formattedValue: 'formatted', displayValue: 'display', value: 'raw' })).toBe('formatted'); + }); + + test('falls back to displayValue when formattedValue is undefined', () => { + expect(getIdentifyingFieldDisplayValue({ displayValue: 'display', value: 'raw' })).toBe('display'); + }); + + test('falls back to value when formattedValue and displayValue are undefined', () => { + expect(getIdentifyingFieldDisplayValue({ value: 'raw' })).toBe('raw'); + }); + + test('joins array values with comma and space', () => { + expect(getIdentifyingFieldDisplayValue({ value: ['a', 'b', 'c'] })).toBe('a, b, c'); + }); + + test('joins array formattedValue with comma and space', () => { + expect(getIdentifyingFieldDisplayValue({ formattedValue: ['x', 'y'] })).toBe('x, y'); + }); + + test('returns single string value as-is', () => { + expect(getIdentifyingFieldDisplayValue({ value: 'single' })).toBe('single'); + }); + + test('handles single-element array', () => { + expect(getIdentifyingFieldDisplayValue({ value: ['only'] })).toBe('only'); + }); +}); From f65331a0a454e32a02333f95df6d3189a680fff1 Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 1 Apr 2026 17:42:52 -0700 Subject: [PATCH 4/8] clean --- .../src/internal/components/editable/actions.ts | 9 +++++---- .../components/src/internal/components/editable/utils.ts | 5 +++-- .../components/src/public/QueryModel/GridFilterModal.tsx | 3 ++- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/components/src/internal/components/editable/actions.ts b/packages/components/src/internal/components/editable/actions.ts index 839a478d09..f3d3ec28f1 100644 --- a/packages/components/src/internal/components/editable/actions.ts +++ b/packages/components/src/internal/components/editable/actions.ts @@ -1555,6 +1555,11 @@ async function insertPastedData( unmatched.push(vt); }); + if (values.length > 10) { + // GitHub Issue 970 + msg = { message: 'Too many values. Maximum allowed is 10.' }; + } + if (unmatched.length) { const valueStr = unmatched .slice(0, 4) @@ -1568,10 +1573,6 @@ async function insertPastedData( .join(', '); msg = { message: `Duplicate values not allowed: ${valueStr}.` }; } - if (values.length > 10) { - // GitHub Issue 970 - msg = { message: 'Too many values. Maximum allowed is 10.' }; - } cv = List(values); } else { const { message, value } = getValidatedEditableGridValue(val, col); diff --git a/packages/components/src/internal/components/editable/utils.ts b/packages/components/src/internal/components/editable/utils.ts index 380c2a1418..8c43c43a70 100644 --- a/packages/components/src/internal/components/editable/utils.ts +++ b/packages/components/src/internal/components/editable/utils.ts @@ -59,15 +59,16 @@ export const getValidatedEditableGridValue = (origValue: any, col: QueryColumn): else if (validValues) { const seen = new Set(); origValue.forEach(val => { + if (message) + return; + const trimmed = val.display?.toString().trim(); if (seen.has(trimmed)) { message = `Duplicate values not allowed: ${trimmed}.` - return false; } seen.add(trimmed); if (validValues.indexOf(trimmed) === -1) { message = `'${trimmed}' is not a valid choice`; - return false; } }) } diff --git a/packages/components/src/public/QueryModel/GridFilterModal.tsx b/packages/components/src/public/QueryModel/GridFilterModal.tsx index 8eb6ac309f..e6e82ffd07 100644 --- a/packages/components/src/public/QueryModel/GridFilterModal.tsx +++ b/packages/components/src/public/QueryModel/GridFilterModal.tsx @@ -84,8 +84,9 @@ export const GridFilterModal: FC = memo(props => { const filterType = newFilter.getFilterType(); if (filterType.getURLSuffix().toLowerCase().startsWith('array') && filterType.isMultiValued()) { + const filterValue = newFilter.getValue(); // GitHub Issue 987: Multi value filter dialog lets you edit and save without any selected values - if (!newFilter.getValue() || (Array.isArray(newFilter.getValue()) && newFilter.getValue().length === 0)) { + if (!filterValue || (Array.isArray(filterValue) && filterValue.length === 0)) { return false; } } From 7567cc906f75d8fbeb8a8e89a3c2b5b07f886854 Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 2 Apr 2026 13:30:53 -0700 Subject: [PATCH 5/8] code review changes --- packages/components/src/index.ts | 4 ++-- .../internal/components/editable/actions.ts | 1 + .../src/internal/components/editable/utils.ts | 8 +++++--- .../components/entities/actions.test.ts | 18 +++++++++--------- .../internal/components/entities/actions.ts | 4 ++-- 5 files changed, 19 insertions(+), 16 deletions(-) diff --git a/packages/components/src/index.ts b/packages/components/src/index.ts index 6cd5370e14..2cbeed8820 100644 --- a/packages/components/src/index.ts +++ b/packages/components/src/index.ts @@ -491,7 +491,7 @@ import { getDataDeleteConfirmationData, getEntityTypeOptions, getExcludedDataTypeNames, - getIdentifyingFieldDisplayValue, + getFieldDisplayValue, getOrderedSelectedMappedKeysFromQueryModel, getParentTypeDataForLineage, getSampleIdentifyingFieldGridData, @@ -1353,7 +1353,7 @@ export { getEntityTypeOptions, getEventDataValueDisplay, getExcludedDataTypeNames, - getIdentifyingFieldDisplayValue, + getFieldDisplayValue, getExpandQueryInfo, getFieldFiltersValidationResult, getFilterForSampleOperation, diff --git a/packages/components/src/internal/components/editable/actions.ts b/packages/components/src/internal/components/editable/actions.ts index f3d3ec28f1..8c0304e2f8 100644 --- a/packages/components/src/internal/components/editable/actions.ts +++ b/packages/components/src/internal/components/editable/actions.ts @@ -505,6 +505,7 @@ async function convertRowToEditorModelData( } else if (col.isMultiChoice && Array.isArray(data)) { const values = data.filter(item => !!item).map(item => ({ raw: item, display: item })); if (values.length > 10) { + // GitHub Issue 970 message = { message: 'Too many values. Maximum allowed is 10.', } diff --git a/packages/components/src/internal/components/editable/utils.ts b/packages/components/src/internal/components/editable/utils.ts index 8c43c43a70..c5df777742 100644 --- a/packages/components/src/internal/components/editable/utils.ts +++ b/packages/components/src/internal/components/editable/utils.ts @@ -66,9 +66,11 @@ export const getValidatedEditableGridValue = (origValue: any, col: QueryColumn): if (seen.has(trimmed)) { message = `Duplicate values not allowed: ${trimmed}.` } - seen.add(trimmed); - if (validValues.indexOf(trimmed) === -1) { - message = `'${trimmed}' is not a valid choice`; + else { + seen.add(trimmed); + if (validValues.indexOf(trimmed) === -1) { + message = `'${trimmed}' is not a valid choice`; + } } }) } diff --git a/packages/components/src/internal/components/entities/actions.test.ts b/packages/components/src/internal/components/entities/actions.test.ts index 11f7655c14..681427749e 100644 --- a/packages/components/src/internal/components/entities/actions.test.ts +++ b/packages/components/src/internal/components/entities/actions.test.ts @@ -1,6 +1,6 @@ import { Map } from 'immutable'; -import { extractEntityTypeOptionFromRow, getChosenParentData, getIdentifyingFieldDisplayValue, sampleGenCellKey } from './actions'; +import { extractEntityTypeOptionFromRow, getChosenParentData, getFieldDisplayValue, sampleGenCellKey } from './actions'; import { EntityDataType, EntityIdCreationModel } from './models'; import { DataClassDataType, SampleTypeDataType } from './constants'; @@ -87,32 +87,32 @@ describe('sampleGenCellKey', () => { }); }); -describe('getIdentifyingFieldDisplayValue', () => { +describe('getFieldDisplayValue', () => { test('returns formattedValue when available', () => { - expect(getIdentifyingFieldDisplayValue({ formattedValue: 'formatted', displayValue: 'display', value: 'raw' })).toBe('formatted'); + expect(getFieldDisplayValue({ formattedValue: 'formatted', displayValue: 'display', value: 'raw' })).toBe('formatted'); }); test('falls back to displayValue when formattedValue is undefined', () => { - expect(getIdentifyingFieldDisplayValue({ displayValue: 'display', value: 'raw' })).toBe('display'); + expect(getFieldDisplayValue({ displayValue: 'display', value: 'raw' })).toBe('display'); }); test('falls back to value when formattedValue and displayValue are undefined', () => { - expect(getIdentifyingFieldDisplayValue({ value: 'raw' })).toBe('raw'); + expect(getFieldDisplayValue({ value: 'raw' })).toBe('raw'); }); test('joins array values with comma and space', () => { - expect(getIdentifyingFieldDisplayValue({ value: ['a', 'b', 'c'] })).toBe('a, b, c'); + expect(getFieldDisplayValue({ value: ['a', 'b', 'c'] })).toBe('a, b, c'); }); test('joins array formattedValue with comma and space', () => { - expect(getIdentifyingFieldDisplayValue({ formattedValue: ['x', 'y'] })).toBe('x, y'); + expect(getFieldDisplayValue({ formattedValue: ['x', 'y'] })).toBe('x, y'); }); test('returns single string value as-is', () => { - expect(getIdentifyingFieldDisplayValue({ value: 'single' })).toBe('single'); + expect(getFieldDisplayValue({ value: 'single' })).toBe('single'); }); test('handles single-element array', () => { - expect(getIdentifyingFieldDisplayValue({ value: ['only'] })).toBe('only'); + expect(getFieldDisplayValue({ value: ['only'] })).toBe('only'); }); }); diff --git a/packages/components/src/internal/components/entities/actions.ts b/packages/components/src/internal/components/entities/actions.ts index 5c3606635a..933aaa28e3 100644 --- a/packages/components/src/internal/components/entities/actions.ts +++ b/packages/components/src/internal/components/entities/actions.ts @@ -1365,7 +1365,7 @@ export function getSingleSampleTypeQueryInfo(sampleIds: number[] | string[]): Pr } // GitHub Issue 928: Spaces not shown between text choices in identifying fields in editable grid -export function getIdentifyingFieldDisplayValue(fieldData: any): string { +export function getFieldDisplayValue(fieldData: any): string { const val = fieldData.formattedValue ?? fieldData.displayValue ?? fieldData.value; if (Array.isArray(val)) return val.join(', '); @@ -1405,7 +1405,7 @@ export function getSampleIdentifyingFieldGridData( // Issue 52038: the Row has data keyed by name so make sure we do the same here (see QueryColumn index() comments) const colData = caseInsensitive(row, c.name); if (colData?.value != null) { - d[c.index] = getIdentifyingFieldDisplayValue(colData); + d[c.index] = getFieldDisplayValue(colData); } }); samplesData[rowId] = d; From 2e443e739112d23ac8a10f918ed692dd62b2c482 Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 2 Apr 2026 15:43:52 -0700 Subject: [PATCH 6/8] publish --- packages/components/package-lock.json | 4 ++-- packages/components/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index c5117a063b..4e73587cd4 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,12 +1,12 @@ { "name": "@labkey/components", - "version": "7.27.1-fb-mvtcBatch3.0", + "version": "7.27.1-fb-mvtcBatch3.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "7.27.1-fb-mvtcBatch3.0", + "version": "7.27.1-fb-mvtcBatch3.1", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/packages/components/package.json b/packages/components/package.json index 910f857629..46e91b8590 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "7.27.1-fb-mvtcBatch3.0", + "version": "7.27.1-fb-mvtcBatch3.1", "description": "Components, models, actions, and utility functions for LabKey applications and pages", "sideEffects": false, "files": [ From fb67b175ac2b504c8b3c6ff46a574a07b009e463 Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 2 Apr 2026 22:32:50 -0700 Subject: [PATCH 7/8] merge from develop --- packages/components/package-lock.json | 12 ++++++------ packages/components/package.json | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index 4cbdb35c05..54191781b4 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,16 +1,16 @@ { "name": "@labkey/components", - "version": "7.28.0", + "version": "7.28.1-fb-mvtcBatch3.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "7.28.0", + "version": "7.28.1-fb-mvtcBatch3.0", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", - "@labkey/api": "1.51.0", + "@labkey/api": "1.51.1-mvtcBatch3.1", "@testing-library/dom": "~10.4.1", "@testing-library/jest-dom": "~6.9.1", "@testing-library/react": "~16.3.2", @@ -3747,9 +3747,9 @@ } }, "node_modules/@labkey/api": { - "version": "1.51.0", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.51.0.tgz", - "integrity": "sha512-pyYXCNbFF3HZQ8KVapcwBl/7cHlVDz0ysPlCRBUQ9czX6s2yLwiho0OWkBd9MpbGVkqGFihbTUsUAU+Y5rz5Pw==", + "version": "1.51.1-mvtcBatch3.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.51.1-mvtcBatch3.1.tgz", + "integrity": "sha512-HB/2tcDrlYiIxKewtqoWsHkFOBdXdndbabq8GkIJ8yzv0z08PNfzT50r34QTCHPNQM//0Sd4WIymYYSCEfOf4g==", "license": "Apache-2.0" }, "node_modules/@labkey/build": { diff --git a/packages/components/package.json b/packages/components/package.json index 2f533c3392..f0d84ae922 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "7.28.0", + "version": "7.28.1-fb-mvtcBatch3.0", "description": "Components, models, actions, and utility functions for LabKey applications and pages", "sideEffects": false, "files": [ From 5bbbdf0476ecafd49e7aaaf0bb7a924cfa0c41d1 Mon Sep 17 00:00:00 2001 From: XingY Date: Sun, 5 Apr 2026 17:32:43 -0700 Subject: [PATCH 8/8] merge from develop --- packages/components/package-lock.json | 4 ++-- packages/components/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index f75c6d9025..8c3750b4f3 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,12 +1,12 @@ { "name": "@labkey/components", - "version": "7.28.2-fb-mvtcBatch3.0", + "version": "7.28.2-fb-mvtcBatch3.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "7.28.2-fb-mvtcBatch3.0", + "version": "7.28.2-fb-mvtcBatch3.1", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/packages/components/package.json b/packages/components/package.json index 2b2babf8a2..5d68fc994b 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "7.28.2-fb-mvtcBatch3.0", + "version": "7.28.2-fb-mvtcBatch3.1", "description": "Components, models, actions, and utility functions for LabKey applications and pages", "sideEffects": false, "files": [