Skip to content

Commit 689695a

Browse files
XingYXingY
committed
fix plain string field escaping
1 parent 38404b2 commit 689695a

2 files changed

Lines changed: 71 additions & 56 deletions

File tree

‎packages/components/src/internal/components/editable/actions.test.ts‎

Lines changed: 47 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,6 @@ describe('generateColumnFillValues', () => {
354354
return result;
355355
}, {});
356356
}
357-
const queryInfo = QueryInfo.fromJsonForTests(sampleSet2QueryInfo);
358357
const lookupFk = 'lookup';
359358
const intFk = 'int';
360359
const floatFk = 'float';
@@ -363,6 +362,17 @@ describe('generateColumnFillValues', () => {
363362
const strFk = 'str';
364363
const quoteFk = 'quote';
365364
const mvFk = 'mv';
365+
const columns = {
366+
[lookupFk]: new QueryColumn({ fieldKey: lookupFk, name: lookupFk, lookup: { isPublic: true } as QueryLookup }),
367+
[intFk]: new QueryColumn({ fieldKey: intFk, name: intFk, jsonType: 'int' }),
368+
[floatFk]: new QueryColumn({ fieldKey: floatFk, name: floatFk, jsonType: 'float' }),
369+
[strFk]: new QueryColumn({ fieldKey: strFk, name: strFk }),
370+
[dateFk]: new QueryColumn({ fieldKey: dateFk, name: dateFk, jsonType: 'date' }),
371+
[datetimeFk]: new QueryColumn({ fieldKey: datetimeFk, name: datetimeFk, jsonType: 'date' }),
372+
[quoteFk]: new QueryColumn({ fieldKey: quoteFk, name: quoteFk }),
373+
[mvFk]: new QueryColumn({ fieldKey: mvFk, name: mvFk, lookup: { multiValued: 'junction' } as QueryLookup }),
374+
};
375+
const queryInfo = QueryInfo.fromJsonForTests({ pkCols: [lookupFk], columns });
366376
const editorModel = new EditorModel({}).merge({
367377
queryInfo,
368378
cellMessages: Map<string, CellMessage>({
@@ -430,7 +440,10 @@ describe('generateColumnFillValues', () => {
430440
...makeCellValues(quoteFk, [['S,1'], ['S,2'], ['']]),
431441
...makeCellValues(mvFk, [['S,1', 'S,2'], ['S2', 'S3'], [''], ['']]),
432442
}),
433-
orderedColumns: List([lookupFk, intFk, floatFk, strFk, dateFk, datetimeFk]),
443+
columnMap: Object.keys(columns).reduce((result, key) => {
444+
return result.set(key, queryInfo.getColumn(key));
445+
}, Map<string, QueryColumn>()),
446+
orderedColumns: List([lookupFk, intFk, floatFk, strFk, dateFk, datetimeFk, quoteFk, mvFk]),
434447
rowCount: 10,
435448
}) as EditorModel;
436449

@@ -574,8 +587,8 @@ describe('generateColumnFillValues', () => {
574587
genCellKey(strFk, 2),
575588
genCellKey(strFk, 3),
576589
]);
577-
// Copied values with commas should be quoted
578-
expect(cellValues).toEqual(['"S,1"', '"S,1"', '"S,1"']);
590+
// Copied values with commas on a non-multi-value column don't need quoting (tab delimiter)
591+
expect(cellValues).toEqual(['S,1', 'S,1', 'S,1']);
579592
});
580593

581594
// Issue 52412
@@ -1191,7 +1204,7 @@ describe('insertPastedData', () => {
11911204
expect(cellMessages.get(genCellKey(mvtc, 2))).toEqual({ message: 'Could not find "bad"' });
11921205
});
11931206

1194-
test('pasting string values with special characters, fromDragFill false', async () => {
1207+
test('pasting string values with special characters', async () => {
11951208
const em = baseEditorModel.applyChanges({
11961209
selectionCells: [genCellKey(fkOne, 0), genCellKey(fkOne, 1), genCellKey(fkOne, 2)],
11971210
selectedColIdx: 0,
@@ -1209,51 +1222,45 @@ describe('insertPastedData', () => {
12091222
const cellValues = changes.cellValues;
12101223
// Space is preserved as-is
12111224
expect(cellValues.get(genCellKey(fkOne, 0))).toEqual(List([{ display: 'hello world', raw: 'hello world' }]));
1212-
// Quoted comma: without fromDragFill, CSV quoting is NOT stripped
1225+
// Quoted comma: CSV quoting is stripped, comma preserved
12131226
expect(cellValues.get(genCellKey(fkOne, 1))).toEqual(
1214-
List([{ display: '"hello, world"', raw: '"hello, world"' }])
1227+
List([{ display: 'hello, world', raw: 'hello, world' }])
12151228
);
1216-
// Escaped double quotes: without fromDragFill, CSV escaping is NOT processed
1229+
// Escaped double quotes: CSV escaping is processed
12171230
expect(cellValues.get(genCellKey(fkOne, 2))).toEqual(
1218-
List([{ display: '"say ""hello"""', raw: '"say ""hello"""' }])
1231+
List([{ display: 'say "hello"', raw: 'say "hello"' }])
12191232
);
12201233
});
12211234

1222-
test('drag fill string values with special characters, fromDragFill true', async () => {
1235+
test('pasting multi-line value into a single cell', async () => {
1236+
const value = '"ab\ncd"';
12231237
const em = baseEditorModel.applyChanges({
1224-
selectionCells: [
1225-
genCellKey(fkOne, 0),
1226-
genCellKey(fkOne, 1),
1227-
genCellKey(fkOne, 2),
1228-
genCellKey(fkOne, 3),
1229-
genCellKey(fkOne, 4),
1230-
genCellKey(fkOne, 5),
1231-
],
1238+
selectionCells: [genCellKey(fkOne, 0)],
12321239
selectedColIdx: 0,
1233-
selectedRowIdx: 5,
1240+
selectedRowIdx: 0,
12341241
});
1235-
const changes = await validateAndInsertPastedData(
1236-
em,
1237-
'hello world\n"hello, world"\n"say ""hello"""',
1238-
undefined,
1239-
true,
1240-
true,
1241-
undefined,
1242-
false,
1243-
[[genCellKey(fkOne, 3), genCellKey(fkOne, 4), genCellKey(fkOne, 5)]],
1244-
true
1245-
);
1242+
const changes = await validateAndInsertPastedData(em, value, undefined, true, true, undefined, true);
12461243
const cellValues = changes.cellValues;
1247-
// Original values unchanged
1248-
expect(cellValues.get(genCellKey(fkOne, 0))).toEqual(List([{ display: 'qwer', raw: 'qwer' }]));
1249-
expect(cellValues.get(genCellKey(fkOne, 1))).toEqual(List([{ display: 'asdf', raw: 'asdf' }]));
1250-
expect(cellValues.get(genCellKey(fkOne, 2))).toEqual(List([{ display: 'zxcv', raw: 'zxcv' }]));
1251-
// Space: no CSV quoting to strip
1252-
expect(cellValues.get(genCellKey(fkOne, 3))).toEqual(List([{ display: 'hello world', raw: 'hello world' }]));
1253-
// Quoted comma: fromDragFill strips CSV quoting, comma preserved in value
1254-
expect(cellValues.get(genCellKey(fkOne, 4))).toEqual(List([{ display: 'hello, world', raw: 'hello, world' }]));
1255-
// Escaped double quotes: fromDragFill strips CSV quoting and unescapes ""
1256-
expect(cellValues.get(genCellKey(fkOne, 5))).toEqual(List([{ display: 'say "hello"', raw: 'say "hello"' }]));
1244+
expect(cellValues.get(genCellKey(fkOne, 0))).toEqual(List([{ display: 'ab\ncd', raw: 'ab\ncd' }]));
1245+
expect(changes.cellMessages.get(genCellKey(fkOne, 0))).toBeUndefined();
1246+
});
1247+
1248+
test('pasting multi-line values into multiple cells', async () => {
1249+
const value = '"ab\n' +
1250+
'cd"\n' +
1251+
'"another\n' +
1252+
'line"';
1253+
const em = baseEditorModel.applyChanges({
1254+
selectionCells: [genCellKey(fkOne, 0), genCellKey(fkOne, 1)],
1255+
selectedColIdx: 0,
1256+
selectedRowIdx: 0,
1257+
});
1258+
const changes = await validateAndInsertPastedData(em, value, undefined, true, true, undefined, true);
1259+
const cellValues = changes.cellValues;
1260+
expect(cellValues.get(genCellKey(fkOne, 0))).toEqual(List([{ display: 'ab\ncd', raw: 'ab\ncd' }]));
1261+
expect(cellValues.get(genCellKey(fkOne, 1))).toEqual(
1262+
List([{ display: 'another\nline', raw: 'another\nline' }])
1263+
);
12571264
});
12581265

12591266
test('pasting exactly A,B into mvtc matches single valid value', async () => {

‎packages/components/src/internal/components/editable/actions.ts‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,6 +1086,15 @@ export function generateFillCellKeys(
10861086
return fillCellKeys;
10871087
}
10881088

1089+
/**
1090+
* For columns that support multile values, the value should be escaped using ',' as delimter.
1091+
* For columns that's single value, escape might be needed if the value contains newline characters, double quotes, or tab characters
1092+
* @param column
1093+
*/
1094+
function getColCopyPasteDelimter(column: QueryColumn) {
1095+
return column?.isJunctionLookup() || column?.isMultiChoice ? ',' : '\t';
1096+
}
1097+
10891098
export function parsePastedLookup(
10901099
column: QueryColumn,
10911100
descriptors: ValueDescriptor[],
@@ -1112,7 +1121,7 @@ export function parsePastedLookup(
11121121

11131122
// Parse pasted strings to split properly around quoted values.
11141123
// Remove the quotes for storing the actual values in the grid.
1115-
const parsedValues = splitMultiValueForImport(value);
1124+
const parsedValues = splitMultiValueForImport(value, getColCopyPasteDelimter(column));
11161125

11171126
// Issue 53055: Do not attempt to resolve multiple values for a single-value column
11181127
if (!column.isJunctionLookup() && parsedValues.length > 1) {
@@ -1214,12 +1223,13 @@ export function generateColumnFillValues(
12141223

12151224
return selectionToFill.map((cellKey, i) => {
12161225
const { fieldKey, rowIdx } = parseCellKey(cellKey);
1226+
const column = editorModel.getColumnFromMap(fieldKey);
12171227
const { isReadonlyCell, isReadonlyRow } = editorModel.getCellReadStatus(fieldKey, rowIdx, readonlyRows);
12181228
// Only need to generate blank values for read only cells, paste will ignore them
12191229
if (isReadonlyCell || isReadonlyRow) return '';
12201230

12211231
const initialValue = initialSelectionValues[i % initialSelectionValues.length];
1222-
let value = joinMultiValueForExport(initialValue.map(v => v.display).toArray());
1232+
let value = joinMultiValueForExport(initialValue.map(v => v.display).toArray(), getColCopyPasteDelimter(column));
12231233
if (incrementType === IncrementType.NUMBER) {
12241234
const amount = increment * (i + 1);
12251235
let raw: number | string;
@@ -1306,8 +1316,7 @@ export function dragFillEvent(
13061316
forUpdate,
13071317
targetContainerPath,
13081318
false,
1309-
selectionToFill,
1310-
true
1319+
selectionToFill
13111320
);
13121321
}
13131322

@@ -1483,8 +1492,7 @@ async function insertPastedData(
14831492
lockRowCount: boolean,
14841493
forUpdate: boolean,
14851494
targetContainerPath: string,
1486-
selectCells: boolean,
1487-
fromDragFill?: boolean
1495+
selectCells: boolean
14881496
): Promise<Partial<EditorModel>> {
14891497
const pastedData = paste.payload.data;
14901498
let cellMessages = editorModel.cellMessages;
@@ -1532,6 +1540,7 @@ async function insertPastedData(
15321540
let cv: List<ValueDescriptor>;
15331541
let msg: CellMessage;
15341542

1543+
const parseDelimter = getColCopyPasteDelimter(col);
15351544
if (col?.isPublicLookup()) {
15361545
// If the column is a lookup and forUpdate is true, then we need to query for the rowIds so we can set the correct raw values,
15371546
// otherwise insert will fail. This is most common for cross-folder sample selection (Issue 50363)
@@ -1552,7 +1561,7 @@ async function insertPastedData(
15521561
const unmatched: string[] = [];
15531562
const values: ValueDescriptor[] = [];
15541563

1555-
const parsedValues = splitMultiValueForImport(val, ',', true, true).sort(caseSensitiveNaturalSort);
1564+
const parsedValues = splitMultiValueForImport(val, parseDelimter, true, true).sort(caseSensitiveNaturalSort);
15561565
const foundValues = new Set<string>();
15571566

15581567
// GitHub Issue 942: Add error for duplicate values
@@ -1591,13 +1600,13 @@ async function insertPastedData(
15911600
cv = List(values);
15921601
} else {
15931602
let valToValidate = val;
1594-
if (fromDragFill && Utils.isString(val)) {
1603+
if (Utils.isString(val)) {
15951604
// GitHub Issue 916: Copying/pasting in the grid doesn't always act as expected
15961605
// drag fill always quoteValueWithDelimiters, needs to remove the extra quotes before validating
15971606
const isMultiLinePasting = NEWLINE_CHARS.find(char => valToValidate.indexOf(char) > -1);
15981607
// multiline pasting has already been parsed
15991608
if (!isMultiLinePasting) {
1600-
const parsedValues = splitMultiValueForImport(val);
1609+
const parsedValues = splitMultiValueForImport(val, parseDelimter);
16011610
if (parsedValues.length === 1) valToValidate = parsedValues[0].trim();
16021611
}
16031612
}
@@ -1676,8 +1685,7 @@ export function validateAndInsertPastedData(
16761685
forUpdate: boolean,
16771686
targetContainerPath: string,
16781687
selectCells: boolean,
1679-
selectionToFill?: string[][],
1680-
fromDragFill?: boolean
1688+
selectionToFill?: string[][]
16811689
): Promise<Partial<EditorModel>> {
16821690
let selectedColIdx: number;
16831691
let selectedRowIdx: number;
@@ -1715,8 +1723,7 @@ export function validateAndInsertPastedData(
17151723
lockRowCount,
17161724
forUpdate,
17171725
targetContainerPath,
1718-
selectCells,
1719-
fromDragFill
1726+
selectCells
17201727
);
17211728
} else {
17221729
const fieldKey = editorModel.getFieldKeyByIndex(selectedColIdx);
@@ -1752,14 +1759,14 @@ export function pasteEvent(
17521759
return undefined;
17531760
}
17541761

1755-
function getCellCopyValue(valueDescriptors: List<ValueDescriptor>): string {
1762+
function getCellCopyValue(valueDescriptors: List<ValueDescriptor>, delimter: string): string {
17561763
if (valueDescriptors && valueDescriptors.size > 0) {
17571764
const values = [];
17581765
valueDescriptors.forEach(vd => {
17591766
values.push(vd.display !== undefined ? vd.display.toString().trim() : '');
17601767
});
17611768

1762-
if (values.length > 0) return joinMultiValueForExport(values);
1769+
if (values.length > 0) return joinMultiValueForExport(values, delimter);
17631770
}
17641771

17651772
return '';
@@ -1786,7 +1793,8 @@ function getCopyValue(model: EditorModel, hideReadOnlyRows: boolean, readonlyRow
17861793

17871794
if (selectionCells.find(key => key === cellKey)) {
17881795
inSelection = true;
1789-
copyValue += cellSep + getCellCopyValue(model.cellValues.get(cellKey));
1796+
const column = model.getColumnFromMap(fieldKey);
1797+
copyValue += cellSep + getCellCopyValue(model.cellValues.get(cellKey), getColCopyPasteDelimter(column));
17901798
cellSep = '\t';
17911799
}
17921800
});

0 commit comments

Comments
 (0)