From 99798fdbb44cfbaac87a36e11bada7efb6829356 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Mon, 24 Jan 2022 16:53:09 +0100 Subject: [PATCH] [ML] Transforms: Fix editing retention policy without default value. (#123450) (#123602) - Previously, a bug with how the form state would handle retention policy and populating the field select, a user could be blocked from setting the field or the form state wouldn't pick up the initial setting. - This is now fixed by setting hasNoInitialSelection to reflect the form set. If no retention policy had been set previously, the select dropdown will be empty until the user does a selection (which will then correctly update the to be updated transform config). - Functional tests have been updated to fail reflect the change and fail without the fix. - Fixed a cloning issue: When loading the wizard it didn't pick up the retention policy value provided by the config to be cloned but always fell back to the first available date field. (cherry picked from commit 1ecb42bf010bdd971f92794c3abe2ee47e4319c2) --- .../step_details/step_details_form.tsx | 2 +- .../edit_transform_flyout_form.tsx | 9 ++- .../test/functional/apps/transform/cloning.ts | 28 +++++++++ .../test/functional/apps/transform/editing.ts | 60 +++++++++++++------ .../permissions/full_transform_access.ts | 2 +- .../services/transform/edit_flyout.ts | 24 +++++++- .../functional/services/transform/wizard.ts | 51 ++++++++++++++++ 7 files changed, 154 insertions(+), 22 deletions(-) diff --git a/x-pack/plugins/transform/public/app/sections/create_transform/components/step_details/step_details_form.tsx b/x-pack/plugins/transform/public/app/sections/create_transform/components/step_details/step_details_form.tsx index 6032a53909b9d8..47d450e55a0030 100644 --- a/x-pack/plugins/transform/public/app/sections/create_transform/components/step_details/step_details_form.tsx +++ b/x-pack/plugins/transform/public/app/sections/create_transform/components/step_details/step_details_form.tsx @@ -218,7 +218,7 @@ export const StepDetailsForm: FC = React.memo( defaults.isRetentionPolicyEnabled ); const [retentionPolicyDateField, setRetentionPolicyDateField] = useState( - isRetentionPolicyAvailable ? dateFieldNames[0] : '' + isRetentionPolicyAvailable ? defaults.retentionPolicyDateField : '' ); const [retentionPolicyMaxAge, setRetentionPolicyMaxAge] = useState( defaults.retentionPolicyMaxAge diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/edit_transform_flyout/edit_transform_flyout_form.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/edit_transform_flyout/edit_transform_flyout_form.tsx index 40ccd68724400b..0b8ffba7d26254 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/edit_transform_flyout/edit_transform_flyout_form.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/edit_transform_flyout/edit_transform_flyout_form.tsx @@ -55,7 +55,9 @@ export const EditTransformFlyoutForm: FC = ({ ); const retentionDateFieldOptions = useMemo(() => { - return Array.isArray(dateFieldNames) ? dateFieldNames.map((text: string) => ({ text })) : []; + return Array.isArray(dateFieldNames) + ? dateFieldNames.map((text: string) => ({ text, value: text })) + : []; }, [dateFieldNames]); return ( @@ -181,6 +183,11 @@ export const EditTransformFlyoutForm: FC = ({ onChange={(e) => dispatch({ field: 'retentionPolicyField', value: e.target.value }) } + hasNoInitialSelection={ + !retentionDateFieldOptions + .map((d) => d.text) + .includes(formFields.retentionPolicyField.value) + } /> ) : ( diff --git a/x-pack/test/functional/apps/transform/cloning.ts b/x-pack/test/functional/apps/transform/cloning.ts index 08b61b03d97b18..b499296422fb8b 100644 --- a/x-pack/test/functional/apps/transform/cloning.ts +++ b/x-pack/test/functional/apps/transform/cloning.ts @@ -35,6 +35,7 @@ function getTransformConfig(): TransformPivotConfig { description: 'ecommerce batch transform with avg(products.base_price) grouped by terms(category)', frequency: '3s', + retention_policy: { time: { field: 'order_date', max_age: '1d' } }, settings: { max_page_search_size: 250, }, @@ -72,6 +73,7 @@ function getTransformConfigWithRuntimeMappings(): TransformPivotConfig { }, description: 'ecommerce batch transform grouped by terms(rt_gender_lower)', frequency: '3s', + retention_policy: { time: { field: 'order_date', max_age: '3d' } }, settings: { max_page_search_size: 250, }, @@ -157,6 +159,9 @@ export default function ({ getService }: FtrProviderContext) { `Women's Clothing`, ], }, + retentionPolicySwitchEnabled: true, + retentionPolicyField: 'order_date', + retentionPolicyMaxAge: '1d', }, }, { @@ -186,6 +191,9 @@ export default function ({ getService }: FtrProviderContext) { column: 0, values: [`female`, `male`], }, + retentionPolicySwitchEnabled: true, + retentionPolicyField: 'order_date', + retentionPolicyMaxAge: '3d', }, }, { @@ -210,6 +218,7 @@ export default function ({ getService }: FtrProviderContext) { 'July 12th 2019, 23:45:36', ], }, + retentionPolicySwitchEnabled: false, }, }, ]; @@ -303,6 +312,7 @@ export default function ({ getService }: FtrProviderContext) { testData.expected.transformPreview.values ); + // assert details step form await transform.testExecution.logTestStep('should load the details step'); await transform.wizard.advanceToDetailsStep(); @@ -333,6 +343,24 @@ export default function ({ getService }: FtrProviderContext) { await transform.wizard.assertContinuousModeSwitchExists(); await transform.wizard.assertContinuousModeSwitchCheckState(false); + await transform.testExecution.logTestStep( + 'should display the retention policy settings with pre-filled configuration' + ); + await transform.wizard.assertRetentionPolicySwitchExists(); + await transform.wizard.assertRetentionPolicySwitchCheckState( + testData.expected.retentionPolicySwitchEnabled + ); + if (testData.expected.retentionPolicySwitchEnabled) { + await transform.wizard.assertRetentionPolicyFieldSelectExists(); + await transform.wizard.assertRetentionPolicyFieldSelectValue( + testData.expected.retentionPolicyField + ); + await transform.wizard.assertRetentionPolicyMaxAgeInputExists(); + await transform.wizard.assertRetentionsPolicyMaxAgeValue( + testData.expected.retentionPolicyMaxAge + ); + } + await transform.testExecution.logTestStep( 'should display the advanced settings and show pre-filled configuration' ); diff --git a/x-pack/test/functional/apps/transform/editing.ts b/x-pack/test/functional/apps/transform/editing.ts index 361ad44fd90fe1..154c91ef6c149a 100644 --- a/x-pack/test/functional/apps/transform/editing.ts +++ b/x-pack/test/functional/apps/transform/editing.ts @@ -5,33 +5,17 @@ * 2.0. */ -import { TransformPivotConfig } from '../../../../plugins/transform/common/types/transform'; import { TRANSFORM_STATE } from '../../../../plugins/transform/common/constants'; import { FtrProviderContext } from '../../ftr_provider_context'; -import { getLatestTransformConfig } from './index'; - -function getTransformConfig(): TransformPivotConfig { - const date = Date.now(); - return { - id: `ec_editing_${date}`, - source: { index: ['ft_ecommerce'] }, - pivot: { - group_by: { category: { terms: { field: 'category.keyword' } } }, - aggregations: { 'products.base_price.avg': { avg: { field: 'products.base_price' } } }, - }, - description: - 'ecommerce batch transform with avg(products.base_price) grouped by terms(category)', - dest: { index: `user-ec_2_${date}` }, - }; -} +import { getLatestTransformConfig, getPivotTransformConfig } from './index'; export default function ({ getService }: FtrProviderContext) { const esArchiver = getService('esArchiver'); const transform = getService('transform'); describe('editing', function () { - const transformConfigWithPivot = getTransformConfig(); + const transformConfigWithPivot = getPivotTransformConfig('editing'); const transformConfigWithLatest = getLatestTransformConfig('editing'); before(async () => { @@ -67,8 +51,14 @@ export default function ({ getService }: FtrProviderContext) { transformDescription: 'updated description', transformDocsPerSecond: '1000', transformFrequency: '10m', + transformRetentionPolicyField: 'order_date', + transformRetentionPolicyMaxAge: '1d', expected: { messageText: 'updated transform.', + retentionPolicy: { + field: '', + maxAge: '', + }, row: { status: TRANSFORM_STATE.STOPPED, type: 'pivot', @@ -83,8 +73,14 @@ export default function ({ getService }: FtrProviderContext) { transformDescription: 'updated description', transformDocsPerSecond: '1000', transformFrequency: '10m', + transformRetentionPolicyField: 'order_date', + transformRetentionPolicyMaxAge: '1d', expected: { messageText: 'updated transform.', + retentionPolicy: { + field: '', + maxAge: '', + }, row: { status: TRANSFORM_STATE.STOPPED, type: 'latest', @@ -152,10 +148,38 @@ export default function ({ getService }: FtrProviderContext) { 'Frequency', testData.transformFrequency ); + + await transform.testExecution.logTestStep('should update the transform retention policy'); + await transform.editFlyout.openTransformEditAccordionRetentionPolicySettings(); + + await transform.editFlyout.assertTransformEditFlyoutRetentionPolicyFieldSelectEnabled( + true + ); + await transform.editFlyout.assertTransformEditFlyoutRetentionPolicyFieldSelectValue( + testData.expected.retentionPolicy.field + ); + await transform.editFlyout.setTransformEditFlyoutRetentionPolicyFieldSelectValue( + testData.transformRetentionPolicyField + ); + + await transform.editFlyout.assertTransformEditFlyoutInputEnabled( + 'RetentionPolicyMaxAge', + true + ); + await transform.editFlyout.assertTransformEditFlyoutInputValue( + 'RetentionPolicyMaxAge', + testData.expected.retentionPolicy.maxAge + ); + await transform.editFlyout.setTransformEditFlyoutInputValue( + 'RetentionPolicyMaxAge', + testData.transformRetentionPolicyMaxAge + ); }); it('updates the transform and displays it correctly in the job list', async () => { await transform.testExecution.logTestStep('should update the transform'); + await transform.editFlyout.assertUpdateTransformButtonExists(); + await transform.editFlyout.assertUpdateTransformButtonEnabled(true); await transform.editFlyout.updateTransform(); await transform.testExecution.logTestStep('should display the transforms table'); diff --git a/x-pack/test/functional/apps/transform/permissions/full_transform_access.ts b/x-pack/test/functional/apps/transform/permissions/full_transform_access.ts index 158d8c8f0ac90e..b121953368a980 100644 --- a/x-pack/test/functional/apps/transform/permissions/full_transform_access.ts +++ b/x-pack/test/functional/apps/transform/permissions/full_transform_access.ts @@ -160,7 +160,7 @@ export default function ({ getService }: FtrProviderContext) { 'should have the retention policy inputs enabled' ); await transform.editFlyout.openTransformEditAccordionRetentionPolicySettings(); - await transform.editFlyout.assertTransformEditFlyoutRetentionPolicySelectEnabled(true); + await transform.editFlyout.assertTransformEditFlyoutRetentionPolicyFieldSelectEnabled(true); await transform.editFlyout.assertTransformEditFlyoutInputEnabled( 'RetentionPolicyMaxAge', true diff --git a/x-pack/test/functional/services/transform/edit_flyout.ts b/x-pack/test/functional/services/transform/edit_flyout.ts index cc230e2c38fcab..10e90411b135fb 100644 --- a/x-pack/test/functional/services/transform/edit_flyout.ts +++ b/x-pack/test/functional/services/transform/edit_flyout.ts @@ -37,7 +37,7 @@ export function TransformEditFlyoutProvider({ getService }: FtrProviderContext) ); }, - async assertTransformEditFlyoutRetentionPolicySelectEnabled(expectedValue: boolean) { + async assertTransformEditFlyoutRetentionPolicyFieldSelectEnabled(expectedValue: boolean) { await testSubjects.existOrFail(`transformEditFlyoutRetentionPolicyFieldSelect`, { timeout: 1000, }); @@ -52,6 +52,28 @@ export function TransformEditFlyoutProvider({ getService }: FtrProviderContext) ); }, + async assertTransformEditFlyoutRetentionPolicyFieldSelectValue(expectedValue: string) { + await testSubjects.existOrFail(`transformEditFlyoutRetentionPolicyFieldSelect`, { + timeout: 1000, + }); + const actualValue = await testSubjects.getAttribute( + 'transformEditFlyoutRetentionPolicyFieldSelect', + 'value' + ); + expect(actualValue).to.eql( + expectedValue, + `Retention policy field option value should be '${expectedValue}' (got '${actualValue}')` + ); + }, + + async setTransformEditFlyoutRetentionPolicyFieldSelectValue(fieldOptionValue: string) { + await testSubjects.selectValue( + 'transformEditFlyoutRetentionPolicyFieldSelect', + fieldOptionValue + ); + await this.assertTransformEditFlyoutRetentionPolicyFieldSelectValue(fieldOptionValue); + }, + async assertTransformEditFlyoutInputEnabled(input: string, expectedValue: boolean) { await testSubjects.existOrFail(`transformEditFlyout${input}Input`, { timeout: 1000 }); const isEnabled = await testSubjects.isEnabled(`transformEditFlyout${input}Input`); diff --git a/x-pack/test/functional/services/transform/wizard.ts b/x-pack/test/functional/services/transform/wizard.ts index c91934d7683983..c4791650787c36 100644 --- a/x-pack/test/functional/services/transform/wizard.ts +++ b/x-pack/test/functional/services/transform/wizard.ts @@ -758,6 +758,57 @@ export function TransformWizardProvider({ getService, getPageObjects }: FtrProvi ); }, + async assertRetentionPolicySwitchExists() { + await testSubjects.existOrFail(`transformRetentionPolicySwitch`, { allowHidden: true }); + }, + + async assertRetentionPolicySwitchCheckState(expectedCheckState: boolean) { + const actualCheckState = + (await testSubjects.getAttribute('transformRetentionPolicySwitch', 'aria-checked')) === + 'true'; + expect(actualCheckState).to.eql( + expectedCheckState, + `Retention policy switch check state should be '${expectedCheckState}' (got '${actualCheckState}')` + ); + }, + + async assertRetentionPolicyFieldSelectExists() { + await testSubjects.existOrFail(`transformRetentionPolicyDateFieldSelect`, { + allowHidden: true, + }); + }, + + async assertRetentionPolicyFieldSelectValue(expectedValue: string) { + await testSubjects.existOrFail(`transformRetentionPolicyDateFieldSelect`, { + timeout: 1000, + }); + const actualValue = await testSubjects.getAttribute( + 'transformRetentionPolicyDateFieldSelect', + 'value' + ); + expect(actualValue).to.eql( + expectedValue, + `Retention policy field option value should be '${expectedValue}' (got '${actualValue}')` + ); + }, + + async assertRetentionPolicyMaxAgeInputExists() { + await testSubjects.existOrFail(`transformRetentionPolicyMaxAgeInput`, { + allowHidden: true, + }); + }, + + async assertRetentionsPolicyMaxAgeValue(expectedValue: string) { + const actualValue = await testSubjects.getAttribute( + 'transformRetentionPolicyMaxAgeInput', + 'value' + ); + expect(actualValue).to.eql( + expectedValue, + `Transform retention policy max age input text should be '${expectedValue}' (got '${actualValue}')` + ); + }, + async assertTransformAdvancedSettingsAccordionExists() { await testSubjects.existOrFail('transformWizardAccordionAdvancedSettings'); },