You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "VisaryLab (via GitHub)" <gi...@apache.org> on 2025/07/16 07:04:37 UTC
[PR] Codex/добавить функциональность для отчетов в api [superset]
VisaryLab opened a new pull request, #34184:
URL: https://github.com/apache/superset/pull/34184
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Codex/добавить функциональность для отчетов в api [superset]
Posted by "VisaryLab (via GitHub)" <gi...@apache.org>.
VisaryLab closed pull request #34184: Codex/добавить функциональность для отчетов в api
URL: https://github.com/apache/superset/pull/34184
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] mistake pull [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on code in PR #34184:
URL: https://github.com/apache/superset/pull/34184#discussion_r2209518601
##########
superset-frontend/src/pages/ReportTemplateList/index.tsx:
##########
@@ -0,0 +1,197 @@
+import { useCallback, useMemo, useState } from 'react';
+import { t, SupersetClient } from '@superset-ui/core';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu';
+import {
+ ListView,
+ ListViewActionsBar,
+ type ListViewActionProps,
+} from 'src/components';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import { UploadReportTemplateModal } from 'src/features/reportTemplates';
+
+interface TemplateObject {
+ id: number;
+ name: string;
+ description: string;
+ dataset_id: number;
+}
+
+interface TemplateListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+}
+
+function ReportTemplateList({ addDangerToast, addSuccessToast }: TemplateListProps) {
+ const [data, setData] = useState<TemplateObject[]>([]);
+ const [count, setCount] = useState(0);
+ const [loading, setLoading] = useState(false);
+ const [showModal, setShowModal] = useState(false);
+
+ const fetchData = useCallback(({ pageIndex, pageSize }: any) => {
+ setLoading(true);
+ const offset = pageIndex * pageSize;
+ SupersetClient.get({
+ endpoint: `/api/v1/report_template/?limit=${pageSize}&offset=${offset}`,
+ })
+ .then(({ json }) => {
+ setData(json.result);
+ setCount(json.count ?? json.result.length);
+ })
+ .catch(err => {
+ addDangerToast(t('Error loading templates: %s', err.message));
+ })
Review Comment:
### Insufficient Error Context in Catch Block <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Error handling only captures the error message without preserving the full error context and stack trace for debugging.
###### Why this matters
Without complete error information, debugging production issues becomes more difficult as valuable context about where and why the error occurred is lost.
###### Suggested change ∙ *Feature Preview*
Add error logging with full error context before showing the toast message:
```typescript
catch(err => {
console.error('Failed to load templates:', err);
addDangerToast(t('Error loading templates: %s', err.message));
})
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f401d99a-cb2e-4d27-ae4b-1f16315bd4ae/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f401d99a-cb2e-4d27-ae4b-1f16315bd4ae?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f401d99a-cb2e-4d27-ae4b-1f16315bd4ae?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f401d99a-cb2e-4d27-ae4b-1f16315bd4ae?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f401d99a-cb2e-4d27-ae4b-1f16315bd4ae)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:e5cac4de-d51b-417e-9de8-5964eb80f2ae -->
[](e5cac4de-d51b-417e-9de8-5964eb80f2ae)
##########
superset-frontend/src/pages/ReportTemplateList/index.tsx:
##########
@@ -0,0 +1,197 @@
+import { useCallback, useMemo, useState } from 'react';
+import { t, SupersetClient } from '@superset-ui/core';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu';
+import {
+ ListView,
+ ListViewActionsBar,
+ type ListViewActionProps,
+} from 'src/components';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import { UploadReportTemplateModal } from 'src/features/reportTemplates';
+
+interface TemplateObject {
+ id: number;
+ name: string;
+ description: string;
+ dataset_id: number;
+}
+
+interface TemplateListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+}
+
+function ReportTemplateList({ addDangerToast, addSuccessToast }: TemplateListProps) {
+ const [data, setData] = useState<TemplateObject[]>([]);
+ const [count, setCount] = useState(0);
+ const [loading, setLoading] = useState(false);
+ const [showModal, setShowModal] = useState(false);
+
+ const fetchData = useCallback(({ pageIndex, pageSize }: any) => {
+ setLoading(true);
+ const offset = pageIndex * pageSize;
+ SupersetClient.get({
+ endpoint: `/api/v1/report_template/?limit=${pageSize}&offset=${offset}`,
+ })
+ .then(({ json }) => {
+ setData(json.result);
+ setCount(json.count ?? json.result.length);
+ })
+ .catch(err => {
+ addDangerToast(t('Error loading templates: %s', err.message));
+ })
+ .finally(() => setLoading(false));
+ }, [addDangerToast]);
+
+ const handleDownload = (id: number) => {
+ SupersetClient.get({
+ endpoint: `/api/v1/report_template/${id}/download`,
+ parseMethod: 'raw',
+ })
Review Comment:
### Missing Request Debouncing <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Download handlers are not debounced, allowing rapid successive clicks that could trigger multiple simultaneous downloads.
###### Why this matters
Multiple simultaneous downloads can overwhelm the server and client resources, potentially causing performance issues or failed requests.
###### Suggested change ∙ *Feature Preview*
Implement debouncing for the download handlers:
```typescript
import { debounce } from 'lodash';
const handleDownload = debounce((id: number) => {
SupersetClient.get({
endpoint: `/api/v1/report_template/${id}/download`,
parseMethod: 'raw',
})
}, 300);
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/57a5494c-09be-4e4d-9ece-78ba207d55ff/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/57a5494c-09be-4e4d-9ece-78ba207d55ff?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/57a5494c-09be-4e4d-9ece-78ba207d55ff?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/57a5494c-09be-4e4d-9ece-78ba207d55ff?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/57a5494c-09be-4e4d-9ece-78ba207d55ff)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:c690c392-37e8-4be3-aa09-3f1c798cabe3 -->
[](c690c392-37e8-4be3-aa09-3f1c798cabe3)
##########
superset-frontend/src/pages/ReportTemplateList/index.tsx:
##########
@@ -0,0 +1,197 @@
+import { useCallback, useMemo, useState } from 'react';
+import { t, SupersetClient } from '@superset-ui/core';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu';
+import {
+ ListView,
+ ListViewActionsBar,
+ type ListViewActionProps,
+} from 'src/components';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import { UploadReportTemplateModal } from 'src/features/reportTemplates';
+
+interface TemplateObject {
+ id: number;
+ name: string;
+ description: string;
+ dataset_id: number;
+}
+
+interface TemplateListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+}
+
+function ReportTemplateList({ addDangerToast, addSuccessToast }: TemplateListProps) {
+ const [data, setData] = useState<TemplateObject[]>([]);
+ const [count, setCount] = useState(0);
+ const [loading, setLoading] = useState(false);
+ const [showModal, setShowModal] = useState(false);
+
+ const fetchData = useCallback(({ pageIndex, pageSize }: any) => {
+ setLoading(true);
+ const offset = pageIndex * pageSize;
+ SupersetClient.get({
+ endpoint: `/api/v1/report_template/?limit=${pageSize}&offset=${offset}`,
+ })
+ .then(({ json }) => {
+ setData(json.result);
+ setCount(json.count ?? json.result.length);
+ })
+ .catch(err => {
+ addDangerToast(t('Error loading templates: %s', err.message));
+ })
+ .finally(() => setLoading(false));
+ }, [addDangerToast]);
+
+ const handleDownload = (id: number) => {
+ SupersetClient.get({
+ endpoint: `/api/v1/report_template/${id}/download`,
+ parseMethod: 'raw',
+ })
+ .then((resp: Response) => resp.blob())
+ .then(blob => {
+ const url = window.URL.createObjectURL(blob);
+ const a = document.createElement('a');
+ a.href = url;
+ a.download = 'template.odt';
+ document.body.appendChild(a);
+ a.click();
+ document.body.removeChild(a);
+ window.URL.revokeObjectURL(url);
+ })
+ .catch(err => {
+ addDangerToast(t('Failed to download template: %s', err.message));
+ });
+ };
+
+ const handleGenerate = (id: number) => {
Review Comment:
### DRY Violation: Duplicate Download Logic <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Duplicate code for file download handling in both handleDownload and handleGenerate functions
###### Why this matters
Code duplication increases maintenance burden and risk of inconsistencies when changes are needed to the download logic
###### Suggested change ∙ *Feature Preview*
Extract common download logic into a reusable function:
```typescript
const downloadFile = (blob: Blob, filename: string) => {
const url = window.URL.createObjectURL(blob);
const a = document.createElement('a');
a.href = url;
a.download = filename;
document.body.appendChild(a);
a.click();
document.body.removeChild(a);
window.URL.revokeObjectURL(url);
};
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/341ff1f6-35ee-429c-ac7c-b93cc54ad990/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/341ff1f6-35ee-429c-ac7c-b93cc54ad990?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/341ff1f6-35ee-429c-ac7c-b93cc54ad990?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/341ff1f6-35ee-429c-ac7c-b93cc54ad990?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/341ff1f6-35ee-429c-ac7c-b93cc54ad990)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:6a613077-556a-4136-ac36-825220fdf41d -->
[](6a613077-556a-4136-ac36-825220fdf41d)
##########
superset-frontend/src/pages/ReportTemplateList/index.tsx:
##########
@@ -0,0 +1,197 @@
+import { useCallback, useMemo, useState } from 'react';
+import { t, SupersetClient } from '@superset-ui/core';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu';
+import {
+ ListView,
+ ListViewActionsBar,
+ type ListViewActionProps,
+} from 'src/components';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import { UploadReportTemplateModal } from 'src/features/reportTemplates';
+
+interface TemplateObject {
+ id: number;
+ name: string;
+ description: string;
+ dataset_id: number;
+}
+
+interface TemplateListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+}
+
+function ReportTemplateList({ addDangerToast, addSuccessToast }: TemplateListProps) {
+ const [data, setData] = useState<TemplateObject[]>([]);
+ const [count, setCount] = useState(0);
+ const [loading, setLoading] = useState(false);
+ const [showModal, setShowModal] = useState(false);
+
+ const fetchData = useCallback(({ pageIndex, pageSize }: any) => {
+ setLoading(true);
+ const offset = pageIndex * pageSize;
+ SupersetClient.get({
+ endpoint: `/api/v1/report_template/?limit=${pageSize}&offset=${offset}`,
+ })
+ .then(({ json }) => {
+ setData(json.result);
+ setCount(json.count ?? json.result.length);
+ })
+ .catch(err => {
+ addDangerToast(t('Error loading templates: %s', err.message));
+ })
+ .finally(() => setLoading(false));
+ }, [addDangerToast]);
+
+ const handleDownload = (id: number) => {
+ SupersetClient.get({
+ endpoint: `/api/v1/report_template/${id}/download`,
+ parseMethod: 'raw',
+ })
+ .then((resp: Response) => resp.blob())
+ .then(blob => {
+ const url = window.URL.createObjectURL(blob);
+ const a = document.createElement('a');
+ a.href = url;
+ a.download = 'template.odt';
+ document.body.appendChild(a);
+ a.click();
+ document.body.removeChild(a);
+ window.URL.revokeObjectURL(url);
+ })
+ .catch(err => {
+ addDangerToast(t('Failed to download template: %s', err.message));
+ });
+ };
+
+ const handleGenerate = (id: number) => {
+ SupersetClient.post({
+ endpoint: `/api/v1/report_template/${id}/generate`,
+ parseMethod: 'raw',
+ })
+ .then((resp: Response) => resp.blob())
+ .then(blob => {
+ const url = window.URL.createObjectURL(blob);
+ const a = document.createElement('a');
+ a.href = url;
+ a.download = 'report.odt';
+ document.body.appendChild(a);
+ a.click();
+ document.body.removeChild(a);
+ window.URL.revokeObjectURL(url);
+ })
+ .catch(err => {
+ addDangerToast(t('Failed to generate report: %s', err.message));
+ });
+ };
+
+ const handleDelete = (id: number) => {
+ SupersetClient.delete({ endpoint: `/api/v1/report_template/${id}` })
+ .then(() => {
+ addSuccessToast(t('Deleted'));
+ fetchData({ pageIndex: 0, pageSize: 25 });
Review Comment:
### Non-optimized Page Size <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The pageSize is hardcoded in multiple places and not optimized based on viewport size or user preferences.
###### Why this matters
Fixed page sizes may lead to unnecessary data fetching or poor performance on different screen sizes and network conditions.
###### Suggested change ∙ *Feature Preview*
Make pageSize configurable and consider viewport size:
```typescript
const defaultPageSize = useMemo(() => {
const viewportHeight = window.innerHeight;
return Math.floor((viewportHeight - 300) / 50); // Approximate rows that fit
}, []);
// Use defaultPageSize instead of hardcoded 25
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/777614d2-c6b9-4467-af41-d597551050bb/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/777614d2-c6b9-4467-af41-d597551050bb?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/777614d2-c6b9-4467-af41-d597551050bb?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/777614d2-c6b9-4467-af41-d597551050bb?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/777614d2-c6b9-4467-af41-d597551050bb)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:4ea298c2-67e0-4ea4-a7ac-65ff39c683b9 -->
[](4ea298c2-67e0-4ea4-a7ac-65ff39c683b9)
##########
superset-frontend/src/pages/ReportTemplateList/index.tsx:
##########
@@ -0,0 +1,197 @@
+import { useCallback, useMemo, useState } from 'react';
+import { t, SupersetClient } from '@superset-ui/core';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu';
+import {
+ ListView,
+ ListViewActionsBar,
+ type ListViewActionProps,
+} from 'src/components';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import { UploadReportTemplateModal } from 'src/features/reportTemplates';
+
+interface TemplateObject {
+ id: number;
+ name: string;
+ description: string;
+ dataset_id: number;
+}
+
+interface TemplateListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+}
+
+function ReportTemplateList({ addDangerToast, addSuccessToast }: TemplateListProps) {
+ const [data, setData] = useState<TemplateObject[]>([]);
+ const [count, setCount] = useState(0);
+ const [loading, setLoading] = useState(false);
+ const [showModal, setShowModal] = useState(false);
+
+ const fetchData = useCallback(({ pageIndex, pageSize }: any) => {
+ setLoading(true);
+ const offset = pageIndex * pageSize;
+ SupersetClient.get({
+ endpoint: `/api/v1/report_template/?limit=${pageSize}&offset=${offset}`,
+ })
+ .then(({ json }) => {
+ setData(json.result);
+ setCount(json.count ?? json.result.length);
Review Comment:
### Incorrect Count Fallback Logic <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The count fallback logic may be incorrect if json.count is 0, as it will use the result length instead.
###### Why this matters
This could lead to incorrect pagination when the total count is 0 but there are results in the current page, affecting the user's ability to navigate through the data correctly.
###### Suggested change ∙ *Feature Preview*
Use nullish coalescing only if count is undefined or null:
```typescript
setCount(json.count !== undefined ? json.count : json.result.length)
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2be8756c-d218-41b0-93a4-8c1b1ff4701f/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2be8756c-d218-41b0-93a4-8c1b1ff4701f?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2be8756c-d218-41b0-93a4-8c1b1ff4701f?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2be8756c-d218-41b0-93a4-8c1b1ff4701f?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2be8756c-d218-41b0-93a4-8c1b1ff4701f)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:8ae006fd-0e4c-4fbe-8725-e6ecd57d25df -->
[](8ae006fd-0e4c-4fbe-8725-e6ecd57d25df)
##########
superset-frontend/src/pages/ReportTemplateList/index.tsx:
##########
@@ -0,0 +1,197 @@
+import { useCallback, useMemo, useState } from 'react';
+import { t, SupersetClient } from '@superset-ui/core';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu';
+import {
+ ListView,
+ ListViewActionsBar,
+ type ListViewActionProps,
+} from 'src/components';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import { UploadReportTemplateModal } from 'src/features/reportTemplates';
+
+interface TemplateObject {
+ id: number;
+ name: string;
+ description: string;
+ dataset_id: number;
+}
+
+interface TemplateListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+}
+
+function ReportTemplateList({ addDangerToast, addSuccessToast }: TemplateListProps) {
+ const [data, setData] = useState<TemplateObject[]>([]);
+ const [count, setCount] = useState(0);
+ const [loading, setLoading] = useState(false);
+ const [showModal, setShowModal] = useState(false);
+
+ const fetchData = useCallback(({ pageIndex, pageSize }: any) => {
+ setLoading(true);
+ const offset = pageIndex * pageSize;
+ SupersetClient.get({
+ endpoint: `/api/v1/report_template/?limit=${pageSize}&offset=${offset}`,
+ })
+ .then(({ json }) => {
+ setData(json.result);
+ setCount(json.count ?? json.result.length);
+ })
+ .catch(err => {
+ addDangerToast(t('Error loading templates: %s', err.message));
+ })
+ .finally(() => setLoading(false));
+ }, [addDangerToast]);
+
+ const handleDownload = (id: number) => {
+ SupersetClient.get({
+ endpoint: `/api/v1/report_template/${id}/download`,
+ parseMethod: 'raw',
+ })
+ .then((resp: Response) => resp.blob())
+ .then(blob => {
+ const url = window.URL.createObjectURL(blob);
+ const a = document.createElement('a');
+ a.href = url;
+ a.download = 'template.odt';
+ document.body.appendChild(a);
+ a.click();
+ document.body.removeChild(a);
+ window.URL.revokeObjectURL(url);
+ })
+ .catch(err => {
+ addDangerToast(t('Failed to download template: %s', err.message));
+ });
+ };
+
+ const handleGenerate = (id: number) => {
+ SupersetClient.post({
+ endpoint: `/api/v1/report_template/${id}/generate`,
+ parseMethod: 'raw',
+ })
+ .then((resp: Response) => resp.blob())
+ .then(blob => {
+ const url = window.URL.createObjectURL(blob);
+ const a = document.createElement('a');
+ a.href = url;
+ a.download = 'report.odt';
+ document.body.appendChild(a);
+ a.click();
+ document.body.removeChild(a);
+ window.URL.revokeObjectURL(url);
+ })
+ .catch(err => {
+ addDangerToast(t('Failed to generate report: %s', err.message));
+ });
+ };
+
+ const handleDelete = (id: number) => {
+ SupersetClient.delete({ endpoint: `/api/v1/report_template/${id}` })
+ .then(() => {
+ addSuccessToast(t('Deleted'));
+ fetchData({ pageIndex: 0, pageSize: 25 });
+ })
+ .catch(err => {
+ addDangerToast(t('Failed to delete: %s', err.message));
+ });
+ };
+
+ const columns = useMemo(
+ () => [
+ {
+ accessor: 'name',
+ Header: t('Name'),
+ id: 'name',
+ },
+ {
+ accessor: 'description',
+ Header: t('Description'),
+ id: 'description',
+ },
+ {
+ accessor: 'dataset_id',
+ Header: t('Dataset ID'),
+ id: 'dataset_id',
+ },
+ {
+ Cell: ({ row: { original } }: any) => {
+ const actions = [
+ {
+ label: 'download',
+ tooltip: t('Download'),
+ placement: 'bottom',
+ icon: 'DownloadOutlined',
+ onClick: () => handleDownload(original.id),
+ },
+ {
+ label: 'edit',
+ tooltip: t('Edit'),
+ placement: 'bottom',
+ icon: 'EditOutlined',
+ onClick: () => handleDownload(original.id),
+ },
Review Comment:
### Edit Button Triggers Download Instead of Edit <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The 'edit' action button incorrectly calls handleDownload instead of an edit function.
###### Why this matters
Users clicking the edit button will unexpectedly download the template instead of being able to edit it, creating confusion and preventing the intended edit functionality.
###### Suggested change ∙ *Feature Preview*
Either implement a proper handleEdit function or remove the edit button if editing is not supported:
```typescript
{
label: 'edit',
tooltip: t('Edit'),
placement: 'bottom',
icon: 'EditOutlined',
onClick: () => handleEdit(original.id), // Implement handleEdit function
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cdcb15ed-15b5-4fe4-875d-313fd5ccfabe/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cdcb15ed-15b5-4fe4-875d-313fd5ccfabe?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cdcb15ed-15b5-4fe4-875d-313fd5ccfabe?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cdcb15ed-15b5-4fe4-875d-313fd5ccfabe?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cdcb15ed-15b5-4fe4-875d-313fd5ccfabe)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:a0476ca5-ecf6-4225-b5b4-54537444db3a -->
[](a0476ca5-ecf6-4225-b5b4-54537444db3a)
##########
superset-frontend/src/pages/ReportTemplateList/index.tsx:
##########
@@ -0,0 +1,197 @@
+import { useCallback, useMemo, useState } from 'react';
+import { t, SupersetClient } from '@superset-ui/core';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu';
+import {
+ ListView,
+ ListViewActionsBar,
+ type ListViewActionProps,
+} from 'src/components';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import { UploadReportTemplateModal } from 'src/features/reportTemplates';
+
+interface TemplateObject {
+ id: number;
+ name: string;
+ description: string;
+ dataset_id: number;
+}
+
+interface TemplateListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+}
+
+function ReportTemplateList({ addDangerToast, addSuccessToast }: TemplateListProps) {
+ const [data, setData] = useState<TemplateObject[]>([]);
+ const [count, setCount] = useState(0);
+ const [loading, setLoading] = useState(false);
+ const [showModal, setShowModal] = useState(false);
+
+ const fetchData = useCallback(({ pageIndex, pageSize }: any) => {
+ setLoading(true);
+ const offset = pageIndex * pageSize;
+ SupersetClient.get({
+ endpoint: `/api/v1/report_template/?limit=${pageSize}&offset=${offset}`,
+ })
+ .then(({ json }) => {
+ setData(json.result);
+ setCount(json.count ?? json.result.length);
+ })
+ .catch(err => {
+ addDangerToast(t('Error loading templates: %s', err.message));
+ })
+ .finally(() => setLoading(false));
+ }, [addDangerToast]);
+
+ const handleDownload = (id: number) => {
+ SupersetClient.get({
+ endpoint: `/api/v1/report_template/${id}/download`,
+ parseMethod: 'raw',
+ })
+ .then((resp: Response) => resp.blob())
+ .then(blob => {
+ const url = window.URL.createObjectURL(blob);
+ const a = document.createElement('a');
+ a.href = url;
+ a.download = 'template.odt';
+ document.body.appendChild(a);
+ a.click();
+ document.body.removeChild(a);
+ window.URL.revokeObjectURL(url);
+ })
+ .catch(err => {
+ addDangerToast(t('Failed to download template: %s', err.message));
+ });
+ };
+
+ const handleGenerate = (id: number) => {
+ SupersetClient.post({
+ endpoint: `/api/v1/report_template/${id}/generate`,
+ parseMethod: 'raw',
+ })
+ .then((resp: Response) => resp.blob())
+ .then(blob => {
+ const url = window.URL.createObjectURL(blob);
+ const a = document.createElement('a');
+ a.href = url;
+ a.download = 'report.odt';
+ document.body.appendChild(a);
+ a.click();
+ document.body.removeChild(a);
+ window.URL.revokeObjectURL(url);
+ })
+ .catch(err => {
+ addDangerToast(t('Failed to generate report: %s', err.message));
+ });
+ };
+
+ const handleDelete = (id: number) => {
+ SupersetClient.delete({ endpoint: `/api/v1/report_template/${id}` })
Review Comment:
### Unprotected Delete Operation <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The delete operation lacks any confirmation or authorization check before execution.
###### Why this matters
Any user with access to the UI could trigger destructive operations without verification, potentially leading to accidental or malicious deletion of templates.
###### Suggested change ∙ *Feature Preview*
Add a confirmation dialog and ensure proper authorization checks are in place:
```typescript
const handleDelete = async (id: number) => {
const confirmed = await confirmDialog({
title: t('Delete template'),
message: t('Are you sure you want to delete this template?'),
});
if (confirmed) {
SupersetClient.delete({ endpoint: `/api/v1/report_template/${id}` })
.then(() => {
addSuccessToast(t('Deleted'));
fetchData({ pageIndex: 0, pageSize: 25 });
})
.catch(err => {
addDangerToast(t('Failed to delete: %s', err.message));
});
}
};
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/234921ab-8c03-45a2-9bf9-db1f25aa6f18/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/234921ab-8c03-45a2-9bf9-db1f25aa6f18?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/234921ab-8c03-45a2-9bf9-db1f25aa6f18?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/234921ab-8c03-45a2-9bf9-db1f25aa6f18?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/234921ab-8c03-45a2-9bf9-db1f25aa6f18)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:8c4db8b8-6de8-4934-87a3-9bcbd35d90ad -->
[](8c4db8b8-6de8-4934-87a3-9bcbd35d90ad)
##########
superset-frontend/src/features/reportTemplates/ReportTemplateModal.tsx:
##########
@@ -0,0 +1,91 @@
+import { useState, useEffect } from 'react';
+import { t, styled, SupersetClient } from '@superset-ui/core';
+import { Modal, Select, Button } from '@superset-ui/core/components';
+
+interface TemplateOption {
+ value: number;
+ label: string;
+}
+
+interface ReportTemplateModalProps {
+ show: boolean;
+ onHide: () => void;
+}
+
+const FullWidthSelect = styled(Select)`
+ width: 100%;
+`;
+
+export default function ReportTemplateModal({ show, onHide }: ReportTemplateModalProps) {
+ const [templates, setTemplates] = useState<TemplateOption[]>([]);
+ const [selected, setSelected] = useState<TemplateOption | null>(null);
+
+ useEffect(() => {
+ if (show) {
+ SupersetClient.get({ endpoint: '/api/v1/report_template/' })
+ .then(({ json }) => {
+ setTemplates(
+ (json.result || []).map((t: any) => ({ value: t.id, label: t.name })),
+ );
+ })
+ .catch(error => {
+ // eslint-disable-next-line no-console
+ console.error('Failed to load templates', error);
+ });
+ }
+ }, [show]);
+
+ const onGenerate = () => {
+ if (!selected) return;
+ SupersetClient.post({
+ endpoint: `/api/v1/report_template/${selected}/generate`,
+ parseMethod: 'raw',
+ })
+ .then((response: Response) => response.blob())
+ .then(blob => {
+ const url = window.URL.createObjectURL(blob);
+ const a = document.createElement('a');
+ a.href = url;
+ a.download = `${selected.label}.odt`;
+ document.body.appendChild(a);
+ a.click();
+ document.body.removeChild(a);
+ window.URL.revokeObjectURL(url);
Review Comment:
### Premature Blob URL Revocation <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The URL.revokeObjectURL is called immediately after creating the download link, which might not give enough time for the download to start.
###### Why this matters
Premature revocation of the blob URL could lead to failed downloads and memory leaks if the download doesn't complete in time.
###### Suggested change ∙ *Feature Preview*
Add a delay before revoking the URL to ensure the download has started:
```typescript
const url = window.URL.createObjectURL(blob);
const a = document.createElement('a');
a.href = url;
a.download = `${selected.label}.odt`;
document.body.appendChild(a);
a.click();
document.body.removeChild(a);
setTimeout(() => window.URL.revokeObjectURL(url), 1000);
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e468ca3b-2d2a-4ab4-a260-f3368da72d90/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e468ca3b-2d2a-4ab4-a260-f3368da72d90?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e468ca3b-2d2a-4ab4-a260-f3368da72d90?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e468ca3b-2d2a-4ab4-a260-f3368da72d90?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e468ca3b-2d2a-4ab4-a260-f3368da72d90)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:adc703cf-3918-4200-9a57-b95b78b15cd5 -->
[](adc703cf-3918-4200-9a57-b95b78b15cd5)
##########
superset-frontend/src/features/reportTemplates/ReportTemplateModal.tsx:
##########
@@ -0,0 +1,91 @@
+import { useState, useEffect } from 'react';
+import { t, styled, SupersetClient } from '@superset-ui/core';
+import { Modal, Select, Button } from '@superset-ui/core/components';
+
+interface TemplateOption {
+ value: number;
+ label: string;
+}
+
+interface ReportTemplateModalProps {
+ show: boolean;
+ onHide: () => void;
+}
+
+const FullWidthSelect = styled(Select)`
+ width: 100%;
+`;
+
+export default function ReportTemplateModal({ show, onHide }: ReportTemplateModalProps) {
+ const [templates, setTemplates] = useState<TemplateOption[]>([]);
+ const [selected, setSelected] = useState<TemplateOption | null>(null);
+
+ useEffect(() => {
+ if (show) {
+ SupersetClient.get({ endpoint: '/api/v1/report_template/' })
+ .then(({ json }) => {
+ setTemplates(
+ (json.result || []).map((t: any) => ({ value: t.id, label: t.name })),
+ );
+ })
+ .catch(error => {
+ // eslint-disable-next-line no-console
+ console.error('Failed to load templates', error);
+ });
+ }
+ }, [show]);
+
+ const onGenerate = () => {
+ if (!selected) return;
+ SupersetClient.post({
+ endpoint: `/api/v1/report_template/${selected}/generate`,
Review Comment:
### Incorrect Template ID in Generate API Call <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The endpoint URL is using the entire selected object instead of selected.value for the template ID.
###### Why this matters
This will cause the API request to fail as it's sending an object where an ID number is expected, resulting in a malformed URL and failed report generation.
###### Suggested change ∙ *Feature Preview*
Change the endpoint URL to use the template ID value:
```typescript
endpoint: `/api/v1/report_template/${selected.value}/generate`,
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4a7df02b-cc32-4147-9d2c-b238bbf43b53/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4a7df02b-cc32-4147-9d2c-b238bbf43b53?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4a7df02b-cc32-4147-9d2c-b238bbf43b53?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4a7df02b-cc32-4147-9d2c-b238bbf43b53?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4a7df02b-cc32-4147-9d2c-b238bbf43b53)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:fbe1cacf-c9ec-4606-bcd8-98857e625149 -->
[](fbe1cacf-c9ec-4606-bcd8-98857e625149)
##########
superset-frontend/src/features/reportTemplates/ReportTemplateModal.tsx:
##########
@@ -0,0 +1,91 @@
+import { useState, useEffect } from 'react';
+import { t, styled, SupersetClient } from '@superset-ui/core';
+import { Modal, Select, Button } from '@superset-ui/core/components';
+
+interface TemplateOption {
+ value: number;
+ label: string;
+}
+
+interface ReportTemplateModalProps {
+ show: boolean;
+ onHide: () => void;
+}
+
+const FullWidthSelect = styled(Select)`
+ width: 100%;
+`;
+
+export default function ReportTemplateModal({ show, onHide }: ReportTemplateModalProps) {
+ const [templates, setTemplates] = useState<TemplateOption[]>([]);
+ const [selected, setSelected] = useState<TemplateOption | null>(null);
+
+ useEffect(() => {
+ if (show) {
+ SupersetClient.get({ endpoint: '/api/v1/report_template/' })
Review Comment:
### Redundant API Calls Without Caching <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The API call to fetch templates is triggered on every show/hide toggle of the modal, without any caching mechanism.
###### Why this matters
Unnecessary network requests increase server load and degrade user experience with potential loading delays each time the modal is opened.
###### Suggested change ∙ *Feature Preview*
Implement a caching mechanism for the templates data. Only fetch if the cache is empty or stale:
```typescript
const [lastFetchTime, setLastFetchTime] = useState<number>(0);
const CACHE_DURATION = 5 * 60 * 1000; // 5 minutes
useEffect(() => {
if (show && (templates.length === 0 || Date.now() - lastFetchTime > CACHE_DURATION)) {
SupersetClient.get({ endpoint: '/api/v1/report_template/' })
.then(({ json }) => {
setTemplates((json.result || []).map((t: any) => ({ value: t.id, label: t.name })));
setLastFetchTime(Date.now());
});
}
}, [show, templates.length, lastFetchTime]);
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8fa0fe10-e79c-4018-ad0b-a6d2a6081d09/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8fa0fe10-e79c-4018-ad0b-a6d2a6081d09?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8fa0fe10-e79c-4018-ad0b-a6d2a6081d09?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8fa0fe10-e79c-4018-ad0b-a6d2a6081d09?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8fa0fe10-e79c-4018-ad0b-a6d2a6081d09)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:48e1e8cc-2503-4265-bc87-c4d369299a38 -->
[](48e1e8cc-2503-4265-bc87-c4d369299a38)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org