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

[VisBuilder-Next] Pie Chart Integration for VisBuilder #7752

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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/7752.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- [VisBuilder-Next] Pie Chart Integration for VisBuilder ([#7752](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7752))
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import { replaceFieldInConfiguration } from './drag_drop/replace_field_in_configuration';
import { reorderFieldsWithinSchema } from './drag_drop/reorder_fields_within_schema';
import { moveFieldBetweenSchemas } from './drag_drop/move_field_between_schemas';
import { validateAggregations } from '../../utils/validations';

export const DATA_TAB_ID = 'data_tab';

Expand All @@ -39,6 +40,7 @@
data: {
search: { aggs: aggService },
},
notifications: { toasts },
},
} = useOpenSearchDashboards<VisBuilderServices>();

Expand Down Expand Up @@ -76,6 +78,18 @@

const panelGroups = Array.from(schemas.all.map((schema) => schema.name));

// Check schema order
if (destinationSchemaName === 'split') {
const validationResult = validateAggregations(aggProps.aggs, schemas.all);

Check warning on line 83 in src/plugins/vis_builder/public/application/components/data_tab/index.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_builder/public/application/components/data_tab/index.tsx#L83

Added line #L83 was not covered by tests
if (!validationResult.valid) {
toasts.addWarning({

Check warning on line 85 in src/plugins/vis_builder/public/application/components/data_tab/index.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_builder/public/application/components/data_tab/index.tsx#L85

Added line #L85 was not covered by tests
title: 'vb_invalid_schema',
text: validationResult.errorMsg,
});
return;

Check warning on line 89 in src/plugins/vis_builder/public/application/components/data_tab/index.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_builder/public/application/components/data_tab/index.tsx#L89

Added line #L89 was not covered by tests
}
}

if (Object.values(FIELD_SELECTOR_ID).includes(sourceSchemaName as FIELD_SELECTOR_ID)) {
if (panelGroups.includes(destinationSchemaName) && !combine) {
addFieldToConfiguration({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,80 @@ describe('validateAggregations', () => {
expect(valid).toBe(true);
expect(errorMsg).not.toBeDefined();
});

test('Split chart should be first in the configuration', () => {
const aggConfigs = dataStart.search.aggs.createAggConfigs(indexPattern as IndexPattern, [
{
id: '0',
enabled: true,
type: BUCKET_TYPES.TERMS,
schema: 'group',
params: {},
},
{
id: '1',
enabled: true,
type: BUCKET_TYPES.TERMS,
schema: 'split',
params: {},
},
]);

const schemas = [{ name: 'split', mustBeFirst: true }, { name: 'group' }];

const { valid, errorMsg } = validateAggregations(aggConfigs.aggs, schemas);

expect(valid).toBe(false);
expect(errorMsg).toMatchInlineSnapshot(`"Split chart must be first in the configuration."`);
});

test('Valid configuration with split chart first', () => {
const aggConfigs = dataStart.search.aggs.createAggConfigs(indexPattern as IndexPattern, [
{
id: '0',
enabled: true,
type: BUCKET_TYPES.TERMS,
schema: 'split',
params: {},
},
{
id: '2',
enabled: true,
type: METRIC_TYPES.COUNT,
schema: 'metric',
params: {},
},
]);

const schemas = [{ name: 'split', mustBeFirst: true }, { name: 'metric' }];

const { valid, errorMsg } = validateAggregations(aggConfigs.aggs, schemas);

expect(valid).toBe(true);
expect(errorMsg).toBeUndefined();
});

test('Valid configuration when schemas are not provided', () => {
const aggConfigs = dataStart.search.aggs.createAggConfigs(indexPattern as IndexPattern, [
{
id: '0',
enabled: true,
type: BUCKET_TYPES.TERMS,
schema: 'group',
params: {},
},
{
id: '1',
enabled: true,
type: BUCKET_TYPES.TERMS,
schema: 'split',
params: {},
},
]);

const { valid, errorMsg } = validateAggregations(aggConfigs.aggs);

expect(valid).toBe(true);
expect(errorMsg).not.toBeDefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import { ValidationResult } from './types';
/**
* Validate if the aggregations to perform are possible
* @param aggs Aggregations to be performed
* @param schemas Optional. All available schemas
* @returns ValidationResult
*/
export const validateAggregations = (aggs: AggConfig[]): ValidationResult => {
export const validateAggregations = (aggs: AggConfig[], schemas?: any[]): ValidationResult => {
// Pipeline aggs should have a valid bucket agg
const metricAggs = aggs.filter((agg) => agg.schema === 'metric');
const lastParentPipelineAgg = findLast(
Expand Down Expand Up @@ -50,5 +51,18 @@ export const validateAggregations = (aggs: AggConfig[]): ValidationResult => {
};
}

const splitSchema = schemas?.find((s) => s.name === 'split');
if (splitSchema && splitSchema.mustBeFirst) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit:

Suggested change
if (splitSchema && splitSchema.mustBeFirst) {
if (splitSchema?.mustBeFirst) {

const firstGroupSchemaIndex = aggs.findIndex((item) => item.schema === 'group');
if (firstGroupSchemaIndex !== -1) {
return {
valid: false,
errorMsg: i18n.translate('visBuilder.aggregation.splitChartOrderErrorMessage', {
defaultMessage: 'Split chart must be first in the configuration.',
}),
};
}
}

return { valid: true };
};
3 changes: 2 additions & 1 deletion src/plugins/vis_builder/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import {
} from '../../opensearch_dashboards_utils/public';
import { opensearchFilters } from '../../data/public';
import { createRawDataVisFn } from './visualizations/vega/utils/expression_helper';
import { VISBUILDER_ENABLE_VEGA_SETTING } from '../common/constants';

export class VisBuilderPlugin
implements
Expand Down Expand Up @@ -107,7 +108,7 @@ export class VisBuilderPlugin

// Register Default Visualizations
const typeService = this.typeService;
registerDefaultTypes(typeService.setup());
registerDefaultTypes(typeService.setup(), core.uiSettings.get(VISBUILDER_ENABLE_VEGA_SETTING));
exp.registerFunction(createRawDataVisFn());

// Register the plugin to core
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,5 @@ export interface VisualizationTypeOptions<T = any> {
searchContext: IExpressionLoaderParams['searchContext'],
useVega: boolean
) => Promise<string | undefined>;
readonly hierarchicalData?: boolean | ((vis: { params: T }) => boolean);
}
15 changes: 12 additions & 3 deletions src/plugins/vis_builder/public/visualizations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,26 @@
import type { TypeServiceSetup } from '../services/type_service';
import { createMetricConfig } from './metric';
import { createTableConfig } from './table';
import { createHistogramConfig, createLineConfig, createAreaConfig } from './vislib';
import {
createHistogramConfig,
createLineConfig,
createAreaConfig,
createPieConfig,
} from './vislib';

export function registerDefaultTypes(typeServiceSetup: TypeServiceSetup) {
const visualizationTypes = [
export function registerDefaultTypes(typeServiceSetup: TypeServiceSetup, useVega: boolean) {
const defaultVisualizationTypes = [
createHistogramConfig,
createLineConfig,
createAreaConfig,
createMetricConfig,
createTableConfig,
];

const visualizationTypes = useVega
? [...defaultVisualizationTypes, createPieConfig]
: defaultVisualizationTypes;

Comment on lines +17 to +28
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not just

Suggested change
const defaultVisualizationTypes = [
createHistogramConfig,
createLineConfig,
createAreaConfig,
createMetricConfig,
createTableConfig,
];
const visualizationTypes = useVega
? [...defaultVisualizationTypes, createPieConfig]
: defaultVisualizationTypes;
const visualizationTypes = [
createHistogramConfig,
createLineConfig,
createAreaConfig,
createMetricConfig,
createTableConfig,
];
if (useVega) visualizationTypes.push(createPieConfig);

visualizationTypes.forEach((createTypeConfig) => {
typeServiceSetup.createVisualizationType(createTypeConfig());
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { AxisFormats } from '../utils/types';
import { buildAxes } from './axes';
import { buildAxes } from '../axes';

export type VegaMarkType =
| 'line'
Expand Down Expand Up @@ -87,7 +87,7 @@ export const buildMarkForVegaLite = (vegaType: string): VegaLiteMark => {
};

/**
* Builds a mark configuration for Vega based on the chart type.
* Builds a mark configuration for Vega useing series data based on the chart type.
*
* @param {VegaMarkType} chartType - The type of chart to build the mark for.
* @param {any} dimensions - The dimensions of the data.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { buildSlicesMarkForVega, buildPieMarks, buildArcMarks } from './mark_slices';

describe('buildSlicesMarkForVega', () => {
it('should return a group mark with correct properties', () => {
const result = buildSlicesMarkForVega(['level1', 'level2'], true, true);
expect(result.type).toBe('group');
expect(result.from).toEqual({ data: 'splits' });
expect(result.encode.enter.width).toEqual({ signal: 'chartWidth' });
expect(result.title).toBeDefined();
expect(result.data).toBeDefined();
expect(result.marks).toBeDefined();
});

it('should handle non-split case correctly', () => {
const result = buildSlicesMarkForVega(['level1'], false, true);
expect(result.from).toBeNull();
expect(result.encode.enter.width).toEqual({ signal: 'width' });
expect(result.title).toBeNull();
});
});

describe('buildPieMarks', () => {
it('should create correct number of marks', () => {
const result = buildPieMarks(['level1', 'level2'], true);
expect(result).toHaveLength(2);
});

it('should create correct transformations', () => {
const result = buildPieMarks(['level1'], true);
expect(result[0].transform).toHaveLength(3);
expect(result[0].transform[0].type).toBe('filter');
expect(result[0].transform[1].type).toBe('aggregate');
expect(result[0].transform[2].type).toBe('pie');
});
});

describe('buildArcMarks', () => {
it('should create correct number of arc marks', () => {
const result = buildArcMarks(['level1', 'level2'], false);
expect(result).toHaveLength(2);
expect(result[0].encode.update.tooltip).toBeUndefined();
});

it('should create arc marks with correct properties', () => {
const result = buildArcMarks(['level1'], true);
expect(result[0].type).toBe('arc');
expect(result[0].encode.enter.fill).toBeDefined();
expect(result[0].encode.update.startAngle).toBeDefined();
expect(result[0].encode.update.tooltip).toBeDefined();
});
});
Loading
Loading