diff --git a/src/platform/packages/private/kbn-esql-editor/src/esql_editor.tsx b/src/platform/packages/private/kbn-esql-editor/src/esql_editor.tsx index 2a870978c4bcb..2094007c2a866 100644 --- a/src/platform/packages/private/kbn-esql-editor/src/esql_editor.tsx +++ b/src/platform/packages/private/kbn-esql-editor/src/esql_editor.tsx @@ -25,13 +25,12 @@ import useObservable from 'react-use/lib/useObservable'; import { KBN_FIELD_TYPES } from '@kbn/field-types'; import type { CoreStart } from '@kbn/core/public'; import type { DataViewsPublicPluginStart } from '@kbn/data-views-plugin/public'; -import type { AggregateQuery, TimeRange } from '@kbn/es-query'; +import type { TimeRange } from '@kbn/es-query'; import type { ExpressionsStart } from '@kbn/expressions-plugin/public'; import { useKibana } from '@kbn/kibana-react-plugin/public'; import type { ILicense } from '@kbn/licensing-plugin/public'; import { ESQLLang, ESQL_LANG_ID, monaco, type ESQLCallbacks } from '@kbn/monaco'; import React, { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import { fixESQLQueryWithVariables } from '@kbn/esql-utils'; import { createPortal } from 'react-dom'; import { css } from '@emotion/react'; import { ESQLVariableType, type ESQLControlVariable } from '@kbn/esql-types'; @@ -59,6 +58,7 @@ import { esqlEditorStyles, } from './esql_editor.styles'; import type { ESQLEditorProps, ESQLEditorDeps, ControlsContext } from './types'; +import { useEditorQuerySync } from './useEditorQuerySync'; // for editor width smaller than this value we want to start hiding some text const BREAKPOINT_WIDTH = 540; @@ -129,16 +129,8 @@ export const ESQLEditor = memo(function ESQLEditor({ const activeSolutionId = useObservable(core.chrome.getActiveSolutionNavId$()); - const fixedQuery = useMemo( - () => fixESQLQueryWithVariables(query.esql, esqlVariables), - [esqlVariables, query.esql] - ); - const variablesService = kibana.services?.esql?.variablesService; const histogramBarTarget = uiSettings?.get('histogram:barTarget') ?? 50; - const [code, setCode] = useState(fixedQuery ?? ''); - // To make server side errors less "sticky", register the state of the code when submitting - const [codeWhenSubmitted, setCodeStateOnSubmission] = useState(code); const [editorHeight, setEditorHeight] = useState( editorIsInline ? EDITOR_INITIAL_HEIGHT_INLINE_EDITING : EDITOR_INITIAL_HEIGHT ); @@ -156,10 +148,29 @@ export const ESQLEditor = memo(function ESQLEditor({ const [isHistoryOpen, setIsHistoryOpen] = useState(false); const [isLanguageComponentOpen, setIsLanguageComponentOpen] = useState(false); const [isCodeEditorExpandedFocused, setIsCodeEditorExpandedFocused] = useState(false); - const [isQueryLoading, setIsQueryLoading] = useState(true); const [abortController, setAbortController] = useState(new AbortController()); const [license, setLicense] = useState(undefined); + const { + handleQuerySubmit, + handleQueryUpdate, + isQueryLoading, + code, + codeWhenSubmitted, + fixedQuery, + } = useEditorQuerySync({ + isLoading: isLoading ?? false, + initialQueryEsql: query.esql, + esqlVariables, + editorIsInline: Boolean(editorIsInline), + isEditorMounted: Boolean(editor1.current), // Pass boolean indicating if editor is mounted + allowQueryCancellation, + onTextLangQuerySubmit, + currentAbortController: abortController, + setNewAbortController: setAbortController, + onTextLangQueryChange, + }); + // contains both client side validation and server messages const [editorMessages, setEditorMessages] = useState<{ errors: MonacoMessage[]; @@ -168,29 +179,6 @@ export const ESQLEditor = memo(function ESQLEditor({ errors: serverErrors ? parseErrors(serverErrors, code) : [], warnings: serverWarning ? parseWarning(serverWarning) : [], }); - const onQueryUpdate = useCallback( - (value: string) => { - onTextLangQueryChange({ esql: value } as AggregateQuery); - }, - [onTextLangQueryChange] - ); - - const onQuerySubmit = useCallback(() => { - if (isQueryLoading && isLoading && allowQueryCancellation) { - abortController?.abort(); - setIsQueryLoading(false); - } else { - setIsQueryLoading(true); - const abc = new AbortController(); - setAbortController(abc); - - const currentValue = editor1.current?.getValue(); - if (currentValue != null) { - setCodeStateOnSubmission(currentValue); - } - onTextLangQuerySubmit({ esql: currentValue } as AggregateQuery, abc); - } - }, [isQueryLoading, isLoading, allowQueryCancellation, abortController, onTextLangQuerySubmit]); const onCommentLine = useCallback(() => { const currentSelection = editor1?.current?.getSelection(); @@ -218,18 +206,6 @@ export const ESQLEditor = memo(function ESQLEditor({ } }, []); - useEffect(() => { - if (!isLoading) setIsQueryLoading(false); - }, [isLoading]); - - useEffect(() => { - if (editor1.current) { - if (code !== fixedQuery) { - setCode(fixedQuery); - } - } - }, [code, fixedQuery]); - // Enable the variables service if the feature is supported in the consumer app useEffect(() => { if (controlsContext?.supportsControls) { @@ -297,7 +273,7 @@ export const ESQLEditor = memo(function ESQLEditor({ monaco.editor.registerCommand('esql.control.time_literal.create', async (...args) => { const position = editor1.current?.getPosition(); await triggerControl( - fixedQuery, + code, // Use code from hook ESQLVariableType.TIME_LITERAL, position, uiActions, @@ -310,7 +286,7 @@ export const ESQLEditor = memo(function ESQLEditor({ monaco.editor.registerCommand('esql.control.fields.create', async (...args) => { const position = editor1.current?.getPosition(); await triggerControl( - fixedQuery, + code, // Use code from hook ESQLVariableType.FIELDS, position, uiActions, @@ -323,7 +299,7 @@ export const ESQLEditor = memo(function ESQLEditor({ monaco.editor.registerCommand('esql.control.values.create', async (...args) => { const position = editor1.current?.getPosition(); await triggerControl( - fixedQuery, + code, // Use code from hook ESQLVariableType.VALUES, position, uiActions, @@ -336,7 +312,7 @@ export const ESQLEditor = memo(function ESQLEditor({ monaco.editor.registerCommand('esql.control.functions.create', async (...args) => { const position = editor1.current?.getPosition(); await triggerControl( - fixedQuery, + code, // Use code from hook ESQLVariableType.FUNCTIONS, position, uiActions, @@ -349,7 +325,7 @@ export const ESQLEditor = memo(function ESQLEditor({ editor1.current?.addCommand( // eslint-disable-next-line no-bitwise monaco.KeyMod.CtrlCmd | monaco.KeyCode.Enter, - onQuerySubmit + handleQuerySubmit // Use handleQuerySubmit from the hook ); const styles = esqlEditorStyles( @@ -781,7 +757,7 @@ export const ESQLEditor = memo(function ESQLEditor({ > { editor1.current = editor; const model = editor.getModel(); @@ -925,8 +901,8 @@ export const ESQLEditor = memo(function ESQLEditor({ }} code={code} onErrorClick={onErrorClick} - runQuery={onQuerySubmit} - updateQuery={onQueryUpdate} + runQuery={handleQuerySubmit} // Use handleQuerySubmit from the hook + updateQuery={handleQueryUpdate} // Add updateQuery prop back detectedTimestamp={detectedTimestamp} hideRunQueryText={hideRunQueryText} editorIsInline={editorIsInline} diff --git a/src/platform/packages/private/kbn-esql-editor/src/useEditorQuerySync.test.tsx b/src/platform/packages/private/kbn-esql-editor/src/useEditorQuerySync.test.tsx new file mode 100644 index 0000000000000..0e94609ba9228 --- /dev/null +++ b/src/platform/packages/private/kbn-esql-editor/src/useEditorQuerySync.test.tsx @@ -0,0 +1,254 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ +import { renderHook, act } from '@testing-library/react'; +import { useEditorQuerySync, UseEditorQuerySyncArgs } from './useEditorQuerySync'; + +describe('useEditorQuerySync', () => { + // Default props to be used across tests + const defaultProps: UseEditorQuerySyncArgs = { + isLoading: false, + initialQueryEsql: 'FROM test', + editorIsInline: false, + isEditorMounted: true, + allowQueryCancellation: false, + onTextLangQuerySubmit: jest.fn(), + currentAbortController: new AbortController(), + setNewAbortController: jest.fn(), + onTextLangQueryChange: jest.fn(), + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should initialize with the provided initialQueryEsql', () => { + const { result } = renderHook(() => useEditorQuerySync(defaultProps)); + + expect(result.current.code).toBe('FROM test'); + expect(result.current.codeWhenSubmitted).toBe('FROM test'); + expect(result.current.isQueryLoading).toBe(false); + }); + + it('should update isQueryLoading when isLoading changes', () => { + const { result, rerender } = renderHook((props) => useEditorQuerySync(props), { + initialProps: { ...defaultProps, isLoading: false }, + }); + + // Simulate a loading change + rerender({ ...defaultProps, isLoading: true }); + + expect(result.current.isQueryLoading).toBe(true); + + // Simulate a loading change back to false + rerender({ ...defaultProps, isLoading: false }); + + expect(result.current.isQueryLoading).toBe(false); + }); + + it('should handle code updates via handleQueryUpdate', () => { + const onTextLangQueryChange = jest.fn(); + + const { result } = renderHook(() => + useEditorQuerySync({ + ...defaultProps, + onTextLangQueryChange, + }) + ); + + // Act: update the code + act(() => { + result.current.handleQueryUpdate('FROM new_table'); + }); + + // Assert: code should be updated + expect(result.current.code).toBe('FROM new_table'); + + // Assert: onTextLangQueryChange should be called since editorIsInline is false + expect(onTextLangQueryChange).toHaveBeenCalledWith({ esql: 'FROM new_table' }); + }); + + it('should not change code but trigger onTextLangQueryChange when editorIsInline is true', () => { + const onTextLangQueryChange = jest.fn(); + + const { result } = renderHook(() => + useEditorQuerySync({ + ...defaultProps, + editorIsInline: true, + onTextLangQueryChange, + }) + ); + + act(() => { + result.current.handleQueryUpdate('FROM new_table'); + }); + expect(result.current.code).toBe('FROM test'); // Should not change the code in inline mode + + // Assert: onTextLangQueryChange should not be called when editorIsInline is true + expect(onTextLangQueryChange).toHaveBeenCalled(); + }); + + it('should handle query submission when not loading', () => { + const onTextLangQuerySubmit = jest.fn(); + const abortController = new AbortController(); + + const { result } = renderHook(() => + useEditorQuerySync({ + ...defaultProps, + isLoading: false, + onTextLangQuerySubmit, + currentAbortController: abortController, + }) + ); + + // Act: submit the query + act(() => { + result.current.handleQuerySubmit(); + }); + + // Assert: onTextLangQuerySubmit should be called with the current code + expect(onTextLangQuerySubmit).toHaveBeenCalledWith({ esql: 'FROM test' }, abortController); + + // Assert: codeWhenSubmitted should be updated to the current code + expect(result.current.codeWhenSubmitted).toBe('FROM test'); + + // Assert: isQueryLoading should be set to true after submission + expect(result.current.isQueryLoading).toBe(true); + }); + + it('should handle query cancellation when loading and allowQueryCancellation is true', () => { + const onTextLangQuerySubmit = jest.fn(); + const abortController = new AbortController(); + const abortSpy = jest.spyOn(abortController, 'abort'); + + const { result } = renderHook(() => + useEditorQuerySync({ + ...defaultProps, + isLoading: true, + allowQueryCancellation: true, + onTextLangQuerySubmit, + currentAbortController: abortController, + }) + ); + + // Act: submit the query (which should trigger cancellation) + act(() => { + result.current.handleQuerySubmit(); + }); + + // Assert: the current abort controller should be aborted + expect(abortSpy).toHaveBeenCalled(); + }); + + it('should not overwrite editor content when loading state changes, but should overwrite on subsequent query changes', () => { + const abortController = new AbortController(); + const onTextLangQuerySubmit = jest.fn(); + const { result, rerender } = renderHook((props) => useEditorQuerySync(props), { + initialProps: { + ...defaultProps, + initialQueryEsql: 'FROM test | LIMIT 10', + isLoading: true, + editorIsInline: false, + isEditorMounted: true, + currentAbortController: abortController, + onTextLangQuerySubmit, + }, + }); + + // Simulate user typing in the editor + act(() => { + result.current.handleQueryUpdate('FROM test | LIMIT 100'); + }); + expect(result.current.code).toBe('FROM test | LIMIT 100'); + + // Simulate loading finishes and query prop changes + rerender({ + ...defaultProps, + isLoading: false, + initialQueryEsql: 'FROM test | LIMIT 10', + isEditorMounted: true, + onTextLangQuerySubmit, + }); + // The code should still show the user change, not the new query + expect(result.current.code).toBe('FROM test | LIMIT 100'); + + // Now, simulate another query prop change (should overwrite) + rerender({ + ...defaultProps, + isLoading: false, + initialQueryEsql: 'FROM test | LIMIT 10', + isEditorMounted: true, + onTextLangQuerySubmit, + }); + // The code should now be overwritten by the new query + act(() => { + result.current.handleQueryUpdate('FROM test | LIMIT 10'); + }); + expect(result.current.code).toBe('FROM test | LIMIT 10'); + + // Now, simulate another query prop change (should overwrite) + rerender({ + ...defaultProps, + isLoading: false, + initialQueryEsql: 'FROM test | LIMIT 1000', + isEditorMounted: true, + onTextLangQuerySubmit, + }); + + expect(result.current.code).toBe('FROM test | LIMIT 1000'); + + act(() => { + result.current.handleQuerySubmit(); + }); + + // Assert: onTextLangQuerySubmit should be called with the current code + expect(onTextLangQuerySubmit).toHaveBeenCalledWith( + { esql: 'FROM test | LIMIT 1000' }, + abortController + ); + }); + it('should overwrite editor when external query changes during loading', () => { + const abortController = new AbortController(); + const onTextLangQuerySubmit = jest.fn(); + const { result, rerender } = renderHook((props) => useEditorQuerySync(props), { + initialProps: { + ...defaultProps, + initialQueryEsql: 'FROM test | LIMIT 1', + isLoading: true, + editorIsInline: false, + isEditorMounted: true, + currentAbortController: abortController, + onTextLangQuerySubmit, + }, + }); + + act(() => { + result.current.handleQueryUpdate('FROM test | LIMIT 10'); + }); + + expect(result.current.code).toBe('FROM test | LIMIT 10'); + + rerender({ + ...defaultProps, + isLoading: true, + initialQueryEsql: 'FROM test | LIMIT 10', + isEditorMounted: true, + onTextLangQuerySubmit, + }); + expect(result.current.code).toBe('FROM test | LIMIT 10'); + + rerender({ + ...defaultProps, + isLoading: true, + initialQueryEsql: 'FROM test | LIMIT 1000', + isEditorMounted: true, + onTextLangQuerySubmit, + }); + expect(result.current.code).toBe('FROM test | LIMIT 1000'); + }); +}); diff --git a/src/platform/packages/private/kbn-esql-editor/src/useEditorQuerySync.ts b/src/platform/packages/private/kbn-esql-editor/src/useEditorQuerySync.ts new file mode 100644 index 0000000000000..52eaf3be5ea77 --- /dev/null +++ b/src/platform/packages/private/kbn-esql-editor/src/useEditorQuerySync.ts @@ -0,0 +1,134 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { useEffect, useState, Dispatch, SetStateAction, useCallback, useMemo } from 'react'; // Remove RefObject +import { AggregateQuery } from '@kbn/es-query'; // Added import +import { fixESQLQueryWithVariables } from '@kbn/esql-utils'; +import { type ESQLControlVariable } from '@kbn/esql-types'; + +export interface UseEditorQuerySyncArgs { + isLoading: boolean; + initialQueryEsql: string; + esqlVariables?: ESQLControlVariable[]; + editorIsInline: boolean; + isEditorMounted: boolean; + allowQueryCancellation?: boolean; + onTextLangQuerySubmit: (query: AggregateQuery, controller: AbortController) => void; + currentAbortController: AbortController; + setNewAbortController: Dispatch>; + onTextLangQueryChange: (query: AggregateQuery) => void; +} + +export const useEditorQuerySync = ({ + isLoading, + initialQueryEsql, + esqlVariables, + editorIsInline, + isEditorMounted, // Add this line + allowQueryCancellation, + onTextLangQuerySubmit, + currentAbortController, + setNewAbortController, + onTextLangQueryChange, +}: UseEditorQuerySyncArgs) => { + const fixedQuery = useMemo( + () => fixESQLQueryWithVariables(initialQueryEsql, esqlVariables), + [initialQueryEsql, esqlVariables] + ); + + const [query, setQuery] = useState({ + external: fixedQuery, + edited: '', + submitted: '', + isLoading, + isLoadingExternal: isLoading, + }); + + useEffect(() => { + if (isEditorMounted) { + setQuery((prevState) => { + const externalQueryChanged = fixedQuery !== prevState.external; + const externalIsLoadingChanged = isLoading !== prevState.isLoadingExternal; + const queryChanged = fixedQuery !== prevState.edited; + + const nextQuery = { + ...prevState, + external: fixedQuery, + isLoadingExternal: isLoading, + edited: prevState.edited === fixedQuery ? '' : prevState.edited, + }; + + if (externalIsLoadingChanged) { + nextQuery.isLoading = isLoading; + } + + if ( + queryChanged && + (editorIsInline || + nextQuery.edited === '' || + (externalQueryChanged && !prevState.isLoading)) + ) { + nextQuery.edited = ''; + } + + return nextQuery; + }); + } + }, [fixedQuery, editorIsInline, isEditorMounted, isLoading]); + + const handleQuerySubmit = useCallback(() => { + if (query.isLoading && isLoading && allowQueryCancellation) { + currentAbortController?.abort(); + setQuery((prevState) => ({ + ...prevState, + isLoading: false, + })); + } else { + const abc = new AbortController(); + setNewAbortController(abc); + setQuery((prevState) => ({ + ...prevState, + submitted: query.edited || fixedQuery, + isLoading: true, + })); + onTextLangQuerySubmit({ esql: query.edited || fixedQuery } as AggregateQuery, abc); + } + }, [ + currentAbortController, + allowQueryCancellation, + isLoading, + query.edited, + query.isLoading, + onTextLangQuerySubmit, + setNewAbortController, + fixedQuery, + ]); + + const handleQueryUpdate = useCallback( + (value: string) => { + if (!editorIsInline) { + // allows to apply changes to the code when the query is running + // preventing a race condition in the inline editor mode, when this is not necessary + // setCode(value); + setQuery((prevState) => ({ ...prevState, edited: value === fixedQuery ? '' : value })); + } + onTextLangQueryChange({ esql: value } as AggregateQuery); + }, + [onTextLangQueryChange, setQuery, editorIsInline, fixedQuery] + ); + + return { + handleQuerySubmit, + handleQueryUpdate, + isQueryLoading: query.isLoading, + code: query.edited || fixedQuery, + codeWhenSubmitted: query.submitted || fixedQuery, + fixedQuery, + }; +};