[Lens] Fixes broken charts with terms agg and no dataview (#169503)

## Summary

In Lens when a dataview is missing, the editing should be allowed. There
are cases which it doesn't. The easiest to reproduce is to create a
visualization with terms and remove the dataview. Then you will see an
error (cannot getFieldByName of undefined) and editing and changing the
dataview is not allowed.

This PR fixes this and all other occurences.

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
This commit is contained in:
Stratoula Kalafateli 2023-10-25 08:43:06 +03:00 committed by GitHub
parent 6f22eef079
commit 33c409bfad
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
26 changed files with 73 additions and 51 deletions

View file

@ -1087,8 +1087,8 @@ export function DimensionEditor(props: DimensionEditorProps) {
selectedColumn &&
operationDefinitionMap[selectedColumn.operationType].getDefaultLabel(
selectedColumn,
props.indexPatterns[state.layers[layerId].indexPatternId],
state.layers[layerId].columns
state.layers[layerId].columns,
props.indexPatterns[state.layers[layerId].indexPatternId]
)
),
[layerId, selectedColumn, props.indexPatterns, state.layers]
@ -1263,8 +1263,8 @@ export function DimensionEditor(props: DimensionEditorProps) {
customLabel:
operationDefinitionMap[selectedColumn.operationType].getDefaultLabel(
selectedColumn,
props.indexPatterns[state.layers[layerId].indexPatternId],
state.layers[layerId].columns
state.layers[layerId].columns,
props.indexPatterns[state.layers[layerId].indexPatternId]
) !== value,
},
},

View file

@ -497,8 +497,8 @@ export function getFormBasedDatasource({
? column.label
: operationDefinitionMap[column.operationType].getDefaultLabel(
column,
indexPatternsMap[layer.indexPatternId],
layer.columns
layer.columns,
indexPatternsMap[layer.indexPatternId]
)
);
});

View file

@ -70,11 +70,11 @@ export const counterRateOperation: OperationDefinition<
};
}
},
getDefaultLabel: (column, indexPattern, columns) => {
getDefaultLabel: (column, columns, indexPattern) => {
const ref = columns[column.references[0]];
return ofName(
ref && 'sourceField' in ref
? indexPattern.getFieldByName(ref.sourceField)?.displayName
? indexPattern?.getFieldByName(ref.sourceField)?.displayName
: undefined,
column.timeScale,
column.timeShift

View file

@ -69,11 +69,11 @@ export const cumulativeSumOperation: OperationDefinition<
};
}
},
getDefaultLabel: (column, indexPattern, columns) => {
getDefaultLabel: (column, columns, indexPattern) => {
const ref = columns[column.references[0]];
return ofName(
ref && 'sourceField' in ref
? indexPattern.getFieldByName(ref.sourceField)?.displayName
? indexPattern?.getFieldByName(ref.sourceField)?.displayName
: undefined,
undefined,
column.timeShift

View file

@ -65,7 +65,7 @@ export const derivativeOperation: OperationDefinition<
};
}
},
getDefaultLabel: (column, indexPattern, columns) => {
getDefaultLabel: (column, columns, indexPattern) => {
return ofName(columns[column.references[0]]?.label, column.timeScale, column.timeShift);
},
toExpression: (layer, columnId) => {

View file

@ -76,7 +76,7 @@ export const movingAverageOperation: OperationDefinition<
};
}
},
getDefaultLabel: (column, indexPattern, columns) => {
getDefaultLabel: (column, columns, indexPattern) => {
return ofName(columns[column.references[0]]?.label, column.timeScale, column.timeShift);
},
toExpression: (layer, columnId) => {

View file

@ -56,11 +56,11 @@ function buildOverallMetricOperation<T extends OverallMetricIndexPatternColumn<s
scale: 'ratio',
};
},
getDefaultLabel: (column, indexPattern, columns) => {
getDefaultLabel: (column, columns, indexPattern) => {
const ref = columns[column.references[0]];
return ofName(
ref && 'sourceField' in ref
? indexPattern.getFieldByName(ref.sourceField)?.displayName
? indexPattern?.getFieldByName(ref.sourceField)?.displayName
: undefined
);
},

View file

@ -56,7 +56,7 @@ export const timeScaleOperation: OperationDefinition<TimeScaleIndexPatternColumn
scale: 'ratio',
};
},
getDefaultLabel: (column, indexPattern, columns) => {
getDefaultLabel: (column, columns, indexPattern) => {
return 'normalize_by_unit';
},
toExpression: (layer, columnId) => {

View file

@ -113,7 +113,7 @@ export const cardinalityOperation: OperationDefinition<
filterable: true,
shiftable: true,
canReduceTimeRange: true,
getDefaultLabel: (column, indexPattern) =>
getDefaultLabel: (column, columns, indexPattern) =>
ofName(
getSafeName(column.sourceField, indexPattern),
column.timeShift,

View file

@ -117,7 +117,7 @@ export const countOperation: OperationDefinition<CountIndexPatternColumn, 'field
return { dataType: 'number', isBucketed: IS_BUCKETED, scale: SCALE };
}
},
getDefaultLabel: (column, indexPattern) => {
getDefaultLabel: (column, columns, indexPattern) => {
const field = indexPattern?.getFieldByName(column.sourceField);
return ofName(field, column.timeShift, column.timeScale, column.reducedTimeRange);
},

View file

@ -776,7 +776,6 @@ describe('date_histogram', () => {
sourceField: 'missing',
params: { interval: 'auto' },
},
indexPattern1,
{
col1: {
label: '',
@ -786,7 +785,8 @@ describe('date_histogram', () => {
sourceField: 'missing',
params: { interval: 'auto' },
} as DateHistogramIndexPatternColumn,
}
},
indexPattern1
)
).toEqual('Missing field');
});

