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: [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]
