Skip to content

Commit

Permalink
[ML] Transforms: Fix retention policy reset (elastic#124698)
Browse files Browse the repository at this point in the history 
The transform edit form was not able to reset the retention policy configuration by emptying the existing form field. This fix adds a switch similar to the creation wizard to allow the user to completely enable/disable the retention policy. The form state management was updated to support passing on `{ retention_policy: null }` to reset the config.
  • Loading branch information
@walterra
walterra authored Feb 7, 2022
1 parent 164eaf2 commit ca77565
Show file tree
Hide file tree
Showing 11 changed files with 365 additions and 184 deletions.
4 changes: 3 additions & 1 deletion x-pack/plugins/transform/common/api_schemas/update_transforms.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ export const postTransformsUpdateRequestSchema = schema.object({
})
),
frequency: schema.maybe(schema.string()),
retention_policy: schema.maybe(retentionPolicySchema),
// maybe: If not set, any existing `retention_policy` config will not be updated.
// nullable: If set to `null`, any existing `retention_policy` will be removed.
retention_policy: schema.maybe(schema.nullable(retentionPolicySchema)),
settings: schema.maybe(settingsSchema),
source: schema.maybe(sourceSchema),
sync: schema.maybe(syncSchema),
Expand Down
4 changes: 2 additions & 2 deletions .../sections/transform_management/components/edit_transform_flyout/edit_transform_flyout.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { useApi } from '../../../../hooks/use_api';
import { EditTransformFlyoutCallout } from './edit_transform_flyout_callout';
import { EditTransformFlyoutForm } from './edit_transform_flyout_form';
import {
applyFormFieldsToTransformConfig,
applyFormStateToTransformConfig,
useEditTransformFlyout,
} from './use_edit_transform_flyout';
import { ManagedTransformsWarningCallout } from '../managed_transforms_callout/managed_transforms_callout';
Expand All @@ -60,7 +60,7 @@ export const EditTransformFlyout: FC<EditTransformFlyoutProps> = ({

async function submitFormHandler() {
setErrorMessage(undefined);
const requestConfig = applyFormFieldsToTransformConfig(config, state.formFields);
const requestConfig = applyFormStateToTransformConfig(config, state);
const transformId = config.id;

const resp = await api.updateTransform(transformId, requestConfig);
Expand Down
193 changes: 106 additions & 87 deletions ...ions/transform_management/components/edit_transform_flyout/edit_transform_flyout_form.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,15 @@

import React, { FC, useEffect, useMemo, useState } from 'react';

import { EuiComboBox, EuiForm, EuiAccordion, EuiSpacer, EuiSelect, EuiFormRow } from '@elastic/eui';
import {
EuiAccordion,
EuiComboBox,
EuiForm,
EuiFormRow,
EuiSelect,
EuiSpacer,
EuiSwitch,
} from '@elastic/eui';

import { i18n } from '@kbn/i18n';

Expand All @@ -28,10 +36,12 @@ export const EditTransformFlyoutForm: FC<EditTransformFlyoutFormProps> = ({
editTransformFlyout: [state, dispatch],
indexPatternId,
}) => {
const formFields = state.formFields;
const { formFields, formSections } = state;
const [dateFieldNames, setDateFieldNames] = useState<string[]>([]);
const [ingestPipelineNames, setIngestPipelineNames] = useState<string[]>([]);

const isRetentionPolicyAvailable = dateFieldNames.length > 0;

const appDeps = useAppDependencies();
const indexPatternsClient = appDeps.data.indexPatterns;
const api = useApi();
Expand Down Expand Up @@ -119,6 +129,100 @@ export const EditTransformFlyoutForm: FC<EditTransformFlyoutFormProps> = ({

<EuiSpacer size="l" />

<EuiSwitch
name="transformEditRetentionPolicySwitch"
label={i18n.translate(
'xpack.transform.transformList.editFlyoutFormRetentionPolicySwitchLabel',
{
defaultMessage: 'Retention policy',
}
)}
checked={formSections.retentionPolicy.enabled}
onChange={(e) =>
dispatch({
section: 'retentionPolicy',
enabled: e.target.checked,
})
}
disabled={!isRetentionPolicyAvailable}
data-test-subj="transformEditRetentionPolicySwitch"
/>
{formSections.retentionPolicy.enabled && (
<div data-test-subj="transformEditRetentionPolicyContent">
<EuiSpacer size="m" />
{
// If data view or date fields info not available
// gracefully defaults to text input
indexPatternId ? (
<EuiFormRow
label={i18n.translate(
'xpack.transform.transformList.editFlyoutFormRetentionPolicyFieldLabel',
{
defaultMessage: 'Field',
}
)}
isInvalid={formFields.retentionPolicyField.errorMessages.length > 0}
error={formFields.retentionPolicyField.errorMessages}
helpText={i18n.translate(
'xpack.transform.transformList.editFlyoutFormRetentionPolicyDateFieldHelpText',
{
defaultMessage:
'Select the date field that can be used to identify out of date documents in the destination index.',
}
)}
>
<EuiSelect
aria-label={i18n.translate(
'xpack.transform.transformList.editFlyoutFormRetentionPolicyFieldSelectAriaLabel',
{
defaultMessage: 'Date field to set retention policy',
}
)}
data-test-subj="transformEditFlyoutRetentionPolicyFieldSelect"
options={retentionDateFieldOptions}
value={formFields.retentionPolicyField.value}
onChange={(e) =>
dispatch({ field: 'retentionPolicyField', value: e.target.value })
}
hasNoInitialSelection={
!retentionDateFieldOptions
.map((d) => d.text)
.includes(formFields.retentionPolicyField.value)
}
/>
</EuiFormRow>
) : (
<EditTransformFlyoutFormTextInput
dataTestSubj="transformEditFlyoutRetentionPolicyFieldInput"
errorMessages={formFields.retentionPolicyField.errorMessages}
label={i18n.translate(
'xpack.transform.transformList.editFlyoutFormRetentionPolicyFieldLabel',
{
defaultMessage: 'Field',
}
)}
onChange={(value) => dispatch({ field: 'retentionPolicyField', value })}
value={formFields.retentionPolicyField.value}
/>
)
}
<EditTransformFlyoutFormTextInput
dataTestSubj="transformEditFlyoutRetentionPolicyMaxAgeInput"
errorMessages={formFields.retentionPolicyMaxAge.errorMessages}
label={i18n.translate(
'xpack.transform.transformList.editFlyoutFormRetentionMaxAgeFieldLabel',
{
defaultMessage: 'Max age',
}
)}
onChange={(value) => dispatch({ field: 'retentionPolicyMaxAge', value })}
value={formFields.retentionPolicyMaxAge.value}
/>
</div>
)}

<EuiSpacer size="l" />

<EuiAccordion
data-test-subj="transformEditAccordionDestination"
id="transformEditAccordionDestination"
Expand Down Expand Up @@ -204,91 +308,6 @@ export const EditTransformFlyoutForm: FC<EditTransformFlyoutFormProps> = ({

<EuiSpacer size="l" />

<EuiAccordion
data-test-subj="transformEditAccordionRetentionPolicy"
id="transformEditAccordionRetentionPolicy"
buttonContent={i18n.translate(
'xpack.transform.transformList.editFlyoutFormRetentionPolicyButtonContent',
{
defaultMessage: 'Retention policy',
}
)}
paddingSize="s"
>
<div data-test-subj="transformEditAccordionRetentionPolicyContent">
{
// If data view or date fields info not available
// gracefully defaults to text input
indexPatternId ? (
<EuiFormRow
label={i18n.translate(
'xpack.transform.transformList.editFlyoutFormRetentionPolicyFieldLabel',
{
defaultMessage: 'Field',
}
)}
isInvalid={formFields.retentionPolicyField.errorMessages.length > 0}
error={formFields.retentionPolicyField.errorMessages}
helpText={i18n.translate(
'xpack.transform.transformList.editFlyoutFormRetentionPolicyDateFieldHelpText',
{
defaultMessage:
'Select the date field that can be used to identify out of date documents in the destination index.',
}
)}
>
<EuiSelect
aria-label={i18n.translate(
'xpack.transform.transformList.editFlyoutFormRetentionPolicyFieldSelectAriaLabel',
{
defaultMessage: 'Date field to set retention policy',
}
)}
data-test-subj="transformEditFlyoutRetentionPolicyFieldSelect"
options={retentionDateFieldOptions}
value={formFields.retentionPolicyField.value}
onChange={(e) =>
dispatch({ field: 'retentionPolicyField', value: e.target.value })
}
hasNoInitialSelection={
!retentionDateFieldOptions
.map((d) => d.text)
.includes(formFields.retentionPolicyField.value)
}
/>
</EuiFormRow>
) : (
<EditTransformFlyoutFormTextInput
dataTestSubj="transformEditFlyoutRetentionPolicyFieldInput"
errorMessages={formFields.retentionPolicyField.errorMessages}
label={i18n.translate(
'xpack.transform.transformList.editFlyoutFormRetentionPolicyFieldLabel',
{
defaultMessage: 'Field',
}
)}
onChange={(value) => dispatch({ field: 'retentionPolicyField', value })}
value={formFields.retentionPolicyField.value}
/>
)
}
<EditTransformFlyoutFormTextInput
dataTestSubj="transformEditFlyoutRetentionPolicyMaxAgeInput"
errorMessages={formFields.retentionPolicyMaxAge.errorMessages}
label={i18n.translate(
'xpack.transform.transformList.editFlyoutFormRetentionMaxAgeFieldLabel',
{
defaultMessage: 'Max age',
}
)}
onChange={(value) => dispatch({ field: 'retentionPolicyMaxAge', value })}
value={formFields.retentionPolicyMaxAge.value}
/>
</div>
</EuiAccordion>

<EuiSpacer size="l" />

<EuiAccordion
data-test-subj="transformEditAccordionAdvancedSettings"
id="transformEditAccordionAdvancedSettings"
Expand Down
54 changes: 28 additions & 26 deletions ...s/transform_management/components/edit_transform_flyout/use_edit_transform_flyout.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { TransformPivotConfig } from '../../../../../../common/types/transform';

import {
applyFormFieldsToTransformConfig,
applyFormStateToTransformConfig,
formReducerFactory,
frequencyValidator,
getDefaultState,
Expand Down Expand Up @@ -47,16 +47,13 @@ const getTransformConfigMock = (): TransformPivotConfig => ({
description: 'the-description',
});

describe('Transform: applyFormFieldsToTransformConfig()', () => {
describe('Transform: applyFormStateToTransformConfig()', () => {
it('should exclude unchanged form fields', () => {
const transformConfigMock = getTransformConfigMock();

const formState = getDefaultState(transformConfigMock);

const updateConfig = applyFormFieldsToTransformConfig(
transformConfigMock,
formState.formFields
);
const updateConfig = applyFormStateToTransformConfig(transformConfigMock, formState);

// This case will return an empty object. In the actual UI, this case should not happen
// because the Update-Button will be disabled when no form field was changed.
Expand Down Expand Up @@ -84,10 +81,7 @@ describe('Transform: applyFormFieldsToTransformConfig()', () => {
},
});

const updateConfig = applyFormFieldsToTransformConfig(
transformConfigMock,
formState.formFields
);
const updateConfig = applyFormStateToTransformConfig(transformConfigMock, formState);

expect(Object.keys(updateConfig)).toHaveLength(4);
expect(updateConfig.description).toBe('the-new-description');
Expand All @@ -108,10 +102,7 @@ describe('Transform: applyFormFieldsToTransformConfig()', () => {
},
});

const updateConfig = applyFormFieldsToTransformConfig(
transformConfigMock,
formState.formFields
);
const updateConfig = applyFormStateToTransformConfig(transformConfigMock, formState);

expect(Object.keys(updateConfig)).toHaveLength(2);
expect(updateConfig.description).toBe('the-updated-description');
Expand All @@ -132,10 +123,7 @@ describe('Transform: applyFormFieldsToTransformConfig()', () => {
},
});

const updateConfig = applyFormFieldsToTransformConfig(
transformConfigMock,
formState.formFields
);
const updateConfig = applyFormStateToTransformConfig(transformConfigMock, formState);
expect(Object.keys(updateConfig)).toHaveLength(1);
// It should include the dependent unchanged destination index
expect(updateConfig.dest?.index).toBe(transformConfigMock.dest.index);
Expand All @@ -159,10 +147,7 @@ describe('Transform: applyFormFieldsToTransformConfig()', () => {
},
});

const updateConfig = applyFormFieldsToTransformConfig(
transformConfigMock,
formState.formFields
);
const updateConfig = applyFormStateToTransformConfig(transformConfigMock, formState);
expect(Object.keys(updateConfig)).toHaveLength(1);
// It should include the dependent unchanged destination index
expect(updateConfig.dest?.index).toBe(transformConfigMock.dest.index);
Expand All @@ -177,15 +162,32 @@ describe('Transform: applyFormFieldsToTransformConfig()', () => {
description: 'the-updated-description',
});

const updateConfig = applyFormFieldsToTransformConfig(
transformConfigMock,
formState.formFields
);
const updateConfig = applyFormStateToTransformConfig(transformConfigMock, formState);
expect(Object.keys(updateConfig)).toHaveLength(1);
// It should exclude the dependent unchanged destination section
expect(typeof updateConfig.dest).toBe('undefined');
expect(updateConfig.description).toBe('the-updated-description');
});

it('should return the config to reset retention policy', () => {
const transformConfigMock = getTransformConfigMock();

const formState = getDefaultState({
...transformConfigMock,
retention_policy: {
time: { field: 'the-time-field', max_age: '1d' },
},
});

formState.formSections.retentionPolicy.enabled = false;

const updateConfig = applyFormStateToTransformConfig(transformConfigMock, formState);

expect(Object.keys(updateConfig)).toHaveLength(1);
// It should exclude the dependent unchanged destination section
expect(typeof updateConfig.dest).toBe('undefined');
expect(updateConfig.retention_policy).toBe(null);
});
});

describe('Transform: formReducerFactory()', () => {
Expand Down
Loading

0 comments on commit ca77565

Please sign in to comment.