codeant-ai-for-open-source[bot] commented on code in PR #41181:
URL: https://github.com/apache/superset/pull/41181#discussion_r3434664487
##########
superset-frontend/src/explore/components/SaveModal.tsx:
##########
@@ -162,6 +167,35 @@ class SaveModal extends Component<SaveModalProps,
SaveModalState> {
t('An error occurred while loading dashboard information.'),
);
}
+ } else {
+ const metadataDashboards = this.props.metadata?.dashboards;
+ if (metadataDashboards?.length) {
+ // Fallback: the chart is already on one or more dashboards (from
Explore API
+ // metadata). Pre-populate with the first dashboard the user can edit
so the
+ // "Save & go to dashboard" button works out of the box.
+ try {
+ let editable: Dashboard | undefined;
+ for (const { id } of metadataDashboards) {
+ // eslint-disable-next-line no-await-in-loop
+ const result = await this.loadDashboard(id).catch(() => null);
+ if (result && canUserEditDashboard(result, this.props.user)) {
+ editable = result as Dashboard;
+ break;
+ }
+ }
Review Comment:
**Suggestion:** The metadata fallback fetches dashboards sequentially using
`await` inside a loop, which creates one network round-trip per dashboard and
can make the modal noticeably slow for charts attached to many dashboards. This
also contradicts the intended behavior to load metadata dashboards in parallel.
Fetch all candidate dashboards concurrently (e.g., `Promise.all`) and then pick
the first editable one by original metadata order. [performance]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Explore Save chart modal slow when chart on many dashboards.
- ⚠️ "Save & go to dashboard" button stays disabled unnecessarily.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open an existing chart's Explore page at `/explore/?slice_id=<id>`
without a
`dashboard_id` URL param so `dashboardId` is undefined in `hydrateExplore`
and hence in
`ExploreViewContainer` (see `src/explore/actions/hydrateExplore.ts:16-21` and
`src/explore/components/ExploreViewContainer/index.tsx:1180-1186` where
`dashboardId` is
derived from `explore.form_data.dashboardId`).
2. Ensure the chart belongs to many dashboards so the bootstrap payload
includes a
populated `metadata.dashboards` array (shape defined in
`src/explore/types.ts:4-20`,
especially `metadata.dashboards?: { id: number; dashboard_title: string
}[]`), which
`hydrateExplore` stores on `explore.metadata` and `ExploreViewContainer`
passes through as
`metadata` (mapStateToProps around
`src/explore/components/ExploreViewContainer/index.tsx:1188-1204`).
3. In the Explore toolbar, click the Save button so `ExploreChartHeader`
dispatches
`setSaveChartModalVisibility(true)` via `useUnsavedChangesPrompt` (see
`src/explore/components/ExploreChartHeader/index.tsx:22-28` where `onSave`
calls
`dispatch(setSaveChartModalVisibility(true))`). This sets
`saveModal.isVisible` in Redux,
causing `ExploreViewContainer` to render `<SaveModal>` conditionally when
`props.isSaveModalVisible` is true
(`src/explore/components/ExploreViewContainer/index.tsx:1191-1199`).
4. When `<SaveModal>` mounts, its `componentDidMount` runs
(`src/explore/components/SaveModal.tsx:5-61` in the 140–219 block). Because
there is no
`dashboardId` and no `SK_DASHBOARD_ID` in `sessionStorage`, execution
follows the `else`
branch that reads `const metadataDashboards =
this.props.metadata?.dashboards;` (lines
31-33 of that block). The fallback then iterates `for (const { id } of
metadataDashboards)` and performs `await this.loadDashboard(id).catch(() =>
null);` inside
the loop (`src/explore/components/SaveModal.tsx` PR hunk lines 178-185 /
snippet lines
39-46), issuing one `/api/v1/dashboard/<id>` request at a time. With many
dashboards,
modal initialization (pre-populating `this.state.dashboard` and loading tabs
via
`loadTabs`) is delayed by roughly N×per-request-latency instead of the max
of concurrent
latencies, making the Save modal noticeably slower whenever the chart is
attached to many
dashboards.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=83f5af6956ae4b7eb92f704833e580ab&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=83f5af6956ae4b7eb92f704833e580ab&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/SaveModal.tsx
**Line:** 178:185
**Comment:**
*Performance: The metadata fallback fetches dashboards sequentially
using `await` inside a loop, which creates one network round-trip per dashboard
and can make the modal noticeably slow for charts attached to many dashboards.
This also contradicts the intended behavior to load metadata dashboards in
parallel. Fetch all candidate dashboards concurrently (e.g., `Promise.all`) and
then pick the first editable one by original metadata order.
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%2F41181&comment_hash=98ce30de55b432693e942369f7303882e9ea5d3107b43a445575e639c4239cd4&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41181&comment_hash=98ce30de55b432693e942369f7303882e9ea5d3107b43a445575e639c4239cd4&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
