From a104f78645bc7777fee672956d3f028ff52ed9ba Mon Sep 17 00:00:00 2001 From: YuryShkoda Date: Fri, 28 Aug 2026 12:42:02 +0300 Subject: [PATCH] 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. --- client/src/app/editor/editor.component.ts | 26 ++++- ...characterColumnFormula.integration.spec.ts | 102 ++++++++++++++++++ 2 files changed, 125 insertions(+), 3 deletions(-) diff --git a/client/src/app/editor/editor.component.ts b/client/src/app/editor/editor.component.ts index 05eec8e..b4fb258 100644 --- a/client/src/app/editor/editor.component.ts +++ b/client/src/app/editor/editor.component.ts @@ -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) } }) diff --git a/client/src/app/editor/utils/characterColumnFormula.integration.spec.ts b/client/src/app/editor/utils/characterColumnFormula.integration.spec.ts index dcd8959..fe98fdd 100644 --- a/client/src/app/editor/utils/characterColumnFormula.integration.spec.ts +++ b/client/src/app/editor/utils/characterColumnFormula.integration.spec.ts @@ -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 "), 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() + }) +})