rusackas commented on code in PR #39461:
URL: https://github.com/apache/superset/pull/39461#discussion_r3430421256
##########
superset-frontend/src/explore/components/SaveModal.tsx:
##########
@@ -67,7 +70,120 @@ import { CHART_WIDTH, CHART_HEIGHT } from
'src/dashboard/constants';
// Session storage key for recent dashboard
const SK_DASHBOARD_ID = 'save_chart_recent_dashboard';
-interface SaveModalProps extends RouteComponentProps {
+/**
+ * Creates URLSearchParams with save action and slice ID, removing
form_data_key.
+ * Exported for testing purposes.
+ */
+export const createRedirectParams = (
+ windowLocationSearch: string,
+ chart: { id: number },
+ action: string,
+): URLSearchParams => {
+ const searchParams = new URLSearchParams(windowLocationSearch);
+ searchParams.set('save_action', action);
+ searchParams.delete('form_data_key');
+ searchParams.set('slice_id', chart.id.toString());
+ return searchParams;
+};
+
+/**
+ * Adds a chart to a dashboard tab by updating the position_json.
+ * Exported for testing purposes.
+ */
+export const addChartToDashboard = async (
+ dashboardId: number,
+ chartId: number,
+ tabId: string,
+ sliceNameParam: string | undefined,
+): Promise<void> => {
+ const dashboardResponse = await SupersetClient.get({
+ endpoint: `/api/v1/dashboard/${dashboardId}`,
+ });
+
+ const dashboardData = dashboardResponse.json.result;
+
+ let positionJson = dashboardData.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;
+ }
+ }
+ }
+
+ 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',
+ },
+ };
+
+ if (positionJson[tabId]) {
+ updatedPositionJson[tabId] = {
+ ...positionJson[tabId],
+ children: [...(positionJson[tabId].children || []), targetRowKey],
+ };
+ } else {
+ throw new Error(`Tab ${tabId} not found in positionJson`);
+ }
+ }
+
+ updatedPositionJson[chartKey] = {
+ type: 'CHART',
+ id: chartKey,
+ children: [],
+ parents: ['ROOT_ID', 'GRID_ID', tabId, targetRowKey],
+ meta: {
+ width: CHART_WIDTH,
+ height: CHART_HEIGHT,
+ chartId,
+ sliceName: sliceNameParam ?? `Chart ${chartId}`,
+ },
+ };
+
+ // Add chart to the target row
+ updatedPositionJson[targetRowKey] = {
+ ...updatedPositionJson[targetRowKey],
+ children: [...(updatedPositionJson[targetRowKey].children || []),
chartKey],
+ };
+
+ await SupersetClient.put({
+ endpoint: `/api/v1/dashboard/${dashboardId}`,
+ headers: { 'Content-Type': 'application/json' },
+ body: JSON.stringify({
+ position_json: JSON.stringify(updatedPositionJson),
+ }),
+ });
+};
+
+interface SaveModalProps {
addDangerToast: (msg: string) => void;
Review Comment:
These prop types came over verbatim from the class component — this PR is a
mechanical FC conversion, so I'm keeping the existing `any` shapes rather than
retyping `actions`/`form_data`/`slice`/`datasource` here. Tightening those
feels like its own pass.
##########
superset-frontend/src/explore/components/SaveModal.tsx:
##########
@@ -793,30 +840,28 @@ class SaveModal extends Component<SaveModalProps,
SaveModalState> {
</div>
);
- render() {
- return (
- <StyledModal
- show={this.props.isVisible}
- onHide={this.onHide}
- title={t('Save chart')}
- footer={this.renderFooter()}
- >
- {this.state.isLoading ? (
- <div
- css={css`
- display: flex;
- justify-content: center;
- `}
- >
- <Loading position="normal" />
- </div>
- ) : (
- this.renderSaveChartModal()
- )}
- </StyledModal>
- );
- }
-}
+ return (
+ <StyledModal
+ show={isVisible}
+ onHide={onHide}
+ title={t('Save chart')}
+ footer={renderFooter()}
+ >
+ {isLoading ? (
+ <div
+ css={css`
+ display: flex;
+ justify-content: center;
+ `}
+ >
+ <Loading position="normal" />
+ </div>
+ ) : (
+ renderSaveChartModal()
+ )}
+ </StyledModal>
+ );
+};
interface StateProps {
datasource: any;
Review Comment:
Same here — these Redux-derived props were already `any` on the connected
class component, so the conversion just carries them across. Happy to type the
`mapStateToProps` shape, but that's a separate cleanup from the FC move.
--
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]
