Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discover] Support custom logic to insert time filter based on dataset type #8932

Merged
merged 4 commits into from
Dec 6, 2024
Merged

[Discover] Support custom logic to insert time filter based on dataset type #8932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5d97d3c
Pass time filter if language overrides hideDatePicker
joshuali925 Nov 23, 2024
0ff21eb
update outdated comments
joshuali925 Nov 26, 2024
a0ea032
update tests
joshuali925 Nov 26, 2024
7f6b2fc
Changeset file for PR #8932 created/updated
opensearch-changeset-bot[bot] Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/8932.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- Support custom logic to insert time filter based on dataset type ([#8932](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8932))
3 changes: 3 additions & 0 deletions src/plugins/data/public/query/query_string/dataset_service/dataset_service.mock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ const createSetupDatasetServiceMock = (): jest.Mocked<DatasetServiceContract> =>
fetchOptions: jest.fn(),
getRecentDatasets: jest.fn(),
addRecentDataset: jest.fn(),
clearCache: jest.fn(),
getLastCacheTime: jest.fn(),
removeFromRecentDatasets: jest.fn(),
};
};

Expand Down
9 changes: 8 additions & 1 deletion src/plugins/data/public/query/query_string/dataset_service/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ export interface DatasetTypeConfig {
id: string;
/** Human-readable title for the dataset type */
title: string;
languageOverrides?: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an example of how or when languageOverrides will be used?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currently no. this is for example if PPL needs different logic to insert the time filter for a specific dataset. Once the dataset overrides this option, PPL will not insert time filter in pplSearchInterceptor, but will pass the timeRange to params.body, and the dataset search strategy would be able to access the user query and timeRange to insert the time filter

for example

    languageOverrides: {
      PPL: {
        hideDatePicker: false,
      },
    },

and in search strategy, the request will contain

    const query: Query = request.body.query;
    const timeRange: TimeRange = request.body.timeRange;

[language: string]: {
/** The override transfers the responsibility of handling the input from
* the language interceptor to the dataset type search strategy. */
hideDatePicker?: boolean;
};
};
/** Metadata for UI representation */
meta: {
/** Icon to represent the dataset type */
Expand All @@ -51,7 +58,7 @@ export interface DatasetTypeConfig {
tooltip?: string;
/** Optional preference for search on page load else defaulted to true */
searchOnLoad?: boolean;
/** Optional supportsTimeFilter determines if a time filter is needed */
/** Optional supportsTimeFilter determines if a time field is supported */
supportsTimeFilter?: boolean;
/** Optional isFieldLoadAsync determines if field loads are async */
isFieldLoadAsync?: boolean;
Expand Down
72 changes: 68 additions & 4 deletions src/plugins/data/public/ui/dataset_selector/configurator.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { render, screen, fireEvent, waitFor } from '@testing-library/react';
import { Configurator } from './configurator';
import '@testing-library/jest-dom';
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import React from 'react';
import { setQueryService, setIndexPatterns } from '../../services';
import { IntlProvider } from 'react-intl';
import { Query } from '../../../../data/public';
import { Dataset } from 'src/plugins/data/common';
import { Query } from '../../../../data/public';
import { setIndexPatterns, setQueryService } from '../../services';
import { Configurator } from './configurator';

const getQueryMock = jest.fn().mockReturnValue({
query: '',
Expand Down Expand Up @@ -358,4 +358,68 @@ describe('Configurator Component', () => {
expect(submitButton).toBeEnabled();
});
});

it('should show the date picker if supportsTimeFilter is undefined', async () => {
const mockDataset = {
...mockBaseDataset,
timeFieldName: undefined,
type: 'index',
};
const { container } = render(
<IntlProvider locale="en" messages={messages}>
<Configurator
services={mockServices}
baseDataset={mockDataset}
onConfirm={mockOnConfirm}
onCancel={mockOnCancel}
onPrevious={mockOnPrevious}
/>
</IntlProvider>
);

expect(
container.querySelector(`[data-test-subj="advancedSelectorTimeFieldSelect"]`)
).toBeTruthy();
});

it('should hide the date picker if supportsTimeFilter is false', async () => {
const mockDataset = {
...mockBaseDataset,
timeFieldName: undefined,
type: 'index',
};
const datasetTypeConfig = mockServices
.getQueryService()
.queryString.getDatasetService()
.getType();
mockServices
.getQueryService()
.queryString.getDatasetService()
.getType.mockReturnValue({
...datasetTypeConfig,
meta: {
supportsTimeFilter: false,
},
});
const { container } = render(
<IntlProvider locale="en" messages={messages}>
<Configurator
services={mockServices}
baseDataset={mockDataset}
onConfirm={mockOnConfirm}
onCancel={mockOnCancel}
onPrevious={mockOnPrevious}
/>
</IntlProvider>
);

expect(
container.querySelector(`[data-test-subj="advancedSelectorTimeFieldSelect"]`)
).toBeFalsy();

mockServices
.getQueryService()
.queryString.getDatasetService()
.getType.mockReturnValue(datasetTypeConfig);
});
});
40 changes: 27 additions & 13 deletions src/plugins/data/public/ui/dataset_selector/configurator.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
const [selectedIndexedView, setSelectedIndexedView] = useState<string | undefined>();
const [indexedViews, setIndexedViews] = useState<DatasetIndexedView[]>([]);
const [isLoadingIndexedViews, setIsLoadingIndexedViews] = useState(false);
const [timeFieldsLoading, setTimeFieldsLoading] = useState(false);

