fix(explore): exclude permalink_key from chart URL params

[sadpandajoe]

SUMMARY

Opening a dashboard permalink and editing a chart into Explore, then refreshing the Explore page, can render Explore's "missing datasource" error. RESERVED_CHART_URL_PARAMS was missing permalink_key, so a dashboard-scoped permalink key could be copied into a chart-scoped Explore URL by the URL-rewrite logic in ExploreViewContainer. On refresh, that key is sent to the backend and resolved against the wrong key-value namespace (dashboard vs. explore permalinks use different backend resources/salts), which fails and surfaces as a missing-datasource error.

This adds permalink_key to RESERVED_CHART_URL_PARAMS, mirroring its existing presence in RESERVED_DASHBOARD_URL_PARAMS and the same exclusion pattern already used in the dashboard's native filter bar.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — this is a fix to URL-parameter handling, not a visual change.

TESTING INSTRUCTIONS

1. Open a dashboard.
2. Open the vertical ellipsis menu on any chart and choose Share > Copy permalink to clipboard.
3. Open the dashboard using that permalink URL.
4. Edit the chart from the dashboard (opens Explore).
5. Refresh the Explore page.
6. Before this change: the page can show a "missing datasource" error. After this change: the chart loads normally.

Also covered by the added unit test: superset-frontend/src/constants.test.ts.

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.73%. Comparing base (c2d653b) to head (7f362a8).
⚠️ Report is 14 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #43354   +/-   ##
=======================================
  Coverage   66.73%   66.73%           
=======================================
  Files        2876     2876           
  Lines      164201   164201           
  Branches    37887    37887           
=======================================
  Hits       109580   109580           
  Misses      52465    52465           
  Partials     2156     2156           

Flag	Coverage Δ
javascript	74.03% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[bito-code-review]

Code Review Agent Run #912344

Actionable Suggestions - 0

Review Details

• Files reviewed - 2 · Commit Range: 7f362a8..7f362a8

  • superset-frontend/src/constants.test.ts
  • superset-frontend/src/constants.ts
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful
  • Eslint (Linter) - ✖︎ Failed

---

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

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[gabotorresruiz]

Thanks for the fix, Joe. The change is clearly safe and I verified it end to end:

• The new test fails on master without the constants change (only the RESERVED_CHART_URL_PARAMS assertion) and passes with it; the urlUtils, ExploreViewContainer and exploreUtils suites all pass locally with the change applied.
• None of the consumers of RESERVED_CHART_URL_PARAMS needs permalink_key to pass through. Nice side effect: getChartUrlParams uses the same list, so this also keeps a stale permalink_key out of the urlParams stored in newly created explore permalinks.
• The failure mode reproduces exactly as described: I put a dashboard-namespace key into an Explore URL on a running master build and got the "Missing datasource" panel, because /api/v1/explore/ resolves permalink_key ahead of form_data_key (superset/commands/explore/get.py) and a dashboard key fails to decode under the explore salt.

One question, not a blocker: I could not reproduce the ingress with the listed steps on master head. The dashboard URL's permalink_key gets replaced by native_filters_key by the filter bar's URL update (its EXCLUDED_URL_PARAMS already contains permalink_key), and "Edit chart" navigates to /explore/?dashboard_page_id=...&slice_id=... without carrying dashboard query params, so on my build the key never reached the Explore URL and refresh worked even without the fix. Which branch or flow did you hit this on? Asking mainly to figure out whether release branches need a backport. The exclusion still has a live effect on master either way: form_data.url_params picks up permalink_key whenever Explore is loaded from a permalink (merge_request_params), and without this change any later URL rewrite copies it back into the address bar, where it then shadows form_data_key on refresh.

[sadpandajoe]

Thanks for digging into this — good catch, and you're right that the literal repro steps alone won't trigger it on every dashboard/build. Answering both parts:

Which flow actually leaks the key (not the "Edit chart" link):

You're correct that EXCLUDED_URL_PARAMS in the filter bar already strips permalink_key from the dashboard's own URL, and that "Edit chart" navigates via a clean dashboard_page_id/slice_id URL with no dashboard query params attached. Neither of those is the leak vector. The actual carrier is the memoized cache in dashboard/util/charts/getFormDataWithExtraFilters.ts, keyed only by sliceId (cachedFormdataByChart[sliceId] / cachedFiltersByChart[sliceId]). While the dashboard is mounted with a permalink in its URL, that cache gets populated with the chart's full form data, including url_params.permalink_key (from merge_request_params, same mechanism you found). When Explore mounts for that same sliceId and hits a cache match (cachedFiltersByChart[sliceId] === filters), it reuses the dashboard's cached form data rather than fetching fresh — leaking url_params.permalink_key along with it. getFormDataWithDashboardContext.ts then spreads ...exploreFormData, ...dashboardContextFormData (dashboard context last), so the leaked key wins and reaches ExploreViewContainer's URL-rewrite.

That cache-hit is a real precondition, not automatic — it depends on dashboard/filter state at the moment "Edit chart" is clicked (matching cached filters for that slice). In testing this reproduced on 1 of 8 seeded example dashboards, so a clean build/dashboard with no prior matching cache entry for that slice — which sounds like what you hit — will genuinely take the clean path and never carry permalink_key into Explore at all. That's consistent with your result, not a discrepancy in the fix.

Branch / backport:

This was reproduced and fixed against master. I checked whether the same code exists on the current release branch (6.2) before answering the backport question, rather than guessing:

• constants.ts on 6.2 has the identical asymmetry — RESERVED_CHART_URL_PARAMS omits permalink_key, RESERVED_DASHBOARD_URL_PARAMS already includes it.
• ExploreViewContainer/index.tsx on 6.2 consumes RESERVED_CHART_URL_PARAMS the same way.
• getFormDataWithExtraFilters.ts's sliceId-keyed cache and getFormDataWithDashboardContext.ts's ...exploreFormData, ...dashboardContextFormData override order are both present on 6.2 as well.

So yes — this looks like a genuine backport candidate for 6.2 (and likely earlier 6.x branches sharing the same code), not a master-only issue. I haven't opened a backport PR; let me know if you'd like one against 6.2.