This is an automated email from the ASF dual-hosted git repository. michael-s-molina pushed a commit to branch 6.2 in repository https://gitbox.apache.org/repos/asf/superset.git
commit fb552055fb72a8cffd3a67013547800260d736d2 Author: Luiz Otavio <[email protected]> AuthorDate: Wed Jul 1 10:33:26 2026 -0300 fix(chart-explore): allow Save (Overwrite) after editing chart properties (#41602) (cherry picked from commit 569cfc982571d07f39bb11a41c467acd08954818) --- .../MessageToasts/ToastPresenter.test.tsx | 8 ++++ .../components/MessageToasts/ToastPresenter.tsx | 8 ++++ .../src/explore/components/SaveModal.test.tsx | 8 ++++ .../src/explore/reducers/exploreReducer.test.ts | 48 +++++++++++++++++++++- .../src/explore/reducers/exploreReducer.ts | 29 +++++++++---- 5 files changed, 92 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/components/MessageToasts/ToastPresenter.test.tsx b/superset-frontend/src/components/MessageToasts/ToastPresenter.test.tsx index 1a3e4894816..60813264394 100644 --- a/superset-frontend/src/components/MessageToasts/ToastPresenter.test.tsx +++ b/superset-frontend/src/components/MessageToasts/ToastPresenter.test.tsx @@ -47,3 +47,11 @@ test('should pass removeToast to the Toast component', async () => { fireEvent.click(getAllByTestId('close-button')[0]); await waitFor(() => expect(removeToast).toHaveBeenCalledTimes(1)); }); + +test('presenter container does not capture pointer events over its empty area', () => { + const { container } = setup(); + const presenter = container.querySelector('#toast-presenter'); + // The container spans a tall fixed region; its empty area must let clicks + // pass through to underlying controls (e.g. the Explore Save button). + expect(presenter).toHaveStyle('pointer-events: none'); +}); diff --git a/superset-frontend/src/components/MessageToasts/ToastPresenter.tsx b/superset-frontend/src/components/MessageToasts/ToastPresenter.tsx index de90f4a8f31..11af975d94a 100644 --- a/superset-frontend/src/components/MessageToasts/ToastPresenter.tsx +++ b/superset-frontend/src/components/MessageToasts/ToastPresenter.tsx @@ -39,6 +39,11 @@ const StyledToastPresenter = styled.div<VisualProps>( height: calc(100vh - 100px); + /* The container spans a tall region on the right edge of the screen. + Let clicks pass through its empty area so it doesn't block underlying + controls (e.g. the Explore Save button); toasts re-enable pointer events. */ + pointer-events: none; + display: flex; flex-direction: ${position === 'bottom' ? 'column-reverse' : 'column'}; align-items: stretch; @@ -107,6 +112,8 @@ const StyledToastPresenter = styled.div<VisualProps>( transition: transform ${theme.motionDurationMid}, opacity ${theme.motionDurationMid}; + /* Off-screen/hidden toasts must not capture clicks; only visible ones do. */ + pointer-events: none; &:after { content: ''; position: absolute; @@ -125,6 +132,7 @@ const StyledToastPresenter = styled.div<VisualProps>( .toast--visible { opacity: 1; transform: translateY(0); + pointer-events: auto; } `, ); diff --git a/superset-frontend/src/explore/components/SaveModal.test.tsx b/superset-frontend/src/explore/components/SaveModal.test.tsx index 1c79be556f5..e8d40e27dee 100644 --- a/superset-frontend/src/explore/components/SaveModal.test.tsx +++ b/superset-frontend/src/explore/components/SaveModal.test.tsx @@ -265,6 +265,14 @@ test('disables overwrite option for non-owner', () => { expect(getByRole('radio', { name: 'Save (Overwrite)' })).toBeDisabled(); }); +test('enables and selects overwrite option for owner', () => { + // Owners is a normalized array of user IDs, matching the current user + const { getByRole } = setup(); + const overwriteRadio = getByRole('radio', { name: 'Save (Overwrite)' }); + expect(overwriteRadio).toBeEnabled(); + expect(overwriteRadio).toBeChecked(); +}); + test('updates slice name and selected dashboard', async () => { const dashboardId = mockEvent.value; const saveDataset = jest.fn().mockResolvedValue(undefined); diff --git a/superset-frontend/src/explore/reducers/exploreReducer.test.ts b/superset-frontend/src/explore/reducers/exploreReducer.test.ts index 9c87137d0ea..26b8007d935 100644 --- a/superset-frontend/src/explore/reducers/exploreReducer.test.ts +++ b/superset-frontend/src/explore/reducers/exploreReducer.test.ts @@ -18,7 +18,7 @@ */ import exploreReducer, { ExploreState } from './exploreReducer'; -import { setStashFormData } from '../actions/exploreActions'; +import { setStashFormData, sliceUpdated } from '../actions/exploreActions'; import { QueryFormData } from '@superset-ui/core'; test('reset hiddenFormData on SET_STASH_FORM_DATA', () => { @@ -52,3 +52,49 @@ test('skips updates when the field is already updated on SET_STASH_FORM_DATA', ( const newState = exploreReducer(initialState, restoreAction); expect(newState).toBe(initialState); }); + +test('normalizes API owner objects to id array on SLICE_UPDATED', () => { + const initialState: ExploreState = { + form_data: {} as unknown as QueryFormData, + controls: {}, + slice: { slice_id: 1, owners: [1] }, + } as ExploreState; + // The chart API returns owners as objects: {id, first_name, last_name} + const action = sliceUpdated({ + slice_id: 1, + slice_name: 'My chart', + owners: [{ id: 5, first_name: 'Superset', last_name: 'Admin' }], + } as never) as Parameters<typeof exploreReducer>[1]; + const newState = exploreReducer(initialState, action); + expect(newState.slice?.owners).toEqual([5]); + expect(newState.metadata?.owners).toEqual(['Superset Admin']); +}); + +test('normalizes select control owners ({value, label}) on SLICE_UPDATED', () => { + const initialState: ExploreState = { + form_data: {} as unknown as QueryFormData, + controls: {}, + slice: { slice_id: 1, owners: [1] }, + } as ExploreState; + const action = sliceUpdated({ + slice_id: 1, + owners: [{ value: 7, label: 'Jane Doe' }], + } as never) as Parameters<typeof exploreReducer>[1]; + const newState = exploreReducer(initialState, action); + expect(newState.slice?.owners).toEqual([7]); + expect(newState.metadata?.owners).toEqual(['Jane Doe']); +}); + +test('keeps numeric owner ids on SLICE_UPDATED', () => { + const initialState: ExploreState = { + form_data: {} as unknown as QueryFormData, + controls: {}, + slice: { slice_id: 1, owners: [1] }, + } as ExploreState; + const action = sliceUpdated({ + slice_id: 1, + owners: [3, 4], + } as never) as Parameters<typeof exploreReducer>[1]; + const newState = exploreReducer(initialState, action); + expect(newState.slice?.owners).toEqual([3, 4]); +}); diff --git a/superset-frontend/src/explore/reducers/exploreReducer.ts b/superset-frontend/src/explore/reducers/exploreReducer.ts index 47b0bf9f01b..bd8a6719e5f 100644 --- a/superset-frontend/src/explore/reducers/exploreReducer.ts +++ b/superset-frontend/src/explore/reducers/exploreReducer.ts @@ -152,9 +152,13 @@ interface SetStashFormDataAction { isHidden: boolean; } -// Owner can be either a number (user ID) or an object with value/label -// This handles both Slice format (number[]) and select control format ({value, label}[]) -type OwnerItem = number | { value: number; label: string }; +// Owner can be a number (user ID), a select control option ({value, label}), +// or an API entity object ({id, first_name, last_name}). This handles the Slice +// format (number[]), select control format, and the raw chart API response format. +type OwnerItem = + | number + | { value: number; label: string } + | { id: number; first_name?: string; last_name?: string }; interface SliceUpdatedAction { type: typeof actions.SLICE_UPDATED; @@ -601,11 +605,20 @@ export default function exploreReducer( }, [actions.SLICE_UPDATED]() { const typedAction = action as SliceUpdatedAction; - // Handle owners that can be either number[] or Array<{value, label}> - const getOwnerId = (owner: OwnerItem): number => - typeof owner === 'number' ? owner : owner.value; - const getOwnerLabel = (owner: OwnerItem): string | null => - typeof owner === 'number' ? null : owner.label; + // Handle owners in number[], {value, label}[], or API {id, first_name, ...}[] form + const getOwnerId = (owner: OwnerItem): number => { + if (typeof owner === 'number') return owner; + return 'value' in owner ? owner.value : owner.id; + }; + const getOwnerLabel = (owner: OwnerItem): string | null => { + if (typeof owner === 'number') return null; + if ('value' in owner) return owner.label; + const name = [owner.first_name, owner.last_name] + .filter(Boolean) + .join(' ') + .trim(); + return name || null; + }; return { ...state, slice: {
