Skip to content

Commit 91b9343

Browse files
[9.1] [UnifiedSearch][ESQL] Fix edited query overwriting when a request is finished (#224671) (#226355)
# Backport This will backport the following commits from `main` to `9.1`: - [[UnifiedSearch][ESQL] Fix edited query overwriting when a request is finished (#224671)](#224671) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Matthias Wilhelm","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-07-03T08:14:41Z","message":"[UnifiedSearch][ESQL] Fix edited query overwriting when a request is finished (#224671)\n\nResolved overwriting changes to the query in the ESQL Editor in UnifiedSearch while the request is still running when the previous request finished","sha":"ca7b021814e3606434934c9dac0b1be8fbed4338","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Discover","release_note:fix","Team:DataDiscovery","Feature:Unified search","Team:ESQL","backport:version","v9.1.0","v8.19.0","v9.2.0"],"title":"[UnifiedSearch][ESQL] Fix edited query overwriting when a request is finished","number":224671,"url":"https://github.com/elastic/kibana/pull/224671","mergeCommit":{"message":"[UnifiedSearch][ESQL] Fix edited query overwriting when a request is finished (#224671)\n\nResolved overwriting changes to the query in the ESQL Editor in UnifiedSearch while the request is still running when the previous request finished","sha":"ca7b021814e3606434934c9dac0b1be8fbed4338"}},"sourceBranch":"main","suggestedTargetBranches":["9.1"],"targetPullRequestStates":[{"branch":"9.1","label":"v9.1.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/226344","number":226344,"state":"OPEN"},{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/224671","number":224671,"mergeCommit":{"message":"[UnifiedSearch][ESQL] Fix edited query overwriting when a request is finished (#224671)\n\nResolved overwriting changes to the query in the ESQL Editor in UnifiedSearch while the request is still running when the previous request finished","sha":"ca7b021814e3606434934c9dac0b1be8fbed4338"}}]}] BACKPORT--> Co-authored-by: Matthias Wilhelm <[email protected]>
1 parent 9df3fca commit 91b9343

File tree

2 files changed

+60
-15
lines changed

2 files changed

+60
-15
lines changed

src/platform/plugins/shared/unified_search/public/search_bar/search_bar.test.tsx

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99

1010
import React from 'react';
11-
import SearchBar from './search_bar';
11+
import SearchBar, { SearchBarProps, SearchBarState, SearchBarUI } from './search_bar';
1212
import { BehaviorSubject } from 'rxjs';
1313
import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public';
1414
import { indexPatternEditorPluginMock as dataViewEditorPluginMock } from '@kbn/data-view-editor-plugin/public/mocks';
@@ -366,4 +366,48 @@ describe('SearchBar', () => {
366366
true
367367
);
368368
});
369+
370+
describe('SearchBarUI.getDerivedStateFromProps', () => {
371+
it('should not return the esql query if props.query doesnt change but loading state changes', () => {
372+
const nextProps = {
373+
query: { esql: 'test' },
374+
isLoading: false,
375+
} as unknown as SearchBarProps;
376+
const prevState = {
377+
currentProps: {
378+
query: { esql: 'test' },
379+
},
380+
query: { esql: 'test_edited' },
381+
isLoading: true,
382+
} as unknown as SearchBarState;
383+
384+
const result = SearchBarUI.getDerivedStateFromProps(nextProps, prevState);
385+
// if the query was returned, it would overwrite the state in the underlying ES|QL editor
386+
expect(result).toEqual({
387+
currentProps: { isLoading: false, query: { esql: 'test' } },
388+
});
389+
});
390+
it('should return the query if props.query and loading state changes', () => {
391+
const nextProps = {
392+
query: { esql: 'test_new_props' },
393+
isLoading: false,
394+
} as unknown as SearchBarProps;
395+
const prevState = {
396+
currentProps: {
397+
query: { esql: 'test' },
398+
},
399+
query: { esql: 'test_edited' },
400+
isLoading: true,
401+
} as unknown as SearchBarState;
402+
403+
const result = SearchBarUI.getDerivedStateFromProps(nextProps, prevState);
404+
// here it makes sense to return the query, because the props.query has changed
405+
expect(result).toEqual({
406+
currentProps: { isLoading: false, query: { esql: 'test_new_props' } },
407+
query: {
408+
esql: 'test_new_props',
409+
},
410+
});
411+
});
412+
});
369413
});

