[ML] Transform: Fix unsupported boolean filter when cloning (#137773)

* [ML] Turn filter boolean into json editor instead

* [ML] Reformat bracelet condition, add isJSONValid check

* [ML] Add functional tests

* [ML] Add functional tests

* Change to useMemo

* [ML] add json is invalid message

* [ML] Refactor into nested agg

* [ML] Remove unused code, change to null, add validator function

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Quynh Nguyen 2022-08-10 09:36:19 -05:00 committed by GitHub
parent 3ef3614197
commit 664ddc2a95
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 284 additions and 41 deletions

View file

@ -7,6 +7,7 @@
import {
continuousModeDelayValidator,
jsonStringValidator,
parseDuration,
retentionPolicyMaxAgeValidator,
transformFrequencyValidator,
@ -137,3 +138,28 @@ describe('transformFrequencyValidator', () => {
expect(transformFrequencyValidator('2h')).toBe(false);
});
});
describe('jsonStringValidator', () => {
it('should return false for non-string input', () => {
expect(jsonStringValidator(false)).toBe(false);
expect(jsonStringValidator(undefined)).toBe(false);
expect(jsonStringValidator(null)).toBe(false);
expect(jsonStringValidator(0)).toBe(false);
expect(jsonStringValidator({})).toBe(false);
});
it('should return whether string is parsable as valid json', () => {
expect(
jsonStringValidator(`{
"must": [],
"must_not": [],
"should": []
}`)
).toBe(true);
expect(
jsonStringValidator(`{
"must":,
}`)
).toBe(false);
});
});

View file

