Skip to content

Commit

Permalink
[Lens] Make average the default suggested metric (elastic#63416)
Browse files Browse the repository at this point in the history
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
2 people authored and wylieconlon committed Apr 16, 2020
1 parent f98928c commit 312e687
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 113 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,16 @@ export function onDrop(
operationsForNewField &&
operationsForNewField.includes(selectedColumn.operationType);

if (!operationsForNewField || operationsForNewField.length === 0) {
return false;
}

// If only the field has changed use the onFieldChange method on the operation to get the
// new column, otherwise use the regular buildColumn to get a new column.
const newColumn = hasFieldChanged
? changeField(selectedColumn, currentIndexPattern, droppedItem.field)
: buildColumn({
op: operationsForNewField ? operationsForNewField[0] : undefined,
op: operationsForNewField[0],
columns: props.state.layers[props.layerId].columns,
indexPattern: currentIndexPattern,
layerId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
getOperationTypesForField,
operationDefinitionMap,
IndexPatternColumn,
OperationType,
} from './operations';
import { operationDefinitions } from './operations/definitions';
import { hasField } from './utils';
Expand Down Expand Up @@ -141,7 +142,13 @@ function getExistingLayerSuggestionsForField(
suggestions.push(
buildSuggestion({
state,
updatedLayer: addFieldAsBucketOperation(layer, layerId, indexPattern, field),
updatedLayer: addFieldAsBucketOperation(
layer,
layerId,
indexPattern,
field,
usableAsBucketOperation
),
layerId,
changeType: 'extended',
})
Expand Down Expand Up @@ -176,26 +183,7 @@ function addFieldAsMetricOperation(
indexPattern: IndexPattern,
field: IndexPatternField
): IndexPatternLayer | undefined {
const operations = getOperationTypesForField(field);
const operationsAlreadyAppliedToThisField = Object.values(layer.columns)
.filter(column => hasField(column) && column.sourceField === field.name)
.map(column => column.operationType);
const operationCandidate = operations.find(
operation => !operationsAlreadyAppliedToThisField.includes(operation)
);

if (!operationCandidate) {
return;
}

const newColumn = buildColumn({
op: operationCandidate,
columns: layer.columns,
layerId,
indexPattern,
suggestedPriority: undefined,
field,
});
const newColumn = getMetricColumn(indexPattern, layerId, field);
const addedColumnId = generateId();

const [, metrics] = separateBucketColumns(layer);
Expand Down Expand Up @@ -226,11 +214,11 @@ function addFieldAsBucketOperation(
layer: IndexPatternLayer,
layerId: string,
indexPattern: IndexPattern,
field: IndexPatternField
field: IndexPatternField,
operation: OperationType
): IndexPatternLayer {
const applicableBucketOperation = getBucketOperation(field);
const newColumn = buildColumn({
op: applicableBucketOperation,
op: operation,
columns: layer.columns,
layerId,
indexPattern,
Expand All @@ -252,15 +240,15 @@ function addFieldAsBucketOperation(

let updatedColumnOrder: string[] = [];
if (oldDateHistogramId) {
if (applicableBucketOperation === 'terms') {
if (operation === 'terms') {
// Insert the new terms bucket above the first date histogram
updatedColumnOrder = [
...buckets.slice(0, oldDateHistogramIndex),
newColumnId,
...buckets.slice(oldDateHistogramIndex, buckets.length),
...metrics,
];
} else if (applicableBucketOperation === 'date_histogram') {
} else if (operation === 'date_histogram') {
// Replace date histogram with new date histogram
delete updatedColumns[oldDateHistogramId];
updatedColumnOrder = layer.columnOrder.map(columnId =>
Expand All @@ -287,8 +275,9 @@ function getEmptyLayerSuggestionsForField(
): IndexPatternSugestion[] {
const indexPattern = state.indexPatterns[indexPatternId];
let newLayer: IndexPatternLayer | undefined;
if (getBucketOperation(field)) {
newLayer = createNewLayerWithBucketAggregation(layerId, indexPattern, field);
const bucketOperation = getBucketOperation(field);
if (bucketOperation) {
newLayer = createNewLayerWithBucketAggregation(layerId, indexPattern, field, bucketOperation);
} else if (indexPattern.timeFieldName && getOperationTypesForField(field).length > 0) {
newLayer = createNewLayerWithMetricAggregation(layerId, indexPattern, field);
}
Expand All @@ -312,7 +301,8 @@ function getEmptyLayerSuggestionsForField(
function createNewLayerWithBucketAggregation(
layerId: string,
indexPattern: IndexPattern,
field: IndexPatternField
field: IndexPatternField,
operation: OperationType
): IndexPatternLayer {
const countColumn = buildColumn({
op: 'count',
Expand All @@ -329,7 +319,7 @@ function createNewLayerWithBucketAggregation(
// let column know about count column
const column = buildColumn({
layerId,
op: getBucketOperation(field),
op: operation,
indexPattern,
columns: {
[col2]: countColumn,
Expand All @@ -355,15 +345,7 @@ function createNewLayerWithMetricAggregation(
): IndexPatternLayer {
const dateField = indexPattern.fields.find(f => f.name === indexPattern.timeFieldName)!;

const operations = getOperationTypesForField(field);
const column = buildColumn({
op: operations[0],
columns: {},
suggestedPriority: undefined,
field,
indexPattern,
layerId,
});
const column = getMetricColumn(indexPattern, layerId, field);

const dateColumn = buildColumn({
op: 'date_histogram',
Expand Down Expand Up @@ -500,12 +482,7 @@ function createChangedNestingSuggestion(state: IndexPatternPrivateState, layerId
});
}

function createMetricSuggestion(
indexPattern: IndexPattern,
layerId: string,
state: IndexPatternPrivateState,
field: IndexPatternField
) {
function getMetricColumn(indexPattern: IndexPattern, layerId: string, field: IndexPatternField) {
const operationDefinitionsMap = _.indexBy(operationDefinitions, 'type');
const [column] = getOperationTypesForField(field)
.map(type =>
Expand All @@ -518,6 +495,16 @@ function createMetricSuggestion(
})
)
.filter(op => (op.dataType === 'number' || op.dataType === 'document') && !op.isBucketed);
return column;
}

function createMetricSuggestion(
indexPattern: IndexPattern,
layerId: string,
state: IndexPatternPrivateState,
field: IndexPatternField
) {
const column = getMetricColumn(indexPattern, layerId, field);

if (!column) {
return;
Expand Down Expand Up @@ -572,21 +559,26 @@ function createAlternativeMetricSuggestions(
return;
}
const field = indexPattern.fields.find(({ name }) => column.sourceField === name)!;
const alternativeMetricOperations = getOperationTypesForField(field).filter(
operationType => operationType !== column.operationType
);
const alternativeMetricOperations = getOperationTypesForField(field)
.map(op =>
buildColumn({
op,
columns: layer.columns,
indexPattern,
layerId,
field,
suggestedPriority: undefined,
})
)
.filter(
fullOperation =>
fullOperation.operationType !== column.operationType && !fullOperation.isBucketed
);
if (alternativeMetricOperations.length === 0) {
return;
}
const newId = generateId();
const newColumn = buildColumn({
op: alternativeMetricOperations[0],
columns: layer.columns,
indexPattern,
layerId,
field,
suggestedPriority: undefined,
});
const newColumn = alternativeMetricOperations[0];
const updatedLayer = {
indexPatternId: indexPattern.id,
columns: { [newId]: newColumn },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export const dateHistogramOperation: OperationDefinition<DateHistogramIndexPatte
displayName: i18n.translate('xpack.lens.indexPattern.dateHistogram', {
defaultMessage: 'Date histogram',
}),
priority: 3, // Higher than any metric
getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type }) => {
if (
type === 'date' &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export const termsOperation: OperationDefinition<TermsIndexPatternColumn> = {
displayName: i18n.translate('xpack.lens.indexPattern.terms', {
defaultMessage: 'Top values',
}),
priority: 3, // Higher than any metric
getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type }) => {
if (
supportedTypes.has(type) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
*/

import { getOperationTypesForField, getAvailableOperationsByMetadata, buildColumn } from './index';
import { AvgIndexPatternColumn, MinIndexPatternColumn } from './definitions/metrics';
import { CountIndexPatternColumn } from './definitions/count';
import { AvgIndexPatternColumn } from './definitions/metrics';
import { IndexPatternPrivateState } from '../types';
import { documentField } from '../document_field';

Expand Down Expand Up @@ -197,33 +196,21 @@ describe('getOperationTypesForField', () => {
expect(column.operationType).toEqual('avg');
expect(column.sourceField).toEqual(field.name);
});

it('should pick a suitable field operation if none is passed in', () => {
const field = expectedIndexPatterns[1].fields[1];
const column = buildColumn({
layerId: 'first',
indexPattern: expectedIndexPatterns[1],
columns: state.layers.first.columns,
suggestedPriority: 0,
field,
}) as MinIndexPatternColumn;
expect(column.operationType).toEqual('avg');
expect(column.sourceField).toEqual(field.name);
});

it('should pick a suitable document operation if none is passed in', () => {
const column = buildColumn({
layerId: 'first',
indexPattern: expectedIndexPatterns[1],
columns: state.layers.first.columns,
suggestedPriority: 0,
field: documentField,
}) as CountIndexPatternColumn;
expect(column.operationType).toEqual('count');
});
});

describe('getAvailableOperationsByMetaData', () => {
it('should put the average operation first', () => {
const numberOperation = getAvailableOperationsByMetadata(expectedIndexPatterns[1]).find(
({ operationMetaData }) =>
!operationMetaData.isBucketed && operationMetaData.dataType === 'number'
)!;
expect(numberOperation.operations[0]).toEqual(
expect.objectContaining({
operationType: 'avg',
})
);
});

it('should list out all field-operation tuples for different operation meta data', () => {
expect(getAvailableOperationsByMetadata(expectedIndexPatterns[1])).toMatchInlineSnapshot(`
Array [
Expand Down Expand Up @@ -278,17 +265,22 @@ describe('getOperationTypesForField', () => {
"operations": Array [
Object {
"field": "bytes",
"operationType": "min",
"operationType": "avg",
"type": "field",
},
Object {
"field": "bytes",
"operationType": "max",
"operationType": "sum",
"type": "field",
},
Object {
"field": "bytes",
"operationType": "avg",
"operationType": "min",
"type": "field",
},
Object {
"field": "bytes",
"operationType": "max",
"type": "field",
},
Object {
Expand All @@ -306,11 +298,6 @@ describe('getOperationTypesForField', () => {
"operationType": "cardinality",
"type": "field",
},
Object {
"field": "bytes",
"operationType": "sum",
"type": "field",
},
],
},
]
Expand Down
Loading

0 comments on commit 312e687

Please sign in to comment.