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)

- 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.
  • Loading branch information
walterra authored Jan 24, 2022
1 parent dcae24a commit 1ecb42b
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 @@ -240,7 +240,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 @@ -331,6 +341,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 1ecb42b

Please sign in to comment.