Merge pull request 'feat(editor): convert CASE rule values on entry' (#447) from feat/372-case-transform-on-entry into release/7.22
Reviewed-on: #447
This commit was merged in pull request #447.
This commit is contained in:
commit
f24a02c47e
7 files changed
+457
No files matched your search
@@ -59,6 +59,10 @@ import { DcValidation } from '../shared/dc-validator/models/dc-validation.model'
|
||||
import { DQRule } from '../shared/dc-validator/models/dq-rules.model'
|
||||
import { getHotDataSchema } from '../shared/dc-validator/utils/getHotDataSchema'
|
||||
import { excelRound } from '../shared/dc-validator/utils/excelRound'
|
||||
import {
|
||||
applyCaseTransform,
|
||||
isCaseTransformSkipped
|
||||
} from '../shared/dc-validator/utils/caseTransform'
|
||||
import { isEmpty } from '../shared/dc-validator/utils/isEmpty'
|
||||
import { hasFormulaRules } from '../shared/dc-validator/utils/hasFormulaRules'
|
||||
import { getFormulaColumnNames } from '../shared/dc-validator/utils/getFormulaColumnNames'
|
||||
@@ -4299,6 +4303,23 @@ export class EditorComponent
|
||||
if (col === firstDerivedCol) th.classList.add('dc-derived-first')
|
||||
}
|
||||
|
||||
// A column whose CASE rule forces a case commits every entry in that
|
||||
// case (see the beforeChange hook) - mark the header so the user can
|
||||
// tell the capitals or the lower-case came from the grid and not
|
||||
// from them. The cell shows the value the grid actually holds, so
|
||||
// nothing here re-cases a value the user has not touched.
|
||||
const caseTransform = column
|
||||
? this.dcValidator?.getCaseTransform(column)
|
||||
: undefined
|
||||
if (caseTransform) {
|
||||
th.classList.add('dc-case-header')
|
||||
const caseHint =
|
||||
caseTransform === 'upper'
|
||||
? 'Values are converted to upper case on entry'
|
||||
: 'Values are converted to lower case on entry'
|
||||
th.title = th.title ? `${th.title}; ${caseHint}` : caseHint
|
||||
}
|
||||
|
||||
// Remove header arrow from Delete column
|
||||
if (col === 0) {
|
||||
th.classList.add('firstColumnHeaderStyle')
|
||||
@@ -4486,6 +4507,36 @@ export class EditorComponent
|
||||
}
|
||||
}
|
||||
|
||||
// CASE=UPCASE / CASE=LOWCASE: commit the value in the case the rule
|
||||
// requires, so it is not flagged as invalid and retyped. Applied here
|
||||
// (rather than in afterChange) because it runs before the commit - the
|
||||
// cursor and selection stay put, undo restores the previous value, and
|
||||
// an edit, a paste range and a fill-down all take the same path. The
|
||||
// CASE validation itself still stands for values that arrive from
|
||||
// anywhere else (API, staged data, Excel import).
|
||||
//
|
||||
// Two exclusions. `loadData` is the grid re-sending its own dataset on
|
||||
// every updateSettings({data}) - converting there would rewrite the
|
||||
// service's values and show rows as modified before the user has
|
||||
// touched them (the unchanged baseline is cloned from what the grid
|
||||
// hands back). Undo and redo restore a previous value rather than
|
||||
// entering a new one, and re-casing it would hand back something other
|
||||
// than what the user asked to get back. A formula column holds a live
|
||||
// `=...` string, which is re-cased into nonsense - HARDFORMULA is
|
||||
// read-only so it never reaches here, but SOFTFORMULA is editable.
|
||||
if (!isCaseTransformSkipped(source) && typeof newValue === 'string') {
|
||||
const caseTransform = this.dcValidator?.getCaseTransform(colName)
|
||||
if (
|
||||
caseTransform &&
|
||||
!this.dcValidator?.hasDqRules(colName, [
|
||||
'HARDFORMULA',
|
||||
'SOFTFORMULA'
|
||||
])
|
||||
) {
|
||||
change[3] = applyCaseTransform(newValue, caseTransform)
|
||||
}
|
||||
}
|
||||
|
||||
// A `=`-led value the user types or pastes is never auto-escaped
|
||||
// (see character-column-formula-plan.md) - it's left exactly as
|
||||
// entered, which genuinely evaluates as a live formula since
|
||||
|
||||
@@ -0,0 +1,192 @@
|
||||
import Handsontable from 'handsontable'
|
||||
import {
|
||||
applyCaseTransform,
|
||||
CaseTransform,
|
||||
isCaseTransformSkipped
|
||||
} from '../../shared/dc-validator/utils/caseTransform'
|
||||
|
||||
/**
|
||||
* Mechanics coverage for the CASE rule transform, against a real Handsontable
|
||||
* instance - the same technique the other grid specs in this folder use.
|
||||
*
|
||||
* editor.component.ts has no TestBed harness, so the hook below mirrors its
|
||||
* beforeChange branch using the real transform function: what is being proven
|
||||
* here is the part that belongs to Handsontable and is easy to get wrong - that
|
||||
* mutating the pending change commits the converted value for an edit, a paste
|
||||
* range and a fill-down, that it does not touch a dataset handed back to the
|
||||
* grid on load, and that undo still restores what was there before.
|
||||
*/
|
||||
describe('CASE rule transform on entry integration', () => {
|
||||
// the rule table DcValidator.getCaseTransform builds from MPE_VALIDATIONS
|
||||
const caseRules: Record<string, CaseTransform> = {
|
||||
UPPER_COL: 'upper',
|
||||
LOWER_COL: 'lower'
|
||||
}
|
||||
// a SOFTFORMULA column is user-editable and holds a live formula string
|
||||
const formulaCols = ['SOFT_FORMULA_COL']
|
||||
|
||||
const columnNames = [
|
||||
'KEY',
|
||||
'UPPER_COL',
|
||||
'LOWER_COL',
|
||||
'LABEL_COL',
|
||||
'SOFT_FORMULA_COL'
|
||||
]
|
||||
|
||||
let container: HTMLDivElement
|
||||
let hot: Handsontable
|
||||
|
||||
const buildGrid = (data: Record<string, any>[]) => {
|
||||
hot = new Handsontable(container, {
|
||||
data,
|
||||
columns: columnNames.map((name) => ({ data: name })),
|
||||
licenseKey: 'non-commercial-and-evaluation'
|
||||
})
|
||||
|
||||
// mirrors editor.component.ts's beforeChange branch
|
||||
hot.addHook('beforeChange', (changes: any[], source: string) => {
|
||||
if (!changes || isCaseTransformSkipped(source)) return
|
||||
|
||||
for (const change of changes) {
|
||||
if (!change) continue
|
||||
|
||||
const [, prop, , newValue] = change
|
||||
const colName =
|
||||
typeof prop === 'string' ? prop : (hot.colToProp(prop) as string)
|
||||
|
||||
if (typeof newValue !== 'string') continue
|
||||
if (formulaCols.includes(colName)) continue
|
||||
|
||||
change[3] = applyCaseTransform(newValue, caseRules[colName])
|
||||
}
|
||||
})
|
||||
|
||||
hot.render()
|
||||
|
||||
return hot
|
||||
}
|
||||
|
||||
const row = () => ({
|
||||
KEY: 1,
|
||||
UPPER_COL: 'keep',
|
||||
LOWER_COL: 'KEEP',
|
||||
LABEL_COL: 'Keep Me',
|
||||
SOFT_FORMULA_COL: '=upper(KEY)'
|
||||
})
|
||||
|
||||
beforeEach(() => {
|
||||
container = document.createElement('div')
|
||||
document.body.appendChild(container)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
hot?.destroy()
|
||||
container.remove()
|
||||
})
|
||||
|
||||
it('commits a typed value in the case the column rule requires', () => {
|
||||
buildGrid([row()])
|
||||
|
||||
hot.setDataAtCell(0, 1, 'typed lower', 'edit')
|
||||
hot.setDataAtCell(0, 2, 'TYPED UPPER', 'edit')
|
||||
|
||||
expect(hot.getDataAtRowProp(0, 'UPPER_COL')).toEqual('TYPED LOWER')
|
||||
expect(hot.getDataAtRowProp(0, 'LOWER_COL')).toEqual('typed upper')
|
||||
})
|
||||
|
||||
it('converts every cell of a pasted range', () => {
|
||||
buildGrid([row(), row()])
|
||||
|
||||
hot.populateFromArray(
|
||||
0,
|
||||
1,
|
||||
[
|
||||
['one', 'ONE'],
|
||||
['two', 'TWO']
|
||||
],
|
||||
1,
|
||||
2,
|
||||
'paste'
|
||||
)
|
||||
|
||||
expect(hot.getDataAtRowProp(0, 'UPPER_COL')).toEqual('ONE')
|
||||
expect(hot.getDataAtRowProp(1, 'UPPER_COL')).toEqual('TWO')
|
||||
expect(hot.getDataAtRowProp(0, 'LOWER_COL')).toEqual('one')
|
||||
expect(hot.getDataAtRowProp(1, 'LOWER_COL')).toEqual('two')
|
||||
})
|
||||
|
||||
it('converts a value filled down into the cells below', () => {
|
||||
buildGrid([row(), row(), row()])
|
||||
|
||||
hot.populateFromArray(0, 1, [['seed']], 2, 1, 'autofill')
|
||||
|
||||
expect(hot.getDataAtRowProp(0, 'UPPER_COL')).toEqual('SEED')
|
||||
expect(hot.getDataAtRowProp(1, 'UPPER_COL')).toEqual('SEED')
|
||||
expect(hot.getDataAtRowProp(2, 'UPPER_COL')).toEqual('SEED')
|
||||
})
|
||||
|
||||
it('leaves the case of the dataset the grid is handed on load alone', () => {
|
||||
const data = [row()]
|
||||
|
||||
buildGrid(data)
|
||||
|
||||
// the service's own values, before the user has touched anything - a
|
||||
// conversion here would rewrite backend data and mark the row modified
|
||||
expect(hot.getDataAtRowProp(0, 'UPPER_COL')).toEqual('keep')
|
||||
expect(hot.getDataAtRowProp(0, 'LOWER_COL')).toEqual('KEEP')
|
||||
|
||||
// the same holds for a later bulk data update (add-row flow, Excel preview)
|
||||
hot.updateSettings({ data: [row()] })
|
||||
|
||||
expect(hot.getDataAtRowProp(0, 'UPPER_COL')).toEqual('keep')
|
||||
expect(hot.getDataAtRowProp(0, 'LOWER_COL')).toEqual('KEEP')
|
||||
})
|
||||
|
||||
it('leaves a column without a CASE rule, and a formula column, exactly as entered', () => {
|
||||
buildGrid([row()])
|
||||
|
||||
hot.setDataAtCell(0, 3, 'Mixed Case Label', 'edit')
|
||||
hot.setDataAtCell(0, 4, '=UPPER(KEY)', 'edit')
|
||||
|
||||
expect(hot.getDataAtRowProp(0, 'LABEL_COL')).toEqual('Mixed Case Label')
|
||||
expect(hot.getDataAtRowProp(0, 'SOFT_FORMULA_COL')).toEqual('=UPPER(KEY)')
|
||||
})
|
||||
|
||||
it('undo restores the value the cell held before the converted entry', () => {
|
||||
buildGrid([row()])
|
||||
|
||||
hot.setDataAtCell(0, 1, 'replaced', 'edit')
|
||||
expect(hot.getDataAtRowProp(0, 'UPPER_COL')).toEqual('REPLACED')
|
||||
|
||||
hot.getPlugin('undoRedo').undo()
|
||||
|
||||
// exactly what was there - the restore takes the 'UndoRedo.undo' source,
|
||||
// which is not user entry and is therefore not converted. A value from the
|
||||
// service that did not satisfy the rule comes back as it arrived, not
|
||||
// silently re-cased into one that does.
|
||||
expect(hot.getDataAtRowProp(0, 'UPPER_COL')).toEqual('keep')
|
||||
})
|
||||
|
||||
it('redo re-applies the converted value', () => {
|
||||
buildGrid([row()])
|
||||
|
||||
hot.setDataAtCell(0, 1, 'replaced', 'edit')
|
||||
hot.getPlugin('undoRedo').undo()
|
||||
hot.getPlugin('undoRedo').redo()
|
||||
|
||||
expect(hot.getDataAtRowProp(0, 'UPPER_COL')).toEqual('REPLACED')
|
||||
})
|
||||
|
||||
it('leaves the value Revert puts back exactly as the service supplied it', () => {
|
||||
buildGrid([row()])
|
||||
|
||||
hot.setDataAtCell(0, 1, 'typed', 'edit')
|
||||
|
||||
// the Revert menu item writes the service's own value with this source -
|
||||
// converting it would leave the cell modified against the unchanged
|
||||
// baseline, so the revert would not have reverted
|
||||
hot.setDataAtCell(0, 1, 'keep', 'revert')
|
||||
|
||||
expect(hot.getDataAtRowProp(0, 'UPPER_COL')).toEqual('keep')
|
||||
})
|
||||
})
|
||||
@@ -14,6 +14,7 @@ import {
|
||||
DcValidationRuleUpdate
|
||||
} from './models/dc-validation.model'
|
||||
import { DQRule, DQRuleTypes } from './models/dq-rules.model'
|
||||
import { CaseTransform } from './utils/caseTransform'
|
||||
import { getDqDataCols } from './utils/getDqDataCols'
|
||||
import { getNotNullDefault } from './utils/getNotNullDefault'
|
||||
import { mergeColsRules } from './utils/mergeColsRules'
|
||||
@@ -247,6 +248,39 @@ export class DcValidator {
|
||||
return isNaN(digits) ? undefined : digits
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the case transform a `CASE` rule asks for on the given column, so
|
||||
* the EDIT grid can commit the value in that case instead of flagging it.
|
||||
*
|
||||
* Only the two rule values the CASE validation understands are mapped
|
||||
* (`UPCASE`, `LOWCASE`, see dq-validation.ts) - anything else leaves the
|
||||
* value alone, which keeps the transform and the validation in step.
|
||||
*
|
||||
* @param col column name or index
|
||||
* @returns 'upper', 'lower', or undefined when the column carries no usable
|
||||
* CASE rule
|
||||
*/
|
||||
getCaseTransform(col: string | number): CaseTransform | undefined {
|
||||
const rule = this.getRule(col)
|
||||
|
||||
// a column that is not in the grid has no data name to match a rule on
|
||||
if (!rule?.data) return undefined
|
||||
|
||||
const caseRule = this.dqrules.find(
|
||||
(dqrule: DQRule) =>
|
||||
dqrule.BASE_COL === rule.data && dqrule.RULE_TYPE === 'CASE'
|
||||
)
|
||||
|
||||
switch (caseRule?.RULE_VALUE) {
|
||||
case 'UPCASE':
|
||||
return 'upper'
|
||||
case 'LOWCASE':
|
||||
return 'lower'
|
||||
default:
|
||||
return undefined
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the RULE_VALUEs of a HARDREGEX/SOFTREGEX rule on the given
|
||||
* column, for display in the column-header info dropdown. Both are
|
||||
|
||||
@@ -1041,6 +1041,66 @@ describe('DC Validator', () => {
|
||||
expect(dcValidator.getFormulaRuleValue('SOME_CHAR_ANY')).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('getCaseTransform', () => {
|
||||
const buildValidator = (dqRules: DQRule[]) =>
|
||||
new DcValidator(
|
||||
example_sasparams,
|
||||
example_dataformats,
|
||||
example_cols,
|
||||
dqRules,
|
||||
example_dqData
|
||||
)
|
||||
|
||||
it('maps CASE=UPCASE to upper and CASE=LOWCASE to lower', () => {
|
||||
const dcValidator = buildValidator(example_dqRules)
|
||||
|
||||
expect(dcValidator.getCaseTransform('SOME_CHAR')).toEqual('upper')
|
||||
expect(dcValidator.getCaseTransform('SOME_CHAR_LOW')).toEqual('lower')
|
||||
})
|
||||
|
||||
it('resolves a column index as well as a name', () => {
|
||||
const dcValidator = buildValidator(example_dqRules)
|
||||
|
||||
// 2 is SOME_CHAR in COLHEADERS - the grid hands the hook a prop index
|
||||
// for some sources
|
||||
expect(dcValidator.getCaseTransform(2)).toEqual('upper')
|
||||
})
|
||||
|
||||
it('returns undefined for a column with no CASE rule', () => {
|
||||
const dcValidator = buildValidator(example_dqRules)
|
||||
|
||||
expect(dcValidator.getCaseTransform('SOME_NUM')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('returns undefined for a column that is not in the grid', () => {
|
||||
const dcValidator = buildValidator([
|
||||
{
|
||||
BASE_COL: 'NOT_IN_THE_GRID',
|
||||
RULE_TYPE: 'CASE',
|
||||
RULE_VALUE: 'UPCASE',
|
||||
X: 0
|
||||
}
|
||||
])
|
||||
|
||||
expect(dcValidator.getCaseTransform('NOT_IN_THE_GRID')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('returns undefined for a CASE rule value the validation does not apply', () => {
|
||||
// dq-validation.ts only acts on UPCASE and LOWCASE; converting a value
|
||||
// for any other rule value would be a transform the rule never asked for
|
||||
const dcValidator = buildValidator([
|
||||
{
|
||||
BASE_COL: 'SOME_CHAR_ANY',
|
||||
RULE_TYPE: 'CASE',
|
||||
RULE_VALUE: 'TITLECASE',
|
||||
X: 0
|
||||
}
|
||||
])
|
||||
|
||||
expect(dcValidator.getCaseTransform('SOME_CHAR_ANY')).toBeUndefined()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
/** Minimal cols[] entry — only the fields rule ordering depends on. */
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
import { applyCaseTransform, isCaseTransformSkipped } from './caseTransform'
|
||||
|
||||
describe('applyCaseTransform', () => {
|
||||
it('upper-cases a string when the column rule is UPCASE', () => {
|
||||
expect(applyCaseTransform('abc', 'upper')).toEqual('ABC')
|
||||
expect(applyCaseTransform('Mixed Case 1', 'upper')).toEqual('MIXED CASE 1')
|
||||
})
|
||||
|
||||
it('lower-cases a string when the column rule is LOWCASE', () => {
|
||||
expect(applyCaseTransform('ABC', 'lower')).toEqual('abc')
|
||||
expect(applyCaseTransform('Mixed Case 1', 'lower')).toEqual('mixed case 1')
|
||||
})
|
||||
|
||||
it('leaves the value alone when the column carries no CASE rule', () => {
|
||||
expect(applyCaseTransform('aBc', undefined)).toEqual('aBc')
|
||||
})
|
||||
|
||||
it('leaves a value that is not a string alone', () => {
|
||||
// a numeric column can still carry a CASE rule by mistake - it must not
|
||||
// become the string '5'
|
||||
expect(applyCaseTransform(5, 'upper')).toEqual(5)
|
||||
expect(applyCaseTransform(5.5, 'lower')).toEqual(5.5)
|
||||
})
|
||||
|
||||
it('leaves blank and missing values alone', () => {
|
||||
expect(applyCaseTransform('', 'upper')).toEqual('')
|
||||
expect(applyCaseTransform(null, 'upper')).toBeNull()
|
||||
expect(applyCaseTransform(undefined, 'lower')).toBeUndefined()
|
||||
// a SAS special missing is a character value on a character column, and
|
||||
// stays exactly as typed
|
||||
expect(applyCaseTransform('.a', 'upper')).toEqual('.A')
|
||||
})
|
||||
})
|
||||
|
||||
describe('isCaseTransformSkipped', () => {
|
||||
it('skips a dataset handed back to the grid on load', () => {
|
||||
expect(isCaseTransformSkipped('loadData')).toBeTrue()
|
||||
})
|
||||
|
||||
it('skips undo and redo, which restore a value rather than enter one', () => {
|
||||
expect(isCaseTransformSkipped('UndoRedo.undo')).toBeTrue()
|
||||
expect(isCaseTransformSkipped('UndoRedo.redo')).toBeTrue()
|
||||
})
|
||||
|
||||
it('skips the Revert menu item, which puts the service value back', () => {
|
||||
expect(isCaseTransformSkipped('revert')).toBeTrue()
|
||||
})
|
||||
|
||||
it('does not skip a value the user enters', () => {
|
||||
expect(isCaseTransformSkipped('edit')).toBeFalse()
|
||||
expect(isCaseTransformSkipped('paste')).toBeFalse()
|
||||
expect(isCaseTransformSkipped('autofill')).toBeFalse()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,55 @@
|
||||
/**
|
||||
* The transform a `CASE` validation asks for, as the grid applies it on entry.
|
||||
*
|
||||
* `CASE=UPCASE` and `CASE=LOWCASE` are point-of-entry rules: a value in the
|
||||
* other case is rejected by the validator, so the user is asked to retype it.
|
||||
* The EDIT grid instead converts the pending value before it commits, so the
|
||||
* typed text lands in the case the rule wants.
|
||||
*/
|
||||
export type CaseTransform = 'upper' | 'lower'
|
||||
|
||||
/**
|
||||
* Change sources whose values must not be re-cased.
|
||||
*
|
||||
* - `loadData` is the grid re-sending its own dataset on every
|
||||
* `updateSettings({data})` (initial render, the add-row flow, the Excel
|
||||
* preview). Converting there would rewrite the values the service supplied
|
||||
* and show rows as modified before the user has touched them.
|
||||
* - `UndoRedo.undo` / `UndoRedo.redo` restore the value the cell held rather
|
||||
* than entering a new one. Re-casing a restored value would hand back
|
||||
* something different from what the user asked to get back - and for a
|
||||
* backend value that does not satisfy the rule it would quietly replace it
|
||||
* with one that does.
|
||||
* - `revert` is the Revert menu item, which puts back the value the service
|
||||
* supplied for that cell. Converting it would leave the cell modified
|
||||
* against the unchanged baseline, i.e. the revert would not have reverted.
|
||||
*/
|
||||
export const CASE_TRANSFORM_SKIPPED_SOURCES = [
|
||||
'loadData',
|
||||
'UndoRedo.undo',
|
||||
'UndoRedo.redo',
|
||||
'revert'
|
||||
]
|
||||
|
||||
export const isCaseTransformSkipped = (source: string): boolean =>
|
||||
CASE_TRANSFORM_SKIPPED_SOURCES.includes(source)
|
||||
|
||||
/**
|
||||
* Applies the column's case transform to a pending cell value.
|
||||
*
|
||||
* Only a string is converted: a numeric value, a SAS missing, `null`,
|
||||
* `undefined` and a blank all pass through untouched, so the transform can be
|
||||
* applied to every change in a batch without inspecting the column type.
|
||||
*
|
||||
* Returns the value to commit - the caller assigns it back onto the pending
|
||||
* change, rather than writing the cell, so the grid keeps its cursor,
|
||||
* selection and undo history.
|
||||
*/
|
||||
export const applyCaseTransform = (
|
||||
value: unknown,
|
||||
transform: CaseTransform | undefined
|
||||
): unknown => {
|
||||
if (!transform || typeof value !== 'string') return value
|
||||
|
||||
return transform === 'upper' ? value.toUpperCase() : value.toLowerCase()
|
||||
}
|
||||
@@ -5057,6 +5057,17 @@ body[cds-theme='dark'] {
|
||||
border-left: 2px solid #1a6ea3 !important;
|
||||
}
|
||||
|
||||
/* a column whose CASE validation forces a case: every entry is converted to
|
||||
it as it is committed, so the header says so rather than the cell pretending
|
||||
to hold a value it does not */
|
||||
.handsontable th.dc-case-header div::after {
|
||||
content: 'Aa';
|
||||
margin-left: 4px;
|
||||
font-size: 9px;
|
||||
font-style: italic;
|
||||
opacity: 0.7;
|
||||
}
|
||||
|
||||
.handsontable td.dc-derived-cell.dc-derived-first {
|
||||
box-shadow: inset 2px 0 0 #1a6ea3;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user