You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "mistercrunch (via GitHub)" <gi...@apache.org> on 2025/08/21 06:42:03 UTC

[PR] Custom drillthrough [superset]

mistercrunch opened a new pull request, #34785:
URL: https://github.com/apache/superset/pull/34785

   This PR introduces customizable drill-to-details functionality that allows dataset owners to configure which chart should be used when users drill through from any chart using their dataset.
   
   <img width="527" height="301" alt="Screenshot 2025-08-20 at 11 41 36 PM" src="https://github.com/user-attachments/assets/4f591c27-3ecb-4edd-8b3a-c5a679f25439" />
   
   <img width="899" height="742" alt="Screenshot 2025-08-20 at 11 36 27 PM" src="https://github.com/user-attachments/assets/a0091aa4-ad7e-4ecc-bddf-b9a4528bdf14" />
   
   Problem: Current drill-to-details uses a one-size-fits-all approach showing all columns in a generic table with no customization options.
   
   Solution: Dataset-level drill-to-details customization allowing association of any existing chart as the drill-to-details target.
   
   Key Benefits:
   - Relevant data display: Show only columns that matter for drill context
   - Flexible chart types: Any chart type or visualization supported
   - Backwards compatible: No-op unless configured, falls back gracefully
   - Cross-dashboard consistency: Same experience across all dashboards
   
   Implementation:
   - Backend: Added drill_through_chart_id column with FK relationship
   - Frontend: ChartSelect component with Dataset Editor integration
   - Enhanced drill modal to render StatefulChart when configured
   - Full TypeScript support with proper type definitions
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3250920302

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Default Agent` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5108498760

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5786548026

### ⚠️ Possible DB migration conflict

@mistercrunch the base branch `master` picked up a new database migration under `superset/migrations/` after this PR was opened, and this PR adds or modifies migration files too. Alembic migrations form a single linear chain, so if both branch off the same parent revision, merging this PR as-is could leave the migration history with two heads instead of one, which makes `superset db upgrade` fail with `Multiple head revisions are present`.

To check and, if needed, fix this:

1. Rebase this branch onto the latest `master`.
2. Run `superset db heads`. If it lists more than one revision, you have a conflict.
3. In your new migration's script, update `down_revision` (and the `Revises` comment) to point at the other head instead, so the chain has a single head again.

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3222634830

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/17228457413) for [48b2a46](https://github.com/apache/superset/commit/48b2a46)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.

mistercrunch commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r2323320730


##########
superset-frontend/package-lock.json:
##########


Review Comment:
   wasn't, now I'm wondering what's not deterministic in our package.json causing these side effects ... 



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3250823703

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/17446236725) for [e8f275d](https://github.com/apache/superset/commit/e8f275d)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5763324590

   # 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️❗ @mistercrunch Your base branch `master` has also updated `superset/migrations`.
   
   **Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5091851874

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3250856656

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/17446236725) for [e8f275d](https://github.com/apache/superset/commit/e8f275d)
   
   • **Environment:** http://34.220.106.89:8080 (admin/admin)
   • **Lifetime:** 24h auto-cleanup
   • **Updates:** New commits create fresh environments automatically


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5084438384

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r3651598072


##########
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,
   );
 
-  const exploreChart = useCallback(() => {
-    history.push(exploreUrl);
-  }, [exploreUrl, history]);
+  const drillThroughFormData = useMemo(() => {
+    if (!dataset?.drill_through_chart_id || !dataset?.id) {
+      return null;
+    }
+
+    const drillThroughBaseFormData = {
+      slice_id: dataset.drill_through_chart_id,
+      datasource: `${dataset.id}__table`,
+      viz_type: 'table',
+    };

Review Comment:
   **Suggestion:** The configured target chart is always initialized with `viz_type: 'table'`, and no target chart form data is loaded here. Consequently, selecting a non-table chart or a table chart with saved visualization settings causes the drill-through visualization to use the wrong chart type or lose its configuration. The target chart's saved form data should be used instead of hardcoding the table type. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ Configured non-table drill targets render as tables.
   - ⚠️ Table targets lose saved visualization configuration.
   - ⚠️ Dataset-level drill customization becomes unreliable.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Configure `SqlaTable.drill_through_chart_id` through the dataset editor so it
   references an existing chart, as represented by
   `superset/connectors/sqla/models.py:1152-1159`.
   
   2. Open any dashboard chart using that dataset and trigger drill-to-detail, causing
   `DrillDetailModal` to compute `drillThroughFormData` at lines 127-149 and pass it to
   `DrillDetailPane` at lines 216-221.
   
   3. The form data created at lines 132-136 uses the selected chart ID but forces `viz_type`
   to `table` and sets the datasource to the dataset table; it does not load the selected
   slice's saved form data.
   
   4. Observe that a configured non-table target is rendered through the table visualization
   path, while saved target-specific controls such as columns, metrics, ordering, formatting,
   and other visualization settings are not applied.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6087cad12b7944b8b5cdfad8b7b5e1d4&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=6087cad12b7944b8b5cdfad8b7b5e1d4&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:** 132:136
   **Comment:**
   	*Logic Error: The configured target chart is always initialized with `viz_type: 'table'`, and no target chart form data is loaded here. Consequently, selecting a non-table chart or a table chart with saved visualization settings causes the drill-through visualization to use the wrong chart type or lose its configuration. The target chart's saved form data should be used instead of hardcoding the table type.
   
   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=ff4390ecbd30c487b160fb2f815eed37ee09fba6fd9f7edc266439bd2670dfe4&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=ff4390ecbd30c487b160fb2f815eed37ee09fba6fd9f7edc266439bd2670dfe4&reaction=dislike'>👎</a>



##########
superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx:
##########
@@ -131,7 +192,12 @@ export default function DrillDetailModal({
       name={t('Drill to detail: %s', chartName)}
       title={t('Drill to detail: %s', chartName)}
       footer={
-        <ModalFooter exploreChart={exploreChart} canExplore={canExplore} />
+        <ModalFooter
+          canExplore={canExplore}
+          showEditButton={showEditButton}
+          onExploreClick={handleExploreClick}
+          isGeneratingUrl={isGeneratingUrl}
+        />

Review Comment:
   **Suggestion:** The modal footer no longer receives `closeModal`, so `ModalFooter` renders its primary close button with an undefined `onClick` handler. Users cannot close the drill-to-detail modal through the Close button; pass `onHideModal` (or the appropriate close callback) as `closeModal`. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ Drill-to-detail users cannot close the modal using Close.
   - ⚠️ Users must rely on alternate modal dismissal behavior.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open a dashboard chart and trigger the existing drill-to-detail flow, which renders
   `DrillDetailModal` from
   `superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx`.
   
   2. The modal renders `ModalFooter` at lines 195-200, but does not pass the `closeModal`
   prop.
   
   3. `ModalFooter` declares `closeModal` at line 45 and assigns it to the primary button's
   `onClick` handler at lines 77-81.
   
   4. Click the Close button (`data-test="close-drilltodetail-modal"`); because `closeModal`
   is `undefined`, `onHideModal` is never invoked and the drill-to-detail modal remains open.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a963eb5e31774485bbd14906d7552f9d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a963eb5e31774485bbd14906d7552f9d&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:** 195:200
   **Comment:**
   	*Logic Error: The modal footer no longer receives `closeModal`, so `ModalFooter` renders its primary close button with an undefined `onClick` handler. Users cannot close the drill-to-detail modal through the Close button; pass `onHideModal` (or the appropriate close callback) as `closeModal`.
   
   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=e3d94cb262cf33de50aff2569eede16d443507eb2fd8cae9b80ed767bca71b99&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=e3d94cb262cf33de50aff2569eede16d443507eb2fd8cae9b80ed767bca71b99&reaction=dislike'>👎</a>



##########
superset/connectors/sqla/models.py:
##########
@@ -1148,6 +1148,16 @@ class SqlaTable(
     always_filter_main_dttm = Column(Boolean, default=False)
     folders = Column(JSON, nullable=True)
 
+    # Drill-through configuration
+    drill_through_chart_id = Column(
+        Integer, ForeignKey("slices.id", ondelete="SET NULL"), nullable=True
+    )

Review Comment:
   **Suggestion:** This adds a database column and foreign key in the model without an accompanying migration in the PR. On an existing deployment, queries or updates that serialize or reference `drill_through_chart_id` will fail because the physical column does not exist. Add and deploy the corresponding Alembic migration. [possible bug]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ Existing deployments cannot reliably load affected datasets.
   - ❌ Dataset editor reads and updates can fail.
   - ⚠️ Upgrades require manual schema repair without migration.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Upgrade an existing Superset deployment to the PR version without applying a schema
   migration; the provided PR contains the model change but no Alembic migration.
   
   2. Open a dataset through the datasource editor or another dataset API path that
   serializes `SqlaTable.data`, which now reads `self.drill_through_chart_id` at
   `superset/connectors/sqla/models.py:1365-1380`.
   
   3. SQLAlchemy generates a query or update referencing `drill_through_chart_id`, but the
   existing `tables` database table has no such physical column.
   
   4. Observe a database error such as an unknown-column error, causing dataset loading or
   saving to fail until the migration is applied.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=83d72909a03c4651b46e17e75e6726c6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=83d72909a03c4651b46e17e75e6726c6&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/connectors/sqla/models.py
   **Line:** 1152:1154
   **Comment:**
   	*Possible Bug: This adds a database column and foreign key in the model without an accompanying migration in the PR. On an existing deployment, queries or updates that serialize or reference `drill_through_chart_id` will fail because the physical column does not exist. Add and deploy the corresponding Alembic migration.
   
   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=c6ad7930acadbcdb41172f0eeaaf3d99b415a424883038898d3e5f7b12f7ad4d&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=c6ad7930acadbcdb41172f0eeaaf3d99b415a424883038898d3e5f7b12f7ad4d&reaction=dislike'>👎</a>



##########
superset/connectors/sqla/models.py:
##########
@@ -1169,6 +1179,7 @@ class SqlaTable(
         "normalize_columns",
         "always_filter_main_dttm",
         "folders",
+        "drill_through_chart_id",
     ]

Review Comment:
   **Suggestion:** Including the raw integer `drill_through_chart_id` in dataset export/import makes exports environment-dependent. Slice IDs are not stable across environments, so an imported dataset can point to an unrelated chart or fail its foreign-key constraint instead of referencing the selected drill-through chart. Export a stable chart identifier and resolve it during import, or omit this relationship from portable dataset exports. [possible bug]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Imported datasets may drill to an unrelated chart.
   - ⚠️ Cross-environment dataset exports lose chart relationship portability.
   - ⚠️ Imports can fail when the referenced slice ID is absent.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Create a dataset with `drill_through_chart_id` set to a chart, using the relationship
   defined at `superset/connectors/sqla/models.py:1152-1159`.
   
   2. Export the dataset through Superset's model export mechanism; `drill_through_chart_id`
   is included in `export_fields` at lines 1163-1184 and is therefore serialized as a raw
   integer.
   
   3. Import that export into another Superset environment where slice IDs differ, or where
   the numeric ID is already assigned to a different chart.
   
   4. The import update path uses `update_from_object_fields` at line 1184, so the numeric
   value can fail its foreign-key constraint or resolve to an unrelated chart; subsequent
   drill-to-detail requests then use the wrong target.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f91df71aa69b477eb4ace8f674f59870&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f91df71aa69b477eb4ace8f674f59870&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/connectors/sqla/models.py
   **Line:** 1181:1183
   **Comment:**
   	*Possible Bug: Including the raw integer `drill_through_chart_id` in dataset export/import makes exports environment-dependent. Slice IDs are not stable across environments, so an imported dataset can point to an unrelated chart or fail its foreign-key constraint instead of referencing the selected drill-through chart. Export a stable chart identifier and resolve it during import, or omit this relationship from portable dataset exports.
   
   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=db8f5c33c9ed4d96fd42ebf5dd6de22d8ccab165ded295e9b9b634154f14ed41&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=db8f5c33c9ed4d96fd42ebf5dd6de22d8ccab165ded295e9b9b634154f14ed41&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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-6102863664

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5706352318

   # 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️❗ @mistercrunch Your base branch `master` has also updated `superset/migrations`.
   
   **Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-6461081082

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "EMachad0 (via GitHub)" <gi...@apache.org>.

EMachad0 commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3249482547

   Hey, I really liked your approach and I believe it addresses all the needs of SIP-160!


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3252115601

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Default Agent` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3544380558

   Any hope of getting this one through? It's a pretty awesome feature :D 


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5719096061

   # 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️❗ @mistercrunch Your base branch `master` has also updated `superset/migrations`.
   
   **Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-6070747933

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-6022159155

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.

