From f75f7edf5cfb47fcdb79cb420d7628cc91243d0d Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 26 Feb 2026 19:00:50 -0800 Subject: [PATCH 1/6] Only do `quoteValueWithDelimiters` on values if the lookup column support multiple values --- packages/components/package.json | 2 +- packages/components/releaseNotes/components.md | 5 +++++ .../src/internal/components/editable/actions.ts | 3 ++- .../src/internal/components/editable/models.ts | 2 +- .../components/src/internal/components/forms/model.ts | 5 +++-- packages/components/src/internal/query/api.ts | 10 ++++++---- 6 files changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/components/package.json b/packages/components/package.json index ea78fad86f..a65bd1cf3c 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "7.21.0", + "version": "7.21.1-fb-lookupComma.1", "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 d1a361d5e3..4983c1ebb2 100644 --- a/packages/components/releaseNotes/components.md +++ b/packages/components/releaseNotes/components.md @@ -1,6 +1,11 @@ # @labkey/components Components, models, actions, and utility functions for LabKey applications and pages +### version 7.X +*Released*: X February 2026 +- GitHub Issue 829: Sample type with lookup to list with text primary key where the value contains a comma doesn't map to lookup + - Only do `quoteValueWithDelimiters` on values if the lookup column support multiple values + ### version 7.21.0 *Released*: 26 February 2026 - Package updates diff --git a/packages/components/src/internal/components/editable/actions.ts b/packages/components/src/internal/components/editable/actions.ts index 73f90073d9..ade88a24df 100644 --- a/packages/components/src/internal/components/editable/actions.ts +++ b/packages/components/src/internal/components/editable/actions.ts @@ -489,9 +489,10 @@ async function convertRowToEditorModelData( const valueDescriptors: ValueDescriptor[] = []; if (data && col?.isPublicLookup()) { + const multiple = col.isJunctionLookup() || col.isExpInput(); // value had better be the rowId here, but it may be several in a comma-separated list. // If it's the display value, which happens to be a number, much confusion will arise. - const values = data.toString().split(','); + const values = multiple ? data.toString().split(',') : [data.toString()]; for (const val of values) { const messageAndValue = await getLookupDisplayValue(col, parseIntIfNumber(val), containerPath); diff --git a/packages/components/src/internal/components/editable/models.ts b/packages/components/src/internal/components/editable/models.ts index b3f9cce47b..31b35ae6c7 100644 --- a/packages/components/src/internal/components/editable/models.ts +++ b/packages/components/src/internal/components/editable/models.ts @@ -354,7 +354,7 @@ export class EditorModel } else if (col.lookup.displayColumn === col.lookup.keyColumn) { row = row.set( col.name, - values.size === 1 ? quoteValueWithDelimiters(values.first()?.display, ',') : undefined + values.size === 1 ? values.first()?.display : undefined ); } else { let val; diff --git a/packages/components/src/internal/components/forms/model.ts b/packages/components/src/internal/components/forms/model.ts index 445efa040a..f78c22d116 100644 --- a/packages/components/src/internal/components/forms/model.ts +++ b/packages/components/src/internal/components/forms/model.ts @@ -225,7 +225,8 @@ export function fetchSearchResults(model: QuerySelectModel, input: any): Promise filterVal, model.valueColumn, model.delimiter, - addExactFilter ? displayColumn : undefined + addExactFilter ? displayColumn : undefined, + model.multiple ); } @@ -442,7 +443,7 @@ export async function initSelect(props: QuerySelectOwnProps): Promise(), valueColumn, }; diff --git a/packages/components/src/internal/query/api.ts b/packages/components/src/internal/query/api.ts index 1bfe44871a..df9292da56 100644 --- a/packages/components/src/internal/query/api.ts +++ b/packages/components/src/internal/query/api.ts @@ -626,7 +626,8 @@ export function handleSelectRowsResponse(response: Query.Response, queryInfo: Qu export function quoteValueColumnWithDelimiters( selectRowsResult: ISelectRowsResult, valueColumn: string, - delimiter: string + delimiter: string, + multiple : boolean = true ): ISelectRowsResult { const rowMap = selectRowsResult.models[selectRowsResult.key]; @@ -634,7 +635,7 @@ export function quoteValueColumnWithDelimiters( const cell = row[valueColumn]; if (Utils.isString(cell?.value)) { cell.displayValue = cell.displayValue ?? cell.value; - cell.value = quoteValueWithDelimiters(cell.value, delimiter); + cell.value = multiple ? quoteValueWithDelimiters(cell.value, delimiter) : cell.value; } }); @@ -646,7 +647,8 @@ export function searchRows( token: any, valueColumn: string, delimiter: string, - exactColumn?: string + exactColumn?: string, + multiple?: boolean ): Promise { return new Promise((resolve, reject) => { let exactFilters, qFilters; @@ -713,7 +715,7 @@ export function searchRows( finalResults = queryResults; } - resolve(quoteValueColumnWithDelimiters(finalResults, valueColumn, delimiter)); + resolve(quoteValueColumnWithDelimiters(finalResults, valueColumn, delimiter, multiple)); }) .catch(reason => { reject(reason); From ffe0849dc751495194f88e13118feeea419b387c Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 26 Feb 2026 19:04:10 -0800 Subject: [PATCH 2/6] Only do `quoteValueWithDelimiters` on values if the lookup column support multiple values --- packages/components/package-lock.json | 4 ++-- .../components/src/internal/components/editable/models.ts | 5 +---- packages/components/src/internal/components/forms/model.ts | 6 +++++- packages/components/src/internal/query/api.ts | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index abf3ee1b12..e653dc1fb4 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,12 +1,12 @@ { "name": "@labkey/components", - "version": "7.21.0", + "version": "7.21.1-fb-lookupComma.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "7.21.0", + "version": "7.21.1-fb-lookupComma.1", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/packages/components/src/internal/components/editable/models.ts b/packages/components/src/internal/components/editable/models.ts index 31b35ae6c7..970905a234 100644 --- a/packages/components/src/internal/components/editable/models.ts +++ b/packages/components/src/internal/components/editable/models.ts @@ -352,10 +352,7 @@ export class EditorModel .toArray(); row = row.set(col.name, valueArray); } else if (col.lookup.displayColumn === col.lookup.keyColumn) { - row = row.set( - col.name, - values.size === 1 ? values.first()?.display : undefined - ); + row = row.set(col.name, values.size === 1 ? values.first()?.display : undefined); } else { let val; if (values.size === 1) val = values.first()?.raw; diff --git a/packages/components/src/internal/components/forms/model.ts b/packages/components/src/internal/components/forms/model.ts index f78c22d116..7814c036dd 100644 --- a/packages/components/src/internal/components/forms/model.ts +++ b/packages/components/src/internal/components/forms/model.ts @@ -443,7 +443,11 @@ export async function initSelect(props: QuerySelectOwnProps): Promise(), valueColumn, }; diff --git a/packages/components/src/internal/query/api.ts b/packages/components/src/internal/query/api.ts index df9292da56..6f136f6f61 100644 --- a/packages/components/src/internal/query/api.ts +++ b/packages/components/src/internal/query/api.ts @@ -627,7 +627,7 @@ export function quoteValueColumnWithDelimiters( selectRowsResult: ISelectRowsResult, valueColumn: string, delimiter: string, - multiple : boolean = true + multiple = true ): ISelectRowsResult { const rowMap = selectRowsResult.models[selectRowsResult.key]; From a8bbdb0d10ee48862a88c1eee4b6f378e5ed85e7 Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 27 Feb 2026 11:36:29 -0800 Subject: [PATCH 3/6] jest --- .../components/editable/models.test.ts | 52 +++++++++++++++++- .../components/src/internal/query/api.test.ts | 53 +++++++++++++------ 2 files changed, 88 insertions(+), 17 deletions(-) diff --git a/packages/components/src/internal/components/editable/models.test.ts b/packages/components/src/internal/components/editable/models.test.ts index 87a988013f..73df4f673e 100644 --- a/packages/components/src/internal/components/editable/models.test.ts +++ b/packages/components/src/internal/components/editable/models.test.ts @@ -1170,12 +1170,18 @@ describe('EditorModel', () => { raw: 123, }, ]), + [genCellKey(sampleColFk, 2)]: List([ + { + display: 'Sample,321', + raw: 321, + }, + ]), }); const editorModel = modifyEm({ cellValues: basicEditorModel.cellValues.merge(cellValues), columnMap: basicEditorModel.columnMap.set(sampleColFk, sampleColumn), orderedColumns: basicEditorModel.orderedColumns.push(sampleColFk), - rowCount: 2, + rowCount: 3, }); expect(editorModel.getRowValue(1, false).get('SampleID')).toBe(123); expect(editorModel.getRowValue(1, false).get('SampleID/Name')).toBe(undefined); @@ -1184,6 +1190,50 @@ describe('EditorModel', () => { expect(editorModel.getRowValue(1, false, () => true).get('SampleID')).toBe(123); expect(editorModel.getRowValue(1, false, () => true).get('SampleID/Name')).toBe('Sample-123'); }); + test('include string list lookup display value', () => { + const lookColumn = new QueryColumn({ + caption: 'List Look', + fieldKey: 'ListLook', + fieldKeyArray: ['ListLook'], + jsonType: 'string', + name: 'ListLook', + shownInInsertView: true, + shownInUpdateView: true, + required: true, + userEditable: true, + lookup: { + schemaName: 'lists', + queryName: 'Location', + displayColumn: 'Path', + keyColumn: 'Path', + }, + }); + const colFk = lookColumn.fieldKey.toLowerCase(); + const cellValues = fromJS({ + [genCellKey(colFk, 0)]: List([{ display: undefined, raw: undefined }]), + [genCellKey(colFk, 1)]: List([ + { + display: 'Building 123', + raw: 'Building 123', + }, + ]), + [genCellKey(colFk, 2)]: List([ + { + display: 'Building, 321', + raw: 'Building, 321', + }, + ]), + }); + const editorModel = modifyEm({ + cellValues: basicEditorModel.cellValues.merge(cellValues), + columnMap: basicEditorModel.columnMap.set(colFk, lookColumn), + orderedColumns: basicEditorModel.orderedColumns.push(colFk), + rowCount: 3, + }); + expect(editorModel.getRowValue(0).get('ListLook')).toBe(undefined); + expect(editorModel.getRowValue(1).get('ListLook')).toBe('Building 123'); + expect(editorModel.getRowValue(2).get('ListLook')).toBe('Building, 321'); + }); }); describe('utils', () => { diff --git a/packages/components/src/internal/query/api.test.ts b/packages/components/src/internal/query/api.test.ts index feb718d6eb..93b6ea5c51 100644 --- a/packages/components/src/internal/query/api.test.ts +++ b/packages/components/src/internal/query/api.test.ts @@ -138,23 +138,26 @@ describe('api', () => { }); describe('quoteValueColumnWithDelimiters', () => { - const results: ISelectRowsResult = { - key: 'test', - models: { - test: { - 1: { Name: { value: 'one', url: 'http://one/test', randomProperty: 123 } }, - 2: { Name: { value: 'with, comma', url: 'http://with, comma/test' } }, - 4: { Name: { value: 'with "quotes", and comma' } }, - 3: { NoName: { value: 'nonesuch', url: 'http://with, comma/test' } }, - 5: { Name: { value: ', comma first', displayValue: ',', url: 'http://with, comma/test' } }, + function getResults(): ISelectRowsResult { + return { + key: 'test', + models: { + test: { + 1: { Name: { value: 'one', url: 'http://one/test', randomProperty: 123 } }, + 2: { Name: { value: 'with, comma', url: 'http://with, comma/test' } }, + 4: { Name: { value: 'with "quotes", and comma' } }, + 3: { NoName: { value: 'nonesuch', url: 'http://with, comma/test' } }, + 5: { Name: { value: ', comma first', displayValue: ',', url: 'http://with, comma/test' } }, + }, }, - }, - orderedModels: List([1, 2, 3, 4, 5]), - queries: {}, - rowCount: 5, - }; - test('encode', () => { - expect(quoteValueColumnWithDelimiters(results, 'Name', ',')).toStrictEqual({ + orderedModels: List([1, 2, 3, 4, 5]), + queries: {}, + rowCount: 5, + }; + } + + test('encode (multiple=true by default)', () => { + expect(quoteValueColumnWithDelimiters(getResults(), 'Name', ',')).toStrictEqual({ key: 'test', models: { test: { @@ -181,6 +184,24 @@ describe('api', () => { rowCount: 5, }); }); + + test('no encode when multiple=false', () => { + expect(quoteValueColumnWithDelimiters(getResults(), 'Name', ',', false)).toStrictEqual({ + key: 'test', + models: { + test: { + 1: { Name: { value: 'one', url: 'http://one/test', displayValue: 'one', randomProperty: 123 } }, + 2: { Name: { value: 'with, comma', url: 'http://with, comma/test', displayValue: 'with, comma' } }, + 4: { Name: { value: 'with "quotes", and comma', displayValue: 'with "quotes", and comma' } }, + 3: { NoName: { value: 'nonesuch', url: 'http://with, comma/test' } }, + 5: { Name: { value: ', comma first', displayValue: ',', url: 'http://with, comma/test' } }, + }, + }, + orderedModels: List([1, 2, 3, 4, 5]), + queries: {}, + rowCount: 5, + }); + }); }); test('splitRowsByContainer', () => { From 488c60d255b12f94c81b65335ba2835a79b8e0ed Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 27 Feb 2026 15:16:16 -0800 Subject: [PATCH 4/6] jest --- .../components/src/internal/components/editable/models.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/src/internal/components/editable/models.test.ts b/packages/components/src/internal/components/editable/models.test.ts index 73df4f673e..8b714e710f 100644 --- a/packages/components/src/internal/components/editable/models.test.ts +++ b/packages/components/src/internal/components/editable/models.test.ts @@ -1189,6 +1189,7 @@ describe('EditorModel', () => { expect(editorModel.getRowValue(1, false, () => false).get('SampleID/Name')).toBe(undefined); expect(editorModel.getRowValue(1, false, () => true).get('SampleID')).toBe(123); expect(editorModel.getRowValue(1, false, () => true).get('SampleID/Name')).toBe('Sample-123'); + expect(editorModel.getRowValue(2, false, () => true).get('SampleID/Name')).toBe('Sample,321'); }); test('include string list lookup display value', () => { const lookColumn = new QueryColumn({ From e43eefec47f01ab27270cae6b64ee858ac297120 Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 27 Feb 2026 16:58:11 -0800 Subject: [PATCH 5/6] Fix lookup key with special characters --- packages/components/src/internal/components/forms/model.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/components/src/internal/components/forms/model.ts b/packages/components/src/internal/components/forms/model.ts index 7814c036dd..01445de078 100644 --- a/packages/components/src/internal/components/forms/model.ts +++ b/packages/components/src/internal/components/forms/model.ts @@ -14,7 +14,7 @@ * limitations under the License. */ import { fromJS, Record as ImmutableRecord, List, Map, OrderedMap } from 'immutable'; -import { Filter, Query } from '@labkey/api'; +import { Filter, Query, QueryKey } from '@labkey/api'; import { QueryInfo } from '../../../public/QueryInfo'; @@ -151,7 +151,8 @@ function getSelectedOptions(model: QuerySelectModel, value: any): Map(); } - const keyPath = [model.valueColumn, 'value']; + // model.valueColumn is fieldKey, not column name + const keyPath = [QueryKey.decodePart(model.valueColumn), 'value']; const sources = model.allResults.merge(model.selectedItems); // multi-value case From 7f18e3234b1c936db05a6a222e5d309ab8aedd72 Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 27 Feb 2026 18:17:12 -0800 Subject: [PATCH 6/6] Selenium tests for string lookup with special characters in query/column/lookup/value --- packages/components/package-lock.json | 4 ++-- packages/components/package.json | 2 +- .../components/src/internal/components/editable/utils.ts | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index e653dc1fb4..8c9b21503a 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,12 +1,12 @@ { "name": "@labkey/components", - "version": "7.21.1-fb-lookupComma.1", + "version": "7.21.1-fb-lookupComma.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "7.21.1-fb-lookupComma.1", + "version": "7.21.1-fb-lookupComma.3", "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 a65bd1cf3c..b2c17cbcb1 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "7.21.1-fb-lookupComma.1", + "version": "7.21.1-fb-lookupComma.3", "description": "Components, models, actions, and utility functions for LabKey applications and pages", "sideEffects": false, "files": [ diff --git a/packages/components/src/internal/components/editable/utils.ts b/packages/components/src/internal/components/editable/utils.ts index b3253cdfe9..1decc9321e 100644 --- a/packages/components/src/internal/components/editable/utils.ts +++ b/packages/components/src/internal/components/editable/utils.ts @@ -1,4 +1,4 @@ -import { Filter, Utils } from '@labkey/api'; +import { Filter, QueryKey, Utils } from '@labkey/api'; import { Operation, QueryColumn } from '../../../public/QueryColumn'; @@ -221,7 +221,8 @@ export function getLookupFilters( } if (lookupKeyValues) { - filters.push(Filter.create(lookup.keyColumn, lookupKeyValues, Filter.Types.IN)); + // lookup.keyColumn is column name, needs to encode to handle cases when the column name contains special characters. + filters.push(Filter.create(QueryKey.encodePart(lookup.keyColumn), lookupKeyValues, Filter.Types.IN)); } const operation = forUpdate ? Operation.update : Operation.insert;