From 02b6c931695ae4d6e869150e45c8bf333131f673 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Sun, 12 Apr 2020 19:48:47 -0600 Subject: [PATCH 1/4] [Maps] fix bug where toggling Scaling type does not re-fetch data --- x-pack/plugins/maps/public/reducers/map.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/maps/public/reducers/map.js b/x-pack/plugins/maps/public/reducers/map.js index 1e20df89c8fad0..599573ac3fd695 100644 --- a/x-pack/plugins/maps/public/reducers/map.js +++ b/x-pack/plugins/maps/public/reducers/map.js @@ -84,7 +84,9 @@ const updateLayerSourceDescriptorProp = (state, layerId, propName, value, newLay [propName]: value, }, }; - if (newLayerType) { + if (newLayerType && newLayerType !== layerList[layerIdx].type) { + // clear out data requests for previous layer type + delete updatedLayer.__dataRequests; updatedLayer.type = newLayerType; } const updatedList = [ From 4e04bfff23075f7f59730d60adf6803bc78ca53e Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Sun, 12 Apr 2020 19:56:54 -0600 Subject: [PATCH 2/4] reset to empty array instead of deleting --- x-pack/plugins/maps/public/reducers/map.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/maps/public/reducers/map.js b/x-pack/plugins/maps/public/reducers/map.js index 599573ac3fd695..7a228717747813 100644 --- a/x-pack/plugins/maps/public/reducers/map.js +++ b/x-pack/plugins/maps/public/reducers/map.js @@ -86,7 +86,7 @@ const updateLayerSourceDescriptorProp = (state, layerId, propName, value, newLay }; if (newLayerType && newLayerType !== layerList[layerIdx].type) { // clear out data requests for previous layer type - delete updatedLayer.__dataRequests; + updatedLayer.__dataRequests = []; updatedLayer.type = newLayerType; } const updatedList = [ From d18d3e72fca34bd7f4f65decc675111309a74685 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Mon, 13 Apr 2020 06:21:41 -0600 Subject: [PATCH 3/4] move setting of layer type to action creator instead of side effect of UPDATE_SOURCE_PROP --- .../maps/public/actions/map_actions.js | 28 ++++++++++++++++++- x-pack/plugins/maps/public/reducers/map.js | 15 ++-------- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/actions/map_actions.js b/x-pack/legacy/plugins/maps/public/actions/map_actions.js index aa55cf0808ef21..43eb73ae8719c9 100644 --- a/x-pack/legacy/plugins/maps/public/actions/map_actions.js +++ b/x-pack/legacy/plugins/maps/public/actions/map_actions.js @@ -663,13 +663,39 @@ export function updateSourceProp(layerId, propName, value, newLayerType) { layerId, propName, value, - newLayerType, }); + if (newLayerType) { + dispatch(updateLayerType(layerId, newLayerType)); + } await dispatch(clearMissingStyleProperties(layerId)); dispatch(syncDataForLayer(layerId)); }; } +function updateLayerType(layerId, newLayerType) { + return (dispatch, getState) => { + const layer = getLayerById(layerId, getState()); + if (!layer || layer.getType() === newLayerType) { + return; + } + layer.getInFlightRequestTokens().forEach(requestToken => { + dispatch(cancelRequest(requestToken)); + }); + dispatch({ + type: UPDATE_LAYER_PROP, + id: layerId, + propName: '__dataRequests', + newValue: [], + }); + dispatch({ + type: UPDATE_LAYER_PROP, + id: layerId, + propName: 'type', + newValue: newLayerType, + }); + }; +} + export function syncDataForLayer(layerId) { return async (dispatch, getState) => { const targetLayer = getLayerById(layerId, getState()); diff --git a/x-pack/plugins/maps/public/reducers/map.js b/x-pack/plugins/maps/public/reducers/map.js index 7a228717747813..7e07569b44b830 100644 --- a/x-pack/plugins/maps/public/reducers/map.js +++ b/x-pack/plugins/maps/public/reducers/map.js @@ -74,7 +74,7 @@ const updateLayerInList = (state, layerId, attribute, newValue) => { return { ...state, layerList: updatedList }; }; -const updateLayerSourceDescriptorProp = (state, layerId, propName, value, newLayerType) => { +const updateLayerSourceDescriptorProp = (state, layerId, propName, value) => { const { layerList } = state; const layerIdx = getLayerIndex(layerList, layerId); const updatedLayer = { @@ -84,11 +84,6 @@ const updateLayerSourceDescriptorProp = (state, layerId, propName, value, newLay [propName]: value, }, }; - if (newLayerType && newLayerType !== layerList[layerIdx].type) { - // clear out data requests for previous layer type - updatedLayer.__dataRequests = []; - updatedLayer.type = newLayerType; - } const updatedList = [ ...layerList.slice(0, layerIdx), updatedLayer, @@ -263,13 +258,7 @@ export function map(state = INITIAL_STATE, action) { case UPDATE_LAYER_PROP: return updateLayerInList(state, action.id, action.propName, action.newValue); case UPDATE_SOURCE_PROP: - return updateLayerSourceDescriptorProp( - state, - action.layerId, - action.propName, - action.value, - action.newLayerType - ); + return updateLayerSourceDescriptorProp(state, action.layerId, action.propName, action.value); case SET_JOINS: const layerDescriptor = state.layerList.find( descriptor => descriptor.id === action.layer.getId() From 61fed6904aea0d4357bdbb7d0d8ad6d04726c662 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Mon, 13 Apr 2020 09:36:24 -0600 Subject: [PATCH 4/4] review feedback --- .../maps/public/actions/map_actions.js | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/actions/map_actions.js b/x-pack/legacy/plugins/maps/public/actions/map_actions.js index 43eb73ae8719c9..7bfbf5761c5b8c 100644 --- a/x-pack/legacy/plugins/maps/public/actions/map_actions.js +++ b/x-pack/legacy/plugins/maps/public/actions/map_actions.js @@ -125,9 +125,21 @@ async function syncDataForAllLayers(dispatch, getState, dataFilters) { export function cancelAllInFlightRequests() { return (dispatch, getState) => { getLayerList(getState()).forEach(layer => { - layer.getInFlightRequestTokens().forEach(requestToken => { - dispatch(cancelRequest(requestToken)); - }); + dispatch(clearDataRequests(layer)); + }); + }; +} + +function clearDataRequests(layer) { + return dispatch => { + layer.getInFlightRequestTokens().forEach(requestToken => { + dispatch(cancelRequest(requestToken)); + }); + dispatch({ + type: UPDATE_LAYER_PROP, + id: layer.getId(), + propName: '__dataRequests', + newValue: [], }); }; } @@ -678,15 +690,7 @@ function updateLayerType(layerId, newLayerType) { if (!layer || layer.getType() === newLayerType) { return; } - layer.getInFlightRequestTokens().forEach(requestToken => { - dispatch(cancelRequest(requestToken)); - }); - dispatch({ - type: UPDATE_LAYER_PROP, - id: layerId, - propName: '__dataRequests', - newValue: [], - }); + dispatch(clearDataRequests(layer)); dispatch({ type: UPDATE_LAYER_PROP, id: layerId,