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

Add data summary panel in discover #8186

Merged
merged 8 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
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
2 changes: 2 additions & 0 deletions changelogs/fragments/8186.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- Add data summary panel in discover ([#8186](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8186))
2 changes: 2 additions & 0 deletions src/plugins/data/common/data_frames/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import { SearchResponse } from 'elasticsearch';
import { BehaviorSubject } from 'rxjs';
import { IFieldType } from './fields';

export * from './_df_cache';
Expand All @@ -19,6 +20,7 @@ export interface DataFrameService {
get: () => IDataFrame | undefined;
set: (dataFrame: IDataFrame) => void;
clear: () => void;
df$: BehaviorSubject<IDataFrame | undefined>;
}

/**
Expand Down
30 changes: 27 additions & 3 deletions src/plugins/data/public/search/search_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
private searchInterceptor!: ISearchInterceptor;
private defaultSearchInterceptor!: ISearchInterceptor;
private usageCollector?: SearchUsageCollector;
private dataFrame$ = new BehaviorSubject<IDataFrame | undefined>(undefined);

constructor(private initializerContext: PluginInitializerContext<ConfigSchema>) {}

Expand Down Expand Up @@ -120,13 +121,31 @@
expressions.registerFunction(aggShardDelay);
}

const dfService: DataFrameService = {
chishui marked this conversation as resolved.
Show resolved Hide resolved
get: () => {
const df = this.dfCache.get();
this.dataFrame$.next(df);
return df;

Check warning on line 128 in src/plugins/data/public/search/search_service.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/search/search_service.ts#L126-L128

Added lines #L126 - L128 were not covered by tests
},
set: (dataFrame: IDataFrame) => {
this.dfCache.set(dataFrame);

Check warning on line 131 in src/plugins/data/public/search/search_service.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/search/search_service.ts#L131

Added line #L131 was not covered by tests
},
clear: () => {
if (this.dfCache.get() === undefined) return;
this.dfCache.clear();
this.dataFrame$.next(undefined);

Check warning on line 136 in src/plugins/data/public/search/search_service.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/search/search_service.ts#L135-L136

Added lines #L135 - L136 were not covered by tests
},
df$: this.dataFrame$,
};

return {
aggs,
usageCollector: this.usageCollector!,
__enhance: (enhancements: SearchEnhancements) => {
this.searchInterceptor = enhancements.searchInterceptor;
},
getDefaultSearchInterceptor: () => this.defaultSearchInterceptor,
df: dfService,
};
}

Expand All @@ -152,16 +171,21 @@

const loadingCount$ = new BehaviorSubject(0);
http.addLoadingCountSource(loadingCount$);

const dfService: DataFrameService = {
get: () => this.dfCache.get(),
set: async (dataFrame: IDataFrame) => {
get: () => {
const df = this.dfCache.get();
this.dataFrame$.next(df);
return df;

Check warning on line 178 in src/plugins/data/public/search/search_service.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/search/search_service.ts#L176-L178

Added lines #L176 - L178 were not covered by tests
},
set: (dataFrame: IDataFrame) => {
this.dfCache.set(dataFrame);
},
clear: () => {
if (this.dfCache.get() === undefined) return;
this.dfCache.clear();
this.dataFrame$.next(undefined);

Check warning on line 186 in src/plugins/data/public/search/search_service.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/search/search_service.ts#L186

Added line #L186 was not covered by tests
},
df$: this.dataFrame$,
};

const searchSourceDependencies: SearchSourceDependencies = {
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data/public/search/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export interface ISearchSetup {
*/
__enhance: (enhancements: SearchEnhancements) => void;
getDefaultSearchInterceptor: () => ISearchInterceptor;
df: DataFrameService;
}

/**
Expand Down
14 changes: 14 additions & 0 deletions src/plugins/data/public/ui/query_editor/query_editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,19 @@ export default class QueryEditorUI extends Component<Props, State> {
return <QueryControls queryControls={queryControls} />;
};

private renderExtensionSearchBarButtion = () => {
chishui marked this conversation as resolved.
Show resolved Hide resolved
const supported = 'query-assist';
chishui marked this conversation as resolved.
Show resolved Hide resolved
if (!this.extensionMap || Object.keys(this.extensionMap).length === 0) return null;
if (!(supported in this.extensionMap) || !this.extensionMap[supported].getSearchBarButton)
return null;
return this.extensionMap[supported].getSearchBarButton({
language: this.props.query.language,
onSelectLanguage: this.onSelectLanguage,
isCollapsed: this.state.isCollapsed,
setIsCollapsed: this.setIsCollapsed,
});
};

public render() {
const className = classNames(this.props.className);

Expand Down Expand Up @@ -455,6 +468,7 @@ export default class QueryEditorUI extends Component<Props, State> {
<EuiFlexGroup responsive={false} gutterSize="s" alignItems="center">
{this.renderQueryControls(languageEditor.TopBar.Controls)}
{!languageEditor.TopBar.Expanded && this.renderToggleIcon()}
{!languageEditor.TopBar.Expanded && this.renderExtensionSearchBarButtion()}
{this.props.savedQueryManagement}
</EuiFlexGroup>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ export interface QueryEditorExtensionConfig {
* @returns The component the query editor extension.
*/
getBanner?: (dependencies: QueryEditorExtensionDependencies) => React.ReactElement | null;

getSearchBarButton?: (
dependencies: QueryEditorExtensionDependencies
) => React.ReactElement | null;
}
const QueryEditorExtensionPortal: React.FC<{ container: Element }> = (props) => {
if (!props.children) return null;
Expand Down
3 changes: 3 additions & 0 deletions src/plugins/query_enhancements/common/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ export const configSchema = schema.object({
defaultValue: [{ language: 'PPL', agentConfig: 'os_query_assist_ppl' }],
}
),
summary: schema.object({
enabled: schema.boolean({ defaultValue: false }),
}),
}),
});

