From d0cf5df302d5953a6e9a9020046f22addb12e453 Mon Sep 17 00:00:00 2001
From: sadpandajoe <jcli38@gmail.com>
Date: Thu, 6 Aug 2026 16:42:54 +0000
Subject: [PATCH] fix(dashboard): redirect using sanitized slug from save
 response

When a dashboard's URL slug in Edit Properties starts with a reserved URL
character (e.g. `?` or `/`), saving succeeded but the post-save redirect
navigated to a malformed/blank URL on first render; a later reload worked
because it used the stored, sanitized slug.

The post-save redirect in `saveDashboardRequest`'s `onUpdateSuccess` handler
built the target from the raw locally-submitted slug. The backend sanitizes
reserved characters out of the slug (`BaseDashboardSchema.post_load` strips
`[^\w\-]`), so the persisted slug returned in the PUT response can differ from
what was submitted. Build the redirect from the response slug, falling back to
the dashboard id when the response has no slug.

Adds regression tests covering the sanitized-slug redirect and the empty-slug
id fallback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 .../dashboard/actions/dashboardState.test.ts  | 74 +++++++++++++++++++
 .../src/dashboard/actions/dashboardState.ts   |  9 ++-
 2 files changed, 81 insertions(+), 2 deletions(-)

diff --git a/superset-frontend/src/dashboard/actions/dashboardState.test.ts b/superset-frontend/src/dashboard/actions/dashboardState.test.ts
index 0c12c31e7e91..0a6fc34a8fd8 100644
--- a/superset-frontend/src/dashboard/actions/dashboardState.test.ts
+++ b/superset-frontend/src/dashboard/actions/dashboardState.test.ts
@@ -298,6 +298,80 @@ describe('dashboardState actions', () => {
         { event: 'dashboard_properties_changed' },
       );
     });
+
+    // Regression for the reserved-URL-character slug redirect. When the
+    // submitted slug starts with a reserved character (e.g. `?` or `/`), the
+    // backend sanitizes it out of the persisted slug
+    // (BaseDashboardSchema.post_load strips `[^\w\-]`). The post-save redirect
+    // must therefore be built from the sanitized slug returned in the PUT
+    // response, not the raw slug the user submitted, or the first render lands
+    // on a malformed/blank URL (a later reload works because it reads the
+    // stored, sanitized slug).
+    test('redirects using the sanitized slug from the PUT response, not the raw submitted slug', async () => {
+      const updatedId = 778;
+      const { getState, dispatch } = setup({
+        dashboardState: { hasUnsavedChanges: true },
+      });
+
+      mockNavigateWithState.mockClear();
+      putStub.mockRestore();
+      putStub = jest.spyOn(SupersetClient, 'put').mockResolvedValue({
+        json: {
+          result: { ...mockDashboardData, id: updatedId, slug: 'test' },
+          last_modified_time: 0,
+        },
+      } as any);
+
+      const thunk = saveDashboardRequest(
+        { ...newDashboardData, slug: '?test' },
+        updatedId,
+        SAVE_TYPE_OVERWRITE,
+      );
+      await thunk(dispatch, getState);
+
+      await waitFor(() => expect(putStub.mock.calls.length).toBe(1));
+      await waitFor(() => expect(mockNavigateWithState).toHaveBeenCalled());
+
+      expect(mockNavigateWithState).toHaveBeenCalledWith('/dashboard/test/', {
+        event: 'dashboard_properties_changed',
+      });
+    });
+
+    // Regression companion: when the PUT response carries no usable slug (null
+    // or empty after sanitization — e.g. a slug composed solely of reserved
+    // characters sanitizes down to an empty string), the redirect must fall
+    // back to the dashboard id rather than emitting `/dashboard//` or echoing
+    // the raw submitted slug.
+    test('redirects using the id when the PUT response slug is empty', async () => {
+      const updatedId = 779;
+      const { getState, dispatch } = setup({
+        dashboardState: { hasUnsavedChanges: true },
+      });
+
+      mockNavigateWithState.mockClear();
+      putStub.mockRestore();
+      putStub = jest.spyOn(SupersetClient, 'put').mockResolvedValue({
+        json: {
+          result: { ...mockDashboardData, id: updatedId, slug: null },
+          last_modified_time: 0,
+        },
+      } as any);
+
+      const thunk = saveDashboardRequest(
+        { ...newDashboardData, slug: '?' },
+        updatedId,
+        SAVE_TYPE_OVERWRITE,
+      );
+      await thunk(dispatch, getState);
+
+      await waitFor(() => expect(putStub.mock.calls.length).toBe(1));
+      await waitFor(() => expect(mockNavigateWithState).toHaveBeenCalled());
+
+      expect(mockNavigateWithState).toHaveBeenCalledWith(
+        `/dashboard/${updatedId}/`,
+        { event: 'dashboard_properties_changed' },
+      );
+    });
   });
 
   test('fetchCharts returns a Promise that resolves after all refreshes', async () => {
diff --git a/superset-frontend/src/dashboard/actions/dashboardState.ts b/superset-frontend/src/dashboard/actions/dashboardState.ts
index 95633c04a50d..7bb79e19e068 100644
--- a/superset-frontend/src/dashboard/actions/dashboardState.ts
+++ b/superset-frontend/src/dashboard/actions/dashboardState.ts
@@ -629,8 +629,13 @@ export function saveDashboardRequest(
         dispatch(saveDashboardRequestSuccess(lastModifiedTime));
       }
       dispatch(saveDashboardFinished());
-      // redirect to the new slug or id
-      navigateWithState(`/dashboard/${slug || id}/`, {
+      // Redirect using the slug from the update response, not the raw
+      // submitted slug. The backend sanitizes reserved URL characters out of
+      // the slug (BaseDashboardSchema.post_load strips `[^\w\-]`), so the raw
+      // slug can differ from what was persisted and would build a malformed
+      // URL on first render. Fall back to the id when the response has no slug.
+      const updatedSlug = updatedDashboard.slug as string | null | undefined;
+      navigateWithState(`/dashboard/${updatedSlug || id}/`, {
         event: 'dashboard_properties_changed',
       });