[bug] address different issues with dataset selector (#8665)

* [bug] address some issues with dataset selector Moved the dataset selector back to the search bar and access it by ref in the sidebar. Avoid out of sync issue. Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * update the logic for ensuring index pattern Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * dont add ability to open button Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * empty state but missing data set selector button Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * fix tests Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * add back styling Signed-off-by: Kawika Avilla <kavilla414@gmail.com> --------- Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
opensearch-project · Oct 19, 2024 · e23f332 · e23f332
1 parent c8b5318
commit e23f332
Show file tree

Hide file tree

Showing 29 changed files with 286 additions and 553 deletions.
diff --git a/src/plugins/data/common/index_patterns/index_patterns/ensure_default_index_pattern.ts b/src/plugins/data/common/index_patterns/index_patterns/ensure_default_index_pattern.ts
@@ -32,9 +32,7 @@ import { includes } from 'lodash';
 import { IndexPatternsContract } from './index_patterns';
 import { UiSettingsCommon } from '../types';
 
-export type EnsureDefaultIndexPattern = (
-  shouldRedirect?: boolean
-) => Promise<unknown | void> | undefined;
+export type EnsureDefaultIndexPattern = () => Promise<unknown | void> | undefined;
 
 export const createEnsureDefaultIndexPattern = (
   uiSettings: UiSettingsCommon,
@@ -44,10 +42,7 @@ export const createEnsureDefaultIndexPattern = (
    * Checks whether a default index pattern is set and exists and defines
    * one otherwise.
    */
-  return async function ensureDefaultIndexPattern(
-    this: IndexPatternsContract,
-    shouldRedirect: boolean = true
-  ) {
+  return async function ensureDefaultIndexPattern(this: IndexPatternsContract) {
     const patterns = await this.getIds();
     let defaultId = await uiSettings.get('defaultIndex');
     let defined = !!defaultId;
@@ -67,6 +62,8 @@ export const createEnsureDefaultIndexPattern = (
       defaultId = patterns[0];
       await uiSettings.set('defaultIndex', defaultId);
     } else {
+      const isEnhancementsEnabled = await uiSettings.get('query:enhancements:enabled');
+      const shouldRedirect = !isEnhancementsEnabled;
       if (shouldRedirect) return onRedirectNoIndexPattern();
       else return;
     }

diff --git a/src/plugins/data/public/query/query_string/language_service/lib/dql_language.ts b/src/plugins/data/public/query/query_string/language_service/lib/dql_language.ts
@@ -24,6 +24,12 @@ export const getDQLLanguageConfig = (
       visualizable: true,
     },
     showDocLinks: true,
+    docLink: {
+      title: i18n.translate('data.dqlLanguage.docLink', {
+        defaultMessage: 'DQL documentation',
+      }),
+      url: 'https://opensearch.org/docs/latest/query-dsl/full-text/query-string/',
+    },
     editorSupportedAppNames: ['discover'],
     supportedAppNames: ['discover', 'dashboards', 'visualize', 'data-explorer', 'vis-builder', '*'],
     sampleQueries: [

diff --git a/src/plugins/data/public/query/query_string/language_service/lib/lucene_language.ts b/src/plugins/data/public/query/query_string/language_service/lib/lucene_language.ts
@@ -24,6 +24,12 @@ export const getLuceneLanguageConfig = (
       visualizable: true,
     },
     showDocLinks: true,
+    docLink: {
+      title: i18n.translate('data.luceneLanguage.docLink', {
+        defaultMessage: 'Lucene documentation',
+      }),
+      url: 'https://opensearch.org/docs/latest/query-dsl/full-text/query-string/',
+    },
     editorSupportedAppNames: ['discover'],
     supportedAppNames: ['discover', 'dashboards', 'visualize', 'data-explorer', 'vis-builder', '*'],
     sampleQueries: [

diff --git a/src/plugins/data/public/query/query_string/language_service/types.ts b/src/plugins/data/public/query/query_string/language_service/types.ts
@@ -55,6 +55,10 @@ export interface LanguageConfig {
     visualizable?: boolean;
   };
   showDocLinks?: boolean;
+  docLink?: {
+    title: string;
+    url: string;
+  };
   editorSupportedAppNames?: string[];
   supportedAppNames?: string[];
   hideDatePicker?: boolean;

diff --git a/src/plugins/data/public/ui/_index.scss b/src/plugins/data/public/ui/_index.scss
@@ -1,4 +1,3 @@
-@import "./common";
 @import "./filter_bar/index";
 @import "./typeahead/index";
 @import "./saved_query_management/index";

diff --git a/src/plugins/data/public/ui/dataset_selector/advanced_selector.tsx b/src/plugins/data/public/ui/dataset_selector/advanced_selector.tsx
@@ -13,27 +13,19 @@ import {
 } from '../../../common';
 import { DatasetExplorer } from './dataset_explorer';
 import { Configurator } from './configurator';
+import { getQueryService } from '../../services';
 import { IDataPluginServices } from '../../types';
 
 export const AdvancedSelector = ({
   services,
   onSelect,
   onCancel,
-  selectedDataset,
-  setSelectedDataset,
-  setIndexPattern,
-  direct = false,
 }: {
   services: IDataPluginServices;
   onSelect: (dataset: Dataset) => void;
   onCancel: () => void;
-  selectedDataset?: Dataset;
-  setSelectedDataset: (data: Dataset | undefined) => void;
-  setIndexPattern: (id: string | undefined) => void;
-  direct?: boolean;
 }) => {
-  const queryService = services.data.query;
-  const queryString = queryService.queryString;
+  const queryString = getQueryService().queryString;
 
   const [path, setPath] = useState<DataStructure[]>([
     {
@@ -56,38 +48,22 @@ export const AdvancedSelector = ({
         }),
     },
   ]);
+  const [selectedDataset, setSelectedDataset] = useState<BaseDataset | undefined>();
 
-  const [currentSelectedDataset, setCurrentSelectedDataset] = useState<BaseDataset | undefined>(
-    selectedDataset
-  );
-
-  return currentSelectedDataset ? (
+  return selectedDataset ? (
     <Configurator
-      baseDataset={currentSelectedDataset}
+      baseDataset={selectedDataset}
       onConfirm={onSelect}
       onCancel={onCancel}
-      onPrevious={() => {
-        setSelectedDataset(undefined);
-        setCurrentSelectedDataset(undefined);
-      }}
-      queryService={queryService}
+      onPrevious={() => setSelectedDataset(undefined)}
     />
   ) : (
     <DatasetExplorer
       services={services}
       queryString={queryString}
       path={path}
       setPath={setPath}
-      onNext={(dataset) => {
-        setSelectedDataset(dataset);
-        setIndexPattern(dataset.id);
-        setCurrentSelectedDataset(dataset);
-        if (direct) {
-          const query = queryString.getInitialQueryByDataset(dataset);
-          queryString.setQuery(query);
-          queryString.getDatasetService().addRecentDataset(dataset);
-        }
-      }}
+      onNext={(dataset) => setSelectedDataset(dataset)}
       onCancel={onCancel}
     />
   );

diff --git a/src/plugins/data/public/ui/dataset_selector/configurator.tsx b/src/plugins/data/public/ui/dataset_selector/configurator.tsx
@@ -20,21 +20,20 @@ import { i18n } from '@osd/i18n';
 import { FormattedMessage } from '@osd/i18n/react';
 import React, { useEffect, useMemo, useState } from 'react';
 import { BaseDataset, DEFAULT_DATA, Dataset, DatasetField } from '../../../common';
-import { getIndexPatterns } from '../../services';
+import { getIndexPatterns, getQueryService } from '../../services';
 
 export const Configurator = ({
   baseDataset,
   onConfirm,
   onCancel,
   onPrevious,
-  queryService,
 }: {
   baseDataset: BaseDataset;
   onConfirm: (dataset: Dataset) => void;
   onCancel: () => void;
   onPrevious: () => void;
-  queryService: any;
 }) => {
+  const queryService = getQueryService();
   const queryString = queryService.queryString;
   const languageService = queryService.queryString.getLanguageService();
   const indexPatternsService = getIndexPatterns();

diff --git a/src/plugins/data/public/ui/dataset_selector/dataset_selector.tsx b/src/plugins/data/public/ui/dataset_selector/dataset_selector.tsx
@@ -33,9 +33,7 @@ type EuiSmallButtonEmptyProps = React.ComponentProps<typeof EuiSmallButtonEmpty>
 
 interface DatasetSelectorProps {
   selectedDataset?: Dataset;
-  setSelectedDataset: (data: Dataset | undefined) => void;
-  setIndexPattern: (id: string | undefined) => void;
-  handleDatasetChange: (dataset: Dataset) => void;
+  setSelectedDataset: (dataset: Dataset) => void;
   services: IDataPluginServices;
 }
 
@@ -73,8 +71,6 @@ const RootComponent: React.FC<
 export const DatasetSelector = ({
   selectedDataset,
   setSelectedDataset,
-  setIndexPattern,
-  handleDatasetChange,
   services,
   appearance,
   buttonProps,
@@ -106,7 +102,7 @@ export const DatasetSelector = ({
 
       // If no dataset is selected, select the first one
       if (!selectedDataset && fetchedDatasets.length > 0) {
-        handleDatasetChange(fetchedDatasets[0]);
+        setSelectedDataset(fetchedDatasets[0]);
       }
     };
 
@@ -183,11 +179,11 @@ export const DatasetSelector = ({
           indexPatterns.find((dataset) => dataset.id === selectedOption.key);
         if (foundDataset) {
           closePopover();
-          handleDatasetChange(foundDataset);
+          setSelectedDataset(foundDataset);
         }
       }
     },
-    [recentDatasets, indexPatterns, handleDatasetChange, closePopover]
+    [recentDatasets, indexPatterns, setSelectedDataset, closePopover]
   );
 
   const datasetTitle = useMemo(() => {
@@ -270,14 +266,10 @@ export const DatasetSelector = ({
                   onSelect={(dataset?: Dataset) => {
                     overlay?.close();
                     if (dataset) {
-                      handleDatasetChange(dataset);
+                      setSelectedDataset(dataset);
                     }
                   }}
                   onCancel={() => overlay?.close()}
-                  selectedDataset={undefined}
-                  setSelectedDataset={setSelectedDataset}
-                  setIndexPattern={setIndexPattern}
-                  direct={true}
                 />
               ),
               {

diff --git a/src/plugins/data/public/ui/dataset_selector/index.test.tsx b/src/plugins/data/public/ui/dataset_selector/index.test.tsx
@@ -5,7 +5,6 @@
 
 import React from 'react';
 import { mount } from 'enzyme';
-import { act } from 'react-dom/test-utils';
 import { DatasetSelector as ConnectedDatasetSelector } from './index';
 import { DatasetSelector } from './dataset_selector';
 import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public';
@@ -38,94 +37,44 @@ describe('ConnectedDatasetSelector', () => {
   const mockServices = {
     data: {
       query: {
+        // @ts-ignore
         queryString: mockQueryString,
       },
     },
   };
 
   beforeEach(() => {
     (useOpenSearchDashboards as jest.Mock).mockReturnValue({ services: mockServices });
-    jest.clearAllMocks();
   });
 
   it('should render DatasetSelector with correct props', () => {
-    const wrapper = mount(
-      <ConnectedDatasetSelector
-        onSubmit={mockOnSubmit}
-        selectedDataset={undefined}
-        setSelectedDataset={jest.fn()}
-        setIndexPattern={jest.fn()}
-        services={mockServices}
-      />
-    );
+    const wrapper = mount(<ConnectedDatasetSelector onSubmit={mockOnSubmit} />);
     expect(wrapper.find(DatasetSelector).props()).toEqual({
       selectedDataset: undefined,
       setSelectedDataset: expect.any(Function),
-      setIndexPattern: expect.any(Function),
-      handleDatasetChange: expect.any(Function),
       services: mockServices,
     });
   });
 
   it('should initialize selectedDataset correctly', () => {
     const mockDataset: Dataset = { id: 'initial', title: 'Initial Dataset', type: 'test' };
+    mockQueryString.getQuery.mockReturnValueOnce({ dataset: mockDataset });
 
-    const wrapper = mount(
-      <ConnectedDatasetSelector
-        onSubmit={mockOnSubmit}
-        selectedDataset={mockDataset}
-        setSelectedDataset={jest.fn()}
-        setIndexPattern={jest.fn()}
-        services={mockServices}
-      />
-    );
+    const wrapper = mount(<ConnectedDatasetSelector onSubmit={mockOnSubmit} />);
     expect(wrapper.find(DatasetSelector).prop('selectedDataset')).toEqual(mockDataset);
   });
 
   it('should call handleDatasetChange only once when dataset changes', () => {
-    const setSelectedDataset = jest.fn();
-    const setIndexPattern = jest.fn();
-    const wrapper = mount(
-      <ConnectedDatasetSelector
-        onSubmit={mockOnSubmit}
-        selectedDataset={undefined}
-        setSelectedDataset={setSelectedDataset}
-        setIndexPattern={setIndexPattern}
-        services={mockServices}
-      />
-    );
-    const handleDatasetChange = wrapper.find(DatasetSelector).prop('handleDatasetChange') as (
+    const wrapper = mount(<ConnectedDatasetSelector onSubmit={mockOnSubmit} />);
+    const setSelectedDataset = wrapper.find(DatasetSelector).prop('setSelectedDataset') as (
       dataset?: Dataset
     ) => void;
 
     const newDataset: Dataset = { id: 'test', title: 'Test Dataset', type: 'test' };
-    act(() => {
-      handleDatasetChange(newDataset);
-    });
+    setSelectedDataset(newDataset);
 
     expect(mockQueryString.getInitialQueryByDataset).toHaveBeenCalledTimes(1);
     expect(mockQueryString.setQuery).toHaveBeenCalledTimes(1);
     expect(mockOnSubmit).toHaveBeenCalledTimes(1);
-    expect(setSelectedDataset).toHaveBeenCalledWith(newDataset);
-    expect(setIndexPattern).toHaveBeenCalledWith(newDataset.id);
-  });
-
-  it('should subscribe to queryString.getUpdates$ and unsubscribe on unmount', () => {
-    const wrapper = mount(
-      <ConnectedDatasetSelector
-        onSubmit={mockOnSubmit}
-        selectedDataset={undefined}
-        setSelectedDataset={jest.fn()}
-        setIndexPattern={jest.fn()}
-        services={mockServices}
-      />
-    );
-
-    expect(mockQueryString.getUpdates$).toHaveBeenCalledTimes(1);
-    expect(mockSubscribe).toHaveBeenCalledTimes(1);
-
-    wrapper.unmount();
-
-    expect(mockUnsubscribe).toHaveBeenCalledTimes(1);
   });
 });