Skip to content

Commit

Permalink
[ML] Transforms: Fix editing retention policy without default value. (#…
Browse files Browse the repository at this point in the history
…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 1ecb42b)
  • Loading branch information
walterra committed Jan 24, 2022
1 parent 9528b85 commit 99798fd
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ export const StepDetailsForm: FC<StepDetailsFormProps> = React.memo(
defaults.isRetentionPolicyEnabled
);
const [retentionPolicyDateField, setRetentionPolicyDateField] = useState(
isRetentionPolicyAvailable ? dateFieldNames[0] : ''
isRetentionPolicyAvailable ? defaults.retentionPolicyDateField : ''
);
const [retentionPolicyMaxAge, setRetentionPolicyMaxAge] = useState(
defaults.retentionPolicyMaxAge
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ export const EditTransformFlyoutForm: FC<EditTransformFlyoutFormProps> = ({
);

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 (
Expand Down Expand Up @@ -181,6 +183,11 @@ export const EditTransformFlyoutForm: FC<EditTransformFlyoutFormProps> = ({
onChange={(e) =>
dispatch({ field: 'retentionPolicyField', value: e.target.value })
}
hasNoInitialSelection={
!retentionDateFieldOptions
.map((d) => d.text)
.includes(formFields.retentionPolicyField.value)
}
/>
</EuiFormRow>
) : (
Expand Down
28 changes: 28 additions & 0 deletions x-pack/test/functional/apps/transform/cloning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -157,6 +159,9 @@ export default function ({ getService }: FtrProviderContext) {
`Women's Clothing`,
],
},
retentionPolicySwitchEnabled: true,
retentionPolicyField: 'order_date',
retentionPolicyMaxAge: '1d',
},
},
{
Expand Down Expand Up @@ -186,6 +191,9 @@ export default function ({ getService }: FtrProviderContext) {
column: 0,
values: [`female`, `male`],
},
retentionPolicySwitchEnabled: true,
retentionPolicyField: 'order_date',
retentionPolicyMaxAge: '3d',
},
},
{
Expand All @@ -210,6 +218,7 @@ export default function ({ getService }: FtrProviderContext) {
'July 12th 2019, 23:45:36',
],
},
retentionPolicySwitchEnabled: false,
},
},
];
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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'
);
Expand Down
60 changes: 42 additions & 18 deletions x-pack/test/functional/apps/transform/editing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand Down Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 23 additions & 1 deletion x-pack/test/functional/services/transform/edit_flyout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function TransformEditFlyoutProvider({ getService }: FtrProviderContext)
);
},

async assertTransformEditFlyoutRetentionPolicySelectEnabled(expectedValue: boolean) {
async assertTransformEditFlyoutRetentionPolicyFieldSelectEnabled(expectedValue: boolean) {
await testSubjects.existOrFail(`transformEditFlyoutRetentionPolicyFieldSelect`, {
timeout: 1000,
});
Expand All @@ -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`);
Expand Down
51 changes: 51 additions & 0 deletions x-pack/test/functional/services/transform/wizard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
},
Expand Down

0 comments on commit 99798fd

Please sign in to comment.