fix(explore): stop dashboard permalink key from leaking into chart URL

RESERVED_CHART_URL_PARAMS was missing permalink_key while the analogous
RESERVED_DASHBOARD_URL_PARAMS already excluded it. This asymmetry let a
dashboard permalink's permalink_key, merged into a chart's form_data via
a shared cache keyed only by sliceId, get copied into the chart's own
Explore URL when opened from a dashboard. On refresh, Explore forwarded
that dashboard-salted key to the explore permalink resolver, which fails
key decoding against the wrong salt and falls back to a stub datasource,
producing the "missing datasource" error.

Add permalink_key to RESERVED_CHART_URL_PARAMS, mirroring the pattern
already used correctly on the dashboard side and in FilterBar's
EXCLUDED_URL_PARAMS. As an accepted side effect, this also makes
Explore's own permalink key (/explore/p/<key>/) drop out of the URL
after refresh instead of staying sticky, matching FilterBar's existing
behavior for dashboards.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

superset-frontend/playwright.config.ts

@@ -47,6 +47,10 @@ export default defineConfig({
   // Retry logic - 2 retries in CI, 0 locally
   retries: process.env.CI ? 2 : 0,
 
+  // Disable capturing Git commit info as the project's history is increasingly dense
+  // and breach Playwright's default 3-seconds `git` command timeout limit
+  captureGitInfo: { commit: false, diff: false },
+
   // Reporter configuration - multiple reporters for better visibility
   reporter: process.env.CI
     ? [

superset-frontend/playwright/tests/recently-archived/delete-modal.spec.ts

@@ -0,0 +1,58 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * With SOFT_DELETE enabled the delete-confirmation modal becomes recoverable:
+ * it explains the object is moved to the archive (and for how long), and drops
+ * the "type DELETE to confirm" friction. Non-destructive — the modal is opened
+ * and dismissed without deleting anything.
+ */
+import { test, expect } from '@playwright/test';
+import { skipUnlessFeatureEnabled } from '../../helpers/featureFlags';
+
+test.beforeEach(async ({ page }) => {
+  await skipUnlessFeatureEnabled(page, 'SOFT_DELETE');
+});
+
+test('chart delete confirmation reflects soft-delete (archive) semantics', async ({
+  page,
+}) => {
+  await page.goto('chart/list/');
+  await page.locator('[data-test="chart-row-delete"]').first().waitFor();
+  await page.locator('[data-test="chart-row-delete"]').first().click();
+
+  // The action reads as "Archive", not "Delete". Scope to the dialog: with
+  // the flag on, every list row's delete action is also named "Archive", so
+  // an unscoped button query is a strict-mode violation (25 rows + modal).
+  const dialog = page.getByRole('dialog');
+  await expect(dialog.getByText(/^Archive .+\?$/)).toBeVisible();
+  await expect(dialog.getByRole('button', { name: 'Archive' })).toBeVisible();
+
+  // Recoverable copy instead of "Are you sure … permanently".
+  await expect(page.getByText(/moved to Recently Archived/i)).toBeVisible();
+  await expect(
+    page.getByText(/recover it there within \d+ days/i),
+  ).toBeVisible();
+
+  // No "type DELETE to confirm" input in recoverable mode.
+  await expect(page.getByTestId('delete-modal-input')).toHaveCount(0);
+
+  // Dismiss without deleting.
+  await page.getByTestId('close-modal-btn').click();
+});

superset-frontend/playwright/tests/recently-archived/recently-archived.spec.ts

@@ -29,7 +29,7 @@
  * restore it and asserts — via the API — that it is live again.
  */
 import { test, expect, Page } from '@playwright/test';
-import { apiGet } from '../../helpers/api/requests';
+import { apiGet, apiPost } from '../../helpers/api/requests';
 import { extractIdFromResponse } from '../../helpers/api/assertions';
 import {
   apiPostChart,
@@ -188,3 +188,58 @@ test('permanently deletes an archived item from the view', async ({ page }) => {
     await TYPES[0].softDelete(page, id).catch(() => {});
   }
 });
+
+test('shows an empty message and no rows when the search matches nothing', async ({
+  page,
+}) => {
+  await page.goto('archived/');
+  await expect(page.getByTestId('archived-list-view')).toBeVisible();
+
+  const search = page.getByPlaceholder(/type a value/i);
+  await search.click();
+  await search.fill(`e2e_nonexistent_${Date.now()}`);
+  await search.press('Enter');
+
+  await expect(
+    page.getByText('No results match your filter criteria'),
+  ).toBeVisible();
+  await expect(page.getByTestId('archived-row-restore')).toHaveCount(0);
+});
+
+test('restoring an already-restored row surfaces an error without crashing', async ({
+  page,
+}) => {
+  const name = `e2e_stale_${Date.now()}`;
+  const id = await TYPES[0].create(page, name);
+  // Capture the uuid before soft-delete (a soft-deleted GET returns 404).
+  const { uuid } = (await (await apiGetDashboard(page, id)).json()).result;
+  try {
+    expect((await apiDeleteDashboard(page, id)).ok()).toBeTruthy();
+
+    await openArchive(page, 'Dashboard', name);
+    await expect(page.getByText(name, { exact: false })).toBeVisible();
+
+    // Simulate another actor restoring the object out from under this view.
+    const restored = await apiPost(
+      page,
+      `api/v1/dashboard/${uuid}/restore`,
+      {},
+    );
+    expect(restored.ok()).toBeTruthy();
+
+    // Clicking the now-stale row's Restore yields a 404 → danger toast, no crash.
+    await page
+      .getByRole('row')
+      .filter({ hasText: name })
+      .getByTestId('archived-row-restore')
+      .click();
+    await expect(
+      page.getByText(`Failed to restore ${name}`, { exact: false }),
+    ).toBeVisible({ timeout: 15000 });
+    // The page is still functional (the list view did not crash).
+    await expect(page.getByTestId('archived-list-view')).toBeVisible();
+  } finally {
+    // Re-archive the (possibly) restored dashboard, whatever happened above.
+    await apiDeleteDashboard(page, id).catch(() => {});
+  }
+});

superset-frontend/src/constants.test.ts

@@ -0,0 +1,31 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+  URL_PARAMS,
+  RESERVED_CHART_URL_PARAMS,
+  RESERVED_DASHBOARD_URL_PARAMS,
+} from 'src/constants';
+
+test('permalinkKey is reserved on both the chart and dashboard URL param lists', () => {
+  // Dashboard and explore permalinks resolve against different backend
+  // KV resources/salts, so a key from one must never leak into the other's
+  // URL via the reserved-params passthrough logic.
+  expect(RESERVED_DASHBOARD_URL_PARAMS).toContain(URL_PARAMS.permalinkKey.name);
+  expect(RESERVED_CHART_URL_PARAMS).toContain(URL_PARAMS.permalinkKey.name);
+});

superset-frontend/src/constants.ts

@@ -123,6 +123,7 @@ export const RESERVED_CHART_URL_PARAMS: string[] = [
   URL_PARAMS.datasourceId.name,
   URL_PARAMS.datasourceType.name,
   URL_PARAMS.datasetId.name,
+  URL_PARAMS.permalinkKey.name,
   URL_PARAMS.versionHistory.name,
 ];
 export const RESERVED_DASHBOARD_URL_PARAMS: string[] = [

superset-frontend/src/dashboard/components/SliceAdder.tsx

@@ -468,7 +468,7 @@ function SliceAdder({
           <AutoSizer>
             {({ height, width }: { height: number; width: number }) => (
               <List
-                style={{ width, height }}
+                style={{ width, height, maxHeight: height }}
                 rowCount={filteredSlices.length}
                 rowHeight={DEFAULT_CELL_HEIGHT}
                 rowProps={listRowProps}

superset-frontend/src/explore/components/DatasourcePanel/DatasourceItems.tsx

@@ -148,7 +148,7 @@ export const DatasourceItems = ({
 
   return (
     <List
-      style={{ width: width - BORDER_WIDTH, height }}
+      style={{ width: width - BORDER_WIDTH, height, maxHeight: height }}
       rowHeight={rowHeight}
       rowCount={flattenedItems.length}
       rowProps={rowProps}

superset-frontend/src/pages/ArchivedList/ArchivedList.test.tsx

@@ -141,7 +141,6 @@ const renderArchivedList = (withStore = store) =>
 beforeEach(() => {
   fetchMock.removeRoutes();
   fetchMock.clearHistory();
-  mockAddDangerToast.mockClear();
 });
 
 test('renders archived rows with Name and Type columns', async () => {
@@ -205,31 +204,6 @@ test('restore failure surfaces an error and leaves the row in place', async () =
   expect(screen.getByText('Deleted Chart One')).toBeInTheDocument();
 });
 
-test('restoring an already-restored row (404) surfaces an error without crashing', async () => {
-  // Simulates another actor having restored the object out from under this
-  // view: the server answers 404 to the now-stale row's restore request.
-  mockRoutes(404);
-  renderArchivedList();
-  await screen.findByTestId('archived-list-view');
-
-  const restoreButtons = await screen.findAllByTestId('archived-row-restore');
-  fireEvent.click(restoreButtons[0]);
-
-  await waitFor(() => {
-    expect(fetchMock.callHistory.calls(/chart\/uuid-1\/restore/)).toHaveLength(
-      1,
-    );
-  });
-  await waitFor(() => {
-    expect(mockAddDangerToast).toHaveBeenCalledWith(
-      expect.stringContaining('Failed to restore Deleted Chart One'),
-    );
-  });
-  expect(mockAddDangerToast).toHaveBeenCalledTimes(1);
-  // The page is still functional -- the list view did not crash.
-  expect(screen.getByTestId('archived-list-view')).toBeInTheDocument();
-});
-
 test('row actions are keyboard-operable (Enter restores)', async () => {
   mockRoutes();
   renderArchivedList();
@@ -299,45 +273,6 @@ test('name search refetches with a contains filter on the name field', async ()
   });
 });
 
-test('a search that matches nothing shows the empty-state and no restore actions', async () => {
-  // The initial load returns real rows; only the search-triggered request
-  // answers empty. If the list were empty from the start, this test could
-  // pass even if the search never fired a request at all -- so the request
-  // itself is asserted below before trusting the rendered empty state.
-  fetchMock.get(infoEndpoint, { permissions: ['can_read', 'can_write'] });
-  fetchMock.getOnce(listEndpoint, {
-    result: mockCharts,
-    count: mockCharts.length,
-  });
-  fetchMock.get(listEndpoint, { result: [], count: 0 });
-  renderArchivedList();
-  await screen.findByText('Deleted Chart One');
-
-  const searchInput = screen.getByPlaceholderText(/type a value/i);
-  fireEvent.change(searchInput, { target: { value: 'e2e_nonexistent' } });
-  fireEvent.keyDown(searchInput, { key: 'Enter', keyCode: 13 });
-
-  await waitFor(() => {
-    const hit = fetchMock.callHistory
-      .calls(/chart\/\?q/)
-      .find(call =>
-        call.url.includes(
-          '(col:slice_name,opr:chart_all_text,value:e2e_nonexistent)',
-        ),
-      );
-    expect(hit).toBeTruthy();
-  });
-
-  // ListView renders this hardcoded copy whenever a filter is active and the
-  // result set is empty, overriding the page's own `emptyState` prop
-  // entirely (see ListView.tsx) -- so this is the actual rendered text, not
-  // the page's "No archived items" default.
-  expect(
-    await screen.findByText('No results match your filter criteria'),
-  ).toBeInTheDocument();
-  expect(screen.queryAllByTestId('archived-row-restore')).toHaveLength(0);
-});
-
 test('switching Type fetches the newly selected resource with its deleted-state filter', async () => {
   mockRoutes();
   renderArchivedList();

superset-frontend/src/pages/ChartList/ChartList.test.tsx

@@ -239,40 +239,6 @@ describe('ChartList', () => {
       screen.getByRole('button', { name: 'Bulk select' }),
     ).toBeInTheDocument();
   });
-
-  test('archive (soft-delete) confirmation reflects recoverable semantics, not delete', async () => {
-    // With SOFT_DELETE on, the same delete affordance becomes reversible: the
-    // dialog reads "Archive", not "Delete", and drops the "type DELETE to
-    // confirm" gate -- that friction is reserved for the permanent purge in
-    // the Recently Archived view, not this one.
-    (
-      isFeatureEnabled as jest.MockedFunction<typeof isFeatureEnabled>
-    ).mockImplementation((feature: string) => feature === 'SOFT_DELETE');
-
-    // isUserEditorOrAdmin requires `username` + `permissions` to recognize an
-    // Admin role (see src/types/bootstrapTypes.ts's isUserWithPermissionsAndRoles);
-    // mockUser lacks both, so row actions would otherwise render disabled.
-    const adminUser = { ...mockUser, username: 'admin', permissions: {} };
-    renderChartList(adminUser);
-    await screen.findByTestId('chart-list-view');
-
-    const deleteButtons = await screen.findAllByTestId('chart-row-delete');
-    fireEvent.click(deleteButtons[0]);
-
-    const dialog = await screen.findByRole('dialog');
-    expect(
-      within(dialog).getByText(`Archive ${mockCharts[0].slice_name}?`),
-    ).toBeInTheDocument();
-    expect(
-      within(dialog).getByRole('button', { name: 'Archive' }),
-    ).toBeInTheDocument();
-    expect(
-      within(dialog).getByText(/moved to Recently Archived/i),
-    ).toBeInTheDocument();
-    expect(within(dialog).getByText(/recover it there/i)).toBeInTheDocument();
-
-    expect(screen.queryByTestId('delete-modal-input')).not.toBeInTheDocument();
-  });
 });
 
 // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks