Skip to content

Commit

Permalink
Fix error states
Browse files Browse the repository at this point in the history
  • Loading branch information
wylieconlon committed Dec 18, 2020
1 parent 742fb94 commit 8820866
Show file tree
Hide file tree
Showing 11 changed files with 269 additions and 121 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -345,4 +345,90 @@ describe('reference editor', () => {
expect(fieldSelect.prop('selectedOperationType')).toEqual('avg');
expect(fieldSelect.prop('incompleteOperation')).toBeUndefined();
});

it('should show the FieldSelect as invalid in the empty state for field-only forms', () => {
wrapper = mount(
<ReferenceEditor
{...getDefaultArgs()}
selectionStyle="field"
validation={{
input: ['field'],
validateMetadata: (meta: OperationMetadata) => true,
}}
/>
);

const fieldSelect = wrapper.find(FieldSelect);
expect(fieldSelect.prop('fieldIsInvalid')).toEqual(true);
expect(fieldSelect.prop('selectedField')).toBeUndefined();
expect(fieldSelect.prop('selectedOperationType')).toBeUndefined();
expect(fieldSelect.prop('incompleteOperation')).toBeUndefined();
});

it('should show the ParamEditor for functions that offer one', () => {
wrapper = mount(
<ReferenceEditor
{...getDefaultArgs()}
layer={{
indexPatternId: '1',
columnOrder: ['ref'],
columns: {
ref: {
label: 'Last value of bytes',
dataType: 'number',
isBucketed: false,
operationType: 'last_value',
sourceField: 'bytes',
params: {
sortField: 'timestamp',
},
},
},
}}
validation={{
input: ['field'],
validateMetadata: (meta: OperationMetadata) => true,
}}
/>
);

expect(wrapper.find('[data-test-subj="lns-indexPattern-lastValue-sortField"]').exists()).toBe(
true
);
});