mistercrunch commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3212872910

   While testing, you might hit a weird known issues related to this comment: https://github.com/apache/superset/pull/23454#issuecomment-3212866369
   
   Refreshing the page fixes it, so you may need to refresh the page between saving the dataset with the new drill-to-chart for it to work in the dashboard.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.

mistercrunch commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r2338391073


##########
superset/datasets/api.py:
##########


Review Comment:
   Yeah thought about this and I'm thinking we need another similar feature for:
   - drill to chart
   - drill to dashboard
   
   Now the great thing is that it's now fairly easy to do drill-to-chart, largely similar feature building on this PR. For drill-to-dashboard, it's just a matter of opening another dashboard with the right filters applied, also fairly easy I think.
   
   The challenge, and where we'd need product/design input @yousoph @kasiazjc , is how does one configure this (?)
   
   Seems it's the dashboard's owner responsibility do define how they want to enable users to drill. So likely a "Dashboard Properties" thing. Guessing there could be a "Drilling" section in dashboard props where the dashboad owner would pick things like:
   - Drill to charts:
     - Select a specific set of charts
     - Any chart present on the dashboard (does this make sense, why not cross-filtering instead? maybe can maintain the state of the dashboard while seeing more details...)
     - OR any chart on the associated dataset (?) beyond the dashboard
   - Drill to dashboard
     - select a set of related dashboard, all filters from the current state would try to carry through, assuming dashboards share filter configurations (?)
   
   That's without touching the complexity of:
   - which specific chart or specific dimension would allow drilling to which set of charts? Say you might want to drill into a specific customer to the "customer scorecard" dashboard, like only customer_name or customer_id would be ones that make sense to target for that specific use case...
   - charts that share dataset are naturally "drillable", charts on different dataset that share some columns are partially drillable, ...
   
   Seems there's quite a bit of complexity here. Could go with simpler hard rules like "we'll allow you to drill only on charts that share the same dataset", OR we'll do our best effort to pass down filters that align across dataset.
   
   Seems like "configurability" here is slippery, there are lots of things people might want to open and/or restrict...



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3250556053

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Default Agent` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.

sadpandajoe commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r3825217838


##########
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:
   This exports an environment-local `slices.id` as dataset configuration. Importing a dataset into another instance can either fail the foreign key before that chart exists or bind the dataset to an unrelated chart with the same ID. Could this use a portable chart reference and resolve it after chart import instead?



##########
superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx:
##########
@@ -109,14 +120,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,
   );
 
-  const exploreChart = useCallback(() => {
-    history.push(exploreUrl);
-  }, [exploreUrl, history]);
+  const drillThroughFormData = useMemo(() => {
+    if (!dataset?.drill_through_chart_id || !dataset?.id) {
+      return null;
+    }
+
+    const drillThroughBaseFormData = {
+      slice_id: dataset.drill_through_chart_id,
+      datasource: `${dataset.id}__table`,
+      viz_type: 'table',

Review Comment:
   This overrides the saved chart's `viz_type`, so configuring any non-table chart for drill-through still renders a table. Could the override contain only the drill context and leave the selected chart's visualization type intact?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r3651880145


##########
superset-frontend/src/components/Datasource/DatasourceModal/index.tsx:
##########
@@ -181,6 +182,7 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
       owners: datasource.owners.map(
         (o: Record<string, number>) => o.value || o.id,
       ),
+      drill_through_chart_id: datasource.drill_through_chart_id || null,

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Falsy number incorrectly coerced to null</b></div>
   <div id="fix">
   
   Using `|| null` incorrectly converts falsy numbers like `0` to `null`. Since chart IDs can be `0`, use `?? null` instead to only fall back to `null` for `null`/`undefined`. This matches the type definition `number | null`.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #1064ad</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/src/utils/cachedSupersetGet.ts:
##########
@@ -27,3 +27,20 @@ export const cachedSupersetGet = cacheWrapper(
   supersetGetCache,
   ({ endpoint }) => endpoint || '',
 );
+
+/**
+ * Invalidates cached dataset drill info for all dashboards containing this dataset
+ */
+export const invalidateDatasetDrillCache = (datasetId: string | number) => {
+  const numericDatasetId =
+    typeof datasetId === 'string'
+      ? Number(datasetId.split('__')[0])
+      : Number(datasetId);
+
+  // Find and delete all cache entries for this dataset's drill info
+  const keysToDelete = Array.from(supersetGetCache.keys()).filter(key =>
+    key.includes(`/api/v1/dataset/${numericDatasetId}/drill_info/`),
+  );
+
+  keysToDelete.forEach(key => supersetGetCache.delete(key));
+};

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Missing unit tests for new function</b></div>
   <div id="fix">
   
   The new `invalidateDatasetDrillCache` function has no unit tests. Per project guidelines, new tools/features should have comprehensive unit tests covering success paths, error scenarios, and edge cases to ensure robust error handling and prevent regressions.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #1064ad</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/src/components/Chart/DrillDetail/DrillDetailModal.test.tsx:
##########
@@ -32,6 +32,14 @@ jest.mock('react-router-dom', () => ({
   }),
 }));
 
+jest.mock('src/explore/exploreUtils', () => ({
+  ...jest.requireActual('src/explore/exploreUtils'),
+  getExploreUrl: jest.fn(
+    ({ formData }) =>
+      `/explore/?dashboard_page_id=&slice_id=${formData.slice_id}`,
+  ),
+}));

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Mock targets unused function</b></div>
   <div id="fix">
   
   The test currently mocks `getExploreUrl`, but the component uses `generateExploreUrl` imported from 'src/explore/exploreUtils/formData'. Update the mock to target `generateExploreUrl` in that module (or mock `postFormData` to avoid real HTTP requests) to ensure the URL generation is stubbed during tests.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #1064ad</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/src/features/datasets/AddDataset/Footer/Footer.test.tsx:
##########
@@ -119,9 +119,9 @@ describe('Footer', () => {
     const dropdownTrigger = screen.getByRole('button', { name: 'down' });
     userEvent.click(dropdownTrigger);
 
-    // Check that the dropdown menu option is visible
+    // Check that the dropdown menu option is in the document
     await waitFor(() => {
-      expect(screen.getByText('Create dataset')).toBeVisible();
+      expect(screen.getByText('Create dataset')).toBeInTheDocument();

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Test assertion weakened</b></div>
   <div id="fix">
   
   This change weakens the assertion. `toBeVisible()` verifies the dropdown is both in the DOM and actually rendered on screen, while `toBeInTheDocument()` only checks DOM presence. If a CSS issue or animation prevents the dropdown from being visible, `toBeInTheDocument()` would still pass but the feature would be broken.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #1064ad</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/src/components/Select/ChartSelect.test.tsx:
##########
@@ -0,0 +1,115 @@
+/**
+ * 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 { render } from 'spec/helpers/testing-library';
+import ChartSelectUsingAsync from './ChartSelect';
+
+const mockOnChange = jest.fn();
+
+const defaultProps = {
+  value: null,
+  onChange: mockOnChange,
+  datasetId: 123,
+  placeholder: 'Select a chart',
+  allowClear: true,
+  ariaLabel: 'Select drill-to-details chart',
+};
+
+test('renders chart select component with default props', () => {
+  const { container } = render(<ChartSelectUsingAsync {...defaultProps} />, {
+    useRedux: true,
+  });
+  expect(container.firstChild).toBeInTheDocument();
+});
+
+test('renders with custom placeholder', () => {
+  const customProps = {
+    ...defaultProps,
+    placeholder: 'Choose your chart',
+  };
+
+  const { container } = render(<ChartSelectUsingAsync {...customProps} />, {
+    useRedux: true,
+  });
+  expect(container.firstChild).toBeInTheDocument();
+});
+
+test('renders with selected value', () => {
+  const propsWithValue = {
+    ...defaultProps,
+    value: 456,
+  };
+
+  const { container } = render(<ChartSelectUsingAsync {...propsWithValue} />, {
+    useRedux: true,
+  });
+  expect(container.firstChild).toBeInTheDocument();
+});
+
+test('renders without dataset filter when datasetId is undefined', () => {
+  const propsWithoutDataset = {
+    ...defaultProps,
+    datasetId: undefined,
+  };
+
+  const { container } = render(
+    <ChartSelectUsingAsync {...propsWithoutDataset} />,
+    { useRedux: true },
+  );
+  expect(container.firstChild).toBeInTheDocument();
+});
+
+test('renders with custom aria label', () => {
+  const propsWithCustomAriaLabel = {
+    ...defaultProps,
+    ariaLabel: 'Custom chart selector',
+  };
+
+  const { container } = render(
+    <ChartSelectUsingAsync {...propsWithCustomAriaLabel} />,
+    { useRedux: true },
+  );
+  expect(container.firstChild).toBeInTheDocument();
+});
+
+test('renders as non-clearable when allowClear is false', () => {
+  const nonClearableProps = {
+    ...defaultProps,
+    allowClear: false,
+  };
+
+  const { container } = render(
+    <ChartSelectUsingAsync {...nonClearableProps} />,
+    { useRedux: true },
+  );
+  expect(container.firstChild).toBeInTheDocument();
+});
+
+test('passes through additional props to SelectAsyncControl', () => {
+  const propsWithExtra = {
+    ...defaultProps,
+    description: 'Test description',
+    hovered: true,
+    'data-testid': 'chart-select',
+  };
+
+  const { container } = render(<ChartSelectUsingAsync {...propsWithExtra} />, {
+    useRedux: true,
+  });
+  expect(container.firstChild).toBeInTheDocument();
+});

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Shallow tests skip core behavior</b></div>
   <div id="fix">
   
   These tests currently only assert that the component renders. To cover core behavior, add tests that:
   1) mock the `/api/v1/chart/` endpoint and verify that selecting an option triggers `onChange` with the correct id,
   2) capture the query parameters when `datasetId` is provided vs undefined,
   3) check that additional props (like `description` and `data-testid`) are passed through to the underlying `SelectAsyncControl`.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #1064ad</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/migrations/versions/2025-08-21_01-54_852e99567fe7_.py:
##########
@@ -0,0 +1,35 @@
+# 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.
+"""merge heads: theme system and drill-through chart
+
+Revision ID: 852e99567fe7
+Revises: ('c233f5365c9e', 'f56ac3accfc9')
+Create Date: 2025-08-21 01:54:38.918459
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "852e99567fe7"
+down_revision = ("c233f5365c9e", "f56ac3accfc9")
+
+
+def upgrade():
+    pass
+
+
+def downgrade():
+    pass

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Merge revision missing migration logic</b></div>
   <div id="fix">
   
   The `upgrade()` and `downgrade()` functions are empty stubs (`pass`). This merge revision must incorporate the actual schema changes from both parent migrations — `c233f5365c9e` (theme system columns + indexes) and `f56ac3accfc9` (drill-through chart FK). Without the migration logic, applying this revision will leave the database incomplete, causing runtime errors when the application tries to use `themes.is_system_default`, `themes.is_system_dark`, or `tables.drill_through_chart_id`.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #1064ad</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/migrations/versions/2025-08-21_01-54_852e99567fe7_.py:
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Invalid module name with trailing underscore</b></div>
   <div id="fix">
   
   The filename contains a trailing underscore which violates module naming conventions. Consider renaming the file to remove the trailing underscore.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #1064ad</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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r3658076279


##########
superset/datasets/api.py:
##########
@@ -220,6 +220,7 @@ class DatasetRestApi(BaseSupersetModelRestApi):
         "changed_on_humanized",
         "changed_by.first_name",
         "changed_by.last_name",
+        "drill_through_chart_id",

Review Comment:
   **Suggestion:** The new API column references `drill_through_chart_id`, but this PR does not add the corresponding SQLAlchemy attribute to the `SqlaTable` model. Dataset queries that build their select list from this value will fail with an attribute-resolution error, and updates cannot persist the field. Add the model column and relationship before exposing it through the API. [possible bug]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ Dataset API reads can fail when selecting exposed fields.
   - ❌ Dataset editor updates cannot persist drill-through chart configuration.
   - ⚠️ Drill-to-details configuration remains unavailable through backend APIs.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Start Superset with this PR applied and use the dataset REST API route implemented by
   `DatasetRestApi` in `superset/datasets/api.py`.
   
   2. Request a dataset using the API's selectable-column path, such as `GET
   /api/v1/dataset/<dataset_id>`, which uses `show_select_columns` at
   `superset/datasets/api.py:210-224`.
   
   3. The datamodel attempts to resolve `drill_through_chart_id` from the `SqlaTable` ORM
   model while constructing the query, but the supplied PR changes show no corresponding
   `SqlaTable` attribute or relationship.
   
   4. Observe an ORM attribute-resolution/query construction error; dataset updates using
   `DatasetPutSchema.drill_through_chart_id` at `superset/datasets/schemas.py:156-178`
   likewise cannot persist the value unless the model column and migration are added.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f627ac7ee50849ec93cd6eef4354abed&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f627ac7ee50849ec93cd6eef4354abed&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/api.py
   **Line:** 223:223
   **Comment:**
   	*Possible Bug: The new API column references `drill_through_chart_id`, but this PR does not add the corresponding SQLAlchemy attribute to the `SqlaTable` model. Dataset queries that build their select list from this value will fail with an attribute-resolution error, and updates cannot persist the field. Add the model column and relationship before exposing it through the API.
   
   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=f9cf68e9b52cdcc7b1df68a51f38c9a89e228c70e2b3e9c374fed25a6166808f&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=f9cf68e9b52cdcc7b1df68a51f38c9a89e228c70e2b3e9c374fed25a6166808f&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})`,
