alex-poor commented on code in PR #39906:
URL: https://github.com/apache/superset/pull/39906#discussion_r3255427889
##########
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
+ | string
+ | number
+ | boolean
+ | null
+ | undefined;
+
+ if (dimensionVal != null) {
+ return {
+ values: [dimensionVal as string | number],
+ filters: [
+ {
+ col,
+ op: '==',
+ val: dimensionVal as string | number | boolean,
+ },
+ ],
+ };
+ }
+ // Value missing on the picked feature (e.g. column not in query yet
+ // because chart was saved pre-feature). Fall through to legacy path so
+ // the click still produces *some* filter rather than a silent error.
+ // eslint-disable-next-line no-console
+ console.warn(
Review Comment:
Addressed in 6feaa03 — both `console.warn` + fallback blocks replaced with
`throw new Error(...)`. That matches the rest of this file's convention for
misconfiguration (`throw new Error('Line column is required')`, etc), and means
a misconfigured chart no longer emits the broken legacy filter. The deck.gl
onClick handler swallows the error so the click just does nothing; users notice
and re-save the chart, instead of being misled by a filter chip showing polygon
JSON. New tests added for both polygon and geojson missing-value cases.
##########
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;
+
+ if (dimensionVal != null) {
+ return {
+ values: [dimensionVal as string | number],
+ filters: [
+ {
+ col,
+ op: '==',
+ val: dimensionVal as string | number | boolean,
+ },
+ ],
+ };
+ }
+ // eslint-disable-next-line no-console
+ console.warn(
Review Comment:
Addressed in 6feaa03 — see reply on the polygon comment above. Same change
applied here.
--
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]
