[maps] show actionable message when term joins have no matches (#105161) (#105588)

* [maps] show actionable message when term joins have no matches

* include message about showing first 10

* fix formatting

* review feedback

* review feedback

* move performInnerJoins into its own function

* lint

* unit test

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Nathan Reese <reese.nathan@gmail.com>
This commit is contained in:
Kibana Machine 2021-07-14 10:35:01 -04:00 committed by GitHub
parent dedbcdb391
commit dcdb65270b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 447 additions and 69 deletions

View file

@ -55,6 +55,7 @@ export type DataRequestContext = {
startLoading(dataId: string, requestToken: symbol, requestMeta?: DataMeta): void;
stopLoading(dataId: string, requestToken: symbol, data: object, resultsMeta?: DataMeta): void;
onLoadError(dataId: string, requestToken: symbol, errorMessage: string): void;
onJoinError(errorMessage: string): void;
updateSourceData(newData: unknown): void;
isRequestStillActive(dataId: string, requestToken: symbol): boolean;
registerCancelCallback(requestToken: symbol, callback: () => void): void;
@ -121,6 +122,8 @@ function getDataRequestContext(
dispatch(endDataLoad(layerId, dataId, requestToken, data, meta)),
onLoadError: (dataId: string, requestToken: symbol, errorMessage: string) =>
dispatch(onDataLoadError(layerId, dataId, requestToken, errorMessage)),
onJoinError: (errorMessage: string) =>
dispatch(setLayerDataLoadErrorStatus(layerId, errorMessage)),
updateSourceData: (newData: object) => {
dispatch(updateSourceDataRequest(layerId, newData));
},
@ -193,13 +196,11 @@ export function syncDataForLayerId(layerId: string | null) {
}
function setLayerDataLoadErrorStatus(layerId: string, errorMessage: string | null) {
return (dispatch: Dispatch) => {
dispatch({
type: SET_LAYER_ERROR_STATUS,
isInErrorState: errorMessage !== null,
layerId,
errorMessage,
});
return {
type: SET_LAYER_ERROR_STATUS,
isInErrorState: errorMessage !== null,
layerId,
errorMessage,
};
}

View file

@ -118,17 +118,23 @@ export class InnerJoin {
});
}
const joinKey = feature.properties[this._leftField.getName()];
const coercedKey =
typeof joinKey === 'undefined' || joinKey === null ? null : joinKey.toString();
if (coercedKey !== null && propertiesMap.has(coercedKey)) {
Object.assign(feature.properties, propertiesMap.get(coercedKey));
const joinKey = this.getJoinKey(feature);
if (joinKey !== null && propertiesMap.has(joinKey)) {
Object.assign(feature.properties, propertiesMap.get(joinKey));
return true;
} else {
return false;
}
}
getJoinKey(feature: Feature): string | null {
const joinKey =
feature.properties && this._leftField
? feature.properties[this._leftField.getName()]
: undefined;
return joinKey === undefined || joinKey === null ? null : joinKey.toString();
}
getRightJoinSource(): ITermJoinSource {
if (!this._rightSource) {
throw new Error('Cannot get rightSource from InnerJoin with incomplete config');

View file

@ -16,6 +16,7 @@ export class MockSyncContext implements DataRequestContext {
registerCancelCallback: (requestToken: symbol, callback: () => void) => void;
startLoading: (dataId: string, requestToken: symbol, meta: DataMeta) => void;
stopLoading: (dataId: string, requestToken: symbol, data: object, meta: DataMeta) => void;
onJoinError: (errorMessage: string) => void;
updateSourceData: (newData: unknown) => void;
forceRefresh: boolean;
@ -38,6 +39,7 @@ export class MockSyncContext implements DataRequestContext {
this.registerCancelCallback = sinon.spy();
this.startLoading = sinon.spy();
this.stopLoading = sinon.spy();
this.onJoinError = sinon.spy();
this.updateSourceData = sinon.spy();
this.forceRefresh = false;
}

View file

@ -0,0 +1,285 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import sinon from 'sinon';
import _ from 'lodash';
import { FeatureCollection } from 'geojson';
import { ESTermSourceDescriptor } from '../../../../common/descriptor_types';
import {
AGG_TYPE,
FEATURE_VISIBLE_PROPERTY_NAME,
SOURCE_TYPES,
} from '../../../../common/constants';
import { performInnerJoins } from './perform_inner_joins';
import { InnerJoin } from '../../joins/inner_join';
import { IVectorSource } from '../../sources/vector_source';
import { IField } from '../../fields/field';
const LEFT_FIELD = 'leftKey';
const COUNT_PROPERTY_NAME = '__kbnjoin__count__d3625663-5b34-4d50-a784-0d743f676a0c';
const featureCollection = {
type: 'FeatureCollection',
features: [
{
type: 'Feature',
properties: {
[FEATURE_VISIBLE_PROPERTY_NAME]: false,
[LEFT_FIELD]: 'alpha',
},
geometry: {
type: 'Point',
coordinates: [-112, 46],
},
},
{
type: 'Feature',
properties: {
[COUNT_PROPERTY_NAME]: 20,
[FEATURE_VISIBLE_PROPERTY_NAME]: true,
[LEFT_FIELD]: 'bravo',
},
geometry: {
type: 'Point',
coordinates: [-100, 40],
},
},
],
};
const joinDescriptor = {
leftField: LEFT_FIELD,
right: {
applyGlobalQuery: true,
applyGlobalTime: true,
id: 'd3625663-5b34-4d50-a784-0d743f676a0c',
indexPatternId: 'myIndexPattern',
metrics: [
{
type: AGG_TYPE.COUNT,
},
],
term: 'rightKey',
type: SOURCE_TYPES.ES_TERM_SOURCE,
} as ESTermSourceDescriptor,
};
const mockVectorSource = ({
getInspectorAdapters: () => {
return undefined;
},
createField: () => {
return {
getName: () => {
return LEFT_FIELD;
},
} as IField;
},
} as unknown) as IVectorSource;
const innerJoin = new InnerJoin(joinDescriptor, mockVectorSource);
const propertiesMap = new Map<string, Record<string | number, unknown>>();
propertiesMap.set('alpha', { [COUNT_PROPERTY_NAME]: 1 });
test('should skip join when no state has changed', () => {
const updateSourceData = sinon.spy();
const onJoinError = sinon.spy();
performInnerJoins(
{
refreshed: false,
featureCollection: _.cloneDeep(featureCollection) as FeatureCollection,
},
[
{
dataHasChanged: false,
join: innerJoin,
},
],
updateSourceData,
onJoinError
);
expect(updateSourceData.notCalled);
expect(onJoinError.notCalled);
});
test('should perform join when features change', () => {
const updateSourceData = sinon.spy();
const onJoinError = sinon.spy();
performInnerJoins(
{
refreshed: true,
featureCollection: _.cloneDeep(featureCollection) as FeatureCollection,
},
[
{
dataHasChanged: false,
join: innerJoin,
},
],
updateSourceData,
onJoinError
);
expect(updateSourceData.calledOnce);
expect(onJoinError.notCalled);
});
test('should perform join when join state changes', () => {
const updateSourceData = sinon.spy();
const onJoinError = sinon.spy();
performInnerJoins(
{
refreshed: false,
featureCollection: _.cloneDeep(featureCollection) as FeatureCollection,
},
[
{
dataHasChanged: true,
join: innerJoin,
},
],
updateSourceData,
onJoinError
);
expect(updateSourceData.calledOnce);
expect(onJoinError.notCalled);
});
test('should call updateSourceData with feature collection with updated feature visibility and join properties', () => {
const updateSourceData = sinon.spy();
const onJoinError = sinon.spy();
performInnerJoins(
{
refreshed: true,
featureCollection: _.cloneDeep(featureCollection) as FeatureCollection,
},
[
{
dataHasChanged: false,
join: innerJoin,
propertiesMap,
},
],
updateSourceData,
onJoinError
);
const firstCallArgs = updateSourceData.args[0];
const updateSourceDataFeatureCollection = firstCallArgs[0];
expect(updateSourceDataFeatureCollection).toEqual({
type: 'FeatureCollection',
features: [
{
type: 'Feature',
properties: {
[COUNT_PROPERTY_NAME]: 1,
[FEATURE_VISIBLE_PROPERTY_NAME]: true,
[LEFT_FIELD]: 'alpha',
},
geometry: {
type: 'Point',
coordinates: [-112, 46],
},
},
{
type: 'Feature',
properties: {
[FEATURE_VISIBLE_PROPERTY_NAME]: false,
[LEFT_FIELD]: 'bravo',
},
geometry: {
type: 'Point',
coordinates: [-100, 40],
},
},
],
});
expect(onJoinError.notCalled);
});
test('should call updateSourceData when no results returned from terms aggregation', () => {
const updateSourceData = sinon.spy();
const onJoinError = sinon.spy();
performInnerJoins(
{
refreshed: false,
featureCollection: _.cloneDeep(featureCollection) as FeatureCollection,
},
[
{
dataHasChanged: true,
join: innerJoin,
},
],
updateSourceData,
onJoinError
);
const firstCallArgs = updateSourceData.args[0];
const updateSourceDataFeatureCollection = firstCallArgs[0];
expect(updateSourceDataFeatureCollection).toEqual({
type: 'FeatureCollection',
features: [
{
type: 'Feature',
properties: {
[FEATURE_VISIBLE_PROPERTY_NAME]: false,
[LEFT_FIELD]: 'alpha',
},
geometry: {
type: 'Point',
coordinates: [-112, 46],
},
},
{
type: 'Feature',
properties: {
[COUNT_PROPERTY_NAME]: 20,
[FEATURE_VISIBLE_PROPERTY_NAME]: false,
[LEFT_FIELD]: 'bravo',
},
geometry: {
type: 'Point',
coordinates: [-100, 40],
},
},
],
});
expect(onJoinError.notCalled);
});
test('should call onJoinError when there are no matching features', () => {
const updateSourceData = sinon.spy();
const onJoinError = sinon.spy();
// instead of returning military alphabet like "alpha" or "bravo", mismatched key returns numbers, like '1'
const propertiesMapFromMismatchedKey = new Map<string, Record<string | number, unknown>>();
propertiesMapFromMismatchedKey.set('1', { [COUNT_PROPERTY_NAME]: 1 });
performInnerJoins(
{
refreshed: false,
featureCollection: _.cloneDeep(featureCollection) as FeatureCollection,
},
[
{
dataHasChanged: true,
join: innerJoin,
propertiesMap: propertiesMapFromMismatchedKey,
},
],
updateSourceData,
onJoinError
);
expect(updateSourceData.notCalled);
expect(onJoinError.calledOnce);
});

View file

@ -0,0 +1,134 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { FeatureCollection } from 'geojson';
import { i18n } from '@kbn/i18n';
import { FEATURE_VISIBLE_PROPERTY_NAME } from '../../../../common/constants';
import { DataRequestContext } from '../../../actions';
import { InnerJoin } from '../../joins/inner_join';
import { PropertiesMap } from '../../../../common/elasticsearch_util';
interface SourceResult {
refreshed: boolean;
featureCollection: FeatureCollection;
}
export interface JoinState {
dataHasChanged: boolean;
join: InnerJoin;
propertiesMap?: PropertiesMap;
}
export function performInnerJoins(
sourceResult: SourceResult,
joinStates: JoinState[],
updateSourceData: DataRequestContext['updateSourceData'],
onJoinError: DataRequestContext['onJoinError']
) {
// should update the store if
// -- source result was refreshed
// -- any of the join configurations changed (joinState changed)
// -- visibility of any of the features has changed
let shouldUpdateStore =
sourceResult.refreshed || joinStates.some((joinState) => joinState.dataHasChanged);
if (!shouldUpdateStore) {
return;
}
const joinStatuses = joinStates.map((joinState) => {
return {
joinedWithAtLeastOneFeature: false,
keys: [] as string[],
joinState,
};
});
for (let i = 0; i < sourceResult.featureCollection.features.length; i++) {
const feature = sourceResult.featureCollection.features[i];
if (!feature.properties) {
feature.properties = {};
}
const oldVisbility = feature.properties[FEATURE_VISIBLE_PROPERTY_NAME];
let isFeatureVisible = true;
for (let j = 0; j < joinStates.length; j++) {
const joinState = joinStates[j];
const innerJoin = joinState.join;
const joinStatus = joinStatuses[j];
const joinKey = innerJoin.getJoinKey(feature);
if (joinKey !== null) {
joinStatus.keys.push(joinKey);
}
const canJoinOnCurrent = joinState.propertiesMap
? innerJoin.joinPropertiesToFeature(feature, joinState.propertiesMap)
: false;
if (canJoinOnCurrent && !joinStatus.joinedWithAtLeastOneFeature) {
joinStatus.joinedWithAtLeastOneFeature = true;
}
isFeatureVisible = isFeatureVisible && canJoinOnCurrent;
}
if (oldVisbility !== isFeatureVisible) {
shouldUpdateStore = true;
}
feature.properties[FEATURE_VISIBLE_PROPERTY_NAME] = isFeatureVisible;
}
if (shouldUpdateStore) {
updateSourceData({ ...sourceResult.featureCollection });
}
const joinStatusesWithoutAnyMatches = joinStatuses.filter((joinStatus) => {
return (
!joinStatus.joinedWithAtLeastOneFeature && joinStatus.joinState.propertiesMap !== undefined
);
});
if (joinStatusesWithoutAnyMatches.length) {
function prettyPrintArray(array: unknown[]) {
return array.length <= 5
? array.join(',')
: array.slice(0, 5).join(',') +
i18n.translate('xpack.maps.vectorLayer.joinError.firstTenMsg', {
defaultMessage: ` (5 of {total})`,
values: { total: array.length },
});
}
const joinStatus = joinStatusesWithoutAnyMatches[0];
const leftFieldName = joinStatus.joinState.join.getLeftField().getName();
const reason =
joinStatus.keys.length === 0
? i18n.translate('xpack.maps.vectorLayer.joinError.noLeftFieldValuesMsg', {
defaultMessage: `Left field: '{leftFieldName}', does not provide any values.`,
values: { leftFieldName },
})
: i18n.translate('xpack.maps.vectorLayer.joinError.noMatchesMsg', {
defaultMessage: `Left field does not match right field. Left field: '{leftFieldName}' has values { leftFieldValues }. Right field: '{rightFieldName}' has values: { rightFieldValues }.`,
values: {
leftFieldName,
leftFieldValues: prettyPrintArray(joinStatus.keys),
rightFieldName: joinStatus.joinState.join
.getRightJoinSource()
.getTermField()
.getName(),
rightFieldValues: prettyPrintArray(
Array.from(joinStatus.joinState.propertiesMap!.keys())
),
},
});
onJoinError(
i18n.translate('xpack.maps.vectorLayer.joinErrorMsg', {
defaultMessage: `Unable to perform term join. {reason}`,
values: { reason },
})
);
}
}

View file

@ -69,17 +69,7 @@ import { IESSource } from '../../sources/es_source';
import { PropertiesMap } from '../../../../common/elasticsearch_util';
import { ITermJoinSource } from '../../sources/term_join_source';
import { addGeoJsonMbSource, getVectorSourceBounds, syncVectorSource } from './utils';
interface SourceResult {
refreshed: boolean;
featureCollection?: FeatureCollection;
}
interface JoinState {
dataHasChanged: boolean;
join: InnerJoin;
propertiesMap?: PropertiesMap;
}
import { JoinState, performInnerJoins } from './perform_inner_joins';
export interface VectorLayerArguments {
source: IVectorSource;
@ -435,51 +425,6 @@ export class VectorLayer extends AbstractLayer implements IVectorLayer {
};
}
_performInnerJoins(
sourceResult: SourceResult,
joinStates: JoinState[],
updateSourceData: DataRequestContext['updateSourceData']
) {
// should update the store if
// -- source result was refreshed
// -- any of the join configurations changed (joinState changed)
// -- visibility of any of the features has changed
let shouldUpdateStore =
sourceResult.refreshed || joinStates.some((joinState) => joinState.dataHasChanged);
if (!shouldUpdateStore) {
return;
}
for (let i = 0; i < sourceResult.featureCollection!.features.length; i++) {
const feature = sourceResult.featureCollection!.features[i];
if (!feature.properties) {
feature.properties = {};
}
const oldVisbility = feature.properties[FEATURE_VISIBLE_PROPERTY_NAME];
let isFeatureVisible = true;
for (let j = 0; j < joinStates.length; j++) {
const joinState = joinStates[j];
const innerJoin = joinState.join;
const canJoinOnCurrent = joinState.propertiesMap
? innerJoin.joinPropertiesToFeature(feature, joinState.propertiesMap)
: false;
isFeatureVisible = isFeatureVisible && canJoinOnCurrent;
}
if (oldVisbility !== isFeatureVisible) {
shouldUpdateStore = true;
}
feature.properties[FEATURE_VISIBLE_PROPERTY_NAME] = isFeatureVisible;
}
if (shouldUpdateStore) {
updateSourceData({ ...sourceResult.featureCollection });
}
}
async _syncSourceStyleMeta(
syncContext: DataRequestContext,
source: IVectorSource,
@ -714,7 +659,12 @@ export class VectorLayer extends AbstractLayer implements IVectorLayer {
}
const joinStates = await this._syncJoins(syncContext, style);
this._performInnerJoins(sourceResult, joinStates, syncContext.updateSourceData);
performInnerJoins(
sourceResult,
joinStates,
syncContext.updateSourceData,
syncContext.onJoinError
);
} catch (error) {
if (!(error instanceof DataRequestAbortError)) {
throw error;