+      })),
+    [],
+  );
+
+  return (
+    <SelectAsyncControl
+      ariaLabel={ariaLabel}
+      dataEndpoint="/api/v1/chart/"
+      searchParams={queryParams}
+      mutator={mutator}
+      value={value}
+      onChange={onChange}
+      placeholder={placeholder}
+      multi={false}
+      {...rest}

Review Comment:
   **Suggestion:** The spread occurs after `multi={false}`, so callers can pass `multi` through `rest` and override the component's single-select configuration. In that case `SelectAsyncControl` will return an array to the `onChange` callback even though `ChartSelectProps` declares a `number | null` value, causing incorrect state updates or downstream type/logic failures. Omit `multi` from the forwarded props or place the enforced value after the spread. [type error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Chart selection callbacks can receive arrays unexpectedly.
   - ⚠️ Dataset chart selection may store incompatible multi-select state.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Render the new `ChartSelectUsingAsync()` component from
   `superset-frontend/src/components/Select/ChartSelect.tsx:48` with a `multi` prop set to
   `true`; `multi` remains accepted because it is not omitted from `ChartSelectProps` at
   lines 28-32.
   
   2. The component renders `SelectAsyncControl` at line 93, first setting `multi={false}` at
   line 101.
   
   3. The subsequent `{...rest}` spread at line 102 overwrites that setting with
   `multi={true}`.
   
   4. SelectAsyncControl at
   `superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx:56`
   operates in multi-select mode and can invoke the callback with an array, while
   `ChartSelectProps.onChange` at line 35 only accepts `number | null`, creating an
   incompatible state value for any consumer that forwards `multi`.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3823493fd9754736964d9cf654e40aef&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=3823493fd9754736964d9cf654e40aef&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:** 101:102
   **Comment:**
   	*Type Error: The spread occurs after `multi={false}`, so callers can pass `multi` through `rest` and override the component's single-select configuration. In that case `SelectAsyncControl` will return an array to the `onChange` callback even though `ChartSelectProps` declares a `number | null` value, causing incorrect state updates or downstream type/logic failures. Omit `multi` from the forwarded props or place the enforced value after the spread.
   
   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=2ea08e11465df486d1baf6b74680753f37b4b2c4cd42af544db6a1625f0d0cb0&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=2ea08e11465df486d1baf6b74680753f37b4b2c4cd42af544db6a1625f0d0cb0&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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5941601200

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "syafiqmuda (via GitHub)" <gi...@apache.org>.

syafiqmuda commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-4242180241

   how to enable this function? I got issue with this drill-to-detail table where I can sort the table or resize the row and column width
   
   https://github.com/apache/superset/issues/32793#issuecomment-4242171581


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5085720858

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

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>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](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) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](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>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](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) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](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>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](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) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r3664364950


