diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index 4451bc391f..d7526cef76 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,12 +1,12 @@ { "name": "@labkey/components", - "version": "7.23.3", + "version": "7.23.4-fb-parseComma.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "7.23.3", + "version": "7.23.4-fb-parseComma.2", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", @@ -24,6 +24,7 @@ "immutable": "~3.8.2", "normalizr": "~3.6.2", "numeral": "~2.0.6", + "papaparse": "5.5.3", "react": "~18.3.1", "react-color": "~2.19.3", "react-datepicker": "~7.6.0", @@ -13194,6 +13195,12 @@ "dev": true, "license": "BlueOak-1.0.0" }, + "node_modules/papaparse": { + "version": "5.5.3", + "resolved": "https://registry.npmjs.org/papaparse/-/papaparse-5.5.3.tgz", + "integrity": "sha512-5QvjGxYVjxO59MGU2lHVYpRWBBtKHnlIAcSe1uNFCkkptUh63NFRj0FJQm7nR67puEruUci/ZkjmEFrjCAyP4A==", + "license": "MIT" + }, "node_modules/param-case": { "version": "3.0.4", "resolved": "https://registry.npmjs.org/param-case/-/param-case-3.0.4.tgz", diff --git a/packages/components/package.json b/packages/components/package.json index eedec7b0d9..ed03e1c052 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "7.23.3", + "version": "7.23.4-fb-parseComma.2", "description": "Components, models, actions, and utility functions for LabKey applications and pages", "sideEffects": false, "files": [ @@ -67,6 +67,7 @@ "immutable": "~3.8.2", "normalizr": "~3.6.2", "numeral": "~2.0.6", + "papaparse": "5.5.3", "react": "~18.3.1", "react-color": "~2.19.3", "react-datepicker": "~7.6.0", diff --git a/packages/components/src/index.ts b/packages/components/src/index.ts index f4a976146b..68b2d7824b 100644 --- a/packages/components/src/index.ts +++ b/packages/components/src/index.ts @@ -66,11 +66,12 @@ import { isNonNegativeFloat, isNonNegativeInteger, isSetEqual, + joinMultiValueForExport, makeCommaSeparatedString, - parseCsvString, parseScientificInt, quoteValueWithDelimiters, setIsTestEnv, + splitMultiValueForImport, uncapitalizeFirstChar, valueIsEmpty, withTransformedKeys, @@ -1503,6 +1504,7 @@ export { JavaDocsLink, JobOperation, joinDateTimeFormat, + joinMultiValueForExport, Key, LabelColorRenderer, LabelHelpTip, @@ -1537,7 +1539,6 @@ export { MAX_EDITABLE_GRID_ROWS, MAX_SELECTION_ACTION_ROWS, MEASUREMENT_UNITS, - UNITS_KIND, MemberType, MenuDivider, MenuHeader, @@ -1575,7 +1576,6 @@ export { ParentEntityRequiredColumns, ParentImportAliasRenderer, parseCellKey, - parseCsvString, parseDate, parseEntityParentKey, parseScientificInt, @@ -1698,6 +1698,7 @@ export { spliceURL, SplitButton, splitDateTimeFormat, + splitMultiValueForImport, STORAGE_UNIQUE_ID_CONCEPT_URI, StorageAmountInput, StorageStatusRenderer, @@ -1723,6 +1724,7 @@ export { UnidentifiedPill, UNIQUE_ID_FIND_FIELD, UnitModel, + UNITS_KIND, updateCellKeySampleIdMap, updateCellValuesForSampleIds, updateColumnLookup, diff --git a/packages/components/src/internal/components/editable/actions.test.ts b/packages/components/src/internal/components/editable/actions.test.ts index 7fefffa7bc..fe48af0c9e 100644 --- a/packages/components/src/internal/components/editable/actions.test.ts +++ b/packages/components/src/internal/components/editable/actions.test.ts @@ -34,6 +34,7 @@ import { genCellKey } from './utils'; import sampleSetQueryInfoJSON from '../../../test/data/sampleSetAllFieldTypes-getQueryDetails.json'; import { MockEditableGridLoader } from './utils.test'; import { MULTI_CHOICE_TYPE } from '../domainproperties/PropDescType'; +import { joinMultiValueForExport } from '../../util/utils'; describe('column mutation actions', () => { const queryInfo = QueryInfo.fromJsonForTests(sampleSet2QueryInfo); @@ -859,7 +860,7 @@ describe('insertPastedData', () => { fieldKey: mvtc, jsonType: 'ARRAY', rangeURI: MULTI_CHOICE_TYPE.rangeURI, - validValues: ['a', 'ab', 'cc', 'cD', 'A,B', 'de'], + validValues: ['a', 'ab', 'cc', 'cD', 'A,B', 'de', 'A', 'B', 'C', 'A,B,C', '"A"', '"A",B', '"A,B,C"'], }), }, }); @@ -1136,6 +1137,20 @@ describe('insertPastedData', () => { ); }); + test('pasting multi-line value', async () => { + const em = baseEditorModel.applyChanges({ + selectionCells: [genCellKey(fkOne, 0)], + selectedColIdx: 0, + selectedRowIdx: 2, + }); + const changes = await validateAndInsertPastedData(em, '"line1\nline2"', undefined, true, true, undefined, true); + const cellValues = changes.cellValues; + expect(cellValues.get(genCellKey(fkOne, 2))).toEqual(List([{ display: 'line1\nline2', raw: 'line1\nline2' }])); + + const cellMessages = changes.cellMessages; + expect(cellMessages.get(genCellKey(fkOne, 2))).toBeUndefined(); + }); + test('pasting multi values', async () => { const em = baseEditorModel.applyChanges({ selectionCells: [genCellKey(mvtc, 0)], @@ -1175,6 +1190,244 @@ describe('insertPastedData', () => { expect(cellMessages.get(genCellKey(mvtc, 1))).toBeUndefined(); expect(cellMessages.get(genCellKey(mvtc, 2))).toEqual({ message: 'Could not find "bad"' }); }); + + test('pasting string values with special characters, fromDragFill false', async () => { + const em = baseEditorModel.applyChanges({ + selectionCells: [genCellKey(fkOne, 0), genCellKey(fkOne, 1), genCellKey(fkOne, 2)], + selectedColIdx: 0, + selectedRowIdx: 2, + }); + const changes = await validateAndInsertPastedData( + em, + 'hello world\n"hello, world"\n"say ""hello"""', + undefined, + true, + true, + undefined, + true + ); + const cellValues = changes.cellValues; + // Space is preserved as-is + expect(cellValues.get(genCellKey(fkOne, 0))).toEqual(List([{ display: 'hello world', raw: 'hello world' }])); + // Quoted comma: without fromDragFill, CSV quoting is NOT stripped + expect(cellValues.get(genCellKey(fkOne, 1))).toEqual( + List([{ display: '"hello, world"', raw: '"hello, world"' }]) + ); + // Escaped double quotes: without fromDragFill, CSV escaping is NOT processed + expect(cellValues.get(genCellKey(fkOne, 2))).toEqual( + List([{ display: '"say ""hello"""', raw: '"say ""hello"""' }]) + ); + }); + + test('drag fill string values with special characters, fromDragFill true', async () => { + const em = baseEditorModel.applyChanges({ + selectionCells: [ + genCellKey(fkOne, 0), + genCellKey(fkOne, 1), + genCellKey(fkOne, 2), + genCellKey(fkOne, 3), + genCellKey(fkOne, 4), + genCellKey(fkOne, 5), + ], + selectedColIdx: 0, + selectedRowIdx: 5, + }); + const changes = await validateAndInsertPastedData( + em, + 'hello world\n"hello, world"\n"say ""hello"""', + undefined, + true, + true, + undefined, + false, + [[genCellKey(fkOne, 3), genCellKey(fkOne, 4), genCellKey(fkOne, 5)]], + true + ); + const cellValues = changes.cellValues; + // Original values unchanged + expect(cellValues.get(genCellKey(fkOne, 0))).toEqual(List([{ display: 'qwer', raw: 'qwer' }])); + expect(cellValues.get(genCellKey(fkOne, 1))).toEqual(List([{ display: 'asdf', raw: 'asdf' }])); + expect(cellValues.get(genCellKey(fkOne, 2))).toEqual(List([{ display: 'zxcv', raw: 'zxcv' }])); + // Space: no CSV quoting to strip + expect(cellValues.get(genCellKey(fkOne, 3))).toEqual(List([{ display: 'hello world', raw: 'hello world' }])); + // Quoted comma: fromDragFill strips CSV quoting, comma preserved in value + expect(cellValues.get(genCellKey(fkOne, 4))).toEqual(List([{ display: 'hello, world', raw: 'hello, world' }])); + // Escaped double quotes: fromDragFill strips CSV quoting and unescapes "" + expect(cellValues.get(genCellKey(fkOne, 5))).toEqual(List([{ display: 'say "hello"', raw: 'say "hello"' }])); + }); + + test('pasting exactly A,B into mvtc matches single valid value', async () => { + const em = baseEditorModel.applyChanges({ + selectionCells: [genCellKey(mvtc, 0)], + selectedColIdx: 3, + selectedRowIdx: 0, + }); + const changes = await validateAndInsertPastedData(em, '"A,B"', undefined, true, true, undefined, true); + expect(changes.cellValues.get(genCellKey(mvtc, 0))).toEqual(List([{ display: 'A,B', raw: 'A,B' }])); + expect(changes.cellMessages.get(genCellKey(mvtc, 0))).toBeUndefined(); + }); + + test('pasting exactly A,B,C into mvtc matches single valid value', async () => { + const em = baseEditorModel.applyChanges({ + selectionCells: [genCellKey(mvtc, 0)], + selectedColIdx: 3, + selectedRowIdx: 0, + }); + const changes = await validateAndInsertPastedData(em, '"A,B,C"', undefined, true, true, undefined, true); + expect(changes.cellValues.get(genCellKey(mvtc, 0))).toEqual(List([{ display: 'A,B,C', raw: 'A,B,C' }])); + expect(changes.cellMessages.get(genCellKey(mvtc, 0))).toBeUndefined(); + }); + + test('pasting escaped "A,B,C" into mvtc matches single valid value', async () => { + const em = baseEditorModel.applyChanges({ + selectionCells: [genCellKey(mvtc, 0)], + selectedColIdx: 3, + selectedRowIdx: 0, + }); + const changes = await validateAndInsertPastedData(em, '"""A,B,C"""', undefined, true, true, undefined, true); + expect(changes.cellValues.get(genCellKey(mvtc, 0))).toEqual(List([{ display: '"A,B,C"', raw: '"A,B,C"' }])); + expect(changes.cellMessages.get(genCellKey(mvtc, 0))).toBeUndefined(); + }); + + test('pasting A, B with space into mvtc parses as two values', async () => { + const em = baseEditorModel.applyChanges({ + selectionCells: [genCellKey(mvtc, 0)], + selectedColIdx: 3, + selectedRowIdx: 0, + }); + const changes = await validateAndInsertPastedData(em, 'A, B', undefined, true, true, undefined, true); + // 'A, B' does not match any validValue, so parsed as CSV → ' B' and 'A' (sorted with leading space) + expect(changes.cellValues.get(genCellKey(mvtc, 0))).toEqual( + List([ + { display: 'A', raw: 'A' }, + { display: 'B', raw: 'B' }, + ]) + ); + expect(changes.cellMessages.get(genCellKey(mvtc, 0))).toBeUndefined(); + }); + + test('pasting quoted "A,B" into mvtc treats as single value', async () => { + const em = baseEditorModel.applyChanges({ + selectionCells: [genCellKey(mvtc, 0)], + selectedColIdx: 3, + selectedRowIdx: 0, + }); + const changes = await validateAndInsertPastedData(em, '"A,B"', undefined, true, true, undefined, true); + // Quoted '"A,B"' is CSV-parsed to 'A,B' which is a valid value + expect(changes.cellValues.get(genCellKey(mvtc, 0))).toEqual(List([{ display: 'A,B', raw: 'A,B' }])); + expect(changes.cellMessages.get(genCellKey(mvtc, 0))).toBeUndefined(); + }); + + test('pasting quoted "A, B" into mvtc, invalid', async () => { + const em = baseEditorModel.applyChanges({ + selectionCells: [genCellKey(mvtc, 0)], + selectedColIdx: 3, + selectedRowIdx: 0, + }); + const changes = await validateAndInsertPastedData(em, '"A, B"', undefined, true, true, undefined, true); + // Quoted '"A,B"' is CSV-parsed to 'A,B' which is a valid value + expect(changes.cellValues.get(genCellKey(mvtc, 0))).toEqual(List([{ display: 'A, B', raw: 'A, B' }])); + expect(changes.cellMessages.get(genCellKey(mvtc, 0))).toEqual({ + message: 'Could not find "A, B". Please make sure values that contain commas are properly quoted.', + }); + }); + + test('pasting mvtc values combined with other valid values', async () => { + const em = baseEditorModel.applyChanges({ + selectionCells: [genCellKey(mvtc, 0), genCellKey(mvtc, 1), genCellKey(mvtc, 2)], + selectedColIdx: 3, + selectedRowIdx: 2, + }); + const changes = await validateAndInsertPastedData( + em, + '"A,B"\n"A,B",cc\nA,B,cc', + undefined, + true, + true, + undefined, + true + ); + const cellValues = changes.cellValues; + // Row 0: 'A,B' exactly matches single validValue + expect(cellValues.get(genCellKey(mvtc, 0))).toEqual(List([{ display: 'A,B', raw: 'A,B' }])); + expect(changes.cellMessages.get(genCellKey(mvtc, 0))).toBeUndefined(); + // Row 1: '"A,B",cc' → CSV parsed to ['A,B', 'cc'], both valid + expect(cellValues.get(genCellKey(mvtc, 1))).toEqual( + List([ + { display: 'A,B', raw: 'A,B' }, + { display: 'cc', raw: 'cc' }, + ]) + ); + expect(changes.cellMessages.get(genCellKey(mvtc, 1))).toBeUndefined(); + // Row 2: 'A,B,cc' without quotes → CSV parsed to ['A', 'B', 'cc'], all valid + expect(cellValues.get(genCellKey(mvtc, 2))).toEqual( + List([ + { display: 'A', raw: 'A' }, + { display: 'B', raw: 'B' }, + { display: 'cc', raw: 'cc' }, + ]) + ); + expect(changes.cellMessages.get(genCellKey(mvtc, 2))).toBeUndefined(); + }); + + test('pasting mvtc values combined with invalid values', async () => { + const em = baseEditorModel.applyChanges({ + selectionCells: [genCellKey(mvtc, 0), genCellKey(mvtc, 1)], + selectedColIdx: 3, + selectedRowIdx: 1, + }); + const changes = await validateAndInsertPastedData( + em, + 'A, B, bad\n"A,B",bad', + undefined, + true, + true, + undefined, + true + ); + const cellValues = changes.cellValues; + const cellMessages = changes.cellMessages; + // Row 0: 'A,B,bad' → CSV parsed to ['A', 'B', 'bad'], 'bad' invalid + expect(cellValues.get(genCellKey(mvtc, 0))).toEqual( + List([ + { display: 'A', raw: 'A' }, + { display: 'B', raw: 'B' }, + { display: 'bad', raw: 'bad' }, + ]) + ); + expect(cellMessages.get(genCellKey(mvtc, 0))).toEqual({ message: 'Could not find "bad"' }); + // Row 1: '"A,B",bad' → CSV parsed to ['A,B', 'bad'], 'bad' invalid + expect(cellValues.get(genCellKey(mvtc, 1))).toEqual( + List([ + { display: 'A,B', raw: 'A,B' }, + { display: 'bad', raw: 'bad' }, + ]) + ); + expect(cellMessages.get(genCellKey(mvtc, 1))).toEqual({ message: 'Could not find "bad"' }); + }); + + test.each([ + { values: ['A', 'B', 'C'], desc: 'simple values' }, + { values: ['"A",B'], desc: 'single value with quotes and comma' }, + { values: ['"A,B,C"'], desc: 'single value with quotes wrapping commas' }, + { values: ['"A",B', 'B'], desc: 'tricky + simple' }, + { values: ['A', '"A"', 'B'], desc: 'quotes among simple' }, + { values: ['A', 'A,B,C', '"A,B,C"'], desc: 'plain + comma + quote-containing' }, + { values: ['"A"', '"A",B', '"A,B,C"'], desc: 'all contain quotes' }, + ])('pasting multi values round-trip: $desc', async ({ values }) => { + const exported = joinMultiValueForExport(values); + const em = baseEditorModel.applyChanges({ + selectionCells: [genCellKey(mvtc, 0)], + selectedColIdx: 3, + selectedRowIdx: 0, + }); + const changes = await validateAndInsertPastedData(em, exported, undefined, true, true, undefined, true); + const cellValues = changes.cellValues.get(genCellKey(mvtc, 0)); + const sortedExpected = [...values].sort(); + const actualValues = cellValues.toArray().map(v => v.raw); + expect(actualValues).toStrictEqual(sortedExpected); + expect(changes.cellMessages.get(genCellKey(mvtc, 0))).toBeUndefined(); + }); }); describe('loadEditorModelData', () => { diff --git a/packages/components/src/internal/components/editable/actions.ts b/packages/components/src/internal/components/editable/actions.ts index 581c245866..9cd6094078 100644 --- a/packages/components/src/internal/components/editable/actions.ts +++ b/packages/components/src/internal/components/editable/actions.ts @@ -1,6 +1,7 @@ import { Filter, getServerContext, QueryKey, Utils } from '@labkey/api'; import { fromJS, List, Map, OrderedMap } from 'immutable'; import { addDays, subDays } from 'date-fns'; +import Papa from 'papaparse'; import { ExtendedMap } from '../../../public/ExtendedMap'; import { QueryColumn, QueryLookup } from '../../../public/QueryColumn'; @@ -11,9 +12,11 @@ import { caseInsensitive, isFloat, isInteger, - parseCsvString, + isSimpleQuotedMultiLine, + joinMultiValueForExport, parseScientificInt, quoteValueWithDelimiters, + splitMultiValueForImport, } from '../../util/utils'; import { ViewInfo } from '../../ViewInfo'; @@ -1108,7 +1111,7 @@ export function parsePastedLookup( // Parse pasted strings to split properly around quoted values. // Remove the quotes for storing the actual values in the grid. - const parsedValues = parseCsvString(value, ',', true); + const parsedValues = splitMultiValueForImport(value); // Issue 53055: Do not attempt to resolve multiple values for a single-value column if (!column.isJunctionLookup() && parsedValues.length > 1) { @@ -1215,7 +1218,7 @@ export function generateColumnFillValues( if (isReadonlyCell || isReadonlyRow) return ''; const initialValue = initialSelectionValues[i % initialSelectionValues.length]; - let value = initialValue.map(v => quoteValueWithDelimiters(v.display, ',')).join(','); + let value = joinMultiValueForExport(initialValue.map(v => v.display).toArray()); if (incrementType === IncrementType.NUMBER) { const amount = increment * (i + 1); let raw: number | string; @@ -1302,7 +1305,8 @@ export function dragFillEvent( forUpdate, targetContainerPath, false, - selectionToFill + selectionToFill, + true ); } @@ -1417,20 +1421,38 @@ function parsePaste(value: string): ParsePastePayload { let numCols = 0; let data = List>(); - if (value === undefined || value === null || typeof value !== 'string') { + if (value === undefined || value == null || typeof value !== 'string') { return { data, numCols, numRows: 0 }; } // remove trailing newline from pasted data to avoid creating an empty row of cells if (value.endsWith('\n')) value = value.substring(0, value.length - 1); - value.split('\n').forEach(rv => { - const columns = List(rv.split('\t')); - if (numCols < columns.size) { - numCols = columns.size; + if (value.indexOf('"') === -1 || isSimpleQuotedMultiLine(value)) { + // parse tsv ONLY if the copied string doesn't contain " + // quoteChar will be stripped during TSV parsing, resulting in incorrect parsed data + const rows = Papa.parse(value, { delimiter: '\t' }).data; + if (!rows || rows.length === 0) { + return { data, numCols, numRows: 0 }; } - data = data.push(columns); - }); + + rows.forEach(row => { + const columns: List = List(row); + if (numCols < columns.size) { + numCols = columns.size; + } + data = data.push(columns); + }); + } else { + // fall back to line by line processing without parsing, to preserver quotes + value.split('\n').forEach(rv => { + const columns = List(rv.split('\t')); + if (numCols < columns.size) { + numCols = columns.size; + } + data = data.push(columns); + }); + } // Normalize the number columns in each row in case a user pasted rows with different numbers of columns in them data = data @@ -1460,7 +1482,8 @@ async function insertPastedData( lockRowCount: boolean, forUpdate: boolean, targetContainerPath: string, - selectCells: boolean + selectCells: boolean, + fromDragFill?: boolean ): Promise> { const pastedData = paste.payload.data; let cellMessages = editorModel.cellMessages; @@ -1525,12 +1548,12 @@ async function insertPastedData( cv = valueDescriptors; msg = message; } else if (col?.isMultiChoice && Utils.isString(val)) { - const parsedValues = parseCsvString(val, ',', true).sort(caseSensitiveNaturalSort); - const unmatched: string[] = []; - const values = []; + const values: ValueDescriptor[] = []; + const parsedValues = splitMultiValueForImport(val, ',', true, true).sort(caseSensitiveNaturalSort); const foundValues = new Set(); + // GitHub Issue 942: Add error for duplicate values const dupValues = new Set(); parsedValues.forEach(v => { @@ -1566,7 +1589,15 @@ async function insertPastedData( } cv = List(values); } else { - const { message, value } = getValidatedEditableGridValue(val, col); + let valToValidate = val; + if (fromDragFill && Utils.isString(val)) { + // GitHub Issue 916: Copying/pasting in the grid doesn't always act as expected + // drag fill always quoteValueWithDelimiters, needs to remove the extra quotes before validating + const parsedValues = splitMultiValueForImport(val); + if (parsedValues.length === 1) valToValidate = parsedValues[0].trim(); + } + + const { message, value } = getValidatedEditableGridValue(valToValidate, col); let display = value; // Issue 52326: Copy/paste of date values across cells changes date formats @@ -1624,7 +1655,7 @@ function getPasteValuesByColumn(paste: PasteModel): List> { row.forEach((value, index) => { // if values contain commas, users will need to paste the values enclosed in quotes // but we don't want to retain these quotes for purposes of selecting values in the grid - parseCsvString(value, ',', true).forEach(v => { + splitMultiValueForImport(value).forEach(v => { if (v.trim().length > 0) valuesByColumn.get(index).push(v.trim()); }); }); @@ -1640,7 +1671,8 @@ export function validateAndInsertPastedData( forUpdate: boolean, targetContainerPath: string, selectCells: boolean, - selectionToFill?: string[][] + selectionToFill?: string[][], + fromDragFill?: boolean ): Promise> { let selectedColIdx: number; let selectedRowIdx: number; @@ -1678,7 +1710,8 @@ export function validateAndInsertPastedData( lockRowCount, forUpdate, targetContainerPath, - selectCells + selectCells, + fromDragFill ); } else { const fieldKey = editorModel.getFieldKeyByIndex(selectedColIdx); @@ -1715,18 +1748,16 @@ export function pasteEvent( } function getCellCopyValue(valueDescriptors: List): string { - let value = ''; - if (valueDescriptors && valueDescriptors.size > 0) { - let sep = ''; - value = valueDescriptors.reduce((agg, vd) => { - agg += sep + (vd.display !== undefined ? vd.display.toString().trim() : ''); - sep = ', '; - return agg; - }, value); + const values = []; + valueDescriptors.forEach(vd => { + values.push(vd.display !== undefined ? vd.display.toString().trim() : ''); + }); + + if (values.length > 0) return joinMultiValueForExport(values); } - return value; + return ''; } function getCopyValue(model: EditorModel, hideReadOnlyRows: boolean, readonlyRows: string[]): string { diff --git a/packages/components/src/internal/components/forms/input/SelectInput.tsx b/packages/components/src/internal/components/forms/input/SelectInput.tsx index 86648fc65f..5b9d9a0a9a 100644 --- a/packages/components/src/internal/components/forms/input/SelectInput.tsx +++ b/packages/components/src/internal/components/forms/input/SelectInput.tsx @@ -31,7 +31,7 @@ import { MIXED_VALUE_DISPLAY, } from '../constants'; import { QueryColumn } from '../../../../public/QueryColumn'; -import { generateId } from '../../../util/utils'; +import { generateId, joinMultiValueForExport } from '../../../util/utils'; import { naturalSortByProperty } from '../../../../public/sort'; const WARN_COLOR = '#8A6D3B'; @@ -498,7 +498,7 @@ export class SelectInputImpl extends Component { if (!skipJoinValues) { // consider removing altogether? - formValue = formValue.join(delimiter); + formValue = joinMultiValueForExport(formValue, delimiter); } } else { formValue = selectedOptions; diff --git a/packages/components/src/internal/components/forms/model.ts b/packages/components/src/internal/components/forms/model.ts index 01445de078..46a0b65bc7 100644 --- a/packages/components/src/internal/components/forms/model.ts +++ b/packages/components/src/internal/components/forms/model.ts @@ -23,12 +23,11 @@ import { SchemaQuery } from '../../../public/SchemaQuery'; import { getQueryDetails, ISelectRowsResult, - quoteValueColumnWithDelimiters, searchRows, selectRowsDeprecated, } from '../../query/api'; import { similaritySortFactory } from '../../util/similaritySortFactory'; -import { caseInsensitive, parseCsvString } from '../../util/utils'; +import { caseInsensitive, splitMultiValueForImport } from '../../util/utils'; import { naturalSort } from '../../../public/sort'; @@ -129,8 +128,10 @@ export function formatSavedResults( const { key, orderedModels } = result; const models = fromJS(result.models[key]); - const orderedResults = orderedModels[key] - .reduce((ordered, k) => ordered.set(k, models.get(k)), OrderedMap()); + const orderedResults = orderedModels[key].reduce( + (ordered, k) => ordered.set(k, models.get(k)), + OrderedMap() + ); return formatResults(model, orderedResults, token); } @@ -157,7 +158,7 @@ function getSelectedOptions(model: QuerySelectModel, value: any): Map { const resultValue = result.getIn(keyPath); @@ -356,7 +357,7 @@ export function buildValueFilter( filter = Filter.create(valueColumn, value, Filter.Types.IN); expectedValueCount = new Set(value).size; } else if (typeof value === 'string') { - const parsed = parseCsvString(value, delimiter, true); + const parsed = splitMultiValueForImport(value, delimiter); filter = Filter.create(valueColumn, parsed, Filter.Types.IN); expectedValueCount = new Set(parsed).size; } @@ -443,13 +444,7 @@ export async function initSelect(props: QuerySelectOwnProps): Promise(), + selectedItems: selectedItems ? fromJS(selectedItems.models[selectedItems.key]) : Map(), valueColumn, }; } diff --git a/packages/components/src/internal/query/api.ts b/packages/components/src/internal/query/api.ts index 6f136f6f61..9d5031b063 100644 --- a/packages/components/src/internal/query/api.ts +++ b/packages/components/src/internal/query/api.ts @@ -622,6 +622,7 @@ export function handleSelectRowsResponse(response: Query.Response, queryInfo: Qu }; } +// deprecated // exported for jest testing export function quoteValueColumnWithDelimiters( selectRowsResult: ISelectRowsResult, @@ -635,7 +636,6 @@ export function quoteValueColumnWithDelimiters( const cell = row[valueColumn]; if (Utils.isString(cell?.value)) { cell.displayValue = cell.displayValue ?? cell.value; - cell.value = multiple ? quoteValueWithDelimiters(cell.value, delimiter) : cell.value; } }); @@ -715,7 +715,7 @@ export function searchRows( finalResults = queryResults; } - resolve(quoteValueColumnWithDelimiters(finalResults, valueColumn, delimiter, multiple)); + resolve(finalResults); }) .catch(reason => { reject(reason); diff --git a/packages/components/src/internal/util/utils.test.ts b/packages/components/src/internal/util/utils.test.ts index ec4d3963e8..fd1d06af02 100644 --- a/packages/components/src/internal/util/utils.test.ts +++ b/packages/components/src/internal/util/utils.test.ts @@ -47,10 +47,13 @@ import { isQuotedWithDelimiters, isSameWithStringCompare, isSetEqual, + isSimpleQuotedMultiLine, + joinMultiValueForExport, makeCommaSeparatedString, parseCsvString, parseScientificInt, quoteValueWithDelimiters, + splitMultiValueForImport, styleStringToObj, toLowerSafe, uncapitalizeFirstChar, @@ -1563,6 +1566,12 @@ describe('quoteValueWithDelimiters', () => { expect(isQuotedWithDelimiters('"a\nb,c""d"', ',')).toBeTruthy(); }); + test('with double quotes', () => { + expect(quoteValueWithDelimiters('"', ',')).toBe('""""'); + expect(isQuotedWithDelimiters('""""', ',')).toBeTruthy(); + expect(isQuotedWithDelimiters('"a"', ',')).toBeTruthy(); + }); + test('round trip', () => { const initialString = 'ab "cd,e"'; expect(parseCsvString(quoteValueWithDelimiters(initialString, ','), ',', true)).toStrictEqual([initialString]); @@ -1584,6 +1593,180 @@ describe('quoteValueWithDelimiters', () => { }); }); +describe('isSimpleQuotedMultiLine', () => { + test('returns false for non-string and falsy values', () => { + expect(isSimpleQuotedMultiLine(null)).toBe(false); + expect(isSimpleQuotedMultiLine(undefined)).toBe(false); + expect(isSimpleQuotedMultiLine('')).toBe(false); + expect(isSimpleQuotedMultiLine(0)).toBe(false); + expect(isSimpleQuotedMultiLine(123)).toBe(false); + }); + + test('returns false for strings without newlines', () => { + expect(isSimpleQuotedMultiLine('"abc"')).toBe(false); + expect(isSimpleQuotedMultiLine('abc')).toBe(false); + expect(isSimpleQuotedMultiLine('"a,b"')).toBe(false); + }); + + test('returns false for unquoted strings with newlines', () => { + expect(isSimpleQuotedMultiLine('a\nb')).toBe(false); + expect(isSimpleQuotedMultiLine('a\rb')).toBe(false); + }); + + test('returns false for strings too short to be quoted', () => { + expect(isSimpleQuotedMultiLine('"\n')).toBe(false); + expect(isSimpleQuotedMultiLine('"\r')).toBe(false); + }); + + test('returns false for quoted strings with internal quotes', () => { + expect(isSimpleQuotedMultiLine('"a""b\nc"')).toBe(false); + expect(isSimpleQuotedMultiLine('"a\n""b"')).toBe(false); + }); + + test('returns true for simple quoted strings with newlines', () => { + expect(isSimpleQuotedMultiLine('"a\nb"')).toBe(true); + expect(isSimpleQuotedMultiLine('"a\rb"')).toBe(true); + expect(isSimpleQuotedMultiLine('"a\r\nb"')).toBe(true); + expect(isSimpleQuotedMultiLine('"a\nb\nc"')).toBe(true); + expect(isSimpleQuotedMultiLine('"\n"')).toBe(true); + }); +}); + +describe('splitMultiValueForImport', () => { + test('null and undefined', () => { + expect(splitMultiValueForImport(null)).toBeNull(); + expect(splitMultiValueForImport(undefined)).toBeUndefined(); + }); + + test('empty string', () => { + expect(splitMultiValueForImport('')).toStrictEqual([]); + }); + + test('simple values', () => { + expect(splitMultiValueForImport('A')).toStrictEqual(['A']); + expect(splitMultiValueForImport('A, B, C')).toStrictEqual(['A', ' B', ' C']); + expect(splitMultiValueForImport('A, B, C', ',', false, true)).toStrictEqual(['A', 'B', 'C']); + expect(splitMultiValueForImport('A,B,C')).toStrictEqual(['A', 'B', 'C']); + }); + + test('whitespace handling', () => { + expect(splitMultiValueForImport(' A , B ')).toStrictEqual([' A ', ' B ']); + expect(splitMultiValueForImport(' A , B , C ')).toStrictEqual([' A ', ' B ', ' C ']); + expect(splitMultiValueForImport(' A , B ', ',', false, true)).toStrictEqual(['A', 'B']); + }); + + test('empty value preservation', () => { + expect(splitMultiValueForImport(',', null, false)).toStrictEqual(['', '']); + expect(splitMultiValueForImport('A,', null, false)).toStrictEqual(['A', '']); + expect(splitMultiValueForImport(',B', null, false)).toStrictEqual(['', 'B']); + expect(splitMultiValueForImport('A,,C', null, false)).toStrictEqual(['A', '', 'C']); + expect(splitMultiValueForImport(' A , B ,', null, false)).toStrictEqual([' A ', ' B ', '']); + expect(splitMultiValueForImport(' A , B , ', null, false)).toStrictEqual([' A ', ' B ', ' ']); + + expect(splitMultiValueForImport(',')).toStrictEqual([]); + expect(splitMultiValueForImport('A,')).toStrictEqual(['A']); + expect(splitMultiValueForImport(',B')).toStrictEqual(['B']); + expect(splitMultiValueForImport('A,,C')).toStrictEqual(['A', 'C']); + expect(splitMultiValueForImport(' A , B ,')).toStrictEqual([' A ', ' B ']); + expect(splitMultiValueForImport(' A , B ,', null, true, true)).toStrictEqual(['A', 'B']); + expect(splitMultiValueForImport(' A , B , ', null, true, true)).toStrictEqual(['A', 'B']); + }); + + test('quoted values with commas', () => { + expect(splitMultiValueForImport('"A,B"')).toStrictEqual(['A,B']); + expect(splitMultiValueForImport('"A,B", C')).toStrictEqual(['A,B', ' C']); + expect(splitMultiValueForImport('"A,B", C', null, false, true)).toStrictEqual(['A,B', 'C']); + expect(splitMultiValueForImport('A,"B,C",D')).toStrictEqual(['A', 'B,C', 'D']); + }); + + test('escaped quotes', () => { + expect(splitMultiValueForImport('"""A"""')).toStrictEqual(['"A"']); + expect(splitMultiValueForImport('"""A"",B"')).toStrictEqual(['"A",B']); + expect(splitMultiValueForImport('"""A,B,C"""')).toStrictEqual(['"A,B,C"']); + }); + + test('quoted empty string', () => { + expect(splitMultiValueForImport('""')).toStrictEqual([]); + expect(splitMultiValueForImport('""', null, false)).toStrictEqual(['']); + expect(splitMultiValueForImport('"", A')).toStrictEqual([' A']); + expect(splitMultiValueForImport('"", A', null, false)).toStrictEqual(['', ' A']); + expect(splitMultiValueForImport('"", A', null, false, true)).toStrictEqual(['', 'A']); + }); + + test('quoted value with leading/trailing whitespace', () => { + expect(splitMultiValueForImport('" A "')).toStrictEqual([' A ']); + }); + + test('malformed input handled gracefully', () => { + expect(splitMultiValueForImport('"abc')).toStrictEqual(['abc']); + }); +}); + +describe('joinMultiValueForExport', () => { + test('simple values', () => { + expect(joinMultiValueForExport(['A', 'B', 'C'])).toBe('A,B,C'); + }); + + test('null and undefined become empty quoted string', () => { + expect(joinMultiValueForExport([null])).toBe(''); + expect(joinMultiValueForExport([undefined])).toBe(''); + expect(joinMultiValueForExport(['A', null, 'B'])).toBe('A,,B'); + }); + + test('empty string is quoted', () => { + expect(joinMultiValueForExport([''])).toBe(''); + expect(joinMultiValueForExport(['A', '', 'B'])).toBe('A,,B'); + }); + + test('values with commas are quoted', () => { + expect(joinMultiValueForExport(['A,B'])).toBe('"A,B"'); + expect(joinMultiValueForExport(['A,B', 'C'])).toBe('"A,B",C'); + }); + + test('values with quotes are escaped and quoted', () => { + expect(joinMultiValueForExport(['"A"'])).toBe('"""A"""'); + expect(joinMultiValueForExport(['A"B'])).toBe('"A""B"'); + }); + + test('values with leading/trailing whitespace are quoted', () => { + expect(joinMultiValueForExport([' A'])).toBe('" A"'); + expect(joinMultiValueForExport(['A '])).toBe('"A "'); + expect(joinMultiValueForExport([' A '])).toBe('" A "'); + }); +}); + +describe('splitMultiValueForImport / joinMultiValueForExport round-trip', () => { + test.each([ + [['A', 'B', 'C']], + [['A,B,C']], + [['"A",B']], + [['"A,B,C"']], + [['"A",B', 'B']], + [['A', '"A"', 'B']], + [['A', 'A,B,C', '"A,B,C"']], + [['"A"', '"A",B', '"A,B,C"']], + ])('split(join(%j)) === original', values => { + const exported = joinMultiValueForExport(values); + const imported = splitMultiValueForImport(exported); + expect(imported).toStrictEqual(values); + }); + + test.each([ + ['A,B,C'], + ['"A,B,C"'], + ['"""A"",B"'], + ['"""A,B,C"""'], + ['"""A"",B",B'], + ['A,"""A""",B'], + ['A,"A,B,C","""A,B,C"""'], + ['"""A""","""A"",B","""A,B,C"""'], + ])('join(split(%j)) === original', str => { + const imported = splitMultiValueForImport(str); + const exported = joinMultiValueForExport(imported); + expect(exported).toBe(str); + }); +}); + describe('arrayEquals', () => { test('ignore order, case sensitive', () => { expect(arrayEquals(undefined, undefined)).toBeTruthy(); diff --git a/packages/components/src/internal/util/utils.ts b/packages/components/src/internal/util/utils.ts index f1efe6d3fd..1108a16a3e 100644 --- a/packages/components/src/internal/util/utils.ts +++ b/packages/components/src/internal/util/utils.ts @@ -16,6 +16,7 @@ import { Set as ImmutableSet, Iterable, List, Map } from 'immutable'; import { getServerContext, Utils } from '@labkey/api'; import { ChangeEvent, CSSProperties } from 'react'; +import Papa from 'papaparse'; import { hasParameter, toggleParameter } from '../url/ActionURL'; import { QueryInfo } from '../../public/QueryInfo'; @@ -649,6 +650,7 @@ export const handleFileInputChange = ( }; }; +// deprecated, use splitMultiValueForImport instead which uses PapaParse and is more robust for handling edge cases with quoted values, delimiters in values, etc. export function parseCsvString(value: string, delimiter: string, removeQuotes?: boolean): string[] { if (delimiter === '"') throw 'Unsupported delimiter: ' + delimiter; @@ -714,6 +716,8 @@ export function parseCsvString(value: string, delimiter: string, removeQuotes?: const TSV_ESCAPE_CHARS = ['\r', '\n', '\\', '"']; function hasTsvEscapeChar(value: any, delimiter: string): boolean { const allEscapedChars = [...TSV_ESCAPE_CHARS, delimiter]; + // if start or end with whitespace, we need to quote to preserve that whitespace when importing back from CSV + if (value[0].trim() === '' || value[value.length - 1].trim() === '') return true; return !!allEscapedChars.find(char => value.indexOf(char) > -1); } @@ -748,6 +752,53 @@ export function isQuotedWithDelimiters(value: any, delimiter: string): boolean { return strVal.startsWith('"') && strVal.endsWith('"'); } +/** + * Returns true if the value is a string that contains a newline character and is quoted with double quotes, + * and does not contain any other double quotes. + * This is used to determine whether we can safely parse a multi-line string as TSV (for paste) without losing its escaped characters. + * @param value + */ +const NEWLINE_CHARS = ['\r', '\n']; +export function isSimpleQuotedMultiLine(value: any): boolean { + if (!value || !Utils.isString(value)) { + return false; + } + + if (!NEWLINE_CHARS.find(char => value.indexOf(char) > -1)) return false; + + const strVal = value + ''; + if (strVal.length <= 2) return false; // need at least 2 characters to be quoted with something in between + if (!strVal.startsWith('"') || !strVal.endsWith('"')) return false; + + const innerValue = strVal.substring(1, strVal.length - 1); + return innerValue.indexOf('"') === -1; +} + +export function joinMultiValueForExport(values: string[], delimiter = ','): string { + return Papa.unparse([values], { delimiter }); +} + +const processParsedResults = (results, removeEmpty = true, trimSpace?: boolean): string[] => { + return results.data[0] + ?.map(value => (trimSpace && Utils.isString(value) ? value.trim() : value)) + .filter(value_ => (removeEmpty ? value_ !== '' : true)); +}; +// Port of Java PageFlowUtil.splitStringToValuesForImport — Google Sheets-compatible CSV parsing +// for multi-value (multi-select) column values. Fixed comma delimiter, double-quote quoting. +export function splitMultiValueForImport( + str: string, + delimiter = ',', + removeEmpty = true, + trimSpace?: boolean +): string[] { + if (str === null) return null; + if (str === undefined) return undefined; + if (!str) { + return []; + } + return processParsedResults(Papa.parse(str, { delimiter }), removeEmpty, trimSpace); +} + export function arrayEquals(a: string[], b: string[], ignoreOrder = true, caseInsensitive?: boolean): boolean { if (a === b) return true; if (a == null && b == null) return true;