bito-code-review[bot] commented on code in PR #39906:
URL: https://github.com/apache/superset/pull/39906#discussion_r3255797652
##########
superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts:
##########
@@ -408,11 +409,50 @@ 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) {
Review Comment:
<!-- Bito Reply -->
The suggestion in the review is not actionable for two reasons:
1. The suggested remediation to 'remove and re-add the chart' is incorrect.
The correct workflow in Superset is to open the chart in Explore, click Run to
re-execute the query, and then Save. The suggestion would lead users to delete
and re-create their work, which is not ideal.
2. The error is thrown inside deck.gl's `onClick` handler, which swallows
exceptions silently. This means that end users do not see the error message;
only developers debugging in the browser DevTools console do. Padding the
message with verbose user-facing remediation steps does not provide any
benefit. The current terse message aligns with the file's existing style.
**superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts**
```
if (dimensionVal == null) {
throw new Error(
- `Cross-filter column "${col}" not present on picked feature. ` +
- `Re-save the chart to refresh its query.`,
+ `Cross-filter column "${col}" not present on picked feature. ` +
+ `This typically means the chart was saved before this column was
added to the query. ` +
+ `Please remove and re-add the chart, then re-save to refresh its
query.`,
);
}
```
--
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]