Expand Down
3 changes: 2 additions & 1 deletion src/plugins/query_enhancements/opensearch_dashboards.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"server": true,
"ui": true,
"requiredPlugins": ["data", "opensearchDashboardsReact", "opensearchDashboardsUtils", "savedObjects", "uiActions"],
"optionalPlugins": ["dataSource"]
"optionalPlugins": ["dataSource", "usageCollection"],
"configPath": ["queryEnhancements"]
}

32 changes: 32 additions & 0 deletions src/plugins/query_enhancements/public/assets/sparkle_color.svg
chishui marked this conversation as resolved.
Show resolved Hide resolved
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
11 changes: 11 additions & 0 deletions src/plugins/query_enhancements/public/assets/sparkle_hollow.svg
Copy link
Member

Choose a reason for hiding this comment

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

what about in darkmode? will this be too hard to see?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for bring this up. I've made some accommodations for dark mode. It looks OK to me.
Screenshot 2024-09-14 at 16 01 51
Screenshot 2024-09-14 at 16 01 41

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
21 changes: 21 additions & 0 deletions src/plugins/query_enhancements/public/assets/sparkle_solid.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 7 additions & 2 deletions src/plugins/query_enhancements/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class QueryEnhancementsPlugin

public setup(
core: CoreSetup<QueryEnhancementsPluginStartDependencies>,
{ data }: QueryEnhancementsPluginSetupDependencies
{ data, usageCollection }: QueryEnhancementsPluginSetupDependencies
): QueryEnhancementsPluginSetup {
const { queryString } = data.query;
const pplSearchInterceptor = new PPLSearchInterceptor({
Expand Down Expand Up @@ -105,7 +105,12 @@ export class QueryEnhancementsPlugin

data.__enhance({
editor: {
queryEditorExtension: createQueryAssistExtension(core.http, data, this.config.queryAssist),
queryEditorExtension: createQueryAssistExtension(
core,
data,
this.config.queryAssist,
usageCollection
chishui marked this conversation as resolved.
Show resolved Hide resolved
),
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
*/

.queryAssist {
&.queryAssist__summary {
margin-top: $euiSizeXS;
}

&.queryAssist__callout {
margin-top: $euiSizeXS;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@

export { QueryAssistBar } from './query_assist_bar';
export { QueryAssistBanner } from './query_assist_banner';
export { QueryAssistSummary } from './query_assist_summary';
export { QueryAssistButton } from './query_assist_button';
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { setData, setStorage } from '../../services';
import { useGenerateQuery } from '../hooks';
import { AgentError, ProhibitedQueryError } from '../utils';
import { QueryAssistInput } from './query_assist_input';
import { useQueryAssist } from '../hooks';

jest.mock('../../../../opensearch_dashboards_react/public', () => ({
useOpenSearchDashboards: jest.fn(),
Expand All @@ -25,6 +26,9 @@ jest.mock('../../../../opensearch_dashboards_react/public', () => ({

jest.mock('../hooks', () => ({
useGenerateQuery: jest.fn().mockReturnValue({ generateQuery: jest.fn(), loading: false }),
useQueryAssist: jest
.fn()
.mockReturnValue({ updateQuestion: jest.fn(), isQueryAssistCollapsed: false }),
}));

jest.mock('./query_assist_input', () => ({
Expand Down Expand Up @@ -86,6 +90,14 @@ describe('QueryAssistBar', () => {
expect(component.container).toBeEmptyDOMElement();
});

it('renders null if question assist is collapsed', () => {
useQueryAssist.mockReturnValueOnce({ updateQuestion: jest.fn(), isQueryAssistCollapsed: true });
const { component } = renderQueryAssistBar({
dependencies: { ...dependencies, isCollapsed: false },
});
expect(component.container).toBeEmptyDOMElement();
});

it('matches snapshot', () => {
const { component } = renderQueryAssistBar();
expect(component.container).toMatchSnapshot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { getPersistedLog, AgentError, ProhibitedQueryError } from '../utils';
import { QueryAssistCallOut, QueryAssistCallOutType } from './call_outs';
import { QueryAssistInput } from './query_assist_input';
import { QueryAssistSubmitButton } from './submit_button';
import { useQueryAssist } from '../hooks';

interface QueryAssistInputProps {
dependencies: QueryEditorExtensionDependencies;
Expand All @@ -42,6 +43,7 @@ export const QueryAssistBar: React.FC<QueryAssistInputProps> = (props) => {
);
const selectedIndex = selectedDataset?.title;
const previousQuestionRef = useRef<string>();
const { updateQuestion, isQueryAssistCollapsed } = useQueryAssist();

useEffect(() => {
const subscription = queryString.getUpdates$().subscribe((query) => {
Expand All @@ -64,6 +66,7 @@ export const QueryAssistBar: React.FC<QueryAssistInputProps> = (props) => {
setAgentError(undefined);
previousQuestionRef.current = inputRef.current.value;
persistedLog.add(inputRef.current.value);
updateQuestion(inputRef.current.value);
const params: QueryAssistParameters = {
question: inputRef.current.value,
index: selectedIndex,
Expand All @@ -90,7 +93,7 @@ export const QueryAssistBar: React.FC<QueryAssistInputProps> = (props) => {
}
};

if (props.dependencies.isCollapsed) return null;
if (props.dependencies.isCollapsed || isQueryAssistCollapsed) return null;

return (
<EuiForm component="form" onSubmit={onSubmit} className="queryAssist queryAssist__form">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/
import { fireEvent, render, screen } from '@testing-library/react';
import React from 'react';
import { QueryAssistButton, QueryAssistButtonProps } from './query_assist_button';
import { useQueryAssist } from '../hooks';

jest.mock('../hooks', () => ({
useQueryAssist: jest.fn(),
}));

describe('query assist button', () => {
const setIsCollapsed = jest.fn();
const updateIsQueryAssistCollapsed = jest.fn();

const props: QueryAssistButtonProps = {
dependencies: {
isCollapsed: false,
setIsCollapsed,
},
};
const renderQueryAssistButton = (isCollapsed: boolean) => {
const component = render(
<div>
<QueryAssistButton
dependencies={{
...props.dependencies,
isCollapsed,
}}
/>
</div>
);
return component;
};

afterEach(() => {
jest.clearAllMocks();
});

it('if query editor collapsed, click button to expand', async () => {
useQueryAssist.mockImplementationOnce(() => ({
isQueryAssistCollapsed: true,
updateIsQueryAssistCollapsed,
}));
renderQueryAssistButton(true);
expect(screen.getByTestId('queryAssist_summary_button')).toBeInTheDocument();
await screen.getByTestId('queryAssist_summary_button');
fireEvent.click(screen.getByTestId('queryAssist_summary_button'));
expect(setIsCollapsed).toHaveBeenCalledWith(false);
expect(updateIsQueryAssistCollapsed).toHaveBeenCalledWith(false);
});

[true, false].forEach((isQueryAssistCollapsed) => {
it('if query editor expanded, click button to switch', async () => {
useQueryAssist.mockImplementationOnce(() => ({
isQueryAssistCollapsed,
updateIsQueryAssistCollapsed,
}));
renderQueryAssistButton(false);
expect(screen.getByTestId('queryAssist_summary_button')).toBeInTheDocument();
await screen.getByTestId('queryAssist_summary_button');
fireEvent.click(screen.getByTestId('queryAssist_summary_button'));
expect(setIsCollapsed).not.toHaveBeenCalled();
expect(updateIsQueryAssistCollapsed).toHaveBeenCalledWith(!isQueryAssistCollapsed);
});
});
});
Loading
Loading