useEffect(() => {
let isMounted = true;
Expand All @@ -91,23 +92,26 @@

const submitDisabled = useMemo(() => {
return (
timeFieldName === undefined &&
!(
languageService.getLanguage(language)?.hideDatePicker ||
dataset.type === DEFAULT_DATA.SET_TYPES.INDEX_PATTERN
) &&
timeFields &&
timeFields.length > 0
timeFieldsLoading ||
(timeFieldName === undefined &&
!(dataset.type === DEFAULT_DATA.SET_TYPES.INDEX_PATTERN) &&
timeFields &&
timeFields.length > 0)
);
}, [dataset, language, timeFieldName, timeFields, languageService]);
}, [dataset, timeFieldName, timeFields, timeFieldsLoading]);

useEffect(() => {
const fetchFields = async () => {
const datasetFields = await queryString
.getDatasetService()
.getType(baseDataset.type)
?.fetchFields(baseDataset);
const datasetType = queryString.getDatasetService().getType(baseDataset.type);
if (!datasetType) {
setTimeFields([]);
return;

Check warning on line 108 in src/plugins/data/public/ui/dataset_selector/configurator.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/ui/dataset_selector/configurator.tsx#L107-L108 

Added lines #L107 - L108 were not covered by tests
}

setTimeFieldsLoading(true);
const datasetFields = await datasetType
.fetchFields(baseDataset)
.finally(() => setTimeFieldsLoading(false));
const dateFields = datasetFields?.filter((field) => field.type === 'date');
setTimeFields(dateFields || []);
};
Expand Down Expand Up @@ -152,6 +156,16 @@
};
}, [indexedViewsService, selectedIndexedView, dataset]);

const shouldRenderDatePickerField = useCallback(() => {
const datasetType = queryString.getDatasetService().getType(dataset.type);

const supportsTimeField = datasetType?.meta?.supportsTimeFilter;
if (supportsTimeField !== undefined) {
return Boolean(supportsTimeField);
}
return true;
}, [dataset.type, queryString]);

