Skip to content

Commit

Permalink
[Defend Workflows] Fix bug when event filter value cannot be changed …
Browse files Browse the repository at this point in the history
…without using {backspace} (elastic#192196)

## 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: 5da28aa
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
f9d60cf 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 <[email protected]>
  • Loading branch information
gergoabraham and elasticmachine committed Sep 13, 2024
1 parent fa74847 commit b31d119
Show file tree
Hide file tree
Showing 4 changed files with 200 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
<AutocompleteFieldMatchComponent
Expand All @@ -192,16 +199,12 @@ describe('AutocompleteFieldMatchComponent', () => {
/>
);

(
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(
<AutocompleteFieldMatchComponent
Expand All @@ -222,15 +225,39 @@ describe('AutocompleteFieldMatchComponent', () => {
/>
);

(
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(
<AutocompleteFieldMatchComponent
autocompleteService={autocompleteStartMock}
indexPattern={{
fields,
id: '1234',
title: 'logstash-*',
}}
isClearable={false}
isDisabled={false}
isLoading={false}
onChange={mockOnChange}
onError={jest.fn()}
placeholder="Placeholder text"
selectedField={getField('machine.os.raw')}
selectedValue="value 1"
/>
);

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,
Expand Down Expand Up @@ -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 ');

Expand Down Expand Up @@ -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({
Expand All @@ -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(
<AutocompleteFieldMatchComponent
autocompleteService={autocompleteStartMock}
indexPattern={{
fields,
id: '1234',
title: 'logstash-*',
}}
isClearable={false}
isDisabled={false}
isLoading={false}
onChange={jest.fn()}
onError={jest.fn()}
placeholder="Placeholder text"
selectedField={getField('machine.os.raw')}
selectedValue="windows"
/>
);

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(
<AutocompleteFieldMatchComponent
Expand All @@ -333,18 +398,15 @@ describe('AutocompleteFieldMatchComponent', () => {
/>
);
act(() => {
(
wrapper.find(EuiComboBox).props() as unknown as {
onSearchChange: (a: string) => void;
}
).onSearchChange(' value 1');
findEuiComboBox().onSearchChange(' value 1');
});

wrapper.update();
const euiFormHelptext = wrapper.find(EuiFormHelpText);
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(
<AutocompleteFieldMatchComponent
Expand All @@ -368,6 +430,7 @@ describe('AutocompleteFieldMatchComponent', () => {
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(
<AutocompleteFieldMatchComponent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,26 @@ export const AutocompleteFieldMatchComponent: React.FC<AutocompleteFieldMatchPro
handleWarning(warning);

if (!err) handleSpacesWarning(searchVal);
setSearchQuery(searchVal);
}

if (searchVal) {
// Clear selected option when user types to allow user to modify value without {backspace}
onChange('');
}

// Update search query unconditionally to show correct suggestions even when input is cleared
setSearchQuery(searchVal);
},
[handleError, handleSpacesWarning, isRequired, selectedField, touched, handleWarning, warning]
[
selectedField,
onChange,
isRequired,
touched,
handleError,
handleWarning,
warning,
handleSpacesWarning,
]
);

const handleCreateOption = useCallback(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { APP_EVENT_FILTERS_PATH } from '../../../../../common/constants';
import type { ArtifactsFixtureType } from '../../fixtures/artifacts_page';
import { getArtifactsListTestsData } from '../../fixtures/artifacts_page';
import {
createArtifactList,
createPerPolicyArtifact,
removeAllArtifacts,
} from '../../tasks/artifacts';
import { loadPage } from '../../tasks/common';
import { indexEndpointHosts } from '../../tasks/index_endpoint_hosts';
import { login } from '../../tasks/login';
import type { ReturnTypeFromChainable } from '../../types';

describe('Event Filters', { tags: ['@ess', '@serverless'] }, () => {
let endpointData: ReturnTypeFromChainable<typeof indexEndpointHosts> | 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);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ interface FormEditingDescription {
}>;
}

interface ArtifactsFixtureType {
export interface ArtifactsFixtureType {
title: string;
pagePrefix: string;
tabId: string;
Expand Down Expand Up @@ -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'],
Expand Down

0 comments on commit b31d119

Please sign in to comment.