[Discover] UI Updates to Dataset Configurator (#8166)

* adding UI changes Signed-off-by: Sean Li <[email protected]> * addressing comments Signed-off-by: Sean Li <[email protected]> * fixing more bugs, adding no time field prop for unsupported languages Signed-off-by: Sean Li <[email protected]> * addressing comments Signed-off-by: Sean Li <[email protected]> * addressing comments Signed-off-by: Sean Li <[email protected]> * Changeset file for PR #8166 created/updated * adressing comments, renaming label Signed-off-by: Sean Li <[email protected]> * addressing comments, adding tests Signed-off-by: Sean Li <[email protected]> * updating snapshots Signed-off-by: Sean Li <[email protected]> * adding i18n translation for no time filter option Signed-off-by: Sean Li <[email protected]> --------- Signed-off-by: Sean Li <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
opensearch-project · Oct 8, 2024 · 3ae4cdc · 3ae4cdc
1 parent acc947e
commit 3ae4cdc
Show file tree

Hide file tree

Showing 10 changed files with 123 additions and 38 deletions.
diff --git a/changelogs/fragments/8166.yml b/changelogs/fragments/8166.yml
@@ -0,0 +1,2 @@
+fix:
+- Fixes UI issues in Discover and data configurator ([#8166](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8166))
diff --git a/...ic/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap b/...ic/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap
diff --git a/src/plugins/data/public/mocks.ts b/src/plugins/data/public/mocks.ts
@@ -91,6 +91,12 @@ const createStartContract = (isEnhancementsEnabled: boolean = false): Start => {
         })
       ),
       clearCache: jest.fn(),
+      create: jest.fn().mockResolvedValue({
+        id: 'test-index-pattern',
+        title: 'Test Index Pattern',
+        type: 'INDEX_PATTERN',
+      }),
+      saveToCache: jest.fn(),
     } as unknown) as IndexPatternsContract,
     dataSources: dataSourceServiceMock.createStartContract(),
   };

diff --git a/src/plugins/data/public/query/query_string/dataset_service/dataset_service.test.ts b/src/plugins/data/public/query/query_string/dataset_service/dataset_service.test.ts
@@ -5,22 +5,28 @@
 
 import { DatasetService } from './dataset_service';
 import { coreMock } from '../../../../../../core/public/mocks';
-import { DataStorage } from 'src/plugins/data/common';
+import { DEFAULT_DATA, DataStorage, Dataset } from 'src/plugins/data/common';
 import { DataStructure } from '../../../../common';
 import { IDataPluginServices } from '../../../types';
