Skip to content

Commit

Permalink
Finalize eligibility check for augmenting visualizations by vis augme…
Browse files Browse the repository at this point in the history 
…nter (#3687)

* Add elibility checker for visualizations that can allow vis_augmenter to augment

Signed-off-by: Ashish Agrawal <ashisagr@amazon.com>

---------

Signed-off-by: Ashish Agrawal <ashisagr@amazon.com>
  • Loading branch information
@lezzago
lezzago committed May 10, 2023
1 parent 03b393f commit 609bcc1
Show file tree
Hide file tree
Showing 9 changed files with 383 additions and 28 deletions.
1 change: 1 addition & 0 deletions src/plugins/vis_augmenter/public/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ export * from './utils';
export * from './constants';
export * from './vega';
export * from './saved_augment_vis';
export * from './test_constants';
33 changes: 33 additions & 0 deletions src/plugins/vis_augmenter/public/test_constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import { OpenSearchDashboardsDatatable } from '../../expressions/public';
import { VIS_LAYER_COLUMN_TYPE, VisLayerTypes, HOVER_PARAM } from './';

const TEST_X_AXIS_ID = 'test-x-axis-id';
const TEST_X_AXIS_ID_DIRTY = 'test.x.axis.id';
const TEST_VALUE_AXIS_ID = 'test-value-axis-id';
const TEST_VALUE_AXIS_ID_DIRTY = 'test.value.axis.id';
const TEST_X_AXIS_TITLE = 'time';
const TEST_VALUE_AXIS_TITLE = 'avg value';
const TEST_PLUGIN = 'test-plugin';
Expand Down Expand Up @@ -53,6 +55,20 @@ const TEST_VALUES_NO_VIS_LAYERS = [
{ [TEST_X_AXIS_ID]: 50, [TEST_VALUE_AXIS_ID]: 5 },
];

const TEST_VALUES_NO_VIS_LAYERS_DIRTY = [
{ [TEST_X_AXIS_ID_DIRTY]: 0, [TEST_VALUE_AXIS_ID_DIRTY]: 5 },
{ [TEST_X_AXIS_ID_DIRTY]: 5, [TEST_VALUE_AXIS_ID_DIRTY]: 10 },
{ [TEST_X_AXIS_ID_DIRTY]: 10, [TEST_VALUE_AXIS_ID_DIRTY]: 6 },
{ [TEST_X_AXIS_ID_DIRTY]: 15, [TEST_VALUE_AXIS_ID_DIRTY]: 4 },
{ [TEST_X_AXIS_ID_DIRTY]: 20, [TEST_VALUE_AXIS_ID_DIRTY]: 5 },
{ [TEST_X_AXIS_ID_DIRTY]: 25 },
{ [TEST_X_AXIS_ID_DIRTY]: 30 },
{ [TEST_X_AXIS_ID_DIRTY]: 35 },
{ [TEST_X_AXIS_ID_DIRTY]: 40 },
{ [TEST_X_AXIS_ID_DIRTY]: 45, [TEST_VALUE_AXIS_ID_DIRTY]: 3 },
{ [TEST_X_AXIS_ID_DIRTY]: 50, [TEST_VALUE_AXIS_ID_DIRTY]: 5 },
];

const TEST_VALUES_SINGLE_VIS_LAYER = [
{ [TEST_X_AXIS_ID]: 0, [TEST_VALUE_AXIS_ID]: 5 },
{ [TEST_X_AXIS_ID]: 5, [TEST_VALUE_AXIS_ID]: 10, [TEST_PLUGIN_RESOURCE_ID]: 2 },
Expand Down Expand Up @@ -111,6 +127,17 @@ export const TEST_COLUMNS_NO_VIS_LAYERS = [
},
];

export const TEST_COLUMNS_NO_VIS_LAYERS_DIRTY = [
{
id: TEST_X_AXIS_ID_DIRTY,
name: TEST_X_AXIS_TITLE,
},
{
id: TEST_VALUE_AXIS_ID_DIRTY,
name: TEST_VALUE_AXIS_TITLE,
},
];

export const TEST_COLUMNS_SINGLE_VIS_LAYER = [
...TEST_COLUMNS_NO_VIS_LAYERS,
{
Expand Down Expand Up @@ -157,6 +184,12 @@ export const TEST_DATATABLE_NO_VIS_LAYERS = {
rows: TEST_VALUES_NO_VIS_LAYERS,
} as OpenSearchDashboardsDatatable;

export const TEST_DATATABLE_NO_VIS_LAYERS_DIRTY = {
type: 'opensearch_dashboards_datatable',
columns: TEST_COLUMNS_NO_VIS_LAYERS_DIRTY,
rows: TEST_VALUES_NO_VIS_LAYERS_DIRTY,
} as OpenSearchDashboardsDatatable;

export const TEST_DATATABLE_SINGLE_VIS_LAYER_EMPTY = {
...TEST_DATATABLE_NO_VIS_LAYERS,
columns: TEST_COLUMNS_SINGLE_VIS_LAYER,
Expand Down
259 changes: 250 additions & 9 deletions src/plugins/vis_augmenter/public/utils/utils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,37 +9,278 @@ import {
getAugmentVisSavedObjs,
getAnyErrors,
isEligibleForVisLayers,
} from './utils';
import {
createSavedAugmentVisLoader,
SavedObjectOpenSearchDashboardsServicesWithAugmentVis,
getMockAugmentVisSavedObjectClient,
generateAugmentVisSavedObject,
ISavedAugmentVis,
VisLayerExpressionFn,
generateVisLayer,
VisLayerTypes,
VisLayerExpressionFn,
} from '../';
import { generateVisLayer } from './';
import { AggConfigs, AggTypesRegistryStart, IndexPattern } from '../../../data/common';
import { mockAggTypesRegistry } from '../../../data/common/search/aggs/test_helpers';

describe('utils', () => {
// TODO: redo / update this test suite when eligibility is finalized.
// Tracked in https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3268
describe('isEligibleForVisLayers', () => {
it('vis is ineligible with invalid type', async () => {
const validConfigStates = [
{
enabled: true,
type: 'max',
params: {},
schema: 'metric',
},
{
enabled: true,
type: 'date_histogram',
params: {},
schema: 'segment',
},
];
const stubIndexPatternWithFields = {
id: '1234',
title: 'logstash-*',
fields: [
{
name: 'response',
type: 'number',
esTypes: ['integer'],
aggregatable: true,
filterable: true,
searchable: true,
},
],
};
const typesRegistry: AggTypesRegistryStart = mockAggTypesRegistry();
const aggs = new AggConfigs(stubIndexPatternWithFields as IndexPattern, validConfigStates, {
typesRegistry,
});
const validVis = ({
params: {
type: 'line',
seriesParams: [
{
type: 'line',
},
],
categoryAxes: [
{
position: 'bottom',
},
],
},
data: {
aggs,
},
} as unknown) as Vis;
it('vis is ineligible with invalid non-line type', async () => {
const vis = ({
params: {
type: 'not-line',
seriesParams: [],
categoryAxes: [
{
position: 'bottom',
},
],
},
data: {
aggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is eligible with valid type', async () => {
it('vis is ineligible with no date_histogram', async () => {
const invalidConfigStates = [
{
enabled: true,
type: 'histogram',
params: {},
},
{
enabled: true,
type: 'metrics',
params: {},
},
];
const invalidAggs = new AggConfigs(
stubIndexPatternWithFields as IndexPattern,
invalidConfigStates,
{
typesRegistry,
}
);
const vis = ({
params: {
type: 'line',
seriesParams: [],
},
data: {
invalidAggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with invalid aggs counts', async () => {
const invalidConfigStates = [
...validConfigStates,
{
enabled: true,
type: 'dot',
params: {},
schema: 'radius',
},
];
const invalidAggs = new AggConfigs(
stubIndexPatternWithFields as IndexPattern,
invalidConfigStates,
{
typesRegistry,
}
);
const vis = ({
params: {
type: 'line',
seriesParams: [],
},
data: {
invalidAggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis)).toEqual(true);
expect(isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with no metric aggs', async () => {
const invalidConfigStates = [
{
enabled: true,
type: 'date_histogram',
params: {},
},
];
const invalidAggs = new AggConfigs(
stubIndexPatternWithFields as IndexPattern,
invalidConfigStates,
{
typesRegistry,
}
);
const vis = ({
params: {
type: 'line',
seriesParams: [],
},
data: {
invalidAggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with series param is not line type', async () => {
const vis = ({
params: {
type: 'line',
seriesParams: [
{
type: 'area',
},
],
categoryAxes: [
{
position: 'bottom',
},
],
},
data: {
aggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with series param not all being line type', async () => {
const vis = ({
params: {
type: 'line',
seriesParams: [
{
type: 'area',
},
{
type: 'line',
},
],
categoryAxes: [
{
position: 'bottom',
},
],
},
data: {
aggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with invalid x-axis due to no segment aggregation', async () => {
const badConfigStates = [
{
enabled: true,
type: 'max',
params: {},
schema: 'metric',
},
{
enabled: true,
type: 'max',
params: {},
schema: 'metric',
},
];
const badAggs = new AggConfigs(stubIndexPatternWithFields as IndexPattern, badConfigStates, {
typesRegistry,
});
const invalidVis = ({
params: {
type: 'line',
seriesParams: [
{
type: 'line',
},
],
categoryAxes: [
{
position: 'bottom',
},
],
},
data: {
badAggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(invalidVis)).toEqual(false);
});
it('vis is ineligible with xaxis not on bottom', async () => {
const invalidVis = ({
params: {
type: 'line',
seriesParams: [
{
type: 'line',
},
],
categoryAxes: [
{
position: 'top',
},
],
},
data: {
aggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(invalidVis)).toEqual(false);
});
it('vis is eligible with valid type', async () => {
expect(isEligibleForVisLayers(validVis)).toEqual(true);
});
});

Expand Down
22 changes: 19 additions & 3 deletions src/plugins/vis_augmenter/public/utils/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,26 @@ import {
isVisLayerWithError,
} from '../';

// TODO: provide a deeper eligibility check.
// Tracked in https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3268
export const isEligibleForVisLayers = (vis: Vis): boolean => {
return vis.params.type === 'line';
// Only support date histogram and ensure there is only 1 x-axis and it has to be on the bottom.
// Additionally to have a valid x-axis, there needs to be a segment aggregation
const hasValidXaxis =
vis.data.aggs !== undefined &&
vis.data.aggs?.byTypeName('date_histogram').length === 1 &&
vis.params.categoryAxes.length === 1 &&
vis.params.categoryAxes[0].position === 'bottom' &&
vis.data.aggs?.bySchemaName('segment').length > 0;
// Support 1 segment for x axis bucket (that is date_histogram) and support metrics for
// multiple supported yaxis only. If there are other aggregation types, this is not
// valid for augmentation
const hasCorrectAggregationCount =
vis.data.aggs !== undefined &&
vis.data.aggs?.bySchemaName('metric').length > 0 &&
vis.data.aggs?.bySchemaName('metric').length === vis.data.aggs?.aggs.length - 1;
const hasOnlyLineSeries =
vis.params.seriesParams.every((seriesParam: { type: string }) => seriesParam.type === 'line') &&
vis.params.type === 'line';
return hasValidXaxis && hasCorrectAggregationCount && hasOnlyLineSeries;
};

/**
Expand Down
Loading

0 comments on commit 609bcc1

Please sign in to comment.