View file

@ -102,7 +102,7 @@ export const dateHistogramOperation: OperationDefinition<
};
}
},
getDefaultLabel: (column, indexPattern) => getSafeName(column.sourceField, indexPattern),
getDefaultLabel: (column, columns, indexPattern) => getSafeName(column.sourceField, indexPattern),
buildColumn({ field }, columnParams) {
return {
label: field.displayName,

View file

@ -52,7 +52,7 @@ export const formulaOperation: OperationDefinition<FormulaIndexPatternColumn, 'm
{
type: 'formula',
displayName: defaultLabel,
getDefaultLabel: (column, indexPattern) => column.params.formula ?? defaultLabel,
getDefaultLabel: (column) => column.params.formula ?? defaultLabel,
input: 'managedReference',
hidden: true,
filterable: {

View file

@ -23,7 +23,7 @@ export const mathOperation: OperationDefinition<MathIndexPatternColumn, 'managed
type: 'math',
displayName: 'Math',
hidden: true,
getDefaultLabel: (column, indexPattern) => 'Math',
getDefaultLabel: (column, columns, indexPattern) => 'Math',
input: 'managedReference',
getDisabledStatus(indexPattern: IndexPattern) {
return undefined;

View file

@ -267,8 +267,8 @@ interface BaseOperationDefinitionProps<
*/
getDefaultLabel: (
column: C,
indexPattern: IndexPattern,
columns: Record<string, GenericIndexPatternColumn>
columns: Record<string, GenericIndexPatternColumn>,
indexPattern?: IndexPattern
) => string;
/**
* This function is called if another column in the same layer changed or got added/removed.

View file

@ -165,7 +165,7 @@ export const lastValueOperation: OperationDefinition<
displayName: i18n.translate('xpack.lens.indexPattern.lastValue', {
defaultMessage: 'Last value',
}),
getDefaultLabel: (column, indexPattern) =>
getDefaultLabel: (column, columns, indexPattern) =>
ofName(
getSafeName(column.sourceField, indexPattern),
column.timeShift,

View file

@ -129,7 +129,7 @@ function buildMetricOperation<T extends MetricColumn<string>>({
(!newField.aggregationRestrictions || newField.aggregationRestrictions![type])
);
},
getDefaultLabel: (column, indexPattern, columns) =>
getDefaultLabel: (column, columns, indexPattern) =>
labelLookup(getSafeName(column.sourceField, indexPattern), column),
buildColumn: ({ field, previousColumn }, columnParams) => {
return {

View file

@ -151,7 +151,7 @@ export const percentileOperation: OperationDefinition<
(!newField.aggregationRestrictions || !newField.aggregationRestrictions.percentiles)
);
},
getDefaultLabel: (column, indexPattern, columns) =>
getDefaultLabel: (column, columns, indexPattern) =>
ofName(
getSafeName(column.sourceField, indexPattern),
column.params.percentile,

View file

@ -109,7 +109,7 @@ export const percentileRanksOperation: OperationDefinition<
(!newField.aggregationRestrictions || !newField.aggregationRestrictions.percentile_ranks)
);
},
getDefaultLabel: (column, indexPattern, columns) =>
getDefaultLabel: (column, columns, indexPattern) =>
ofName(
getSafeName(column.sourceField, indexPattern),
column.params.value,

View file

@ -97,8 +97,8 @@ export const rangeOperation: OperationDefinition<
};
}
},
getDefaultLabel: (column, indexPattern) =>
indexPattern.getFieldByName(column.sourceField)?.displayName ??
getDefaultLabel: (column, columns, indexPattern) =>
indexPattern?.getFieldByName(column.sourceField)?.displayName ??
i18n.translate('xpack.lens.indexPattern.missingFieldLabel', {
defaultMessage: 'Missing field',
}),

View file

@ -123,8 +123,8 @@ describe('static_value', () => {
value: '23',
},
},
createMockedIndexPattern(),
layer.columns
layer.columns,
createMockedIndexPattern()
)
).toBe('Static value: 23');
});
@ -142,8 +142,8 @@ describe('static_value', () => {
value: '',
},
},
createMockedIndexPattern(),
layer.columns
layer.columns,
createMockedIndexPattern()
)
).toBe('Static value');
});

View file

@ -386,9 +386,9 @@ export const termsOperation: OperationDefinition<
}),
}).toAst();
},
getDefaultLabel: (column, indexPattern) =>
getDefaultLabel: (column, columns, indexPattern) =>
ofName(
indexPattern.getFieldByName(column.sourceField)?.displayName,
indexPattern?.getFieldByName(column.sourceField)?.displayName,
column.params.secondaryFields?.length,
column.params.orderBy.type === 'rare',
column.params.orderBy.type === 'significant',

View file

@ -1177,12 +1177,34 @@ describe('terms', () => {
},
sourceField: 'source',
} as TermsIndexPatternColumn,
createMockedIndexPattern(),
{}
{},
createMockedIndexPattern()
)
).toBe('Top 3 values of source');
});
it('should not fail if dataview is not given', () => {
expect(
termsOperation.getDefaultLabel(
{
dataType: 'string',
isBucketed: true,
// Private
operationType: 'terms',
params: {
orderBy: { type: 'alphabetical', fallback: true },
size: 3,
orderDirection: 'asc',
},
sourceField: 'source',
} as TermsIndexPatternColumn,
{},
undefined
)
).toBe('Top 3 values of Missing field');
});
it('should return main value with single counter for two fields', () => {
expect(
termsOperation.getDefaultLabel(
@ -1200,8 +1222,8 @@ describe('terms', () => {
},
sourceField: 'source',
} as TermsIndexPatternColumn,
createMockedIndexPattern(),
{}
{},
createMockedIndexPattern()
)
).toBe('Top values of source + 1 other');
});
@ -1223,8 +1245,8 @@ describe('terms', () => {
},
sourceField: 'source',
} as TermsIndexPatternColumn,
createMockedIndexPattern(),
{}
{},
createMockedIndexPattern()
)
).toBe('Top values of source + 2 others');
});

View file

@ -2577,7 +2577,6 @@ describe('state_helpers', () => {
operationType: 'testReference',
references: ['col1'],
},
indexPattern,
{
col2: {
label: 'Default label',
@ -2586,7 +2585,8 @@ describe('state_helpers', () => {
operationType: 'testReference',
references: ['col1'],
},
}
},
indexPattern
);
});

View file

@ -636,7 +636,7 @@ export function replaceColumn({
previousColumn.customLabel &&
hypotheticalLayer.columns[columnId] &&
previousColumn.label !==
previousDefinition.getDefaultLabel(previousColumn, indexPattern, tempLayer.columns)
previousDefinition.getDefaultLabel(previousColumn, tempLayer.columns, indexPattern)
) {
hypotheticalLayer.columns[columnId].customLabel = true;
hypotheticalLayer.columns[columnId].label = previousColumn.label;
@ -1723,8 +1723,8 @@ export function updateDefaultLabels(
...col,
label: operationDefinitionMap[col.operationType].getDefaultLabel(
col,
indexPattern,
copiedColumns
copiedColumns,
indexPattern
),
};
}

View file

@ -242,8 +242,8 @@ function getExpressionForLayer(
? col.label
: operationDefinitionMap[col.operationType].getDefaultLabel(
col,
indexPattern,
layer.columns
layer.columns,
indexPattern
),
},
];
@ -398,8 +398,8 @@ function getExpressionForLayer(
? col.label
: operationDefinitionMap[col.operationType].getDefaultLabel(
col,
indexPattern,
layer.columns
layer.columns,
indexPattern
),
],
targetUnit: [col.timeScale!],