diff --git a/superset-frontend/src/explore/components/SaveModal.test.tsx b/superset-frontend/src/explore/components/SaveModal.test.tsx index ae01446affb4..1b809dde07cd 100644 --- a/superset-frontend/src/explore/components/SaveModal.test.tsx +++ b/superset-frontend/src/explore/components/SaveModal.test.tsx @@ -615,6 +615,172 @@ test('onTabChange correctly updates selectedTab via forceUpdate', () => { }); }); +const ownerUser = { + userId: 1, + username: 'testuser', + firstName: 'Test', + lastName: 'User', + isActive: true, + isAnonymous: false, + permissions: {}, + roles: { Alpha: [['can_write', 'Dashboard']] as [string, string][] }, + groups: [], +}; + +const makeMetadataDashboard = (id: number, title: string) => ({ + id, + dashboard_title: title, + owners: [{ id: 1, first_name: 'Test', last_name: 'User' }], + extra_owners: [], + roles: [], + url: `/superset/dashboard/${id}/`, + slug: null, + thumbnail_url: null, + published: true, + changed_by_name: 'Test User', + changed_by: { id: 1, first_name: 'Test', last_name: 'User' }, + changed_on: '2024-01-01', + charts: [], +}); + +test('pre-populates dashboard from metadata.dashboards when dashboardId prop is absent', async () => { + const dashboardId = 5; + const dashboardTitle = 'Chart Dashboard'; + + const myProps = { + ...defaultProps, + dashboardId: null, + metadata: { + dashboards: [{ id: dashboardId, dashboard_title: dashboardTitle }], + owners: ['Test User'], + created_on_humanized: '2 days ago', + changed_on_humanized: '1 day ago', + }, + user: ownerUser, + slice: { slice_id: 1, slice_name: 'My Chart', owners: [1] }, + dispatch: jest.fn(), + addDangerToast: jest.fn(), + }; + + const component = new TestSaveModal(myProps); + const mockFull = makeMetadataDashboard(dashboardId, dashboardTitle); + + component.loadDashboard = jest.fn().mockResolvedValue(mockFull); + component.loadTabs = jest.fn().mockResolvedValue([]); + + const stateUpdates: any[] = []; + component.setState = jest.fn((update: any) => { + stateUpdates.push(update); + }); + + try { + sessionStorage.clear(); + } catch (_) { + // ignore + } + + await component.componentDidMount(); + + expect(component.loadDashboard).toHaveBeenCalledWith(dashboardId); + expect(stateUpdates).toContainEqual({ + dashboard: { label: dashboardTitle, value: dashboardId }, + }); + expect(component.loadTabs).toHaveBeenCalledWith(dashboardId); +}); + +test('skips non-editable dashboards and picks the first editable one from metadata', async () => { + const editableId = 7; + const editableTitle = 'Editable Dashboard'; + + const myProps = { + ...defaultProps, + dashboardId: null, + metadata: { + dashboards: [ + { id: 6, dashboard_title: 'Not Mine' }, + { id: editableId, dashboard_title: editableTitle }, + ], + owners: ['Test User'], + created_on_humanized: '2 days ago', + changed_on_humanized: '1 day ago', + }, + user: ownerUser, + slice: { slice_id: 1, slice_name: 'My Chart', owners: [1] }, + dispatch: jest.fn(), + addDangerToast: jest.fn(), + }; + + const component = new TestSaveModal(myProps); + + const notMine = makeMetadataDashboard(6, 'Not Mine'); + notMine.owners = [{ id: 99, first_name: 'Other', last_name: 'Owner' }]; + const editable = makeMetadataDashboard(editableId, editableTitle); + + component.loadDashboard = jest + .fn() + .mockImplementation((id: number) => + Promise.resolve(id === 6 ? notMine : editable), + ); + component.loadTabs = jest.fn().mockResolvedValue([]); + + const stateUpdates: any[] = []; + component.setState = jest.fn((update: any) => { + stateUpdates.push(update); + }); + + try { + sessionStorage.clear(); + } catch (_) { + // ignore + } + + await component.componentDidMount(); + + expect(stateUpdates).toContainEqual({ + dashboard: { label: editableTitle, value: editableId }, + }); + expect(component.loadTabs).toHaveBeenCalledWith(editableId); +}); + +test('does not use metadata fallback when dashboardId prop is set', async () => { + const propDashboardId = 3; + const propDashboardTitle = 'Prop Dashboard'; + + const myProps = { + ...defaultProps, + dashboardId: propDashboardId, + metadata: { + dashboards: [{ id: 99, dashboard_title: 'Should Not Be Used' }], + owners: ['Test User'], + created_on_humanized: '2 days ago', + changed_on_humanized: '1 day ago', + }, + user: ownerUser, + slice: { slice_id: 1, slice_name: 'My Chart', owners: [1] }, + dispatch: jest.fn(), + addDangerToast: jest.fn(), + }; + + const component = new TestSaveModal(myProps); + const mockFull = makeMetadataDashboard(propDashboardId, propDashboardTitle); + + component.loadDashboard = jest.fn().mockResolvedValue(mockFull); + component.loadTabs = jest.fn().mockResolvedValue([]); + + const stateUpdates: any[] = []; + component.setState = jest.fn((update: any) => { + stateUpdates.push(update); + }); + + await component.componentDidMount(); + + expect(component.loadDashboard).toHaveBeenCalledWith(propDashboardId); + expect(component.loadDashboard).not.toHaveBeenCalledWith(99); + expect(stateUpdates).toContainEqual({ + dashboard: { label: propDashboardTitle, value: propDashboardId }, + }); +}); + test('chart placement logic finds row with available space', () => { // Test case 1: Row has space (8 + 4 = 12 <= 12) const positionJson1 = { diff --git a/superset-frontend/src/explore/components/SaveModal.tsx b/superset-frontend/src/explore/components/SaveModal.tsx index 9d6cbeab6c08..fce7c0cf5937 100755 --- a/superset-frontend/src/explore/components/SaveModal.tsx +++ b/superset-frontend/src/explore/components/SaveModal.tsx @@ -54,7 +54,11 @@ import { isUserAdmin, } from 'src/dashboard/util/permissionUtils'; import { setSaveChartModalVisibility } from 'src/explore/actions/saveModalActions'; -import { SaveActionType, ChartStatusType } from 'src/explore/types'; +import { + SaveActionType, + ChartStatusType, + ExplorePageInitialData, +} from 'src/explore/types'; import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; import { removeChartState, @@ -81,6 +85,7 @@ interface SaveModalProps extends RouteComponentProps { isVisible: boolean; dispatch: Dispatch; theme: SupersetTheme; + metadata?: ExplorePageInitialData['metadata']; } type SaveModalState = { @@ -162,6 +167,35 @@ class SaveModal extends Component { t('An error occurred while loading dashboard information.'), ); } + } else { + const metadataDashboards = this.props.metadata?.dashboards; + if (metadataDashboards?.length) { + // Fallback: the chart is already on one or more dashboards (from Explore API + // metadata). Pre-populate with the first dashboard the user can edit so the + // "Save & go to dashboard" button works out of the box. + try { + let editable: Dashboard | undefined; + for (const { id } of metadataDashboards) { + // eslint-disable-next-line no-await-in-loop + const result = await this.loadDashboard(id).catch(() => null); + if (result && canUserEditDashboard(result, this.props.user)) { + editable = result as Dashboard; + break; + } + } + if (editable) { + this.setState({ + dashboard: { + label: editable.dashboard_title, + value: editable.id, + }, + }); + await this.loadTabs(editable.id); + } + } catch (error) { + logging.warn(error); + } + } } } @@ -826,6 +860,7 @@ interface StateProps { dashboards: any; alert: any; isVisible: boolean; + metadata?: ExplorePageInitialData['metadata']; } function mapStateToProps({ @@ -841,6 +876,7 @@ function mapStateToProps({ dashboards: saveModal.dashboards, alert: saveModal.saveModalAlert, isVisible: saveModal.isVisible, + metadata: explore.metadata, }; }