Copilot commented on code in PR #42853:
URL: https://github.com/apache/superset/pull/42853#discussion_r3732490584
##########
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,
+ },
Review Comment:
The backend slug sanitization (BaseDashboardSchema.post_load) can turn a
submitted slug like "?" into an empty string (""), not `null`. This test
currently stubs the PUT response with `slug: null`, so it doesn't exercise the
real "sanitized to empty string" redirect case that the production bug
describes.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
