bito-code-review[bot] commented on code in PR #39906:
URL: https://github.com/apache/superset/pull/39906#discussion_r3255671149
##########
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:
<div>
<div id="suggestion">
<div id="issue"><b>Ambiguous error message</b></div>
<div id="fix">
The error on line 432-435 tells users to 're-save the chart,' but doesn't
explain WHY the column is missing (e.g., 'chart was saved before this column
was added to the query'). This leaves users confused about the cause and
whether re-saving will actually resolve the issue. Consider a more helpful
message like: 'Cross-filter column "{col}" not present on picked feature. The
chart was saved before this column was added to the query. Please remove and
re-add this chart to refresh its query.'
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
---
a/superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts
+++
b/superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts
@@ -429,8 +429,10 @@ const getLineColumnFilters = ({
| undefined;
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.`,
);
}
```
</div>
</details>
</div>
<small><i>Code Review Run #03b7c8</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts:
##########
@@ -436,6 +476,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) {
+ throw new Error(
+ `Cross-filter column "${col}" not present on picked feature
properties`,
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Inconsistent error context</b></div>
<div id="fix">
The error message at line 494-495 only says the column is 'not present on
picked feature properties' without explaining why or what action to take. Users
will see this cryptic error without understanding that this is likely a legacy
chart issue. Consider adding guidance similar to the message in
`getLineColumnFilters`.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
---
a/superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts
+++
b/superset-frontend/plugins/preset-chart-deckgl/src/utils/crossFiltersDataMask.ts
@@ -490,8 +490,10 @@ const getGeojsonFilters = ({
| undefined;
if (dimensionVal == null) {
throw new Error(
- `Cross-filter column "${col}" not present on picked feature
properties`,
+ `Cross-filter column "${col}" not present on picked feature
properties. ` +
+ `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.`,
);
}
```
</div>
</details>
</div>
<small><i>Code Review Run #03b7c8</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
