[Discover] Query editor state sync fixes (opensearch-project#8803)

* mosty working * fix reference issue * Changeset file for PR opensearch-project#8803 created/updated * Apply suggestions from code review Co-authored-by: Kawika Avilla <[email protected]> Signed-off-by: Ashwin P Chandran <[email protected]> --------- Signed-off-by: Ashwin P Chandran <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Kawika Avilla <[email protected]>
raintygao · Nov 4, 2024 · 358d823 · 358d823
1 parent 005a83a
commit 358d823
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 48 deletions.
diff --git a/changelogs/fragments/8803.yml 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))
diff --git 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,26 @@ export const QueryEditorUI: React.FC<Props> = (props) => {
   const headerRef = useRef<HTMLDivElement>(null);
   const bannerRef = useRef<HTMLDivElement>(null);
   const queryControlsContainer = useRef<HTMLDivElement>(null);
+  // 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;
   const languageManager = queryString.getLanguageService();
   const extensionMap = languageManager.getQueryEditorExtensionMap();
   const services = props.opensearchDashboards.services;
+  const { query } = useQueryStringManager({
+    queryString,
+  });
+  const queryRef = useRef(query);
+
+  // 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]);
 
   const persistedLogRef = useRef<PersistedLog>(
     props.persistedLog ||
-      getQueryLog(services.uiSettings, services.storage, services.appName, props.query.language)
+      getQueryLog(services.uiSettings, services.storage, services.appName, query.language)
   );
   const abortControllerRef = useRef<AbortController>();
 
@@ -95,17 +107,13 @@ export const QueryEditorUI: React.FC<Props> = (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 +122,7 @@ export const QueryEditorUI: React.FC<Props> = (props) => {
     }
     return (
       <QueryEditorExtensions
-        language={props.query.language}
+        language={query.language}
         onSelectLanguage={onSelectLanguage}
         isCollapsed={isCollapsed}
         setIsCollapsed={setIsCollapsed}
@@ -126,27 +134,29 @@ export const QueryEditorUI: React.FC<Props> = (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 +165,13 @@ export const QueryEditorUI: React.FC<Props> = (props) => {
   const onQueryStringChange = (value: string) => {
     onChange({
       query: value,
-      language: props.query.language,
-      dataset: props.query.dataset,
+      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 +185,6 @@ export const QueryEditorUI: React.FC<Props> = (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 +226,10 @@ export const QueryEditorUI: React.FC<Props> = (props) => {
   ): Promise<monaco.languages.CompletionList> => {
     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: queryRef.current.language,
       indexPattern,
       position,
       services,
@@ -263,7 +266,7 @@ export const QueryEditorUI: React.FC<Props> = (props) => {
     };
   };
 
-  const useQueryEditor = props.query.language !== 'kuery' && props.query.language !== 'lucene';
+  const useQueryEditor = query.language !== 'kuery' && query.language !== 'lucene';
 
   const languageSelector = (
     <QueryLanguageSelector
@@ -274,8 +277,8 @@ export const QueryEditorUI: React.FC<Props> = (props) => {
   );
 
   const baseInputProps = {
-    languageId: props.query.language,
-    value: getQueryString(),
+    languageId: query.language,
+    value: toUser(editorQuery.query),
   };
 
   const defaultInputProps: DefaultInputProps = {
@@ -287,7 +290,12 @@ export const QueryEditorUI: React.FC<Props> = (props) => {
       inputRef.current = editor;
       // eslint-disable-next-line no-bitwise
       editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.Enter, () => {
-        onSubmit(props.query);
+        const newQuery = {
+          ...queryRef.current,
+          query: editor.getValue(),
+        };
+
+        onSubmit(newQuery);
       });
 
       return () => {
@@ -305,7 +313,7 @@ export const QueryEditorUI: React.FC<Props> = (props) => {
           data-test-subj="queryEditorFooterTimestamp"
           className="queryEditor__footerItem"
         >
-          {props.query.dataset?.timeFieldName || ''}
+          {query.dataset?.timeFieldName || ''}
         </EuiText>,
         <QueryResult queryStatus={props.queryStatus!} />,
       ],
@@ -336,22 +344,14 @@ export const QueryEditorUI: React.FC<Props> = (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);
       });
-
-      // Optional: Cleanup on component unmount
-      return () => {
-        disposable.dispose();
-      };
     },
     provideCompletionItems,
     prepend: props.prepend,
@@ -361,7 +361,7 @@ export const QueryEditorUI: React.FC<Props> = (props) => {
           {`${lineCount ?? 1} ${lineCount === 1 || !lineCount ? 'line' : 'lines'}`}
         </EuiText>,
         <EuiText size="xs" color="subdued" className="queryEditor__footerItem">
-          {props.query.dataset?.timeFieldName || ''}
+          {query.dataset?.timeFieldName || ''}
         </EuiText>,
         <QueryResult queryStatus={props.queryStatus!} />,
       ],
@@ -383,7 +383,7 @@ export const QueryEditorUI: React.FC<Props> = (props) => {
     },
   };
 
-  const languageEditorFunc = languageManager.getLanguage(props.query.language)!.editor;
+  const languageEditorFunc = languageManager.getLanguage(query.language)!.editor;
 
   const languageEditor = useQueryEditor
     ? languageEditorFunc(singleLineInputProps, {}, defaultInputProps)
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		fix:
		- Query Editor state sync issues in Discover ([#8803](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8803))