rusackas commented on code in PR #39461:
URL: https://github.com/apache/superset/pull/39461#discussion_r3229482994
##########
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:
The behavior is preserved by 82bab7b03 (`fix(SaveModal): restore saveAction
history state dropped in function component conversion`). A targeted regression
test for this needs a `useHistory` spy (the current test harness uses
`BrowserRouter` and doesn't intercept `history.replace`'s second-arg state), so
I'm holding that off to a follow-up to avoid bloating this PR further — leaving
the thread open as a tracking marker for that test.
##########
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:
Behavior is in 81123abec (`fix(SaveModal): guard cleared dashboard selection
and strip OUT_OF_TAB hash`). Same caveat as the saveAction thread above —
testing the gotodash redirect path requires mocking `useHistory.push` plus the
full dashboard-create fetch chain, so deferring to a focused follow-up rather
than wedging it into the conversion PR.
##########
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:
The guard is in 81123abec. The existing `mock-async-select` test stub always
wraps its onChange as `{ label, value }`, so exercising the cleared-selection
path needs either a per-test override of that mock to call `onChange(null)` or
moving to the real `AsyncSelect` with a clear button — both larger than this
PR, deferring to a follow-up. Thread left open as the tracking marker.
##########
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:
Behavior is in 8132732e3 (`fix(AdhocFilterControl): reset partitionColumn on
datasource change`) — the `useEffect` now unconditionally calls
`setPartitionColumn(null)` before the fetch, so a switch from a 1-partition
source to a 0/multi-partition source leaves it null. The test for this needs an
integration-style assertion (either through the AdhocFilter popover surfacing
the "latest partition" operator, or by exposing the partition prop via a data
attribute) — deferring to a follow-up to keep the diff bounded.
##########
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:
Behavior is in 03cdd206c (`fix(SqlLab): retry fetchQueryResults when
resultsKey changes`) — the mount-only guard was replaced with an effect that
fires on `resultsKey` changes and dedupes when the value hasn't moved. The test
would need `useQueryEditor` mocked to return a late-arriving `resultsKey` and
verify the dispatch count — deferring to a follow-up since the test scaffolding
is non-trivial and not strictly load-bearing for this PR's mechanical
conversion.
--
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]
