fix(editor): stop turning missing columns into undefined, and fix column lookup for sparse rows
Writing back an unescaped cell's value unconditionally materialized a column the backend never populated as an explicit `undefined`, which the SASjs adapter rejects at submit time. dynamicCellValidation's column-name lookup (Object.keys(row)[column]) only worked by coincidence when every row had a full key set - fixed to use hot.colToProp instead.
This commit is contained in:
@@ -962,8 +962,15 @@ export class EditorComponent implements OnInit, AfterViewInit, OnDestroy {
|
||||
if (!isCharacterColumn(this.cols, colName)) continue
|
||||
|
||||
const { value, wasEscaped } = escapeCharacterColumnValue(row[colName])
|
||||
// A row whose backend source never had this column at all (a
|
||||
// sparse row) must stay that way - writing back unconditionally
|
||||
// would materialize a new own property set to undefined, which
|
||||
// the SASjs adapter rejects at submit time even though nothing
|
||||
// about this cell ever changed.
|
||||
if (!wasEscaped) continue
|
||||
|
||||
row[colName] = value
|
||||
if (wasEscaped) markAutoEscaped(this.autoEscapedCells, rowKey, colName)
|
||||
markAutoEscaped(this.autoEscapedCells, rowKey, colName)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2906,7 +2913,13 @@ export class EditorComponent implements OnInit, AfterViewInit, OnDestroy {
|
||||
|
||||
const cellData = hot.getDataAtCell(row, column)
|
||||
const clickedRow = this.helperService.deepClone(this.dataSource[row])
|
||||
const clickedColumnKey = Object.keys(clickedRow)[column]
|
||||
// hot.colToProp resolves the column's actual configured data key -
|
||||
// Object.keys(clickedRow)[column] only worked by coincidence for a
|
||||
// fully-populated row; a sparse row (one whose backend source never
|
||||
// populated some earlier column) has fewer real keys than the grid has
|
||||
// columns, so counting into its own key list drifts out of alignment
|
||||
// (or runs out of bounds entirely) once column indexes past the gap.
|
||||
const clickedColumnKey = hot.colToProp(column) as string
|
||||
const skipRender = !!opts?.skipRender
|
||||
const myEpoch = this.validationEpoch
|
||||
|
||||
@@ -3874,11 +3887,18 @@ export class EditorComponent implements OnInit, AfterViewInit, OnDestroy {
|
||||
if (!isCharacterColumn(this.cols, colName)) continue
|
||||
|
||||
const { value, wasEscaped } = escapeCharacterColumnValue(row[colName])
|
||||
// A row whose backend source never had this column at all (a
|
||||
// sparse row) must stay that way - writing back unconditionally
|
||||
// would materialize a new own property set to undefined, which
|
||||
// the SASjs adapter rejects at submit time even though nothing
|
||||
// about this cell ever changed.
|
||||
if (!wasEscaped) continue
|
||||
|
||||
row[colName] = value
|
||||
if (this.dataSourceRaw[rowIndex]) {
|
||||
this.dataSourceRaw[rowIndex][colName] = value
|
||||
}
|
||||
if (wasEscaped) markAutoEscaped(this.autoEscapedCells, rowKey, colName)
|
||||
markAutoEscaped(this.autoEscapedCells, rowKey, colName)
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -946,3 +946,105 @@ describe('a primary key configured as a HARDFORMULA/SOFTFORMULA column freezes t
|
||||
hot.destroy()
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* A row whose backend source never included a given character column at
|
||||
* all (a sparse row - e.g. a table where only some rows use a particular
|
||||
* optional column) must stay that way after the escape loop runs, not
|
||||
* gain that key as a new own property set to undefined.
|
||||
* escapeCharacterColumnValue(undefined) legitimately returns
|
||||
* { value: undefined, wasEscaped: false } - there's nothing to escape -
|
||||
* but writing that back via row[colName] = value unconditionally
|
||||
* materializes a property the row never had. The SASjs adapter's own
|
||||
* request validation rejects any row holding an explicit `undefined`
|
||||
* value ("Can't assign undefined to <COL>"), so a submit fails for a
|
||||
* cell that was never touched at all. Mirrors initSetup/
|
||||
* getPendingExcelPreview's own escape loop.
|
||||
*/
|
||||
describe('the escape loop never materializes a missing column key as an explicit undefined value', () => {
|
||||
const runEscapeLoop = (dataSource: any[], headerColumns: string[]): void => {
|
||||
for (const row of dataSource) {
|
||||
for (const colName of headerColumns) {
|
||||
const { value, wasEscaped } = escapeCharacterColumnValue(row[colName])
|
||||
if (!wasEscaped) continue
|
||||
|
||||
row[colName] = value
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
it('leaves a genuinely-missing column key missing, not undefined, when nothing needs escaping', () => {
|
||||
const dataSource: any[] = [
|
||||
{ PRIMARY_KEY_FIELD: 1, PLAIN_TEXT_COL: 'note-1' }
|
||||
]
|
||||
// OTHER_COL is never a key on this row at all - this table's backend
|
||||
// source simply never populated it for this particular row.
|
||||
|
||||
runEscapeLoop(dataSource, [
|
||||
'PRIMARY_KEY_FIELD',
|
||||
'PLAIN_TEXT_COL',
|
||||
'OTHER_COL'
|
||||
])
|
||||
|
||||
expect(
|
||||
Object.prototype.hasOwnProperty.call(dataSource[0], 'OTHER_COL')
|
||||
).toEqual(false)
|
||||
})
|
||||
|
||||
it('still escapes and writes back a genuine =-led value', () => {
|
||||
const dataSource: any[] = [{ PRIMARY_KEY_FIELD: 1, PLAIN_TEXT_COL: '=1+1' }]
|
||||
|
||||
runEscapeLoop(dataSource, ['PRIMARY_KEY_FIELD', 'PLAIN_TEXT_COL'])
|
||||
|
||||
expect(dataSource[0].PLAIN_TEXT_COL).toEqual("'=1+1")
|
||||
})
|
||||
|
||||
it('leaves an existing plain (non-formula) value untouched, still present', () => {
|
||||
const dataSource: any[] = [
|
||||
{ PRIMARY_KEY_FIELD: 1, PLAIN_TEXT_COL: 'note-1' }
|
||||
]
|
||||
|
||||
runEscapeLoop(dataSource, ['PRIMARY_KEY_FIELD', 'PLAIN_TEXT_COL'])
|
||||
|
||||
expect(dataSource[0].PLAIN_TEXT_COL).toEqual('note-1')
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* Now that the escape loop above leaves a genuinely-missing column key
|
||||
* missing (instead of materializing it as undefined), a sparse row - one
|
||||
* whose backend source never populated some earlier column - has fewer
|
||||
* real keys than the grid has columns. dynamicCellValidation
|
||||
* (editor.component.ts) used to resolve the clicked column's name via
|
||||
* `Object.keys(clickedRow)[column]`, a positional guess that only worked
|
||||
* by coincidence when every row had the full set of keys. For a sparse
|
||||
* row it drifts out of alignment (or runs out of bounds entirely) once
|
||||
* the clicked column index passes the gap, resolving to `undefined` -
|
||||
* which then got sent as a literal `variable_nm: undefined` in a
|
||||
* getdynamiccolvals request, tripping the same SASjs adapter validation
|
||||
* ("Can't assign undefined to variable_nm"). hot.colToProp(column) reads
|
||||
* the column's actual configured data key instead, unaffected by the
|
||||
* row's own key count/order.
|
||||
*/
|
||||
describe("dynamicCellValidation resolves the clicked column by its Handsontable mapping, not by counting the row object's own keys", () => {
|
||||
it('demonstrates the failure mode: Object.keys(row)[column] runs out of bounds for a sparse row', () => {
|
||||
const hot = new Handsontable(document.createElement('div'), {
|
||||
data: [{ PRIMARY_KEY_FIELD: 1, LAST_COL: 'x' }], // MIDDLE_COL missing entirely
|
||||
columns: [
|
||||
{ data: 'PRIMARY_KEY_FIELD' },
|
||||
{ data: 'MIDDLE_COL' },
|
||||
{ data: 'LAST_COL' }
|
||||
],
|
||||
licenseKey: 'non-commercial-and-evaluation'
|
||||
})
|
||||
hot.render()
|
||||
|
||||
const lastColIndex = 2
|
||||
const clickedRow = { PRIMARY_KEY_FIELD: 1, LAST_COL: 'x' }
|
||||
|
||||
expect(Object.keys(clickedRow)[lastColIndex]).toBeUndefined()
|
||||
expect(hot.colToProp(lastColIndex)).toEqual('LAST_COL')
|
||||
|
||||
hot.destroy()
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user