diff --git a/.github/workflows/superset-docs-verify.yml b/.github/workflows/superset-docs-verify.yml index 52271f0a4a6b..1306fec1bf1f 100644 --- a/.github/workflows/superset-docs-verify.yml +++ b/.github/workflows/superset-docs-verify.yml @@ -117,6 +117,7 @@ jobs: run_id: ${{ github.event.workflow_run.id }} name: database-diagnostics path: docs/src/data/ + if_no_artifact_found: 'warning' - name: Use fresh diagnostics run: | if [ -f "src/data/databases-diagnostics.json" ]; then diff --git a/UPDATING.md b/UPDATING.md index 455e9a10ba48..bbfd509ea440 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -24,6 +24,14 @@ assists people when migrating to a new version. ## Next +### Deck.gl MapBox viewport and opacity controls are functional + +The Deck.gl MapBox chart's **Opacity**, **Default longitude**, **Default latitude**, and **Zoom** controls were previously non-functional — changing them had no effect on the rendered map. These controls are now wired up correctly. + +**Behavior change for existing charts:** Previously, the viewport controls had hard-coded default values (`-122.405293`, `37.772123`, zoom `11` — San Francisco) that were stored in each chart's `form_data` but never applied. The map always used `fitBounds` to center on the data. With this fix, those stored values are now respected, which means existing MapBox charts may open centered on the old default coordinates instead of fitting to data bounds. + +**To restore fit-to-data behavior:** Open the chart in Explore, clear the **Default longitude**, **Default latitude**, and **Zoom** fields in the Viewport section, and re-save the chart. + ### ClickHouse minimum driver version bump The minimum required version of `clickhouse-connect` has been raised to `>=0.13.0`. If you are using the ClickHouse connector, please upgrade your `clickhouse-connect` package. The `_mutate_label` workaround that appended hash suffixes to column aliases has also been removed, as it is no longer needed with modern versions of the driver. diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Operator.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Operator.ts index 0d2cb5b59ba7..98695555068e 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/types/Operator.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/types/Operator.ts @@ -30,6 +30,7 @@ const BINARY_OPERATORS = [ '<=', 'ILIKE', 'LIKE', + 'NOT ILIKE', 'NOT LIKE', 'REGEX', 'TEMPORAL_RANGE', diff --git a/superset-frontend/plugins/legacy-plugin-chart-map-box/src/MapBox.tsx b/superset-frontend/plugins/legacy-plugin-chart-map-box/src/MapBox.tsx index 361a27610c94..a69659de0b38 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-map-box/src/MapBox.tsx +++ b/superset-frontend/plugins/legacy-plugin-chart-map-box/src/MapBox.tsx @@ -61,6 +61,9 @@ interface MapBoxProps { renderWhileDragging?: boolean; rgb?: (string | number)[]; bounds?: [[number, number], [number, number]]; // May be undefined for empty datasets + viewportLongitude?: number; + viewportLatitude?: number; + viewportZoom?: number; } interface MapBoxState { @@ -82,30 +85,10 @@ class MapBox extends Component { constructor(props: MapBoxProps) { super(props); - const { width = 400, height = 400, bounds } = this.props; - // Get a viewport that fits the given bounds, which all marks to be clustered. - // Derive lat, lon and zoom from this viewport. This is only done on initial - // render as the bounds don't update as we pan/zoom in the current design. - - let latitude = 0; - let longitude = 0; - let zoom = 1; - - // Guard against empty datasets where bounds may be undefined - if (bounds && bounds[0] && bounds[1]) { - const mercator = new WebMercatorViewport({ - width, - height, - }).fitBounds(bounds); - ({ latitude, longitude, zoom } = mercator); - } + const fitBounds = this.computeFitBoundsViewport(); this.state = { - viewport: { - longitude, - latitude, - zoom, - }, + viewport: this.mergeViewportWithProps(fitBounds), }; this.handleViewportChange = this.handleViewportChange.bind(this); } @@ -116,6 +99,75 @@ class MapBox extends Component { onViewportChange!(viewport); } + mergeViewportWithProps( + fitBounds: Viewport, + viewport: Viewport = fitBounds, + props: MapBoxProps = this.props, + useFitBoundsForUnset = true, + ): Viewport { + const { viewportLongitude, viewportLatitude, viewportZoom } = props; + + return { + ...viewport, + longitude: + viewportLongitude ?? + (useFitBoundsForUnset ? fitBounds.longitude : viewport.longitude), + latitude: + viewportLatitude ?? + (useFitBoundsForUnset ? fitBounds.latitude : viewport.latitude), + zoom: + viewportZoom ?? (useFitBoundsForUnset ? fitBounds.zoom : viewport.zoom), + }; + } + + computeFitBoundsViewport(): Viewport { + const { width = 400, height = 400, bounds } = this.props; + if (bounds && bounds[0] && bounds[1]) { + const mercator = new WebMercatorViewport({ width, height }).fitBounds( + bounds, + ); + return { + latitude: mercator.latitude, + longitude: mercator.longitude, + zoom: mercator.zoom, + }; + } + return { latitude: 0, longitude: 0, zoom: 1 }; + } + + componentDidUpdate(prevProps: MapBoxProps) { + const { viewport } = this.state; + const fitBoundsInputsChanged = + prevProps.width !== this.props.width || + prevProps.height !== this.props.height || + prevProps.bounds !== this.props.bounds; + const viewportPropsChanged = + prevProps.viewportLongitude !== this.props.viewportLongitude || + prevProps.viewportLatitude !== this.props.viewportLatitude || + prevProps.viewportZoom !== this.props.viewportZoom; + + if (!fitBoundsInputsChanged && !viewportPropsChanged) { + return; + } + + const fitBounds = this.computeFitBoundsViewport(); + const nextViewport = this.mergeViewportWithProps( + fitBounds, + viewport, + this.props, + fitBoundsInputsChanged || viewportPropsChanged, + ); + + const viewportChanged = + nextViewport.longitude !== viewport.longitude || + nextViewport.latitude !== viewport.latitude || + nextViewport.zoom !== viewport.zoom; + + if (viewportChanged) { + this.setState({ viewport: nextViewport }); + } + } + render() { const { width, diff --git a/superset-frontend/plugins/legacy-plugin-chart-map-box/src/controlPanel.ts b/superset-frontend/plugins/legacy-plugin-chart-map-box/src/controlPanel.ts index 80d6ab0c7f6b..8b1c846e048c 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-map-box/src/controlPanel.ts +++ b/superset-frontend/plugins/legacy-plugin-chart-map-box/src/controlPanel.ts @@ -241,6 +241,7 @@ const config: ControlPanelConfig = { label: t('Opacity'), default: 1, isFloat: true, + renderTrigger: true, description: t( 'Opacity of all clusters, points, and labels. Between 0 and 1.', ), @@ -273,7 +274,7 @@ const config: ControlPanelConfig = { type: 'TextControl', label: t('Default longitude'), renderTrigger: true, - default: -122.405293, + default: '', isFloat: true, description: t('Longitude of default viewport'), places: 8, @@ -287,7 +288,7 @@ const config: ControlPanelConfig = { type: 'TextControl', label: t('Default latitude'), renderTrigger: true, - default: 37.772123, + default: '', isFloat: true, description: t('Latitude of default viewport'), places: 8, @@ -304,7 +305,7 @@ const config: ControlPanelConfig = { label: t('Zoom'), renderTrigger: true, isFloat: true, - default: 11, + default: '', description: t('Zoom level of the map'), places: 8, // Viewport zoom shouldn't prompt user to re-run query diff --git a/superset-frontend/plugins/legacy-plugin-chart-map-box/src/transformProps.ts b/superset-frontend/plugins/legacy-plugin-chart-map-box/src/transformProps.ts index caaeb9b3f55b..45eca8d4448b 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-map-box/src/transformProps.ts +++ b/superset-frontend/plugins/legacy-plugin-chart-map-box/src/transformProps.ts @@ -23,6 +23,30 @@ import { ChartProps } from '@superset-ui/core'; import { DEFAULT_POINT_RADIUS, DEFAULT_MAX_ZOOM } from './MapBox'; const NOOP = () => {}; +const MIN_LONGITUDE = -180; +const MAX_LONGITUDE = 180; +const MIN_LATITUDE = -90; +const MAX_LATITUDE = 90; +const MIN_ZOOM = 0; + +function toFiniteNumber( + value: string | number | null | undefined, +): number | undefined { + if (value === null || value === undefined) return undefined; + const normalizedValue = typeof value === 'string' ? value.trim() : value; + if (normalizedValue === '') return undefined; + const num = Number(normalizedValue); + return Number.isFinite(num) ? num : undefined; +} + +function clampNumber( + value: number | undefined, + min: number, + max: number, +): number | undefined { + if (value === undefined) return undefined; + return Math.min(max, Math.max(min, value)); +} interface ClusterProperties { metric: number; @@ -45,6 +69,9 @@ export default function transformProps(chartProps: ChartProps) { pandasAggfunc, pointRadiusUnit, renderWhileDragging, + viewportLongitude, + viewportLatitude, + viewportZoom, } = formData; // Validate mapbox color @@ -93,7 +120,6 @@ export default function transformProps(chartProps: ChartProps) { aggregatorName: pandasAggfunc, bounds, clusterer, - globalOpacity, hasCustomMetric, mapboxApiKey, mapStyle: mapboxStyle, @@ -116,5 +142,21 @@ export default function transformProps(chartProps: ChartProps) { pointRadiusUnit, renderWhileDragging, rgb, + viewportLongitude: clampNumber( + toFiniteNumber(viewportLongitude), + MIN_LONGITUDE, + MAX_LONGITUDE, + ), + viewportLatitude: clampNumber( + toFiniteNumber(viewportLatitude), + MIN_LATITUDE, + MAX_LATITUDE, + ), + viewportZoom: clampNumber( + toFiniteNumber(viewportZoom), + MIN_ZOOM, + DEFAULT_MAX_ZOOM, + ), + globalOpacity: Math.min(1, Math.max(0, toFiniteNumber(globalOpacity) ?? 1)), }; } diff --git a/superset-frontend/plugins/legacy-plugin-chart-map-box/test/MapBox.test.tsx b/superset-frontend/plugins/legacy-plugin-chart-map-box/test/MapBox.test.tsx new file mode 100644 index 000000000000..f0782f5fc14d --- /dev/null +++ b/superset-frontend/plugins/legacy-plugin-chart-map-box/test/MapBox.test.tsx @@ -0,0 +1,381 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { type ReactNode } from 'react'; +import { render } from '@testing-library/react'; +import MapBox from '../src/MapBox'; + +// Capture the most recent viewport props passed to MapGL +let lastMapGLProps: Record = {}; +const mockFitBounds = jest.fn(); + +jest.mock('react-map-gl', () => { + const MockMapGL = (props: Record) => { + lastMapGLProps = props; + return
{props.children as ReactNode}
; + }; + return { __esModule: true, default: MockMapGL }; +}); + +jest.mock('@math.gl/web-mercator', () => ({ + WebMercatorViewport: jest + .fn() + .mockImplementation( + ({ width, height }: { width: number; height: number }) => ({ + fitBounds: (bounds: [[number, number], [number, number]]) => + mockFitBounds(bounds, width, height), + }), + ), +})); + +jest.mock('../src/ScatterPlotGlowOverlay', () => { + const MockOverlay = (props: Record) => ( +
+ ); + return { __esModule: true, default: MockOverlay }; +}); + +const defaultProps = { + width: 800, + height: 600, + clusterer: { + getClusters: jest.fn().mockReturnValue([]), + }, + globalOpacity: 1, + mapboxApiKey: 'test-key', + mapStyle: 'mapbox://styles/mapbox/light-v9', + pointRadius: 60, + pointRadiusUnit: 'Pixels', + renderWhileDragging: true, + rgb: ['', 255, 0, 0] as (string | number)[], + hasCustomMetric: false, + bounds: [ + [-74.0, 40.7], + [-73.9, 40.8], + ] as [[number, number], [number, number]], + onViewportChange: jest.fn(), +}; + +beforeEach(() => { + lastMapGLProps = {}; + jest.clearAllMocks(); + mockFitBounds.mockImplementation( + ( + bounds: [[number, number], [number, number]], + width: number, + height: number, + ) => ({ + latitude: Number(((bounds[0][1] + bounds[1][1]) / 2).toFixed(2)), + longitude: Number(((bounds[0][0] + bounds[1][0]) / 2).toFixed(2)), + zoom: Number((10 + width / 1000 + height / 10000).toFixed(2)), + }), + ); +}); + +test('initializes viewport from bounds', () => { + render(); + expect(lastMapGLProps.latitude).toBe(40.75); + expect(lastMapGLProps.longitude).toBe(-73.95); + expect(lastMapGLProps.zoom).toBe(10.86); +}); + +test('updates viewport when viewport props change', () => { + const { rerender } = render( + , + ); + + rerender( + , + ); + + expect(lastMapGLProps.longitude).toBe(-122.4); + expect(lastMapGLProps.latitude).toBe(37.8); + expect(lastMapGLProps.zoom).toBe(5); +}); + +test('does not loop when viewport state matches new props', () => { + const { rerender } = render( + , + ); + + // Re-render with same props that match the initial viewport state + rerender( + , + ); + + // Viewport should still be the fitBounds-computed values since props didn't change + expect(lastMapGLProps.longitude).toBe(-73.95); + expect(lastMapGLProps.latitude).toBe(40.75); + expect(lastMapGLProps.zoom).toBe(10); +}); + +test('passes globalOpacity to ScatterPlotGlowOverlay', () => { + const { getByTestId } = render( + , + ); + const overlay = getByTestId('scatter-overlay'); + expect(overlay.dataset.opacity).toBe('0.5'); +}); + +test('initializes viewport from props when provided', () => { + render( + , + ); + expect(lastMapGLProps.longitude).toBe(-122.4); + expect(lastMapGLProps.latitude).toBe(37.8); + expect(lastMapGLProps.zoom).toBe(5); +}); + +test('handles undefined bounds gracefully', () => { + render(); + expect(lastMapGLProps.longitude).toBe(0); + expect(lastMapGLProps.latitude).toBe(0); + expect(lastMapGLProps.zoom).toBe(1); +}); + +test('applies partial viewport props on update', () => { + const { rerender } = render(); + + rerender(); + + expect(lastMapGLProps.longitude).toBe(-122.4); + expect(lastMapGLProps.latitude).toBe(40.75); + expect(lastMapGLProps.zoom).toBe(10.86); +}); + +test('restores fitBounds when viewport props are cleared', () => { + const { rerender } = render( + , + ); + + // Clear all viewport props (simulates user clearing the controls) + rerender(); + + // Should revert to fitBounds values + expect(lastMapGLProps.longitude).toBe(-73.95); + expect(lastMapGLProps.latitude).toBe(40.75); + expect(lastMapGLProps.zoom).toBe(10.86); +}); + +test('restores only cleared viewport props, keeps the rest', () => { + const { rerender } = render( + , + ); + + // Clear only longitude, keep lat/zoom + rerender( + , + ); + + // Longitude reverts to fitBounds, lat/zoom stay + expect(lastMapGLProps.longitude).toBe(-73.95); + expect(lastMapGLProps.latitude).toBe(37.8); + expect(lastMapGLProps.zoom).toBe(5); +}); + +test('applies changed viewport props even when another is cleared simultaneously', () => { + const { rerender } = render( + , + ); + + // Clear longitude, change latitude simultaneously + rerender( + , + ); + + // Longitude reverts to fitBounds, latitude should be the NEW value + expect(lastMapGLProps.longitude).toBe(-73.95); + expect(lastMapGLProps.latitude).toBe(40.0); + expect(lastMapGLProps.zoom).toBe(5); +}); + +test('falls back to default viewport when cleared with undefined bounds', () => { + const { rerender } = render( + , + ); + + // Clear viewport props — no bounds to fitBounds to + rerender(); + + // Should fall back to {0, 0, 1} + expect(lastMapGLProps.longitude).toBe(0); + expect(lastMapGLProps.latitude).toBe(0); + expect(lastMapGLProps.zoom).toBe(1); +}); + +test('recomputes fitBounds when bounds change and no explicit viewport is set', () => { + const { rerender } = render(); + + rerender( + , + ); + + expect(lastMapGLProps.longitude).toBe(-122.5); + expect(lastMapGLProps.latitude).toBe(37.3); + expect(lastMapGLProps.zoom).toBe(10.86); +}); + +test('recomputes fitBounds when chart size changes and no explicit viewport is set', () => { + const { rerender } = render(); + + rerender(); + + expect(lastMapGLProps.longitude).toBe(-73.95); + expect(lastMapGLProps.latitude).toBe(40.75); + expect(lastMapGLProps.zoom).toBe(11.29); +}); + +test('recomputes only implicit viewport fields when bounds change', () => { + const { rerender } = render( + , + ); + + rerender( + , + ); + + expect(lastMapGLProps.longitude).toBe(-122.4); + expect(lastMapGLProps.latitude).toBe(37.3); + expect(lastMapGLProps.zoom).toBe(10.86); +}); + +test('recomputes only implicit viewport fields when chart size changes', () => { + const { rerender } = render( + , + ); + + rerender( + , + ); + + expect(lastMapGLProps.longitude).toBe(-73.95); + expect(lastMapGLProps.latitude).toBe(37.8); + expect(lastMapGLProps.zoom).toBe(11.29); +}); + +test('recomputes implicit position when zoom stays explicit across bounds changes', () => { + const { rerender } = render(); + + rerender( + , + ); + + expect(lastMapGLProps.longitude).toBe(-122.5); + expect(lastMapGLProps.latitude).toBe(37.3); + expect(lastMapGLProps.zoom).toBe(5); +}); + +test('does not recompute fitBounds on bounds change when an explicit viewport is set', () => { + const { rerender } = render( + , + ); + + rerender( + , + ); + + expect(lastMapGLProps.longitude).toBe(-122.4); + expect(lastMapGLProps.latitude).toBe(37.8); + expect(lastMapGLProps.zoom).toBe(5); +}); diff --git a/superset-frontend/plugins/legacy-plugin-chart-map-box/test/controlPanel.test.ts b/superset-frontend/plugins/legacy-plugin-chart-map-box/test/controlPanel.test.ts new file mode 100644 index 000000000000..2a72db463669 --- /dev/null +++ b/superset-frontend/plugins/legacy-plugin-chart-map-box/test/controlPanel.test.ts @@ -0,0 +1,81 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import type { + ControlPanelConfig, + CustomControlItem, +} from '@superset-ui/chart-controls'; +import controlPanel from '../src/controlPanel'; + +type ControlConfig = Required; + +function isCustomControlItem( + controlItem: unknown, +): controlItem is CustomControlItem & { config: ControlConfig } { + return ( + typeof controlItem === 'object' && + controlItem !== null && + 'name' in controlItem && + 'config' in controlItem + ); +} + +function getControl( + panel: ControlPanelConfig, + controlName: string, +): CustomControlItem & { config: ControlConfig } { + const item = (panel.controlPanelSections || []) + .flatMap(section => section?.controlSetRows || []) + .flat() + .find( + controlItem => + isCustomControlItem(controlItem) && controlItem.name === controlName, + ); + + if (!isCustomControlItem(item)) { + throw new Error(`Control "${controlName}" not found`); + } + + return item; +} + +test('viewport controls default to empty values and rerender without query refresh', () => { + const longitudeControl = getControl(controlPanel, 'viewport_longitude'); + const latitudeControl = getControl(controlPanel, 'viewport_latitude'); + const zoomControl = getControl(controlPanel, 'viewport_zoom'); + + expect(longitudeControl.config.default).toBe(''); + expect(latitudeControl.config.default).toBe(''); + expect(zoomControl.config.default).toBe(''); + + expect(longitudeControl.config.renderTrigger).toBe(true); + expect(latitudeControl.config.renderTrigger).toBe(true); + expect(zoomControl.config.renderTrigger).toBe(true); + + expect(longitudeControl.config.dontRefreshOnChange).toBe(true); + expect(latitudeControl.config.dontRefreshOnChange).toBe(true); + expect(zoomControl.config.dontRefreshOnChange).toBe(true); +}); + +test('opacity control rerenders immediately when changed', () => { + const opacityControl = getControl(controlPanel, 'global_opacity'); + + expect(opacityControl.config.default).toBe(1); + expect(opacityControl.config.renderTrigger).toBe(true); + expect(opacityControl.config.isFloat).toBe(true); +}); diff --git a/superset-frontend/plugins/legacy-plugin-chart-map-box/test/transformProps.test.ts b/superset-frontend/plugins/legacy-plugin-chart-map-box/test/transformProps.test.ts new file mode 100644 index 000000000000..483f64963dc9 --- /dev/null +++ b/superset-frontend/plugins/legacy-plugin-chart-map-box/test/transformProps.test.ts @@ -0,0 +1,230 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { ChartProps } from '@superset-ui/core'; +import { supersetTheme } from '@apache-superset/core/theme'; + +jest.mock('supercluster', () => { + const MockSupercluster = jest.fn().mockImplementation(() => ({ + load: jest.fn(), + getClusters: jest.fn().mockReturnValue([]), + })); + return { __esModule: true, default: MockSupercluster }; +}); + +// Import after mocking supercluster to avoid ESM parse error +// eslint-disable-next-line import/first +import transformProps from '../src/transformProps'; + +type TransformPropsResult = { + globalOpacity?: number; + onViewportChange?: (viewport: { + latitude: number; + longitude: number; + zoom: number; + }) => void; + viewportLongitude?: number; + viewportLatitude?: number; + viewportZoom?: number; +}; + +const baseFormData = { + clusteringRadius: 60, + globalOpacity: 0.8, + mapboxColor: 'rgb(0, 139, 139)', + mapboxStyle: 'mapbox://styles/mapbox/light-v9', + pandasAggfunc: 'sum', + pointRadiusUnit: 'Pixels', + renderWhileDragging: true, + viewportLongitude: -73.935242, + viewportLatitude: 40.73061, + viewportZoom: 9, +}; + +const baseQueriesData = [ + { + data: { + bounds: [ + [-74.0, 40.7], + [-73.9, 40.8], + ] as [[number, number], [number, number]], + geoJSON: { features: [] }, + hasCustomMetric: false, + mapboxApiKey: 'test-api-key', + }, + }, +]; + +function createChartProps(overrides: Record = {}) { + return new ChartProps({ + formData: { ...baseFormData, ...overrides }, + width: 800, + height: 600, + queriesData: baseQueriesData, + theme: supersetTheme, + }); +} + +function getTransformPropsResult( + overrides: Record = {}, +): TransformPropsResult { + return transformProps(createChartProps(overrides)) as TransformPropsResult; +} + +test('extracts globalOpacity from formData', () => { + const result = getTransformPropsResult({ globalOpacity: 0.5 }); + expect(result.globalOpacity).toBe(0.5); +}); + +test('extracts viewport values from formData', () => { + const result = getTransformPropsResult({ + viewportLongitude: -122.4, + viewportLatitude: 37.8, + viewportZoom: 12, + }); + expect(result).toEqual( + expect.objectContaining({ + viewportLongitude: -122.4, + viewportLatitude: 37.8, + viewportZoom: 12, + }), + ); +}); + +test('clamps viewport values to safe map ranges', () => { + const result = getTransformPropsResult({ + viewportLongitude: 190, + viewportLatitude: -100, + viewportZoom: 99, + }); + expect(result).toEqual( + expect.objectContaining({ + viewportLongitude: 180, + viewportLatitude: -90, + viewportZoom: 16, + }), + ); +}); + +test('provides onViewportChange callback that updates control values', () => { + const setControlValue = jest.fn(); + const chartProps = new ChartProps({ + formData: baseFormData, + width: 800, + height: 600, + queriesData: baseQueriesData, + hooks: { setControlValue }, + theme: supersetTheme, + }); + const result = transformProps(chartProps) as TransformPropsResult; + expect(result.onViewportChange).toBeDefined(); + + result.onViewportChange!({ + latitude: 51.5, + longitude: -0.12, + zoom: 10, + }); + + expect(setControlValue).toHaveBeenCalledWith('viewport_longitude', -0.12); + expect(setControlValue).toHaveBeenCalledWith('viewport_latitude', 51.5); + expect(setControlValue).toHaveBeenCalledWith('viewport_zoom', 10); +}); + +test('normalizes string viewport values to numbers', () => { + const result = getTransformPropsResult({ + viewportLongitude: '-122.4', + viewportLatitude: '37.8', + viewportZoom: '12', + }); + expect(result.viewportLongitude).toBe(-122.4); + expect(result.viewportLatitude).toBe(37.8); + expect(result.viewportZoom).toBe(12); +}); + +test('normalizes empty viewport values to undefined', () => { + const result = getTransformPropsResult({ + viewportLongitude: '', + viewportLatitude: '', + viewportZoom: '', + }); + expect(result.viewportLongitude).toBeUndefined(); + expect(result.viewportLatitude).toBeUndefined(); + expect(result.viewportZoom).toBeUndefined(); +}); + +test('normalizes whitespace-only viewport values to undefined', () => { + const result = getTransformPropsResult({ + viewportLongitude: ' ', + viewportLatitude: '\t', + viewportZoom: ' \n ', + }); + expect(result.viewportLongitude).toBeUndefined(); + expect(result.viewportLatitude).toBeUndefined(); + expect(result.viewportZoom).toBeUndefined(); +}); + +test('normalizes string opacity to number', () => { + const result = getTransformPropsResult({ globalOpacity: '0.5' }); + expect(result.globalOpacity).toBe(0.5); +}); + +test('defaults empty opacity to 1', () => { + const result = getTransformPropsResult({ globalOpacity: '' }); + expect(result.globalOpacity).toBe(1); +}); + +test('defaults whitespace-only opacity to 1', () => { + const result = getTransformPropsResult({ globalOpacity: ' ' }); + expect(result.globalOpacity).toBe(1); +}); + +test('clamps opacity to [0, 1] range', () => { + expect(getTransformPropsResult({ globalOpacity: 5 }).globalOpacity).toBe(1); + expect(getTransformPropsResult({ globalOpacity: -1 }).globalOpacity).toBe(0); +}); + +test('passes through numeric values unchanged', () => { + const result = getTransformPropsResult({ + viewportLongitude: -122.4, + viewportLatitude: 37.8, + viewportZoom: 12, + globalOpacity: 0.8, + }); + expect(result.viewportLongitude).toBe(-122.4); + expect(result.viewportLatitude).toBe(37.8); + expect(result.viewportZoom).toBe(12); + expect(result.globalOpacity).toBe(0.8); +}); + +test('calls onError and returns empty object for invalid color', () => { + const onError = jest.fn(); + const chartProps = new ChartProps({ + formData: { ...baseFormData, mapboxColor: 'invalid-color' }, + width: 800, + height: 600, + queriesData: baseQueriesData, + hooks: { onError }, + theme: supersetTheme, + }); + const result = transformProps(chartProps); + expect(onError).toHaveBeenCalledWith( + "Color field must be of form 'rgb(%d, %d, %d)'", + ); + expect(result).toEqual({}); +}); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx index ab6a9f22e695..cc53dcbcb616 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx @@ -45,12 +45,16 @@ import { useEffect, useImperativeHandle, useMemo, + useRef, useState, RefObject, memo, } from 'react'; import rison from 'rison'; -import { PluginFilterSelectCustomizeProps } from 'src/filters/components/Select/types'; +import { + PluginFilterSelectCustomizeProps, + SelectFilterOperatorType, +} from 'src/filters/components/Select/types'; import { useSelector } from 'react-redux'; import { getChartDataRequest } from 'src/components/Chart/chartAction'; import { @@ -624,6 +628,49 @@ const FiltersConfigForm = ( forceUpdate(); }; + const currentOperatorType: SelectFilterOperatorType = + formFilter?.controlValues?.operatorType ?? + filterToEdit?.controlValues?.operatorType ?? + SelectFilterOperatorType.Exact; + + const selectedColumnIsString = useMemo(() => { + const columnName = formFilter?.column; + if (!columnName || !datasetDetails?.columns) return true; + const colMeta = datasetDetails.columns.find( + (c: { column_name: string }) => c.column_name === columnName, + ); + if (!colMeta) return true; + return colMeta.type_generic === GenericDataType.String; + }, [formFilter?.column, datasetDetails?.columns]); + + const onOperatorTypeChanged = (value: SelectFilterOperatorType) => { + const previous = form.getFieldValue('filters')?.[filterId].controlValues; + setNativeFilterFieldValues(form, filterId, { + controlValues: { + ...previous, + operatorType: value, + }, + defaultDataMask: null, + }); + formChanged(); + forceUpdate(); + }; + + const prevColumnRef = useRef(formFilter?.column); + const datasetLoaded = !!datasetDetails?.columns; + useEffect(() => { + const columnChanged = prevColumnRef.current !== formFilter?.column; + if ( + (columnChanged || datasetLoaded) && + !selectedColumnIsString && + currentOperatorType !== SelectFilterOperatorType.Exact + ) { + onOperatorTypeChanged(SelectFilterOperatorType.Exact); + } + prevColumnRef.current = formFilter?.column; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [formFilter?.column, selectedColumnIsString, datasetLoaded]); + const validatePreFilter = () => setTimeout( () => @@ -685,6 +732,7 @@ const FiltersConfigForm = ( 'columns.filterable', 'columns.is_dttm', 'columns.type', + 'columns.type_generic', 'columns.verbose_name', 'database.id', 'database.database_name', @@ -1501,6 +1549,67 @@ const FiltersConfigForm = ( hidden initialValue={null} /> + {!isChartCustomization && + itemTypeField === 'filter_select' && ( + + {t('Match type')} +   + + + } + > + (parentRef?.current as HTMLElement) || document.body - : (trigger: HTMLElement) => - (trigger?.parentNode as HTMLElement) || document.body - } - showSearch={showSearch} - mode={multiSelect ? 'multiple' : 'single'} - placeholder={placeholderText} - onClear={() => onSearch('')} - onSearch={onSearch} - onBlur={handleBlur} - onFocus={setFocusedFilter} - onMouseEnter={setHoveredFilter} - onMouseLeave={unsetHoveredFilter} - // @ts-expect-error - onChange={handleChange} - ref={inputRef} - loading={isRefreshing} - oneLine={filterBarOrientation === FilterBarOrientation.Horizontal} - invertSelection={inverseSelection && excludeFilterValues} - options={options} - sortComparator={sortComparator} - onOpenChange={setFilterActive} - className="select-container" - /> + {isLikeOperator ? ( + + ) : ( +