@ -127,3 +127,20 @@ export const transformFrequencyValidator = (value: string): boolean => {
export function transformSettingsMaxPageSearchSizeValidator(value: number): boolean {
return value >= 10 && value <= 10000;
}
/**
* Validates whether string input can be parsed as a valid JSON
* @param value User input value.
*/
export function jsonStringValidator(value: unknown): boolean {
if (typeof value !== 'string') return false;
try {
return !!JSON.parse(value);
} catch (e) {
// eslint-disable-next-line no-console
console.error(`JSON is invalid.\n${e}`);
return false;
}
return true;
}

View file

@ -26,7 +26,7 @@ exports[`Transform: <AggLabelForm /> Date histogram aggregation 1`] = `
button={
<EuiButtonIcon
aria-label="Edit aggregation"
data-test-subj="transformAggregationEntryEditButton"
data-test-subj="transformAggregationEntryEditButton_the-group-by-agg-name"
iconType="pencil"
onClick={[Function]}
size="s"

View file

@ -86,7 +86,7 @@ export const AggLabelForm: React.FC<Props> = ({
size="s"
iconType="pencil"
onClick={() => setPopoverVisibility(!isPopoverVisible)}
data-test-subj="transformAggregationEntryEditButton"
data-test-subj={`transformAggregationEntryEditButton_${item.aggName}`}
/>
}
isOpen={isPopoverVisible}

View file

@ -6,13 +6,15 @@
*/
import React from 'react';
import { EuiSpacer } from '@elastic/eui';
import { EuiCallOut, EuiSpacer } from '@elastic/eui';
import { CodeEditor } from '@kbn/kibana-react-plugin/public';
import { FormattedMessage } from '@kbn/i18n-react';
import { FilterAggConfigEditor } from '../types';
export const FilterEditorForm: FilterAggConfigEditor['aggTypeConfig']['FilterAggFormComponent'] = ({
config,
onChange,
isValid,
}) => {
return (
<>
@ -36,6 +38,17 @@ export const FilterEditorForm: FilterAggConfigEditor['aggTypeConfig']['FilterAgg
}}
value={config || ''}
/>
{isValid === false ? (
<>
<EuiSpacer size="m" />
<EuiCallOut color="danger" iconType="alert" size="s">
<FormattedMessage
id="xpack.transform.agg.filterEditorForm.jsonInvalidErrorMessage"
defaultMessage="JSON is invalid."
/>
</EuiCallOut>
</>
) : null}
</>
);
};

View file

@ -25,7 +25,7 @@ export function getSupportedFilterAggs(
fieldName: string,
dataView: DataView,
runtimeMappings?: RuntimeMappings
): FilterAggType[] {
): FilterAggType[] | undefined {
const dataViewField = dataView.fields.getByName(fieldName);
if (dataViewField !== undefined) {
@ -39,7 +39,10 @@ export function getSupportedFilterAggs(
];
}
throw new Error(`The field ${fieldName} does not exist in the index or runtime fields`);
// Some aggs like filter boolean might have fields that don't exist
// but we still support it as JSON
// eslint-disable-next-line no-console
console.error(`The field ${fieldName} does not exist in the index or runtime fields`);
}
/**
@ -67,45 +70,52 @@ export const FilterAggForm: PivotAggsConfigFilter['AggFormComponent'] = ({
const filterAggTypeConfig = aggConfig?.aggTypeConfig;
const filterAgg = aggConfig?.filterAgg ?? '';
const isValid = filterAggTypeConfig?.isValid ? filterAggTypeConfig?.isValid() : undefined;
return (
<>
<EuiFormRow
label={
<>
<FormattedMessage
id="xpack.transform.agg.popoverForm.filerAggLabel"
defaultMessage="Filter query"
/>
<EuiToolTip
content={
<FormattedMessage
id="xpack.transform.agg.popoverForm.filerQueryAdvancedSuggestionTooltip"
defaultMessage="To add other filter query aggregations, edit the JSON config."
{filterAggsOptions !== undefined ? (
<EuiFormRow
label={
<>
<FormattedMessage
id="xpack.transform.agg.popoverForm.filerAggLabel"
defaultMessage="Filter query"
/>
<EuiToolTip
content={
<FormattedMessage
id="xpack.transform.agg.popoverForm.filerQueryAdvancedSuggestionTooltip"
defaultMessage="To add other filter query aggregations, edit the JSON config."
/>
}
>
<EuiIcon
size="s"
color="subdued"
type="questionInCircle"
className="eui-alignTop"
/>
}
>
<EuiIcon size="s" color="subdued" type="questionInCircle" className="eui-alignTop" />
</EuiToolTip>
</>
}
>
<EuiSelect
options={[{ text: '', value: '' }].concat(
filterAggsOptions.map((v) => ({ text: v, value: v }))
)}
value={filterAgg}
onChange={(e) => {
// have to reset aggTypeConfig of filterAgg change
const filterAggUpdate = e.target.value as FilterAggType;
onChange({
filterAgg: filterAggUpdate,
aggTypeConfig: getFilterAggTypeConfig(filterAggUpdate),
});
}}
data-test-subj="transformFilterAggTypeSelector"
/>
</EuiFormRow>
</EuiToolTip>
</>
}
>
<EuiSelect
options={[{ text: '', value: '' }].concat(
filterAggsOptions.map((v) => ({ text: v, value: v }))
)}
value={filterAgg}
onChange={(e) => {
// have to reset aggTypeConfig of filterAgg change
const filterAggUpdate = e.target.value as FilterAggType;
onChange({
filterAgg: filterAggUpdate,
aggTypeConfig: getFilterAggTypeConfig(filterAggUpdate),
});
}}
data-test-subj="transformFilterAggTypeSelector"
/>
</EuiFormRow>
) : null}
{filterAgg !== '' && filterAggTypeConfig?.FilterAggFormComponent && (
<filterAggTypeConfig.FilterAggFormComponent
config={filterAggTypeConfig?.filterAggConfig}
@ -119,6 +129,7 @@ export const FilterAggForm: PivotAggsConfigFilter['AggFormComponent'] = ({
});
}}
selectedField={selectedField}
isValid={isValid}
/>
)}
</>

View file

@ -5,6 +5,7 @@
* 2.0.
*/
import { jsonStringValidator } from '../../../../../../common/validators';
import {
isPivotAggsConfigWithUiSupport,
PivotAggsConfigBase,
@ -188,6 +189,9 @@ export function getFilterAggTypeConfig(
null,
2
),
isValid() {
return jsonStringValidator(this.filterAggConfig);
},
getEsAggConfig(fieldName) {
return JSON.parse(this.filterAggConfig!);
},
@ -199,6 +203,9 @@ export function getFilterAggTypeConfig(
getEsAggConfig() {
return this.filterAggConfig !== undefined ? JSON.parse(this.filterAggConfig!) : {};
},
isValid() {
return jsonStringValidator(this.filterAggConfig);
},
};
}
}

View file

@ -18,6 +18,8 @@ type FilterAggForm<T> = FC<{
onChange: (arg: Partial<{ config: Partial<T> }>) => void;
/** Selected field for the aggregation */
selectedField?: string;
/** Whether the configuration is valid */
isValid?: boolean;
}>;
interface FilterAggTypeConfig<U, R> {

View file

@ -88,6 +88,86 @@ function getTransformConfigWithRuntimeMappings(): TransformPivotConfig {
};
}
function getTransformConfigWithBoolFilterAgg(): TransformPivotConfig {
const date = Date.now();
return {
id: `ec_cloning_filter_agg_${date}`,
source: {
index: ['ft_ecommerce'],
},
// @ts-ignore Boolean filter doesn't have to have field
pivot: {
group_by: {
category: {
terms: {
field: 'category.keyword',
},
},
},
aggregations: {
'products.base_price.avg': {
avg: {
field: 'products.base_price',
},
},
Saturday: {
filter: {
term: {
day_of_week: 'Saturday',
},
},
aggs: {
'saturday.products.base_price.max': {
max: {
field: 'products.base_price',
},
},
},
},
FEMALE: {
filter: {
bool: {
must: [],
must_not: [],
should: [],
},
},
aggs: {
'female.products.base_price.sum': {
sum: {
field: 'products.base_price',
},
},
},
},
user_exists: {
filter: {
exists: {
field: 'user',
},
},
aggs: {
'user_exists.order_date.min': {
min: {
field: 'order_date',
},
},
},
},
},
},
description: 'ecommerce batch transform with filter aggregations',
frequency: '3s',
retention_policy: { time: { field: 'order_date', max_age: '3d' } },
settings: {
max_page_search_size: 250,
num_failure_retries: 5,
},
dest: { index: `user-ec_2_${date}` },
};
}
export default function ({ getService }: FtrProviderContext) {
const esArchiver = getService('esArchiver');
const transform = getService('transform');
@ -95,6 +175,8 @@ export default function ({ getService }: FtrProviderContext) {
describe('cloning', function () {
const transformConfigWithPivot = getTransformConfig();
const transformConfigWithRuntimeMapping = getTransformConfigWithRuntimeMappings();
const transformConfigWithBoolFilterAgg = getTransformConfigWithBoolFilterAgg();
const transformConfigWithLatest = getLatestTransformConfig('cloning');
before(async () => {
@ -108,10 +190,15 @@ export default function ({ getService }: FtrProviderContext) {
transformConfigWithRuntimeMapping.id,
transformConfigWithRuntimeMapping
);
await transform.api.createAndRunTransform(
transformConfigWithBoolFilterAgg.id,
transformConfigWithBoolFilterAgg
);
await transform.api.createAndRunTransform(
transformConfigWithLatest.id,
transformConfigWithLatest
);
await transform.testResources.setKibanaTimeZoneToUTC();
await transform.securityUI.loginAsTransformPowerUser();
@ -208,6 +295,59 @@ export default function ({ getService }: FtrProviderContext) {
),
},
},
{
type: 'pivot' as const,
suiteTitle: 'clone transform with filter agg',
originalConfig: transformConfigWithBoolFilterAgg,
transformId: `clone_${transformConfigWithBoolFilterAgg.id}`,
transformDescription: `a cloned transform with filter agg`,
get destinationIndex(): string {
return `user-${this.transformId}`;
},
expected: {
runtimeMappingsEditorValueArr: [''],
aggs: {
index: 0,
label: 'products.base_price.avg',
},
editableAggregations: [
'products.base_price.avg',
// term filter
'Saturday',
'saturday.products.base_price.max',
// boolean filter
'FEMALE',
'female.products.base_price.sum',
// exist filter
'user_exists',
'user_exists.order_date.min',
],
indexPreview: {
columns: 10,
rows: 5,
},
groupBy: {
index: 0,
label: 'category',
},
transformPreview: {
column: 0,
values: [
`Men's Accessories`,
`Men's Clothing`,
`Men's Shoes`,
`Women's Accessories`,
`Women's Clothing`,
],
},
retentionPolicySwitchEnabled: true,
retentionPolicyField: 'order_date',
retentionPolicyMaxAge: '3d',
numFailureRetries: getNumFailureRetriesStr(
transformConfigWithBoolFilterAgg.settings?.num_failure_retries
),
},
},
{
type: 'latest' as const,
suiteTitle: 'clone transform with latest function',
@ -308,6 +448,14 @@ export default function ({ getService }: FtrProviderContext) {
testData.expected.aggs.index,
testData.expected.aggs.label
);
if (
Array.isArray(testData.expected.editableAggregations) &&
testData.expected.editableAggregations?.length > 0
) {
for (const aggName of testData.expected.editableAggregations) {
await transform.wizard.assertAggregationEntryEditPopoverValid(aggName);
}
}
} else if (isLatestTransform(testData.originalConfig)) {
await transform.testExecution.logTestStep('should show pre-filler unique keys');
await transform.wizard.assertUniqueKeysInputValue(

View file

@ -19,6 +19,7 @@ export type HistogramCharts = Array<{
export function TransformWizardProvider({ getService, getPageObjects }: FtrProviderContext) {
const aceEditor = getService('aceEditor');
const browser = getService('browser');
const canvasElement = getService('canvasElement');
const log = getService('log');
const testSubjects = getService('testSubjects');
@ -1049,5 +1050,23 @@ export function TransformWizardProvider({ getService, getPageObjects }: FtrProvi
await this.assertProgressbarExists();
});
},
async assertAggregationEntryEditPopoverValid(aggName: string) {
await retry.tryForTime(5000, async () => {
await testSubjects.click(`transformAggregationEntryEditButton_${aggName}`);
await testSubjects.existOrFail(`transformAggPopoverForm_${aggName}`);
const isApplyAggChangeEnabled = await testSubjects.isEnabled(
`~transformAggPopoverForm_${aggName} > ~transformApplyAggChanges`
);
expect(isApplyAggChangeEnabled).to.eql(
true,
'Expected Transform aggregation entry `Apply` to be enabled'
);
// escape popover
await browser.pressKeys(browser.keys.ESCAPE);
});
},
};
}