Skip to content

Commit

Permalink
Resolve nitpicks and remove dimensions
Browse files Browse the repository at this point in the history 
Signed-off-by: Ashish Agrawal <ashisagr@amazon.com>
  • Loading branch information
@lezzago
lezzago committed May 5, 2023
1 parent 6ad721d commit 6ceeb3d
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 59 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';
109 changes: 72 additions & 37 deletions src/plugins/vis_augmenter/public/utils/utils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { Vis, VislibDimensions } from '../../../visualizations/public';
import { Vis } from '../../../visualizations/public';
import {
buildPipelineFromAugmentVisSavedObjs,
getAugmentVisSavedObjs,
Expand All @@ -23,18 +23,7 @@ import { mockAggTypesRegistry } from '../../../data/common/search/aggs/test_help

describe('utils', () => {
describe('isEligibleForVisLayers', () => {
const validDimensions = {
x: {
params: {
bounds: {
min: '2023-03-22T21:55:12.455Z',
max: '2023-03-23T21:55:12.455Z',
},
},
label: 'order_date per 30 minutes',
},
} as VislibDimensions;
const configStates = [
const validConfigStates = [
{
enabled: true,
type: 'max',
Expand Down Expand Up @@ -63,7 +52,7 @@ describe('utils', () => {
],
};
const typesRegistry: AggTypesRegistryStart = mockAggTypesRegistry();
const aggs = new AggConfigs(stubIndexPatternWithFields as IndexPattern, configStates, {
const aggs = new AggConfigs(stubIndexPatternWithFields as IndexPattern, validConfigStates, {
typesRegistry,
});
const validVis = ({
Expand Down Expand Up @@ -99,7 +88,7 @@ describe('utils', () => {
aggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis, validDimensions)).toEqual(false);
expect(isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with no date_histogram', async () => {
const invalidConfigStates = [
Expand Down Expand Up @@ -130,27 +119,17 @@ describe('utils', () => {
invalidAggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis, validDimensions)).toEqual(false);
expect(isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with invalid aggs counts', async () => {
const invalidConfigStates = [
{
enabled: true,
type: 'date_histogram',
params: {},
},
...validConfigStates,
{
enabled: true,
type: 'dot',
params: {},
schema: 'radius',
},
{
enabled: true,
type: 'metrics',
params: {},
schema: 'metrics',
},
];
const invalidAggs = new AggConfigs(
stubIndexPatternWithFields as IndexPattern,
Expand All @@ -168,7 +147,7 @@ describe('utils', () => {
invalidAggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis, validDimensions)).toEqual(false);
expect(isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with no metric aggs', async () => {
const invalidConfigStates = [
Expand All @@ -194,7 +173,7 @@ describe('utils', () => {
invalidAggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis, validDimensions)).toEqual(false);
expect(isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with series param is not line type', async () => {
const vis = ({
Expand All @@ -215,13 +194,69 @@ describe('utils', () => {
aggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis, validDimensions)).toEqual(false);
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 dimensions', async () => {
const invalidDimensions = {
x: null,
} as VislibDimensions;
expect(isEligibleForVisLayers(validVis, invalidDimensions)).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 = ({
Expand All @@ -242,10 +277,10 @@ describe('utils', () => {
aggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(invalidVis, validDimensions)).toEqual(false);
expect(isEligibleForVisLayers(invalidVis)).toEqual(false);
});
it('vis is eligible with valid type', async () => {
expect(isEligibleForVisLayers(validVis, validDimensions)).toEqual(true);
expect(isEligibleForVisLayers(validVis)).toEqual(true);
});
});

Expand Down
23 changes: 12 additions & 11 deletions src/plugins/vis_augmenter/public/utils/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { get, isEmpty } from 'lodash';
import { Vis, VislibDimensions } from '../../../../plugins/visualizations/public';
import { Vis } from '../../../../plugins/visualizations/public';
import {
formatExpression,
buildExpressionFunction,
Expand All @@ -19,25 +19,26 @@ import {
isVisLayerWithError,
} from '../';

export const isEligibleForVisLayers = (vis: Vis, dimensions: VislibDimensions): boolean => {
// Only support date histogram and ensure there is only 1 x-axis and it has to be on the bottom
const isValidXaxis =
export const isEligibleForVisLayers = (vis: Vis): boolean => {
// 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.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;
let isOnlyLine = vis.params.type === 'line';
vis.params.seriesParams.forEach((seriesParam: { type: string }) => {
isOnlyLine = isOnlyLine && seriesParam.type === 'line';
});
const isValidDimensions = dimensions.x !== null;
return isValidXaxis && hasCorrectAggregationCount && isOnlyLine && isValidDimensions;
const hasOnlyLineSeries =
vis.params.seriesParams.every((seriesParam: { type: string }) => seriesParam.type === 'line') &&
vis.params.type === 'line';
return hasValidXaxis && hasCorrectAggregationCount && hasOnlyLineSeries;
};

/**
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_type_vega/public/expressions/helpers.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
import {
TEST_DATATABLE_NO_VIS_LAYERS,
TEST_DATATABLE_NO_VIS_LAYERS_DIRTY,
} from '../../../vis_augmenter/public/test_constants';
} from '../../../vis_augmenter/public';

describe('helpers', function () {
describe('formatDatatable()', function () {
Expand Down
14 changes: 6 additions & 8 deletions src/plugins/vis_type_vega/public/expressions/helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,13 @@ export const formatDatatable = (
});

// clean row keys to remove "." as that will cause vega to not process it correctly
const updatedRows: OpenSearchDashboardsDatatableRow[] = [];
datatable.rows.forEach((row) => {
const newRow = {};
for (const [key, value] of Object.entries(row)) {
const updatedRows: OpenSearchDashboardsDatatableRow[] = datatable.rows.map((row) =>
Object.entries(row).reduce((updatedRow, [key, value]) => {
const cleanKey = key.replaceAll('.', '-');
Object.assign(newRow, { [cleanKey]: value });
}
updatedRows.push(newRow);
});
return Object.assign(updatedRow, { [cleanKey]: value });
}, {})
);

datatable.rows = updatedRows;
return datatable;
};
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/vis_type_vislib/public/line_to_expression.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ export const toExpressionAst = async (vis: Vis, params: any) => {
// Checks if there are vislayers to overlay. If not, default to the vislib implementation.
const dimensions: VislibDimensions = await buildVislibDimensions(vis, params);
if (
!isEligibleForVisLayers(vis, dimensions) ||
params.visLayers == null ||
Object.keys(params.visLayers).length === 0
Object.keys(params.visLayers).length === 0 ||
!isEligibleForVisLayers(vis)
) {
// Render using vislib instead of vega-lite
const visConfig = { ...vis.params, dimensions };
Expand Down

0 comments on commit 6ceeb3d

Please sign in to comment.