Skip to content

Commit

Permalink
[Lens] Handle invalid values gracefully for static value operation (#…
Browse files Browse the repository at this point in the history
…172198)

## Summary

Fixes #171959 

I've extended valid static value check to 15 digits (which is the max
support by JS as implementing the [64-bit IEEE
574](https://en.wikipedia.org/wiki/Double-precision_floating-point_format)).

<img width="1224" alt="Screenshot 2023-11-30 at 10 23 13"
src="https://github.com/elastic/kibana/assets/924948/bf88c0c8-9e51-4c8f-912d-abd82f292eda">

Note: an alternative approach would be to make it pass nonetheless and
trunc the numeric value at 15th digit.

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Stratoula Kalafateli <[email protected]>
  • Loading branch information
dej611 and stratoula authored Dec 1, 2023
1 parent 5ad3eec commit 1f8c816
Showing 1 changed file with 25 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,9 @@ export const staticValueOperation: OperationDefinition<
},
getErrorMessage(layer, columnId) {
const column = layer.columns[columnId] as StaticValueIndexPatternColumn;
const isValid = isValidNumber(column.params.value, false, undefined, undefined, 15);

return column.params.value != null && !isValidNumber(column.params.value)
return column.params.value != null && !isValid
? [
i18n.translate('xpack.lens.indexPattern.staticValueError', {
defaultMessage: 'The static value of {value} is not a valid number',
Expand All @@ -89,7 +90,8 @@ export const staticValueOperation: OperationDefinition<
const params = currentColumn.params;
// TODO: improve this logic
const useDisplayLabel = currentColumn.label !== defaultLabel;
const label = isValidNumber(params.value)
const isValid = isValidNumber(params.value, false, undefined, undefined, 15);
const label = isValid
? useDisplayLabel
? currentColumn.label
: params?.value ?? defaultLabel
Expand All @@ -98,11 +100,11 @@ export const staticValueOperation: OperationDefinition<
return [
{
type: 'function',
function: isValidNumber(params.value) ? 'mathColumn' : 'mapColumn',
function: isValid ? 'mathColumn' : 'mapColumn',
arguments: {
id: [columnId],
name: [label || defaultLabel],
expression: [String(isValidNumber(params.value) ? params.value! : defaultValue)],
expression: [String(isValid ? params.value! : defaultValue)],
},
},
];
Expand Down Expand Up @@ -163,7 +165,10 @@ export const staticValueOperation: OperationDefinition<
const onChange = useCallback(
(newValue) => {
// even if debounced it's triggering for empty string with the previous valid value
if (currentColumn.params.value === newValue) {
if (
currentColumn.params.value === newValue ||
!isValidNumber(newValue, false, undefined, undefined, 15)
) {
return;
}
// Because of upstream specific UX flows, we need fresh layer state here
Expand Down Expand Up @@ -209,13 +214,26 @@ export const staticValueOperation: OperationDefinition<
const onChangeHandler = useCallback(
(e: React.ChangeEvent<HTMLInputElement>) => {
const value = e.currentTarget.value;
handleInputChange(isValidNumber(value) ? value : undefined);
handleInputChange(value);
},
[handleInputChange]
);

const inputValueIsValid = isValidNumber(inputValue, false, undefined, undefined, 15);

return (
<EuiFormRow label={paramEditorCustomProps?.labels?.[0] || defaultLabel} fullWidth>
<EuiFormRow
label={paramEditorCustomProps?.labels?.[0] || defaultLabel}
fullWidth
isInvalid={!inputValueIsValid}
error={
!inputValueIsValid &&
i18n.translate('xpack.lens.indexPattern.staticValueError', {
defaultMessage: 'The static value of {value} is not a valid number',
values: { value: inputValue ?? "''" },
})
}
>
<EuiFieldNumber
fullWidth
data-test-subj="lns-indexPattern-static_value-input"
Expand Down

0 comments on commit 1f8c816

Please sign in to comment.