addressing comments, adding tests

Signed-off-by: Sean Li <[email protected]>
opensearch-project · Oct 2, 2024 · 51e4490 · 51e4490
1 parent 347a24c
commit 51e4490
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 55 deletions.
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/language_service/types.ts b/src/plugins/data/public/query/query_string/language_service/types.ts
@@ -52,5 +52,5 @@ export interface LanguageConfig {
   showDocLinks?: boolean;
   editorSupportedAppNames?: string[];
   supportedAppNames?: string[];
-  disableDatePicker?: boolean;
+  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,7 +18,7 @@ import {
 } from '@elastic/eui';
 import { i18n } from '@osd/i18n';
 import { FormattedMessage } from '@osd/i18n/react';
-import React, { useEffect, useState } from 'react';
+import React, { useEffect, useMemo, useState } from 'react';
 import { BaseDataset, DEFAULT_DATA, Dataset, DatasetField } from '../../../common';
 import { getIndexPatterns, getQueryService } from '../../services';
 
@@ -52,16 +52,17 @@ export const Configurator = ({
     return languages[0];
   });
 
-  const shouldDisplayTimeFieldSelector = (ds: Dataset, lang: string, fields: DatasetField[]) => {
+  const submitDisabled = useMemo(() => {
     return (
+      timeFieldName === undefined &&
       !(
-        languageService.getLanguage(lang)?.disableDatePicker ||
-        ds.type === DEFAULT_DATA.SET_TYPES.INDEX_PATTERN
+        languageService.getLanguage(language)?.hideDatePicker ||
+        dataset.type === DEFAULT_DATA.SET_TYPES.INDEX_PATTERN
       ) &&
-      fields &&
-      fields.length > 0
+      timeFields &&
+      timeFields.length > 0
     );
-  };
+  }, [dataset, language, timeFieldName, timeFields, languageService]);
 
   useEffect(() => {
     const fetchFields = async () => {
@@ -109,47 +110,6 @@ export const Configurator = ({
           >
             <EuiFieldText disabled value={dataset.title} />
           </EuiFormRow>
-          {dataset.type === DEFAULT_DATA.SET_TYPES.INDEX_PATTERN &&
-            !languageService.getLanguage(language)?.disableDatePicker && (
-              <EuiFormRow
-                label={i18n.translate(
-                  'data.explorer.datasetSelector.advancedSelector.configurator.indexPatternTimeFieldLabel',
-                  {
-                    defaultMessage: 'Time field',
-                  }
-                )}
-              >
-                <EuiFieldText disabled value={dataset.timeFieldName ?? 'No time field'} />
-              </EuiFormRow>
-            )}
-          {shouldDisplayTimeFieldSelector(dataset, language, timeFields) && (
-            <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={dataset.type !== DEFAULT_DATA.SET_TYPES.INDEX_PATTERN}
-              />
-            </EuiFormRow>
-          )}
           <EuiFormRow
             label={i18n.translate(
               'data.explorer.datasetSelector.advancedSelector.configurator.languageLabel',
@@ -170,6 +130,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>
@@ -191,10 +191,7 @@ export const Configurator = ({
             onConfirm(dataset);
           }}
           fill
-          disabled={
-            timeFieldName === undefined &&
-            shouldDisplayTimeFieldSelector(dataset, language, timeFields)
-          }
+          disabled={submitDisabled}
         >
           <FormattedMessage
             id="data.explorer.datasetSelector.advancedSelector.confirm"

diff --git a/src/plugins/query_enhancements/public/plugin.tsx b/src/plugins/query_enhancements/public/plugin.tsx
@@ -100,7 +100,7 @@ export class QueryEnhancementsPlugin
       editor: enhancedSQLQueryEditor,
       editorSupportedAppNames: ['discover'],
       supportedAppNames: ['discover', 'data-explorer'],
-      disableDatePicker: true,
+      hideDatePicker: true,
     };
     queryString.getLanguageService().registerLanguage(sqlLanguageConfig);