it('should hide the ParamEditor for incomplete functions', () => {
wrapper = mount(
<ReferenceEditor
{...getDefaultArgs()}
layer={{
indexPatternId: '1',
columnOrder: ['ref'],
columns: {
ref: {
label: 'Last value of bytes',
dataType: 'number',
isBucketed: false,
operationType: 'last_value',
sourceField: 'bytes',
params: {
sortField: 'timestamp',
},
},
},
incompleteColumns: {
ref: { operationType: 'max' },
},
}}
validation={{
input: ['field'],
validateMetadata: (meta: OperationMetadata) => true,
}}
/>
);

expect(wrapper.find('[data-test-subj="lns-indexPattern-lastValue-sortField"]').exists()).toBe(
false
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,14 @@ export function ReferenceEditor(props: ReferenceEditorProps) {
? [functionOptions.find(({ value }) => value === column.operationType)!]
: [];

// If the operationType is incomplete, the user needs to select a field- so
// the function is marked as valid.
const showOperationInvalid = !column && !Boolean(incompleteInfo?.operationType);
// The field is invalid if the operation has been updated without a field,
// or if we are in a field-only mode but empty state
const showFieldInvalid =
Boolean(incompleteInfo?.operationType) || (selectionStyle === 'field' && !column);

return (
<div id={columnId}>
<div>
Expand All @@ -188,11 +196,7 @@ export function ReferenceEditor(props: ReferenceEditorProps) {
defaultMessage: 'Choose a sub-function',
})}
fullWidth
isInvalid={
// If the operationType is incomplete, the user needs to select a field- so
// the function is marked as valid.
!column && !Boolean(incompleteInfo?.operationType)
}
isInvalid={showOperationInvalid}
>
<EuiComboBox
fullWidth
Expand All @@ -206,7 +210,7 @@ export function ReferenceEditor(props: ReferenceEditorProps) {
}
)}
options={functionOptions}
isInvalid={!column && !Boolean(incompleteInfo?.operationType)}
isInvalid={showOperationInvalid}
selectedOptions={selectedOption}
singleSelection={{ asPlainText: true }}
onChange={(choices) => {
Expand Down Expand Up @@ -234,10 +238,10 @@ export function ReferenceEditor(props: ReferenceEditorProps) {
defaultMessage: 'Select a field',
})}
fullWidth
isInvalid={Boolean(incompleteInfo?.operationType)}
isInvalid={showFieldInvalid}
>
<FieldSelect
fieldIsInvalid={Boolean(incompleteInfo?.operationType)}
fieldIsInvalid={showFieldInvalid}
currentIndexPattern={currentIndexPattern}
existingFields={existingFields}
operationSupportMatrix={operationSupportMatrix}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export const {
getErrorMessages,
isReferenced,
resetIncomplete,
isColumnValidAsReference,
isOperationAllowedAsReference,
} = actualHelpers;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { FormattedIndexPatternColumn, ReferenceBasedIndexPatternColumn } from '.
import { IndexPatternLayer } from '../../../types';
import {
buildLabelFunction,
getErrorsForDateReference,
checkForDateHistogram,
dateBasedOperationToExpression,
hasDateField,
Expand Down Expand Up @@ -89,9 +90,10 @@ export const counterRateOperation: OperationDefinition<
isTransferable: (column, newIndexPattern) => {
return hasDateField(newIndexPattern);
},
getErrorMessage: (layer: IndexPatternLayer) => {
return checkForDateHistogram(
getErrorMessage: (layer: IndexPatternLayer, columnId: string) => {
return getErrorsForDateReference(
layer,
columnId,
i18n.translate('xpack.lens.indexPattern.counterRate', {
defaultMessage: 'Counter rate',
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@
import { i18n } from '@kbn/i18n';
import { FormattedIndexPatternColumn, ReferenceBasedIndexPatternColumn } from '../column_types';
import { IndexPatternLayer } from '../../../types';
import { checkForDateHistogram, dateBasedOperationToExpression } from './utils';
import {
checkForDateHistogram,
getErrorsForDateReference,
dateBasedOperationToExpression,
} from './utils';
import { OperationDefinition } from '..';

const ofName = (name?: string) => {
Expand Down Expand Up @@ -81,9 +85,10 @@ export const cumulativeSumOperation: OperationDefinition<
isTransferable: () => {
return true;
},
getErrorMessage: (layer: IndexPatternLayer) => {
return checkForDateHistogram(
getErrorMessage: (layer: IndexPatternLayer, columnId: string) => {
return getErrorsForDateReference(
layer,
columnId,
i18n.translate('xpack.lens.indexPattern.cumulativeSum', {
defaultMessage: 'Cumulative sum',
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { IndexPatternLayer } from '../../../types';
import {
buildLabelFunction,
checkForDateHistogram,
getErrorsForDateReference,
dateBasedOperationToExpression,
hasDateField,
} from './utils';
Expand Down Expand Up @@ -91,9 +92,10 @@ export const derivativeOperation: OperationDefinition<
return hasDateField(newIndexPattern);
},
onOtherColumnChanged: adjustTimeScaleOnOtherColumnChange,
getErrorMessage: (layer: IndexPatternLayer) => {
return checkForDateHistogram(
getErrorMessage: (layer: IndexPatternLayer, columnId: string) => {
return getErrorsForDateReference(
layer,
columnId,
i18n.translate('xpack.lens.indexPattern.derivative', {
defaultMessage: 'Differences',
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { IndexPatternLayer } from '../../../types';
import {
buildLabelFunction,
checkForDateHistogram,
getErrorsForDateReference,
dateBasedOperationToExpression,
hasDateField,
} from './utils';
Expand Down Expand Up @@ -99,9 +100,10 @@ export const movingAverageOperation: OperationDefinition<
return hasDateField(newIndexPattern);
},
onOtherColumnChanged: adjustTimeScaleOnOtherColumnChange,
getErrorMessage: (layer: IndexPatternLayer) => {
return checkForDateHistogram(
getErrorMessage: (layer: IndexPatternLayer, columnId: string) => {
return getErrorsForDateReference(
layer,
columnId,
i18n.translate('xpack.lens.indexPattern.movingAverage', {
defaultMessage: 'Moving average',
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { checkReferences } from './utils';
import { operationDefinitionMap } from '..';
import { createMockedReferenceOperation } from '../../mocks';

// Mock prevents issue with circular loading
jest.mock('..');

describe('utils', () => {
beforeEach(() => {
// @ts-expect-error test-only operation type
operationDefinitionMap.testReference = createMockedReferenceOperation();
});

describe('checkReferences', () => {
it('should show an error if the reference is missing', () => {
expect(
checkReferences(
{
columns: {
ref: {
label: 'Label',
// @ts-expect-error test-only operation type
operationType: 'testReference',
isBucketed: false,
dataType: 'number',
references: ['missing'],
},
},
columnOrder: ['ref'],
indexPatternId: '',
},
'ref'
)
).toEqual(['"Label" is not fully configured']);
});

it('should show an error if the reference is not allowed per the requirements', () => {
expect(
checkReferences(
{
columns: {
ref: {
label: 'Label',
// @ts-expect-error test-only operation type
operationType: 'testReference',
isBucketed: false,
dataType: 'number',
references: ['invalid'],
},
invalid: {
label: 'Date',
operationType: 'date_histogram',
isBucketed: true,
dataType: 'date',
sourceField: 'timestamp',
params: { interval: 'auto' },
},
},
columnOrder: ['invalid', 'ref'],
indexPatternId: '',
},
'ref'
)
).toEqual(['Dimension "Label" is configured incorrectly']);
});
});
});
Loading

0 comments on commit 8820866

Please sign in to comment.