fix(deckgl): emit usable cross-filter values from polygon and geojson clicks#39906
fix(deckgl): emit usable cross-filter values from polygon and geojson clicks#39906alex-poor wants to merge 2 commits intoapache:masterfrom
Conversation
… clicks Adds a new optional `cross_filter_column` control to the deck_polygon and deck_geojson chart types. When set, clicking a feature emits a `<dimension> == <value>` filter (e.g. `sa3_name == 'Christchurch West'`) that other charts on the dashboard can match against by column name. Previously the emitted filter was a `REPLACE(<geometry_col>, ' ', '') == <JSON.stringify(path)>` SQL clause comparing a bare coordinate path string against the full stored GeoJSON Feature, which could never match in practice. Receiving charts saw "No data" and the dashboard chip showed a wall of polygon JSON. Polygon resolves the dimension value from the picked feature's top-level fields (groupby/columns spread by addPropertiesToFeature), falling back to extraProps for js_columns. Geojson reads from the picked Feature's properties object (the standard non-spatial location). The legacy behaviour is preserved as a fallback when cross_filter_column is unset so existing dashboards keep working. Also fixes a related click-handling regression: deck.gl v9 dropped event.leftButton/event.rightButton from the picking event. The commonLayerProps wiring was checking those undefined fields, so left clicks silently emitted nothing and right clicks never opened the context menu. Dispatch now uses event.type and srcEvent.button. Supersedes apache#28262. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The flagged issue is correct: adding the cross_filter_column control to GeoJSON without updating the query to include the selected column causes a mismatch, leading to fallback filters. To resolve, update Geojson/buildQuery.ts to include cross_filter_column in queried columns, similar to Polygon layer. No other comments found in the PR. superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/buildQuery.ts |
There was a problem hiding this comment.
Code Review Agent Run #7bad6c
Actionable Suggestions - 2
-
superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts - 2
- CWE-1321: Object injection vulnerability via dynamic property access · Line 424-424
- CWE-1321: Object injection vulnerability in GeoJSON properties access · Line 488-488
Review Details
-
Files reviewed - 7 · Commit Range:
2d7bdfb..2d7bdfb- superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/controlPanel.ts
- superset-frontend/plugins/preset-chart-deckgl/src/layers/Polygon/buildQuery.ts
- superset-frontend/plugins/preset-chart-deckgl/src/layers/Polygon/controlPanel.ts
- superset-frontend/plugins/preset-chart-deckgl/src/layers/common.tsx
- superset-frontend/plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx
- superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.test.ts
- superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
…transform Reviewers pointed out that the geojson control was wired up but the value was never hydrated onto picked features. Geojson/transformProps parses each row's geojson cell into a Feature and discards every other column, so when the cross-filter dimension lived in a sibling row column rather than inside the GeoJSON properties, the click handler never found it and silently fell back to the broken legacy filter. Add cross_filter_column to the queried columns in Geojson/buildQuery, and merge the row value into the parsed Feature's properties in Geojson/transformProps. Pre-existing properties on the GeoJSON Feature take a back seat; the row-level value wins so what the user picked in the control is what gets emitted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review Agent Run #d9b4fb
Actionable Suggestions - 2
-
superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/transformProps.ts - 1
- CWE-1321: Prototype Pollution in Feature Properties Assignment · Line 47-51
-
superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/buildQuery.ts - 1
- Incorrect duplicate detection · Line 66-68
Review Details
-
Files reviewed - 2 · Commit Range:
2d7bdfb..cd83850- superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/buildQuery.ts
- superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/transformProps.ts
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
SUMMARY
deck.gl Polygon and Geojson charts emit cross-filters whose values cannot match anything in receiving charts. Clicking a region produces a chip with a long polygon-coordinate JSON string, and any receiving chart filtered by it shows "No data". The emitted SQL clause is:
…which compares a bare coordinate path to a column that typically stores a full GeoJSON Feature. It can never match.
This PR adds a new optional Cross-filter column control (
cross_filter_column) to deck_polygon and deck_geojson. When set, clicking a feature emits a clean dimension filter:…which other charts on the dashboard match by column name and renders as a readable chip.
Resolution paths for the dimension value:
data.object[col](groupby/columns are spread onto the picked feature byaddPropertiesToFeature), falling back todata.object.extraProps[col]forjs_columnsoverlap. The control is added to the chart's query inbuildQuery.tsso the column actually comes back in records.data.object.properties[col]— the standard non-spatial location on a GeoJSON Feature.When
cross_filter_columnis unset (or its value is missing on the picked feature), behavior falls back to the legacy filter so existing dashboards keep working. Aconsole.warnflags the fallback case so users know to re-save.Bonus fix: deck.gl v9 event-shape regression
While testing locally I found that left-click on any deck.gl chart with cross-filtering enabled has been silently emitting nothing since the deck.gl v9 upgrade. The handler in
commonLayerPropswas checkingevent.leftButton/event.rightButton, which deck.gl v9 no longer provides:Both flags are
undefinedin v9; the actual event shape is{ type, offsetCenter, srcEvent, tapCount }. Dispatch now usesevent.typeandsrcEvent.button(standard MouseEvent semantics —button === 0is left,=== 2is right). This restores left-click filter emission and right-click context menus across all deck.gl chart types, not just the polygon/geojson cases above.This fix is included in scope because the cross-filter feature is unobservably broken without it — even verifying the original bug locally requires the click handler to fire at all.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — text-only diff. Symptom is described in detail above.
TESTING INSTRUCTIONS
Polygon Column = geojson(any dataset whose geometry column stores GeoJSON Features will do).sa3_name).cross_filter_columnunset: clicking emits the legacy broken filter (chip = polygon JSON), which is what older charts expect. Backwards-compatible.cross_filter_column = 'sa3_name'(set under Customize → Advanced → Cross-filter column, then re-run the chart so the column lands in records, then save): clicking emitssa3_name == '<region>'. Chip readssa3_name: <region>. Receiving chart filters correctly.propertiesobject.Automated tests
crossFiltersDataMask.test.tsextended with 6 new cases: polygon w/ top-level dimension, polygon w/ extraProps fallback, polygon w/ legacy fallback when value missing (validates the warn), polygon legacy path (no cross_filter_column), geojson w/ properties, geojson legacy path.preset-chart-deckglpass; tsc and prettier clean.ADDITIONAL INFORMATION
This supersedes #28262 (open since April 2024, marked stale by maintainers, targets the now-removed
legacy-preset-chart-deckgl/path, predates the merged cross-filter feature in #33789).