Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calculation of widget using maps API #658

Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
8260b99
Removed dead code
Apr 27, 2023
ebdd99c
Changed behavior of getGeometryToIntersect
Apr 27, 2023
61777e8
Refactor: reduced unneeded widget calculation
Apr 27, 2023
bb7cc40
Refactor: getGeometryToIntersect returns a Geometry
Apr 28, 2023
a327add
Support for spatialFilter param in API call
Apr 28, 2023
04bae81
Support for remote widgets
Apr 28, 2023
1d3bfb3
Remote calculation enabled for Formula
Apr 28, 2023
863827d
Remote calculation enabled for Category
Apr 28, 2023
3b8c7e4
Remote calculation enabled for Histogram
Apr 28, 2023
de93aba
Remote calculation enabled for Bar
Apr 28, 2023
8700698
Remote calculation enabled for Pie
Apr 28, 2023
fb815fe
Remote calculation enabled for Range
Apr 28, 2023
b6d0241
Tests
Apr 29, 2023
ea023b9
Remote calculation enabled for TimeSeries
May 3, 2023
f10b102
Merge branch 'master' into feature/sc-304698/backend-based-widget-cal…
May 4, 2023
32f47cc
Remote calculation controlled by Feature Flag
May 3, 2023
9cb3f3b
Removed debug logging
May 4, 2023
4607396
Changed the feature flag to `2023-remote-widgets`
May 4, 2023
0901ae8
Prepare changelog for alpha
VictorVelarde May 4, 2023
2fe5fc9
v2.1.0-alpha.0
VictorVelarde May 4, 2023
49f912b
Merge branch 'master' into feature/sc-304698/backend-based-widget-cal…
May 8, 2023
5682c11
Fix
May 8, 2023
18b4837
v2.1.0-alpha.2
VictorVelarde May 8, 2023
d56168d
Fix
May 8, 2023
cdf71ef
v2.1.0-alpha.3
VictorVelarde May 9, 2023
3f14aeb
react-redux: export setViewPort and setViewStateDirect
zbigg May 11, 2023
1d0de4a
v2.1.0-alpha.4
zbigg May 11, 2023
e211ce5
Fixed "no data available at this zoom level"
May 11, 2023
4a424e7
Merge branch 'master' into feature/sc-304698/backend-based-widget-cal…
May 11, 2023
ede8f91
v2.1.0-alpha.5
zbigg May 11, 2023
42e0898
Fix normalizeObjectKeys for null/undefined values
May 12, 2023
dca0727
Support for NoData in FormulaWidget
May 12, 2023
2033236
Fix NoData for HistogramWidget
May 12, 2023
a5e0fb9
Merge branch 'master' into feature/sc-304698/backend-based-widget-cal…
May 12, 2023
354dbe3
v2.1.0-alpha.7
VictorVelarde May 12, 2023
3ff12ad
Optimized viewport wrapping the globe (fix 314403)
May 15, 2023
66a5add
v2.0.3-alpha.1
VictorVelarde May 15, 2023
021ccf7
Partial revert of wrapping the globe optimization
May 17, 2023
df14fa4
Minor optimization
May 17, 2023
b65da53
Merge branch 'master' into feature/sc-304698/backend-based-widget-cal…
VictorVelarde May 17, 2023
3c50889
v2.0.3-alpha.2
VictorVelarde May 17, 2023
6fa38d8
Aligning peer dependencies
May 18, 2023
756e46d
Switch to quadbin 0.1.9
May 18, 2023
c96c6cd
Refactor geo functions in their own file
May 18, 2023
6e3652c
Fixed warnings
May 18, 2023
72831a1
Merge branch 'master' into feature/sc-304698/backend-based-widget-cal…
VictorVelarde May 18, 2023
0c18699
Prepare changelog for landing
VictorVelarde May 18, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion packages/react-api/src/api/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -27,7 +29,7 @@ export function executeModel(props) {
)}`
);

const { source, model, params, opts } = props;
const { source, model, params, spatialFilter, opts } = props;

checkCredentials(source.credentials);

Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't get much better because you have to measure size of GET url few lines above and that must have proper url as input.

(btw, it should create proper URL first, measure and test and not rely of more less accourate assumptions that JSON.stringify(queryParams). is same length as new URLSearchParams(queryParams).toString() but that's other story)

This is done correctly in getStats albeit it's not much more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to introduce an utility function for this, in a later PR.

queryParams.params = params;
queryParams.filters = filters;
queryParams.queryParameters = source.queryParameters;
if (spatialFilter) {
queryParams.spatialFilter = spatialFilter;
}
}
return makeCall({
url,
Expand Down
4 changes: 2 additions & 2 deletions packages/react-api/src/hooks/useCartoLayerProps.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -18,7 +18,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 geometryToIntersect = getGeometryToIntersect(viewport, spatialFilter);

Expand Down
2 changes: 2 additions & 0 deletions packages/react-core/__tests__/filters/tileFeatures.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ describe('getGeometryToIntersect', () => {
test('returns null in case no 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', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-core/src/filters/tileFeatures.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import tileFeaturesSpatialIndex from './tileFeaturesSpatialIndex';
* @returns { Geometry? } the geometry to use for filtering
*/
export function getGeometryToIntersect(viewport, spatialFilter) {
return spatialFilter
return spatialFilter && spatialFilter.geometry
? spatialFilter.geometry
: Array.isArray(viewport) && viewport.length === 4
? bboxPolygon(viewport).geometry
Expand Down
5 changes: 4 additions & 1 deletion packages/react-core/src/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { _FeatureFlags } from '.';

export {
getRequest,
postRequest,
Expand Down Expand Up @@ -38,7 +40,8 @@ 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';
} from './utils/featureFlags';
1 change: 1 addition & 0 deletions packages/react-core/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export {
} from './utils/featureSelectionConstants';

export {
Flags as _FeatureFlags,
hasFlag as _hasFeatureFlag,
setFlags as _setFeatureFlags,
clearFlags as _clearFeatureFlags
Expand Down
3 changes: 3 additions & 0 deletions packages/react-core/src/utils/featureFlags.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
export enum Flags {
REMOTE_WIDGETS = '2023-remote-widgets'
}
export function setFlags(flags: Record<string, any> | string[]): void
export function clearFlags(): void
export function hasFlag(flag: string): boolean
4 changes: 4 additions & 0 deletions packages/react-core/src/utils/featureFlags.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
let featureFlags = [];

export const Flags = Object.freeze({
REMOTE_WIDGETS: '2023-remote-widgets'
});

export function setFlags(flags) {
const isValidFlag = (f) => typeof f === 'string' && f;

Expand Down
4 changes: 3 additions & 1 deletion packages/react-redux/src/slices/cartoSlice.d.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -159,6 +159,8 @@ export function setFeatureSelectionEnabled(enabled: boolean): {
payload: boolean;
};

export function selectViewport(state: any): Viewport | null;

export function selectSpatialFilter(state: any, sourceId?: string): Feature<Polygon | MultiPolygon> | null;

export function selectFeatureSelectionMode(state: any): string | null;
7 changes: 7 additions & 0 deletions packages/react-redux/src/slices/cartoSlice.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,13 +307,7 @@ function TimeSeriesWidgetUIContent({
<Typography color='textSecondary' variant='caption'>
{currentDate}
</Typography>
<Typography
xs
fontSize={12}
ml={1}
color='textSecondary'
variant='caption'
>
<Typography xs fontSize={12} ml={1} color='textSecondary' variant='caption'>
({capitalize(stepSize)})
</Typography>
</Box>
Expand Down
107 changes: 101 additions & 6 deletions packages/react-widgets/__tests__/hooks/useWidgetFetch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,47 @@ 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, mockReduxHooks, mockSetup } from '../mockReduxHooks';
import { selectViewport } from '@carto/react-redux';
import bboxPolygon from '@turf/bbox-polygon';

const PARAMS_MOCK = {
column: '__test__'
};

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()
Expand All @@ -35,6 +59,7 @@ describe('useWidgetFetch', () => {
dataSource: 'test',
params: PARAMS_MOCK,
global: false,
attemptRemoteCalculation: false,
onError
}}
/>
Expand All @@ -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();
Expand All @@ -65,6 +92,7 @@ describe('useWidgetFetch', () => {
dataSource: 'test',
params: PARAMS_MOCK,
global: true,
attemptRemoteCalculation: false,
onError
}}
/>
Expand All @@ -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();
Expand All @@ -93,6 +123,7 @@ describe('useWidgetFetch', () => {
dataSource: 'test',
params: PARAMS_MOCK,
global: false,
remoteCalculation: false,
onError
}}
/>
Expand All @@ -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(
<TestComponent
modelFn={modelFn}
args={{
id: 'test',
dataSource: 'test',
params: PARAMS_MOCK,
global: false,
attemptRemoteCalculation: true,
onError
}}
/>
);

// 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(
<TestComponent
modelFn={modelFn}
args={{
id: 'test',
dataSource: 'test',
params: PARAMS_MOCK,
global: true,
attemptRemoteCalculation: true,
onError
}}
/>
);

// 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
Expand Down
4 changes: 4 additions & 0 deletions packages/react-widgets/__tests__/mockReduxHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
33 changes: 24 additions & 9 deletions packages/react-widgets/__tests__/models/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 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);
}
);

const props2 = { source: V3_SOURCE, global: true };
wrapModelCall(props2, fromLocal, fromRemote);
expect(fromRemote).toHaveBeenCalledWith(props2);
test('should throw error if global is true but fromRemote is missing', () => {
expect(() =>
wrapModelCall({ source: V3_SOURCE, global: true }, fromLocal)
).toThrowError();
});

test('should throw error if global is true but fromRemote is missing', () => {
test('should throw error if remoteCalculation is true but fromRemote is missing', () => {
expect(() =>
wrapModelCall({ source: V2_SOURCE, global: true }, fromLocal)
wrapModelCall({ source: V3_SOURCE, remoteCalculation: true }, fromLocal)
).toThrowError();
});

Expand Down
Loading