fix(editor): clear sort before reading cells to preserve on cancel
cancelEdit()'s getFormulaCellsToPreserveOnCancel callbacks passed a physical rowIndex straight into visual-row-expecting Handsontable APIs (getCommentAtCell, getDataAtRowProp) while the grid could still be sorted, since clearSort() only ran later, right before updateSettings(). On a sorted grid this read the wrong row's comment/value, corrupting which formula cells got preserved across cancel. Move clearSort() (via getCurrentSortConfigs()) ahead of those callbacks instead.
This commit is contained in:
@@ -1228,12 +1228,6 @@ export class EditorComponent implements OnInit, AfterViewInit, OnDestroy {
|
||||
* not immediately after their own updateSettings() call the way this
|
||||
* wrapper restores it. Don't "simplify" those two to use this instead
|
||||
* without preserving that later restore point.
|
||||
*
|
||||
* This bug (and the visual/physical row translations elsewhere in this
|
||||
* file) was verified specifically against handsontable@18.0.0 - that's
|
||||
* why package.json pins it exactly rather than allowing ^18.0.0 to
|
||||
* float. Bumping the version should come with re-verifying this class of
|
||||
* bug still needs a workaround (or no longer does).
|
||||
*/
|
||||
private updateSettingsSortSafe(
|
||||
settings: Handsontable.GridSettings,
|
||||
@@ -1353,10 +1347,16 @@ export class EditorComponent implements OnInit, AfterViewInit, OnDestroy {
|
||||
|
||||
const hot = this.hotInstance
|
||||
const columnSorting = hot.getPlugin('multiColumnSorting')
|
||||
const columnSortConfig = columnSorting.getSortConfig()
|
||||
const sortConfigs = Array.isArray(columnSortConfig)
|
||||
? columnSortConfig
|
||||
: [columnSortConfig]
|
||||
const sortConfigs = this.getCurrentSortConfigs()
|
||||
|
||||
// Cleared here, before anything below reads a row by index - both the
|
||||
// getFormulaCellsToPreserveOnCancel callbacks just below (physical
|
||||
// rowIndex passed straight to visual-row-expecting Handsontable APIs)
|
||||
// and the later updateSettings() call (see editTable()'s own comment -
|
||||
// corrupts formula cell references while a sort is active) need the
|
||||
// grid unsorted. Restored below, after the re-render, from the
|
||||
// sortConfigs captured here.
|
||||
if (sortConfigs.length > 0) columnSorting.clearSort()
|
||||
|
||||
if (this.dataSourceUnchanged) {
|
||||
// dataSourceUnchanged deliberately holds the RAW pre-formula value for
|
||||
@@ -1392,12 +1392,10 @@ export class EditorComponent implements OnInit, AfterViewInit, OnDestroy {
|
||||
this.hotTable.data = this.dataSource
|
||||
this.hotTable.readOnly = true
|
||||
|
||||
// See editTable()'s own comment - updateSettings() while a sort is
|
||||
// still active corrupts formula cell references ('#REF!'), so clear it
|
||||
// first and restore sortConfigs afterward (below) instead of leaving
|
||||
// the sort applied across the call.
|
||||
if (sortConfigs.length > 0) columnSorting.clearSort()
|
||||
|
||||
// Sort was already cleared above (before the getFormulaCellsToPreserveOnCancel
|
||||
// callbacks); still cleared here going into updateSettings() - see
|
||||
// editTable()'s own comment: it corrupts formula cell references
|
||||
// ('#REF!') while a sort is active. Restored below via sortConfigs.
|
||||
hot.updateSettings(
|
||||
{
|
||||
readOnly: this.hotTable.readOnly,
|
||||
|
||||
@@ -4,6 +4,7 @@ import { classifyRow } from './classifyRow'
|
||||
import { normalizeSortConfig } from './normalizeSortConfig'
|
||||
import { EDIT_STATUS_COLUMN_NAME } from '../../shared/dc-validator/utils/editStatusColumnRule'
|
||||
import { parseFormulaRule } from '../../shared/dc-validator/utils/parseFormulaRule'
|
||||
import { getFormulaCellsToPreserveOnCancel } from '../../shared/dc-validator/utils/getFormulaCellsToPreserveOnCancel'
|
||||
|
||||
/**
|
||||
* afterChange delivers changes as [visualRow, prop, oldValue, newValue] -
|
||||
@@ -366,3 +367,103 @@ describe('updateSettings must not run while a sort is active - it corrupts formu
|
||||
hot.destroy()
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* getFormulaCellsToPreserveOnCancel's rowIndex parameter is established as
|
||||
* PHYSICAL (it iterates 0..dataSource.length, matching dataSource's own
|
||||
* order - see the file's own docblock). cancelEdit() passes that rowIndex
|
||||
* straight into commentsPlugin.getCommentAtCell and hot.getDataAtRowProp,
|
||||
* both of which - same as updateEditStatusForRow/syncOverwrittenCommentForCell
|
||||
* above - document a VISUAL row. On a sorted grid this both checks and
|
||||
* reads the wrong row's cell: a row that actually has a comment can go
|
||||
* unflagged while a different row gets wrongly flagged, carrying that other
|
||||
* row's live value onto its own prop - corrupting cancelEdit()'s restore
|
||||
* rather than just skipping it. The fix is to clear the sort before this
|
||||
* runs at all (cancelEdit() now does this - see its own comment), rather
|
||||
* than translate physical->visual per call, since dataSourceUnchanged's
|
||||
* restore right after also needs an unsorted physical dataSource to index
|
||||
* into.
|
||||
*/
|
||||
describe('getFormulaCellsToPreserveOnCancel callbacks resolve the correct cell on a sorted grid', () => {
|
||||
const formulaBaseCols = ['FORMULA_HARD_COL']
|
||||
|
||||
const setup = (): { hot: Handsontable } => {
|
||||
const dataSource = [
|
||||
{ PRIMARY_KEY_FIELD: 1, SOME_CHAR: 'b', FORMULA_HARD_COL: 10 },
|
||||
{ PRIMARY_KEY_FIELD: 2, SOME_CHAR: 'a', FORMULA_HARD_COL: 20 }
|
||||
]
|
||||
const hot = new Handsontable(document.createElement('div'), {
|
||||
data: dataSource,
|
||||
columns: [
|
||||
{ data: 'PRIMARY_KEY_FIELD' },
|
||||
{ data: 'SOME_CHAR' },
|
||||
{ data: 'FORMULA_HARD_COL' }
|
||||
],
|
||||
multiColumnSorting: true,
|
||||
comments: true,
|
||||
licenseKey: 'non-commercial-and-evaluation'
|
||||
})
|
||||
hot.render()
|
||||
|
||||
// Physical row 0 (PK=1) has the comment/live value that must be
|
||||
// preserved. Ascending by SOME_CHAR flips it to visual row 1; physical
|
||||
// row 1 (PK=2, no comment) becomes visual row 0.
|
||||
const commentsPlugin: any = hot.getPlugin('comments')
|
||||
commentsPlugin.setCommentAtCell(0, 2, 'Original value: 5')
|
||||
|
||||
const sortPlugin: any = hot.getPlugin('multiColumnSorting')
|
||||
sortPlugin.sort({ column: 1, sortOrder: 'asc' })
|
||||
hot.render()
|
||||
|
||||
return { hot }
|
||||
}
|
||||
|
||||
it('demonstrates the failure mode: physical rowIndex passed straight to visual-row APIs resolves the wrong cell', () => {
|
||||
const { hot } = setup()
|
||||
const commentsPlugin: any = hot.getPlugin('comments')
|
||||
|
||||
const toPreserve = getFormulaCellsToPreserveOnCancel(
|
||||
2,
|
||||
formulaBaseCols,
|
||||
(rowIndex, baseCol) =>
|
||||
!!commentsPlugin.getCommentAtCell(rowIndex, hot.propToCol(baseCol)),
|
||||
(rowIndex, prop) => hot.getDataAtRowProp(rowIndex, prop)
|
||||
)
|
||||
|
||||
// Physical row 0's comment lives at visual row 1 post-sort, but the
|
||||
// buggy call checked physical rowIndex 0 straight as a visual row -
|
||||
// landing on physical row 1's cell (no comment, value 20) instead.
|
||||
// rowIndex 1 then wrongly reads AS visual row 1, which is physical row
|
||||
// 0's actual commented cell (value 10) - so the wrong row (1, not 0)
|
||||
// gets flagged, carrying the wrong row's value.
|
||||
expect(toPreserve).toEqual([
|
||||
{ rowIndex: 1, prop: 'FORMULA_HARD_COL', value: 10 }
|
||||
])
|
||||
|
||||
hot.destroy()
|
||||
})
|
||||
|
||||
it('resolves the correct cell once the sort is cleared before reading', () => {
|
||||
const { hot } = setup()
|
||||
const commentsPlugin: any = hot.getPlugin('comments')
|
||||
const sortPlugin: any = hot.getPlugin('multiColumnSorting')
|
||||
const sortConfigs = normalizeSortConfig(sortPlugin.getSortConfig())
|
||||
|
||||
sortPlugin.clearSort()
|
||||
|
||||
const toPreserve = getFormulaCellsToPreserveOnCancel(
|
||||
2,
|
||||
formulaBaseCols,
|
||||
(rowIndex, baseCol) =>
|
||||
!!commentsPlugin.getCommentAtCell(rowIndex, hot.propToCol(baseCol)),
|
||||
(rowIndex, prop) => hot.getDataAtRowProp(rowIndex, prop)
|
||||
)
|
||||
|
||||
expect(toPreserve).toEqual([
|
||||
{ rowIndex: 0, prop: 'FORMULA_HARD_COL', value: 10 }
|
||||
])
|
||||
|
||||
for (const sc of sortConfigs) sortPlugin.sort(sc)
|
||||
hot.destroy()
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user