From 513c24f02f077b8cf288636c69eb90435d85f160 Mon Sep 17 00:00:00 2001 From: Ivan Date: Fri, 4 Aug 2023 11:46:38 +0200 Subject: [PATCH] Calculation of widget using maps API under FF (#658) --- CHANGELOG.md | 2 + package.json | 2 +- packages/react-api/src/api/model.js | 12 +- .../react-api/src/hooks/useCartoLayerProps.js | 4 +- .../react-api/src/hooks/useGeojsonFeatures.js | 46 ++++---- .../react-api/src/hooks/useTileFeatures.js | 46 ++++---- .../react-core/__tests__/utils/geo.test.js | 63 +++++++++++ packages/react-core/package.json | 2 +- .../react-core/src/filters/geojsonFeatures.js | 2 +- .../react-core/src/filters/tileFeatures.d.ts | 3 - .../react-core/src/filters/tileFeatures.js | 7 +- .../src/filters/tileFeaturesSpatialIndex.js | 2 +- packages/react-core/src/index.d.ts | 10 ++ packages/react-core/src/index.js | 8 ++ .../react-core/src/utils/featureFlags.d.ts | 6 + packages/react-core/src/utils/featureFlags.js | 34 ++++++ packages/react-core/src/utils/geo.d.ts | 6 + packages/react-core/src/utils/geo.js | 37 ++++++ .../react-redux/src/slices/cartoSlice.d.ts | 15 ++- packages/react-redux/src/slices/cartoSlice.js | 31 ++++- .../CategoryItem.js | 20 ++-- .../__tests__/hooks/useWidgetFetch.test.js | 107 +++++++++++++++++- .../react-widgets/__tests__/mockReduxHooks.js | 4 + .../__tests__/models/utils.test.js | 63 +++++++---- .../react-widgets/src/hooks/useWidgetFetch.js | 55 +++++++-- .../react-widgets/src/models/CategoryModel.js | 3 +- .../react-widgets/src/models/FormulaModel.js | 3 +- .../src/models/HistogramModel.js | 13 ++- packages/react-widgets/src/models/utils.js | 26 ++++- .../react-widgets/src/widgets/BarWidget.js | 14 ++- .../src/widgets/CategoryWidget.js | 14 ++- .../src/widgets/FormulaWidget.js | 20 ++-- .../src/widgets/HistogramWidget.js | 14 ++- .../react-widgets/src/widgets/PieWidget.js | 14 ++- .../react-widgets/src/widgets/RangeWidget.js | 9 +- .../src/widgets/TimeSeriesWidget.js | 11 +- yarn.lock | 21 +++- 37 files changed, 603 insertions(+), 146 deletions(-) create mode 100644 packages/react-core/__tests__/utils/geo.test.js create mode 100644 packages/react-core/src/utils/featureFlags.d.ts create mode 100644 packages/react-core/src/utils/featureFlags.js create mode 100644 packages/react-core/src/utils/geo.d.ts create mode 100644 packages/react-core/src/utils/geo.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 289fb7407..e80056086 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Not released +- Calculation of widget using maps API under FF [#658](https://github.com/CartoDB/carto-react/pull/658) + ## 1.5 ### 1.5.1 (2023-02-06) diff --git a/package.json b/package.json index 3a1402749..3f52b22f5 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "lint-staged": "^10.5.3", "nyc": "^15.1.0", "patch-package": "6.4.7", - "quadbin": "^0.1.5", + "quadbin": "^0.1.9", "react": "^17.0.1", "react-dom": "^17.0.1", "react-redux": "^7.2.2", diff --git a/packages/react-api/src/api/model.js b/packages/react-api/src/api/model.js index 3d604420f..360e172ef 100644 --- a/packages/react-api/src/api/model.js +++ b/packages/react-api/src/api/model.js @@ -9,10 +9,12 @@ const AVAILABLE_MODELS = ['category', 'histogram', 'formula', 'timeseries', 'ran /** * Execute a SQL model request. * + * @typedef { import('geojson').Polygon | import('geojson').MultiPolygon } SpatialFilter * @param { object } props * @param { string } props.model - widget's model that we want to get the data for * @param { object } props.source - source that owns the column * @param { object } props.params - widget's props + * @param { SpatialFilter= } props.spatialFilter - restrict widget calculation to an area * @param { object= } props.opts - Additional options for the HTTP request */ export function executeModel(props) { @@ -27,7 +29,7 @@ export function executeModel(props) { )}` ); - const { source, model, params, opts } = props; + const { source, model, params, spatialFilter, opts } = props; checkCredentials(source.credentials); @@ -52,13 +54,21 @@ export function executeModel(props) { filtersLogicalOperator }; + if (spatialFilter) { + queryParams.spatialFilter = JSON.stringify(spatialFilter); + } + const isGet = url.length + JSON.stringify(queryParams).length <= URL_LENGTH; if (isGet) { url += '?' + new URLSearchParams(queryParams).toString(); } else { + // undo the JSON.stringify, @todo find a better pattern queryParams.params = params; queryParams.filters = filters; queryParams.queryParameters = source.queryParameters; + if (spatialFilter) { + queryParams.spatialFilter = spatialFilter; + } } return makeCall({ url, diff --git a/packages/react-api/src/hooks/useCartoLayerProps.js b/packages/react-api/src/hooks/useCartoLayerProps.js index de9326c46..b43406a98 100644 --- a/packages/react-api/src/hooks/useCartoLayerProps.js +++ b/packages/react-api/src/hooks/useCartoLayerProps.js @@ -1,6 +1,6 @@ import { useCallback } from 'react'; import { useSelector } from 'react-redux'; -import { selectSpatialFilter } from '@carto/react-redux'; +import { selectSpatialFilter, selectViewport } from '@carto/react-redux'; import useGeojsonFeatures from './useGeojsonFeatures'; import useTileFeatures from './useTileFeatures'; import { getDataFilterExtensionProps } from './dataFilterExtensionUtil'; @@ -15,7 +15,7 @@ export default function useCartoLayerProps({ viewportFeatures = true, viewporFeaturesDebounceTimeout = 250 }) { - const viewport = useSelector((state) => state.carto.viewport); + const viewport = useSelector(selectViewport); const spatialFilter = useSelector((state) => selectSpatialFilter(state, source?.id)); const [onDataLoadForGeojson] = useGeojsonFeatures({ diff --git a/packages/react-api/src/hooks/useGeojsonFeatures.js b/packages/react-api/src/hooks/useGeojsonFeatures.js index cd2a1d15b..e8f34802e 100644 --- a/packages/react-api/src/hooks/useGeojsonFeatures.js +++ b/packages/react-api/src/hooks/useGeojsonFeatures.js @@ -43,27 +43,31 @@ export default function useGeojsonFeatures({ [computeFeatures] ); - useEffect(() => { - if (sourceId && isGeoJsonLoaded) { - clearDebounce(); - setSourceFeaturesReady(false); - debounceIdRef.current = debouncedComputeFeatures({ - viewport, - spatialFilter, - uniqueIdProperty - }); - } - }, [ - viewport, - spatialFilter, - uniqueIdProperty, - sourceId, - isGeoJsonLoaded, - debouncedComputeFeatures, - setSourceFeaturesReady, - clearDebounce, - debounceIdRef - ]); + useEffect( + () => { + if (sourceId && isGeoJsonLoaded) { + clearDebounce(); + setSourceFeaturesReady(false); + debounceIdRef.current = debouncedComputeFeatures({ + viewport, + spatialFilter, + uniqueIdProperty + }); + } + }, + // eslint-disable-next-line react-hooks/exhaustive-deps + [ + // eslint-disable-next-line react-hooks/exhaustive-deps + spatialFilter ? spatialFilter : viewport, + uniqueIdProperty, + sourceId, + isGeoJsonLoaded, + debouncedComputeFeatures, + setSourceFeaturesReady, + clearDebounce, + debounceIdRef + ] + ); const onDataLoad = useCallback( (geojson) => { diff --git a/packages/react-api/src/hooks/useTileFeatures.js b/packages/react-api/src/hooks/useTileFeatures.js index 00bcf5e23..98d0c1d1d 100644 --- a/packages/react-api/src/hooks/useTileFeatures.js +++ b/packages/react-api/src/hooks/useTileFeatures.js @@ -97,27 +97,31 @@ export default function useTileFeatures({ loadTiles ]); - useEffect(() => { - if (sourceId && isTilesetLoaded) { - clearDebounce(); - setSourceFeaturesReady(false); - debounceIdRef.current = debouncedComputeFeatures({ - viewport, - spatialFilter, - uniqueIdProperty - }); - } - }, [ - viewport, - spatialFilter, - uniqueIdProperty, - debouncedComputeFeatures, - sourceId, - isTilesetLoaded, - setSourceFeaturesReady, - clearDebounce, - debounceIdRef - ]); + useEffect( + () => { + if (sourceId && isTilesetLoaded) { + clearDebounce(); + setSourceFeaturesReady(false); + debounceIdRef.current = debouncedComputeFeatures({ + viewport, + spatialFilter, + uniqueIdProperty + }); + } + }, + // eslint-disable-next-line react-hooks/exhaustive-deps + [ + // eslint-disable-next-line react-hooks/exhaustive-deps + spatialFilter ? spatialFilter : viewport, + uniqueIdProperty, + debouncedComputeFeatures, + sourceId, + isTilesetLoaded, + setSourceFeaturesReady, + clearDebounce, + debounceIdRef + ] + ); const onViewportLoad = useCallback( (tiles) => { diff --git a/packages/react-core/__tests__/utils/geo.test.js b/packages/react-core/__tests__/utils/geo.test.js new file mode 100644 index 000000000..3cbc0d57b --- /dev/null +++ b/packages/react-core/__tests__/utils/geo.test.js @@ -0,0 +1,63 @@ +import bboxPolygon from '@turf/bbox-polygon'; +import { isGlobalViewport, getGeometryToIntersect } from '../../src/utils/geo'; + +/** @type { import('../../src').Viewport } */ +const viewport = [-10, -10, 10, 10]; // west - south - east - north +const viewportGeometry = bboxPolygon(viewport).geometry; + +/** @type { import('geojson').Polygon } */ +const filterGeometry = { + type: 'Polygon', + coordinates: [ + [ + [-1, -1], + [1, -1], + [1, 1], + [-1, 1], + [-1, -1] + ] + ] +}; + +describe('isGlobalViewport', () => { + const normalViewports = [ + { v: null }, + { v: viewport }, + { + v: [-344.2596303029739, -75.05112877980663, 230.26452782294038, 75.05112877980655] + }, + { v: [-125.2596303029739, -85.05112877980663, 230.26452782294038, 85.05112877980655] } + ]; + const globalViewports = [ + { v: [-344.2596303029739, -85.05112877980663, 230.26452782294038, 85.05112877980655] } + ]; + + test.each(normalViewports)('return false for normal viewports', ({ v }) => { + expect(!isGlobalViewport(v)); + }); + + test.each(globalViewports)('return true for global viewports', ({ v }) => { + console.log(viewport); + expect(isGlobalViewport(v)); + }); +}); + +describe('getGeometryToIntersect', () => { + test('returns null in case no or invalid viewport or geometry is present', () => { + expect(getGeometryToIntersect(null, null)).toStrictEqual(null); + expect(getGeometryToIntersect([], null)).toStrictEqual(null); + expect(getGeometryToIntersect(null, {})).toStrictEqual(null); + expect(getGeometryToIntersect([], {})).toStrictEqual(null); + }); + + test('returns the viewport as geometry', () => { + expect(getGeometryToIntersect(viewport, null)).toStrictEqual(viewportGeometry); + }); + + test('returns the filter as geometry', () => { + expect(getGeometryToIntersect(null, filterGeometry)).toStrictEqual(filterGeometry); + expect(getGeometryToIntersect(viewport, filterGeometry)).toStrictEqual( + filterGeometry + ); + }); +}); diff --git a/packages/react-core/package.json b/packages/react-core/package.json index 4cbd08256..8b17c8020 100644 --- a/packages/react-core/package.json +++ b/packages/react-core/package.json @@ -71,6 +71,6 @@ "@turf/boolean-within": "^6.3.0", "@turf/intersect": "^6.3.0", "h3-js": "^3.7.2", - "quadbin": "^0.1.5" + "quadbin": "^0.1.9" } } diff --git a/packages/react-core/src/filters/geojsonFeatures.js b/packages/react-core/src/filters/geojsonFeatures.js index 5c7693ddc..bd8417834 100644 --- a/packages/react-core/src/filters/geojsonFeatures.js +++ b/packages/react-core/src/filters/geojsonFeatures.js @@ -1,5 +1,5 @@ import intersects from '@turf/boolean-intersects'; -import { getGeometryToIntersect } from './tileFeatures'; +import { getGeometryToIntersect } from '../utils/geo'; export function geojsonFeatures({ geojson, viewport, geometry, uniqueIdProperty }) { let uniqueIdx = 0; diff --git a/packages/react-core/src/filters/tileFeatures.d.ts b/packages/react-core/src/filters/tileFeatures.d.ts index b30085f20..3b6de3471 100644 --- a/packages/react-core/src/filters/tileFeatures.d.ts +++ b/packages/react-core/src/filters/tileFeatures.d.ts @@ -1,5 +1,2 @@ import { TileFeatures, TileFeaturesResponse } from '../types'; -import { Geometry, Feature, Polygon, MultiPolygon } from 'geojson'; - -export function getGeometryToIntersect(viewport: number[], geometry: Geometry | null): Feature | null; export function tileFeatures(arg: TileFeatures): TileFeaturesResponse; \ No newline at end of file diff --git a/packages/react-core/src/filters/tileFeatures.js b/packages/react-core/src/filters/tileFeatures.js index fd352c21a..3a1db9ece 100644 --- a/packages/react-core/src/filters/tileFeatures.js +++ b/packages/react-core/src/filters/tileFeatures.js @@ -1,12 +1,7 @@ -import bboxPolygon from '@turf/bbox-polygon'; -import intersect from '@turf/intersect'; +import { getGeometryToIntersect } from '../utils/geo'; import tileFeaturesGeometries from './tileFeaturesGeometries'; import tileFeaturesSpatialIndex from './tileFeaturesSpatialIndex'; -export function getGeometryToIntersect(viewport, geometry) { - return geometry ? intersect(bboxPolygon(viewport), geometry) : bboxPolygon(viewport); -} - export function tileFeatures({ tiles, viewport, diff --git a/packages/react-core/src/filters/tileFeaturesSpatialIndex.js b/packages/react-core/src/filters/tileFeaturesSpatialIndex.js index b9f2b2756..4caac7bb4 100644 --- a/packages/react-core/src/filters/tileFeaturesSpatialIndex.js +++ b/packages/react-core/src/filters/tileFeaturesSpatialIndex.js @@ -41,7 +41,7 @@ export default function tileFeaturesSpatialIndex({ } tile.data.forEach((d) => { - if (cellsDictionary[d.id]) { + if (d.id in cellsDictionary) { map.set(d.id, { ...d.properties, [spatialIndexIDName]: d.id }); } }); diff --git a/packages/react-core/src/index.d.ts b/packages/react-core/src/index.d.ts index d0e0c8cb9..be9db20bc 100644 --- a/packages/react-core/src/index.d.ts +++ b/packages/react-core/src/index.d.ts @@ -1,3 +1,5 @@ +import { _FeatureFlags } from '.'; + export { getRequest, postRequest, @@ -13,6 +15,7 @@ export { debounce } from './utils/debounce'; export { throttle } from './utils/throttle'; export { randomString } from './utils/randomString'; export { assert as _assert } from './utils/assert'; +export { getGeometryToIntersect, isGlobalViewport } from './utils/geo'; export { makeIntervalComplete } from './utils/makeIntervalComplete'; @@ -36,3 +39,10 @@ export { groupValuesByDateColumn } from './operations/groupByDate'; export { SpatialIndex } from './operations/constants/SpatialIndexTypes' export { FEATURE_SELECTION_MODES, EDIT_MODES, MASK_ID } from './utils/featureSelectionConstants'; + +export { + Flags as _FeatureFlags, + hasFlag as _hasFeatureFlag, + setFlags as _setFeatureFlags, + clearFlags as _clearFeatureFlags +} from './utils/featureFlags'; diff --git a/packages/react-core/src/index.js b/packages/react-core/src/index.js index 634e7becb..64acf71b4 100644 --- a/packages/react-core/src/index.js +++ b/packages/react-core/src/index.js @@ -13,6 +13,7 @@ export { debounce } from './utils/debounce'; export { throttle } from './utils/throttle'; export { randomString } from './utils/randomString'; export { assert as _assert } from './utils/assert'; +export { getGeometryToIntersect, isGlobalViewport } from './utils/geo'; export { makeIntervalComplete } from './utils/makeIntervalComplete'; @@ -47,3 +48,10 @@ export { EDIT_MODES, MASK_ID } from './utils/featureSelectionConstants'; + +export { + Flags as _FeatureFlags, + hasFlag as _hasFeatureFlag, + setFlags as _setFeatureFlags, + clearFlags as _clearFeatureFlags +} from './utils/featureFlags'; diff --git a/packages/react-core/src/utils/featureFlags.d.ts b/packages/react-core/src/utils/featureFlags.d.ts new file mode 100644 index 000000000..1beb1280d --- /dev/null +++ b/packages/react-core/src/utils/featureFlags.d.ts @@ -0,0 +1,6 @@ +export enum Flags { + REMOTE_WIDGETS = '2023-remote-widgets' +} +export function setFlags(flags: Record | string[]): void +export function clearFlags(): void +export function hasFlag(flag: string): boolean diff --git a/packages/react-core/src/utils/featureFlags.js b/packages/react-core/src/utils/featureFlags.js new file mode 100644 index 000000000..79783ee70 --- /dev/null +++ b/packages/react-core/src/utils/featureFlags.js @@ -0,0 +1,34 @@ +let featureFlags = []; + +export const Flags = Object.freeze({ + REMOTE_WIDGETS: '2023-remote-widgets' +}); + +export function setFlags(flags) { + const isValidFlag = (f) => typeof f === 'string' && f; + + if (Array.isArray(flags) && flags.every(isValidFlag)) { + featureFlags = flags; + } else if ( + !Array.isArray(flags) && + typeof flags === 'object' && + Object.keys(flags).every(isValidFlag) + ) { + featureFlags = []; + for (const [flag, value] of Object.entries(flags)) { + if (value) { + featureFlags.push(flag); + } + } + } else { + throw new Error(`Invalid feature flags: ${flags}`); + } +} + +export function clearFlags() { + featureFlags = []; +} + +export function hasFlag(flag) { + return featureFlags.includes(flag); +} diff --git a/packages/react-core/src/utils/geo.d.ts b/packages/react-core/src/utils/geo.d.ts new file mode 100644 index 000000000..fef625c1c --- /dev/null +++ b/packages/react-core/src/utils/geo.d.ts @@ -0,0 +1,6 @@ +import { Viewport } from '../types'; +import { Polygon, MultiPolygon } from 'geojson'; + +export function getGeometryToIntersect(viewport: Viewport | null, geometry: Polygon | MultiPolygon | null): Polygon | MultiPolygon | null; + +export function isGlobalViewport(viewport: Viewport | null): boolean; diff --git a/packages/react-core/src/utils/geo.js b/packages/react-core/src/utils/geo.js new file mode 100644 index 000000000..822ad4b79 --- /dev/null +++ b/packages/react-core/src/utils/geo.js @@ -0,0 +1,37 @@ +import bboxPolygon from '@turf/bbox-polygon'; + +/** + * Select the geometry to use for widget calculation and data filtering. + * If a spatial filter (mask) is set, use the mask otherwise use the current viewport. + * Since it's possible that no mask and no viewport is set, return null in this case. + * + * @typedef { import('geojson').Polygon | import('geojson').MultiPolygon } Geometry + * @typedef { import('../types').Viewport? } Viewport + * + * @param { Viewport? } viewport viewport [minX, minY, maxX, maxY], if any + * @param { Geometry? } geometry the active spatial filter (mask), if any + * @returns { Geometry? } the geometry to use for filtering + */ +export function getGeometryToIntersect(viewport, geometry) { + return geometry && geometry.coordinates + ? geometry + : Array.isArray(viewport) && viewport.length === 4 + ? bboxPolygon(viewport).geometry + : null; +} + +/** + * Check if a viewport is large enough to represent a global coverage. + * In this case the spatial filter parameter for widget calculation + * can be removed. + * + * @param { import('../types').Viewport? } viewport + * @returns { boolean } + */ +export function isGlobalViewport(viewport) { + if (viewport) { + const [minx, miny, maxx, maxy] = viewport; + return maxx - minx > 179.5 * 2 && maxy - miny > 85.05 * 2; + } + return false; +} diff --git a/packages/react-redux/src/slices/cartoSlice.d.ts b/packages/react-redux/src/slices/cartoSlice.d.ts index 7b5103745..3f503d91d 100644 --- a/packages/react-redux/src/slices/cartoSlice.d.ts +++ b/packages/react-redux/src/slices/cartoSlice.d.ts @@ -1,6 +1,6 @@ import { Credentials } from '@carto/react-api/'; import { SourceProps } from '@carto/react-api/types'; -import { FiltersLogicalOperators, _FilterTypes } from '@carto/react-core'; +import { FiltersLogicalOperators, Viewport, _FilterTypes } from '@carto/react-core'; import { CartoBasemapsNames, GMapsBasemapsNames } from '@carto/react-basemaps/'; import { InitialCartoState, CartoState, ViewState } from '../types'; import { AnyAction, Reducer } from 'redux'; @@ -138,6 +138,15 @@ export function selectAreFeaturesReadyForSource(state: any, id: string): boolean export function setViewState(viewState: ViewState): Function; +export const setViewStateDirect: (viewState: ViewState) => { + type: 'carto/setViewState'; + payload: ViewState; +}; + +export const setViewPort: () => { + type: 'carto/setViewPort'; +}; + export function setFeaturesReady(data: FeaturesReadyData): { type: CartoActions.SET_FEATURES_READY; payload: FeaturesReadyData; @@ -158,6 +167,8 @@ export function setFeatureSelectionEnabled(enabled: boolean): { payload: boolean; }; -export function selectSpatialFilter(state: any, sourceId?: string): object | null; +export function selectViewport(state: any): Viewport | null; + +export function selectSpatialFilter(state: any, sourceId?: string): Feature | null; export function selectFeatureSelectionMode(state: any): string | null; diff --git a/packages/react-redux/src/slices/cartoSlice.js b/packages/react-redux/src/slices/cartoSlice.js index 86a5b2f36..f8f206145 100644 --- a/packages/react-redux/src/slices/cartoSlice.js +++ b/packages/react-redux/src/slices/cartoSlice.js @@ -336,6 +336,13 @@ export const selectSourceById = (state, id) => state.carto.dataSources[id]; export const checkIfSourceIsDroppingFeature = (state, id) => state.carto.dataSources[id]?.isDroppingFeatures; +/** + * Redux selector to select the active viewport + */ +export const selectViewport = (state) => { + return state.carto.viewport ? state.carto.viewport : null; +}; + /** * Redux selector to select the spatial filter of a given sourceId or the root one */ @@ -373,7 +380,9 @@ const NOT_ALLOWED_DECK_PROPS = [ ]; /** - * Action to set the current ViewState + * Action to set the current ViewState. + * + * Requires redux-thunk middleware, also invokes debounced `setViewPort`. * @param {Object} viewState */ export const setViewState = (viewState) => { @@ -394,6 +403,26 @@ export const setViewState = (viewState) => { }; }; +/** + * Action to set the current ViewState. + * + * Doesn't refresh widgets immetiately, requires user to call `setViewPort` once all updates are ready. + * @param {Object} viewState + */ +export const setViewStateDirect = (viewState) => ({ + type: 'carto/setViewState', + payload: viewState +}); + +/** + * Sync current viewport state deriving it from `viewState`. + * + * Causes widgets in remote mode to refresh its data. + */ +export const setViewPort = () => ({ + type: 'carto/setViewPort' +}); + /** * Action to set the ready features state of a layer * @param {object} sourceId - the id of the source diff --git a/packages/react-ui/src/widgets/comparative/ComparativeCategoryWidgetUI/CategoryItem.js b/packages/react-ui/src/widgets/comparative/ComparativeCategoryWidgetUI/CategoryItem.js index be73d660c..49c3c70aa 100644 --- a/packages/react-ui/src/widgets/comparative/ComparativeCategoryWidgetUI/CategoryItem.js +++ b/packages/react-ui/src/widgets/comparative/ComparativeCategoryWidgetUI/CategoryItem.js @@ -27,7 +27,7 @@ function ComparativeCategoryTooltip({ item, index, names, formatter = IDENTITY_F const name = names[index]; const compareValue = ((data.value - reference.value) / (reference.value || 1)) * 100; - const signText = Math.sign(compareValue) === -1 ? '-' : '+' + const signText = Math.sign(compareValue) === -1 ? '-' : '+'; const valueColor = Math.sign(compareValue) === -1 ? theme.palette.error.main @@ -41,11 +41,7 @@ function ComparativeCategoryTooltip({ item, index, names, formatter = IDENTITY_F {item.label} - +
- {signText}{formatter(Math.abs(compareValue))} + {signText} + {formatter(Math.abs(compareValue))} @@ -78,7 +75,7 @@ ComparativeCategoryTooltip.propTypes = { item: transposedCategoryItemPropTypes, names: PropTypes.arrayOf(PropTypes.string).isRequired, formatter: PropTypes.func, - index: PropTypes.number, + index: PropTypes.number }; const StyledTooltip = withStyles((theme) => ({ @@ -111,7 +108,12 @@ function CategoryItem({ } const tooltipContent = (index) => ( - + ); return ( diff --git a/packages/react-widgets/__tests__/hooks/useWidgetFetch.test.js b/packages/react-widgets/__tests__/hooks/useWidgetFetch.test.js index 3412f3e06..c84e45bbf 100644 --- a/packages/react-widgets/__tests__/hooks/useWidgetFetch.test.js +++ b/packages/react-widgets/__tests__/hooks/useWidgetFetch.test.js @@ -3,7 +3,9 @@ import { DEFAULT_INVALID_COLUMN_ERR } from '../../src/widgets/utils/constants'; import { act, render, screen } from '@testing-library/react'; import React from 'react'; import useWidgetFetch from '../../src/hooks/useWidgetFetch'; -import { mockReduxHooks } from '../mockReduxHooks'; +import { mockClear, mockSetup } from '../mockReduxHooks'; +import { selectViewport } from '@carto/react-redux'; +import bboxPolygon from '@turf/bbox-polygon'; const PARAMS_MOCK = { column: '__test__' @@ -11,15 +13,37 @@ const PARAMS_MOCK = { const SOURCE_MOCK = { id: 'test', - data: 'testTable' + data: 'testTable', + type: 'table', + credentials: { + apiVersion: 'v3' + } }; +const viewport = [-10, -5, 8, 9]; +const spatialFilter = bboxPolygon([-10, -5, 8, 9]).geometry; + jest.mock('../../src/hooks/useWidgetSource', () => () => SOURCE_MOCK); describe('useWidgetFetch', () => { - mockReduxHooks(); + beforeAll(() => { + const { useDispatch, useSelector } = mockSetup(); + const defaultSelector = jest.fn(); + + useDispatch.mockReturnValue(jest.fn()); + useSelector.mockImplementation((selector) => { + if (selector === selectViewport) { + return viewport; + } + return defaultSelector; + }); + }); - test('should work correctly', async () => { + afterAll(() => { + mockClear(); + }); + + it('should work correctly (no remote attempt)', async () => { const onError = jest.fn(); const modelFn = jest .fn() @@ -35,6 +59,7 @@ describe('useWidgetFetch', () => { dataSource: 'test', params: PARAMS_MOCK, global: false, + attemptRemoteCalculation: false, onError }} /> @@ -44,7 +69,9 @@ describe('useWidgetFetch', () => { expect(modelFn).toBeCalledWith({ source: SOURCE_MOCK, ...PARAMS_MOCK, - global: false + global: false, + remoteCalculation: false, + spatialFilter: spatialFilter }); expect(screen.getByText('loading')).toBeInTheDocument(); @@ -65,6 +92,7 @@ describe('useWidgetFetch', () => { dataSource: 'test', params: PARAMS_MOCK, global: true, + attemptRemoteCalculation: false, onError }} /> @@ -74,7 +102,9 @@ describe('useWidgetFetch', () => { expect(modelFn).toBeCalledWith({ source: SOURCE_MOCK, ...PARAMS_MOCK, - global: true + global: true, + remoteCalculation: false, + spatialFilter: null // never in global mode }); expect(screen.getByText('loading')).toBeInTheDocument(); @@ -93,6 +123,7 @@ describe('useWidgetFetch', () => { dataSource: 'test', params: PARAMS_MOCK, global: false, + remoteCalculation: false, onError }} /> @@ -104,6 +135,70 @@ describe('useWidgetFetch', () => { expect(onError).toBeCalledTimes(1); expect(screen.queryByText(DEFAULT_INVALID_COLUMN_ERR)).toBeInTheDocument(); }); + + it('should work correctly (non-global, remote attempt)', async () => { + const onError = jest.fn(); + const modelFn = jest + .fn() + .mockImplementation( + () => new Promise((resolve) => setTimeout(() => resolve('data'), 100)) + ); + + const { rerender } = render( + + ); + + // Test modelFn is called with the right params + expect(modelFn).toBeCalledWith({ + source: SOURCE_MOCK, + ...PARAMS_MOCK, + global: false, + remoteCalculation: true, + spatialFilter: spatialFilter + }); + }); + + it('should work correctly (global, remote attempt)', async () => { + const onError = jest.fn(); + const modelFn = jest + .fn() + .mockImplementation( + () => new Promise((resolve) => setTimeout(() => resolve('data'), 100)) + ); + + const { rerender } = render( + + ); + + // Test modelFn is called with the right params + expect(modelFn).toBeCalledWith({ + source: SOURCE_MOCK, + ...PARAMS_MOCK, + global: true, + remoteCalculation: true, + spatialFilter: null // no spatial filter for glboal case + }); + }); }); // Aux diff --git a/packages/react-widgets/__tests__/mockReduxHooks.js b/packages/react-widgets/__tests__/mockReduxHooks.js index 726f5c4e7..65e4397ef 100644 --- a/packages/react-widgets/__tests__/mockReduxHooks.js +++ b/packages/react-widgets/__tests__/mockReduxHooks.js @@ -11,6 +11,10 @@ export function mockReduxHooks(dispatchValue, selectorValue) { useSelectorSpy.mockReturnValue(mockSelectorFn); } +export function mockSetup() { + return { useDispatch: useDispatchSpy, useSelector: useSelectorSpy }; +} + export function mockClear() { useDispatchSpy.mockClear(); useSelectorSpy.mockClear(); diff --git a/packages/react-widgets/__tests__/models/utils.test.js b/packages/react-widgets/__tests__/models/utils.test.js index 8a3775f78..267e3b9e8 100644 --- a/packages/react-widgets/__tests__/models/utils.test.js +++ b/packages/react-widgets/__tests__/models/utils.test.js @@ -41,19 +41,34 @@ const fromRemote = jest.fn(); describe('utils', () => { describe('wrapModelCall', () => { - test('should work correctly', () => { - const props = { source: V2_SOURCE, global: false }; - wrapModelCall(props, fromLocal, fromRemote); - expect(fromLocal).toHaveBeenCalledWith(props); - - const props2 = { source: V3_SOURCE, global: true }; - wrapModelCall(props2, fromLocal, fromRemote); - expect(fromRemote).toHaveBeenCalledWith(props2); - }); + const cases = [ + // source, global, remoteCalculation, expectedFn + [V2_SOURCE, false, false, fromLocal], + [V3_SOURCE, false, false, fromLocal], + [V3_SOURCE, true, false, fromRemote], + [V2_SOURCE, false, true, fromLocal], + [V3_SOURCE, false, true, fromRemote], + [V3_SOURCE, true, true, fromRemote] + ]; + + test.each(cases)( + 'should work correctly', + (source, global, remoteCalculation, expectedFn) => { + const props = { source, global, remoteCalculation }; + wrapModelCall(props, fromLocal, fromRemote); + expect(expectedFn).toHaveBeenCalledWith(props); + } + ); test('should throw error if global is true but fromRemote is missing', () => { expect(() => - wrapModelCall({ source: V2_SOURCE, global: true }, fromLocal) + wrapModelCall({ source: V3_SOURCE, global: true }, fromLocal) + ).toThrowError(); + }); + + test('should throw error if remoteCalculation is true but fromRemote is missing', () => { + expect(() => + wrapModelCall({ source: V3_SOURCE, remoteCalculation: true }, fromLocal) ).toThrowError(); }); @@ -102,20 +117,20 @@ describe('utils', () => { }); describe('normalizeObjectKeys', () => { - test('should work correctly', () => { - const test = { VALUE: 1 }; - const test2 = [{ TICK: 0, VALUE: 1 }]; - const test3 = [{ TICK: [{ VALUE: 0 }], VALUE: 1 }]; - - expect(JSON.stringify(normalizeObjectKeys(test))).toEqual( - JSON.stringify(test).toLowerCase() - ); - expect(JSON.stringify(normalizeObjectKeys(test2))).toEqual( - JSON.stringify(test2).toLowerCase() - ); - expect(JSON.stringify(normalizeObjectKeys(test3))).toEqual( - JSON.stringify(test3).toLowerCase() - ); + const tests = [ + // single objects + { VALUE: 1 }, + { A: null, B: undefined, C: 'hello' }, + { A: { X: null }, B: { X: undefined }, C: 'hello' }, + // array of objects + [{ TICK: 0, VALUE: 1 }], + [{ TICK: [{ VALUE: 0 }], VALUE: 1 }], + [{ A: null, B: undefined, C: 'hello' }], + [{ A: { X: null }, B: { X: undefined }, C: 'hello' }] + ]; + test.each(tests)('should work correctly for %p', (test) => { + const normalized = normalizeObjectKeys(test); + expect(JSON.stringify(normalized)).toEqual(JSON.stringify(test).toLowerCase()); }); }); diff --git a/packages/react-widgets/src/hooks/useWidgetFetch.js b/packages/react-widgets/src/hooks/useWidgetFetch.js index 7a2077950..625552021 100644 --- a/packages/react-widgets/src/hooks/useWidgetFetch.js +++ b/packages/react-widgets/src/hooks/useWidgetFetch.js @@ -1,25 +1,53 @@ -import { InvalidColumnError } from '@carto/react-core'; -import { selectAreFeaturesReadyForSource } from '@carto/react-redux'; +import { + InvalidColumnError, + getGeometryToIntersect, + isGlobalViewport +} from '@carto/react-core'; +import { + selectAreFeaturesReadyForSource, + selectSpatialFilter, + selectViewport +} from '@carto/react-redux'; import { dequal } from 'dequal'; import { useState } from 'react'; import { useSelector } from 'react-redux'; import { DEFAULT_INVALID_COLUMN_ERR } from '../widgets/utils/constants'; import useCustomCompareEffect from './useCustomCompareEffect'; import useWidgetSource from './useWidgetSource'; +import { isRemoteCalculationSupported } from '../models/utils'; export default function useWidgetFetch( modelFn, - { id, dataSource, params, global, onError, enabled = true } + { + id, + dataSource, + params, + global, + onError, + enabled = true, + attemptRemoteCalculation = false + } ) { // State const [data, setData] = useState(); const [isLoading, setIsLoading] = useState(false); const [warning, setWarning] = useState(''); + const source = useWidgetSource({ dataSource, id }); + const remoteCalculation = + attemptRemoteCalculation && isRemoteCalculationSupported({ source }); + const isSourceReady = useSelector( - (state) => global || selectAreFeaturesReadyForSource(state, dataSource) + (state) => + global || remoteCalculation || selectAreFeaturesReadyForSource(state, dataSource) ); - const source = useWidgetSource({ dataSource, id }); + + const viewport = useSelector(selectViewport); + const spatialFilter = useSelector((state) => selectSpatialFilter(state, dataSource)); + const geometryToIntersect = + global || (!spatialFilter && isGlobalViewport(viewport)) + ? null + : getGeometryToIntersect(viewport, spatialFilter ? spatialFilter.geometry : null); useCustomCompareEffect( () => { @@ -30,7 +58,9 @@ export default function useWidgetFetch( modelFn({ source, ...params, - global + global, + remoteCalculation, + spatialFilter: geometryToIntersect }) .then((data) => { if (data !== null && data !== undefined) { @@ -49,9 +79,18 @@ export default function useWidgetFetch( }); } }, - [params, source, onError, isSourceReady, global, enabled], + [ + params, + source, + onError, + isSourceReady, + global, + remoteCalculation, + geometryToIntersect, + enabled + ], dequal ); - return { data, isLoading, isSourceReady, source, warning }; + return { data, isLoading, isSourceReady, source, warning, remoteCalculation }; } diff --git a/packages/react-widgets/src/models/CategoryModel.js b/packages/react-widgets/src/models/CategoryModel.js index 2827795f6..ebde077e9 100644 --- a/packages/react-widgets/src/models/CategoryModel.js +++ b/packages/react-widgets/src/models/CategoryModel.js @@ -22,12 +22,13 @@ function fromLocal(props) { // From remote function fromRemote(props) { - const { source, abortController, ...params } = props; + const { source, spatialFilter, abortController, ...params } = props; const { column, operation, operationColumn } = params; return _executeModel({ model: 'category', source, + spatialFilter, params: { column, operation, diff --git a/packages/react-widgets/src/models/FormulaModel.js b/packages/react-widgets/src/models/FormulaModel.js index f8e2352fb..0baced5f1 100644 --- a/packages/react-widgets/src/models/FormulaModel.js +++ b/packages/react-widgets/src/models/FormulaModel.js @@ -21,12 +21,13 @@ function fromLocal(props) { // From remote function fromRemote(props) { - const { source, abortController, ...params } = props; + const { source, spatialFilter, abortController, ...params } = props; const { column, operation } = params; return _executeModel({ model: 'formula', source, + spatialFilter, params: { column: column || '*', operation }, opts: { abortController } }).then((res) => normalizeObjectKeys(res.rows[0])); diff --git a/packages/react-widgets/src/models/HistogramModel.js b/packages/react-widgets/src/models/HistogramModel.js index 327eeec07..7160a29bb 100644 --- a/packages/react-widgets/src/models/HistogramModel.js +++ b/packages/react-widgets/src/models/HistogramModel.js @@ -21,19 +21,22 @@ function fromLocal(props) { // From remote async function fromRemote(props) { - const { source, abortController, ...params } = props; - + const { source, spatialFilter, abortController, ...params } = props; const { column, operation, ticks } = params; const data = await _executeModel({ model: 'histogram', source, + spatialFilter, params: { column, operation, ticks }, opts: { abortController } }).then((res) => normalizeObjectKeys(res.rows)); - const result = Array(ticks.length + 1).fill(0); - data.forEach(({ tick, value }) => (result[tick] = value)); + if (data.length) { + const result = Array(ticks.length + 1).fill(0); + data.forEach(({ tick, value }) => (result[tick] = value)); + return result; + } - return result; + return []; } diff --git a/packages/react-widgets/src/models/utils.js b/packages/react-widgets/src/models/utils.js index 62ef405b4..81aa71e08 100644 --- a/packages/react-widgets/src/models/utils.js +++ b/packages/react-widgets/src/models/utils.js @@ -1,8 +1,18 @@ import { AggregationTypes, _filtersToSQL } from '@carto/react-core'; import { MAP_TYPES, API_VERSIONS } from '@deck.gl/carto'; +export function isRemoteCalculationSupported(props) { + const { source } = props; + + return ( + source && + source.type !== MAP_TYPES.TILESET && + source.credentials.apiVersion !== API_VERSIONS.V2 + ); +} + export function wrapModelCall(props, fromLocal, fromRemote) { - const { source, global } = props; + const { source, global, remoteCalculation } = props; if (global) { if (source.type === MAP_TYPES.TILESET) @@ -19,9 +29,17 @@ export function wrapModelCall(props, fromLocal, fromRemote) { } return fromRemote(props); - } + } else if (remoteCalculation && isRemoteCalculationSupported(props)) { + if (!fromRemote) { + throw new Error(`Remote calculation isn't supported for this widget`); + } - return fromLocal(props); + // The widget supports remote calculation, preferred whenever possible + return fromRemote(props); + } else { + // Local calculation, it requires data to be available + return fromLocal(props); + } } export function formatTableNameWithFilters(props) { @@ -45,7 +63,7 @@ export function normalizeObjectKeys(el) { return Object.entries(el).reduce((acc, [key, value]) => { acc[key.toLowerCase()] = - typeof value === 'object' ? normalizeObjectKeys(value) : value; + typeof value === 'object' && value ? normalizeObjectKeys(value) : value; return acc; }, {}); } diff --git a/packages/react-widgets/src/widgets/BarWidget.js b/packages/react-widgets/src/widgets/BarWidget.js index 4b61107c2..68554db08 100644 --- a/packages/react-widgets/src/widgets/BarWidget.js +++ b/packages/react-widgets/src/widgets/BarWidget.js @@ -3,7 +3,12 @@ import { useDispatch } from 'react-redux'; import PropTypes from 'prop-types'; import { addFilter, removeFilter } from '@carto/react-redux'; import { BarWidgetUI, WrapperWidgetUI } from '@carto/react-ui'; -import { _FilterTypes as FilterTypes, AggregationTypes } from '@carto/react-core'; +import { + _FilterTypes as FilterTypes, + AggregationTypes, + _hasFeatureFlag, + _FeatureFlags +} from '@carto/react-core'; import { getCategories } from '../models'; import { useWidgetFilterValues } from '../hooks/useWidgetFilterValues'; import { columnAggregationOn } from './utils/propTypesFns'; @@ -65,7 +70,8 @@ function BarWidget({ const { data: _data = [], isLoading, - warning + warning, + remoteCalculation } = useWidgetFetch(getCategories, { id, dataSource, @@ -76,7 +82,8 @@ function BarWidget({ operation }, global, - onError + onError, + attemptRemoteCalculation: _hasFeatureFlag(_FeatureFlags.REMOTE_WIDGETS) }); const sortedData = useMemo(() => { @@ -141,6 +148,7 @@ function BarWidget({ global={global} droppingFeaturesAlertProps={droppingFeaturesAlertProps} noDataAlertProps={noDataAlertProps} + showDroppingFeaturesAlert={!remoteCalculation} > {(!!sortedData.length || isLoading) && ( {(!!data.length || isLoading) && ( - + {value !== undefined && ( + + )} ); diff --git a/packages/react-widgets/src/widgets/HistogramWidget.js b/packages/react-widgets/src/widgets/HistogramWidget.js index 3ecd7aacf..22b690cf3 100644 --- a/packages/react-widgets/src/widgets/HistogramWidget.js +++ b/packages/react-widgets/src/widgets/HistogramWidget.js @@ -3,7 +3,12 @@ import { useDispatch } from 'react-redux'; import { PropTypes } from 'prop-types'; import { addFilter, removeFilter } from '@carto/react-redux'; import { WrapperWidgetUI, HistogramWidgetUI } from '@carto/react-ui'; -import { _FilterTypes as FilterTypes, AggregationTypes } from '@carto/react-core'; +import { + _FilterTypes as FilterTypes, + AggregationTypes, + _hasFeatureFlag, + _FeatureFlags +} from '@carto/react-core'; import { getHistogram } from '../models'; import { useWidgetFilterValues } from '../hooks/useWidgetFilterValues'; import useWidgetFetch from '../hooks/useWidgetFetch'; @@ -101,7 +106,8 @@ function HistogramWidget({ let { data = EMPTY_ARRAY, isLoading, - warning = _warning + warning = _warning, + remoteCalculation } = useWidgetFetch(getHistogram, { id, dataSource, @@ -112,7 +118,8 @@ function HistogramWidget({ }, global, onError, - enabled: !!ticks.length + enabled: !!ticks.length, + attemptRemoteCalculation: _hasFeatureFlag(_FeatureFlags.REMOTE_WIDGETS) }); const thresholdsFromFilters = useWidgetFilterValues({ @@ -176,6 +183,7 @@ function HistogramWidget({ global={global} droppingFeaturesAlertProps={droppingFeaturesAlertProps} noDataAlertProps={noDataAlertProps} + showDroppingFeaturesAlert={!remoteCalculation} > {(!!data.length || isLoading) && ( {(!!data.length || isLoading) && ( {(!!data.length || isLoading) && ( =2.2.7 <3", through@^2.3.4, through@^2.3.6, through@^2.3.8: resolved "https://registry.yarnpkg.com/through/-/through-2.3.8.tgz#0dd4c9ffaabc357960b1b724115d7e0e86a2e1f5" integrity sha1-DdTJ/6q8NXlgsbckEV1+Doai4fU= +tilebelt@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/tilebelt/-/tilebelt-1.0.1.tgz#3bbf7113b3fec468efb0d9148f4bb71ef126a21a" + integrity sha512-cxHzpa5JgsugY9NUVRH43gPaGJw/29LecAn4X7UGOP64+kB8pU4VQ3bIhSyfb5Mk4jDxwl3yk330L/EIhbJ5aw== + timers-browserify@^2.0.4: version "2.0.12" resolved "https://registry.yarnpkg.com/timers-browserify/-/timers-browserify-2.0.12.tgz#44a45c11fbf407f34f97bccd1577c652361b00ee"