alex-poor commented on code in PR #39906:
URL: https://github.com/apache/superset/pull/39906#discussion_r3197168623
##########
superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/controlPanel.ts:
##########
@@ -367,6 +368,7 @@ const config: ControlPanelConfig = {
{
label: t('Advanced'),
controlSetRows: [
+ [crossFilterColumn],
Review Comment:
Good catch — fixed in cd83850. Geojson/buildQuery now includes
`cross_filter_column` in the queried columns, and Geojson/transformProps merges
the row value onto each parsed Feature's `properties` so the click handler
finds it whether or not the GeoJSON blob itself carried the column.
##########
superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts:
##########
@@ -436,6 +478,40 @@ const getGeojsonFilters = ({
formData: LayerFormData;
data: PickingInfo;
}): FilterResult => {
+ // Preferred path: emit on a property of the picked GeoJSON Feature.
+ if (formData.cross_filter_column) {
+ const col = formData.cross_filter_column;
+ const properties = (data.object?.properties ?? {}) as Record<
+ string,
+ unknown
+ >;
+ const dimensionVal = properties[col] as
+ | string
+ | number
+ | boolean
+ | null
+ | undefined;
Review Comment:
Fixed in cd83850. `cross_filter_column` is now added to the deck_geojson
query in buildQuery.ts and surfaced onto `feature.properties` in
transformProps.ts, so this code path is reachable for any dataset column the
user picks — not just ones already embedded in the GeoJSON blob.
##########
superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts:
##########
@@ -408,11 +409,52 @@ const getLineColumnFilters = ({
data: PickingInfo;
}): FilterResult => {
const path = (data?.object?.path || data.object?.polygon) as string;
- const val = JSON.stringify(path);
if (!formData.line_column) throw new Error('Line column is required');
if (!path) throw new Error('Position of picked data is required');
+ // Preferred path: emit on a dimension column the user selected. The value
+ // can land either directly on the picked feature (groupby/excluded keys are
+ // spread by addPropertiesToFeature) or under extraProps when it overlaps
+ // with js_columns (addJsColumnsToExtraProps).
+ if (formData.cross_filter_column) {
+ const col = formData.cross_filter_column;
+ const obj = data.object ?? {};
+ const extraProps = (obj.extraProps ?? {}) as Record<string, unknown>;
+ const dimensionVal = (obj[col] ?? extraProps[col]) as
Review Comment:
Disagree — `col` here is `formData.cross_filter_column`, set by an
authenticated chart owner via the column-picker control (sourced from
`sharedControls.groupby`, which only offers real dataset column names). It is
owner-configured chart metadata, not untrusted runtime input. The same
dynamic-property-access pattern is used throughout this plugin (e.g.
`o.object[fd.line_column]` in Polygon.tsx tooltip rendering, and the existing
spatial filter paths in this same file). The suggested
`/^[a-zA-Z_][a-zA-Z0-9_]*$/` regex would also incorrectly reject many
legitimate column names — anything with hyphens, dots, or non-ASCII characters.
##########
superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts:
##########
@@ -436,6 +478,40 @@ const getGeojsonFilters = ({
formData: LayerFormData;
data: PickingInfo;
}): FilterResult => {
+ // Preferred path: emit on a property of the picked GeoJSON Feature.
+ if (formData.cross_filter_column) {
+ const col = formData.cross_filter_column;
+ const properties = (data.object?.properties ?? {}) as Record<
+ string,
+ unknown
+ >;
+ const dimensionVal = properties[col] as
Review Comment:
Same as above — `col` originates from chart-owner form_data, not untrusted
input. Not changing; flagging this as a false positive.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
