diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index d0bd8ef1ce..8c3750b4f3 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,16 +1,16 @@ { "name": "@labkey/components", - "version": "7.28.1", + "version": "7.28.2-fb-mvtcBatch3.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "7.28.1", + "version": "7.28.2-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", @@ -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 c53395834b..5d68fc994b 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "7.28.1", + "version": "7.28.2-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/releaseNotes/components.md b/packages/components/releaseNotes/components.md index 3c4f6d68e4..52fbdac262 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.28.1 *Released*: 3 April 2026 - GitHub Issue 613: Use "Status" instead of "SampleState" in error messaging. diff --git a/packages/components/src/index.ts b/packages/components/src/index.ts index bee47e17d1..4c01381ea3 100644 --- a/packages/components/src/index.ts +++ b/packages/components/src/index.ts @@ -492,6 +492,7 @@ import { getDataDeleteConfirmationData, getEntityTypeOptions, getExcludedDataTypeNames, + getFieldDisplayValue, getOrderedSelectedMappedKeysFromQueryModel, getParentTypeDataForLineage, getSampleIdentifyingFieldGridData, @@ -1353,6 +1354,7 @@ export { getEntityTypeOptions, getEventDataValueDisplay, getExcludedDataTypeNames, + getFieldDisplayValue, getExpandQueryInfo, getFieldFiltersValidationResult, getFilterForSampleOperation, 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/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 119ca17617..45402a45a6 100644 --- a/packages/components/src/internal/components/editable/actions.ts +++ b/packages/components/src/internal/components/editable/actions.ts @@ -508,6 +508,12 @@ 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.', + } + } valueDescriptors.push(...values); } else { let display = data; @@ -535,8 +541,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 { @@ -1592,6 +1597,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) 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 1decc9321e..c5df777742 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,30 @@ 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) { + 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}.` + } + else { + seen.add(trimmed); + if (validValues.indexOf(trimmed) === -1) { + message = `'${trimmed}' is not a valid choice`; + } + } + }) + } + } else if (value != null && value !== '' && !col?.isPublicLookup()) { if (validValues) { const trimmed = origValue?.toString().trim(); diff --git a/packages/components/src/internal/components/entities/actions.test.ts b/packages/components/src/internal/components/entities/actions.test.ts index 2ebb0d37f8..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, sampleGenCellKey } from './actions'; +import { extractEntityTypeOptionFromRow, getChosenParentData, getFieldDisplayValue, 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('getFieldDisplayValue', () => { + test('returns formattedValue when available', () => { + expect(getFieldDisplayValue({ formattedValue: 'formatted', displayValue: 'display', value: 'raw' })).toBe('formatted'); + }); + + test('falls back to displayValue when formattedValue is undefined', () => { + expect(getFieldDisplayValue({ displayValue: 'display', value: 'raw' })).toBe('display'); + }); + + test('falls back to value when formattedValue and displayValue are undefined', () => { + expect(getFieldDisplayValue({ value: 'raw' })).toBe('raw'); + }); + + test('joins array values with comma and space', () => { + expect(getFieldDisplayValue({ value: ['a', 'b', 'c'] })).toBe('a, b, c'); + }); + + test('joins array formattedValue with comma and space', () => { + expect(getFieldDisplayValue({ formattedValue: ['x', 'y'] })).toBe('x, y'); + }); + + test('returns single string value as-is', () => { + expect(getFieldDisplayValue({ value: 'single' })).toBe('single'); + }); + + test('handles single-element array', () => { + 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 3fddfead08..933aaa28e3 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 getFieldDisplayValue(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] = getFieldDisplayValue(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..e6e82ffd07 100644 --- a/packages/components/src/public/QueryModel/GridFilterModal.tsx +++ b/packages/components/src/public/QueryModel/GridFilterModal.tsx @@ -78,7 +78,21 @@ 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()) { + const filterValue = newFilter.getValue(); + // GitHub Issue 987: Multi value filter dialog lets you edit and save without any selected values + if (!filterValue || (Array.isArray(filterValue) && filterValue.length === 0)) { + return false; + } + } + return true; + + }) .forEach(newFilter => { updatedFilters.push({ fieldKey: activeFieldKey, 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);