[9.0] [EDR Workflows] Fix event filters cannot be saved bug (#213805) (#213994)

# Backport This will backport the following commits from `main` to `9.0`: - [[EDR Workflows] Fix event filters cannot be saved bug (#213805)](https://github.com/elastic/kibana/pull/213805)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Gergő Ábrahám <gergo.abraham@elastic.co>
2025-04-23 09:19:04 -04:00 · 2025-03-12 07:58:03 +11:00 · 2025-03-12 07:58:03 +11:00 · b5f13960d0
commit b5f13960d0
parent 09fba2d479
3 changed files with 218 additions and 55 deletions
--- a/x-pack/solutions/security/plugins/security_solution/public/management/pages/event_filters/view/components/event_filters_flyout.test.tsx
+++ b/x-pack/solutions/security/plugins/security_solution/public/management/pages/event_filters/view/components/event_filters_flyout.test.tsx
@ -14,6 +14,8 @@ import userEvent, { type UserEvent } from '@testing-library/user-event';
 import type { AppContextTestRender } from '../../../../../common/mock/endpoint';
 import { createAppRootMockRenderer } from '../../../../../common/mock/endpoint';

+import { stubIndexPattern } from '@kbn/data-plugin/common/stubs';
+import { useFetchIndex } from '../../../../../common/containers/source';
 import { getInitialExceptionFromEvent } from '../utils';
 import { useCreateArtifact } from '../../../../hooks/artifacts/use_create_artifact';
 import { useGetEndpointSpecificPolicies } from '../../../../services/policies/hooks';
@ -26,6 +28,7 @@ import type { ExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-t

 // mocked modules
 jest.mock('../../../../../common/lib/kibana');
+jest.mock('../../../../../common/containers/source');
 jest.mock('../../../../services/policies/hooks');
 jest.mock('../../../../services/policies/policies');
 jest.mock('../../../../hooks/artifacts/use_create_artifact');
@ -194,6 +197,11 @@ describe('Event filter flyout', () => {
      return { isLoading: false, isRefetching: false };
    });

+    (useFetchIndex as jest.Mock).mockImplementation(() => [
+      false,
+      { indexPatterns: stubIndexPattern },
+    ]);
+
    render = (props) => {
      renderResult = mockedContext.render(
        <EventFiltersFlyout {...props} onCancel={onCancelMock} />
@ -281,17 +289,29 @@ describe('Event filter flyout', () => {
      ],
      name: 'some name',
    };
+
    beforeEach(() => {
      const exception = exceptionsGenerator.generateEventFilterForCreate(exceptionOptions);
      (getInitialExceptionFromEvent as jest.Mock).mockImplementation(() => {
        return exception;
      });
    });
-    it('should change to "add event filter" button enabled', () => {
+
+    it('should display "Add event filter"/"Save" button disabled when form hasn\'t changed', () => {
      render();
+      const confirmButton = renderResult.getByTestId('add-exception-confirm-button');
+      expect(confirmButton.hasAttribute('disabled')).toBeTruthy();
+    });
+
+    it('should enable "Add event filter"/"Save" button when user enters/modifies filter name', async () => {
+      render();
+
+      await user.type(renderResult.getByTestId('eventFilters-form-name-input'), 'a');
+
      const confirmButton = renderResult.getByTestId('add-exception-confirm-button');
      expect(confirmButton.hasAttribute('disabled')).toBeFalsy();
    });
+
    it('should prevent close when submitting data', async () => {
      (useCreateArtifact as jest.Mock).mockImplementation(() => {
        return { isLoading: true, mutateAsync: jest.fn() };
--- a/x-pack/solutions/security/plugins/security_solution/public/management/pages/event_filters/view/components/form.test.tsx
+++ b/x-pack/solutions/security/plugins/security_solution/public/management/pages/event_filters/view/components/form.test.tsx
@ -4,7 +4,7 @@
 * 2.0; you may not use this file except in compliance with the Elastic License
 * 2.0.
 */
-import React from 'react';
+import React, { useCallback, useState } from 'react';
 import { act, cleanup, fireEvent } from '@testing-library/react';
 import { stubIndexPattern } from '@kbn/data-plugin/common/stubs';
 import { useFetchIndex } from '../../../../../common/containers/source';
@ -48,6 +48,39 @@ jest.mock('../../../../../common/hooks/use_license', () => {
  };
 });

+/** When some props and states change, `EventFilterForm` will recreate its internal `processChanged` function,
+ * and therefore will call it from a `useEffect` hook.
+ *
+ * Amongst the aforementioned props is the item itself, which comes from the outside - the test environment.
+ * Amongst the aforementioned states is the `hasFormChanged` state, which will change from `false` to `true`
+ * on the first user action.
+ *
+ * In the browser, the `item` prop is a state, a state from the parent component, therefore both
+ * `item` and `hasFormChange` will change at the same re-render, which ensures that the `useEffect` will
+ * call `processChanged` with consistent data.
+ *
+ * In the test environment, however, we need a trick to make sure that the data is updated in the same re-render
+ * both outside and inside the component, in order to not receive additional calls from `processChanged` with outdated
+ * data.
+ *
+ * This `TestComponentWrapper` component is meant to provide this kind of syncronisation, by turning the `item` prop
+ * into a state.
+ *
+ */
+const TestComponentWrapper: typeof EventFiltersForm = (formProps: ArtifactFormComponentProps) => {
+  const [item, setItem] = useState(formProps.item);
+
+  const handleOnChange: ArtifactFormComponentProps['onChange'] = useCallback(
+    (formStatus) => {
+      setItem(formStatus.item);
+      formProps.onChange(formStatus);
+    },
+    [formProps]
+  );
+
+  return <EventFiltersForm {...formProps} item={item} onChange={handleOnChange} />;
+};
+
 describe('Event filter form', () => {
  const formPrefix = 'eventFilters-form';
  const generator = new EndpointDocGenerator('effected-policy-select');
@ -56,8 +89,9 @@ describe('Event filter form', () => {
  let mockedContext: AppContextTestRender;
  let renderResult: ReturnType<AppContextTestRender['render']>;
  let latestUpdatedItem: ArtifactFormComponentProps['item'];
+  let isLatestUpdatedItemValid: boolean;

-  const getUI = () => <EventFiltersForm {...formProps} />;
+  const getUI = () => <TestComponentWrapper {...formProps} />;
  const render = () => {
    return (renderResult = mockedContext.render(getUI()));
  };
@ -128,6 +162,14 @@ describe('Event filter form', () => {
    return policies;
  }

+  const setValidItemForEditing = () => {
+    formProps.mode = 'edit';
+    formProps.item.name = 'item name';
+    formProps.item.entries = [
+      { field: 'test.field', operator: 'included', type: 'match', value: 'test value' },
+    ];
+  };
+
  beforeEach(async () => {
    (useCurrentUser as jest.Mock).mockReturnValue({ username: 'test-username' });
    (useKibana as jest.Mock).mockReturnValue({
@ -156,6 +198,7 @@ describe('Event filter form', () => {
      policiesIsLoading: false,
      onChange: jest.fn((updates) => {
        latestUpdatedItem = updates.item;
+        isLatestUpdatedItemValid = updates.isValid;
      }),
      policies: [],
    };
@ -169,7 +212,9 @@ describe('Event filter form', () => {
    it('should render correctly without data', () => {
      formProps.policies = createPolicies();
      formProps.policiesIsLoading = true;
-      formProps.item.tags = [formProps.policies.map((p) => `policy:${p.id}`)[0]];
+      formProps.item.tags = [
+        formProps.policies.map((p) => `${BY_POLICY_ARTIFACT_TAG_PREFIX}${p.id}`)[0],
+      ];
      formProps.item.entries = [];
      render();
      expect(renderResult.getByTestId('loading-spinner')).not.toBeNull();
@ -202,14 +247,7 @@ describe('Event filter form', () => {
      render();
      const nameInput = renderResult.getByTestId(`${formPrefix}-name-input`);

-      act(() => {
-        fireEvent.change(nameInput, {
-          target: {
-            value: 'Exception name',
-          },
-        });
-        fireEvent.blur(nameInput);
-      });
+      await userEvent.type(nameInput, 'Exception name');
      rerenderWithLatestProps();

      expect(formProps.item?.name).toBe('Exception name');
@ -220,14 +258,8 @@ describe('Event filter form', () => {
      render();
      const nameInput = renderResult.getByTestId(`${formPrefix}-name-input`);

-      act(() => {
-        fireEvent.change(nameInput, {
-          target: {
-            value: '   ',
-          },
-        });
-        fireEvent.blur(nameInput);
-      });
+      await userEvent.type(nameInput, '   ');
+      fireEvent.blur(nameInput);
      rerenderWithLatestProps();

      expect(formProps.item.name).toBe('');
@ -236,16 +268,9 @@ describe('Event filter form', () => {

    it('should change description', async () => {
      render();
-      const nameInput = renderResult.getByTestId(`${formPrefix}-description-input`);
+      const descriptionInput = renderResult.getByTestId(`${formPrefix}-description-input`);

-      act(() => {
-        fireEvent.change(nameInput, {
-          target: {
-            value: 'Exception description',
-          },
-        });
-        fireEvent.blur(nameInput);
-      });
+      await userEvent.type(descriptionInput, 'Exception description');
      rerenderWithLatestProps();

      expect(formProps.item.description).toBe('Exception description');
@ -255,18 +280,52 @@ describe('Event filter form', () => {
      render();
      const commentInput = renderResult.getByLabelText('Comment Input');

-      act(() => {
-        fireEvent.change(commentInput, {
-          target: {
-            value: 'Exception comment',
-          },
-        });
-        fireEvent.blur(commentInput);
-      });
+      await userEvent.type(commentInput, 'Exception comment');
      rerenderWithLatestProps();

      expect(formProps.item.comments).toEqual([{ comment: 'Exception comment' }]);
    });
+
+    describe('when opened for editing', () => {
+      beforeEach(() => {
+        setValidItemForEditing();
+      });
+
+      it('item should not be valid when opened for editing', async () => {
+        render();
+        expect(isLatestUpdatedItemValid).toBe(false);
+      });
+
+      it('item should be valid after editing name', async () => {
+        render();
+        const nameInput = renderResult.getByTestId(`${formPrefix}-name-input`);
+
+        await userEvent.type(nameInput, '2');
+
+        expect(latestUpdatedItem.name).toBe('item name2');
+        expect(isLatestUpdatedItemValid).toBe(true);
+      });
+
+      it('item should be valid after editing description', async () => {
+        render();
+        const descriptionInput = renderResult.getByTestId(`${formPrefix}-description-input`);
+
+        await userEvent.type(descriptionInput, 'd');
+
+        expect(latestUpdatedItem.description).toBe('d');
+        expect(isLatestUpdatedItemValid).toBe(true);
+      });
+
+      it('item should be valid after editing comment', async () => {
+        render();
+        const commentInput = renderResult.getByLabelText('Comment Input');
+
+        await userEvent.type(commentInput, 'c');
+
+        expect(latestUpdatedItem.comments).toEqual([{ comment: 'c' }]);
+        expect(isLatestUpdatedItemValid).toBe(true);
+      });
+    });
  });

  describe('Policy section', () => {
@ -280,7 +339,9 @@ describe('Event filter form', () => {

    it('should display loader when policies are still loading', () => {
      formProps.policiesIsLoading = true;
-      formProps.item.tags = [formProps.policies.map((p) => `policy:${p.id}`)[0]];
+      formProps.item.tags = [
+        formProps.policies.map((p) => `${BY_POLICY_ARTIFACT_TAG_PREFIX}${p.id}`)[0],
+      ];
      render();
      expect(renderResult.getByTestId('loading-spinner')).not.toBeNull();
    });
@ -298,7 +359,9 @@ describe('Event filter form', () => {
    });

    it('should call onChange when a policy is selected from the policy selection', async () => {
-      formProps.item.tags = [formProps.policies.map((p) => `policy:${p.id}`)[0]];
+      formProps.item.tags = [
+        formProps.policies.map((p) => `${BY_POLICY_ARTIFACT_TAG_PREFIX}${p.id}`)[0],
+      ];
      render();
      const policyId = formProps.policies[0].id;
      await userEvent.click(renderResult.getByTestId(`${formPrefix}-effectedPolicies-perPolicy`));
@ -308,7 +371,7 @@ describe('Event filter form', () => {
      const expected = createOnChangeArgs({
        item: {
          ...formProps.item,
-          tags: [`policy:${policyId}`],
+          tags: [`${BY_POLICY_ARTIFACT_TAG_PREFIX}${policyId}`],
        },
      });
      expect(formProps.onChange).toHaveBeenCalledWith(expected);
@ -326,7 +389,9 @@ describe('Event filter form', () => {
    });

    it('should retain the previous policy selection when switching from per-policy to global', async () => {
-      formProps.item.tags = [formProps.policies.map((p) => `policy:${p.id}`)[0]];
+      formProps.item.tags = [
+        formProps.policies.map((p) => `${BY_POLICY_ARTIFACT_TAG_PREFIX}${p.id}`)[0],
+      ];
      render();
      const policyId = formProps.policies[0].id;
      // move to per-policy and select the first
@ -339,7 +404,7 @@ describe('Event filter form', () => {
      expect(
        renderResult.queryByTestId(`${formPrefix}-effectedPolicies-policiesSelectable`)
      ).toBeTruthy();
-      expect(formProps.item.tags).toEqual([`policy:${policyId}`]);
+      expect(formProps.item.tags).toEqual([`${BY_POLICY_ARTIFACT_TAG_PREFIX}${policyId}`]);

      // move back to global
      await userEvent.click(renderResult.getByTestId('eventFilters-form-effectedPolicies-global'));
@ -356,10 +421,10 @@ describe('Event filter form', () => {
        renderResult.getByTestId('eventFilters-form-effectedPolicies-perPolicy')
      );
      // eslint-disable-next-line require-atomic-updates
-      formProps.item.tags = [`policy:${policyId}`];
+      formProps.item.tags = [`${BY_POLICY_ARTIFACT_TAG_PREFIX}${policyId}`];
      rerender();
      // on change called with the previous policy
-      expect(formProps.item.tags).toEqual([`policy:${policyId}`]);
+      expect(formProps.item.tags).toEqual([`${BY_POLICY_ARTIFACT_TAG_PREFIX}${policyId}`]);
      // the previous selected policy should be selected
      // expect(renderResult.getByTestId(`policy-${policyId}`)).toHaveAttribute(
      //   'data-test-selected',
@ -380,18 +445,62 @@ describe('Event filter form', () => {
      expect(formProps.onChange).toHaveBeenLastCalledWith(
        expect.objectContaining({
          item: expect.objectContaining({
-            tags: ['some:random_tag', 'policy:id-0'],
+            tags: ['some:random_tag', `${BY_POLICY_ARTIFACT_TAG_PREFIX}id-0`],
          }),
        })
      );
    });
+
+    describe('when opened for editing', () => {
+      beforeEach(() => {
+        setValidItemForEditing();
+      });
+
+      it('item should be valid after changing to global assignment', async () => {
+        formProps.item.tags = [];
+        render();
+        expect(isLatestUpdatedItemValid).toBe(false);
+
+        await userEvent.click(
+          renderResult.getByTestId('eventFilters-form-effectedPolicies-global')
+        );
+
+        expect(isLatestUpdatedItemValid).toBe(true);
+        expect(latestUpdatedItem.tags).toEqual([GLOBAL_ARTIFACT_TAG]);
+      });
+
+      it('item should be valid after changing to per policy assignment', async () => {
+        formProps.item.tags = [GLOBAL_ARTIFACT_TAG];
+        render();
+        expect(isLatestUpdatedItemValid).toBe(false);
+
+        await userEvent.click(
+          renderResult.getByTestId('eventFilters-form-effectedPolicies-perPolicy')
+        );
+
+        expect(isLatestUpdatedItemValid).toBe(true);
+        expect(latestUpdatedItem.tags).toEqual([]);
+      });
+
+      it('item should be valid after changing policy assignment', async () => {
+        formProps.item.tags = [];
+        render();
+        expect(isLatestUpdatedItemValid).toBe(false);
+
+        const policyId = formProps.policies[0].id;
+        await userEvent.click(renderResult.getByTestId(`policy-${policyId}`));
+
+        expect(isLatestUpdatedItemValid).toBe(true);
+        expect(latestUpdatedItem.tags).toEqual([`${BY_POLICY_ARTIFACT_TAG_PREFIX}${policyId}`]);
+      });
+    });
  });

  describe('Policy section with downgraded license', () => {
    beforeEach(() => {
      const policies = createPolicies();
      formProps.policies = policies;
-      formProps.item.tags = [policies.map((p) => `policy:${p.id}`)[0]];
+      formProps.item.tags = [policies.map((p) => `${BY_POLICY_ARTIFACT_TAG_PREFIX}${p.id}`)[0]];
      formProps.mode = 'edit';
      // downgrade license
      (licenseService.isPlatinumPlus as jest.Mock).mockReturnValue(false);
@ -413,7 +522,7 @@ describe('Event filter form', () => {

    it('should show disabled assignment section when edit mode and no license with by policy', async () => {
      render();
-      formProps.item.tags = ['policy:id-0'];
+      formProps.item.tags = [`${BY_POLICY_ARTIFACT_TAG_PREFIX}id-0`];
      rerender();

      expect(
@ -579,6 +688,36 @@ describe('Event filter form', () => {

      expect(await renderResult.findByTestId(tooltipTextSelector)).toBeInTheDocument();
    });
+
+    describe('when opened for editing', () => {
+      beforeEach(() => {
+        setValidItemForEditing();
+      });
+
+      it('item should be valid after changing to event filtering', async () => {
+        formProps.item.tags = [FILTER_PROCESS_DESCENDANTS_TAG];
+        render();
+        expect(isLatestUpdatedItemValid).toBe(false);
+
+        await userEvent.click(renderResult.getByTestId(`${formPrefix}-filterEventsButton`));
+
+        expect(isLatestUpdatedItemValid).toBe(true);
+        expect(latestUpdatedItem.tags).toEqual([]);
+      });
+
+      it('item should be valid after changing to process descendant filtering', async () => {
+        formProps.item.tags = [];
+        render();
+        expect(isLatestUpdatedItemValid).toBe(false);
+
+        await userEvent.click(
+          renderResult.getByTestId(`${formPrefix}-filterProcessDescendantsButton`)
+        );
+
+        expect(isLatestUpdatedItemValid).toBe(true);
+        expect(latestUpdatedItem.tags).toEqual([FILTER_PROCESS_DESCENDANTS_TAG]);
+      });
+    });
  });

  describe('Warnings', () => {
--- a/x-pack/solutions/security/plugins/security_solution/public/management/pages/event_filters/view/components/form.tsx
+++ b/x-pack/solutions/security/plugins/security_solution/public/management/pages/event_filters/view/components/form.tsx
@ -197,9 +197,7 @@ export const EventFiltersForm: React.FC<ArtifactFormComponentProps & { allowSele
        [exception]
      );

-      const [areConditionsValid, setAreConditionsValid] = useState(
-        !!exception.entries.length || false
-      );
+      const [areConditionsValid, setAreConditionsValid] = useState(!!exception.entries.length);
      // compute this for initial render only
      const existingComments = useMemo<ExceptionListItemSchema['comments']>(
        () => (exception as ExceptionListItemSchema)?.comments,
@ -238,7 +236,7 @@ export const EventFiltersForm: React.FC<ArtifactFormComponentProps & { allowSele
          cleanupEntries(item);
          onChange({
            item,
-            isValid: isFormValid && areConditionsValid,
+            isValid: isFormValid && areConditionsValid && hasFormChanged,
            confirmModalLabels: hasWildcardWithWrongOperator
              ? CONFIRM_WARNING_MODAL_LABELS(
                  i18n.translate(
@ -251,7 +249,14 @@ export const EventFiltersForm: React.FC<ArtifactFormComponentProps & { allowSele
              : undefined,
          });
        },
-        [areConditionsValid, exception, isFormValid, onChange, hasWildcardWithWrongOperator]
+        [
+          areConditionsValid,
+          exception,
+          hasFormChanged,
+          isFormValid,
+          onChange,
+          hasWildcardWithWrongOperator,
+        ]
      );

      // set initial state of `wasByPolicy` that checks
@ -556,11 +561,11 @@ export const EventFiltersForm: React.FC<ArtifactFormComponentProps & { allowSele
      // conditions and handler
      const handleOnBuilderChange = useCallback(
        (arg: OnChangeProps) => {
-          const hasDuplicates =
+          const isCalledWithoutChanges =
            (!hasFormChanged && arg.exceptionItems[0] === undefined) ||
            isEqual(arg.exceptionItems[0]?.entries, exception?.entries);

-          if (hasDuplicates) {
+          if (isCalledWithoutChanges) {
            const addedFields = arg.exceptionItems[0]?.entries.map((e) => e.field) || [''];

            if (isFilterProcessDescendantsFeatureEnabled && isFilterProcessDescendantsSelected) {
@ -568,7 +573,6 @@ export const EventFiltersForm: React.FC<ArtifactFormComponentProps & { allowSele
            }

            setHasDuplicateFields(computeHasDuplicateFields(getAddedFieldsCounts(addedFields)));
-            if (!hasFormChanged) setHasFormChanged(true);
            return;
          } else {
            setHasDuplicateFields(false);