src/platform/plugins/shared/unified_search/public/search_bar/search_bar.tsx

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ export interface SearchBarOwnProps<QT extends AggregateQuery | Query = Query> {
147147
export type SearchBarProps<QT extends Query | AggregateQuery = Query> = SearchBarOwnProps<QT> &
148148
SearchBarInjectedDeps;
149149

150-
interface State<QT extends Query | AggregateQuery = Query> {
150+
export interface SearchBarState<QT extends Query | AggregateQuery = Query> {
151151
isFiltersVisible: boolean;
152152
openQueryBarMenu: boolean;
153153
showSavedQueryPopover: boolean;
@@ -157,9 +157,9 @@ interface State<QT extends Query | AggregateQuery = Query> {
157157
dateRangeTo: string;
158158
}
159159

160-
class SearchBarUI<QT extends (Query | AggregateQuery) | Query = Query> extends Component<
160+
export class SearchBarUI<QT extends (Query | AggregateQuery) | Query = Query> extends Component<
161161
SearchBarProps<QT> & WithEuiThemeProps,
162-
State<QT | Query>
162+
SearchBarState<QT | Query>
163163
> {
164164
public static defaultProps = {
165165
showQueryMenu: true,
@@ -177,7 +177,7 @@ class SearchBarUI<QT extends (Query | AggregateQuery) | Query = Query> extends C
177177

178178
public static getDerivedStateFromProps(
179179
nextProps: SearchBarProps,
180-
prevState: State<AggregateQuery | Query>
180+
prevState: SearchBarState<AggregateQuery | Query>
181181
) {
182182
if (isEqual(prevState.currentProps, nextProps)) {
183183
return null;
@@ -204,7 +204,13 @@ class SearchBarUI<QT extends (Query | AggregateQuery) | Query = Query> extends C
204204
query: '',
205205
language: nextProps.query.language,
206206
};
207-
} else if (nextProps.query && !isOfQueryType(nextProps.query)) {
207+
} else if (
208+
nextProps.query &&
209+
isOfAggregateQueryType(nextProps.query) &&
210+
nextProps.query.esql !== get(prevState, 'currentProps.query.esql')
211+
) {
212+
// this code is just overriding the query with a new one in case the query has changed in props
213+
// without the props check it would override any edits to the query, if e.g. results were returned and isLoading switches from true to false
208214
nextQuery = nextProps.query;
209215
}
210216

@@ -248,14 +254,9 @@ class SearchBarUI<QT extends (Query | AggregateQuery) | Query = Query> extends C
248254
/*
249255
Keep the "draft" value in local state until the user actually submits the query. There are a couple advantages:
250256
251-
1. Each app doesn't have to maintain its own "draft" value if it wants to put off updating the query in app state
252-
until the user manually submits their changes. Most apps have watches on the query value in app state so we don't
253-
want to trigger those on every keypress. Also, some apps (e.g. dashboard) already juggle multiple query values,
254-
each with slightly different semantics and I'd rather not add yet another variable to the mix.
255-
256-
2. Changes to the local component state won't trigger an Angular digest cycle. Triggering digest cycles on every
257-
keypress has been a major source of performance issues for us in previous implementations of the query bar.
258-
See https://github.com/elastic/kibana/issues/14086
257+
Each app doesn't have to maintain its own "draft" value if it wants to put off updating the query in app state
258+
until the user manually submits their changes. Some apps have watches on the query value in app state so we don't
259+
want to trigger those on every keypress.
259260
*/
260261
public state = {
261262
isFiltersVisible: true,
@@ -265,7 +266,7 @@ class SearchBarUI<QT extends (Query | AggregateQuery) | Query = Query> extends C
265266
query: this.props.query ? { ...this.props.query } : undefined,
266267
dateRangeFrom: get(this.props, 'dateRangeFrom', 'now-15m'),
267268
dateRangeTo: get(this.props, 'dateRangeTo', 'now'),
268-
} as State<QT>;
269+
} as SearchBarState<QT>;
269270

270271
public isDirty = () => {
271272
if (!this.props.showDatePicker && this.state.query && this.props.query) {

0 commit comments

Comments
 (0)