You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "marunrun (via GitHub)" <gi...@apache.org> on 2025/09/19 08:35:50 UTC
[PR] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
marunrun opened a new pull request, #35208:
URL: https://github.com/apache/superset/pull/35208
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
Backend search and backend sorting have been added in the new version,But the view Query and export are not applied correctly.
I only modified the View Query, and I'm not sure if the export needs to be modified.
- Modified ViewQueryModal to accept and use ownState parameter
- Updated useExploreAdditionalActionsMenu to pass ownState to ViewQueryModal
- Fixed query parameter mismatch between View Query display and actual execution
- Ensures View Query shows correct query statements matching TableChart search behavior
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
Chart Explore
https://github.com/user-attachments/assets/81cd0b2d-82bd-45bb-a62f-9e4a61ddddc9
https://github.com/user-attachments/assets/4b680759-f3b4-4f4c-bfdf-ae8f8070da16
----
Dashboard
https://github.com/user-attachments/assets/8021039e-84a2-47d1-9668-57b1781a9f59
https://github.com/user-attachments/assets/9dff6c34-fcba-454d-bb9d-96d0ed87de0e
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #35208:
URL: https://github.com/apache/superset/pull/35208#issuecomment-4796444161
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 6fc5a388c69f2e015099a8047e0032fede3a361f |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a3cc8294a010e0008ef5b15 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-35208--superset-docs-preview.netlify.app](https://deploy-preview-35208--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcrJDYAgEADAXvYtcT0wageWseASSVAR8Yqxd_nNY17wYZ19hB4gAxVo0VOysXceSTkWO1PQkzgtX2I7ODypjezd-gxjig1VWrdlVxMWyIjYspGqkPD9) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-35208--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "marunrun (via GitHub)" <gi...@apache.org>.
marunrun commented on PR #35208:
URL: https://github.com/apache/superset/pull/35208#issuecomment-4601186839
> Thanks @marunrun! This has developed merge conflicts with master. Could you rebase on the latest master so CI can run cleanly, then we'll take a review pass?
I've already rebased it.
--
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on code in PR #35208:
URL: https://github.com/apache/superset/pull/35208#discussion_r2362187750
##########
superset-frontend/src/dashboard/components/SliceHeader/index.tsx:
##########
@@ -57,6 +57,7 @@ type SliceHeaderProps = SliceHeaderControlsProps & {
filters: object;
handleToggleFullSize: () => void;
formData: object;
+ ownState?: object;
Review Comment:
### Generic object typing for ownState parameter <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The ownState parameter is typed as a generic object which provides no type safety or IntelliSense support.
###### Why this matters
Using a generic object type defeats the purpose of TypeScript's type system, making the code prone to runtime errors and reducing developer productivity through lack of autocomplete and compile-time validation.
###### Suggested change ∙ *Feature Preview*
Define a proper interface or use a more specific type for ownState. For example:
```typescript
interface OwnState {
// Define the expected properties based on the actual structure
[key: string]: any; // or more specific types
}
ownState?: OwnState;
```
Or if the structure is known, define specific properties.
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b9f7fd1a-710c-4f9a-a0d2-2caebdcb3af6/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b9f7fd1a-710c-4f9a-a0d2-2caebdcb3af6?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b9f7fd1a-710c-4f9a-a0d2-2caebdcb3af6?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b9f7fd1a-710c-4f9a-a0d2-2caebdcb3af6?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b9f7fd1a-710c-4f9a-a0d2-2caebdcb3af6)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:4f79a341-b341-4461-91c7-d31425eab5e2 -->
[](4f79a341-b341-4461-91c7-d31425eab5e2)
--
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #35208:
URL: https://github.com/apache/superset/pull/35208#issuecomment-4599685668
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/35208?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 63.97%. Comparing base ([`2abbb64`](https://app.codecov.io/gh/apache/superset/commit/2abbb64e6bcae04e9748893e0a11ff574a3680b2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`3117a04`](https://app.codecov.io/gh/apache/superset/commit/3117a047388742d85baf75a7d349a3d8456db5e4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #35208 +/- ##
=======================================
Coverage 63.97% 63.97%
=======================================
Files 2658 2658
Lines 143094 143098 +4
Branches 32906 32910 +4
=======================================
+ Hits 91538 91541 +3
- Misses 49991 49992 +1
Partials 1565 1565
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/35208/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/35208/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.41% <100.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/35208?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #35208:
URL: https://github.com/apache/superset/pull/35208#issuecomment-4797736299
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #35208:
URL: https://github.com/apache/superset/pull/35208#issuecomment-4599032443
Thanks @marunrun! This has developed merge conflicts with master. Could you rebase on the latest master so CI can run cleanly, then we'll take a review pass?
--
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "marunrun (via GitHub)" <gi...@apache.org>.
marunrun commented on code in PR #35208:
URL: https://github.com/apache/superset/pull/35208#discussion_r3472613714
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx:
##########
@@ -1018,6 +1018,7 @@ export const useExploreAdditionalActionsMenu = (
modalBody={
<ViewQueryModal
latestQueryFormData={latestQueryFormData as QueryFormData}
+ ownState={ownState}
Review Comment:
Good catch — fixed in c8da7e6e81. `ViewQueryModal` now strips `clientView` with `omit(ownState, ['clientView'])` before calling `getChartDataRequest`, matching the existing chart query path in `ExploreViewContainer` (`omit(dataMask[slice_id]?.ownState, ['clientView'])`) and `Dashboard`'s `activeAllDashboardFilters`. Added a test asserting `clientView` is removed while `searchText`/`order_by` are preserved.
--
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #35208:
URL: https://github.com/apache/superset/pull/35208#issuecomment-4732502241
The `ownState` threading looks right, but `pre-commit` is failing — prettier wants to reformat the new `useCallback` in `ViewQueryModal.tsx` (the function arg and dep array need wrapping). Run `pre-commit run --all-files` (or `npx prettier --write` on that file) and push, then CI should go green. While you're in there, mind adding a quick `ViewQueryModal.test.tsx` case asserting `ownState` is passed to `getChartDataRequest`?
--
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "marunrun (via GitHub)" <gi...@apache.org>.
marunrun commented on code in PR #35208:
URL: https://github.com/apache/superset/pull/35208#discussion_r3472613714
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx:
##########
@@ -1018,6 +1018,7 @@ export const useExploreAdditionalActionsMenu = (
modalBody={
<ViewQueryModal
latestQueryFormData={latestQueryFormData as QueryFormData}
+ ownState={ownState}
Review Comment:
Good catch — fixed in c8da7e6e81. `ViewQueryModal` now strips `clientView` with `omit(ownState, ['clientView'])` before calling `getChartDataRequest`, matching the existing chart query path in `ExploreViewContainer` (`omit(dataMask[slice_id]?.ownState, ['clientView'])`) and `Dashboard`'s `activeAllDashboardFilters`. Added a test asserting `clientView` is removed while `searchText`/`order_by` are preserved.
--
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #35208:
URL: https://github.com/apache/superset/pull/35208#issuecomment-4746878953
Still red on `pre-commit` here, @marunrun - the `useCallback` in `ViewQueryModal.tsx` needs the prettier reformat I mentioned. `npx prettier --write` on that file and push should clear it. Any luck adding a `ViewQueryModal.test.tsx` case asserting `ownState` reaches `getChartDataRequest`?
--
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #35208:
URL: https://github.com/apache/superset/pull/35208#issuecomment-4601104855
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #35208:
URL: https://github.com/apache/superset/pull/35208
--
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [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 #35208:
URL: https://github.com/apache/superset/pull/35208#discussion_r3472310832
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx:
##########
@@ -1018,6 +1018,7 @@ export const useExploreAdditionalActionsMenu = (
modalBody={
<ViewQueryModal
latestQueryFormData={latestQueryFormData as QueryFormData}
+ ownState={ownState}
Review Comment:
**Suggestion:** Passing raw `ownState` into `ViewQueryModal` forwards `clientView` (rows/columns) into `getChartDataRequest`, but query paths elsewhere explicitly strip `clientView` before backend requests. This can bloat the payload and cause View Query failures/timeouts (or 413) on large tables. Pass a sanitized `ownState` that omits `clientView` for View Query, keeping only server-relevant state like search/sort keys. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Explore "View query" may fail on large table charts.
- ⚠️ Oversized payloads increase latency for View Query calls.
- ⚠️ Inconsistent behavior vs. other paths that strip clientView.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a Table chart using the table plugin so that it renders many rows
client-side; `plugins/plugin-chart-table/src/TableChart.tsx` around lines 1563–1594 shows
`clientViewRows` being tracked and written into `ownState.clientView` (with `rows`,
`columns`, and `count`) via `updateTableOwnState`.
2. Open that chart in Explore so that the Explore view uses the same plugin and data mask
wiring; the TableChart component will push a large `clientView` snapshot (all filtered
rows and visible columns) into the chart's `ownState`.
3. In the Explore header, open the actions menu and click "View query"; the menu item is
wired via `src/explore/components/useExploreAdditionalActionsMenu/index.tsx` lines 30–43,
which render a `ModalTrigger` whose `modalBody` mounts `<ViewQueryModal
latestQueryFormData={latestQueryFormData as QueryFormData} ownState={ownState} />`.
4. When the `ViewQueryModal` mounts, its `useEffect` calls `loadChartData('query')`, which
in turn calls `getChartDataRequest` with `ownState` unchanged
(`src/explore/components/controls/ViewQueryModal.tsx` lines 53–65). Because `ownState` now
includes the large `clientView` snapshot, the POST body sent by `getChartDataRequest`
(defined in `src/components/Chart/chartAction.ts` lines 504–513) can become very large and
include thousands of rows and columns. On sufficiently large tables this inflated payload
can cause the View Query request to be slow or rejected by the server (e.g., 413 Payload
Too Large), breaking the View Query dialog specifically for table charts with extensive
client-side filtering.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=231c650de0204c36a9eace7618134da3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=231c650de0204c36a9eace7618134da3&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/useExploreAdditionalActionsMenu/index.tsx
**Line:** 1021:1021
**Comment:**
*Api Mismatch: Passing raw `ownState` into `ViewQueryModal` forwards `clientView` (rows/columns) into `getChartDataRequest`, but query paths elsewhere explicitly strip `clientView` before backend requests. This can bloat the payload and cause View Query failures/timeouts (or 413) on large tables. Pass a sanitized `ownState` that omits `clientView` for View Query, keeping only server-relevant state like search/sort keys.
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%2F35208&comment_hash=332bfecb5927a28a5bbfa5473ecb79ed3c325970d33baa42a641fadf1072b22a&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F35208&comment_hash=332bfecb5927a28a5bbfa5473ecb79ed3c325970d33baa42a641fadf1072b22a&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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "marunrun (via GitHub)" <gi...@apache.org>.
marunrun commented on code in PR #35208:
URL: https://github.com/apache/superset/pull/35208#discussion_r3472619059
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -48,38 +50,42 @@ const ViewQueryModalContainer = styled.div`
gap: ${({ theme }) => theme.sizeUnit * 4}px;
`;
-const ViewQueryModal: FC<Props> = ({ latestQueryFormData }) => {
+const ViewQueryModal: FC<Props> = ({ latestQueryFormData, ownState }) => {
const [result, setResult] = useState<Result[]>([]);
const [isLoading, setIsLoading] = useState(false);
const [error, setError] = useState<string | null>(null);
- const loadChartData = (resultType: string) => {
- setIsLoading(true);
- getChartDataRequest({
- formData: latestQueryFormData,
- resultFormat: 'json',
- resultType,
- })
- .then(({ json }) => {
- setResult(ensureIsArray(json.result) as Result[]);
- setIsLoading(false);
- setError(null);
+ const loadChartData = useCallback(
+ (resultType: string) => {
+ setIsLoading(true);
+ getChartDataRequest({
+ formData: latestQueryFormData,
+ resultFormat: 'json',
+ resultType,
+ ownState: ownState || {},
})
- .catch(response => {
- getClientErrorObject(response).then(({ error, message }) => {
- setError(
- error ||
- message ||
- response.statusText ||
- t('Sorry, An error occurred'),
- );
+ .then(({ json }) => {
+ setResult(ensureIsArray(json.result) as Result[]);
setIsLoading(false);
+ setError(null);
Review Comment:
Agreed this is a real concern in principle, but I'm intentionally leaving it out of this PR. The lack of request cancellation/sequencing is a pre-existing property of `getChartDataRequest` → `SupersetClient.post` (no abort support) and of `ViewQueryModal` before this PR — it isn't introduced by these changes. Adding proper AbortController or sequence-guard handling is a broader, standalone improvement that should cover all `getChartDataRequest` call sites, not just this modal. I'll file it as a separate issue so it gets the right scope rather than being bolted onto a query-consistency bug fix.
--
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "marunrun (via GitHub)" <gi...@apache.org>.
marunrun commented on code in PR #35208:
URL: https://github.com/apache/superset/pull/35208#discussion_r3472615840
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx:
##########
@@ -1018,6 +1018,7 @@ export const useExploreAdditionalActionsMenu = (
modalBody={
<ViewQueryModal
latestQueryFormData={latestQueryFormData as QueryFormData}
+ ownState={ownState}
Review Comment:
Good catch — fixed in c8da7e6e81. `ViewQueryModal` now strips `clientView` with `omit(ownState, ['clientView'])` before calling `getChartDataRequest`, matching the existing chart query path in `ExploreViewContainer` (`omit(dataMask[slice_id]?.ownState, ['clientView'])`) and `Dashboard`'s `activeAllDashboardFilters`. Added a test asserting `clientView` is removed while `searchText`/`order_by` are preserved.
--
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #35208:
URL: https://github.com/apache/superset/pull/35208#discussion_r2535869005
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:
##########
@@ -140,6 +140,8 @@ export interface SliceHeaderControlsProps {
supersetCanCSV?: boolean;
crossFiltersEnabled?: boolean;
+
+ ownState?: object;
Review Comment:
The type `object` is too generic for TypeScript. Use `JsonObject` instead (imported from `@superset-ui/core`) to maintain type consistency with other parts of the codebase where `ownState` is used (e.g., `Chart.tsx` line 78, `ExploreChartPanel/index.tsx` line 66). This provides better type safety and aligns with established patterns.
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -76,7 +78,7 @@ const ViewQueryModal: FC<Props> = ({ latestQueryFormData }) => {
};
useEffect(() => {
loadChartData('query');
- }, [JSON.stringify(latestQueryFormData)]);
+ }, [JSON.stringify(latestQueryFormData), JSON.stringify(ownState)]);
Review Comment:
Using `JSON.stringify()` in dependency arrays is an anti-pattern that creates new string references on every render, causing unnecessary re-executions. Additionally, `loadChartData` is defined inside the component but not included in the dependency array, which violates the exhaustive-deps rule.
Consider using `useMemo` to memoize the stringified values, or better yet, restructure to avoid this pattern:
```typescript
const loadChartData = useCallback((resultType: string) => {
setIsLoading(true);
getChartDataRequest({
formData: latestQueryFormData,
resultFormat: 'json',
resultType,
ownState: ownState || {},
})
// ... rest of the implementation
}, [latestQueryFormData, ownState]);
useEffect(() => {
loadChartData('query');
}, [loadChartData]);
```
This approach properly tracks dependencies without the overhead of JSON stringification.
##########
superset-frontend/src/dashboard/components/SliceHeader/index.tsx:
##########
@@ -57,6 +57,7 @@ type SliceHeaderProps = SliceHeaderControlsProps & {
filters: object;
handleToggleFullSize: () => void;
formData: object;
+ ownState?: object;
Review Comment:
The type `object` is too generic for TypeScript. Use `JsonObject` instead (imported from `@superset-ui/core`) to maintain type consistency with other parts of the codebase where `ownState` is used. This provides better type safety and aligns with established patterns.
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -31,6 +31,7 @@ import ViewQuery from 'src/explore/components/controls/ViewQuery';
interface Props {
latestQueryFormData: QueryFormData;
+ ownState?: object;
Review Comment:
The type `object` is too generic for TypeScript. Use `JsonObject` instead (imported from `@superset-ui/core`) to maintain type consistency with other parts of the codebase where `ownState` is used (e.g., `Chart.tsx` line 78). This provides better type safety and aligns with established patterns.
```typescript
import {
styled,
ensureIsArray,
t,
getClientErrorObject,
QueryFormData,
JsonObject,
} from '@superset-ui/core';
interface Props {
latestQueryFormData: QueryFormData;
ownState?: JsonObject;
}
```
```suggestion
ownState?: JsonObject;
```
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx:
##########
@@ -497,7 +497,10 @@ export const useExploreAdditionalActionsMenu = (
}
modalTitle={t('View query')}
modalBody={
- <ViewQueryModal latestQueryFormData={latestQueryFormData} />
+ <ViewQueryModal
+ latestQueryFormData={latestQueryFormData}
+ ownState={ownState}
+ />
Review Comment:
The `ownState` dependency is missing from the `useMemo` dependency array. Since `ownState` is now used in the `ViewQueryModal` component within this memoized menu, it should be included in the dependency array to ensure the menu is re-created when `ownState` changes. Add `ownState` to the dependency array at line 548.
--
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [superset]
Posted by "marunrun (via GitHub)" <gi...@apache.org>.
marunrun commented on code in PR #35208:
URL: https://github.com/apache/superset/pull/35208#discussion_r3472617519
##########
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:
##########
@@ -705,6 +705,18 @@ const Chart = (props: ChartProps) => {
height={getHeaderHeight()}
exportPivotExcel={exportPivotExcel as unknown as (arg0: string) => void}
chartHolderRef={props.chartHolderRef}
+ ownState={createOwnStateWithChartState(
+ (dataMask[props.id]?.ownState as JsonObject) || EMPTY_OBJECT,
+ {
+ state:
+ getChartStateWithFallback(
+ chartState as { state?: JsonObject } | undefined,
+ formData as JsonObject,
+ slice.viz_type,
+ ) ?? undefined,
+ },
+ slice.viz_type,
+ )}
Review Comment:
Valid point, and this one I did introduce — fixed in c8da7e6e81. The `ownState` computation (`createOwnStateWithChartState`) is now wrapped in `useMemo` keyed on `[dataMaskOwnState, chartState, formData, sliceVizType]`, so it holds a stable reference across re-renders that don't change its logical value. Both `ownState={...}` call sites in `Chart.tsx` now consume that memoized value, so `ViewQueryModal` only refetches on real state changes.
--
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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [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 #35208:
URL: https://github.com/apache/superset/pull/35208#discussion_r3472314326
##########
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:
##########
@@ -705,6 +705,18 @@ const Chart = (props: ChartProps) => {
height={getHeaderHeight()}
exportPivotExcel={exportPivotExcel as unknown as (arg0: string) => void}
chartHolderRef={props.chartHolderRef}
+ ownState={createOwnStateWithChartState(
+ (dataMask[props.id]?.ownState as JsonObject) || EMPTY_OBJECT,
+ {
+ state:
+ getChartStateWithFallback(
+ chartState as { state?: JsonObject } | undefined,
+ formData as JsonObject,
+ slice.viz_type,
+ ) ?? undefined,
+ },
+ slice.viz_type,
+ )}
Review Comment:
**Suggestion:** `ownState` is recomputed inline during every render and passed as a new object reference each time. Since `ViewQueryModal` now refetches when `ownState` changes, this can trigger repeated backend query requests while the modal is open even when the effective state did not change. Memoize the computed `ownState` (for example with `useMemo`) before passing it down so refetching only happens on real state changes. [performance]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dashboard "View query" issues duplicate chart data requests.
- ⚠️ Extra chart-data traffic loads backend unnecessarily during viewing.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. On a dashboard, a chart is rendered by `Chart` in
`superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:170-815`. In
its JSX return, it passes an `ownState` prop into `SliceHeader` at lines 666-720, computed
inline as a new object on every render via `createOwnStateWithChartState` (lines 708-719
and helper at 151-168).
2. `SliceHeader` in
`superset-frontend/src/dashboard/components/SliceHeader/index.tsx:145-402` forwards this
`ownState` prop to `SliceHeaderControls` at line 359-392 (`ownState={ownState}` at
391-392), so every re-render of `Chart` produces a fresh `ownState` object reference on
`SliceHeaderControls`.
3. `SliceHeaderControls` in
`superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:166-695` uses
this prop when constructing the "View query" menu item (lines 479-501). The `ModalTrigger`
renders `<ViewQueryModal latestQueryFormData={props.formData} ownState={props.ownState}
/>` at 489-492, so each new `ownState` reference from `Chart` is passed down to
`ViewQueryModal`.
4. `ViewQueryModal` in
`superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:53-88` defines
`loadChartData` with `useCallback` depending on `latestQueryFormData` and `ownState` (line
58 `useCallback` with deps at 84), and a `useEffect` that calls `loadChartData('query')`
whenever that callback identity changes (lines 86-88). Because `Chart` recomputes a new
`ownState` object every render (lines 708-719) without memoization, any re-render of
`Chart` that doesn't actually change the logical `ownState` (for example, toggling
`isStreamingModalVisible` in `Chart` at lines 266-278 or updating other local state) still
passes a new `ownState` reference into `ViewQueryModal`, causing `loadChartData` to change
identity and the effect to fire again. This results in repeated `getChartDataRequest`
backend calls while the "View query" modal is open, even though the effective ownState and
query haven't changed.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=219dfb8ac8a34bc785f20188db4e9ae3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=219dfb8ac8a34bc785f20188db4e9ae3&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/components/gridComponents/Chart/Chart.tsx
**Line:** 708:719
**Comment:**
*Performance: `ownState` is recomputed inline during every render and passed as a new object reference each time. Since `ViewQueryModal` now refetches when `ownState` changes, this can trigger repeated backend query requests while the modal is open even when the effective state did not change. Memoize the computed `ownState` (for example with `useMemo`) before passing it down so refetching only happens on real state changes.
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%2F35208&comment_hash=a7b1011633955fb24cc79a74a8c1edf8e6bb35ee3515b803c08838e49899835c&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F35208&comment_hash=a7b1011633955fb24cc79a74a8c1edf8e6bb35ee3515b803c08838e49899835c&reaction=dislike'>👎</a>
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -48,38 +50,42 @@ const ViewQueryModalContainer = styled.div`
gap: ${({ theme }) => theme.sizeUnit * 4}px;
`;
-const ViewQueryModal: FC<Props> = ({ latestQueryFormData }) => {
+const ViewQueryModal: FC<Props> = ({ latestQueryFormData, ownState }) => {
const [result, setResult] = useState<Result[]>([]);
const [isLoading, setIsLoading] = useState(false);
const [error, setError] = useState<string | null>(null);
- const loadChartData = (resultType: string) => {
- setIsLoading(true);
- getChartDataRequest({
- formData: latestQueryFormData,
- resultFormat: 'json',
- resultType,
- })
- .then(({ json }) => {
- setResult(ensureIsArray(json.result) as Result[]);
- setIsLoading(false);
- setError(null);
+ const loadChartData = useCallback(
+ (resultType: string) => {
+ setIsLoading(true);
+ getChartDataRequest({
+ formData: latestQueryFormData,
+ resultFormat: 'json',
+ resultType,
+ ownState: ownState || {},
})
- .catch(response => {
- getClientErrorObject(response).then(({ error, message }) => {
- setError(
- error ||
- message ||
- response.statusText ||
- t('Sorry, An error occurred'),
- );
+ .then(({ json }) => {
+ setResult(ensureIsArray(json.result) as Result[]);
setIsLoading(false);
+ setError(null);
Review Comment:
**Suggestion:** The async request has no cancellation or stale-response guard, so if multiple requests are fired (for example from rapid prop/state changes), an older response can resolve last and overwrite newer results in the modal. Add request sequencing or abort handling in `useEffect` cleanup so only the latest in-flight request can update component state. [race condition]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ View query can show SQL from outdated chart state.
- ⚠️ Explore and dashboard users may misinterpret stale queries.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In Explore, the additional actions menu hook `useExploreAdditionalActionsMenu`
(`superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx:194-1085`)
creates the "View query" menu item. At lines 1009-1023 it renders `<ViewQueryModal
latestQueryFormData={latestQueryFormData as QueryFormData} ownState={ownState} />` inside
a `ModalTrigger`, so the modal receives the current query form data and ownState whenever
it is opened or when those props change.
2. `ViewQueryModal`
(`superset-frontend/src/explore/components/controls/ViewQueryModal.tsx`) defines
`loadChartData` with `useCallback` at lines 58-84. Inside, it calls `getChartDataRequest`
(imported from `chartAction.ts`) at lines 61-66 with `{ formData: latestQueryFormData,
resultFormat: 'json', resultType, ownState: ownState || {} }`, and on success runs
`setResult(...)` and clears loading/error at lines 67-71.
3. A `useEffect` at lines 86-88 calls `loadChartData('query')` whenever `loadChartData`'s
identity changes. Since `useCallback` depends on `[latestQueryFormData, ownState]` (line
84), every change to either prop while the modal stays open triggers a new
`getChartDataRequest` call without cancelling any in-flight request. For example, in
Explore, repeatedly changing controls and re-running the chart while keeping "View query"
open will send a new request on each change; similarly on dashboards,
`SliceHeaderControls` passes `props.formData` and `props.ownState` into `ViewQueryModal`
at `superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:489-492`, so
rapid filter or state changes can also trigger multiple overlapping requests.
4. `getChartDataRequest` in `superset-frontend/src/components/Chart/chartAction.ts:52-95`
simply forwards to `legacyChartDataRequest` or `v1ChartDataRequest`, both of which use
`SupersetClient.post` with no abort logic or sequencing (see `v1ChartDataRequest` at
453-50 and `getChartDataRequest` at 52-95). Because there is no cancellation or "latest
request wins" guard in `ViewQueryModal`, if an older request resolves after a newer one,
its `.then` handler at lines 67-71 will run last and overwrite `result` state with stale
data. In practice this manifests as "View query" occasionally showing SQL from a previous
chart state when users change form controls and trigger multiple backend queries in quick
succession while the modal remains open.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=806479cc0f1d4734b5ecab667d0cedb9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=806479cc0f1d4734b5ecab667d0cedb9&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/ViewQueryModal.tsx
**Line:** 61:70
**Comment:**
*Race Condition: The async request has no cancellation or stale-response guard, so if multiple requests are fired (for example from rapid prop/state changes), an older response can resolve last and overwrite newer results in the modal. Add request sequencing or abort handling in `useEffect` cleanup so only the latest in-flight request can update component state.
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%2F35208&comment_hash=07a84f96bf566466e229d13110f6d7b4fa9b6ad85e5aef02d19839bbaf8247f5&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F35208&comment_hash=07a84f96bf566466e229d13110f6d7b4fa9b6ad85e5aef02d19839bbaf8247f5&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] fix(view-query): Add ownState parameter to ViewQueryModal for query consistency [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 #35208:
URL: https://github.com/apache/superset/pull/35208#discussion_r3472621702
##########
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx:
##########
@@ -48,38 +50,42 @@ const ViewQueryModalContainer = styled.div`
gap: ${({ theme }) => theme.sizeUnit * 4}px;
`;
-const ViewQueryModal: FC<Props> = ({ latestQueryFormData }) => {
+const ViewQueryModal: FC<Props> = ({ latestQueryFormData, ownState }) => {
const [result, setResult] = useState<Result[]>([]);
const [isLoading, setIsLoading] = useState(false);
const [error, setError] = useState<string | null>(null);
- const loadChartData = (resultType: string) => {
- setIsLoading(true);
- getChartDataRequest({
- formData: latestQueryFormData,
- resultFormat: 'json',
- resultType,
- })
- .then(({ json }) => {
- setResult(ensureIsArray(json.result) as Result[]);
- setIsLoading(false);
- setError(null);
+ const loadChartData = useCallback(
+ (resultType: string) => {
+ setIsLoading(true);
+ getChartDataRequest({
+ formData: latestQueryFormData,
+ resultFormat: 'json',
+ resultType,
+ ownState: ownState || {},
})
- .catch(response => {
- getClientErrorObject(response).then(({ error, message }) => {
- setError(
- error ||
- message ||
- response.statusText ||
- t('Sorry, An error occurred'),
- );
+ .then(({ json }) => {
+ setResult(ensureIsArray(json.result) as Result[]);
setIsLoading(false);
+ setError(null);
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> When a race-condition/cancellation issue originates in a shared request helper or existing request pipeline, scope the review to the shared abstraction or all affected call sites rather than treating it as a defect introduced by a single modal change.
**Applied to:**
- `**/*.ts`
- `**/*.tsx`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI Settings](https://app.codeant.ai/org/settings/learnings)*
--
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