Skip to content

Commit

Permalink
[Vis Builder] Bug fixes for datasource picker and auto time interval (o…
Browse files Browse the repository at this point in the history
…pensearch-project#2632)

* Fixes auto time interval

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

* Fixes change datasource while editing agg

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

* Misc fixes

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

* Updates Changelog

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

* Correctly sets timerange

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

* Fixes rebase

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>
Signed-off-by: Arpit Bandejiya <abandeji@amazon.com>
  • Loading branch information
ashwin-pc authored and Arpit-Bandejiya committed Jan 13, 2023
1 parent d905e21 commit df3e965
Show file tree
Hide file tree
Showing 19 changed files with 166 additions and 64 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
* [Vis Builder] Fixes visualization shift when editing agg ([2401](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2401))
* [Vis Builder] Renames "Histogram" to "Bar" in vis type picker ([2401](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2401))
* [Vis Builder] Update vislib params and misc fixes ([2610](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2610))
* [Vis Builder] Bug fixes for datasource picker and auto time interval ([2632](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2632))
* [MD] Add data source param to low-level search call in Discover ([#2431](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2431))
* [Multi DataSource] Skip data source view in index pattern step when pick default ([#2574](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2574))
* [Multi DataSource] Address UX comments on Edit Data source page ([#2629](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2629))
Expand Down
5 changes: 2 additions & 3 deletions src/core/public/saved_objects/simple_saved_object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ export class SimpleSavedObject<T = unknown> {
error,
references,
migrationVersion,
// eslint-disable-next-line @typescript-eslint/naming-convention
updated_at,
updated_at: updateAt,
}: SavedObjectType<T>
) {
this.id = id;
Expand All @@ -73,7 +72,7 @@ export class SimpleSavedObject<T = unknown> {
this.references = references || [];
this._version = version;
this.migrationVersion = migrationVersion;
this.updated_at = updated_at;
this.updated_at = updateAt;
if (error) {
this.error = error;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@ import { cloneDeep } from 'lodash';
import { BucketAggType, IndexPatternField, propFilter } from '../../../../../../data/common';
import { Schema } from '../../../../../../vis_default_editor/public';
import { COUNT_FIELD, FieldDragDataType } from '../../../utils/drag_drop/types';
import { useTypedDispatch, useTypedSelector } from '../../../utils/state_management';
import { useTypedDispatch } from '../../../utils/state_management';
import { DropboxDisplay, DropboxProps } from '../dropbox';
import { useDrop } from '../../../utils/drag_drop';
import {
editDraftAgg,
reorderAgg,
updateAggConfigParams,
} from '../../../utils/state_management/visualization_slice';
import { useIndexPatterns } from '../../../utils/use/use_index_pattern';
import { useOpenSearchDashboards } from '../../../../../../opensearch_dashboards_react/public';
import { VisBuilderServices } from '../../../../types';
import { useAggs } from '../../../utils/use';

const filterByName = propFilter('name');
const filterByType = propFilter('type');
Expand All @@ -30,36 +30,34 @@ export interface UseDropboxProps extends Pick<DropboxProps, 'id' | 'label'> {
export const useDropbox = (props: UseDropboxProps): DropboxProps => {
const { id: dropboxId, label, schema } = props;
const [validAggTypes, setValidAggTypes] = useState<string[]>([]);
const { aggConfigs, indexPattern, aggs, timeRange } = useAggs();
const dispatch = useTypedDispatch();
const indexPattern = useIndexPatterns().selected;
const {
services: {
data: {
search: { aggs: aggService },
},
},
} = useOpenSearchDashboards<VisBuilderServices>();
const aggConfigParams = useTypedSelector(
(state) => state.visualization.activeVisualization?.aggConfigParams
);

const aggConfigs = useMemo(() => {
return indexPattern && aggService.createAggConfigs(indexPattern, cloneDeep(aggConfigParams));
}, [aggConfigParams, aggService, indexPattern]);

const aggs = useMemo(() => aggConfigs?.aggs ?? [], [aggConfigs?.aggs]);

const dropboxAggs = aggs.filter((agg) => agg.schema === schema.name);
const dropboxAggs = useMemo(() => aggs.filter((agg) => agg.schema === schema.name), [
aggs,
schema.name,
]);

const displayFields: DropboxDisplay[] = useMemo(
() =>
dropboxAggs?.map(
(agg): DropboxDisplay => ({
id: agg.id,
label: agg.makeLabel(),
})
) || [],
[dropboxAggs]
(agg): DropboxDisplay => {
// For timeseries aggregations that have timeinterval set as auto, the current timerange is required to calculate the label accurately
agg.aggConfigs.setTimeRange(timeRange);
return {
id: agg.id,
label: agg.makeLabel(),
};
}
) ?? [],
[dropboxAggs, timeRange]
);

// Event handlers for each dropbox action type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const Option: FC<Props> = ({ title, children, initialIsOpen = false }) =>
buttonContent={title}
className="vbOption"
initialIsOpen={initialIsOpen}
data-test-subj={`vbOption-${title.replace(/\s+/g, '-')}`}
>
<EuiSpacer size="s" />
<EuiPanel color="subdued" className="vbOption__panel">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const Workspace: FC = ({ children }) => {
useEffect(() => {
async function loadExpression() {
const schemas = ui.containerConfig.data.schemas;
const [valid, errorMsg] = validateSchemaState(schemas, rootState);
const [valid, errorMsg] = validateSchemaState(schemas, rootState.visualization);

if (!valid) {
if (errorMsg) {
Expand All @@ -50,12 +50,13 @@ export const Workspace: FC = ({ children }) => {
setExpression(undefined);
return;
}
const exp = await toExpression(rootState);

const exp = await toExpression(rootState, searchContext);
setExpression(exp);
}

loadExpression();
}, [rootState, toExpression, toasts, ui.containerConfig.data.schemas]);
}, [rootState, toExpression, toasts, ui.containerConfig.data.schemas, searchContext]);

useLayoutEffect(() => {
const subscription = data.query.state$.subscribe(({ state }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const getPreloadedStore = async (services: VisBuilderServices) => {

// Infer the `RootState` and `AppDispatch` types from the store itself
export type RootState = ReturnType<typeof rootReducer>;
export type RenderState = Omit<RootState, 'metadata'>; // Remaining state after auxillary states are removed
type Store = ReturnType<typeof configurePreloadedStore>;
export type AppDispatch = Store['dispatch'];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const slice = createSlice({
setIndexPattern: (state, action: PayloadAction<string>) => {
state.indexPattern = action.payload;
state.activeVisualization!.aggConfigParams = [];
state.activeVisualization!.draftAgg = undefined;
},
setSearchField: (state, action: PayloadAction<string>) => {
state.searchField = action.payload;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

export { useAggs } from './use_aggs';
export { useVisualizationType } from './use_visualization_type';
export { useIndexPatterns } from './use_index_pattern';
export { useSavedVisBuilderVis } from './use_saved_vis_builder_vis';
57 changes: 57 additions & 0 deletions src/plugins/vis_builder/public/application/utils/use/use_aggs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { cloneDeep } from 'lodash';
import { useLayoutEffect, useMemo, useState } from 'react';
import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public';
import { VisBuilderServices } from '../../../types';
import { useTypedSelector, useTypedDispatch } from '../state_management';
import { useIndexPatterns } from './use_index_pattern';

/**
* Returns common agg parameters from the store and app context
* @returns { indexPattern, aggConfigs, aggs, timeRange }
*/
export const useAggs = () => {
const {
services: {
data: {
search: { aggs: aggService },
query: {
timefilter: { timefilter },
},
},
},
} = useOpenSearchDashboards<VisBuilderServices>();
const indexPattern = useIndexPatterns().selected;
const [timeRange, setTimeRange] = useState(timefilter.getTime());
const aggConfigParams = useTypedSelector(
(state) => state.visualization.activeVisualization?.aggConfigParams
);
const dispatch = useTypedDispatch();

const aggConfigs = useMemo(() => {
const configs =
indexPattern && aggService.createAggConfigs(indexPattern, cloneDeep(aggConfigParams));
return configs;
}, [aggConfigParams, aggService, indexPattern]);

useLayoutEffect(() => {
const subscription = timefilter.getTimeUpdate$().subscribe(() => {
setTimeRange(timefilter.getTime());
});

return () => {
subscription.unsubscribe();
};
}, [dispatch, timefilter]);

return {
indexPattern,
aggConfigs,
aggs: aggConfigs?.aggs ?? [],
timeRange,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,19 @@

import { countBy } from 'lodash';
import { Schemas } from '../../../../vis_default_editor/public';
import { RootState } from './state_management';
import { VisualizationState } from './state_management';

export const validateSchemaState = (schemas: Schemas, state: RootState): [boolean, string?] => {
const activeViz = state.visualization.activeVisualization;
/**
* Validate if the visualization state fits the vis type schema criteria
* @param schemas Visualization type config Schema objects
* @param state visualization state
* @returns [Validity, 'Message']
*/
export const validateSchemaState = (
schemas: Schemas,
state: VisualizationState
): [boolean, string?] => {
const activeViz = state.activeVisualization;
const vizName = activeViz?.name;
const aggs = activeViz?.aggConfigParams;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
import { validateSchemaState } from '../application/utils/validate_schema_state';
import { getExpressionLoader, getTypeService } from '../plugin_services';
import { PersistedState } from '../../../visualizations/public';
import { RenderState, VisualizationState } from '../application/utils/state_management';

// 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;
Expand Down Expand Up @@ -121,24 +122,24 @@ export class VisBuilderEmbeddable extends Embeddable<SavedObjectEmbeddableInput,
const { visualization, style } = this.serializedState;

const vizStateWithoutIndex = JSON.parse(visualization);
const visualizationState = {
const visualizationState: VisualizationState = {
searchField: vizStateWithoutIndex.searchField,
activeVisualization: vizStateWithoutIndex.activeVisualization,
indexPattern: this.savedVisBuilder?.searchSourceFields?.index,
};
const rootState = {
const renderState: RenderState = {
visualization: visualizationState,
style: JSON.parse(style),
};
const visualizationName = rootState.visualization?.activeVisualization?.name ?? '';
const visualizationName = renderState.visualization?.activeVisualization?.name ?? '';
const visualizationType = getTypeService().get(visualizationName);
if (!visualizationType) {
this.onContainerError(new Error(`Invalid visualization type ${visualizationName}`));
return;
}
const { toExpression, ui } = visualizationType;
const schemas = ui.containerConfig.data.schemas;
const [valid, errorMsg] = validateSchemaState(schemas, rootState);
const [valid, errorMsg] = validateSchemaState(schemas, visualizationState);

if (!valid) {
if (errorMsg) {
Expand All @@ -147,7 +148,11 @@ export class VisBuilderEmbeddable extends Embeddable<SavedObjectEmbeddableInput,
}
} else {
// TODO: handle error in Expression creation
const exp = await toExpression(rootState);
const exp = await toExpression(renderState, {
filters: this.filters,
query: this.query,
timeRange: this.timeRange,
});
return exp;
}
};
Expand Down Expand Up @@ -268,12 +273,15 @@ export class VisBuilderEmbeddable extends Embeddable<SavedObjectEmbeddableInput,
// Check if rootState has changed
if (!isEqual(this.getSerializedState(), this.serializedState)) {
this.serializedState = this.getSerializedState();
this.expression = (await this.getExpression()) ?? '';
dirty = true;
}

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

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

Expand Down
8 changes: 6 additions & 2 deletions src/plugins/vis_builder/public/services/type_service/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
*/
import { ReactElement } from 'react';
import { IconType } from '@elastic/eui';
import { RootState } from '../../application/utils/state_management';
import { RenderState } from '../../application/utils/state_management';
import { Schemas } from '../../../../vis_default_editor/public';
import { IExpressionLoaderParams } from '../../../../expressions/public';

export interface DataTabConfig {
schemas: Schemas;
Expand All @@ -28,5 +29,8 @@ export interface VisualizationTypeOptions<T = any> {
style: StyleTabConfig<T>;
};
};
readonly toExpression: (state: RootState) => Promise<string | undefined>;
readonly toExpression: (
state: RenderState,
searchContext: IExpressionLoaderParams['searchContext']
) => Promise<string | undefined>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { IconType } from '@elastic/eui';
import { RootState } from '../../application/utils/state_management';
import { IExpressionLoaderParams } from '../../../../expressions/public';
import { RenderState } from '../../application/utils/state_management';
import { VisualizationTypeOptions } from './types';

type IVisualizationType = VisualizationTypeOptions;
Expand All @@ -15,7 +16,10 @@ export class VisualizationType implements IVisualizationType {
public readonly icon: IconType;
public readonly stage: 'experimental' | 'production';
public readonly ui: IVisualizationType['ui'];
public readonly toExpression: (state: RootState) => Promise<string | undefined>;
public readonly toExpression: (
state: RenderState,
searchContext: IExpressionLoaderParams['searchContext']
) => Promise<string | undefined>;

constructor(options: VisualizationTypeOptions) {
this.name = options.name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { SchemaConfig } from '../../../../visualizations/public';
import { MetricVisExpressionFunctionDefinition } from '../../../../vis_type_metric/public';
import { AggConfigs, IAggConfig } from '../../../../data/common';
import { buildExpression, buildExpressionFunction } from '../../../../expressions/public';
import { RootState } from '../../application/utils/state_management';
import { RenderState } from '../../application/utils/state_management';
import { MetricOptionsDefaults } from './metric_viz_type';
import { getAggExpressionFunctions } from '../common/expression_helpers';

Expand Down Expand Up @@ -82,7 +82,7 @@ const getVisSchemas = (aggConfigs: AggConfigs): any => {
return schemas;
};

export interface MetricRootState extends RootState {
export interface MetricRootState extends RenderState {
style: MetricOptionsDefaults;
}

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

import { buildVislibDimensions } from '../../../../../visualizations/public';
import { buildExpression, buildExpressionFunction } from '../../../../../expressions/public';
import {
buildExpression,
buildExpressionFunction,
IExpressionLoaderParams,
} from '../../../../../expressions/public';
import { AreaOptionsDefaults } from './area_vis_type';
import { getAggExpressionFunctions } from '../../common/expression_helpers';
import { VislibRootState, getValueAxes, getPipelineParams } from '../common';
import { createVis } from '../common/create_vis';

export const toExpression = async ({
style: styleState,
visualization,
}: VislibRootState<AreaOptionsDefaults>) => {
export const toExpression = async (
{ style: styleState, visualization }: VislibRootState<AreaOptionsDefaults>,
searchContext: IExpressionLoaderParams['searchContext']
) => {
const { aggConfigs, expressionFns, indexPattern } = await getAggExpressionFunctions(
visualization
);
const { addLegend, addTooltip, legendPosition, type } = styleState;

const vis = await createVis(type, aggConfigs, indexPattern);
const vis = await createVis(type, aggConfigs, indexPattern, searchContext?.timeRange);

const params = getPipelineParams();
const dimensions = await buildVislibDimensions(vis, params);
Expand Down
Loading

0 comments on commit df3e965

Please sign in to comment.