Skip to content

fix(explore): preserve preview chart name on save#39908

Merged
richardfogaca merged 5 commits intoapache:masterfrom
richardfogaca:richardfogaca/update-chart-preview-save-name
May 7, 2026
Merged

fix(explore): preserve preview chart name on save#39908
richardfogaca merged 5 commits intoapache:masterfrom
richardfogaca:richardfogaca/update-chart-preview-save-name

Conversation

@richardfogaca
Copy link
Copy Markdown
Contributor

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:

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:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • 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

@richardfogaca richardfogaca self-assigned this May 6, 2026
@pull-request-size pull-request-size Bot added size/L and removed size/M labels May 6, 2026
@richardfogaca richardfogaca marked this pull request as ready for review May 6, 2026 17:12
Copilot AI review requested due to automatic review settings May 6, 2026 17:13
@dosubot dosubot Bot added explore Namespace | Anything related to Explore explore:save Related to saving changes in Explore labels May 6, 2026
@richardfogaca richardfogaca requested a review from kgabryje May 6, 2026 17:14
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 7, 2026

Code Review Agent Run #421d25

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: 12db287..d216319
    • superset-frontend/src/explore/actions/hydrateExplore.test.ts
    • superset-frontend/src/explore/actions/hydrateExplore.ts
    • superset-frontend/src/explore/components/SaveModal.test.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Comment thread superset-frontend/src/explore/actions/hydrateExplore.ts Outdated
Copy link
Copy Markdown
Member

@kgabryje kgabryje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with a nonblocking readability nit

@bito-code-review
Copy link
Copy Markdown
Contributor

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.

@richardfogaca richardfogaca merged commit 8c80cae into apache:master May 7, 2026
66 of 67 checks passed
@richardfogaca richardfogaca deleted the richardfogaca/update-chart-preview-save-name branch May 7, 2026 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

explore:save Related to saving changes in Explore explore Namespace | Anything related to Explore size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants