codeant-ai-for-open-source[bot] commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r3651597500
##########
superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx:
##########
@@ -233,6 +236,10 @@ export default function DrillDetailPane({
// Download page of results & trim cache if page not in cache
useEffect(() => {
+ // Skip table data fetching if we're using a drill-through chart
+ if (dataset?.drill_through_chart_id) {
+ return;
Review Comment:
**Suggestion:** When a drill-through chart ID is configured but
`drillThroughFormData` is still unavailable or fails to load, this early return
disables the table data request. The subsequent rendering condition only
displays `StatefulChart` when form data exists, so the pane has no fallback
data and can remain empty indefinitely. Only skip the table fetch once the
drill-through chart is ready, or render an explicit loading/error state and
fallback to the table. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Configured drill-detail panes can render blank.
- ❌ Generic table fallback is disabled before chart readiness.
- ⚠️ Drill-through failures provide no explicit loading or error state.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the drill-detail pane, which renders `DrillDetailPane` in
`superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx:81`
with a dataset
containing `drill_through_chart_id` and the optional `drillThroughFormData`
prop still
unset.
2. The data-fetching effect at
`superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx:237`
evaluates
`dataset?.drill_through_chart_id` at line 240 and returns immediately at
line 241, so
`getDatasourceSamples()` at line 247 is never called.
3. During the same render, the chart branch at
`superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx:299`
is skipped
because it requires both the chart ID and `drillThroughFormData`.
4. `tableContent` therefore remains `null`; `bootstrapping` is false at line
292 because a
drill-through chart ID exists, and the pane can display no table or chart
until form data
becomes available. If form-data loading fails or never completes, the pane
remains empty
indefinitely.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d8d50056a90f4a5bb45dd6c7ec9101fa&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=d8d50056a90f4a5bb45dd6c7ec9101fa&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:** 240:241
**Comment:**
*Logic Error: When a drill-through chart ID is configured but
`drillThroughFormData` is still unavailable or fails to load, this early return
disables the table data request. The subsequent rendering condition only
displays `StatefulChart` when form data exists, so the pane has no fallback
data and can remain empty indefinitely. Only skip the table fetch once the
drill-through chart is ready, or render an explicit loading/error state and
fallback to the table.
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=709abc173a5390edde5e8a19f1d830863a6109ea3b7b1f3c60a8464f82cde23d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=709abc173a5390edde5e8a19f1d830863a6109ea3b7b1f3c60a8464f82cde23d&reaction=dislike'>👎</a>
##########
superset-frontend/src/components/Select/ChartSelect.tsx:
##########
@@ -0,0 +1,105 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useMemo } from 'react';
+import { t } from '@superset-ui/core';
+import SelectAsyncControl from
'src/explore/components/controls/SelectAsyncControl';
+import type { ComponentProps } from 'react';
+import rison from 'rison';
+
+// Extract the actual props from SelectAsyncControl component
+type SelectAsyncControlProps = ComponentProps<typeof SelectAsyncControl>;
+
+export interface ChartSelectProps
+ extends Omit<
+ SelectAsyncControlProps,
+ 'onChange' | 'dataEndpoint' | 'mutator' | 'addDangerToast'
+ > {
+ // ChartSelect-specific props that override base props
+ value?: number | null;
+ onChange: (value: number | null) => void;
+ datasetId?: number;
+}
+
+/**
+ * A chart selection component built on SelectAsyncControl
+ * @param value - The selected chart ID
+ * @param onChange - Callback when selection changes
+ * @param datasetId - Optional dataset ID to filter charts
+ * @param placeholder - Optional placeholder text
+ * @param ariaLabel - ARIA label for accessibility
+ * @param rest - All other props are passed through to SelectAsyncControl
+ */
+export default function ChartSelectUsingAsync({
+ value,
+ onChange,
+ datasetId,
+ placeholder = t('Select a chart'),
+ ariaLabel = t('Select drill-to-details chart'),
+ ...rest
+}: ChartSelectProps) {
+ // Build query parameters for filtering charts by dataset
+ const queryParams = useMemo(() => {
+ if (!datasetId) return undefined;
+
+ const filters = [
+ {
+ col: 'datasource_id',
+ opr: 'eq',
+ value: datasetId,
+ },
+ {
+ col: 'datasource_type',
+ opr: 'eq',
+ value: 'table',
+ },
+ ];
+
+ return {
+ q: rison.encode({
+ filters,
+ order_column: 'slice_name',
+ order_direction: 'asc',
+ }),
+ };
+ }, [datasetId]);
+
+ // Transform response to format expected by SelectAsyncControl
+ const mutator = useMemo(
+ () => (response: any) =>
+ response.result.map((chart: any) => ({
+ value: chart.id,
+ label: `${chart.slice_name} (${chart.viz_type})`,
+ })),
+ [],
Review Comment:
**Suggestion:** The response mutator assumes every successful or error
response contains a `result` array. If the chart API returns an error-shaped or
otherwise incomplete payload, `response.result.map` throws and breaks the
selector instead of allowing `SelectAsyncControl` to handle the failed request.
Normalize the response or guard the result before mapping. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dataset chart selector can crash on malformed responses.
- ⚠️ Drill-to-details chart configuration becomes unavailable during API
failures.
- ⚠️ Error handling is bypassed by the mutator exception.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Render `ChartSelectUsingAsync` from
`superset-frontend/src/components/Select/ChartSelect.tsx:48`, which
configures
`SelectAsyncControl` at lines 93-103 to load charts from `/api/v1/chart/` at
line 95.
2. Trigger the async chart request with a response that does not contain the
normal
chart-list `result` array, such as an error-shaped API payload.
3. `SelectAsyncControl` invokes the `mutator` created at lines 83-89 with
that response.
4. The expression `response.result.map(...)` at line 85 attempts to access
or call `map`
on an absent or non-array value, producing a runtime error and leaving the
chart selector
unable to render its options or process the request failure.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6f7c6276e35f4002937b9e14515b2bc7&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=6f7c6276e35f4002937b9e14515b2bc7&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/Select/ChartSelect.tsx
**Line:** 83:89
**Comment:**
*Possible Bug: The response mutator assumes every successful or error
response contains a `result` array. If the chart API returns an error-shaped or
otherwise incomplete payload, `response.result.map` throws and breaks the
selector instead of allowing `SelectAsyncControl` to handle the failed request.
Normalize the response or guard the result before mapping.
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=fe09693a4022e578a816b1eb9ca50b1e3b40d7931bb9a4398c1c5ac709c52ad8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=fe09693a4022e578a816b1eb9ca50b1e3b40d7931bb9a4398c1c5ac709c52ad8&reaction=dislike'>👎</a>
##########
superset/datasets/schemas.py:
##########
@@ -322,6 +323,7 @@ def fix_extra(self, data: dict[str, Any], **kwargs: Any) ->
dict[str, Any]:
fetch_values_predicate = fields.String(allow_none=True)
extra = fields.Dict(allow_none=True)
uuid = fields.UUID(required=True)
+ drill_through_chart_id = fields.Integer(allow_none=True)
Review Comment:
**Suggestion:** The import schema serializes the relationship as a
database-specific integer primary key. Dataset exports imported into another
environment can fail the foreign-key constraint when that slice ID does not
exist, or silently point to an unrelated chart when the ID has been reused. Use
a portable chart identifier such as the chart UUID and resolve it during
import. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Dataset imports can fail with invalid chart references.
- ❌ Cross-environment drill-through can target another chart.
- ⚠️ Dataset export/import loses relationship portability.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a dataset's drill-through chart through the dataset update
flow, which
persists the chart's database primary key in `drill_through_chart_id`.
2. Export that dataset using the versioned dataset import/export schema in
`superset/datasets/schemas.py:326`; the relationship is represented as an
integer chart ID
rather than a portable chart UUID.
3. Import the export into another Superset environment through the dataset
import path,
where chart primary keys are independently assigned and the original numeric
ID is not
guaranteed to identify the same chart.
4. If the referenced ID does not exist, the imported dataset can fail its
chart
foreign-key constraint; if it belongs to another chart, the dataset can
reference the
wrong drill-through target.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c9961049883848698369691107c8d812&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=c9961049883848698369691107c8d812&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/datasets/schemas.py
**Line:** 326:326
**Comment:**
*Possible Bug: The import schema serializes the relationship as a
database-specific integer primary key. Dataset exports imported into another
environment can fail the foreign-key constraint when that slice ID does not
exist, or silently point to an unrelated chart when the ID has been reused. Use
a portable chart identifier such as the chart UUID and resolve it during import.
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=4e6dcafbd0b396eb6d6e8633181819b4cdd35574e31699019361b8ce08b63a28&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=4e6dcafbd0b396eb6d6e8633181819b4cdd35574e31699019361b8ce08b63a28&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]
