[ML] Transforms: Fix editing retention policy without default value. (#123450) (#125926)

- 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 1ecb42bf01)
2025-04-24 01:38:56 -04:00 · 2022-02-17 14:48:51 +01:00 · 2022-02-17 14:48:51 +01:00 · f41c1b4980
commit f41c1b4980
parent ade71c13ca
7 changed files with 154 additions and 22 deletions
--- 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
@ -226,7 +226,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
--- 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<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 (
@ -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>
            ) : (
--- 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();

@ -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'
          );
--- 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');
--- 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
--- 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`);
--- 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');
    },