Skip to content

Commit

Permalink
fix: Conditional date formatting (#1104)
Browse files Browse the repository at this point in the history
Conditional formatting for date columns now properly prevents conditions
with empty values from being applied.
  • Loading branch information
bmingles authored Feb 27, 2023
1 parent 144605a commit 2f503ba
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {
BooleanCondition,
CharCondition,
getLabelForCharCondition,
isDateConditionValid,
getDefaultValueForType,
} from './ConditionalFormattingUtils';

const log = Log.module('ConditionEditor');
Expand Down Expand Up @@ -262,7 +264,7 @@ function ConditionEditor(props: ConditionEditorProps): JSX.Element {
if (selectedColumnType !== prevColumnType) {
// Column type changed, reset condition and value fields
setCondition(getDefaultConditionForType(selectedColumnType));
setValue(undefined);
setValue(getDefaultValueForType(selectedColumnType));
setStartValue(undefined);
setEndValue(undefined);
setPrevColumnType(selectedColumnType);
Expand Down Expand Up @@ -322,9 +324,7 @@ function ConditionEditor(props: ConditionEditorProps): JSX.Element {
'Unable to create formatting rule. Condition is not selected.'
);
isValid = false;
}

if (
} else if (
TableUtils.isNumberType(column.type) &&
!isNumberConditionValid(
selectedCondition as NumberCondition,
Expand All @@ -338,7 +338,20 @@ function ConditionEditor(props: ConditionEditorProps): JSX.Element {
conditionValue
);
isValid = false;
} else if (
TableUtils.isDateType(column.type) &&
!isDateConditionValid(
selectedCondition as DateCondition,
conditionValue
)
) {
log.debug(
'Unable to create formatting rule. Invalid date condition',
conditionValue
);
isValid = false;
}

onChange(
{
condition: selectedCondition,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { Column } from '@deephaven/jsapi-shim';
import IrisGridTestUtils from '../../IrisGridTestUtils';
import {
DateCondition,
FormatStyleType,
FormatterType,
FormattingRule,
getFormatColumns,
isDateConditionValid,
StringCondition,
} from './ConditionalFormattingUtils';

Expand Down Expand Up @@ -215,3 +217,46 @@ describe('getFormatColumns', () => {
]);
});
});

describe('isDateConditionValid', () => {
const values = {
valid: '2023-02-23T11:46:31.000000000 NY',
invalid: 'blah',
empty: '',
undefined,
};

it.each([DateCondition.IS_NULL, DateCondition.IS_NOT_NULL])(
'should return true for null check conditions: %s',
condition => {
const testValues = [
values.valid,
values.invalid,
values.empty,
values.undefined,
];

testValues.forEach(value => {
expect(isDateConditionValid(condition, value)).toBeTruthy();
});
}
);

it.each([
DateCondition.IS_AFTER,
DateCondition.IS_AFTER_OR_EQUAL,
DateCondition.IS_BEFORE_OR_EQUAL,
DateCondition.IS_BEFORE,
DateCondition.IS_EXACTLY,
DateCondition.IS_NOT_EXACTLY,
])(
'should return false for empty value when condition requires it: %s',
condition => {
const testValues = [values.empty, values.undefined];

testValues.forEach(value => {
expect(isDateConditionValid(condition, value)).toBeFalsy();
});
}
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,23 @@ export function getFormatColumns(
return result;
}

/**
* Validate that a given date condition + value pair is valid.
* @param condition
* @param value
*/
export function isDateConditionValid(condition: DateCondition, value?: string) {
switch (condition) {
case DateCondition.IS_NULL:
case DateCondition.IS_NOT_NULL:
return true;

default:
// Proper date validation will be addressed by Issue #1108
return value != null && value !== '';
}
}

export function isSupportedColumn({ type }: ModelColumn): boolean {
return (
TableUtils.isNumberType(type) ||
Expand Down

0 comments on commit 2f503ba

Please sign in to comment.