sadpandajoe commented on code in PR #39461:
URL: https://github.com/apache/superset/pull/39461#discussion_r3191445042
##########
superset-frontend/src/explore/components/SaveModal.tsx:
##########
@@ -75,668 +191,599 @@ interface SaveModalProps extends RouteComponentProps {
dashboardId: '' | number | null;
isVisible: boolean;
dispatch: Dispatch;
- theme: SupersetTheme;
}
-type SaveModalState = {
- newSliceName?: string;
- datasetName: string;
- action: SaveActionType;
- isLoading: boolean;
- saveStatus?: string | null;
- dashboard?: { label: string; value: string | number };
- selectedTab?: { label: string; value: string | number };
- tabsData: TabTreeNode[];
-};
-
export const StyledModal = styled(Modal)`
.ant-modal-body {
overflow: visible;
}
`;
-class SaveModal extends Component<SaveModalProps, SaveModalState> {
- constructor(props: SaveModalProps) {
- super(props);
- this.state = {
- newSliceName: props.sliceName,
- datasetName: props.datasource?.name,
- action: this.canOverwriteSlice()
- ? ChartStatusType.overwrite
- : ChartStatusType.saveas,
- isLoading: false,
- dashboard: undefined,
- tabsData: [],
- selectedTab: undefined,
- };
- this.onDashboardChange = this.onDashboardChange.bind(this);
- this.onSliceNameChange = this.onSliceNameChange.bind(this);
- this.changeAction = this.changeAction.bind(this);
- this.saveOrOverwrite = this.saveOrOverwrite.bind(this);
- this.isNewDashboard = this.isNewDashboard.bind(this);
- this.onHide = this.onHide.bind(this);
- }
+const SaveModal = ({
+ addDangerToast,
+ actions,
+ form_data,
+ user,
+ alert: alertProp,
+ sliceName = '',
+ slice,
+ datasource,
+ dashboardId: dashboardIdProp,
+ isVisible,
+}: SaveModalProps) => {
+ const dispatch = useDispatch();
+ const history = useHistory();
+ const theme = useTheme();
+
+ const canOverwriteSlice = useCallback(
+ (): boolean =>
+ slice?.owners?.includes(user.userId) && !slice?.is_managed_externally,
+ [slice, user.userId],
+ );
- isNewDashboard(): boolean {
- const { dashboard } = this.state;
- return typeof dashboard?.value === 'string';
- }
+ const [newSliceName, setNewSliceName] = useState<string | undefined>(
+ sliceName,
+ );
+ const [datasetName, setDatasetName] = useState<string>(datasource?.name);
+ const [action, setAction] = useState<SaveActionType>(
+ canOverwriteSlice() ? ChartStatusType.overwrite : ChartStatusType.saveas,
+ );
+ const [isLoading, setIsLoading] = useState<boolean>(false);
+ const [dashboard, setDashboard] = useState<
+ { label: string; value: string | number } | undefined
+ >(undefined);
+ const [tabsData, setTabsData] = useState<TabTreeNode[]>([]);
+ const [selectedTab, setSelectedTab] = useState<
+ { label: string; value: string | number } | undefined
+ >(undefined);
+
+ const isNewDashboard = useCallback(
+ (): boolean => typeof dashboard?.value === 'string',
+ [dashboard?.value],
+ );
- canOverwriteSlice(): boolean {
- return (
- this.props.slice?.owners?.includes(this.props.user.userId) &&
- !this.props.slice?.is_managed_externally
- );
- }
+ const loadDashboard = useCallback(async (id: number) => {
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/dashboard/${id}`,
+ });
+ return response.json.result;
+ }, []);
- async componentDidMount() {
- let { dashboardId } = this.props;
- if (!dashboardId) {
- let lastDashboard = null;
+ const loadTabs = useCallback(
+ async (dashboardId: number) => {
try {
- lastDashboard = sessionStorage.getItem(SK_DASHBOARD_ID);
- } catch (error) {
- // continue regardless of error
- }
- dashboardId = lastDashboard && parseInt(lastDashboard, 10);
- }
- if (dashboardId) {
- try {
- const result = (await this.loadDashboard(dashboardId)) as Dashboard;
- if (canUserEditDashboard(result, this.props.user)) {
- this.setState({
- dashboard: { label: result.dashboard_title, value: result.id },
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/dashboard/${dashboardId}/tabs`,
+ });
+
+ const { result } = response.json;
+ if (!result || !Array.isArray(result.tab_tree)) {
+ logging.warn('Invalid tabs response format');
+ setTabsData([]);
+ return [];
+ }
+ const tabTree = result.tab_tree;
+ const gridTabIds = new Set<string>();
+ const convertToTreeData = (nodes: TabNode[]): TabTreeNode[] =>
+ nodes.map(node => {
+ const isGridTab =
+ Array.isArray(node.parents) && node.parents.includes('GRID_ID');
+ if (isGridTab) {
+ gridTabIds.add(node.value);
+ }
+ return {
+ value: node.value,
+ title: node.title,
+ key: node.value,
+ children:
+ node.children && node.children.length > 0
+ ? convertToTreeData(node.children)
+ : undefined,
+ };
});
- await this.loadTabs(dashboardId);
+
+ const treeData = convertToTreeData(tabTree);
+
+ // Add "Out of tab" option at the beginning
+ if (gridTabIds.size > 0) {
+ const tabsDataWithOutOfTab = [
+ {
+ value: 'OUT_OF_TAB',
+ title: 'Out of tab',
+ key: 'OUT_OF_TAB',
+ children: undefined,
+ },
+ ...treeData,
+ ];
+
+ setTabsData(tabsDataWithOutOfTab);
+ setSelectedTab({ value: 'OUT_OF_TAB', label: 'Out of tab' });
+ } else if (treeData.length > 0) {
+ const firstTab = treeData[0];
+ setTabsData(treeData);
+ setSelectedTab({ value: firstTab.value, label: firstTab.title });
+ } else {
+ setTabsData([]);
}
+
+ return treeData;
} catch (error) {
- logging.warn(error);
- this.props.addDangerToast(
- t('An error occurred while loading dashboard information.'),
- );
+ logging.error('Error loading tabs:', error);
+ setTabsData([]);
+ return [];
}
- }
- }
-
- handleDatasetNameChange = (e: FormEvent<HTMLInputElement>) => {
- // @ts-expect-error
- this.setState({ datasetName: e.target.value });
- };
-
- onSliceNameChange(event: ChangeEvent<HTMLInputElement>) {
- this.setState({ newSliceName: event.target.value });
- }
+ },
+ [setTabsData, setSelectedTab],
+ );
- onDashboardChange = async (
- dashboard:
- | {
- label: string;
- value: string | number;
+ useEffect(() => {
+ const initializeDashboard = async () => {
+ let dashboardId = dashboardIdProp;
+ if (!dashboardId) {
+ let lastDashboard = null;
+ try {
+ lastDashboard = sessionStorage.getItem(SK_DASHBOARD_ID);
+ } catch (error) {
+ // continue regardless of error
}
- | undefined,
- ) => {
- this.setState({
- dashboard,
- tabsData: [],
- selectedTab: undefined,
- });
+ dashboardId = lastDashboard && parseInt(lastDashboard, 10);
+ }
+ if (dashboardId) {
+ try {
+ const result = (await loadDashboard(dashboardId)) as Dashboard;
+ if (canUserEditDashboard(result, user)) {
+ setDashboard({ label: result.dashboard_title, value: result.id });
+ await loadTabs(dashboardId);
+ }
+ } catch (error) {
+ logging.warn(error);
+ addDangerToast(
+ t('An error occurred while loading dashboard information.'),
+ );
+ }
+ }
+ };
+ initializeDashboard();
+ }, [dashboardIdProp, user, loadDashboard, loadTabs, addDangerToast]);
+
+ const handleDatasetNameChange = useCallback(
+ (e: FormEvent<HTMLInputElement>) => {
+ // @ts-expect-error
+ setDatasetName(e.target.value);
+ },
+ [],
+ );
- if (dashboard && typeof dashboard.value === 'number') {
- await this.loadTabs(dashboard.value);
- }
- };
- changeAction(action: SaveActionType) {
- this.setState({ action });
- }
+ const onSliceNameChange = useCallback(
+ (event: ChangeEvent<HTMLInputElement>) => {
+ setNewSliceName(event.target.value);
+ },
+ [],
+ );
- onHide() {
- this.props.dispatch(setSaveChartModalVisibility(false));
- }
+ const onDashboardChange = useCallback(
Review Comment:
We should add a test for this. Clearing the dashboard select sends `null`
and used to crash on `newDashboard.value`. Worth asserting that calling
`onDashboardChange(null)` doesn't throw and clears the dashboard state.
##########
superset-frontend/src/explore/components/SaveModal.tsx:
##########
@@ -75,668 +191,599 @@ interface SaveModalProps extends RouteComponentProps {
dashboardId: '' | number | null;
isVisible: boolean;
dispatch: Dispatch;
- theme: SupersetTheme;
}
-type SaveModalState = {
- newSliceName?: string;
- datasetName: string;
- action: SaveActionType;
- isLoading: boolean;
- saveStatus?: string | null;
- dashboard?: { label: string; value: string | number };
- selectedTab?: { label: string; value: string | number };
- tabsData: TabTreeNode[];
-};
-
export const StyledModal = styled(Modal)`
.ant-modal-body {
overflow: visible;
}
`;
-class SaveModal extends Component<SaveModalProps, SaveModalState> {
- constructor(props: SaveModalProps) {
- super(props);
- this.state = {
- newSliceName: props.sliceName,
- datasetName: props.datasource?.name,
- action: this.canOverwriteSlice()
- ? ChartStatusType.overwrite
- : ChartStatusType.saveas,
- isLoading: false,
- dashboard: undefined,
- tabsData: [],
- selectedTab: undefined,
- };
- this.onDashboardChange = this.onDashboardChange.bind(this);
- this.onSliceNameChange = this.onSliceNameChange.bind(this);
- this.changeAction = this.changeAction.bind(this);
- this.saveOrOverwrite = this.saveOrOverwrite.bind(this);
- this.isNewDashboard = this.isNewDashboard.bind(this);
- this.onHide = this.onHide.bind(this);
- }
+const SaveModal = ({
+ addDangerToast,
+ actions,
+ form_data,
+ user,
+ alert: alertProp,
+ sliceName = '',
+ slice,
+ datasource,
+ dashboardId: dashboardIdProp,
+ isVisible,
+}: SaveModalProps) => {
+ const dispatch = useDispatch();
+ const history = useHistory();
+ const theme = useTheme();
+
+ const canOverwriteSlice = useCallback(
+ (): boolean =>
+ slice?.owners?.includes(user.userId) && !slice?.is_managed_externally,
+ [slice, user.userId],
+ );
- isNewDashboard(): boolean {
- const { dashboard } = this.state;
- return typeof dashboard?.value === 'string';
- }
+ const [newSliceName, setNewSliceName] = useState<string | undefined>(
+ sliceName,
+ );
+ const [datasetName, setDatasetName] = useState<string>(datasource?.name);
+ const [action, setAction] = useState<SaveActionType>(
+ canOverwriteSlice() ? ChartStatusType.overwrite : ChartStatusType.saveas,
+ );
+ const [isLoading, setIsLoading] = useState<boolean>(false);
+ const [dashboard, setDashboard] = useState<
+ { label: string; value: string | number } | undefined
+ >(undefined);
+ const [tabsData, setTabsData] = useState<TabTreeNode[]>([]);
+ const [selectedTab, setSelectedTab] = useState<
+ { label: string; value: string | number } | undefined
+ >(undefined);
+
+ const isNewDashboard = useCallback(
+ (): boolean => typeof dashboard?.value === 'string',
+ [dashboard?.value],
+ );
- canOverwriteSlice(): boolean {
- return (
- this.props.slice?.owners?.includes(this.props.user.userId) &&
- !this.props.slice?.is_managed_externally
- );
- }
+ const loadDashboard = useCallback(async (id: number) => {
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/dashboard/${id}`,
+ });
+ return response.json.result;
+ }, []);
- async componentDidMount() {
- let { dashboardId } = this.props;
- if (!dashboardId) {
- let lastDashboard = null;
+ const loadTabs = useCallback(
+ async (dashboardId: number) => {
try {
- lastDashboard = sessionStorage.getItem(SK_DASHBOARD_ID);
- } catch (error) {
- // continue regardless of error
- }
- dashboardId = lastDashboard && parseInt(lastDashboard, 10);
- }
- if (dashboardId) {
- try {
- const result = (await this.loadDashboard(dashboardId)) as Dashboard;
- if (canUserEditDashboard(result, this.props.user)) {
- this.setState({
- dashboard: { label: result.dashboard_title, value: result.id },
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/dashboard/${dashboardId}/tabs`,
+ });
+
+ const { result } = response.json;
+ if (!result || !Array.isArray(result.tab_tree)) {
+ logging.warn('Invalid tabs response format');
+ setTabsData([]);
+ return [];
+ }
+ const tabTree = result.tab_tree;
+ const gridTabIds = new Set<string>();
+ const convertToTreeData = (nodes: TabNode[]): TabTreeNode[] =>
+ nodes.map(node => {
+ const isGridTab =
+ Array.isArray(node.parents) && node.parents.includes('GRID_ID');
+ if (isGridTab) {
+ gridTabIds.add(node.value);
+ }
+ return {
+ value: node.value,
+ title: node.title,
+ key: node.value,
+ children:
+ node.children && node.children.length > 0
+ ? convertToTreeData(node.children)
+ : undefined,
+ };
});
- await this.loadTabs(dashboardId);
+
+ const treeData = convertToTreeData(tabTree);
+
+ // Add "Out of tab" option at the beginning
+ if (gridTabIds.size > 0) {
+ const tabsDataWithOutOfTab = [
+ {
+ value: 'OUT_OF_TAB',
+ title: 'Out of tab',
+ key: 'OUT_OF_TAB',
+ children: undefined,
+ },
+ ...treeData,
+ ];
+
+ setTabsData(tabsDataWithOutOfTab);
+ setSelectedTab({ value: 'OUT_OF_TAB', label: 'Out of tab' });
+ } else if (treeData.length > 0) {
+ const firstTab = treeData[0];
+ setTabsData(treeData);
+ setSelectedTab({ value: firstTab.value, label: firstTab.title });
+ } else {
+ setTabsData([]);
}
+
+ return treeData;
} catch (error) {
- logging.warn(error);
- this.props.addDangerToast(
- t('An error occurred while loading dashboard information.'),
- );
+ logging.error('Error loading tabs:', error);
+ setTabsData([]);
+ return [];
}
- }
- }
-
- handleDatasetNameChange = (e: FormEvent<HTMLInputElement>) => {
- // @ts-expect-error
- this.setState({ datasetName: e.target.value });
- };
-
- onSliceNameChange(event: ChangeEvent<HTMLInputElement>) {
- this.setState({ newSliceName: event.target.value });
- }
+ },
+ [setTabsData, setSelectedTab],
+ );
- onDashboardChange = async (
- dashboard:
- | {
- label: string;
- value: string | number;
+ useEffect(() => {
+ const initializeDashboard = async () => {
+ let dashboardId = dashboardIdProp;
+ if (!dashboardId) {
+ let lastDashboard = null;
+ try {
+ lastDashboard = sessionStorage.getItem(SK_DASHBOARD_ID);
+ } catch (error) {
+ // continue regardless of error
}
- | undefined,
- ) => {
- this.setState({
- dashboard,
- tabsData: [],
- selectedTab: undefined,
- });
+ dashboardId = lastDashboard && parseInt(lastDashboard, 10);
+ }
+ if (dashboardId) {
+ try {
+ const result = (await loadDashboard(dashboardId)) as Dashboard;
+ if (canUserEditDashboard(result, user)) {
+ setDashboard({ label: result.dashboard_title, value: result.id });
+ await loadTabs(dashboardId);
+ }
+ } catch (error) {
+ logging.warn(error);
+ addDangerToast(
+ t('An error occurred while loading dashboard information.'),
+ );
+ }
+ }
+ };
+ initializeDashboard();
+ }, [dashboardIdProp, user, loadDashboard, loadTabs, addDangerToast]);
+
+ const handleDatasetNameChange = useCallback(
+ (e: FormEvent<HTMLInputElement>) => {
+ // @ts-expect-error
+ setDatasetName(e.target.value);
+ },
+ [],
+ );
- if (dashboard && typeof dashboard.value === 'number') {
- await this.loadTabs(dashboard.value);
- }
- };
- changeAction(action: SaveActionType) {
- this.setState({ action });
- }
+ const onSliceNameChange = useCallback(
+ (event: ChangeEvent<HTMLInputElement>) => {
+ setNewSliceName(event.target.value);
+ },
+ [],
+ );
- onHide() {
- this.props.dispatch(setSaveChartModalVisibility(false));
- }
+ const onDashboardChange = useCallback(
+ async (value: SelectValue) => {
+ // AsyncSelect's onChange is typed with the broad antd SelectValue, but
+ // this Select is single-mode with labeled options, so the runtime value
+ // is either a { label, value } object or null/undefined when cleared.
+ const newDashboard =
+ value && typeof value === 'object' && !Array.isArray(value)
+ ? (value as { label: string; value: string | number })
+ : undefined;
+ setDashboard(newDashboard);
+ setTabsData([]);
+ setSelectedTab(undefined);
+
+ if (newDashboard && typeof newDashboard.value === 'number') {
+ await loadTabs(newDashboard.value);
+ }
+ },
+ [loadTabs],
+ );
- handleRedirect = (windowLocationSearch: string, chart: any) => {
- const searchParams = new URLSearchParams(windowLocationSearch);
- searchParams.delete('form_data_key');
- searchParams.set('slice_id', chart.id.toString());
- return searchParams;
- };
+ const changeAction = useCallback((newAction: SaveActionType) => {
+ setAction(newAction);
+ }, []);
- async saveOrOverwrite(gotodash: boolean) {
- this.setState({ isLoading: true });
- const tableState = this.props.form_data?.table_state;
- const sliceId = this.props.slice?.slice_id;
- const vizType = this.props.form_data?.viz_type;
- if (sliceId && vizType && tableState) {
- this.props.dispatch(updateChartState(sliceId, vizType, tableState));
- }
+ const onHide = useCallback(() => {
+ dispatch(setSaveChartModalVisibility(false));
+ }, [dispatch]);
- // Create or retrieve dashboard
- type DashboardGetResponse = {
- id: number;
- url: string;
- dashboard_title: string;
- };
+ const handleRedirect = useCallback(
+ (windowLocationSearch: string, chart: { id: number }) =>
+ createRedirectParams(windowLocationSearch, chart, action),
+ [action],
+ );
- try {
- if (this.props.datasource?.type === DatasourceType.Query) {
- const { schema, sql, database } = this.props.datasource;
- const { templateParams } = this.props.datasource;
-
- await this.props.actions.saveDataset({
- schema,
- sql,
- database,
- templateParams,
- datasourceName: this.state.datasetName,
- });
+ const addChartToDashboardTab = useCallback(
+ async (
+ dashboardId: number,
+ chartId: number,
+ tabId: string,
+ sliceNameParam: string | undefined,
+ ) => {
+ try {
+ await addChartToDashboard(dashboardId, chartId, tabId, sliceNameParam);
+ } catch (error) {
+ throw new Error(`Error adding chart to dashboard tab: ${error}`);
}
+ },
+ [],
+ );
- // Get chart dashboards
- let sliceDashboards: number[] = [];
- if (this.props.slice && this.state.action === 'overwrite') {
- sliceDashboards = await this.props.actions.getSliceDashboards(
- this.props.slice,
- );
+ const saveOrOverwrite = useCallback(
+ async (gotodash: boolean) => {
+ setIsLoading(true);
+ const tableState = form_data?.table_state;
+ const sliceId = slice?.slice_id;
+ const vizType = form_data?.viz_type;
+ if (sliceId && vizType && tableState) {
+ dispatch(updateChartState(sliceId, vizType, tableState));
}
- const formData = this.props.form_data || {};
- delete formData.url_params;
+ // Create or retrieve dashboard
+ type DashboardGetResponse = {
+ id: number;
+ url: string;
+ dashboard_title: string;
+ };
- let dashboard: DashboardGetResponse | null = null;
- let selectedTabId: string | undefined;
- if (this.state.dashboard) {
- let validId = this.state.dashboard.value;
- if (this.isNewDashboard()) {
- const response = await this.props.actions.createDashboard(
- this.state.dashboard.label,
- );
- validId = response.id;
+ try {
+ if (datasource?.type === DatasourceType.Query) {
+ const { schema, sql, database } = datasource;
+ const { templateParams } = datasource;
+
+ await actions.saveDataset({
+ schema,
+ sql,
+ database,
+ templateParams,
+ datasourceName: datasetName,
+ });
}
- try {
- dashboard = await this.loadDashboard(validId as number);
- } catch (error) {
- this.props.actions.saveSliceFailed();
- return;
+ // Get chart dashboards
+ let sliceDashboards: number[] = [];
+ if (slice && action === 'overwrite') {
+ sliceDashboards = await actions.getSliceDashboards(slice);
}
- if (isDefined(dashboard) && isDefined(dashboard?.id)) {
- sliceDashboards = sliceDashboards.includes(dashboard.id)
- ? sliceDashboards
- : [...sliceDashboards, dashboard.id];
- formData.dashboards = sliceDashboards;
- if (
- this.state.action === ChartStatusType.saveas &&
- this.state.selectedTab?.value !== 'OUT_OF_TAB'
- ) {
- selectedTabId = this.state.selectedTab?.value as string;
+ const formData = form_data || {};
+ delete formData.url_params;
+
+ let dashboardResult: DashboardGetResponse | null = null;
+ let selectedTabId: string | undefined;
+ if (dashboard) {
+ let validId = dashboard.value;
+ if (isNewDashboard()) {
+ const response = await actions.createDashboard(dashboard.label);
+ validId = response.id;
}
- }
- }
- // Sets the form data
- this.props.actions.setFormData({ ...formData });
-
- // Update or create slice
- let value: { id: number };
- if (this.state.action === 'overwrite') {
- value = await this.props.actions.updateSlice(
- this.props.slice,
- this.state.newSliceName,
- sliceDashboards,
- dashboard
- ? {
- title: dashboard.dashboard_title,
- new: this.isNewDashboard(),
- }
- : null,
- );
- } else {
- value = await this.props.actions.createSlice(
- this.state.newSliceName,
- sliceDashboards,
- dashboard
- ? {
- title: dashboard.dashboard_title,
- new: this.isNewDashboard(),
- }
- : null,
- );
- if (dashboard && selectedTabId) {
try {
- await this.addChartToDashboardTab(
- dashboard.id,
- value.id,
- selectedTabId,
- this.state.newSliceName,
- );
+ dashboardResult = await loadDashboard(validId as number);
} catch (error) {
- logging.error('Error adding chart to dashboard tab:', error);
- this.props.addDangerToast(
- t('Chart was saved but could not be added to the selected tab.'),
- );
+ actions.saveSliceFailed();
+ return;
+ }
+
+ if (isDefined(dashboardResult) && isDefined(dashboardResult?.id)) {
+ sliceDashboards = sliceDashboards.includes(dashboardResult.id)
+ ? sliceDashboards
+ : [...sliceDashboards, dashboardResult.id];
+ formData.dashboards = sliceDashboards;
+ if (
+ action === ChartStatusType.saveas &&
+ selectedTab?.value !== 'OUT_OF_TAB'
+ ) {
+ selectedTabId = selectedTab?.value as string;
+ }
}
}
- }
- try {
- if (dashboard) {
- sessionStorage.setItem(SK_DASHBOARD_ID, `${dashboard.id}`);
+ // Sets the form data
+ actions.setFormData({ ...formData });
+
+ // Update or create slice
+ let value: { id: number };
+ if (action === 'overwrite') {
+ value = await actions.updateSlice(
+ slice,
+ newSliceName,
+ sliceDashboards,
+ dashboardResult
+ ? {
+ title: dashboardResult.dashboard_title,
+ new: isNewDashboard(),
+ }
+ : null,
+ );
} else {
- sessionStorage.removeItem(SK_DASHBOARD_ID);
+ value = await actions.createSlice(
+ newSliceName,
+ sliceDashboards,
+ dashboardResult
+ ? {
+ title: dashboardResult.dashboard_title,
+ new: isNewDashboard(),
+ }
+ : null,
+ );
+ if (dashboardResult && selectedTabId) {
+ try {
+ await addChartToDashboardTab(
+ dashboardResult.id,
+ value.id,
+ selectedTabId,
+ newSliceName,
+ );
+ } catch (error) {
+ logging.error('Error adding chart to dashboard tab:', error);
+ addDangerToast(
+ t(
+ 'Chart was saved but could not be added to the selected
tab.',
+ ),
+ );
+ }
+ }
}
- } catch (error) {
- // continue regardless of error
- }
- // Go to new dashboard url
- if (gotodash && dashboard) {
- let { url } = dashboard;
- if (this.state.selectedTab?.value) {
- url += `#${this.state.selectedTab.value}`;
+ try {
+ if (dashboardResult) {
+ sessionStorage.setItem(SK_DASHBOARD_ID, `${dashboardResult.id}`);
+ } else {
+ sessionStorage.removeItem(SK_DASHBOARD_ID);
+ }
+ } catch (error) {
+ // continue regardless of error
}
- this.props.dispatch(removeChartState(value.id));
- this.props.history.push(url);
- return;
- }
- const searchParams = this.handleRedirect(window.location.search, value);
- this.props.history.replace(`/explore/?${searchParams.toString()}`, {
- saveAction: this.state.action,
- });
-
- this.setState({ isLoading: false });
- this.onHide();
- } finally {
- this.setState({ isLoading: false });
- }
- }
- /* Adds a chart to the specified dashboard tab. If an existing row has
space, the chart is added there; otherwise, a new row is created.
- * @param {number} dashboardId - ID of the dashboard.
- * @param {number} chartId - ID of the chart to add.
- * @param {string} tabId - ID of the dashboard tab where the chart is added.
- * @param {string | undefined} sliceName - Chart name
- */
- addChartToDashboardTab = async (
- dashboardId: number,
- chartId: number,
- tabId: string,
- sliceName: string | undefined,
- ) => {
- try {
- const dashboardResponse = await SupersetClient.get({
- endpoint: `/api/v1/dashboard/${dashboardId}`,
- });
-
- const dashboard = dashboardResponse.json.result;
-
- let positionJson = dashboard.position_json;
- if (typeof positionJson === 'string') {
- positionJson = JSON.parse(positionJson);
- }
- positionJson = positionJson || {};
-
- const chartKey = `CHART-${chartId}`;
-
- // Find a row in the tab with available space
- const tabChildren = positionJson[tabId]?.children || [];
- let targetRowKey: string | null = null;
-
- for (const childKey of tabChildren) {
- const child = positionJson[childKey];
- if (child?.type === 'ROW') {
- const rowChildren = child.children || [];
- const totalWidth = rowChildren.reduce((sum: number, key: string) => {
- const component = positionJson[key];
- return sum + (component?.meta?.width || 0);
- }, 0);
-
- if (totalWidth + CHART_WIDTH <= GRID_COLUMN_COUNT) {
- targetRowKey = childKey;
- break;
+ // Go to new dashboard url
+ if (gotodash && dashboardResult) {
+ let { url } = dashboardResult;
+ // OUT_OF_TAB is a synthetic 'no tab' sentinel — it isn't a real
+ // dashboard anchor, so skip appending it as a URL fragment.
+ if (selectedTab?.value && selectedTab.value !== 'OUT_OF_TAB') {
+ url += `#${selectedTab.value}`;
}
+ dispatch(removeChartState(value.id));
+ history.push(url);
+ return;
}
- }
-
- const updatedPositionJson = { ...positionJson };
-
- // Create a new row if no existing row has space
- if (!targetRowKey) {
- targetRowKey = `ROW-${nanoid()}`;
- updatedPositionJson[targetRowKey] = {
- type: 'ROW',
- id: targetRowKey,
- children: [],
- parents: ['ROOT_ID', 'GRID_ID', tabId],
- meta: {
- background: 'BACKGROUND_TRANSPARENT',
- },
- };
+ const searchParams = handleRedirect(window.location.search, value);
+ history.replace(`/explore/?${searchParams.toString()}`, {
Review Comment:
We should add a test for this. We should verify that `history.replace` is
called with `{ saveAction: action }` to prevent any regressions.
##########
superset-frontend/src/components/CopyToClipboard/index.tsx:
##########
@@ -16,126 +16,137 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { Component, cloneElement, ReactElement } from 'react';
+import { cloneElement, isValidElement, ReactElement, useCallback } from
'react';
import { t } from '@apache-superset/core/translation';
import { css, SupersetTheme } from '@apache-superset/core/theme';
import copyTextToClipboard from 'src/utils/copy';
import { Tooltip } from '@superset-ui/core/components';
import withToasts from '../MessageToasts/withToasts';
import type { CopyToClipboardProps } from './types';
-const defaultProps: Partial<CopyToClipboardProps> = {
- copyNode: <span>{t('Copy')}</span>,
- onCopyEnd: () => {},
- shouldShowText: true,
- wrapped: true,
- tooltipText: t('Copy to clipboard'),
- hideTooltip: false,
-};
+function CopyToClip({
+ copyNode = <span>{t('Copy')}</span>,
+ onCopyEnd = () => {},
+ shouldShowText = true,
+ wrapped = true,
+ tooltipText = t('Copy to clipboard'),
+ hideTooltip = false,
+ disabled,
+ getText,
+ text,
+ addSuccessToast,
+ addDangerToast,
+}: CopyToClipboardProps) {
+ const copyToClipboard = useCallback(
+ (textToCopy: Promise<string>) => {
+ copyTextToClipboard(() => textToCopy)
+ .then(() => {
+ addSuccessToast(t('Copied to clipboard!'));
+ })
+ .catch(() => {
+ addDangerToast(
+ t(
+ 'Sorry, your browser does not support copying. Use Ctrl / Cmd +
C!',
+ ),
+ );
+ })
+ .finally(() => {
+ if (onCopyEnd) onCopyEnd();
+ });
+ },
+ [addSuccessToast, addDangerToast, onCopyEnd],
+ );
-class CopyToClip extends Component<CopyToClipboardProps> {
- static defaultProps = defaultProps;
-
- constructor(props: CopyToClipboardProps) {
- super(props);
- this.copyToClipboard = this.copyToClipboard.bind(this);
- this.onClick = this.onClick.bind(this);
- }
-
- onClick() {
- if (this.props.disabled) {
+ const onClick = useCallback(() => {
+ if (disabled) {
return;
}
- if (this.props.getText) {
- this.props.getText((d: string) => {
- this.copyToClipboard(Promise.resolve(d));
+ if (getText) {
+ getText((d: string) => {
+ copyToClipboard(Promise.resolve(d));
});
} else {
- this.copyToClipboard(Promise.resolve(this.props.text || ''));
+ copyToClipboard(Promise.resolve(text || ''));
}
- }
-
- getDecoratedCopyNode() {
- const copyNode = this.props.copyNode as ReactElement;
- const { disabled } = this.props;
- return cloneElement(copyNode, {
- style: {
- ...copyNode.props.style,
- cursor: disabled ? 'not-allowed' : 'pointer',
- },
- onClick: disabled ? undefined : this.onClick,
- 'aria-disabled': disabled || undefined,
- tabIndex: disabled ? -1 : copyNode.props.tabIndex,
- });
- }
+ }, [disabled, getText, text, copyToClipboard]);
- copyToClipboard(textToCopy: Promise<string>) {
- copyTextToClipboard(() => textToCopy)
- .then(() => {
- this.props.addSuccessToast(t('Copied to clipboard!'));
- })
- .catch(() => {
- this.props.addDangerToast(
- t(
- 'Sorry, your browser does not support copying. Use Ctrl / Cmd +
C!',
- ),
- );
- })
- .finally(() => {
- if (this.props.onCopyEnd) this.props.onCopyEnd();
+ const getDecoratedCopyNode = useCallback(() => {
+ const cursor = disabled ? 'not-allowed' : 'pointer';
+ if (isValidElement(copyNode)) {
Review Comment:
We should add a test for this. Passing a string or number `copyNode` used to
crash `cloneElement`. A test that renders `<CopyToClipboard copyNode="text" />`
and asserts no throw would prevent the guard from being removed in a future
cleanup.
##########
superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx:
##########
@@ -89,168 +89,199 @@ const TabTitle = styled.span`
text-transform: none;
`;
-const AddTabIconWrapper = styled.span`
- display: inline-flex;
- vertical-align: middle;
-`;
-
// Get the user's OS
const userOS = detectOS();
type TabbedSqlEditorsProps = ReturnType<typeof mergeProps>;
-class TabbedSqlEditors extends PureComponent<TabbedSqlEditorsProps> {
- constructor(props: TabbedSqlEditorsProps) {
- super(props);
- this.removeQueryEditor = this.removeQueryEditor.bind(this);
- this.handleSelect = this.handleSelect.bind(this);
- this.handleEdit = this.handleEdit.bind(this);
- }
+function TabbedSqlEditors({
+ actions,
+ queryEditors = DEFAULT_PROPS.queryEditors,
+ queries,
+ tabHistory,
+ displayLimit,
+ offline = DEFAULT_PROPS.offline,
+ defaultQueryLimit,
+ maxRow,
+ saveQueryWarning = DEFAULT_PROPS.saveQueryWarning,
+ scheduleQueryWarning = DEFAULT_PROPS.scheduleQueryWarning,
+}: TabbedSqlEditorsProps) {
+ const activeQueryEditor = useMemo(() => {
+ if (tabHistory.length === 0) {
+ return queryEditors[0];
+ }
+ const qeid = tabHistory[tabHistory.length - 1];
+ return queryEditors.find(qe => qe.id === qeid) || null;
+ }, [tabHistory, queryEditors]);
+
+ // Track the last persisted resultsKey we fetched, so the effect retries when
+ // the active query editor resolves after mount (or its latest query changes)
+ // but dedupes when the same resultsKey has already been fetched.
+ const fetchedResultsKeyRef = useRef<string | null>(null);
Review Comment:
We should add a test for this. The earlier mount-only guard silently skipped
persisted-results fetching when `activeQueryEditor` hydrated late. Worth
verifying `fetchQueryResults` fires once when `resultsKey` resolves after
mount, and dedupes when it doesn't change.
##########
superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.tsx:
##########
@@ -154,71 +142,54 @@ function optionsForSelect(props:
AdhocFilterControlProps): FilterOption[] {
);
}
-class AdhocFilterControl extends Component<
- AdhocFilterControlProps,
- AdhocFilterControlState
-> {
- optionRenderer: (option: FilterOption) => JSX.Element;
- valueRenderer: (adhocFilter: AdhocFilter, index: number) => JSX.Element;
-
- constructor(props: AdhocFilterControlProps) {
- super(props);
- this.onRemoveFilter = this.onRemoveFilter.bind(this);
- this.onNewFilter = this.onNewFilter.bind(this);
- this.onFilterEdit = this.onFilterEdit.bind(this);
- this.moveLabel = this.moveLabel.bind(this);
- this.onChange = this.onChange.bind(this);
- this.mapOption = this.mapOption.bind(this);
- this.getMetricExpression = this.getMetricExpression.bind(this);
- this.removeFilter = this.removeFilter.bind(this);
-
- const filters = (this.props.value || []).map(filter =>
+function AdhocFilterControl({
+ label,
+ name = '',
+ sections,
+ operators,
+ onChange = () => {},
+ value,
+ datasource,
+ columns = [],
+ savedMetrics = [],
+ selectedMetrics = [],
+ canDelete,
+}: AdhocFilterControlProps) {
+ const [values, setValues] = useState<AdhocFilter[]>(() =>
+ (value || []).map(filter =>
isDictionaryForAdhocFilter(filter) ? new AdhocFilter(filter) : filter,
- );
+ ),
+ );
+ const [partitionColumn, setPartitionColumn] = useState<string | null>(null);
- this.optionRenderer = option => <FilterDefinitionOption option={option} />;
- this.valueRenderer = (adhocFilter, index) => (
- <AdhocFilterOption
- key={index}
- index={index}
- adhocFilter={adhocFilter}
- onFilterEdit={this.onFilterEdit}
- options={this.state.options}
- sections={this.props.sections}
- operators={this.props.operators as Operators[] | undefined}
- datasource={this.props.datasource}
- onRemoveFilter={e => {
- e.stopPropagation();
- this.onRemoveFilter(index);
- }}
- onMoveLabel={this.moveLabel}
- onDropLabel={() => this.props.onChange?.(this.state.values)}
- partitionColumn={this.state.partitionColumn}
- />
- );
- this.state = {
- values: filters,
- options: optionsForSelect(this.props),
- partitionColumn: null,
- };
- }
+ const options = useMemo(
+ () =>
+ optionsForSelect({
+ columns,
+ selectedMetrics,
+ savedMetrics,
+ }),
+ [columns, selectedMetrics, savedMetrics],
+ );
- componentDidMount() {
- const { datasource } = this.props;
+ useEffect(() => {
+ // Clear stale partition state before (re)resolving; only 1-partition-key
Review Comment:
We should add a test for this. Switching from a single-partition datasource
to a zero/multi-partition one should clear `partitionColumn`, otherwise the
"latest partition" operator leaks across datasources.
##########
superset-frontend/src/explore/components/SaveModal.tsx:
##########
@@ -75,668 +191,599 @@ interface SaveModalProps extends RouteComponentProps {
dashboardId: '' | number | null;
isVisible: boolean;
dispatch: Dispatch;
- theme: SupersetTheme;
}
-type SaveModalState = {
- newSliceName?: string;
- datasetName: string;
- action: SaveActionType;
- isLoading: boolean;
- saveStatus?: string | null;
- dashboard?: { label: string; value: string | number };
- selectedTab?: { label: string; value: string | number };
- tabsData: TabTreeNode[];
-};
-
export const StyledModal = styled(Modal)`
.ant-modal-body {
overflow: visible;
}
`;
-class SaveModal extends Component<SaveModalProps, SaveModalState> {
- constructor(props: SaveModalProps) {
- super(props);
- this.state = {
- newSliceName: props.sliceName,
- datasetName: props.datasource?.name,
- action: this.canOverwriteSlice()
- ? ChartStatusType.overwrite
- : ChartStatusType.saveas,
- isLoading: false,
- dashboard: undefined,
- tabsData: [],
- selectedTab: undefined,
- };
- this.onDashboardChange = this.onDashboardChange.bind(this);
- this.onSliceNameChange = this.onSliceNameChange.bind(this);
- this.changeAction = this.changeAction.bind(this);
- this.saveOrOverwrite = this.saveOrOverwrite.bind(this);
- this.isNewDashboard = this.isNewDashboard.bind(this);
- this.onHide = this.onHide.bind(this);
- }
+const SaveModal = ({
+ addDangerToast,
+ actions,
+ form_data,
+ user,
+ alert: alertProp,
+ sliceName = '',
+ slice,
+ datasource,
+ dashboardId: dashboardIdProp,
+ isVisible,
+}: SaveModalProps) => {
+ const dispatch = useDispatch();
+ const history = useHistory();
+ const theme = useTheme();
+
+ const canOverwriteSlice = useCallback(
+ (): boolean =>
+ slice?.owners?.includes(user.userId) && !slice?.is_managed_externally,
+ [slice, user.userId],
+ );
- isNewDashboard(): boolean {
- const { dashboard } = this.state;
- return typeof dashboard?.value === 'string';
- }
+ const [newSliceName, setNewSliceName] = useState<string | undefined>(
+ sliceName,
+ );
+ const [datasetName, setDatasetName] = useState<string>(datasource?.name);
+ const [action, setAction] = useState<SaveActionType>(
+ canOverwriteSlice() ? ChartStatusType.overwrite : ChartStatusType.saveas,
+ );
+ const [isLoading, setIsLoading] = useState<boolean>(false);
+ const [dashboard, setDashboard] = useState<
+ { label: string; value: string | number } | undefined
+ >(undefined);
+ const [tabsData, setTabsData] = useState<TabTreeNode[]>([]);
+ const [selectedTab, setSelectedTab] = useState<
+ { label: string; value: string | number } | undefined
+ >(undefined);
+
+ const isNewDashboard = useCallback(
+ (): boolean => typeof dashboard?.value === 'string',
+ [dashboard?.value],
+ );
- canOverwriteSlice(): boolean {
- return (
- this.props.slice?.owners?.includes(this.props.user.userId) &&
- !this.props.slice?.is_managed_externally
- );
- }
+ const loadDashboard = useCallback(async (id: number) => {
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/dashboard/${id}`,
+ });
+ return response.json.result;
+ }, []);
- async componentDidMount() {
- let { dashboardId } = this.props;
- if (!dashboardId) {
- let lastDashboard = null;
+ const loadTabs = useCallback(
+ async (dashboardId: number) => {
try {
- lastDashboard = sessionStorage.getItem(SK_DASHBOARD_ID);
- } catch (error) {
- // continue regardless of error
- }
- dashboardId = lastDashboard && parseInt(lastDashboard, 10);
- }
- if (dashboardId) {
- try {
- const result = (await this.loadDashboard(dashboardId)) as Dashboard;
- if (canUserEditDashboard(result, this.props.user)) {
- this.setState({
- dashboard: { label: result.dashboard_title, value: result.id },
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/dashboard/${dashboardId}/tabs`,
+ });
+
+ const { result } = response.json;
+ if (!result || !Array.isArray(result.tab_tree)) {
+ logging.warn('Invalid tabs response format');
+ setTabsData([]);
+ return [];
+ }
+ const tabTree = result.tab_tree;
+ const gridTabIds = new Set<string>();
+ const convertToTreeData = (nodes: TabNode[]): TabTreeNode[] =>
+ nodes.map(node => {
+ const isGridTab =
+ Array.isArray(node.parents) && node.parents.includes('GRID_ID');
+ if (isGridTab) {
+ gridTabIds.add(node.value);
+ }
+ return {
+ value: node.value,
+ title: node.title,
+ key: node.value,
+ children:
+ node.children && node.children.length > 0
+ ? convertToTreeData(node.children)
+ : undefined,
+ };
});
- await this.loadTabs(dashboardId);
+
+ const treeData = convertToTreeData(tabTree);
+
+ // Add "Out of tab" option at the beginning
+ if (gridTabIds.size > 0) {
+ const tabsDataWithOutOfTab = [
+ {
+ value: 'OUT_OF_TAB',
+ title: 'Out of tab',
+ key: 'OUT_OF_TAB',
+ children: undefined,
+ },
+ ...treeData,
+ ];
+
+ setTabsData(tabsDataWithOutOfTab);
+ setSelectedTab({ value: 'OUT_OF_TAB', label: 'Out of tab' });
+ } else if (treeData.length > 0) {
+ const firstTab = treeData[0];
+ setTabsData(treeData);
+ setSelectedTab({ value: firstTab.value, label: firstTab.title });
+ } else {
+ setTabsData([]);
}
+
+ return treeData;
} catch (error) {
- logging.warn(error);
- this.props.addDangerToast(
- t('An error occurred while loading dashboard information.'),
- );
+ logging.error('Error loading tabs:', error);
+ setTabsData([]);
+ return [];
}
- }
- }
-
- handleDatasetNameChange = (e: FormEvent<HTMLInputElement>) => {
- // @ts-expect-error
- this.setState({ datasetName: e.target.value });
- };
-
- onSliceNameChange(event: ChangeEvent<HTMLInputElement>) {
- this.setState({ newSliceName: event.target.value });
- }
+ },
+ [setTabsData, setSelectedTab],
+ );
- onDashboardChange = async (
- dashboard:
- | {
- label: string;
- value: string | number;
+ useEffect(() => {
+ const initializeDashboard = async () => {
+ let dashboardId = dashboardIdProp;
+ if (!dashboardId) {
+ let lastDashboard = null;
+ try {
+ lastDashboard = sessionStorage.getItem(SK_DASHBOARD_ID);
+ } catch (error) {
+ // continue regardless of error
}
- | undefined,
- ) => {
- this.setState({
- dashboard,
- tabsData: [],
- selectedTab: undefined,
- });
+ dashboardId = lastDashboard && parseInt(lastDashboard, 10);
+ }
+ if (dashboardId) {
+ try {
+ const result = (await loadDashboard(dashboardId)) as Dashboard;
+ if (canUserEditDashboard(result, user)) {
+ setDashboard({ label: result.dashboard_title, value: result.id });
+ await loadTabs(dashboardId);
+ }
+ } catch (error) {
+ logging.warn(error);
+ addDangerToast(
+ t('An error occurred while loading dashboard information.'),
+ );
+ }
+ }
+ };
+ initializeDashboard();
+ }, [dashboardIdProp, user, loadDashboard, loadTabs, addDangerToast]);
+
+ const handleDatasetNameChange = useCallback(
+ (e: FormEvent<HTMLInputElement>) => {
+ // @ts-expect-error
+ setDatasetName(e.target.value);
+ },
+ [],
+ );
- if (dashboard && typeof dashboard.value === 'number') {
- await this.loadTabs(dashboard.value);
- }
- };
- changeAction(action: SaveActionType) {
- this.setState({ action });
- }
+ const onSliceNameChange = useCallback(
+ (event: ChangeEvent<HTMLInputElement>) => {
+ setNewSliceName(event.target.value);
+ },
+ [],
+ );
- onHide() {
- this.props.dispatch(setSaveChartModalVisibility(false));
- }
+ const onDashboardChange = useCallback(
+ async (value: SelectValue) => {
+ // AsyncSelect's onChange is typed with the broad antd SelectValue, but
+ // this Select is single-mode with labeled options, so the runtime value
+ // is either a { label, value } object or null/undefined when cleared.
+ const newDashboard =
+ value && typeof value === 'object' && !Array.isArray(value)
+ ? (value as { label: string; value: string | number })
+ : undefined;
+ setDashboard(newDashboard);
+ setTabsData([]);
+ setSelectedTab(undefined);
+
+ if (newDashboard && typeof newDashboard.value === 'number') {
+ await loadTabs(newDashboard.value);
+ }
+ },
+ [loadTabs],
+ );
- handleRedirect = (windowLocationSearch: string, chart: any) => {
- const searchParams = new URLSearchParams(windowLocationSearch);
- searchParams.delete('form_data_key');
- searchParams.set('slice_id', chart.id.toString());
- return searchParams;
- };
+ const changeAction = useCallback((newAction: SaveActionType) => {
+ setAction(newAction);
+ }, []);
- async saveOrOverwrite(gotodash: boolean) {
- this.setState({ isLoading: true });
- const tableState = this.props.form_data?.table_state;
- const sliceId = this.props.slice?.slice_id;
- const vizType = this.props.form_data?.viz_type;
- if (sliceId && vizType && tableState) {
- this.props.dispatch(updateChartState(sliceId, vizType, tableState));
- }
+ const onHide = useCallback(() => {
+ dispatch(setSaveChartModalVisibility(false));
+ }, [dispatch]);
- // Create or retrieve dashboard
- type DashboardGetResponse = {
- id: number;
- url: string;
- dashboard_title: string;
- };
+ const handleRedirect = useCallback(
+ (windowLocationSearch: string, chart: { id: number }) =>
+ createRedirectParams(windowLocationSearch, chart, action),
+ [action],
+ );
- try {
- if (this.props.datasource?.type === DatasourceType.Query) {
- const { schema, sql, database } = this.props.datasource;
- const { templateParams } = this.props.datasource;
-
- await this.props.actions.saveDataset({
- schema,
- sql,
- database,
- templateParams,
- datasourceName: this.state.datasetName,
- });
+ const addChartToDashboardTab = useCallback(
+ async (
+ dashboardId: number,
+ chartId: number,
+ tabId: string,
+ sliceNameParam: string | undefined,
+ ) => {
+ try {
+ await addChartToDashboard(dashboardId, chartId, tabId, sliceNameParam);
+ } catch (error) {
+ throw new Error(`Error adding chart to dashboard tab: ${error}`);
}
+ },
+ [],
+ );
- // Get chart dashboards
- let sliceDashboards: number[] = [];
- if (this.props.slice && this.state.action === 'overwrite') {
- sliceDashboards = await this.props.actions.getSliceDashboards(
- this.props.slice,
- );
+ const saveOrOverwrite = useCallback(
+ async (gotodash: boolean) => {
+ setIsLoading(true);
+ const tableState = form_data?.table_state;
+ const sliceId = slice?.slice_id;
+ const vizType = form_data?.viz_type;
+ if (sliceId && vizType && tableState) {
+ dispatch(updateChartState(sliceId, vizType, tableState));
}
- const formData = this.props.form_data || {};
- delete formData.url_params;
+ // Create or retrieve dashboard
+ type DashboardGetResponse = {
+ id: number;
+ url: string;
+ dashboard_title: string;
+ };
- let dashboard: DashboardGetResponse | null = null;
- let selectedTabId: string | undefined;
- if (this.state.dashboard) {
- let validId = this.state.dashboard.value;
- if (this.isNewDashboard()) {
- const response = await this.props.actions.createDashboard(
- this.state.dashboard.label,
- );
- validId = response.id;
+ try {
+ if (datasource?.type === DatasourceType.Query) {
+ const { schema, sql, database } = datasource;
+ const { templateParams } = datasource;
+
+ await actions.saveDataset({
+ schema,
+ sql,
+ database,
+ templateParams,
+ datasourceName: datasetName,
+ });
}
- try {
- dashboard = await this.loadDashboard(validId as number);
- } catch (error) {
- this.props.actions.saveSliceFailed();
- return;
+ // Get chart dashboards
+ let sliceDashboards: number[] = [];
+ if (slice && action === 'overwrite') {
+ sliceDashboards = await actions.getSliceDashboards(slice);
}
- if (isDefined(dashboard) && isDefined(dashboard?.id)) {
- sliceDashboards = sliceDashboards.includes(dashboard.id)
- ? sliceDashboards
- : [...sliceDashboards, dashboard.id];
- formData.dashboards = sliceDashboards;
- if (
- this.state.action === ChartStatusType.saveas &&
- this.state.selectedTab?.value !== 'OUT_OF_TAB'
- ) {
- selectedTabId = this.state.selectedTab?.value as string;
+ const formData = form_data || {};
+ delete formData.url_params;
+
+ let dashboardResult: DashboardGetResponse | null = null;
+ let selectedTabId: string | undefined;
+ if (dashboard) {
+ let validId = dashboard.value;
+ if (isNewDashboard()) {
+ const response = await actions.createDashboard(dashboard.label);
+ validId = response.id;
}
- }
- }
- // Sets the form data
- this.props.actions.setFormData({ ...formData });
-
- // Update or create slice
- let value: { id: number };
- if (this.state.action === 'overwrite') {
- value = await this.props.actions.updateSlice(
- this.props.slice,
- this.state.newSliceName,
- sliceDashboards,
- dashboard
- ? {
- title: dashboard.dashboard_title,
- new: this.isNewDashboard(),
- }
- : null,
- );
- } else {
- value = await this.props.actions.createSlice(
- this.state.newSliceName,
- sliceDashboards,
- dashboard
- ? {
- title: dashboard.dashboard_title,
- new: this.isNewDashboard(),
- }
- : null,
- );
- if (dashboard && selectedTabId) {
try {
- await this.addChartToDashboardTab(
- dashboard.id,
- value.id,
- selectedTabId,
- this.state.newSliceName,
- );
+ dashboardResult = await loadDashboard(validId as number);
} catch (error) {
- logging.error('Error adding chart to dashboard tab:', error);
- this.props.addDangerToast(
- t('Chart was saved but could not be added to the selected tab.'),
- );
+ actions.saveSliceFailed();
+ return;
+ }
+
+ if (isDefined(dashboardResult) && isDefined(dashboardResult?.id)) {
+ sliceDashboards = sliceDashboards.includes(dashboardResult.id)
+ ? sliceDashboards
+ : [...sliceDashboards, dashboardResult.id];
+ formData.dashboards = sliceDashboards;
+ if (
+ action === ChartStatusType.saveas &&
+ selectedTab?.value !== 'OUT_OF_TAB'
+ ) {
+ selectedTabId = selectedTab?.value as string;
+ }
}
}
- }
- try {
- if (dashboard) {
- sessionStorage.setItem(SK_DASHBOARD_ID, `${dashboard.id}`);
+ // Sets the form data
+ actions.setFormData({ ...formData });
+
+ // Update or create slice
+ let value: { id: number };
+ if (action === 'overwrite') {
+ value = await actions.updateSlice(
+ slice,
+ newSliceName,
+ sliceDashboards,
+ dashboardResult
+ ? {
+ title: dashboardResult.dashboard_title,
+ new: isNewDashboard(),
+ }
+ : null,
+ );
} else {
- sessionStorage.removeItem(SK_DASHBOARD_ID);
+ value = await actions.createSlice(
+ newSliceName,
+ sliceDashboards,
+ dashboardResult
+ ? {
+ title: dashboardResult.dashboard_title,
+ new: isNewDashboard(),
+ }
+ : null,
+ );
+ if (dashboardResult && selectedTabId) {
+ try {
+ await addChartToDashboardTab(
+ dashboardResult.id,
+ value.id,
+ selectedTabId,
+ newSliceName,
+ );
+ } catch (error) {
+ logging.error('Error adding chart to dashboard tab:', error);
+ addDangerToast(
+ t(
+ 'Chart was saved but could not be added to the selected
tab.',
+ ),
+ );
+ }
+ }
}
- } catch (error) {
- // continue regardless of error
- }
- // Go to new dashboard url
- if (gotodash && dashboard) {
- let { url } = dashboard;
- if (this.state.selectedTab?.value) {
- url += `#${this.state.selectedTab.value}`;
+ try {
+ if (dashboardResult) {
+ sessionStorage.setItem(SK_DASHBOARD_ID, `${dashboardResult.id}`);
+ } else {
+ sessionStorage.removeItem(SK_DASHBOARD_ID);
+ }
+ } catch (error) {
+ // continue regardless of error
}
- this.props.dispatch(removeChartState(value.id));
- this.props.history.push(url);
- return;
- }
- const searchParams = this.handleRedirect(window.location.search, value);
- this.props.history.replace(`/explore/?${searchParams.toString()}`, {
- saveAction: this.state.action,
- });
-
- this.setState({ isLoading: false });
- this.onHide();
- } finally {
- this.setState({ isLoading: false });
- }
- }
- /* Adds a chart to the specified dashboard tab. If an existing row has
space, the chart is added there; otherwise, a new row is created.
- * @param {number} dashboardId - ID of the dashboard.
- * @param {number} chartId - ID of the chart to add.
- * @param {string} tabId - ID of the dashboard tab where the chart is added.
- * @param {string | undefined} sliceName - Chart name
- */
- addChartToDashboardTab = async (
- dashboardId: number,
- chartId: number,
- tabId: string,
- sliceName: string | undefined,
- ) => {
- try {
- const dashboardResponse = await SupersetClient.get({
- endpoint: `/api/v1/dashboard/${dashboardId}`,
- });
-
- const dashboard = dashboardResponse.json.result;
-
- let positionJson = dashboard.position_json;
- if (typeof positionJson === 'string') {
- positionJson = JSON.parse(positionJson);
- }
- positionJson = positionJson || {};
-
- const chartKey = `CHART-${chartId}`;
-
- // Find a row in the tab with available space
- const tabChildren = positionJson[tabId]?.children || [];
- let targetRowKey: string | null = null;
-
- for (const childKey of tabChildren) {
- const child = positionJson[childKey];
- if (child?.type === 'ROW') {
- const rowChildren = child.children || [];
- const totalWidth = rowChildren.reduce((sum: number, key: string) => {
- const component = positionJson[key];
- return sum + (component?.meta?.width || 0);
- }, 0);
-
- if (totalWidth + CHART_WIDTH <= GRID_COLUMN_COUNT) {
- targetRowKey = childKey;
- break;
+ // Go to new dashboard url
+ if (gotodash && dashboardResult) {
+ let { url } = dashboardResult;
+ // OUT_OF_TAB is a synthetic 'no tab' sentinel — it isn't a real
+ // dashboard anchor, so skip appending it as a URL fragment.
+ if (selectedTab?.value && selectedTab.value !== 'OUT_OF_TAB') {
Review Comment:
We should add a test for this. We should verify that the redirect URL
doesn't append `#OUT_OF_TAB` when that sentinel is the selected tab value.
--
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]
