Skip to content

Commit

Permalink
[Discover] Allow save query to load correctly in Discover
Browse files Browse the repository at this point in the history
* add save query logic in Discover
* add save query logic in VisBuilder
* remove double render

Issue Resolve
opensearch-project#5942

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
  • Loading branch information
ananzh committed Feb 26, 2024
1 parent d56b04d commit 56411ba
Show file tree
Hide file tree
Showing 13 changed files with 106 additions and 20 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [BUG] Remove duplicate sample data as id 90943e30-9a47-11e8-b64d-95841ca0b247 ([5668](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5668))
- [BUG][Multiple Datasource] Fix datasource testing connection unexpectedly passed with wrong endpoint [#5663](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5663)
- [Table Visualization] Fix filter action buttons for split table aggregations ([#5619](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5619))
- [BUG][Discover] Allow save query to load correctly in Discover ([#5951](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5951))

### 🚞 Infrastructure

Expand Down
5 changes: 3 additions & 2 deletions src/plugins/data_explorer/public/components/app_container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,14 @@ export const AppContainer = ({ view, params }: { view?: View; params: AppMountPa

const MemoizedPanel = memo(Panel);
const MemoizedCanvas = memo(Canvas);
const MemoizedContext = memo(Context);

// Render the application DOM.
return (
<EuiPage className="deLayout" paddingSize="none">
{/* TODO: improve fallback state */}
<Suspense fallback={<div>Loading...</div>}>
<Context {...params}>
<MemoizedContext {...params}>
<EuiResizableContainer direction={isMobile ? 'vertical' : 'horizontal'}>
{(EuiResizablePanel, EuiResizableButton) => (
<>
Expand All @@ -53,7 +54,7 @@ export const AppContainer = ({ view, params }: { view?: View; params: AppMountPa
</>
)}
</EuiResizableContainer>
</Context>
</MemoizedContext>
</Suspense>
</EuiPage>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export interface DiscoverState {
/**
* Metadata for the view
*/
savedQuery?: string;
metadata?: {
/**
* Number of lines to display per row
Expand Down Expand Up @@ -110,6 +111,9 @@ export const discoverSlice = createSlice({
setState(state, action: PayloadAction<DiscoverState>) {
return action.payload;
},
getState(state, action:PayloadAction<DiscoverState>) {

Check failure on line 114 in src/plugins/discover/public/application/utils/state_management/discover_slice.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux (ciGroup1)

Insert `·`
return state;
},
addColumn(state, action: PayloadAction<{ column: string; index?: number }>) {
const columns = utils.addColumn(state.columns || [], action.payload);
return { ...state, columns: buildColumns(columns) };
Expand Down Expand Up @@ -188,6 +192,12 @@ export const discoverSlice = createSlice({
},
};
},
setSavedQuery(state, action: PayloadAction<string>) {
return {
...state,
savedQuery: action.payload,
}

Check failure on line 199 in src/plugins/discover/public/application/utils/state_management/discover_slice.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux (ciGroup1)

Insert `;`
}

Check failure on line 200 in src/plugins/discover/public/application/utils/state_management/discover_slice.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux (ciGroup1)

Insert `,`
},
});

Expand All @@ -201,8 +211,10 @@ export const {
setSort,
setInterval,
setState,
getState,
updateState,
setSavedSearchId,
setMetadata,
setSavedQuery,
} = discoverSlice.actions;
export const { reducer } = discoverSlice;
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { SortOrder } from '../../../saved_searches/types';
import { DOC_HIDE_TIME_COLUMN_SETTING } from '../../../../common';
import { OpenSearchSearchHit } from '../../doc_views/doc_views_types';
import { popularizeField } from '../../helpers/popularize_field';
import { buildColumns } from '../../utils/columns';

interface Props {
rows?: OpenSearchSearchHit[];
Expand All @@ -41,7 +42,20 @@ export const DiscoverTable = ({ rows }: Props) => {
} = services;

const { refetch$, indexPattern, savedSearch } = useDiscoverContext();
const { columns, sort } = useSelector((state) => state.discover);
const { columns } = useSelector((state) => {
const stateColumns = state.discover.columns;
// check if state columns is not undefined, otherwise use buildColumns
return {
columns: stateColumns !== undefined ? stateColumns : buildColumns([]),
};
});
const { sort } = useSelector((state) => {
const stateSort = state.discover.sort;
// check if state sort is not undefined, otherwise assign an empty array
return {
sort: stateSort !== undefined ? stateSort : [],
};
});
const dispatch = useDispatch();
const onAddColumn = (col: string) => {
if (indexPattern && capabilities.discover?.save) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,21 @@ import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_re
import { filterColumns } from '../utils/filter_columns';
import { DEFAULT_COLUMNS_SETTING, MODIFY_COLUMNS_ON_SWITCH } from '../../../../common';
import { OpenSearchSearchHit } from '../../../application/doc_views/doc_views_types';
import { buildColumns } from '../../utils/columns';
import './discover_canvas.scss';

// eslint-disable-next-line import/no-default-export
export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewProps) {
const { data$, refetch$, indexPattern } = useDiscoverContext();
const {
services: { uiSettings },
} = useOpenSearchDashboards<DiscoverViewServices>();
const { columns } = useSelector((state) => state.discover);
const { services: {uiSettings, capabilities} } = useOpenSearchDashboards<DiscoverViewServices>();

Check failure on line 29 in src/plugins/discover/public/application/view_components/canvas/index.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux (ciGroup1)

Replace `·services:·{uiSettings,·capabilities}` with `⏎····services:·{·uiSettings,·capabilities·},⏎·`
const { columns } = useSelector(state => {

Check failure on line 30 in src/plugins/discover/public/application/view_components/canvas/index.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux (ciGroup1)

Replace `state` with `(state)`
const stateColumns = state.discover.columns;

Check failure on line 32 in src/plugins/discover/public/application/view_components/canvas/index.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux (ciGroup1)

Delete `··`
// check if stateColumns is not undefined, otherwise use buildColumns
return {
columns: stateColumns !== undefined ? stateColumns : buildColumns([])

Check failure on line 35 in src/plugins/discover/public/application/view_components/canvas/index.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux (ciGroup1)

Insert `,`
};
});
const filteredColumns = filterColumns(
columns,
indexPattern,
Expand Down Expand Up @@ -89,6 +95,7 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro
}, [dispatch, filteredColumns, indexPattern]);

const timeField = indexPattern?.timeFieldName ? indexPattern.timeFieldName : undefined;
const showSaveQuery = !!capabilities.discover?.saveQuery;

return (
<EuiPanel
Expand All @@ -103,6 +110,7 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro
setHeaderActionMenu,
onQuerySubmit,
}}
showSaveQuery={showSaveQuery}
/>
{fetchState.status === ResultStatus.NO_RESULTS && (
<DiscoverNoResults timeFieldName={timeField} queryLanguage={''} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,22 @@ import { getTopNavLinks } from '../../components/top_nav/get_top_nav_links';
import { useDiscoverContext } from '../context';
import { getRootBreadcrumbs } from '../../helpers/breadcrumbs';
import { opensearchFilters, connectStorageToQueryState } from '../../../../../data/public';
import { useDispatch, setSavedQuery, useSelector, setState } from '../../utils/state_management';

export interface TopNavProps {
opts: {
setHeaderActionMenu: AppMountParameters['setHeaderActionMenu'];
onQuerySubmit: (payload: { dateRange: TimeRange; query?: Query }, isUpdate?: boolean) => void;
};
showSaveQuery: boolean;
}

export const TopNav = ({ opts }: TopNavProps) => {
export const TopNav = ({ opts, showSaveQuery }: TopNavProps) => {
const { services } = useOpenSearchDashboards<DiscoverViewServices>();
const { inspectorAdapters, savedSearch, indexPattern } = useDiscoverContext();
const [indexPatterns, setIndexPatterns] = useState<IndexPattern[] | undefined>(undefined);
const state = useSelector((s) => s.discover);
const dispatch = useDispatch();

const {
navigation: {
Expand All @@ -36,16 +40,10 @@ export const TopNav = ({ opts }: TopNavProps) => {
},
data,
chrome,
osdUrlStateStorage,
} = services;

const topNavLinks = savedSearch ? getTopNavLinks(services, inspectorAdapters, savedSearch) : [];

connectStorageToQueryState(services.data.query, osdUrlStateStorage, {
filters: opensearchFilters.FilterStateStore.APP_STATE,
query: true,
});

useEffect(() => {
let isMounted = true;
const getDefaultIndexPattern = async () => {
Expand Down Expand Up @@ -79,17 +77,30 @@ export const TopNav = ({ opts }: TopNavProps) => {
indexPattern,
]);

const updateSavedQueryId = (newSavedQueryId: string | undefined) => {
if (newSavedQueryId) {
dispatch(setSavedQuery(newSavedQueryId));
} else {
// remove savedQueryId from state
const newState = { ...state };
delete newState.savedQuery;
dispatch(setState(newState));
}
};

return (
<TopNavMenu
appName={PLUGIN_ID}
config={topNavLinks}
showSearchBar
showDatePicker={showDatePicker}
showSaveQuery
showSaveQuery={showSaveQuery}
useDefaultBehaviors
setMenuMountPoint={opts.setHeaderActionMenu}
indexPatterns={indexPattern ? [indexPattern] : indexPatterns}
onQuerySubmit={opts.onQuerySubmit}
savedQueryId={state.savedQuery}
onSavedQueryIdChange={updateSavedQueryId}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,13 @@ export default function DiscoverPanel(props: ViewProps) {
const { data$, indexPattern } = useDiscoverContext();
const [fetchState, setFetchState] = useState<SearchData>(data$.getValue());

const { columns } = useSelector((state) => ({
columns: state.discover.columns,
}));
const { columns } = useSelector((state) => {
const stateColumns = state.discover.columns;
// check if state columns is not undefined, otherwise use buildColumns
return {
columns: stateColumns !== undefined ? stateColumns : buildColumns([]),
};
});

const prevColumns = useRef(columns);
const dispatch = useDispatch();
Expand All @@ -47,6 +51,7 @@ export default function DiscoverPanel(props: ViewProps) {
if (columns !== prevColumns.current) {
let updatedColumns = buildColumns(columns);
if (
columns &&
timeFieldname &&
!prevColumns.current.includes(timeFieldname) &&
columns.includes(timeFieldname)
Expand Down
7 changes: 7 additions & 0 deletions src/plugins/discover/public/url_generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ export interface DiscoverUrlGeneratorState {
* whether to hash the data in the url to avoid url length issues.
*/
useHash?: boolean;
/**
* Saved query Id
*/
savedQuery?: string;
}

interface Params {
Expand All @@ -99,12 +103,14 @@ export class DiscoverUrlGenerator
savedSearchId,
timeRange,
useHash = this.params.useHash,
savedQuery,
}: DiscoverUrlGeneratorState): Promise<string> => {
const savedSearchPath = savedSearchId ? encodeURIComponent(savedSearchId) : '';
const appState: {
query?: Query;
filters?: Filter[];
index?: string;
savedQuery?: string;
} = {};
const queryState: QueryState = {};

Expand All @@ -117,6 +123,7 @@ export class DiscoverUrlGenerator
if (filters && filters.length)
queryState.filters = filters?.filter((f) => opensearchFilters.isFilterPinned(f));
if (refreshInterval) queryState.refreshInterval = refreshInterval;
if (savedQuery) appState.savedQuery = savedQuery;

let url = `${this.params.appBasePath}#/${savedSearchPath}`;
url = setStateToOsdUrl<QueryState>('_g', queryState, { useHash }, url);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { VisBuilderServices } from '../../types';
import './top_nav.scss';
import { useIndexPatterns, useSavedVisBuilderVis } from '../utils/use';
import { useTypedSelector, useTypedDispatch } from '../utils/state_management';
import { setSavedQuery, setState } from '../utils/state_management/visualization_slice';
import { setEditorState } from '../utils/state_management/metadata_slice';
import { useCanSave } from '../utils/use/use_can_save';
import { saveStateToSavedObject } from '../../saved_visualizations/transforms';
Expand All @@ -29,6 +30,7 @@ export const TopNav = () => {
ui: { TopNavMenu },
},
appName,
capabilities,
} = services;
const rootState = useTypedSelector((state) => state);
const dispatch = useTypedDispatch();
Expand Down Expand Up @@ -78,6 +80,18 @@ export const TopNav = () => {
dispatch(setEditorState({ state: 'loading' }));
});

const updateSavedQueryId = (newSavedQueryId: string | undefined) => {
if (newSavedQueryId) {
dispatch(setSavedQuery(newSavedQueryId));
} else {
// remove savedQueryId from state
const newState = rootState;
delete newState.visualization.savedQuery;
dispatch(setState(newState.visualization));
}
};
const showSaveQuery=!!capabilities['visualization-visbuilder']?.saveQuery;

Check failure on line 93 in src/plugins/vis_builder/public/application/components/top_nav.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux (ciGroup1)

Replace `=` with `·=·`

return (
<div className="vbTopNav">
<TopNavMenu
Expand All @@ -87,8 +101,10 @@ export const TopNav = () => {
indexPatterns={indexPattern ? [indexPattern] : []}
showDatePicker={!!indexPattern?.timeFieldName ?? true}
showSearchBar
showSaveQuery
showSaveQuery={showSaveQuery}
useDefaultBehaviors
savedQueryId={rootState.visualization.savedQuery}
onSavedQueryIdChange={updateSavedQueryId}
/>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface VisualizationState {
aggConfigParams: CreateAggConfigParams[];
draftAgg?: CreateAggConfigParams;
};
savedQuery?: string

Check failure on line 19 in src/plugins/vis_builder/public/application/utils/state_management/visualization_slice.ts

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux (ciGroup1)

Insert `;`
}

const initialState: VisualizationState = {
Expand Down Expand Up @@ -115,6 +116,12 @@ export const slice = createSlice({
[action.payload.paramName]: action.payload.value,
};
},
setSavedQuery(state, action: PayloadAction<string>) {
return {
...state,
savedQuery: action.payload,
}

Check failure on line 123 in src/plugins/vis_builder/public/application/utils/state_management/visualization_slice.ts

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux (ciGroup1)

Insert `;`
},
setState: (_state, action: PayloadAction<VisualizationState>) => {
return action.payload;
},
Expand All @@ -138,5 +145,6 @@ export const {
updateAggConfigParams,
setAggParamValue,
reorderAgg,
setSavedQuery,
setState,
} = slice.actions;
1 change: 1 addition & 0 deletions src/plugins/vis_builder/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ export class VisBuilderPlugin
embeddable: pluginsStart.embeddable,
dashboard: pluginsStart.dashboard,
uiActions: pluginsStart.uiActions,
capabilities: coreStart.application.capabilities,
};

// Instantiate the store
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/vis_builder/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { AppMountParameters, CoreStart, ToastsStart, ScopedHistory } from '../..
import { IOsdUrlStateStorage } from '../../opensearch_dashboards_utils/public';
import { DataPublicPluginSetup } from '../../data/public';
import { UiActionsStart } from '../../ui_actions/public';
import { Capabilities } from '../../../core/public';

export type VisBuilderSetup = TypeServiceSetup;
export interface VisBuilderStart extends TypeServiceStart {
Expand Down Expand Up @@ -54,6 +55,7 @@ export interface VisBuilderServices extends CoreStart {
osdUrlStateStorage: IOsdUrlStateStorage;
dashboard: DashboardStart;
uiActions: UiActionsStart;
capabilities: Capabilities;
}

export interface ISavedVis {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_builder/server/capabilities_provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ export const capabilitiesProvider = () => ({
show: true,
// showWriteControls: true,
// save: true,
// saveQuery: true,
saveQuery: true,
},
});

0 comments on commit 56411ba

Please sign in to comment.