From d82a279bd487e66915dbb967fe4976ebac35cf88 Mon Sep 17 00:00:00 2001 From: Ashwin P Chandran Date: Mon, 4 Nov 2024 02:20:31 +0000 Subject: [PATCH 1/4] mosty working --- .../public/ui/query_editor/query_editor.tsx | 110 +++++++++++------- 1 file changed, 66 insertions(+), 44 deletions(-) diff --git a/src/plugins/data/public/ui/query_editor/query_editor.tsx b/src/plugins/data/public/ui/query_editor/query_editor.tsx index dd6058f8c61a..7ca0ce45fda9 100644 --- a/src/plugins/data/public/ui/query_editor/query_editor.tsx +++ b/src/plugins/data/public/ui/query_editor/query_editor.tsx @@ -25,6 +25,7 @@ import { RecentQueriesTable, QueryResult, QueryStatus, + useQueryStringManager, } from '../..'; import { OpenSearchDashboardsReactContextValue } from '../../../../opensearch_dashboards_react/public'; import { fromUser, getQueryLog, PersistedLog, toUser } from '../../query'; @@ -74,15 +75,22 @@ export const QueryEditorUI: React.FC = (props) => { const headerRef = useRef(null); const bannerRef = useRef(null); const queryControlsContainer = useRef(null); + // TODO: There should only be one source of truth for query state and it should match the app query state. Firing the query should be restricted to when the user is ready, but should not prevent the app query from changing. + const editorQuery = props.query; // local query state managed by the editor. Not to be confused by the app query state. const queryString = getQueryService().queryString; const languageManager = queryString.getLanguageService(); const extensionMap = languageManager.getQueryEditorExtensionMap(); const services = props.opensearchDashboards.services; + const { query } = useQueryStringManager({ + queryString, + }); + + // console.log('QueryEditorUI', query); const persistedLogRef = useRef( props.persistedLog || - getQueryLog(services.uiSettings, services.storage, services.appName, props.query.language) + getQueryLog(services.uiSettings, services.storage, services.appName, query.language) ); const abortControllerRef = useRef(); @@ -95,17 +103,13 @@ export const QueryEditorUI: React.FC = (props) => { }; }, []); - const getQueryString = useCallback(() => { - return toUser(props.query.query); - }, [props.query]); - const renderQueryEditorExtensions = () => { if ( !( headerRef.current && bannerRef.current && queryControlsContainer.current && - props.query.language && + query.language && extensionMap && Object.keys(extensionMap).length > 0 ) @@ -114,7 +118,7 @@ export const QueryEditorUI: React.FC = (props) => { } return ( = (props) => { ); }; - const onSubmit = (query: Query, dateRange?: TimeRange) => { + const onSubmit = (currentQuery: Query, dateRange?: TimeRange) => { if (props.onSubmit) { if (persistedLogRef.current) { - persistedLogRef.current.add(query.query); + persistedLogRef.current.add(currentQuery.query); } props.onSubmit( { - query: fromUser(query.query), - language: query.language, - dataset: query.dataset, + ...currentQuery, + query: fromUser(currentQuery.query), }, dateRange ); } }; - const onChange = (query: Query, dateRange?: TimeRange) => { + const onChange = (currentQuery: Query, dateRange?: TimeRange) => { if (props.onChange) { props.onChange( - { query: fromUser(query.query), language: query.language, dataset: query.dataset }, + { + ...currentQuery, + query: fromUser(currentQuery.query), + }, dateRange ); } @@ -155,13 +161,14 @@ export const QueryEditorUI: React.FC = (props) => { const onQueryStringChange = (value: string) => { onChange({ query: value, - language: props.query.language, - dataset: props.query.dataset, + // Its fiune to reference the query state her since it is not updated yet + language: query.language, + dataset: query.dataset, }); }; - const onClickRecentQuery = (query: Query, timeRange?: TimeRange) => { - onSubmit(query, timeRange); + const onClickRecentQuery = (currentQuery: Query, timeRange?: TimeRange) => { + onSubmit(currentQuery, timeRange); }; const onInputChange = (value: string) => { @@ -175,13 +182,6 @@ export const QueryEditorUI: React.FC = (props) => { }; const onSelectLanguage = (languageId: string) => { - // Send telemetry info every time the user opts in or out of kuery - // As a result it is important this function only ever gets called in the - // UI component's change handler. - services.http.post('/api/opensearch-dashboards/dql_opt_in_stats', { - body: JSON.stringify({ opt_in: languageId === 'kuery' }), - }); - const newQuery = queryString.getInitialQueryByLanguage(languageId); onChange(newQuery); @@ -223,10 +223,10 @@ export const QueryEditorUI: React.FC = (props) => { ): Promise => { const indexPattern = await fetchIndexPattern(); const suggestions = await services.data.autocomplete.getQuerySuggestions({ - query: getQueryString(), + query: inputRef.current?.getValue() ?? '', selectionStart: model.getOffsetAt(position), selectionEnd: model.getOffsetAt(position), - language: props.query.language, + language: query.language, indexPattern, position, services, @@ -263,7 +263,7 @@ export const QueryEditorUI: React.FC = (props) => { }; }; - const useQueryEditor = props.query.language !== 'kuery' && props.query.language !== 'lucene'; + const useQueryEditor = query.language !== 'kuery' && query.language !== 'lucene'; const languageSelector = ( = (props) => { ); const baseInputProps = { - languageId: props.query.language, - value: getQueryString(), + languageId: query.language, + value: toUser(editorQuery.query), }; const defaultInputProps: DefaultInputProps = { @@ -287,7 +287,13 @@ export const QueryEditorUI: React.FC = (props) => { inputRef.current = editor; // eslint-disable-next-line no-bitwise editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.Enter, () => { - onSubmit(props.query); + // debugger + const newQuery = { + ...query, + query: editor.getValue(), + }; + + onSubmit(newQuery); }); return () => { @@ -305,7 +311,7 @@ export const QueryEditorUI: React.FC = (props) => { data-test-subj="queryEditorFooterTimestamp" className="queryEditor__footerItem" > - {props.query.dataset?.timeFieldName || ''} + {query.dataset?.timeFieldName || ''} , , ], @@ -336,21 +342,37 @@ export const QueryEditorUI: React.FC = (props) => { editorDidMount: (editor: monaco.editor.IStandaloneCodeEditor) => { inputRef.current = editor; - const handleEnterPress = () => { - onSubmit(props.query); - }; + editor.addCommand(monaco.KeyCode.Enter, () => { + const newQuery = { + ...query, + query: editor.getValue(), + }; - const disposable = editor.onKeyDown((e) => { - if (e.keyCode === monaco.KeyCode.Enter) { - // Prevent default Enter key behavior - e.preventDefault(); - handleEnterPress(); - } + onSubmit(newQuery); }); + // const handleEnterPress = () => { + // const newQuery = { + // ...props.query, + // query: editor.getValue(), + // }; + + // debugger + + // onSubmit(newQuery); + // }; + + // const disposable = editor.onKeyDown((e) => { + // if (e.keyCode === monaco.KeyCode.Enter) { + // // Prevent default Enter key behavior + // e.preventDefault(); + // handleEnterPress(); + // } + // }); + // Optional: Cleanup on component unmount return () => { - disposable.dispose(); + // disposable.dispose(); }; }, provideCompletionItems, @@ -361,7 +383,7 @@ export const QueryEditorUI: React.FC = (props) => { {`${lineCount ?? 1} ${lineCount === 1 || !lineCount ? 'line' : 'lines'}`} , - {props.query.dataset?.timeFieldName || ''} + {query.dataset?.timeFieldName || ''} , , ], @@ -383,7 +405,7 @@ export const QueryEditorUI: React.FC = (props) => { }, }; - const languageEditorFunc = languageManager.getLanguage(props.query.language)!.editor; + const languageEditorFunc = languageManager.getLanguage(query.language)!.editor; const languageEditor = useQueryEditor ? languageEditorFunc(singleLineInputProps, {}, defaultInputProps) From fa93a42ff474c8effe5140e2b6c97e179011d5cb Mon Sep 17 00:00:00 2001 From: Ashwin P Chandran Date: Mon, 4 Nov 2024 02:47:20 +0000 Subject: [PATCH 2/4] fix reference issue --- .../public/ui/query_editor/query_editor.tsx | 35 ++++--------------- 1 file changed, 7 insertions(+), 28 deletions(-) diff --git a/src/plugins/data/public/ui/query_editor/query_editor.tsx b/src/plugins/data/public/ui/query_editor/query_editor.tsx index 7ca0ce45fda9..02f83901a667 100644 --- a/src/plugins/data/public/ui/query_editor/query_editor.tsx +++ b/src/plugins/data/public/ui/query_editor/query_editor.tsx @@ -85,8 +85,12 @@ export const QueryEditorUI: React.FC = (props) => { const { query } = useQueryStringManager({ queryString, }); + const queryRef = useRef(query); - // console.log('QueryEditorUI', query); + // Monaco editor commands dont reference the query state directly since the commands are registered at startup, so we need to keep a ref to the query state that it can use when needed + useEffect(() => { + queryRef.current = query; + }, [query]); const persistedLogRef = useRef( props.persistedLog || @@ -226,7 +230,7 @@ export const QueryEditorUI: React.FC = (props) => { query: inputRef.current?.getValue() ?? '', selectionStart: model.getOffsetAt(position), selectionEnd: model.getOffsetAt(position), - language: query.language, + language: queryRef.current.language, indexPattern, position, services, @@ -287,9 +291,8 @@ export const QueryEditorUI: React.FC = (props) => { inputRef.current = editor; // eslint-disable-next-line no-bitwise editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.Enter, () => { - // debugger const newQuery = { - ...query, + ...queryRef.current, query: editor.getValue(), }; @@ -350,30 +353,6 @@ export const QueryEditorUI: React.FC = (props) => { onSubmit(newQuery); }); - - // const handleEnterPress = () => { - // const newQuery = { - // ...props.query, - // query: editor.getValue(), - // }; - - // debugger - - // onSubmit(newQuery); - // }; - - // const disposable = editor.onKeyDown((e) => { - // if (e.keyCode === monaco.KeyCode.Enter) { - // // Prevent default Enter key behavior - // e.preventDefault(); - // handleEnterPress(); - // } - // }); - - // Optional: Cleanup on component unmount - return () => { - // disposable.dispose(); - }; }, provideCompletionItems, prepend: props.prepend, From d5de43f12bbd0935101435717d757d5ecd7dd676 Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Mon, 4 Nov 2024 03:13:28 +0000 Subject: [PATCH 3/4] Changeset file for PR #8803 created/updated --- changelogs/fragments/8803.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/8803.yml diff --git a/changelogs/fragments/8803.yml b/changelogs/fragments/8803.yml new file mode 100644 index 000000000000..0cf82dc68ddf --- /dev/null +++ b/changelogs/fragments/8803.yml @@ -0,0 +1,2 @@ +fix: +- Query Editor state sync issues in Discover ([#8803](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8803)) \ No newline at end of file From b8968698786b45758c8af6896346985278cca939 Mon Sep 17 00:00:00 2001 From: Ashwin P Chandran Date: Sun, 3 Nov 2024 19:38:10 -0800 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Kawika Avilla Signed-off-by: Ashwin P Chandran --- src/plugins/data/public/ui/query_editor/query_editor.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/plugins/data/public/ui/query_editor/query_editor.tsx b/src/plugins/data/public/ui/query_editor/query_editor.tsx index 02f83901a667..2223b577b513 100644 --- a/src/plugins/data/public/ui/query_editor/query_editor.tsx +++ b/src/plugins/data/public/ui/query_editor/query_editor.tsx @@ -75,7 +75,7 @@ export const QueryEditorUI: React.FC = (props) => { const headerRef = useRef(null); const bannerRef = useRef(null); const queryControlsContainer = useRef(null); - // TODO: There should only be one source of truth for query state and it should match the app query state. Firing the query should be restricted to when the user is ready, but should not prevent the app query from changing. + // TODO: https://github.com/opensearch-project/OpenSearch-Dashboards/issues/8801 const editorQuery = props.query; // local query state managed by the editor. Not to be confused by the app query state. const queryString = getQueryService().queryString; @@ -87,7 +87,7 @@ export const QueryEditorUI: React.FC = (props) => { }); const queryRef = useRef(query); - // Monaco editor commands dont reference the query state directly since the commands are registered at startup, so we need to keep a ref to the query state that it can use when needed + // Monaco commands are registered once at startup, we need a ref to access the latest query state inside command callbacks useEffect(() => { queryRef.current = query; }, [query]); @@ -165,7 +165,6 @@ export const QueryEditorUI: React.FC = (props) => { const onQueryStringChange = (value: string) => { onChange({ query: value, - // Its fiune to reference the query state her since it is not updated yet language: query.language, dataset: query.dataset, });