You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "richardfogaca (via GitHub)" <gi...@apache.org> on 2026/05/06 12:04:53 UTC
[PR] fix(explore): preserve preview chart name on save [superset]
richardfogaca opened a new pull request, #39908:
URL: https://github.com/apache/superset/pull/39908
### SUMMARY
Explore preview routes can carry a staged chart name in form data, for example when `update_chart` is called with `generate_preview=true` and a new `chart_name`.
Before this change, Explore hydration left the active Explore slice name unset for that preview state, so downstream UI fell back to the persisted saved slice name. The preview page could show the staged name in form data, but the Save overwrite dialog initialized its chart-name input from the old saved chart name, making it easy to overwrite without applying the staged rename.
This updates Explore hydration so the active `sliceName` comes from current preview form data before falling back to the saved slice. The Save overwrite dialog already consumes the active Explore name, so it now pre-fills with the staged chart name without adding Save-modal-specific branching.
#### What Changed
- Hydration now derives the active Explore chart name from preview form data first.
- The saved slice name remains the fallback when preview form data does not include a chart name.
- Added focused coverage for the hydration boundary and the Save modal's initial chart-name value when the saved slice has a stale name.
#### Reviewer Focus
Please focus on the Explore hydration boundary in `superset-frontend/src/explore/actions/hydrateExplore.ts`. The fix intentionally avoids special-casing the Save modal; the modal should keep reading the current Explore name it already receives through props.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A - behavior was verified in browser QA, but no public screenshot is attached.
### TESTING INSTRUCTIONS
#### Validation
Manual QA performed:
1. Opened an Explore preview URL generated for a saved chart where preview form data had a new `slice_name` and the persisted chart still had the old name.
2. Confirmed the Explore title used the preview name.
3. Opened Save and confirmed `Save (Overwrite)` initialized the chart-name input from the preview name.
4. Saved the overwrite and confirmed the chart API reported the new name afterward.
Automated checks:
```bash
cd superset-frontend
npm run test -- src/explore/actions/hydrateExplore.test.ts src/explore/components/SaveModal.test.tsx -t 'hydrates sliceName from preview form data before saved slice name|initializes chart name from current Explore slice name'
```
Also ran `git diff --check` on the changed files.
### ADDITIONAL INFORMATION
- [ ] 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(explore): preserve preview chart name on save [superset]
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #39908:
URL: https://github.com/apache/superset/pull/39908#discussion_r3201182927
##########
superset-frontend/src/explore/actions/hydrateExplore.ts:
##########
@@ -77,6 +77,10 @@ export const hydrateExplore =
const fallbackSlice = sliceId ? sliceEntities?.slices?.[sliceId] : null;
const initialSlice = slice ?? fallbackSlice;
const initialFormData = form_data ?? initialSlice?.form_data;
+ const isCachedFormData = getUrlParam(URL_PARAMS.formDataKey) !== null;
+ const initialSliceName =
+ (isCachedFormData ? initialFormData : initialSlice)?.slice_name ??
+ (isCachedFormData ? initialSlice : initialFormData)?.slice_name;
Review Comment:
this expression is kinda hard to read... perhaps it would be better with if/else?
Or maybe even sth like:
```
const [primary, fallback] = isCachedFormData
? [initialFormData, initialSlice]
: [initialSlice, initialFormData];
const initialSliceName = primary?.slice_name ??
fallback?.slice_name;
```
up to you though
--
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(explore): preserve preview chart name on save [superset]
Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on code in PR #39908:
URL: https://github.com/apache/superset/pull/39908#discussion_r3201192016
##########
superset-frontend/src/explore/actions/hydrateExplore.ts:
##########
@@ -77,6 +77,10 @@ export const hydrateExplore =
const fallbackSlice = sliceId ? sliceEntities?.slices?.[sliceId] : null;
const initialSlice = slice ?? fallbackSlice;
const initialFormData = form_data ?? initialSlice?.form_data;
+ const isCachedFormData = getUrlParam(URL_PARAMS.formDataKey) !== null;
+ const initialSliceName =
+ (isCachedFormData ? initialFormData : initialSlice)?.slice_name ??
+ (isCachedFormData ? initialSlice : initialFormData)?.slice_name;
Review Comment:
yes you're right, I'll improve that, thanks @kgabryje !
--
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(explore): preserve preview chart name on save [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39908:
URL: https://github.com/apache/superset/pull/39908#issuecomment-4394356961
<div>
<h3>Code Review Agent Run #421d25</h3>
<div>
<details>
<summary>
<b>Actionable Suggestions - 0</b>
</summary>
</details>
</div>
<div>
<details>
<summary>
<b>Review Details</b>
</summary>
<ul>
<li>
<div id="file_reviewed">
Files reviewed - <b>3</b> · Commit Range: <code>12db287..d216319</code>
<ul>
<li>superset-frontend/src/explore/actions/hydrateExplore.test.ts</li><li>superset-frontend/src/explore/actions/hydrateExplore.ts</li><li>superset-frontend/src/explore/components/SaveModal.test.tsx</li>
</ul>
</div>
</li>
<li>
<div id="file_skipped">
Files skipped - <b>0</b>
<ul>
</ul>
</div>
</li>
<li>
<div id="tools">
Tools
<ul>
<li><b>Whispers</b> (Secret Scanner) - ✔︎ Successful</li><li><b>Detect-secrets</b> (Secret Scanner) - ✔︎ Successful</li><li><b>Eslint</b> (Linter) - ✔︎ Successful</li>
</ul>
</div>
</li>
</ul>
</details>
</div>
<hr>
<details>
<summary>
<b>Bito Usage Guide</b>
</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>
<span><i>AI Code Review powered by</i> <sub><sub><a href="https://bito.ai/" target="_blank"><img src="https://bito.ai/wp-content/uploads/2023/10/Logo-Bito-Black-cropped.svg" alt="Bito Logo" width="50" height="20" /></a></sub></sub></span>
</div>
--
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(explore): preserve preview chart name on save [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39908:
URL: https://github.com/apache/superset/pull/39908#issuecomment-4396851326
<!-- Bito Reply -->
The destructuring approach you suggested improves readability by clearly separating the primary and fallback assignments based on the condition, avoiding the nested ternary operators. It achieves the same logic with better clarity. If you prefer if/else for even more explicit control flow, that could work too, but the destructuring is concise and effective.
--
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(explore): preserve preview chart name on save [superset]
Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca merged PR #39908:
URL: https://github.com/apache/superset/pull/39908
--
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(explore): preserve preview chart name on save [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39908:
URL: https://github.com/apache/superset/pull/39908#issuecomment-4387726165
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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(explore): preserve preview chart name on save [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39908:
URL: https://github.com/apache/superset/pull/39908#issuecomment-4402855035
**Bito Automatic Review Skipped – PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save 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