##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx:
##########
@@ -75,6 +75,7 @@ import Fieldset from '../Fieldset';
 import Field from '../Field';
 import { fetchSyncedColumns, updateColumns } from '../../utils';
 import DatasetUsageTab from './components/DatasetUsageTab';
+import ChartSelect from '../Select/ChartSelect';

Review Comment:
   **Suggestion:** The relative import points to `src/components/Datasource/components/Select/ChartSelect`, but `ChartSelect` is located at `src/components/Select/ChartSelect`. The datasource editor will fail to resolve this module and the frontend build/runtime will break. Update the import path to traverse out of the datasource component directories before entering `components/Select`. [import error]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ Frontend build fails resolving the datasource editor import.
   - ❌ Dataset editing cannot load with the invalid module path.
   - ⚠️ Drill-to-details chart configuration becomes unavailable.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Build or load the frontend containing
   `superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx`;
   the module resolver evaluates the import at line 78.
   
   2. Resolve `../Select/ChartSelect` relative to the editor directory
   `src/components/Datasource/components/DatasourceEditor`; this points to the nonexistent
   path `src/components/Datasource/components/Select/ChartSelect`.
   
   3. The actual implementation is `src/components/Select/ChartSelect.tsx`, confirmed by
   repository file discovery and its exported component at `ChartSelect.tsx:48`.
   
   4. Observe a module-resolution error during the frontend build or runtime loading of the
   datasource editor; the import should traverse three parent directories using
   `../../../Select/ChartSelect`.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=579584eee6634a938ce59bdd462a207b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=579584eee6634a938ce59bdd462a207b&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/components/DatasourceEditor/DatasourceEditor.jsx
   **Line:** 78:78
   **Comment:**
   	*Import Error: The relative import points to `src/components/Datasource/components/Select/ChartSelect`, but `ChartSelect` is located at `src/components/Select/ChartSelect`. The datasource editor will fail to resolve this module and the frontend build/runtime will break. Update the import path to traverse out of the datasource component directories before entering `components/Select`.
   
   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=5eed0d63853a769cbab5fb9c914f1e82e7330cfd835e3ec9e5bf4828ca2c1d50&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=5eed0d63853a769cbab5fb9c914f1e82e7330cfd835e3ec9e5bf4828ca2c1d50&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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r3656268115


##########
superset/datasets/schemas.py:
##########
@@ -174,6 +174,7 @@ class DatasetPutSchema(Schema):
     columns = fields.List(fields.Nested(DatasetColumnsPutSchema))
     metrics = fields.List(fields.Nested(DatasetMetricsPutSchema))
     folders = fields.List(fields.Nested(FolderSchema), required=False)
+    drill_through_chart_id = fields.Integer(allow_none=True)

Review Comment:
   **Suggestion:** The schema accepts any existing slice ID, but does not validate that the selected chart uses this dataset as its datasource. A chart from another dataset can therefore be configured successfully; when drill-through renders it, the source dataset's drill filters may not match the target chart's columns or datasource, producing incorrect results or a failed query. Validate the chart's datasource before persisting the relationship. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Drill-to-details can render results from an unrelated dataset.
   - ❌ Cross-dataset filters can cause target-chart query failures.
   - ⚠️ Dataset configuration permits invalid chart relationships.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Create dataset A and a chart backed by dataset B, then obtain the chart's slice ID
   through the existing chart configuration flow described by this PR.
   
   2. Submit a dataset update for dataset A through the dataset API path that deserializes
   requests with `DatasetPutSchema` in `superset/datasets/schemas.py:156`; set
   `drill_through_chart_id` to dataset B's chart ID.
   
   3. `DatasetPutSchema.drill_through_chart_id` at `superset/datasets/schemas.py:177` only
   checks that the value is an integer or null; it performs no slice lookup and no datasource
   comparison, so the cross-dataset ID passes schema validation.
   
   4. Open a dashboard containing a chart based on dataset A and invoke drill-to-details. The
   PR's configured-target flow renders the selected chart, causing filters originating from
   dataset A to be applied to a chart backed by dataset B; incompatible column filters can
   produce an incorrect result or a failed query.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=90e32f2001644564a5d6c0a72772c220&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=90e32f2001644564a5d6c0a72772c220&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:** 177:177
   **Comment:**
   	*Logic Error: The schema accepts any existing slice ID, but does not validate that the selected chart uses this dataset as its datasource. A chart from another dataset can therefore be configured successfully; when drill-through renders it, the source dataset's drill filters may not match the target chart's columns or datasource, producing incorrect results or a failed query. Validate the chart's datasource before persisting the relationship.
   
   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=e3a69d648010476f46a955bcfda682fcc1876326a485be2b6a0923a746e11741&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=e3a69d648010476f46a955bcfda682fcc1876326a485be2b6a0923a746e11741&reaction=dislike'>👎</a>



