From 70d6d4241050dd91c692f47b295d4a40c985a323 Mon Sep 17 00:00:00 2001 From: jansule Date: Wed, 4 Jan 2023 14:49:10 +0100 Subject: [PATCH 1/2] bfs: datastructure refactoring --- superset-frontend/package-lock.json | 17 ++++++++++ .../plugins/plugin-chart-ol/package.json | 2 ++ .../src/components/ChartLayer.tsx | 15 +++++---- .../plugins/plugin-chart-ol/src/types.ts | 22 +++++++++++-- .../plugin-chart-ol/src/util/chartUtil.tsx | 33 ++++++++++--------- .../src/util/controlPanelUtil.tsx | 5 ++- .../plugin-chart-ol/src/util/geometryUtil.ts | 30 +++-------------- .../plugin-chart-ol/src/util/mapUtil.tsx | 14 ++++---- .../src/util/transformPropsUtil.ts | 17 +++++++--- 9 files changed, 89 insertions(+), 66 deletions(-) diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index db612c8d07893..83a42ca472ff7 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -33340,6 +33340,14 @@ "node": ">=6.9.0" } }, + "node_modules/geojson": { + "version": "0.5.0", + "resolved": "https://registry.npmjs.org/geojson/-/geojson-0.5.0.tgz", + "integrity": "sha512-/Bx5lEn+qRF4TfQ5aLu6NH+UKtvIv7Lhc487y/c8BdludrCTpiWf9wyI0RTyqg49MFefIAvFDuEi5Dfd/zgNxQ==", + "engines": { + "node": ">= 0.10" + } + }, "node_modules/geojson-flatten": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/geojson-flatten/-/geojson-flatten-1.0.4.tgz", @@ -62043,6 +62051,8 @@ "version": "0.0.1", "license": "Apache-2.0", "dependencies": { + "@types/geojson": "^7946.0.10", + "geojson": "^0.5.0", "geostyler": "^11.0.0", "geostyler-data": "^1.0.0", "geostyler-openlayers-parser": "^4.0.0", @@ -77221,7 +77231,9 @@ "requires": { "@airbnb/config-babel": "^2.0.1", "@babel/cli": "^7.16.0", + "@types/geojson": "^7946.0.10", "@types/jest": "^26.0.4", + "geojson": "^0.5.0", "geostyler": "^11.0.0", "geostyler-data": "^1.0.0", "geostyler-openlayers-parser": "^4.0.0", @@ -88697,6 +88709,11 @@ "resolved": "https://registry.npmjs.org/gensync/-/gensync-1.0.0-beta.2.tgz", "integrity": "sha512-3hN7NaskYvMDLQY55gnW3NQ+mesEAepTqlg+VEbj7zzqEMBVNhzcGYYeqFo/TlYz6eQiFcp1HcsCZO+nGgS8zg==" }, + "geojson": { + "version": "0.5.0", + "resolved": "https://registry.npmjs.org/geojson/-/geojson-0.5.0.tgz", + "integrity": "sha512-/Bx5lEn+qRF4TfQ5aLu6NH+UKtvIv7Lhc487y/c8BdludrCTpiWf9wyI0RTyqg49MFefIAvFDuEi5Dfd/zgNxQ==" + }, "geojson-flatten": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/geojson-flatten/-/geojson-flatten-1.0.4.tgz", diff --git a/superset-frontend/plugins/plugin-chart-ol/package.json b/superset-frontend/plugins/plugin-chart-ol/package.json index 1b5bec08100d2..5e797dd157ebf 100644 --- a/superset-frontend/plugins/plugin-chart-ol/package.json +++ b/superset-frontend/plugins/plugin-chart-ol/package.json @@ -30,6 +30,8 @@ "access": "public" }, "dependencies": { + "@types/geojson": "^7946.0.10", + "geojson": "^0.5.0", "geostyler": "^11.0.0", "geostyler-data": "^1.0.0", "geostyler-openlayers-parser": "^4.0.0", diff --git a/superset-frontend/plugins/plugin-chart-ol/src/components/ChartLayer.tsx b/superset-frontend/plugins/plugin-chart-ol/src/components/ChartLayer.tsx index 29999a32ed30d..24da61f297ad1 100644 --- a/superset-frontend/plugins/plugin-chart-ol/src/components/ChartLayer.tsx +++ b/superset-frontend/plugins/plugin-chart-ol/src/components/ChartLayer.tsx @@ -10,7 +10,7 @@ import { SupportedVizTypes, } from '../types'; import { createChartComponent } from '../util/chartUtil'; -import { getCoordinateFromLocation } from '../util/geometryUtil'; +import { getCoordinateFromGeometry } from '../util/geometryUtil'; /** * Custom OpenLayers layer that displays a number of charts @@ -19,7 +19,10 @@ import { getCoordinateFromLocation } from '../util/geometryUtil'; export class ChartLayer extends Layer { chartsPerZoom: ChartsPerZoom = {}; - chartConfigs: ChartConfig = {}; + chartConfigs: ChartConfig = { + type: 'FeatureCollection', + features: [], + }; chartSizeValues: ChartSizeValues = {}; @@ -187,8 +190,7 @@ export class ChartLayer extends Layer { * @param zoom The zoom level. */ createChartsPerZoom(zoom: number) { - const locations = Object.keys(this.chartConfigs); - const charts = locations.map(loc => { + const charts = this.chartConfigs.features.map(feature => { const container = document.createElement('div'); let chartWidth = 0; @@ -200,9 +202,8 @@ export class ChartLayer extends Layer { // add location to container const chartComponent = createChartComponent( - loc, this.chartVizType, - this.chartConfigs, + feature, chartWidth, chartHeight, ); @@ -210,7 +211,7 @@ export class ChartLayer extends Layer { return { htmlElement: container, - coordinate: getCoordinateFromLocation(loc), + coordinate: getCoordinateFromGeometry(feature.geometry), width: chartWidth, height: chartHeight, }; diff --git a/superset-frontend/plugins/plugin-chart-ol/src/types.ts b/superset-frontend/plugins/plugin-chart-ol/src/types.ts index 21a22291ed671..7eda64d789890 100644 --- a/superset-frontend/plugins/plugin-chart-ol/src/types.ts +++ b/superset-frontend/plugins/plugin-chart-ol/src/types.ts @@ -26,6 +26,7 @@ import Source from 'ol/source/Source'; import { Coordinate } from 'ol/coordinate'; import { DataNode, TreeProps } from 'antd/lib/tree'; import { Map } from 'ol'; +import { Feature, FeatureCollection, Point } from 'geojson'; export interface SupersetOlPluginStylesProps { height: number; @@ -42,11 +43,26 @@ export type SupportedVizTypes = | 'echarts_timeseries_scatter' | 'echarts_timeseries_step'; -// TODO check if we can import an existing typing, instead -export type ChartConfig = { - [key: string]: any; +// TODO find a way to reference props from plugin-chart-echarts +export type ChartConfigProperties = { + setDataMask: any; + labelMap: any; + labelMapB: any; + groupby: any; + selectedValues: any; + formData: any; + groupbyB: any; + seriesBreakdown: any; + legendData: any; + echartOptions: any; }; +export type ChartConfigFeature = Feature; +export type ChartConfig = FeatureCollection< + ChartConfigFeature['geometry'], + ChartConfigFeature['properties'] +>; + interface SupersetOlPluginCustomizeProps { headerText: string; geomColumn: string; diff --git a/superset-frontend/plugins/plugin-chart-ol/src/util/chartUtil.tsx b/superset-frontend/plugins/plugin-chart-ol/src/util/chartUtil.tsx index e78abe5e5e9dd..89bcf2cde2677 100644 --- a/superset-frontend/plugins/plugin-chart-ol/src/util/chartUtil.tsx +++ b/superset-frontend/plugins/plugin-chart-ol/src/util/chartUtil.tsx @@ -23,7 +23,7 @@ import { EchartsPie, EchartsTimeseries, } from '@superset-ui/plugin-chart-echarts'; -import { ChartConfig } from '../types'; +import { ChartConfigFeature } from '../types'; /** * Create a chart component for a location. @@ -36,24 +36,25 @@ import { ChartConfig } from '../types'; * @returns The chart as React component */ export const createChartComponent = ( - loc: string, chartVizType: string, - chartConfigs: ChartConfig, + chartConfig: ChartConfigFeature, chartWidth: number, chartHeight: number, ) => { let chartComponent; - const config = chartConfigs[loc].echartOptions; - const { setDataMask } = chartConfigs[loc]; - const { labelMap } = chartConfigs[loc]; - const { labelMapB } = chartConfigs[loc]; - const { groupby } = chartConfigs[loc]; - const { selectedValues } = chartConfigs[loc]; - const { formData } = chartConfigs[loc]; - const { groupbyB } = chartConfigs[loc]; - const { seriesBreakdown } = chartConfigs[loc]; - const { legendData } = chartConfigs[loc]; + const { + setDataMask, + labelMap, + labelMapB, + groupby, + selectedValues, + formData, + groupbyB, + seriesBreakdown, + legendData, + echartOptions, + } = chartConfig.properties; switch (chartVizType) { case 'pie': @@ -63,7 +64,7 @@ export const createChartComponent = ( { + response.result.forEach((config: any) => { const configString = JSON.stringify(config); const sameId = config.id === parsedValue.id; const isUpdated = configString !== value; @@ -115,7 +114,7 @@ export const selectedChartMutator = ( }); } } else { - response.result.forEach((config: ChartConfig) => { + response.result.forEach((config: any) => { const configString = JSON.stringify(config); const label = config.slice_name; diff --git a/superset-frontend/plugins/plugin-chart-ol/src/util/geometryUtil.ts b/superset-frontend/plugins/plugin-chart-ol/src/util/geometryUtil.ts index c6e1f957f15c8..d2333c1371d93 100644 --- a/superset-frontend/plugins/plugin-chart-ol/src/util/geometryUtil.ts +++ b/superset-frontend/plugins/plugin-chart-ol/src/util/geometryUtil.ts @@ -25,28 +25,7 @@ import GeoJSON from 'ol/format/GeoJSON'; import Feature from 'ol/Feature'; import { Point } from 'ol/geom'; import VectorSource from 'ol/source/Vector'; - -/** - * Converts locations to OpenLayers features. - * - * @param locations The incoming data - * - * @returns An array of OpenLayers features - */ -export const convertLocationsToFeatures = (locations: Array) => { - const features = locations.map((geoJsonString: String) => { - const geom3857 = new GeoJSON().readGeometry(geoJsonString, { - // TODO: adapt to map projection - featureProjection: 'EPSG:3857', - }); - - return new Feature({ - geometry: geom3857, - }); - }); - - return features; -}; +import { Geometry } from 'geojson'; /** * Converts a location string to a coordinate. @@ -55,8 +34,9 @@ export const convertLocationsToFeatures = (locations: Array) => { * * @returns The coordinate */ -export const getCoordinateFromLocation = (location: string) => { - const geom: Point = new GeoJSON().readGeometry(location, { +export const getCoordinateFromGeometry = (geometry: Geometry) => { + // TODO change signature so the we already get a coordinate array + const geom: Point = new GeoJSON().readGeometry(geometry, { // TODO: adapt to map projection featureProjection: 'EPSG:3857', }) as Point; @@ -69,7 +49,7 @@ export const getCoordinateFromLocation = (location: string) => { * @param features An Array of OpenLayers features * @returns The OpenLayers extent or undefined */ -export const getExtendFromFeatures = (features: Feature[]) => { +export const getExtentFromFeatures = (features: Feature[]) => { if (features.length === 0) { return undefined; } diff --git a/superset-frontend/plugins/plugin-chart-ol/src/util/mapUtil.tsx b/superset-frontend/plugins/plugin-chart-ol/src/util/mapUtil.tsx index a7fb958154975..94b95c0e1e91f 100644 --- a/superset-frontend/plugins/plugin-chart-ol/src/util/mapUtil.tsx +++ b/superset-frontend/plugins/plugin-chart-ol/src/util/mapUtil.tsx @@ -18,11 +18,9 @@ */ import { Map } from 'ol'; +import GeoJSON from 'ol/format/GeoJSON'; import { ChartConfig } from '../types'; -import { - convertLocationsToFeatures, - getExtendFromFeatures, -} from './geometryUtil'; +import { getExtentFromFeatures } from './geometryUtil'; // default map extent of world if no features are found // TODO: move to generic config file or plugin configuration @@ -35,10 +33,12 @@ const defaultExtent = [-16000000, -7279000, 20500000, 11000000]; export const fitMapToCharts = (olMap: Map, chartConfigs: ChartConfig) => { const view = olMap.getView(); + const features = new GeoJSON().readFeatures(chartConfigs, { + // TODO: adapt to map projection + featureProjection: 'EPSG:3857', + }); - const locations = Object.keys(chartConfigs).map(c => JSON.parse(c)); - const features = convertLocationsToFeatures(locations); - const extent = getExtendFromFeatures(features) || defaultExtent; + const extent = getExtentFromFeatures(features) || defaultExtent; view.fit(extent, { // tested for a desktop size monitor diff --git a/superset-frontend/plugins/plugin-chart-ol/src/util/transformPropsUtil.ts b/superset-frontend/plugins/plugin-chart-ol/src/util/transformPropsUtil.ts index dcef4b9ae37be..273b95206af54 100644 --- a/superset-frontend/plugins/plugin-chart-ol/src/util/transformPropsUtil.ts +++ b/superset-frontend/plugins/plugin-chart-ol/src/util/transformPropsUtil.ts @@ -26,6 +26,7 @@ import { LocationConfigMapping, SelectedChartConfig, ChartConfig, + ChartConfigFeature, } from '../types'; /** @@ -167,7 +168,10 @@ export const getEchartConfigs = ( break; } - const chartConfigs: ChartConfig = {}; + const chartConfigs: ChartConfig = { + type: 'FeatureCollection', + features: [], + }; Object.keys(dataByLocation).forEach(location => { const { queriesData } = chartProps; @@ -192,10 +196,13 @@ export const getEchartConfigs = ( // TODO create proper clone of argument const transformedProps = chartTransformer(config); - if (!Object.keys(chartConfigs).includes(location)) { - chartConfigs[location] = {}; - } - chartConfigs[location] = transformedProps; + const feature: ChartConfigFeature = { + type: 'Feature', + geometry: JSON.parse(location), + properties: transformedProps, + }; + + chartConfigs.features.push(feature); }); return chartConfigs; }; From adcdb4eb7a76c6fa5c4bf13b065f735d1de96994 Mon Sep 17 00:00:00 2001 From: jansule Date: Thu, 5 Jan 2023 14:48:09 +0100 Subject: [PATCH 2/2] bfs: remove confusing comment --- .../plugins/plugin-chart-ol/src/util/geometryUtil.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/superset-frontend/plugins/plugin-chart-ol/src/util/geometryUtil.ts b/superset-frontend/plugins/plugin-chart-ol/src/util/geometryUtil.ts index d2333c1371d93..48a31b2078997 100644 --- a/superset-frontend/plugins/plugin-chart-ol/src/util/geometryUtil.ts +++ b/superset-frontend/plugins/plugin-chart-ol/src/util/geometryUtil.ts @@ -35,7 +35,6 @@ import { Geometry } from 'geojson'; * @returns The coordinate */ export const getCoordinateFromGeometry = (geometry: Geometry) => { - // TODO change signature so the we already get a coordinate array const geom: Point = new GeoJSON().readGeometry(geometry, { // TODO: adapt to map projection featureProjection: 'EPSG:3857',