+import { indexPatternTypeConfig } from './lib';
+import { dataPluginMock } from '../../../mocks';
+import { IndexPatternsContract } from '../../..';
 
 describe('DatasetService', () => {
   let service: DatasetService;
   let uiSettings: ReturnType<typeof coreMock.createSetup>['uiSettings'];
   let sessionStorage: DataStorage;
   let mockDataPluginServices: jest.Mocked<IDataPluginServices>;
+  let indexPatterns: IndexPatternsContract;
 
   beforeEach(() => {
     uiSettings = coreMock.createSetup().uiSettings;
     sessionStorage = new DataStorage(window.sessionStorage, 'opensearchDashboards.');
     mockDataPluginServices = {} as jest.Mocked<IDataPluginServices>;
 
     service = new DatasetService(uiSettings, sessionStorage);
+    indexPatterns = dataPluginMock.createStartContract().indexPatterns;
+    service.init(indexPatterns);
   });
 
   const mockResult = {
@@ -98,4 +104,29 @@ describe('DatasetService', () => {
 
     expect(service.getLastCacheTime()).toEqual(time);
   });
+  test('calling cacheDataset on dataset caches it', async () => {
+    const mockDataset = {
+      id: 'test-dataset',
+      title: 'Test Dataset',
+      type: mockType.id,
+    } as Dataset;
+    service.registerType(mockType);
+
+    await service.cacheDataset(mockDataset);
+    expect(indexPatterns.create).toHaveBeenCalledTimes(1);
+    expect(indexPatterns.saveToCache).toHaveBeenCalledTimes(1);
+  });
+
+  test('calling cacheDataset on index pattern does not cache it', async () => {
+    service.registerType(indexPatternTypeConfig);
+    const mockDataset = {
+      id: 'test-index-pattern',
+      title: 'Test Index Pattern',
+      type: DEFAULT_DATA.SET_TYPES.INDEX_PATTERN,
+    } as Dataset;
+
+    await service.cacheDataset(mockDataset);
+    expect(indexPatterns.create).toHaveBeenCalledTimes(0);
+    expect(indexPatterns.saveToCache).toHaveBeenCalledTimes(0);
+  });
 });
diff --git a/src/plugins/data/public/query/query_string/dataset_service/dataset_service.ts b/src/plugins/data/public/query/query_string/dataset_service/dataset_service.ts
@@ -9,7 +9,6 @@ import {
   DataStructure,
   IndexPatternSpec,
   DEFAULT_DATA,
-  IFieldType,
   UI_SETTINGS,
   DataStorage,
   CachedDataStructure,
@@ -64,14 +63,11 @@ export class DatasetService {
 
   public async cacheDataset(dataset: Dataset): Promise<void> {
     const type = this.getType(dataset.type);
-    if (dataset) {
+    if (dataset && dataset.type !== DEFAULT_DATA.SET_TYPES.INDEX_PATTERN) {
       const spec = {
         id: dataset.id,
         title: dataset.title,
-        timeFieldName: {
-          name: dataset.timeFieldName,
-          type: 'date',
-        } as Partial<IFieldType>,
+        timeFieldName: dataset.timeFieldName,
         fields: await type?.fetchFields(dataset),
         dataSourceRef: dataset.dataSource
           ? {

diff --git a/src/plugins/data/public/query/query_string/dataset_service/lib/index_type.ts b/src/plugins/data/public/query/query_string/dataset_service/lib/index_type.ts
@@ -80,6 +80,7 @@ export const indexTypeConfig: DatasetTypeConfig = {
     return fields.map((field: any) => ({
       name: field.name,
       type: field.type,
+      aggregatable: field?.aggregatable,
     }));
   },
 

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
@@ -52,4 +52,5 @@ export interface LanguageConfig {
   showDocLinks?: boolean;
   editorSupportedAppNames?: string[];
   supportedAppNames?: string[];
+  hideDatePicker?: boolean;
 }
diff --git a/src/plugins/data/public/ui/dataset_selector/configurator.tsx b/src/plugins/data/public/ui/dataset_selector/configurator.tsx
@@ -18,8 +18,8 @@ import {
 } from '@elastic/eui';
 import { i18n } from '@osd/i18n';
 import { FormattedMessage } from '@osd/i18n/react';
-import React, { useEffect, useState } from 'react';
-import { BaseDataset, Dataset, DatasetField } from '../../../common';
+import React, { useEffect, useMemo, useState } from 'react';
+import { BaseDataset, DEFAULT_DATA, Dataset, DatasetField } from '../../../common';
 import { getIndexPatterns, getQueryService } from '../../services';
 
 export const Configurator = ({
@@ -41,8 +41,14 @@ export const Configurator = ({
   const languages = type?.supportedLanguages(baseDataset) || [];
 
   const [dataset, setDataset] = useState<Dataset>(baseDataset);
-  const [timeFields, setTimeFields] = useState<DatasetField[]>();
+  const [timeFields, setTimeFields] = useState<DatasetField[]>([]);
   const [timeFieldName, setTimeFieldName] = useState<string | undefined>(dataset.timeFieldName);
+  const noTimeFilter = i18n.translate(
+    'data.explorer.datasetSelector.advancedSelector.configurator.timeField.noTimeFieldOptionLabel',
+    {
+      defaultMessage: "I don't want to use the time filter",
+    }
+  );
   const [language, setLanguage] = useState<string>(() => {
     const currentLanguage = queryString.getQuery().language;
     if (languages.includes(currentLanguage)) {
@@ -51,6 +57,18 @@ export const Configurator = ({
     return languages[0];
   });
 
+  const submitDisabled = useMemo(() => {
+    return (
+      timeFieldName === undefined &&
+      !(
+        languageService.getLanguage(language)?.hideDatePicker ||
+        dataset.type === DEFAULT_DATA.SET_TYPES.INDEX_PATTERN
+      ) &&
+      timeFields &&
+      timeFields.length > 0
+    );
+  }, [dataset, language, timeFieldName, timeFields, languageService]);
+
   useEffect(() => {
     const fetchFields = async () => {
       const datasetFields = await queryString
@@ -97,33 +115,6 @@ export const Configurator = ({
           >
             <EuiFieldText disabled value={dataset.title} />
           </EuiFormRow>
-          {timeFields && timeFields.length > 0 && (
-            <EuiFormRow
-              label={i18n.translate(
-                'data.explorer.datasetSelector.advancedSelector.configurator.timeFieldLabel',
-                {
-                  defaultMessage: 'Time field',
-                }
-              )}
-            >
-              <EuiSelect
-                options={[
-                  ...timeFields.map((field) => ({
-                    text: field.displayName || field.name,
-                    value: field.name,
-                  })),
-                  { text: '-----', value: '', disabled: true },
-                  { text: 'No time field', value: undefined },
-                ]}
-                value={timeFieldName}
-                onChange={(e) => {
-                  const value = e.target.value === 'undefined' ? undefined : e.target.value;
-                  setTimeFieldName(value);
-                  setDataset({ ...dataset, timeFieldName: value });
-                }}
-              />
-            </EuiFormRow>
-          )}
           <EuiFormRow
             label={i18n.translate(
               'data.explorer.datasetSelector.advancedSelector.configurator.languageLabel',
@@ -144,6 +135,46 @@ export const Configurator = ({
               }}
             />
           </EuiFormRow>
+          {!languageService.getLanguage(language)?.hideDatePicker &&
+            (dataset.type === DEFAULT_DATA.SET_TYPES.INDEX_PATTERN ? (
+              <EuiFormRow
+                label={i18n.translate(
+                  'data.explorer.datasetSelector.advancedSelector.configurator.indexPatternTimeFieldLabel',
+                  {
+                    defaultMessage: 'Time field',
+                  }
+                )}
+              >
+                <EuiFieldText disabled value={dataset.timeFieldName ?? 'No time field'} />
+              </EuiFormRow>
+            ) : (
+              <EuiFormRow
+                label={i18n.translate(
+                  'data.explorer.datasetSelector.advancedSelector.configurator.timeFieldLabel',
+                  {
+                    defaultMessage: 'Time field',
+                  }
+                )}
+              >
+                <EuiSelect
+                  options={[
+                    ...timeFields.map((field) => ({
+                      text: field.displayName || field.name,
+                      value: field.name,
+                    })),
+                    { text: '-----', value: '-----', disabled: true },
+                    { text: noTimeFilter, value: noTimeFilter },
+                  ]}
+                  value={timeFieldName}
+                  onChange={(e) => {
+                    const value = e.target.value === noTimeFilter ? undefined : e.target.value;
+                    setTimeFieldName(e.target.value);
+                    setDataset({ ...dataset, timeFieldName: value });
+                  }}
+                  hasNoInitialSelection
+                />
+              </EuiFormRow>
+            ))}
         </EuiForm>
       </EuiModalBody>
       <EuiModalFooter>
@@ -165,6 +196,7 @@ export const Configurator = ({
             onConfirm(dataset);
           }}
           fill
+          disabled={submitDisabled}
         >
           <FormattedMessage
             id="data.explorer.datasetSelector.advancedSelector.confirm"

diff --git a/src/plugins/data_explorer/public/components/app_container.tsx b/src/plugins/data_explorer/public/components/app_container.tsx
@@ -84,7 +84,10 @@ export const AppContainer = React.memo(
         )}
 
         <EuiPage
-          className={classNames('deLayout', isEnhancementsEnabled && 'dsc--next')}
+          className={classNames(
+            'deLayout',
+            isEnhancementsEnabled && !showActionsInGroup ? 'dsc--next' : undefined
+          )}
           paddingSize="none"
           grow={false}
         >

diff --git a/src/plugins/query_enhancements/public/plugin.tsx b/src/plugins/query_enhancements/public/plugin.tsx
@@ -100,6 +100,7 @@ export class QueryEnhancementsPlugin
       editor: enhancedSQLQueryEditor,
       editorSupportedAppNames: ['discover'],
       supportedAppNames: ['discover', 'data-explorer'],
+      hideDatePicker: true,
     };
     queryString.getLanguageService().registerLanguage(sqlLanguageConfig);
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		fix:
		- Fixes UI issues in Discover and data configurator ([#8166](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8166))