##########
superset-frontend/src/dashboard/hooks/useDashboardFormData.test.tsx:
##########
@@ -0,0 +1,171 @@
+/**
+ * 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 { renderHook } from '@testing-library/react-hooks';
+import { Provider } from 'react-redux';
+import configureMockStore from 'redux-mock-store';
+import { useDashboardFormData } from './useDashboardFormData';
+
+const mockStore = configureMockStore([]);
+
+const createMockState = (overrides = {}) => ({
+  dashboardInfo: {
+    id: 123,
+    metadata: {
+      chart_configuration: {},
+    },
+  },
+  dashboardState: {
+    sliceIds: [1, 2, 3],
+  },
+  nativeFilters: {
+    filters: {},
+  },
+  dataMask: {},
+  ...overrides,
+});
+
+const renderUseDashboardFormData = (
+  chartId: number | null | undefined,
+  state = {},
+) => {
+  const store = mockStore(createMockState(state));
+  return renderHook(() => useDashboardFormData(chartId), {
+    wrapper: ({ children }) => <Provider store={store}>{children}</Provider>,
+  });
+};
+
+test('returns base dashboard context when chartId is null', () => {
+  const { result } = renderUseDashboardFormData(null);
+
+  expect(result.current).toEqual({ dashboardId: 123 });
+});
+
+test('returns base dashboard context when chartId is undefined', () => {
+  const { result } = renderUseDashboardFormData(undefined);
+
+  expect(result.current).toEqual({ dashboardId: 123 });
+});
+
+test('returns base dashboard context when required state is missing', () => {
+  const { result } = renderUseDashboardFormData(1, {
+    nativeFilters: null,
+  });
+
+  expect(result.current).toEqual({ dashboardId: 123 });
+});
+
+test('returns base dashboard context when no filters apply to chart', () => {
+  const { result } = renderUseDashboardFormData(1, {
+    nativeFilters: {
+      filters: {
+        'filter-1': {
+          scope: [2, 3], // Doesn't include chartId 1
+        },
+      },
+    },
+    dashboardInfo: {
+      id: 123,
+      metadata: {
+        chart_configuration: {
+          'filter-1': {
+            id: 'filter-1',
+            scope: [2, 3],
+          },
+        },
+      },
+    },
+  });
+
+  expect(result.current).toEqual({ dashboardId: 123 });
+});
+
+test('returns dashboard context with extra form data when filters apply', () => {
+  const mockState = {
+    nativeFilters: {
+      filters: {
+        'filter-1': {
+          scope: [1, 2, 3], // Includes chartId 1
+        },
+      },
+    },
+    dashboardInfo: {
+      id: 123,
+      metadata: {
+        chart_configuration: {
+          'filter-1': {
+            id: 'filter-1',
+            scope: [1, 2, 3],
+          },
+        },
+      },
+    },
+    dataMask: {
+      'filter-1': {
+        extraFormData: {
+          filters: [
+            {
+              col: 'country',
+              op: 'IN',
+              val: ['USA', 'Canada'],
+            },
+          ],
+        },
+      },
+    },
+  };
+
+  // Mock the external utility functions
+  jest.mock('../util/activeAllDashboardFilters', () => ({
+    getAllActiveFilters: () => ({
+      'filter-1': {
+        scope: [1, 2, 3],
+      },
+    }),
+  }));
+
+  jest.mock('../components/nativeFilters/utils', () => ({
+    getExtraFormData: () => ({
+      filters: [
+        {
+          col: 'country',
+          op: 'IN',
+          val: ['USA', 'Canada'],
+        },
+      ],
+    }),
+  }));

Review Comment:
   **Suggestion:** These `jest.mock` calls execute inside the test after `useDashboardFormData` and its dependencies have already been imported and evaluated, so they do not reliably replace the functions used by the hook. As a result, this test does not actually isolate or verify the mocked filter behavior and may pass without testing the intended extra-form-data path. Move the mocks to module scope or use `jest.doMock` before dynamically importing the hook. [possible bug]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ The extra-form-data test does not reliably isolate utility behavior.
   - ⚠️ Regression tests may pass without exercising the intended mock path.
   - ⚠️ Future utility changes can make this test flaky or misleading.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Run the Jest test file
   `superset-frontend/src/dashboard/hooks/useDashboardFormData.test.tsx`; the hook is
   statically imported at line 22 before any test body executes.
   
   2. Jest evaluates `useDashboardFormData` and its imports, including
   `../util/activeAllDashboardFilters` and `../components/nativeFilters/utils`, before
   entering the test at line 98.
   
   3. During that test, the `jest.mock` calls at lines 134 and 142 execute only after those
   dependencies have already been imported and cached.
   
   4. The test renders `useDashboardFormData(1, mockState)` at line 154, so the hook can call
   the real imported utilities rather than the intended mock implementations; the assertion
   at lines 156-157 may therefore pass or fail based on production utility behavior instead
   of the explicitly configured mock return values.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=50a05a8bbfe54a71ae718e3c1189a877&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=50a05a8bbfe54a71ae718e3c1189a877&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/dashboard/hooks/useDashboardFormData.test.tsx
   **Line:** 133:152
   **Comment:**
   	*Possible Bug: These `jest.mock` calls execute inside the test after `useDashboardFormData` and its dependencies have already been imported and evaluated, so they do not reliably replace the functions used by the hook. As a result, this test does not actually isolate or verify the mocked filter behavior and may pass without testing the intended extra-form-data path. Move the mocks to module scope or use `jest.doMock` before dynamically importing the hook.
   
   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=b29db4e6c5a7425369b9301e22b10b3c44c744148c84ade70eb6c3d8302e58ad&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=b29db4e6c5a7425369b9301e22b10b3c44c744148c84ade70eb6c3d8302e58ad&reaction=dislike'>👎</a>



##########
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:** The table fetch is skipped solely because `dataset.drill_through_chart_id` is set, even when `drillThroughFormData` is still null or failed to load. In that state the configured chart cannot be rendered, but the fallback table also has no results, leaving the drill-detail pane blank or empty. Only skip the table fetch once the drill-through form data is available, or render an explicit loading/error state and restore the normal fallback when it is unavailable. [possible bug]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Drill details can show blank content during chart configuration loading.
   - ⚠️ Normal table fallback is disabled before chart data is ready.
   - ⚠️ Drill-through failures lack an explicit recovery state.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Configure a dataset with `drill_through_chart_id` and open a dashboard chart that uses
   this dataset, causing the drill-detail flow to render `DrillDetailPane` in
   `superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx:81`.
   
   2. During the drill-through chart configuration/load flow, render `DrillDetailPane` while
   its optional `drillThroughFormData` prop at
   `superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx:85-90` is still
   `null` or unavailable.
   
   3. The effect at
   `superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx:239-242` returns
   solely because `dataset?.drill_through_chart_id` is set, so `getDatasourceSamples()` is
   never called by the normal fallback path at lines 243-250.
   
   4. The render branch at
   `superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx:299-311` also
   cannot create the `StatefulChart` because it requires `drillThroughFormData`; the table
   therefore has no fetched results and the pane can remain blank until the chart data
   becomes available, with no explicit loading or error fallback.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c24046218ea145d8ab52a354f454abf1&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c24046218ea145d8ab52a354f454abf1&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:** 239:242
   **Comment:**
   	*Possible Bug: The table fetch is skipped solely because `dataset.drill_through_chart_id` is set, even when `drillThroughFormData` is still null or failed to load. In that state the configured chart cannot be rendered, but the fallback table also has no results, leaving the drill-detail pane blank or empty. Only skip the table fetch once the drill-through form data is available, or render an explicit loading/error state and restore the normal fallback when it is unavailable.
   
   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=0eba7efcceaf066e136c9e0a5e548ab9372ac5e5eda94f61f5a0d7692678aceb&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=0eba7efcceaf066e136c9e0a5e548ab9372ac5e5eda94f61f5a0d7692678aceb&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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r4051129106


##########
superset-frontend/src/dashboard/hooks/useDashboardFormData.ts:
##########
@@ -0,0 +1,108 @@
+/**
+ * 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 { useSelector } from 'react-redux';
+import { RootState, DashboardContextFormData } from '../types';
+import { getExtraFormData } from '../components/nativeFilters/utils';
+import { getAllActiveFilters } from '../util/activeAllDashboardFilters';
+import { getFilterIdsAppliedOnChart } from '../util/getFilterIdsAppliedOnChart';
+
+/**
+ * Hook that provides dashboard context as formatted formData for charts.
+ * This encapsulates all the complex logic for determining which dashboard
+ * filters, colors, and other context should be applied to a specific chart.
+ *
+ * @param chartId - The ID of the chart to get dashboard context for
+ * @returns Dashboard context formatted as QueryFormData fields
+ */
+export const useDashboardFormData = (
+  chartId: number | null | undefined,
+): DashboardContextFormData => {
+  // Dashboard state selectors
+  const dashboardId = useSelector<RootState, number>(
+    ({ dashboardInfo }) => dashboardInfo.id,
+  );
+
+  const nativeFilters = useSelector(
+    (state: RootState) => state.nativeFilters?.filters,
+  );
+
+  const dataMask = useSelector((state: RootState) => state.dataMask);
+
+  const chartConfiguration = useSelector(
+    (state: RootState) =>
+      state.dashboardInfo.metadata?.chart_configuration || {},
+  );
+
+  const allSliceIds = useSelector(
+    (state: RootState) => state.dashboardState.sliceIds,
+  );
+
+  // Compute dashboard context for the chart
+  return useMemo((): DashboardContextFormData => {
+    const baseContext: DashboardContextFormData = { dashboardId };
+
+    // Early return if we don't have required data or chartId
+    if (
+      chartId == null ||
+      !nativeFilters ||
+      !dataMask ||
+      !chartConfiguration ||
+      !allSliceIds
+    ) {
+      return baseContext;
+    }
+
+    // Get active filters using the same logic as normal dashboard charts
+    const activeFilters = getAllActiveFilters({
+      chartConfiguration,
+      nativeFilters,
+      dataMask,
+      allSliceIds,
+    });
+
+    // Find which filters apply to this specific chart
+    const filterIdsAppliedOnChart = getFilterIdsAppliedOnChart(
+      activeFilters,
+      chartId,
+    );
+
+    // If no filters apply, return just the base context
+    if (filterIdsAppliedOnChart.length === 0) {
+      return baseContext;

Review Comment:
   Yes. The context should be derived from the **source chart ID**, not the configured drill-through target ID.
   
   The source chart is the chart whose dashboard filters are active, while the target chart may not be present in the dashboard and therefore cannot be resolved by `getAllActiveFilters` against `allSliceIds`.
   
   Update `DrillDetailModal` to pass the source `chartId`:
   
   ```typescript
   const dashboardContextFormData = useDashboardFormData(chartId);
   ```
   
   The hook can retain its current behavior and treat its argument as the chart whose filter scope should be evaluated. The resulting `extra_form_data` can then be applied to the configured target chart through `drillThroughFormData`.
   
   This preserves filters scoped specifically to the source chart as well as dashboard-wide filters, while still rendering the selected target chart. The hook documentation and parameter naming should be updated to make this distinction explicit, and the tests should verify that the target chart ID is not required to be present in `allSliceIds`.



##########
superset/datasets/schemas.py:
##########
@@ -174,6 +174,7 @@ class DatasetPutSchema(Schema):
     columns = fields.List(fields.Nested(DatasetColumnsPutSchema))
     metrics = fields.List(fields.Nested(DatasetMetricsPutSchema))
     folders = fields.List(fields.Nested(FolderSchema), required=False)
+    drill_through_chart_id = fields.Integer(allow_none=True)

Review Comment:
   Yes. The update path should enforce the same invariant as the UI: `drill_through_chart_id` must be `NULL` or reference a chart whose datasource is the dataset being updated.
   
   The schema field alone only validates the integer type, so API/import callers can bypass the UI restriction. Validate during dataset update, ideally in the command/API validation layer where the dataset and `Slice` are available, and return a `422`/validation error for mismatches. For example:
   
   ```python
   if drill_through_chart_id is not None:
       chart = db.session.get(Slice, drill_through_chart_id)
       if chart is None or (
           chart.datasource_id != dataset.id
           or chart.datasource_type != "table"
       ):
           raise ValidationError(
               {"drill_through_chart_id": [
                   "The selected chart must use this dataset."
               ]}
           )
   ```
   
   This should also be applied to dataset imports or any other write path that can persist `drill_through_chart_id`. Keeping the check server-side prevents malformed API requests from configuring cross-dataset charts and avoids invalid drill filters or queries.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-6281394024

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.

mistercrunch commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3216539088

   🎪 @unknown Creating ephemeral environment for commit `8148a7c`
   
   **Action:** [View workflow](https://github.com/repo/actions/runs/run)
   **Environment:** `8148a7c`
   **Powered by:** [Superset Showtime](https://github.com/mistercrunch/superset-showtime)
   
   *Building and deploying... Watch the labels for progress updates.*


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3231744779

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/17285175663) for [e990879](https://github.com/apache/superset/commit/e990879)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

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>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](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) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](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>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](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) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](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>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](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) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5082248136

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5094534853

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r3664361032


##########
superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx:
##########
@@ -113,7 +117,7 @@ const SelectAsyncControl = ({
     if (!loaded) {
       loadOptions();
     }
-  }, [addDangerToast, dataEndpoint, mutator, value, loaded]);
+  }, [addDangerToast, dataEndpoint, mutator, value, loaded, searchParams]);

Review Comment:
   **Suggestion:** Adding `searchParams` to the effect dependencies does not reload options when it changes because `loaded` remains true after the first request. If a `ChartSelect` instance is reused with a different `datasetId`, the endpoint query changes but no request is made, leaving charts from the previous dataset displayed. Reset `loaded` and options when the endpoint parameters change, or track the loaded parameters instead of using a one-shot flag. [stale reference]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Chart selector can show charts from the wrong dataset.
   - ⚠️ Dataset drill-to-details configuration may save an invalid chart.
   - ⚠️ Dataset editor options become stale after dataset changes.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Render `ChartSelectUsingAsync()` from
   `superset-frontend/src/components/Select/ChartSelect.tsx:48` with a dataset ID; it derives
   an encoded `q` filter from `datasetId` at lines 56-80 and passes it as `searchParams` at
   line 96.
   
   2. On the initial render, `SelectAsyncControl` requests `/api/v1/chart/` with those
   parameters at
   `superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx:101-105`,
   stores the response, and sets `loaded` to true at lines 113-115.
   
   3. Reuse the same component instance with a different `datasetId`, causing `ChartSelect`
   to produce new `searchParams` and the effect at line 120 to rerun.
   
   4. The effect still sees `loaded === true`, so the `if (!loaded)` guard at lines 117-119
   skips the request; the options remain from the previous dataset even though the API query
   has changed. The production caller is the dataset editor at
   `superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx:1065-1069`,
   where the selector is rendered with `datasource.id`.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=10df5863cb7e4ce392763bde8f17cebd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=10df5863cb7e4ce392763bde8f17cebd&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/explore/components/controls/SelectAsyncControl/index.tsx
   **Line:** 120:120
   **Comment:**
   	*Stale Reference: Adding `searchParams` to the effect dependencies does not reload options when it changes because `loaded` remains true after the first request. If a `ChartSelect` instance is reused with a different `datasetId`, the endpoint query changes but no request is made, leaving charts from the previous dataset displayed. Reset `loaded` and options when the endpoint parameters change, or track the loaded parameters instead of using a one-shot flag.
   
   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=4efbc87351db1349b5a3a4ba8b849df32ef99bf80a1b5c552d60ce2c8015af3b&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=4efbc87351db1349b5a3a4ba8b849df32ef99bf80a1b5c552d60ce2c8015af3b&reaction=dislike'>👎</a>



##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -277,16 +275,13 @@ const ResultSet = ({
     const openInNewWindow = clickEvent.metaKey;
     logAction(LOG_ACTIONS_SQLLAB_CREATE_CHART, {});
     if (results?.query_id) {
-      const key = await postFormData(results.query_id, 'query', {
+      const url = await generateExploreUrl(results.query_id, 'query', {

Review Comment:
   **Suggestion:** The new asynchronous URL generation can reject when the form-data POST fails, but this click handler has no catch or user-facing error path. Clicking Create chart during a network or server failure therefore produces an unhandled promise rejection and no feedback to the user. Catch the error and show the same danger toast used for other explore URL generation failures. [possible bug]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ SQL Lab chart creation fails silently on network or server errors.
   - ⚠️ Users receive no actionable error feedback.
   - ⚠️ Failed URL generation creates unhandled promise rejections.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open SQL Lab and execute a query whose results include a query ID; `ResultSet` renders
   the Create chart action and binds it to `createExploreResultsOnClick()` at
   `superset-frontend/src/SqlLab/components/ResultSet/index.tsx:341`.
   
   2. Click Create chart, invoking `createExploreResultsOnClick()` at
   `superset-frontend/src/SqlLab/components/ResultSet/index.tsx:272`, which calls
   `generateExploreUrl()` at line 278.
   
   3. The URL helper posts form data through `postFormData()` in
   `superset-frontend/src/explore/exploreUtils/formData.ts:37`; a failed request rejects the
   returned promise.
   
   4. Because `createExploreResultsOnClick()` has no `try/catch` or rejection handler around
   line 278, the click produces an unhandled promise rejection and neither `history.push()`
   nor a danger toast executes. The analogous drill-through handler already catches this
   failure and calls `addDangerToast()` at
   `superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx:41-44`.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2cbcebf6b1d54559a77a3d6405a58688&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=2cbcebf6b1d54559a77a3d6405a58688&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/SqlLab/components/ResultSet/index.tsx
   **Line:** 278:278
   **Comment:**
   	*Possible Bug: The new asynchronous URL generation can reject when the form-data POST fails, but this click handler has no catch or user-facing error path. Clicking Create chart during a network or server failure therefore produces an unhandled promise rejection and no feedback to the user. Catch the error and show the same danger toast used for other explore URL generation failures.
   
   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=7bc1110b3a364c9b78ff33c72e6272b8c1063c5531d80f2fa5b3020d9437385a&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=7bc1110b3a364c9b78ff33c72e6272b8c1063c5531d80f2fa5b3020d9437385a&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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r3664363577


##########
superset-frontend/src/dashboard/hooks/useDashboardFormData.ts:
##########
@@ -0,0 +1,108 @@
+/**
+ * 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 { useSelector } from 'react-redux';
+import { RootState, DashboardContextFormData } from '../types';
+import { getExtraFormData } from '../components/nativeFilters/utils';
+import { getAllActiveFilters } from '../util/activeAllDashboardFilters';
+import { getFilterIdsAppliedOnChart } from '../util/getFilterIdsAppliedOnChart';
+
+/**
+ * Hook that provides dashboard context as formatted formData for charts.
+ * This encapsulates all the complex logic for determining which dashboard
+ * filters, colors, and other context should be applied to a specific chart.
+ *
+ * @param chartId - The ID of the chart to get dashboard context for
+ * @returns Dashboard context formatted as QueryFormData fields
+ */
+export const useDashboardFormData = (
+  chartId: number | null | undefined,
+): DashboardContextFormData => {
+  // Dashboard state selectors
+  const dashboardId = useSelector<RootState, number>(
+    ({ dashboardInfo }) => dashboardInfo.id,
+  );
+
+  const nativeFilters = useSelector(
+    (state: RootState) => state.nativeFilters?.filters,
+  );
+
+  const dataMask = useSelector((state: RootState) => state.dataMask);
+
+  const chartConfiguration = useSelector(
+    (state: RootState) =>
+      state.dashboardInfo.metadata?.chart_configuration || {},
+  );
+
+  const allSliceIds = useSelector(
+    (state: RootState) => state.dashboardState.sliceIds,
+  );
+
+  // Compute dashboard context for the chart
+  return useMemo((): DashboardContextFormData => {
+    const baseContext: DashboardContextFormData = { dashboardId };
+
+    // Early return if we don't have required data or chartId
+    if (
+      chartId == null ||
+      !nativeFilters ||
+      !dataMask ||
+      !chartConfiguration ||
+      !allSliceIds
+    ) {
+      return baseContext;
+    }
+
+    // Get active filters using the same logic as normal dashboard charts
+    const activeFilters = getAllActiveFilters({
+      chartConfiguration,
+      nativeFilters,
+      dataMask,
+      allSliceIds,
+    });
+
+    // Find which filters apply to this specific chart
+    const filterIdsAppliedOnChart = getFilterIdsAppliedOnChart(
+      activeFilters,
+      chartId,
+    );
+
+    // If no filters apply, return just the base context
+    if (filterIdsAppliedOnChart.length === 0) {
+      return baseContext;

Review Comment:
   **Suggestion:** The configured drill-through chart is often not included in the current dashboard's `allSliceIds`. `getAllActiveFilters` consequently scopes filters to dashboard chart IDs, and this lookup returns no matching filters for the drill-through chart, so the rendered target chart silently loses active dashboard filter context. Resolve the applicable filters from the source chart/dashboard context or explicitly include the configured target chart when constructing its scope. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Configured drill-through charts can ignore active dashboard filters.
   - ⚠️ Drill results may not match the source dashboard state.
   - ⚠️ StatefulChart receives incomplete dashboard form data.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Configure a dataset to use a chart that is not present on the current dashboard as its
   drill-through target; DrillDetailModal passes that configured target ID to
   useDashboardFormData at
   superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx:122-125.
   
   2. Open the chart context menu on a dashboard chart and choose drill-to-details;
   ChartContextMenu renders DrillDetailModal at
   superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx:447-457 and
   supplies the current dashboard context.
   
   3. With an active native filter whose scope contains the source dashboard chart but not
   the configured target chart, useDashboardFormData builds activeFilters through
   getAllActiveFilters at
   superset-frontend/src/dashboard/hooks/useDashboardFormData.ts:73-79. For dashboard-wide
   filters without an explicit target, getAllActiveFilters falls back to
   dashboardState.sliceIds at
   superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts:98-104, which excludes
   charts outside the dashboard.
   
   4. getFilterIdsAppliedOnChart checks the target ID against each filter scope at
   superset-frontend/src/dashboard/util/getFilterIdsAppliedOnChart.ts:31-37, returns an empty
   array at useDashboardFormData.ts:82-89, and drops extra_form_data. DrillDetailModal then
   renders the target StatefulChart with this incomplete form data at
   superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx:299-305, so the
   configured chart loses the active dashboard filter context.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=12d6be01daca416a8b959d6a82c3dd03&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=12d6be01daca416a8b959d6a82c3dd03&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/dashboard/hooks/useDashboardFormData.ts
   **Line:** 81:89
   **Comment:**
   	*Logic Error: The configured drill-through chart is often not included in the current dashboard's `allSliceIds`. `getAllActiveFilters` consequently scopes filters to dashboard chart IDs, and this lookup returns no matching filters for the drill-through chart, so the rendered target chart silently loses active dashboard filter context. Resolve the applicable filters from the source chart/dashboard context or explicitly include the configured target chart when constructing its scope.
   
   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=f908b4e75fd6d7dd1f9850e57dc61b4e7f429bccc63ae75aa276b690ed07d863&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=f908b4e75fd6d7dd1f9850e57dc61b4e7f429bccc63ae75aa276b690ed07d863&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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5953088773

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "msyavuz (via GitHub)" <gi...@apache.org>.

msyavuz commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r2322040100


##########
superset-frontend/package-lock.json:
##########


Review Comment:
   Is this change intentional?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3248414863

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Default Agent` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3231766550

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/17285175663) for [e990879](https://github.com/apache/superset/commit/e990879)
   
   • **Environment:** http://34.219.243.85:8080 (admin/admin)
   • **Lifetime:** 24h auto-cleanup
   • **Updates:** New commits create fresh environments automatically


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5110595898

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5104119817

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.

sadpandajoe commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r4051126653


##########
superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx:
##########
@@ -33,37 +33,46 @@ import { isEmbedded } from 'src/dashboard/util/isEmbedded';
 import { Slice } from 'src/types/Chart';
 import { RootState } from 'src/dashboard/types';
 import { findPermission } from 'src/utils/findPermission';
+import { useToasts } from 'src/components/MessageToasts/withToasts';
+import { getFormDataWithDashboardContext } from 'src/explore/controlUtils/getFormDataWithDashboardContext';
+import { useDashboardFormData } from 'src/dashboard/hooks/useDashboardFormData';
+import { generateExploreUrl } from 'src/explore/exploreUtils/formData';
 import { Dataset } from '../types';
 import DrillDetailPane from './DrillDetailPane';
 
 interface ModalFooterProps {
   canExplore: boolean;
   closeModal?: () => void;
-  exploreChart: () => void;
+  showEditButton: boolean;
+  onExploreClick?: (event: React.MouseEvent) => void;
+  isGeneratingUrl: boolean;
 }
 
 const ModalFooter = ({
   canExplore,
   closeModal,
-  exploreChart,
+  showEditButton,
+  onExploreClick,
+  isGeneratingUrl,
 }: ModalFooterProps) => {
   const theme = useTheme();
 
   return (
     <>
-      {!isEmbedded() && (
+      {!isEmbedded() && showEditButton && (

Review Comment:
   This hides the existing Explore/Edit action for every dataset without the new configuration, so the claimed no-op fallback removes the only shortcut back to the source chart after upgrade. Should the unconfigured path keep the previous source-chart action?



##########
superset/datasets/schemas.py:
##########
@@ -174,6 +174,7 @@ class DatasetPutSchema(Schema):
     columns = fields.List(fields.Nested(DatasetColumnsPutSchema))
     metrics = fields.List(fields.Nested(DatasetMetricsPutSchema))
     folders = fields.List(fields.Nested(FolderSchema), required=False)
+    drill_through_chart_id = fields.Integer(allow_none=True)

Review Comment:
   The runtime dataset API now accepts and returns this field, but the checked-in OpenAPI document has no `drill_through_chart_id` property. Generated clients therefore cannot configure the feature and may reject the response; could the API specification be regenerated with this field?



##########
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' | 'multi'
+  > {
+  // 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({

Review Comment:
   This request gets only Flask-AppBuilder’s default 20-row page, and `SelectAsyncControl` neither paginates nor sends a server-side search term. A valid chart that sorts after the first page can never be selected; can this use the paginated async selector instead?



##########
superset/datasets/schemas.py:
##########
@@ -174,6 +174,7 @@ class DatasetPutSchema(Schema):
     columns = fields.List(fields.Nested(DatasetColumnsPutSchema))
     metrics = fields.List(fields.Nested(DatasetMetricsPutSchema))
     folders = fields.List(fields.Nested(FolderSchema), required=False)
+    drill_through_chart_id = fields.Integer(allow_none=True)

Review Comment:
   Agreed—the UI restricts this relationship to charts from the dataset, but the PUT contract accepts any slice ID; the renderer then keeps that chart’s controls while replacing its datasource, so incompatible columns or metrics fail the drill query. Should the update command verify that the target chart uses this dataset?



##########
superset-frontend/src/dashboard/hooks/useDashboardFormData.ts:
##########
@@ -0,0 +1,108 @@
+/**
+ * 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 { useSelector } from 'react-redux';
+import { RootState, DashboardContextFormData } from '../types';
+import { getExtraFormData } from '../components/nativeFilters/utils';
+import { getAllActiveFilters } from '../util/activeAllDashboardFilters';
+import { getFilterIdsAppliedOnChart } from '../util/getFilterIdsAppliedOnChart';
+
+/**
+ * Hook that provides dashboard context as formatted formData for charts.
+ * This encapsulates all the complex logic for determining which dashboard
+ * filters, colors, and other context should be applied to a specific chart.
+ *
+ * @param chartId - The ID of the chart to get dashboard context for
+ * @returns Dashboard context formatted as QueryFormData fields
+ */
+export const useDashboardFormData = (
+  chartId: number | null | undefined,
+): DashboardContextFormData => {
+  // Dashboard state selectors
+  const dashboardId = useSelector<RootState, number>(
+    ({ dashboardInfo }) => dashboardInfo.id,
+  );
+
+  const nativeFilters = useSelector(
+    (state: RootState) => state.nativeFilters?.filters,
+  );
+
+  const dataMask = useSelector((state: RootState) => state.dataMask);
+
+  const chartConfiguration = useSelector(
+    (state: RootState) =>
+      state.dashboardInfo.metadata?.chart_configuration || {},
+  );
+
+  const allSliceIds = useSelector(
+    (state: RootState) => state.dashboardState.sliceIds,
+  );
+
+  // Compute dashboard context for the chart
+  return useMemo((): DashboardContextFormData => {
+    const baseContext: DashboardContextFormData = { dashboardId };
+
+    // Early return if we don't have required data or chartId
+    if (
+      chartId == null ||
+      !nativeFilters ||
+      !dataMask ||
+      !chartConfiguration ||
+      !allSliceIds
+    ) {
+      return baseContext;
+    }
+
+    // Get active filters using the same logic as normal dashboard charts
+    const activeFilters = getAllActiveFilters({
+      chartConfiguration,
+      nativeFilters,
+      dataMask,
+      allSliceIds,
+    });
+
+    // Find which filters apply to this specific chart
+    const filterIdsAppliedOnChart = getFilterIdsAppliedOnChart(
+      activeFilters,
+      chartId,
+    );
+
+    // If no filters apply, return just the base context
+    if (filterIdsAppliedOnChart.length === 0) {
+      return baseContext;

Review Comment:
   Agreed—the target chart is normally off-dashboard, so this lookup finds no native-filter scope and the drill result can ignore filters applied to the source chart. Should the context be derived from the source chart ID instead?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5259065419

   Conflicts are back after last month's rebase, and CI never got past queued on the last push here — cypress, playwright, python-unit, sonarqube, codecov all stuck, only docs-preview and labeler ran. Anyone seen that on other branches, or is this one specifically stuck?


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5289383384

   LMK if you want help getting this one patched up and reviewed.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3222361174

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/17226072189) for [5ffb332](https://github.com/apache/superset/commit/5ffb332)
   
   • **Environment:** http://35.87.32.89:8080 (admin/admin)
   • **Lifetime:** 24h auto-cleanup
   • **Updates:** New commits create fresh environments automatically


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3250374597

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/17443098146) for [f0bb7b6](https://github.com/apache/superset/commit/f0bb7b6)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3252047660

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/17454628800) for [775063a](https://github.com/apache/superset/commit/775063a)
   
   • **Environment:** http://52.37.181.94:8080 (admin/admin)
   • **Lifetime:** 24h auto-cleanup
   • **Updates:** New commits create fresh environments automatically


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5040138904

   Rebase is still needed here (mergeable shows conflicting) and CI's red across the board (cypress, playwright, lint-frontend). Also worth resolving before another look: bito's flag that `getExploreUrl` needs `formData.datasource`, the duplicate `drill_through_chart_id` in `datasets/schemas.py`'s select columns, and the GHAS DOM-text-as-HTML finding on `DrillDetailModal.tsx`.
   
   Pushing some commits here to see if I can nudge it forward..


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5087020325

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-5704340421

   # 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️ 🙅‍♂️❗ @mistercrunch Your base branch `master` has also updated `superset/migrations`.
   
   **Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-6307975592

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-6455867482

### ⚠️ Possible DB migration conflict

To check and, if needed, fix this:

See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.

--
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3211874357

   @mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/17137384259). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3211903953

   @mistercrunch Ephemeral environment spinning up at http://44.243.96.179:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r2318364264


##########
superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx:
##########
@@ -109,10 +110,19 @@ 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 exploreUrl = useMemo(() => {
+    // Use drill-through chart ID if configured, otherwise use original chart
+    const targetChartId = dataset?.drill_through_chart_id || chartId;
+    return getExploreUrl({
+      formData: {
+        slice_id: targetChartId,

Review Comment:
   
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Missing required datasource property</b></div>
   <div id="fix">
   
   The `getExploreUrl` function requires `formData.datasource` to be present, but the current implementation only provides `formData.slice_id`. According to the `getExploreUrl` function (line 139-141), it returns `null` when `formData.datasource` is missing, which will break the explore functionality. The `formData` should include the `datasource` property from the existing `formData` prop: `formData: { ...formData, slice_id: targetChartId }`
   </div>
   <details>
   <summary>
   <b>Code suggestion</b>
   </summary>
   <blockquote>Check the AI-generated fix before applying</blockquote>
   <div id="code">
   
   
   ```suggestion
           ...formData,
           slice_id: targetChartId,
   ```
   
   </div>
   </details>
   </div>
   
   
   
   <small><i>Code Review Run <a href=https://github.com/apache/superset/pull/34785#issuecomment-3248414863>#8bf56d</a></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/src/dashboard/hooks/useDashboardFormData.ts:
##########
@@ -0,0 +1,106 @@
+/**
+ * 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 { useSelector } from 'react-redux';
+import { RootState, DashboardContextFormData } from '../types';
+import { getExtraFormData } from '../components/nativeFilters/utils';
+import { getAllActiveFilters } from '../util/activeAllDashboardFilters';
+
+/**
+ * Hook that provides dashboard context as formatted formData for charts.
+ * This encapsulates all the complex logic for determining which dashboard
+ * filters, colors, and other context should be applied to a specific chart.
+ *
+ * @param chartId - The ID of the chart to get dashboard context for
+ * @returns Dashboard context formatted as QueryFormData fields
+ */
+export const useDashboardFormData = (
+  chartId: number | null | undefined,
+): DashboardContextFormData => {
+  // Dashboard state selectors
+  const dashboardId = useSelector<RootState, number>(
+    ({ dashboardInfo }) => dashboardInfo.id,
+  );
+
+  const nativeFilters = useSelector(
+    (state: RootState) => state.nativeFilters?.filters,
+  );
+
+  const dataMask = useSelector((state: RootState) => state.dataMask);
+
+  const chartConfiguration = useSelector(
+    (state: RootState) =>
+      state.dashboardInfo.metadata?.chart_configuration || {},
+  );
+
+  const allSliceIds = useSelector(
+    (state: RootState) => state.dashboardState.sliceIds,
+  );
+
+  // Compute dashboard context for the chart
+  return useMemo((): DashboardContextFormData => {
+    const baseContext: DashboardContextFormData = { dashboardId };
+
+    // Early return if we don't have required data or chartId
+    if (
+      !chartId ||

Review Comment:
   
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Invalid null check excludes chart ID 0</b></div>
   <div id="fix">
   
   The null check `!chartId` will incorrectly exclude chart ID 0, which is a valid chart ID. This will cause the hook to return incomplete dashboard context for chart ID 0, breaking filter functionality for that chart. Use explicit null/undefined checks instead.
   </div>
   <details>
   <summary>
   <b>Code suggestion</b>
   </summary>
   <blockquote>Check the AI-generated fix before applying</blockquote>
   <div id="code">
   
   
   ```suggestion
         chartId == null ||
   ```
   
   </div>
   </details>
   </div>
   
   
   
   <small><i>Code Review Run <a href=https://github.com/apache/superset/pull/34785#issuecomment-3248414863>#8bf56d</a></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/datasets/schemas.py:
##########
@@ -411,6 +413,7 @@ class DatasetDrillInfoSchema(Schema):
     created_on_humanized = fields.String()
     changed_by = fields.Nested(UserSchema)
     changed_on_humanized = fields.String()
+    drill_through_chart_id = fields.Integer(allow_none=True)

Review Comment:
   
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Missing field in API select columns</b></div>
   <div id="fix">
   
   Adding `drill_through_chart_id` field to the schema without ensuring it's included in the API's select columns will cause serialization issues. The field won't be loaded from the database, potentially causing AttributeError or missing data. Please add `"drill_through_chart_id"` to the `drill_info_select_columns` list in the datasets API drill method.
   </div>
   <details>
   <summary>
   <b>Code suggestion</b>
   </summary>
   <blockquote>Check the AI-generated fix before applying</blockquote>
   <div id="code">
   
   
   ```
    @@ -1245,6 +1245,7 @@
     "columns.column_name",
     "columns.verbose_name",
     "columns.groupby",
    +"drill_through_chart_id",
     ]
   ```
   
   </div>
   </details>
   </div>
   
   
   
   <small><i>Code Review Run <a href=https://github.com/apache/superset/pull/34785#issuecomment-3248414863>#8bf56d</a></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/src/components/Chart/DrillDetail/DrillDetailModal.test.tsx:
##########
@@ -32,6 +32,14 @@ jest.mock('react-router-dom', () => ({
   }),
 }));
 
+jest.mock('src/explore/exploreUtils', () => ({
+  ...jest.requireActual('src/explore/exploreUtils'),
+  getExploreUrl: jest.fn(
+    ({ formData }) =>
+      `/explore/?dashboard_page_id=&slice_id=${formData.slice_id}`,

Review Comment:
   
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Mock function parameter signature mismatch</b></div>
   <div id="fix">
   
   The mocked `getExploreUrl` function parameter destructuring is incorrect. The real function expects an object with `formData` as a property, but the mock destructures expecting `formData` as the direct parameter. This will cause `formData.slice_id` to be undefined, breaking the test assertion that expects `/explore/?dashboard_page_id=&slice_id=${sliceId}`.
   </div>
   <details>
   <summary>
   <b>Code suggestion</b>
   </summary>
   <blockquote>Check the AI-generated fix before applying</blockquote>
   <div id="code">
   
   
   ```suggestion
     getExploreUrl: jest.fn(
       (params) =>
         `/explore/?dashboard_page_id=&slice_id=${params.formData.slice_id}`,
   ```
   
   </div>
   </details>
   </div>
   
   
   
   <small><i>Code Review Run <a href=https://github.com/apache/superset/pull/34785#issuecomment-3248414863>#8bf56d</a></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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3250418635

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/17443098146) for [f0bb7b6](https://github.com/apache/superset/commit/f0bb7b6)
   
   • **Environment:** http://54.213.17.221:8080 (admin/admin)
   • **Lifetime:** 24h auto-cleanup
   • **Updates:** New commits create fresh environments automatically


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3252011998

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/17454628800) for [775063a](https://github.com/apache/superset/commit/775063a)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.

github-advanced-security[bot] commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r2320872616


##########
superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx:
##########
@@ -109,14 +119,74 @@
     findPermission('can_explore', 'Superset', state.user?.roles),
   );
 
-  const exploreUrl = useMemo(
-    () => `/explore/?dashboard_page_id=${dashboardPageId}&slice_id=${chartId}`,
-    [chartId, dashboardPageId],
+  // Determine if we should show the Explore button
+  const showEditButton = Boolean(dataset?.drill_through_chart_id);
+
+  // Get dashboard context for the drill-through chart using the proper hook
+  const dashboardContextFormData = useDashboardFormData(
+    dataset?.drill_through_chart_id,
   );
 
-  const exploreChart = useCallback(() => {
-    history.push(exploreUrl);
-  }, [exploreUrl, history]);
+  // Generate formData for drill-through chart using proper dashboard context resolution
+  const drillThroughFormData = useMemo(() => {
+    if (!dataset?.drill_through_chart_id || !dataset?.id) {
+      return null;
+    }
+
+    // Create base formData for the drill-through chart
+    const drillThroughBaseFormData = {
+      slice_id: dataset.drill_through_chart_id,
+      datasource: `${dataset.id}__table`,
+      viz_type: 'table', // Default viz type
+    };
+
+    // Use the enhanced dashboard context function that handles drill-to-detail filters
+    return getFormDataWithDashboardContext(
+      drillThroughBaseFormData,
+      dashboardContextFormData,
+      undefined, // saveAction
+      initialFilters, // drillToDetailFilters - drill-down filters from context menu
+    );
+  }, [
+    dataset?.drill_through_chart_id,
+    dataset?.id,
+    dashboardContextFormData,
+    initialFilters,
+  ]);
+
+  // Handle explore button click - generate URL and navigate
+  const handleExploreClick = async (event: React.MouseEvent) => {
+    event.preventDefault();
+
+    if (
+      !dataset?.drill_through_chart_id ||
+      !drillThroughFormData ||
+      !dataset?.id
+    ) {
+      return;
+    }
+
+    setIsGeneratingUrl(true);
+
+    try {
+      const url = await generateExploreUrl(
+        dataset.id, // datasourceId
+        'table', // datasourceType
+        drillThroughFormData, // our perfectly crafted formData with dashboard context + drill-down filters
+        {
+          chartId: dataset.drill_through_chart_id,
+          dashboardPageId,
+        },
+      );
+
+      // Navigate to the explore page
+      window.location.href = url;

Review Comment:
   ## DOM text reinterpreted as HTML
   
   [DOM text](1) is reinterpreted as HTML without escaping meta-characters.
   
   [Show more details](https://github.com/apache/superset/security/code-scanning/2043)



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3252415336

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Default Agent` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3222341434

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/17226072189) for [5ffb332](https://github.com/apache/superset/commit/5ffb332)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3222651649

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/17228457413) for [48b2a46](https://github.com/apache/superset/commit/48b2a46)
   
   • **Environment:** http://52.39.117.218:8080 (admin/admin)
   • **Lifetime:** 24h auto-cleanup
   • **Updates:** New commits create fresh environments automatically


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "EMachad0 (via GitHub)" <gi...@apache.org>.

EMachad0 commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r2337339626


##########
superset/datasets/api.py:
##########


Review Comment:
   I just realized the drill trough chart id is on the dataset level
   
   I understand this does not addresses one problem described on SIP-160,
   > As a superset user, we use a single dataset for multiple charts, currently the entire dataset is displayed in the drilldown, we would like the ability to filter and display only the columns that are most relevant to each chart.
   
   I really like the idea of being able to use any chart on the drilldown, do you see putting the drill through chart id on chart level as an option?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "Gunnnn (via GitHub)" <gi...@apache.org>.

Gunnnn commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-3579850774

   What can i do to make this PR happen? Please help me to begin, i need it dreadfully for my work!


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat: allow drill-to-details to a specific chart (optionaly) configured at the dataset level [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on PR #34785:
URL: https://github.com/apache/superset/pull/34785#issuecomment-4765066629

   @mistercrunch there's real demand for this one (a few folks pinging the thread) but it's conflicting and cypress is red. I'll rebase it and see how it does...


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org