codeant-ai-for-open-source[bot] commented on code in PR #39461:
URL: https://github.com/apache/superset/pull/39461#discussion_r3429899394
##########
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:
**Suggestion:** Replace these `Record<string, any>` prop types with concrete
existing domain types (or narrowly scoped interfaces) for `actions`,
`form_data`, `slice`, and `datasource` to avoid unsafe access. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This TypeScript code uses `any` through `Record<string, any>` in the
component props. The custom rule explicitly flags new or modified TS/TSX code
that uses `any`, so this is a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2ac63835b81a4b8cbf6d8e2b96b351f3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=2ac63835b81a4b8cbf6d8e2b96b351f3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/explore/components/SaveModal.tsx
**Line:** 187:195
**Comment:**
*Custom Rule: Replace these `Record<string, any>` prop types with
concrete existing domain types (or narrowly scoped interfaces) for `actions`,
`form_data`, `slice`, and `datasource` to avoid unsafe access.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39461&comment_hash=08301271ecd526e8c470ef309b1666bff35ffd97928432d3130abd11b010dc8b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39461&comment_hash=08301271ecd526e8c470ef309b1666bff35ffd97928432d3130abd11b010dc8b&reaction=dislike'>👎</a>
##########
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:
**Suggestion:** Replace `any` in the Redux-derived state props with concrete
types from the explore/saveModal state models so the connected component is
fully type-safe. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
These Redux-derived prop declarations directly use `any`, which is exactly
what the TypeScript rule forbids in modified TS/TSX code. The violation is
present in the final file state.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4b333d011dbe4154b66ca63c3b1762ba&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=4b333d011dbe4154b66ca63c3b1762ba&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/explore/components/SaveModal.tsx
**Line:** 867:870
**Comment:**
*Custom Rule: Replace `any` in the Redux-derived state props with
concrete types from the explore/saveModal state models so the connected
component is fully type-safe.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39461&comment_hash=76f52cc578da7e518902fec98dc17a8a928a7da56351491d698914722bfa3d4c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39461&comment_hash=76f52cc578da7e518902fec98dc17a8a928a7da56351491d698914722bfa3d4c&reaction=dislike'>👎</a>
--
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]
