Skip to content

Commit

Permalink
[VisBuilder] Fix filter and query bugs
Browse files Browse the repository at this point in the history
In this PR:
add filter and query in vb
remove saveDisabledReason
clean filterManager when start vb
add opensearch_dashboards_context to embeddable. This can add filter and query in the expression.

Issue Resolve
opensearch-project#5643
opensearch-project#5644
opensearch-project#5645
opensearch-project#5646
opensearch-project#6512

Signed-off-by: Anan Zhuang <[email protected]>
  • Loading branch information
ananzh committed Apr 18, 2024
1 parent 9408e49 commit 59bf1f2
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 66 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [BUG][Multiple Datasource] Modify the button of selectable component to fix the title overflow issue ([#6465](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6465))
- [BUG][Multiple Datasource] Validation succeed as long as status code in response is 200 ([#6399](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6399))
- [BUG][Multiple Datasource] Add validation for title length to be no longer than 32 characters [#6452](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6452))
- [BUG][VisBuilder] Fix filter and query bugs ([#6460](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6460))

### 🚞 Infrastructure

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import './top_nav.scss';
import { useIndexPatterns, useSavedVisBuilderVis } from '../utils/use';
import { useTypedSelector, useTypedDispatch } from '../utils/state_management';
import { setEditorState } from '../utils/state_management/metadata_slice';
import { useCanSave } from '../utils/use/use_can_save';
import { saveStateToSavedObject } from '../../saved_visualizations/transforms';
import { TopNavMenuData } from '../../../../navigation/public';
import { opensearchFilters, connectStorageToQueryState } from '../../../../data/public';
Expand All @@ -33,7 +32,6 @@ export const TopNav = () => {
const rootState = useTypedSelector((state) => state);
const dispatch = useTypedDispatch();

const saveDisabledReason = useCanSave();
const savedVisBuilderVis = useSavedVisBuilderVis(visualizationIdFromUrl);
connectStorageToQueryState(services.data.query, services.osdUrlStateStorage, {
filters: opensearchFilters.FilterStateStore.APP_STATE,
Expand All @@ -53,7 +51,6 @@ export const TopNav = () => {
{
visualizationIdFromUrl,
savedVisBuilderVis: saveStateToSavedObject(savedVisBuilderVis, rootState, indexPattern),
saveDisabledReason,
dispatch,
originatingApp,
},
Expand All @@ -67,7 +64,6 @@ export const TopNav = () => {
savedVisBuilderVis,
services,
visualizationIdFromUrl,
saveDisabledReason,
dispatch,
indexPattern,
originatingApp,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ describe('getOnSave', () => {
},
],
},
"searchSourceFields": Object {},
"searchSourceFields": Object {
"filter": null,
"query": null,
},
"styleState": "",
"title": "new title",
"version": 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,16 @@ import { VisBuilderSavedObject } from '../../types';
import { AppDispatch } from './state_management';
import { EDIT_PATH, VISBUILDER_SAVED_OBJECT } from '../../../common';
import { setEditorState } from './state_management/metadata_slice';

export interface TopNavConfigParams {
visualizationIdFromUrl: string;
savedVisBuilderVis: VisBuilderSavedObject;
saveDisabledReason?: string;
dispatch: AppDispatch;
originatingApp?: string;
}

export const getTopNavConfig = (
{
visualizationIdFromUrl,
savedVisBuilderVis,
saveDisabledReason,
dispatch,
originatingApp,
}: TopNavConfigParams,
{ visualizationIdFromUrl, savedVisBuilderVis, dispatch, originatingApp }: TopNavConfigParams,
services: VisBuilderServices
) => {
const {
Expand Down Expand Up @@ -84,8 +78,7 @@ export const getTopNavConfig = (
defaultMessage: 'save',
}),
testId: 'visBuilderSaveButton',
disableButton: !!saveDisabledReason,
tooltip: saveDisabledReason,
disableButton: false,
run: (_anchorElement) => {
const saveModal = (
<SavedObjectSaveModalOrigin
Expand Down Expand Up @@ -123,8 +116,7 @@ export const getTopNavConfig = (
}
),
testId: 'visBuilderSaveAndReturnButton',
disableButton: !!saveDisabledReason,
tooltip: saveDisabledReason,
disableButton: false,
run: async () => {
const saveOptions = {
newTitle: savedVisBuilderVis.title,
Expand Down Expand Up @@ -172,7 +164,7 @@ export const getOnSave = (
returnToOrigin: boolean;
newDescription?: string;
}) => {
const { embeddable, toastNotifications, application, history } = services;
const { data, embeddable, toastNotifications, application, history } = services;
const stateTransfer = embeddable.getStateTransfer();

if (!savedVisBuilderVis) {
Expand All @@ -183,6 +175,10 @@ export const getOnSave = (
savedVisBuilderVis.title = newTitle;
savedVisBuilderVis.description = newDescription;
savedVisBuilderVis.copyOnSave = newCopyOnSave;
const searchSourceInstance = savedVisBuilderVis.searchSourceFields;
searchSourceInstance.query = data.query.queryString.getQuery() || null;
searchSourceInstance.filter = data.query.filterManager.getFilters() || null;
savedVisBuilderVis.searchSourceFields = searchSourceInstance;
const newlyCreated = !savedVisBuilderVis.id || savedVisBuilderVis.copyOnSave;

try {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export const useSavedVisBuilderVis = (visualizationIdFromUrl: string | undefined
const {
application: { navigateToApp },
chrome,
data,
history,
http: { basePath },
toastNotifications,
Expand All @@ -61,6 +62,19 @@ export const useSavedVisBuilderVis = (visualizationIdFromUrl: string | undefined
const { title, state } = getStateFromSavedObject(savedVisBuilderVis);
chrome.setBreadcrumbs(getEditBreadcrumbs(title, navigateToApp));
chrome.docTitle.change(title);
// sync initial app filters from savedObject to filterManager
const filters = savedVisBuilderVis.searchSourceFields.filter;
const query =
savedVisBuilderVis.searchSourceFields.query || data.query.queryString.getDefaultQuery();
const actualFilters = [];
if (filters !== undefined) {
const result = typeof filters === 'function' ? filters() : filters;
if (result !== undefined) {
actualFilters.push(...(Array.isArray(result) ? result : [result]));
}
}
data.query.filterManager.setAppFilters(actualFilters);
data.query.queryString.setQuery(query);

dispatch(setUIStateState(state.ui));
dispatch(setStyleState(state.style));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,18 @@ import {
getTypeService,
getUIActions,
} from '../plugin_services';
import { PersistedState } from '../../../visualizations/public';
import { PersistedState, prepareJson } from '../../../visualizations/public';
import { VisBuilderSavedVis } from '../saved_visualizations/transforms';
import { handleVisEvent } from '../application/utils/handle_vis_event';
import { VisBuilderEmbeddableFactoryDeps } from './vis_builder_embeddable_factory';
import { VisBuilderSavedObject } from '../types';

// Apparently this needs to match the saved object type for the clone and replace panel actions to work
export const VISBUILDER_EMBEDDABLE = VISBUILDER_SAVED_OBJECT;

export interface VisBuilderEmbeddableConfiguration {
savedVis: VisBuilderSavedVis;
savedObject: VisBuilderSavedObject;
indexPatterns?: IIndexPattern[];
editPath: string;
editUrl: string;
Expand Down Expand Up @@ -79,6 +81,7 @@ export class VisBuilderEmbeddable extends Embeddable<VisBuilderInput, VisBuilder
private subscriptions: Subscription[] = [];
private node?: HTMLElement;
private savedVis?: VisBuilderSavedVis;
private savedObject?: VisBuilderSavedObject;
private serializedState?: string;
private uiState: PersistedState;
private readonly deps: VisBuilderEmbeddableFactoryDeps;
Expand All @@ -87,6 +90,7 @@ export class VisBuilderEmbeddable extends Embeddable<VisBuilderInput, VisBuilder
timefilter: TimefilterContract,
{
savedVis,
savedObject,
editPath,
editUrl,
editable,
Expand Down Expand Up @@ -116,6 +120,7 @@ export class VisBuilderEmbeddable extends Embeddable<VisBuilderInput, VisBuilder

this.deps = deps;
this.savedVis = savedVis;
this.savedObject = savedObject;
this.uiState = new PersistedState(savedVis.state.ui);
this.uiState.on('change', this.uiStateChangeHandler);
this.uiState.on('reload', this.reload);
Expand Down Expand Up @@ -246,6 +251,29 @@ export class VisBuilderEmbeddable extends Embeddable<VisBuilderInput, VisBuilder
this.autoRefreshFetchSubscription.unsubscribe();
}

private async updateExpression() {
// Construct the initial part of the pipeline with context management.
let pipeline = `opensearchDashboards | opensearch_dashboards_context `;

// Access the query and filters from savedObject if available.
const query = this.savedObject?.searchSourceFields?.query;
const filters = this.savedObject?.searchSourceFields?.filter;

// Append query and filters to the pipeline string if they exist.
if (query) {
pipeline += prepareJson('query', query);
}
if (filters) {
pipeline += prepareJson('filters', filters);
}

// Get the current expression from your getExpression method.
const currentExpression = (await this.getExpression()) ?? '';

// Replace 'opensearchDashboards' with the constructed pipeline in the existing expression.
return currentExpression.replace('opensearchDashboards', pipeline);
}

private async updateHandler() {
const expressionParams: IExpressionLoaderParams = {
searchContext: {
Expand All @@ -261,6 +289,8 @@ export class VisBuilderEmbeddable extends Embeddable<VisBuilderInput, VisBuilder
this.abortController = new AbortController();
const abortController = this.abortController;

this.expression = await this.updateExpression();

if (this.handler && !abortController.signal.aborted) {
this.handler.update(this.expression, expressionParams);
}
Expand Down Expand Up @@ -296,12 +326,8 @@ export class VisBuilderEmbeddable extends Embeddable<VisBuilderInput, VisBuilder
dirty = true;
}

if (dirty) {
this.expression = (await this.getExpression()) ?? '';

if (this.handler) {
this.updateHandler();
}
if (this.handler && dirty) {
this.updateHandler();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export class VisBuilderEmbeddableFactory
getTimeFilter(),
{
savedVis,
savedObject,
editUrl,
editPath,
editable: true,
Expand Down
4 changes: 4 additions & 0 deletions src/plugins/vis_builder/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ export class VisBuilderPlugin

// make sure the index pattern list is up to date
pluginsStart.data.indexPatterns.clearCache();
// make sure the filterManager is refreshed
const filters = pluginsStart.data.query.filterManager.getFilters();
const pinFilters = filters.filter(opensearchFilters.isFilterPinned);
pluginsStart.data.query.filterManager.setFilters(pinFilters ? pinFilters : []);
// make sure a default index pattern exists
// if not, the page will be redirected to management and visualize won't be rendered
// TODO: Add the redirect
Expand Down
1 change: 1 addition & 0 deletions src/plugins/visualizations/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,4 @@ export { ExprVisAPIEvents } from './expressions/vis';
export { VisualizationListItem } from './vis_types/vis_type_alias_registry';
export { VISUALIZE_ENABLE_LABS_SETTING } from '../common/constants';
export { createSavedVisLoader } from './saved_visualizations';
export { prepareJson } from './legacy/build_pipeline';

0 comments on commit 59bf1f2

Please sign in to comment.