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.

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

Fix UI and detection of external data source in query assist #7494

Merged
Changes from 1 commit
Commits
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
Next Next commit
fix(queryEditorExtensions): use dataset manager to determine external…
… datasource

Remove datasource and indexpattern since they are no longer the source
of truth after dataset manager is added, and they are not used in query
enhancement plugin.

Signed-off-by: Joshua Li <[email protected]>
joshuali925 committed Jul 25, 2024
commit bcafd93fe22bd383fb213ca0a49c4d045bf713aa
7 changes: 2 additions & 5 deletions src/plugins/data/public/ui/query_editor/query_editor.tsx
Original file line number Diff line number Diff line change
@@ -39,7 +39,6 @@ monaco.languages.register({ id: LANGUAGE_ID_KUERY });

export interface QueryEditorProps {
indexPatterns: Array<IIndexPattern | string>;
dataSource?: DataSource;
query: Query;
dataSetContainerRef?: React.RefCallback<HTMLDivElement>;
settings: Settings;
@@ -142,8 +141,6 @@ export default class QueryEditorUI extends Component<Props, State> {
configMap={this.extensionMap}
componentContainer={this.headerRef.current}
bannerContainer={this.bannerRef.current}
indexPatterns={this.props.indexPatterns}
dataSource={this.props.dataSource}
/>
);
}
@@ -375,11 +372,11 @@ export default class QueryEditorUI extends Component<Props, State> {
// eslint-disable-next-line no-unsanitized/property
style.innerHTML = `
.${containerId} .monaco-editor .view-lines {
padding-left: 15px;
padding-left: 15px;
}
.${containerId} .monaco-editor .cursor {
height: ${customCursorHeight}px !important;
margin-top: ${(38 - customCursorHeight) / 2}px !important;
margin-top: ${(38 - customCursorHeight) / 2}px !important;
}
`;

Original file line number Diff line number Diff line change
@@ -6,7 +6,6 @@
import { render, waitFor } from '@testing-library/react';
import React, { ComponentProps } from 'react';
import { of } from 'rxjs';
import { IIndexPattern } from '../../../../common';
import { QueryEditorExtension } from './query_editor_extension';

jest.mock('react-dom', () => ({
@@ -16,21 +15,6 @@ jest.mock('react-dom', () => ({

type QueryEditorExtensionProps = ComponentProps<typeof QueryEditorExtension>;

const mockIndexPattern = {
id: '1234',
title: 'logstash-*',
fields: [
{
name: 'response',
type: 'number',
esTypes: ['integer'],
aggregatable: true,
filterable: true,
searchable: true,
},
],
} as IIndexPattern;

describe('QueryEditorExtension', () => {
const getComponentMock = jest.fn();
const getBannerMock = jest.fn();
@@ -45,7 +29,6 @@ describe('QueryEditorExtension', () => {
getBanner: getBannerMock,
},
dependencies: {
indexPatterns: [mockIndexPattern],
language: 'Test',
},
componentContainer: document.createElement('div'),
Original file line number Diff line number Diff line change
@@ -7,8 +7,6 @@ import { EuiErrorBoundary } from '@elastic/eui';
import React, { useEffect, useMemo, useRef, useState } from 'react';
import ReactDOM from 'react-dom';
import { Observable } from 'rxjs';
import { IIndexPattern } from '../../../../common';
import { DataSource } from '../../../data_sources/datasource';

interface QueryEditorExtensionProps {
config: QueryEditorExtensionConfig;
@@ -18,14 +16,6 @@ interface QueryEditorExtensionProps {
}

export interface QueryEditorExtensionDependencies {
/**
* Currently selected index patterns.
*/
indexPatterns?: Array<IIndexPattern | string>;
/**
* Currently selected data source.
*/
dataSource?: DataSource;
/**
* Currently selected query language.
*/
Original file line number Diff line number Diff line change
@@ -14,33 +14,16 @@ type QueryEditorExtensionsProps = ComponentProps<typeof QueryEditorExtensions>;
jest.mock('./query_editor_extension', () => ({
QueryEditorExtension: jest.fn(({ config, dependencies }: QueryEditorExtensionProps) => (
<div>
Mocked QueryEditorExtension {config.id} with{' '}
{dependencies.indexPatterns?.map((i) => (typeof i === 'string' ? i : i.title)).join(', ')}
Mocked QueryEditorExtension {config.id} with {dependencies.language}
</div>
)),
}));

describe('QueryEditorExtensions', () => {
const defaultProps: QueryEditorExtensionsProps = {
indexPatterns: [
{
id: '1234',
title: 'logstash-*',
fields: [
{
name: 'response',
type: 'number',
esTypes: ['integer'],
aggregatable: true,
filterable: true,
searchable: true,
},
],
},
],
componentContainer: document.createElement('div'),
bannerContainer: document.createElement('div'),
language: 'Test',
language: 'Test-lang',
};

beforeEach(() => {
@@ -59,8 +42,8 @@ describe('QueryEditorExtensions', () => {

it('correctly orders configurations based on order property', () => {
const configMap = {
'1': { id: '1', order: 2, isEnabled: jest.fn(), getComponent: jest.fn() },
'2': { id: '2', order: 1, isEnabled: jest.fn(), getComponent: jest.fn() },
'1': { id: '1', order: 2, isEnabled$: jest.fn(), getComponent: jest.fn() },
'2': { id: '2', order: 1, isEnabled$: jest.fn(), getComponent: jest.fn() },
};

const { getAllByText } = render(
@@ -75,18 +58,18 @@ describe('QueryEditorExtensions', () => {

it('passes dependencies correctly to QueryEditorExtension', async () => {
const configMap = {
'1': { id: '1', order: 1, isEnabled: jest.fn(), getComponent: jest.fn() },
'1': { id: '1', order: 1, isEnabled$: jest.fn(), getComponent: jest.fn() },
};

const { getByText } = render(<QueryEditorExtensions {...defaultProps} configMap={configMap} />);

await waitFor(() => {
expect(getByText(/logstash-\*/)).toBeInTheDocument();
expect(getByText(/Test-lang/)).toBeInTheDocument();
});

expect(QueryEditorExtension).toHaveBeenCalledWith(
expect.objectContaining({
dependencies: { indexPatterns: defaultProps.indexPatterns, language: 'Test' },
dependencies: { language: 'Test-lang' },
}),
expect.anything()
);
Original file line number Diff line number Diff line change
@@ -48,7 +48,6 @@ export interface QueryEditorTopRowProps {
disableAutoFocus?: boolean;
screenTitle?: string;
indexPatterns?: Array<IIndexPattern | string>;
dataSource?: DataSource;
isLoading?: boolean;
prepend?: React.ComponentProps<typeof EuiCompressedFieldText>['prepend'];
showQueryEditor?: boolean;
@@ -220,7 +219,6 @@ export default function QueryEditorTopRow(props: QueryEditorTopRowProps) {
<QueryEditor
disableAutoFocus={props.disableAutoFocus}
indexPatterns={props.indexPatterns!}
dataSource={props.dataSource}
prepend={props.prepend}
query={parsedQuery}
dataSetContainerRef={props.dataSetContainerRef}
Original file line number Diff line number Diff line change
@@ -210,7 +210,6 @@ export function createSearchBar({
showSaveQuery={props.showSaveQuery}
screenTitle={props.screenTitle}
indexPatterns={props.indexPatterns}
dataSource={props.dataSource}
indicateNoData={props.indicateNoData}
timeHistory={data.query.timefilter.history}
dateRangeFrom={timeRange.from}
2 changes: 0 additions & 2 deletions src/plugins/data/public/ui/search_bar/search_bar.tsx
Original file line number Diff line number Diff line change
@@ -60,7 +60,6 @@ interface SearchBarInjectedDeps {

export interface SearchBarOwnProps {
indexPatterns?: IIndexPattern[];
dataSource?: DataSource;
isLoading?: boolean;
customSubmitButton?: React.ReactNode;
screenTitle?: string;
@@ -498,7 +497,6 @@ class SearchBarUI extends Component<SearchBarProps, State> {
screenTitle={this.props.screenTitle}
onSubmit={this.onQueryBarSubmit}
indexPatterns={this.props.indexPatterns}
dataSource={this.props.dataSource}
isLoading={this.props.isLoading}
prepend={this.props.showFilterBar ? savedQueryManagement : undefined}
showDatePicker={this.props.showDatePicker}
Original file line number Diff line number Diff line change
@@ -126,10 +126,6 @@ export const TopNav = ({ opts, showSaveQuery, isEnhancementsEnabled }: TopNavPro
useDefaultBehaviors
setMenuMountPoint={opts.setHeaderActionMenu}
indexPatterns={indexPattern ? [indexPattern] : indexPatterns}
// TODO after
// https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6833
// is ported to main, pass dataSource to TopNavMenu by picking
// commit 328e08e688c again.
onQuerySubmit={opts.onQuerySubmit}
savedQueryId={state.savedQuery}
onSavedQueryIdChange={updateSavedQueryId}
Original file line number Diff line number Diff line change
@@ -5,8 +5,8 @@

import { HttpSetup } from 'opensearch-dashboards/public';
import React, { useEffect, useState } from 'react';
import { of } from 'rxjs';
import { distinctUntilChanged, map, switchMap } from 'rxjs/operators';
import { SIMPLE_DATA_SOURCE_TYPES } from '../../../../data/common';
import {
DataPublicPluginSetup,
QueryEditorExtensionConfig,
@@ -28,6 +28,10 @@ const getAvailableLanguages$ = (
data.query.dataSet.getUpdates$().pipe(
distinctUntilChanged(),
switchMap(async (simpleDataSet) => {
// currently query assist tool relies on opensearch API to get index
// mappings, external data source types (e.g. s3) are not supported
if (simpleDataSet?.dataSourceRef?.type === SIMPLE_DATA_SOURCE_TYPES.EXTERNAL) return [];

const dataSourceId = simpleDataSet?.dataSourceRef?.id;
const cached = availableLanguagesByDataSource.get(dataSourceId);
if (cached !== undefined) return cached;
@@ -52,16 +56,10 @@ export const createQueryAssistExtension = (
return {
id: 'query-assist',
order: 1000,
isEnabled$: (dependencies) => {
// currently query assist tool relies on opensearch API to get index
// mappings, non-default data source types are not supported
if (dependencies.dataSource && dependencies.dataSource?.getType() !== 'default')
return of(false);

return getAvailableLanguages$(availableLanguagesByDataSource, http, data).pipe(
isEnabled$: () =>
getAvailableLanguages$(availableLanguagesByDataSource, http, data).pipe(
map((languages) => languages.length > 0)
);
},
),
getComponent: (dependencies) => {
// only show the component if user is on a supported language.
return (