diff --git a/x-pack/plugins/maps/public/actions/store_actions.js b/x-pack/plugins/maps/public/actions/store_actions.js index 8c8d8c71f06f..2ca017243636 100644 --- a/x-pack/plugins/maps/public/actions/store_actions.js +++ b/x-pack/plugins/maps/public/actions/store_actions.js @@ -37,7 +37,7 @@ export const SET_JOINS = 'SET_JOINS'; export const SET_QUERY = 'SET_QUERY'; export const TRIGGER_REFRESH_TIMER = 'TRIGGER_REFRESH_TIMER'; export const UPDATE_LAYER_PROP = 'UPDATE_LAYER_PROP'; -export const UPDATE_LAYER_STYLE_FOR_SELECTED_LAYER = 'UPDATE_LAYER_STYLE'; +export const UPDATE_LAYER_STYLE = 'UPDATE_LAYER_STYLE'; export const PROMOTE_TEMPORARY_STYLES = 'PROMOTE_TEMPORARY_STYLES'; export const CLEAR_TEMPORARY_STYLES = 'CLEAR_TEMPORARY_STYLES'; export const TOUCH_LAYER = 'TOUCH_LAYER'; @@ -328,13 +328,14 @@ export function onDataLoadError(layerId, dataId, requestToken, errorMessage) { } export function updateSourceProp(layerId, propName, value) { - return (dispatch) => { + return async (dispatch) => { dispatch({ type: UPDATE_SOURCE_PROP, layerId, propName, value, }); + await dispatch(clearMissingStyleProperties(layerId)); dispatch(syncDataForLayer(layerId)); }; } @@ -458,25 +459,62 @@ export function triggerRefreshTimer() { }; } -export function updateLayerStyleForSelectedLayer(style, temporary = true) { - return { - type: UPDATE_LAYER_STYLE_FOR_SELECTED_LAYER, - style: { - ...style, - temporary - }, +export function clearMissingStyleProperties(layerId) { + return async (dispatch, getState) => { + const targetLayer = getLayerList(getState()).find(layer => { + return layer.getId() === layerId; + }); + if (!targetLayer) { + return; + } + + const style = targetLayer.getCurrentStyle(); + if (!style) { + return; + } + const ordinalFields = await targetLayer.getOrdinalFields(); + const { hasChanges, nextStyleDescriptor } = style.getDescriptorWithMissingStylePropsRemoved(ordinalFields); + if (hasChanges) { + dispatch(updateLayerStyle(layerId, nextStyleDescriptor)); + } + }; +} + +export function updateLayerStyle(layerId, styleDescriptor, temporary = true) { + return (dispatch) => { + dispatch({ + type: UPDATE_LAYER_STYLE, + layerId, + style: { + ...styleDescriptor, + temporary + }, + }); + + // Style update may require re-fetch, for example ES search may need to retrieve field used for dynamic styling + dispatch(syncDataForLayer(layerId)); + }; +} + +export function updateLayerStyleForSelectedLayer(styleDescriptor, temporary = true) { + return (dispatch, getState) => { + const selectedLayer = getSelectedLayer(getState()); + if (!selectedLayer) { + return; + } + dispatch(updateLayerStyle(selectedLayer.getId(), styleDescriptor, temporary)); }; } export function setJoinsForLayer(layer, joins) { return async (dispatch) => { - console.warn('Not Implemented: must remove any styles that are driven by join-fields that are no longer present'); await dispatch({ type: SET_JOINS, layer: layer, joins: joins }); + await dispatch(clearMissingStyleProperties(layer.getId())); dispatch(syncDataForLayer(layer.getId())); }; } diff --git a/x-pack/plugins/maps/public/shared/layers/styles/abstract_style.js b/x-pack/plugins/maps/public/shared/layers/styles/abstract_style.js new file mode 100644 index 000000000000..5e89dbe6a4b6 --- /dev/null +++ b/x-pack/plugins/maps/public/shared/layers/styles/abstract_style.js @@ -0,0 +1,14 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export class AbstractStyle { + + getDescriptorWithMissingStylePropsRemoved(/* nextOrdinalFields */) { + return { + hasChanges: false, + }; + } +} diff --git a/x-pack/plugins/maps/public/shared/layers/styles/heatmap_style.js b/x-pack/plugins/maps/public/shared/layers/styles/heatmap_style.js index 2fa18b0bda14..ca2e7062a899 100644 --- a/x-pack/plugins/maps/public/shared/layers/styles/heatmap_style.js +++ b/x-pack/plugins/maps/public/shared/layers/styles/heatmap_style.js @@ -5,12 +5,14 @@ */ import { GRID_RESOLUTION } from '../grid_resolution'; +import { AbstractStyle } from './abstract_style'; -export class HeatmapStyle { +export class HeatmapStyle extends AbstractStyle { static type = 'HEATMAP'; constructor(styleDescriptor = {}) { + super(); this._descriptor = HeatmapStyle.createDescriptor( styleDescriptor.refinement, styleDescriptor.properties diff --git a/x-pack/plugins/maps/public/shared/layers/styles/tile_style.js b/x-pack/plugins/maps/public/shared/layers/styles/tile_style.js index fc07c01aeafc..368555da5ae1 100644 --- a/x-pack/plugins/maps/public/shared/layers/styles/tile_style.js +++ b/x-pack/plugins/maps/public/shared/layers/styles/tile_style.js @@ -4,11 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -export class TileStyle { +import { AbstractStyle } from './abstract_style'; + +export class TileStyle extends AbstractStyle { static type = 'TILE'; constructor(styleDescriptor = {}) { + super(); this._descriptor = TileStyle.createDescriptor(styleDescriptor.properties); } diff --git a/x-pack/plugins/maps/public/shared/layers/styles/vector_style.js b/x-pack/plugins/maps/public/shared/layers/styles/vector_style.js index a20289a0a3eb..ddd8ed25252b 100644 --- a/x-pack/plugins/maps/public/shared/layers/styles/vector_style.js +++ b/x-pack/plugins/maps/public/shared/layers/styles/vector_style.js @@ -12,8 +12,9 @@ import { ColorGradient } from '../../icons/color_gradient'; import { getHexColorRangeStrings } from '../../utils/color_utils'; import { VectorStyleEditor } from './components/vector/vector_style_editor'; import { getDefaultStaticProperties } from './vector_style_defaults'; +import { AbstractStyle } from './abstract_style'; -export class VectorStyle { +export class VectorStyle extends AbstractStyle { static type = 'VECTOR'; static STYLE_TYPE = { 'DYNAMIC': 'DYNAMIC', 'STATIC': 'STATIC' }; @@ -23,6 +24,7 @@ export class VectorStyle { } constructor(descriptor = {}) { + super(); this._descriptor = VectorStyle.createDescriptor(descriptor.properties); } @@ -65,6 +67,63 @@ export class VectorStyle { ); } + /* + * Changes to source descriptor and join descriptor will impact style properties. + * For instance, a style property may be dynamically tied to the value of an ordinal field defined + * by a join or a metric aggregation. The metric aggregation or join may be edited or removed. + * When this happens, the style will be linked to a no-longer-existing ordinal field. + * This method provides a way for a style to clean itself and return a descriptor that unsets any dynamic + * properties that are tied to missing oridinal fields + * + * This method does not update its descriptor. It just returns a new descriptor that the caller + * can then use to update store state via dispatch. + */ + getDescriptorWithMissingStylePropsRemoved(nextOrdinalFields) { + const originalProperties = this.getProperties(); + const updatedProperties = {}; + Object.keys(originalProperties).forEach(propertyName => { + if (!this._isPropertyDynamic(propertyName)) { + return; + } + + const fieldName = _.get(originalProperties[propertyName], 'options.field.name'); + if (!fieldName) { + return; + } + + const matchingOrdinalField = nextOrdinalFields.find(oridinalField => { + return fieldName === oridinalField.name; + }); + + if (matchingOrdinalField) { + return; + } + + updatedProperties[propertyName] = { + type: VectorStyle.STYLE_TYPE.DYNAMIC, + options: { + ...originalProperties[propertyName].options + } + }; + delete updatedProperties[propertyName].options.field; + }); + + if (Object.keys(updatedProperties).length === 0) { + return { + hasChanges: false, + nextStyleDescriptor: { ...this._descriptor }, + }; + } + + return { + hasChanges: true, + nextStyleDescriptor: VectorStyle.createDescriptor({ + ...originalProperties, + ...updatedProperties, + }) + }; + } + getSourceFieldNames() { const properties = this.getProperties(); const fieldNames = []; @@ -246,7 +305,7 @@ export class VectorStyle { return _.get(styleDescriptor, 'options.color', null); } - const isDynamicConfigComplete = _.has(styleDescriptor, 'options.field') + const isDynamicConfigComplete = _.has(styleDescriptor, 'options.field.name') && _.has(styleDescriptor, 'options.color'); if (isDynamicConfigComplete) { return this._getMBDataDrivenColor({ @@ -263,7 +322,7 @@ export class VectorStyle { return styleDescriptor.options.size; } - const isDynamicConfigComplete = _.has(styleDescriptor, 'options.field') + const isDynamicConfigComplete = _.has(styleDescriptor, 'options.field.name') && _.has(styleDescriptor, 'options.minSize') && _.has(styleDescriptor, 'options.maxSize'); if (isDynamicConfigComplete) { diff --git a/x-pack/plugins/maps/public/shared/layers/styles/vector_style.test.js b/x-pack/plugins/maps/public/shared/layers/styles/vector_style.test.js new file mode 100644 index 000000000000..a732e0d1a71c --- /dev/null +++ b/x-pack/plugins/maps/public/shared/layers/styles/vector_style.test.js @@ -0,0 +1,68 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { VectorStyle } from './vector_style'; + +describe('getDescriptorWithMissingStylePropsRemoved', () => { + const fieldName = 'doIStillExist'; + const properties = { + fillColor: { + type: VectorStyle.STYLE_TYPE.STATIC, + options: {} + }, + lineColor: { + type: VectorStyle.STYLE_TYPE.DYNAMIC, + options: {} + }, + iconSize: { + type: VectorStyle.STYLE_TYPE.DYNAMIC, + options: { + color: 'a color', + field: { name: fieldName } + } + } + }; + + it('Should return no changes when next oridinal fields contain existing style property fields', () => { + const vectorStyle = new VectorStyle({ properties }); + + const nextOridinalFields = [ + { name: fieldName } + ]; + const { hasChanges } = vectorStyle.getDescriptorWithMissingStylePropsRemoved(nextOridinalFields); + expect(hasChanges).toBe(false); + }); + + it('Should clear missing fields when next oridinal fields do not contain existing style property fields', () => { + const vectorStyle = new VectorStyle({ properties }); + + const nextOridinalFields = []; + const { hasChanges, nextStyleDescriptor } = vectorStyle.getDescriptorWithMissingStylePropsRemoved(nextOridinalFields); + expect(hasChanges).toBe(true); + expect(nextStyleDescriptor.properties).toEqual({ + fillColor: { + options: {}, + type: 'STATIC', + }, + iconSize: { + options: { + color: 'a color', + }, + type: 'DYNAMIC', + }, + lineColor: { + options: {}, + type: 'DYNAMIC', + }, + lineWidth: { + options: { + size: 1, + }, + type: 'STATIC', + }, + }); + }); +}); diff --git a/x-pack/plugins/maps/public/store/map.js b/x-pack/plugins/maps/public/store/map.js index 9b5c48a20fe9..770f48fc231d 100644 --- a/x-pack/plugins/maps/public/store/map.js +++ b/x-pack/plugins/maps/public/store/map.js @@ -22,7 +22,7 @@ import { MAP_DESTROYED, SET_QUERY, UPDATE_LAYER_PROP, - UPDATE_LAYER_STYLE_FOR_SELECTED_LAYER, + UPDATE_LAYER_STYLE, PROMOTE_TEMPORARY_STYLES, CLEAR_TEMPORARY_STYLES, SET_JOINS, @@ -247,8 +247,8 @@ export function map(state = INITIAL_STATE, action) { // TODO: Simplify cases below case TOGGLE_LAYER_VISIBLE: return updateLayerInList(state, action.layerId, 'visible'); - case UPDATE_LAYER_STYLE_FOR_SELECTED_LAYER: - const styleLayerId = state.selectedLayerId; + case UPDATE_LAYER_STYLE: + const styleLayerId = action.layerId; const styleLayerIdx = getLayerIndex(state.layerList, styleLayerId); const layerStyle = state.layerList[styleLayerIdx].style; const layerPrevStyle = layerStyle.previousStyle || layerStyle;