Skip to content

Commit

Permalink
[Lens] Fix error message layer indexing (elastic#180898)
Browse files Browse the repository at this point in the history
## Summary

Fixes elastic#179443 


### 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
  • Loading branch information
dej611 authored Apr 16, 2024
1 parent 58c0059 commit 538396f
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2786,6 +2786,48 @@ describe('xy_visualization', () => {
},
]);
});
it('should return an error with batched messages for the same error with the correct index for multiple layers', () => {
expect(
getErrorMessages(xyVisualization, {
...exampleState(),
layers: [
{
layerId: 'referenceLine',
layerType: layerTypes.REFERENCELINE,
accessors: [],
},
{
layerId: 'first',
layerType: layerTypes.DATA,
seriesType: 'area',
xAccessor: 'a',
accessors: ['a'],
},
{
layerId: 'second',
layerType: layerTypes.DATA,
seriesType: 'area',
xAccessor: undefined,
accessors: [],
splitAccessor: 'a',
},
{
layerId: 'third',
layerType: layerTypes.DATA,
seriesType: 'area',
xAccessor: undefined,
accessors: [],
splitAccessor: 'a',
},
],
})
).toEqual([
{
shortMessage: 'Missing Vertical axis.',
longMessage: 'Layers 3, 4 require a field for the Vertical axis.',
},
]);
});
it("should return an error when some layers are complete but other layers aren't", () => {
expect(
getErrorMessages(xyVisualization, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,6 @@ export const getXyVisualization = ({
const hasNoAccessors = ({ accessors }: XYDataLayerConfig) =>
accessors == null || accessors.length === 0;

const dataLayers = getDataLayers(state.layers);
const hasNoSplitAccessor = ({ splitAccessor, seriesType }: XYDataLayerConfig) =>
seriesType.includes('percentage') && splitAccessor == null;

Expand All @@ -837,13 +836,8 @@ export const getXyVisualization = ({
['Break down', hasNoSplitAccessor],
];

// filter out those layers with no accessors at all
const filteredLayers = dataLayers.filter(
({ accessors, xAccessor, splitAccessor, layerType }) =>
accessors.length > 0 || xAccessor != null || splitAccessor != null
);
for (const [dimension, criteria] of checks) {
const result = validateLayersForDimension(dimension, filteredLayers, criteria);
const result = validateLayersForDimension(dimension, state.layers, criteria);
if (!result.valid) {
errors.push({
severity: 'error',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,31 +395,53 @@ export function getLayersByType(state: State, byType?: string) {

export function validateLayersForDimension(
dimension: string,
layers: XYDataLayerConfig[],
allLayers: XYLayerConfig[],
missingCriteria: (layer: XYDataLayerConfig) => boolean
):
| { valid: true }
| {
valid: false;
payload: { shortMessage: string; longMessage: React.ReactNode };
} {
const dataLayers = allLayers
.map((layer, i) => ({ layer, originalIndex: i }))
.filter(({ layer }) => isDataLayer(layer)) as Array<{
layer: XYDataLayerConfig;
originalIndex: number;
}>;

// filter out those layers with no accessors at all
const filteredLayers = dataLayers.filter(
({ layer: { accessors, xAccessor, splitAccessor } }) =>
accessors.length > 0 || xAccessor != null || splitAccessor != null
);
// Multiple layers must be consistent:
// * either a dimension is missing in ALL of them
// * or should not miss on any
if (layers.every(missingCriteria) || !layers.some(missingCriteria)) {
if (
filteredLayers.every(({ layer }) => missingCriteria(layer)) ||
!filteredLayers.some(({ layer }) => missingCriteria(layer))
) {
return { valid: true };
}
// otherwise it's an error and it has to be reported
const layerMissingAccessors = layers.reduce((missing: number[], layer, i) => {
if (missingCriteria(layer)) {
missing.push(i);
}
return missing;
}, []);
const layerMissingAccessors = filteredLayers.reduce(
(missing: number[], { layer, originalIndex }) => {
if (missingCriteria(layer)) {
missing.push(originalIndex);
}
return missing;
},
[]
);

return {
valid: false,
payload: getMessageIdsForDimension(dimension, layerMissingAccessors, isHorizontalChart(layers)),
payload: getMessageIdsForDimension(
dimension,
layerMissingAccessors,
isHorizontalChart(dataLayers.map(({ layer }) => layer))
),
};
}

Expand Down

0 comments on commit 538396f

Please sign in to comment.