[Maps] clean features in locked tooltip after re-fetch (#48016) (#48164)

* [Maps] clean features in locked tooltip after re-fetch

* fetch geometry from FEATURE_ID_PROPERTY_NAME instead of _id

* set FEATURE_ID_PROPERTY_NAME for pew pew source

* do not update tooltip state if no features were removed

* set FEATURE_ID_PROPERTY_NAME for EMS_file source and kibana_regionmap source

* avoid adding lodash to map actions

* use if else instead of early return

x-pack/legacy/plugins/maps/public/actions/map_actions.js

@@ -23,7 +23,7 @@ import {
   unregisterCancelCallback
 } from '../reducers/non_serializable_instances';
 import { updateFlyout } from '../actions/ui_actions';
-import { SOURCE_DATA_ID_ORIGIN } from '../../common/constants';
+import { FEATURE_ID_PROPERTY_NAME, SOURCE_DATA_ID_ORIGIN } from '../../common/constants';
 
 export const SET_SELECTED_LAYER = 'SET_SELECTED_LAYER';
 export const SET_TRANSIENT_LAYER = 'SET_TRANSIENT_LAYER';
@@ -203,11 +203,36 @@ function setLayerDataLoadErrorStatus(layerId, errorMessage) {
   };
 }
 
-export function clearTooltipStateForLayer(layerId) {
+export function cleanTooltipStateForLayer(layerId, layerFeatures = []) {
   return (dispatch, getState) => {
     const tooltipState = getTooltipState(getState());
-    if (tooltipState && tooltipState.layerId === layerId) {
+
+    if (!tooltipState) {
+      return;
+    }
+
+    const nextTooltipFeatures = tooltipState.features.filter(tooltipFeature => {
+      if (tooltipFeature.layerId !== layerId) {
+        // feature from another layer, keep it
+        return true;
+      }
+
+      // Keep feature if it is still in layer
+      return layerFeatures.some(layerFeature => {
+        return layerFeature.properties[FEATURE_ID_PROPERTY_NAME] === tooltipFeature.id;
+      });
+    });
+
+    if (tooltipState.features.length === nextTooltipFeatures.length) {
+      // no features got removed, nothing to update
+      return;
+    }
+
+    if (nextTooltipFeatures.length === 0) {
+      // all features removed from tooltip, close tooltip
       dispatch(setTooltipState(null));
+    } else {
+      dispatch(setTooltipState({ ...tooltipState, features: nextTooltipFeatures }));
     }
   };
 }
@@ -223,7 +248,7 @@ export function toggleLayerVisible(layerId) {
     const makeVisible = !layer.isVisible();
 
     if (!makeVisible) {
-      dispatch(clearTooltipStateForLayer(layerId));
+      dispatch(cleanTooltipStateForLayer(layerId));
     }
 
     await dispatch({
@@ -456,7 +481,8 @@ export function updateSourceDataRequest(layerId, newData) {
 export function endDataLoad(layerId, dataId, requestToken, data, meta) {
   return async (dispatch) => {
     dispatch(unregisterCancelCallback(requestToken));
-    dispatch(clearTooltipStateForLayer(layerId));
+    const features = data ? data.features : [];
+    dispatch(cleanTooltipStateForLayer(layerId, features));
     dispatch({
       type: LAYER_DATA_LOAD_ENDED,
       layerId,
@@ -478,7 +504,7 @@ export function endDataLoad(layerId, dataId, requestToken, data, meta) {
 export function onDataLoadError(layerId, dataId, requestToken, errorMessage) {
   return async (dispatch) => {
     dispatch(unregisterCancelCallback(requestToken));
-    dispatch(clearTooltipStateForLayer(layerId));
+    dispatch(cleanTooltipStateForLayer(layerId));
     dispatch({
       type: LAYER_DATA_LOAD_ERROR,
       data: null,
@@ -599,7 +625,7 @@ export function removeLayer(layerId) {
     layerGettingRemoved.getInFlightRequestTokens().forEach(requestToken => {
       dispatch(cancelRequest(requestToken));
     });
-    dispatch(clearTooltipStateForLayer(layerId));
+    dispatch(cleanTooltipStateForLayer(layerId));
     layerGettingRemoved.destroy();
     dispatch({
       type: REMOVE_LAYER,

x-pack/legacy/plugins/maps/public/elasticsearch_geo_utils.js

@@ -11,6 +11,7 @@ import {
   DECIMAL_DEGREES_PRECISION,
   ES_GEO_FIELD_TYPE,
   ES_SPATIAL_RELATIONS,
+  FEATURE_ID_PROPERTY_NAME,
   GEO_JSON_TYPE,
   POLYGON_COORDINATES_EXTERIOR_INDEX,
   LON_INDEX,
@@ -74,6 +75,11 @@ export function hitsToGeoJson(hits, flattenHit, geoFieldName, geoFieldType) {
     // don't include geometry field value in properties
     delete properties[geoFieldName];
 
+    // _id is unique to Elasticsearch documents.
+    // Move _id to FEATURE_ID_PROPERTY_NAME to standardize featureId keys across all sources
+    properties[FEATURE_ID_PROPERTY_NAME] = properties._id;
+    delete properties._id;
+
     //create new geojson Feature for every individual geojson geometry.
     for (let j = 0; j < tmpGeometriesAccumulator.length; j++) {
       features.push({

x-pack/legacy/plugins/maps/public/elasticsearch_geo_utils.test.js

@@ -7,6 +7,7 @@
 jest.mock('ui/new_platform');
 jest.mock('ui/index_patterns');
 
+import { FEATURE_ID_PROPERTY_NAME } from '../common/constants';
 import {
   hitsToGeoJson,
   geoPointToGeometry,
@@ -15,7 +16,6 @@ import {
   convertMapExtentToPolygon,
   roundCoordinates,
 } from './elasticsearch_geo_utils';
-
 import { flattenHitWrapper } from 'ui/index_patterns';
 
 const geoFieldName = 'location';
@@ -33,10 +33,33 @@ const flattenHitMock = hit => {
       properties[fieldName] = hit._source[fieldName];
     }
   }
+  for (const fieldName in hit.fields) {
+    if (hit.fields.hasOwnProperty(fieldName)) {
+      properties[fieldName] = hit.fields[fieldName];
+    }
+  }
+  properties._id = hit._id;
+
   return properties;
 };
 
 describe('hitsToGeoJson', () => {
+  it('Should set FEATURE_ID_PROPERTY_NAME to _id', () => {
+    const docId = 'if3mu20BBQNX22Q14Ppm';
+    const hits = [
+      {
+        _id: docId,
+        fields: {
+          [geoFieldName]: '20,100'
+        }
+      }
+    ];
+    const geojson = hitsToGeoJson(hits, flattenHitMock, geoFieldName, 'geo_point');
+    expect(geojson.type).toBe('FeatureCollection');
+    expect(geojson.features.length).toBe(1);
+    expect(geojson.features[0].properties[FEATURE_ID_PROPERTY_NAME]).toBe(docId);
+  });
+
   it('Should convert elasitcsearch hits to geojson', () => {
     const hits = [
       {
@@ -63,7 +86,6 @@ describe('hitsToGeoJson', () => {
     });
   });
 
-
   it('Should handle documents where geoField is not populated', () => {
     const hits = [
       {

x-pack/legacy/plugins/maps/public/layers/sources/ems_file_source/ems_file_source.js

@@ -7,7 +7,7 @@
 import { AbstractVectorSource } from '../vector_source';
 import { VECTOR_SHAPE_TYPES } from '../vector_feature_types';
 import React from 'react';
-import { EMS_FILE } from '../../../../common/constants';
+import { EMS_FILE, FEATURE_ID_PROPERTY_NAME } from '../../../../common/constants';
 import { getEMSClient } from '../../../meta';
 import { EMSFileCreateSourceEditor } from './create_source_editor';
 import { i18n } from '@kbn/i18n';
@@ -79,6 +79,16 @@ export class EMSFileSource extends AbstractVectorSource {
       featureCollectionPath: 'data',
       fetchUrl: emsFileLayer.getDefaultFormatUrl()
     });
+
+    const emsIdField = emsFileLayer._config.fields.find(field => {
+      return field.type === 'id';
+    });
+    featureCollection.features.forEach((feature, index) => {
+      feature.properties[FEATURE_ID_PROPERTY_NAME] = emsIdField
+        ? feature.properties[emsIdField.id]
+        : index;
+    });
+
     return {
       data: featureCollection,
       meta: {}

x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/convert_to_geojson.js

@@ -6,7 +6,7 @@
 
 import { RENDER_AS } from './render_as';
 import { getTileBoundingBox } from './geo_tile_utils';
-import { EMPTY_FEATURE_COLLECTION } from '../../../../common/constants';
+import { EMPTY_FEATURE_COLLECTION, FEATURE_ID_PROPERTY_NAME } from '../../../../common/constants';
 
 export function convertToGeoJson({ table, renderAs }) {
 
@@ -35,7 +35,9 @@ export function convertToGeoJson({ table, renderAs }) {
       return;
     }
 
-    const properties = {};
+    const properties = {
+      [FEATURE_ID_PROPERTY_NAME]: gridKey
+    };
     metricColumns.forEach(metricColumn => {
       properties[metricColumn.aggConfig.id] = row[metricColumn.id];
     });

x-pack/legacy/plugins/maps/public/layers/sources/es_pew_pew_source/convert_to_lines.js

@@ -6,6 +6,8 @@
 
 import _ from 'lodash';
 
+import { FEATURE_ID_PROPERTY_NAME } from '../../../../common/constants';
+
 const LAT_INDEX = 0;
 const LON_INDEX = 1;
 
@@ -27,7 +29,7 @@ export function convertToLines(esResponse) {
     const sourceBuckets = _.get(destBucket, 'sourceGrid.buckets', []);
     for (let j = 0; j < sourceBuckets.length; j++) {
       const {
-        key, // eslint-disable-line no-unused-vars
+        key,
         sourceCentroid,
         ...rest
       } = sourceBuckets[j];
@@ -46,6 +48,7 @@ export function convertToLines(esResponse) {
           coordinates: [[sourceCentroid.location.lon, sourceCentroid.location.lat], dest]
         },
         properties: {
+          [FEATURE_ID_PROPERTY_NAME]: `${dest.join()},${key}`,
           ...rest
         }
       });

x-pack/legacy/plugins/maps/public/layers/sources/es_search_source/es_search_source.js

@@ -14,7 +14,13 @@ import { SearchSource } from '../../../kibana_services';
 import { hitsToGeoJson } from '../../../elasticsearch_geo_utils';
 import { CreateSourceEditor } from './create_source_editor';
 import { UpdateSourceEditor } from './update_source_editor';
-import { ES_SEARCH, ES_GEO_FIELD_TYPE, ES_SIZE_LIMIT, SORT_ORDER } from '../../../../common/constants';
+import {
+  ES_SEARCH,
+  ES_GEO_FIELD_TYPE,
+  ES_SIZE_LIMIT,
+  FEATURE_ID_PROPERTY_NAME,
+  SORT_ORDER,
+} from '../../../../common/constants';
 import { i18n } from '@kbn/i18n';
 import { getDataSourceLabel } from '../../../../common/i18n_getters';
 import { ESTooltipProperty } from '../../tooltips/es_tooltip_property';
@@ -359,7 +365,7 @@ export class ESSearchSource extends AbstractESSource {
 
   async filterAndFormatPropertiesToHtml(properties) {
     const indexPattern = await this._getIndexPattern();
-    const propertyValues = await this._loadTooltipProperties(properties._id, indexPattern);
+    const propertyValues = await this._loadTooltipProperties(properties[FEATURE_ID_PROPERTY_NAME], indexPattern);
 
     return this._descriptor.tooltipProperties.map(propertyName => {
       return new ESTooltipProperty(propertyName, propertyName, propertyValues[propertyName], indexPattern);
@@ -466,7 +472,7 @@ export class ESSearchSource extends AbstractESSource {
 
     return {
       index: properties._index, // Can not use index pattern title because it may reference many indices
-      id: properties._id,
+      id: properties[FEATURE_ID_PROPERTY_NAME],
       path: geoField.name,
     };
   }

x-pack/legacy/plugins/maps/public/layers/sources/kibana_regionmap_source/kibana_regionmap_source.js

@@ -10,6 +10,7 @@ import { CreateSourceEditor } from './create_source_editor';
 import { getKibanaRegionList } from '../../../meta';
 import { i18n } from '@kbn/i18n';
 import { getDataSourceLabel } from '../../../../common/i18n_getters';
+import { FEATURE_ID_PROPERTY_NAME } from '../../../../common/constants';
 
 export class KibanaRegionmapSource extends AbstractVectorSource {
 
@@ -80,6 +81,9 @@ export class KibanaRegionmapSource extends AbstractVectorSource {
       featureCollectionPath: vectorFileMeta.meta.feature_collection_path,
       fetchUrl: vectorFileMeta.url
     });
+    featureCollection.features.forEach((feature, index) => {
+      feature.properties[FEATURE_ID_PROPERTY_NAME] = index;
+    });
     return {
       data: featureCollection
     };

x-pack/legacy/plugins/maps/public/layers/vector_layer.js

@@ -544,10 +544,8 @@ export class VectorLayer extends AbstractLayer {
     for (let i = 0; i < featureCollection.features.length; i++) {
       const id = randomizedIds[i];
       const feature = featureCollection.features[i];
-      feature.properties[FEATURE_ID_PROPERTY_NAME] = id;
-      feature.id = id;
+      feature.id = id; // Mapbox feature state id, must be integer
     }
-
   }
 
   async syncData(syncContext) {