codeant-ai-for-open-source[bot] commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r3664367898
##########
superset-frontend/src/components/Datasource/DatasourceModal/index.tsx:
##########
@@ -203,6 +205,10 @@ const DatasourceModal:
FunctionComponent<DatasourceModalProps> = ({
const { json } = await SupersetClient.get({
endpoint: `/api/v1/dataset/${currentDatasource?.id}`,
});
+
+ // Invalidate drill info cache to pick up any drill-through config
changes
+ invalidateDatasetDrillCache(currentDatasource.id);
Review Comment:
**Suggestion:** The cache is invalidated only after both the dataset PUT and
the follow-up GET succeed. If the PUT succeeds but the GET fails, the catch
path reports a save error without clearing the old drill-info entry, so
subsequent drill-to-detail operations continue using the previous chart
configuration. Invalidate the drill-info cache immediately after a successful
mutation, independently of the refresh GET. [cache]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dataset saves can leave stale drill configuration cached.
- ⚠️ Subsequent dashboard drill actions use the previous chart.
- ⚠️ A failed refresh falsely indicates the new configuration was not
applied.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the dataset editor and save a changed drill-through chart;
DatasourceModal.tsx:200-203 successfully sends the dataset PUT request.
2. Make the follow-up dataset GET at DatasourceModal.tsx:205-207 fail
through a transient
network or server error after the PUT has already succeeded.
3. Because invalidateDatasetDrillCache is called only at
DatasourceModal.tsx:209-210 after
the GET resolves, the catch path at lines 221-224 reports an error without
deleting cached
drill-info entries.
4. Reopen or use drill-to-detail from SliceHeaderControls.tsx:588; the
dataset drill
metadata is loaded through cachedSupersetGet at
superset-frontend/src/hooks/apiResources/datasets.ts:99-103, so the stale
cached
configuration can continue selecting the previous chart.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0f7de58c3c124a4c9593a17c2e7e4866&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0f7de58c3c124a4c9593a17c2e7e4866&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/components/Datasource/DatasourceModal/index.tsx
**Line:** 209:210
**Comment:**
*Cache: The cache is invalidated only after both the dataset PUT and
the follow-up GET succeed. If the PUT succeeds but the GET fails, the catch
path reports a save error without clearing the old drill-info entry, so
subsequent drill-to-detail operations continue using the previous chart
configuration. Invalidate the drill-info cache immediately after a successful
mutation, independently of the refresh GET.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=82b66d8bda75243c2aac90674312c27a040e2f2c7bac0c605eb92d4ed00de92e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=82b66d8bda75243c2aac90674312c27a040e2f2c7bac0c605eb92d4ed00de92e&reaction=dislike'>👎</a>
##########
superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx:
##########
@@ -109,14 +119,65 @@ export default function DrillDetailModal({
findPermission('can_explore', 'Superset', state.user?.roles),
);
- const exploreUrl = useMemo(
- () => `/explore/?dashboard_page_id=${dashboardPageId}&slice_id=${chartId}`,
- [chartId, dashboardPageId],
+ const showEditButton = Boolean(dataset?.drill_through_chart_id);
+ const dashboardContextFormData = useDashboardFormData(
+ dataset?.drill_through_chart_id,
);
Review Comment:
**Suggestion:** Dashboard context is calculated for the configured target
chart only. A target chart that is not present in the current dashboard, or
whose scope does not include that chart ID, receives no scoped dashboard
filters even when the source chart is affected by them. This makes
drill-through results differ from the filtered source chart; the context should
account for the source chart's active filters or explicitly merge the
applicable source and target scopes. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Drill results can omit filters applied to the source chart.
- ⚠️ Cross-dashboard drill consistency breaks for scoped filters.
- ⚠️ Users see results differing from the chart they drilled from.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a dashboard native filter whose scope includes the source chart
but excludes
the dataset-level drill-through target chart; scoped filter application is
determined by
getFilterIdsAppliedOnChart at
superset-frontend/src/dashboard/util/getFilterIdsAppliedOnChart.ts:31-37.
2. Trigger drill-to-detail from the source chart through
ChartContextMenu.tsx:447, which
passes the source chart ID and drill form data into DrillDetailModal.
3. DrillDetailModal.tsx:123-125 asks useDashboardFormData for the target
chart ID, not the
source chart ID; useDashboardFormData.ts:81-85 therefore excludes filters
whose scope does
not contain the target ID.
4. DrillDetailModal.tsx:138-143 merges that target-only dashboard context
with the drill
filters, so the StatefulChart at DrillDetailPane.tsx:303-309 queries without
dashboard
filters that affected the source chart, producing different drill-through
results.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e3fe2e31e3cb442ca5289a0276669f5b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e3fe2e31e3cb442ca5289a0276669f5b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx
**Line:** 123:125
**Comment:**
*Incorrect Condition Logic: Dashboard context is calculated for the
configured target chart only. A target chart that is not present in the current
dashboard, or whose scope does not include that chart ID, receives no scoped
dashboard filters even when the source chart is affected by them. This makes
drill-through results differ from the filtered source chart; the context should
account for the source chart's active filters or explicitly merge the
applicable source and target scopes.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=1779e61a642c2bfdbb2ab6440f90430910cbc8bb32f135e513764de62756cc75&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=1779e61a642c2bfdbb2ab6440f90430910cbc8bb32f135e513764de62756cc75&reaction=dislike'>👎</a>
##########
superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx:
##########
@@ -282,12 +289,27 @@ export default function DrillDetailPane({
resultsPages,
]);
- const bootstrapping = !responseError && !resultsPages.size;
+ const bootstrapping =
+ !dataset?.drill_through_chart_id && !responseError && !resultsPages.size;
const allowHTML = formData.allow_render_html ?? true;
let tableContent = null;
- if (responseError) {
+
+ // If a drill-through chart is configured, use it instead of the table
+ if (dataset?.drill_through_chart_id && drillThroughFormData) {
+ tableContent = (
+ <Flex vertical style={{ height: '100%' }}>
+ <StatefulChart
+ chartId={dataset.drill_through_chart_id}
+ formDataOverrides={drillThroughFormData}
+ height="100%"
+ width="100%"
+ showLoading
+ />
Review Comment:
**Suggestion:** `StatefulChart` refetches whenever the `formDataOverrides`
object changes, but its fetch callback captures the props from its initial
render. When dashboard filters or drill filters change while the modal is open,
this call therefore triggers a request using the original overrides and the
displayed chart remains stale. The chart component or this integration must
ensure that refetches use the latest form data. [stale reference]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Open drill views can display stale filtered results.
- ⚠️ Dashboard filter changes are not reflected reliably.
- ⚠️ Users may export or inspect outdated drill data.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open drill-to-detail from the dashboard context-menu path at
superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx:447,
which
supplies drill filters and dashboard context to the modal.
2. DrillDetailPane.tsx:299-309 renders StatefulChart with the memoized
drillThroughFormData as formDataOverrides.
3. When dashboard context or drill filters change,
DrillDetailModal.tsx:127-149 produces a
new override object, and StatefulChart.tsx:66-74 correctly detects the
changed
formDataOverrides and invokes fetchData.
4. StatefulChart.tsx:170-180 defines fetchData with an empty dependency
array, so that
callback retains the props from its initial render;
StatefulChart.tsx:193-212 consequently
fetches using the original overrides even though the effect was triggered by
newer
overrides, leaving the displayed chart stale.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9a0476ad56ab4f539c25263687014562&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9a0476ad56ab4f539c25263687014562&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx
**Line:** 299:309
**Comment:**
*Stale Reference: `StatefulChart` refetches whenever the
`formDataOverrides` object changes, but its fetch callback captures the props
from its initial render. When dashboard filters or drill filters change while
the modal is open, this call therefore triggers a request using the original
overrides and the displayed chart remains stale. The chart component or this
integration must ensure that refetches use the latest form data.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=b839ca12fc08ba20773456deebbc10e33a5d262f51f84eb51d4ed46f3c592325&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=b839ca12fc08ba20773456deebbc10e33a5d262f51f84eb51d4ed46f3c592325&reaction=dislike'>👎</a>
--
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]
