From b31d119e5532c362c44a134547f461fd7db56770 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20=C3=81brah=C3=A1m?= Date: Fri, 13 Sep 2024 18:59:44 +0200 Subject: [PATCH] [Defend Workflows] Fix bug when event filter value cannot be changed without using {backspace} (#192196) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary This PR does 2 things around editing the value field for an Event Filter. 1. It fixes the issue when during editing an existing Event Filter you cannot update the value without pressing {backspace}, by clearing selection when user is typing: 5da28aa25404e6a2bdc8a2e9cfce2f6546976d07 Before: ![before](https://github.com/user-attachments/assets/7355c788-a9fd-4ea5-81b3-89ae41db2ee7) ➡️ After: ![after](https://github.com/user-attachments/assets/aa928fa4-6203-4fad-8f9b-4e586ac4d562) 2. Improves suggestions: before the change, suggestions were there initially, but after they are narrowed down by typing, they won't be displayed again after the user clears the input field. Therefore f9d60cf223644a3072d1a60fe273586338247b96 sets the suggestion search query unconditionally, helping with this issue. Before: ![before](https://github.com/user-attachments/assets/87ccfba6-5b9d-4976-a5af-13c3d56db373) ➡️ After: ![after](https://github.com/user-attachments/assets/c21a909c-4b45-470e-9e77-9edc269f07f7) ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Elastic Machine --- .../src/field_value_match/index.test.tsx | 121 +++++++++++++----- .../src/field_value_match/index.tsx | 20 ++- .../cypress/e2e/artifacts/event_filters.cy.ts | 87 +++++++++++++ .../cypress/fixtures/artifacts_page.ts | 6 +- 4 files changed, 200 insertions(+), 34 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/management/cypress/e2e/artifacts/event_filters.cy.ts diff --git a/packages/kbn-securitysolution-autocomplete/src/field_value_match/index.test.tsx b/packages/kbn-securitysolution-autocomplete/src/field_value_match/index.test.tsx index 1247a9cd3d5bd9..e83d79b180e905 100644 --- a/packages/kbn-securitysolution-autocomplete/src/field_value_match/index.test.tsx +++ b/packages/kbn-securitysolution-autocomplete/src/field_value_match/index.test.tsx @@ -32,6 +32,13 @@ describe('AutocompleteFieldMatchComponent', () => { .fn() .mockResolvedValue([false, true, ['value 3', 'value 4'], jest.fn()]); + const findEuiComboBox = () => + wrapper.find(EuiComboBox).props() as unknown as { + onChange: (a: EuiComboBoxOptionOption[]) => void; + onSearchChange: (a: string) => void; + onCreateOption: (a: string) => void; + }; + beforeEach(() => { (useFieldValueAutocomplete as jest.Mock).mockReturnValue([ false, @@ -171,7 +178,7 @@ describe('AutocompleteFieldMatchComponent', () => { ).toEqual('127.0.0.1'); }); - test('it invokes "onChange" when new value created', async () => { + test('it invokes "onChange" when new value created', () => { const mockOnChange = jest.fn(); wrapper = mount( { /> ); - ( - wrapper.find(EuiComboBox).props() as unknown as { - onCreateOption: (a: string) => void; - } - ).onCreateOption('127.0.0.1'); + findEuiComboBox().onCreateOption('127.0.0.1'); expect(mockOnChange).toHaveBeenCalledWith('127.0.0.1'); }); - test('it invokes "onChange" when new value selected', async () => { + test('it invokes "onChange" when new value selected', () => { const mockOnChange = jest.fn(); wrapper = mount( { /> ); - ( - wrapper.find(EuiComboBox).props() as unknown as { - onChange: (a: EuiComboBoxOptionOption[]) => void; - } - ).onChange([{ label: 'value 1' }]); + findEuiComboBox().onChange([{ label: 'value 1' }]); expect(mockOnChange).toHaveBeenCalledWith('value 1'); }); + test('it invokes "onChange" with empty value (i.e. clears selection) when new value searched', () => { + const mockOnChange = jest.fn(); + wrapper = mount( + + ); + + act(() => { + findEuiComboBox().onSearchChange('value 12'); + }); + + expect(mockOnChange).toHaveBeenCalledWith(''); + }); + test('should show the warning helper text if the new value contains spaces when change', async () => { (useFieldValueAutocomplete as jest.Mock).mockReturnValue([ false, @@ -259,13 +286,7 @@ describe('AutocompleteFieldMatchComponent', () => { /> ); - await waitFor(() => - ( - wrapper.find(EuiComboBox).props() as unknown as { - onChange: (a: EuiComboBoxOptionOption[]) => void; - } - ).onChange([{ label: ' value 1 ' }]) - ); + await waitFor(() => findEuiComboBox().onChange([{ label: ' value 1 ' }])); wrapper.update(); expect(mockOnChange).toHaveBeenCalledWith(' value 1 '); @@ -293,11 +314,7 @@ describe('AutocompleteFieldMatchComponent', () => { /> ); act(() => { - ( - wrapper.find(EuiComboBox).props() as unknown as { - onSearchChange: (a: string) => void; - } - ).onSearchChange('value 1'); + findEuiComboBox().onSearchChange('value 1'); }); expect(useFieldValueAutocomplete).toHaveBeenCalledWith({ @@ -313,6 +330,54 @@ describe('AutocompleteFieldMatchComponent', () => { selectedField: getField('machine.os.raw'), }); }); + + test('it refreshes autocomplete with search query when input field is cleared', () => { + wrapper = mount( + + ); + + act(() => { + findEuiComboBox().onSearchChange('value 1'); + }); + act(() => { + findEuiComboBox().onSearchChange(''); + }); + + // 1st call is initial render, 2nd call sets the search query: + expect(useFieldValueAutocomplete).toHaveBeenNthCalledWith(2, { + autocompleteService: autocompleteStartMock, + fieldValue: 'windows', + indexPattern: { fields, id: '1234', title: 'logstash-*' }, + operatorType: 'match', + query: 'value 1', + selectedField: getField('machine.os.raw'), + }); + // last call is the refresh when input field is cleared + expect(useFieldValueAutocomplete).toHaveBeenLastCalledWith({ + autocompleteService: autocompleteStartMock, + fieldValue: 'windows', + indexPattern: { fields, id: '1234', title: 'logstash-*' }, + operatorType: 'match', + query: '', + selectedField: getField('machine.os.raw'), + }); + }); + test('should show the warning helper text if the new value contains spaces when searching a new query', () => { wrapper = mount( { /> ); act(() => { - ( - wrapper.find(EuiComboBox).props() as unknown as { - onSearchChange: (a: string) => void; - } - ).onSearchChange(' value 1'); + findEuiComboBox().onSearchChange(' value 1'); }); wrapper.update(); @@ -345,6 +406,7 @@ describe('AutocompleteFieldMatchComponent', () => { expect(euiFormHelptext.length).toBeTruthy(); expect(euiFormHelptext.text()).toEqual('Warning: there is a space'); }); + test('should show the warning helper text if selectedValue contains spaces when editing', () => { wrapper = mount( { expect(euiFormHelptext.length).toBeTruthy(); expect(euiFormHelptext.text()).toEqual('Warning: there is a space'); }); + test('should not show the warning helper text if selectedValue is falsy', () => { wrapper = mount( { + let endpointData: ReturnTypeFromChainable | undefined; + + const CONDITION_VALUE = 'valuesAutocompleteMatch'; + const SUBMIT_BUTTON = 'EventFiltersListPage-flyout-submitButton'; + + before(() => { + indexEndpointHosts().then((indexEndpoints) => { + endpointData = indexEndpoints; + }); + }); + + after(() => { + removeAllArtifacts(); + + endpointData?.cleanup(); + endpointData = undefined; + }); + + beforeEach(() => { + removeAllArtifacts(); + }); + + describe('when editing event filter value', () => { + let eventFiltersMock: ArtifactsFixtureType; + beforeEach(() => { + login(); + + eventFiltersMock = getArtifactsListTestsData().find( + ({ tabId }) => tabId === 'eventFilters' + ) as ArtifactsFixtureType; + + createArtifactList(eventFiltersMock.createRequestBody.list_id); + createPerPolicyArtifact(eventFiltersMock.artifactName, eventFiltersMock.createRequestBody); + + loadPage(APP_EVENT_FILTERS_PATH); + + cy.getByTestSubj('EventFiltersListPage-card-header-actions-button').click(); + cy.getByTestSubj('EventFiltersListPage-card-cardEditAction').click(); + cy.getByTestSubj('EventFiltersListPage-flyout').should('exist'); + }); + + it('should be able to modify after deleting value with {backspace}', () => { + cy.getByTestSubj(CONDITION_VALUE).type(' {backspace}.lnk{enter}'); + cy.getByTestSubj(SUBMIT_BUTTON).click(); + + cy.getByTestSubj('EventFiltersListPage-flyout').should('not.exist'); + cy.contains('notepad.exe.lnk'); + }); + + it('should be able to modify without using {backspace}', () => { + cy.getByTestSubj(CONDITION_VALUE).type('.lnk{enter}'); + cy.getByTestSubj(SUBMIT_BUTTON).click(); + + cy.getByTestSubj('EventFiltersListPage-flyout').should('not.exist'); + cy.contains('notepad.exe.lnk'); + }); + + it('should show suggestions when filter value is cleared', () => { + cy.getByTestSubj(CONDITION_VALUE).clear(); + cy.getByTestSubj(CONDITION_VALUE).type('aaaaaaaaaaaaaa as custom input'); + cy.get('button[role="option"]').should('have.length', 0); + + cy.getByTestSubj(CONDITION_VALUE).find('input').clear(); + cy.get('button[role="option"]').should('have.length.above', 1); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/management/cypress/fixtures/artifacts_page.ts b/x-pack/plugins/security_solution/public/management/cypress/fixtures/artifacts_page.ts index 97a6807a59f0af..2113347bfc4460 100644 --- a/x-pack/plugins/security_solution/public/management/cypress/fixtures/artifacts_page.ts +++ b/x-pack/plugins/security_solution/public/management/cypress/fixtures/artifacts_page.ts @@ -17,7 +17,7 @@ interface FormEditingDescription { }>; } -interface ArtifactsFixtureType { +export interface ArtifactsFixtureType { title: string; pagePrefix: string; tabId: string; @@ -275,10 +275,10 @@ export const getArtifactsListTestsData = (): ArtifactsFixtureType[] => [ list_id: ENDPOINT_ARTIFACT_LISTS.eventFilters.id, entries: [ { - field: 'agent.id', + field: 'process.name', operator: 'included', type: 'match', - value: 'mr agent', + value: 'notepad.exe', }, ], os_types: ['windows'],