return (
<>
<EuiModalHeader>
Expand Down Expand Up @@ -256,7 +270,7 @@
data-test-subj="advancedSelectorLanguageSelect"
/>
</EuiFormRow>
{!languageService.getLanguage(language)?.hideDatePicker &&
{shouldRenderDatePickerField() &&
(dataset.type === DEFAULT_DATA.SET_TYPES.INDEX_PATTERN ? (
<EuiFormRow
label={i18n.translate(
Expand Down
62 changes: 55 additions & 7 deletions src/plugins/data/public/ui/query_editor/query_editor_top_row.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { Query, UI_SETTINGS } from '../../../common';
import { coreMock } from '../../../../../core/public/mocks';
import { dataPluginMock } from '../../mocks';
import React from 'react';
import { I18nProvider } from '@osd/i18n/react';
import { createEditor, DQLBody, QueryEditorTopRow, SingleLineInput } from '../';
import { OpenSearchDashboardsContextProvider } from 'src/plugins/opensearch_dashboards_react/public';
import { cleanup, render, waitFor } from '@testing-library/react';
import { LanguageConfig } from '../../query';
import React from 'react';
import { OpenSearchDashboardsContextProvider } from 'src/plugins/opensearch_dashboards_react/public';
import { createEditor, DQLBody, QueryEditorTopRow, SingleLineInput } from '../';
import { coreMock } from '../../../../../core/public/mocks';
import { Query, UI_SETTINGS } from '../../../common';
import { dataPluginMock } from '../../mocks';
import { DatasetTypeConfig, LanguageConfig } from '../../query';
import { datasetServiceMock } from '../../query/query_string/dataset_service/dataset_service.mock';
import { getQueryService } from '../../services';

const startMock = coreMock.createStart();
Expand Down Expand Up @@ -66,6 +67,7 @@ const createMockStorage = () => ({
});

const dataPlugin = dataPluginMock.createStartContract(true);
const datasetService = datasetServiceMock.createStartContract();

function wrapQueryEditorTopRowInContext(testProps: any) {
const defaultOptions = {
Expand Down Expand Up @@ -111,6 +113,7 @@ describe('QueryEditorTopRow', () => {
beforeEach(() => {
jest.clearAllMocks();
(getQueryService as jest.Mock).mockReturnValue(dataPlugin.query);
dataPlugin.query.queryString.getDatasetService = jest.fn().mockReturnValue(datasetService);
});

afterEach(() => {
Expand Down Expand Up @@ -155,4 +158,49 @@ describe('QueryEditorTopRow', () => {
await waitFor(() => expect(container.querySelector(QUERY_EDITOR)).toBeTruthy());
expect(container.querySelector(DATE_PICKER)).toBeFalsy();
});

it('Should not render date picker if dataset type does not support time field', async () => {
const query: Query = {
query: 'test query',
dataset: datasetService.getDefault(),
language: 'test-language',
};
dataPlugin.query.queryString.getQuery = jest.fn().mockReturnValue(query);
datasetService.getType.mockReturnValue({
meta: { supportsTimeFilter: false },
} as DatasetTypeConfig);

const { container } = render(
wrapQueryEditorTopRowInContext({
query,
showQueryEditor: false,
showDatePicker: true,
})
);
await waitFor(() => expect(container.querySelector(QUERY_EDITOR)).toBeTruthy());
expect(container.querySelector(DATE_PICKER)).toBeFalsy();
});

it('Should render date picker if dataset overrides hideDatePicker to false', async () => {
const query: Query = {
query: 'test query',
dataset: datasetService.getDefault(),
language: 'test-language',
};
dataPlugin.query.queryString.getQuery = jest.fn().mockReturnValue(query);
datasetService.getType.mockReturnValue(({
meta: { supportsTimeFilter: true },
languageOverrides: { 'test-language': { hideDatePicker: false } },
} as unknown) as DatasetTypeConfig);

const { container } = render(
wrapQueryEditorTopRowInContext({
query,
showQueryEditor: false,
showDatePicker: true,
})
);
await waitFor(() => expect(container.querySelector(QUERY_EDITOR)).toBeTruthy());
expect(container.querySelector(DATE_PICKER)).toBeTruthy();
});
});
55 changes: 45 additions & 10 deletions src/plugins/data/public/ui/query_editor/query_editor_top_row.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,18 +224,53 @@ export default function QueryEditorTopRow(props: QueryEditorTopRowProps) {
);
}

/**
* Determines if the date picker should be rendered based on UI settings, dataset configuration, and language settings.
*
* @returns {boolean} Whether the date picker should be rendered
*
* UI Settings permutations (isDatePickerEnabled):
* - showDatePicker=true || showAutoRefreshOnly=true => true
* - showDatePicker=false && showAutoRefreshOnly=false => false
* - both undefined => true (default)
* If isDatePickerEnabled is false, returns false immediately
*
* Dataset Type permutations (datasetType?.meta?.supportsTimeFilter):
* - supportsTimeFilter=false => false
*
* Language permutations (when dataset.meta.supportsTimeFilter is undefined or true):
* - queryLanguage=undefined => true (shows date picker)
* - queryLanguage exists:
* - languageOverrides[queryLanguage].hideDatePicker=true => false
* - languageOverrides[queryLanguage].hideDatePicker=false => true
* - hideDatePicker=true => false
* - hideDatePicker=false => true
* - hideDatePicker=undefined => true
*/
function shouldRenderDatePicker(): boolean {
return (
Boolean((props.showDatePicker || props.showAutoRefreshOnly) ?? true) &&
!(
queryLanguage &&
data.query.queryString.getLanguageService().getLanguage(queryLanguage)?.hideDatePicker
) &&
(props.query?.dataset
? data.query.queryString.getDatasetService().getType(props.query.dataset.type)?.meta
?.supportsTimeFilter !== false
: true)
const { queryString } = data.query;
const datasetService = queryString.getDatasetService();
const languageService = queryString.getLanguageService();
const isDatePickerEnabled = Boolean(
(props.showDatePicker || props.showAutoRefreshOnly) ?? true
);
if (!isDatePickerEnabled) return false;

// Get dataset type configuration
const datasetType = props.query?.dataset
? datasetService.getType(props.query?.dataset.type)
: undefined;
// Check if dataset type explicitly configures the `supportsTimeFilter` option
if (datasetType?.meta?.supportsTimeFilter === false) return false;

if (
queryLanguage &&
datasetType?.languageOverrides?.[queryLanguage]?.hideDatePicker !== undefined
) {
return Boolean(!datasetType.languageOverrides[queryLanguage].hideDatePicker);
}

return Boolean(!(queryLanguage && languageService.getLanguage(queryLanguage)?.hideDatePicker));
}

function shouldRenderQueryEditor(): boolean {
Expand Down
7 changes: 5 additions & 2 deletions src/plugins/query_enhancements/common/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { CoreSetup } from 'opensearch-dashboards/public';
import { PollQueryResultsParams } from '../../data/common';
import { PollQueryResultsParams, TimeRange } from '../../data/common';

export interface QueryAggConfig {
[key: string]: {
Expand All @@ -26,7 +26,10 @@ export interface EnhancedFetchContext {
http: CoreSetup['http'];
path: string;
signal?: AbortSignal;
body?: { pollQueryResultsParams: PollQueryResultsParams };
body?: {
pollQueryResultsParams?: PollQueryResultsParams;
timeRange?: TimeRange;
};
}

export interface QueryStatusOptions<T> {
Expand Down
1 change: 1 addition & 0 deletions src/plugins/query_enhancements/common/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export const fetch = (context: EnhancedFetchContext, query: Query, aggConfig?: Q
query: { ...query, format: 'jdbc' },
aggConfig,
pollQueryResultsParams: context.body?.pollQueryResultsParams,
timeRange: context.body?.timeRange,
});
return from(
http.fetch({
Expand Down
Loading
Loading