korbit-ai[bot] commented on code in PR #34222:
URL: https://github.com/apache/superset/pull/34222#discussion_r2215830076
##########
superset/report_templates/models.py:
##########
@@ -0,0 +1,25 @@
+from __future__ import annotations
+
+from flask_appbuilder import Model
+from sqlalchemy import Column, Integer, String, Text, ForeignKey
+from sqlalchemy.orm import relationship
+
+from superset.models.helpers import AuditMixinNullable
+from superset.connectors.sqla.models import SqlaTable
+
+
+class ReportTemplate(Model, AuditMixinNullable):
+ """Template for generating reports based on datasets."""
+
+ __tablename__ = "report_templates"
+
+ id = Column(Integer, primary_key=True)
+ name = Column(String(250), nullable=False)
+ description = Column(Text)
+ dataset_id = Column(Integer, ForeignKey("tables.id"), nullable=False)
+ template_path = Column(String(1024), nullable=False)
Review Comment:
### Unexplained Magic Number in Column Definition <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The hardcoded string length 1024 for template_path lacks context and
explanation for why this specific length was chosen.
###### Why this matters
Future developers may be unsure if this limit is arbitrary or based on
specific requirements, making it harder to maintain or modify the code
confidently.
###### Suggested change ∙ *Feature Preview*
```python
# Define a constant at module level
MAX_TEMPLATE_PATH_LENGTH = 1024 # Maximum length based on common filesystem
path limitations
# Use in column definition
template_path = Column(String(MAX_TEMPLATE_PATH_LENGTH), nullable=False)
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3cfae0d0-6a53-4b8d-8853-3a8eb836be2f/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3cfae0d0-6a53-4b8d-8853-3a8eb836be2f?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3cfae0d0-6a53-4b8d-8853-3a8eb836be2f?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3cfae0d0-6a53-4b8d-8853-3a8eb836be2f?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3cfae0d0-6a53-4b8d-8853-3a8eb836be2f)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:16867f8a-6931-4aa0-abcb-e6b0ea0930c2 -->
[](16867f8a-6931-4aa0-abcb-e6b0ea0930c2)
##########
superset-frontend/src/features/reportTemplates/UploadReportTemplateModal.tsx:
##########
@@ -0,0 +1,107 @@
+import { useState } from 'react';
+import { t, SupersetClient } from '@superset-ui/core';
+import { Modal, Input, Button } from '@superset-ui/core/components';
+import DatasetSelect from
'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DatasetSelect';
+
+interface UploadReportTemplateModalProps {
+ show: boolean;
+ onHide: () => void;
+ onUpload: () => void;
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+}
+
+export default function UploadReportTemplateModal({
+ show,
+ onHide,
+ onUpload,
+ addDangerToast,
+ addSuccessToast,
+}: UploadReportTemplateModalProps) {
+ const [name, setName] = useState('');
+ const [dataset, setDataset] = useState<{ label: string; value: number } |
null>(null);
+ const [description, setDescription] = useState('');
+ const [file, setFile] = useState<File | null>(null);
+
+ const reset = () => {
+ setName('');
+ setDataset(null);
+ setDescription('');
+ setFile(null);
+ };
+
+ const handleUpload = () => {
+ if (!file || !name || !dataset) {
+ addDangerToast(t('All fields are required'));
+ return;
+ }
+ const form = new FormData();
+ form.append('template', file);
+ form.append('name', name);
+ form.append('dataset_id', String(dataset.value));
+ if (description) form.append('description', description);
+ SupersetClient.post({
+ endpoint: '/api/v1/report_template/',
+ postPayload: form,
+ stringify: false,
+ })
+ .then(() => {
+ addSuccessToast(t('Template uploaded'));
+ reset();
+ onUpload();
+ onHide();
+ })
+ .catch(err => {
+ addDangerToast(t('Failed to upload template: %s', err.message));
+ });
Review Comment:
### Insufficient Error Context in Catch Block <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The error handling in the catch block only displays the error message
without capturing additional context about the failed request.
###### Why this matters
When debugging production issues, having only the error message may be
insufficient to understand the root cause, especially for network or
server-side failures.
###### Suggested change ∙ *Feature Preview*
Enhance the error handler to include more context about the failed request
and potentially log the error:
```typescript
.catch(err => {
console.error('Template upload failed:', { error: err, templateName: name,
datasetId: dataset.value });
const errorMessage = err.response?.data?.message || err.message;
addDangerToast(t('Failed to upload template: %s', errorMessage));
});
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/38466169-d1e3-426f-860f-e741b524c3e7/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/38466169-d1e3-426f-860f-e741b524c3e7?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/38466169-d1e3-426f-860f-e741b524c3e7?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/38466169-d1e3-426f-860f-e741b524c3e7?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/38466169-d1e3-426f-860f-e741b524c3e7)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:b00226e7-cc4e-490c-9119-a81001fe5763 -->
[](b00226e7-cc4e-490c-9119-a81001fe5763)
##########
superset-frontend/src/features/reportTemplates/UploadReportTemplateModal.tsx:
##########
@@ -0,0 +1,107 @@
+import { useState } from 'react';
+import { t, SupersetClient } from '@superset-ui/core';
+import { Modal, Input, Button } from '@superset-ui/core/components';
+import DatasetSelect from
'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DatasetSelect';
+
+interface UploadReportTemplateModalProps {
+ show: boolean;
+ onHide: () => void;
+ onUpload: () => void;
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+}
+
+export default function UploadReportTemplateModal({
+ show,
+ onHide,
+ onUpload,
+ addDangerToast,
+ addSuccessToast,
+}: UploadReportTemplateModalProps) {
+ const [name, setName] = useState('');
+ const [dataset, setDataset] = useState<{ label: string; value: number } |
null>(null);
+ const [description, setDescription] = useState('');
+ const [file, setFile] = useState<File | null>(null);
+
+ const reset = () => {
+ setName('');
+ setDataset(null);
+ setDescription('');
+ setFile(null);
+ };
+
+ const handleUpload = () => {
+ if (!file || !name || !dataset) {
+ addDangerToast(t('All fields are required'));
+ return;
+ }
+ const form = new FormData();
+ form.append('template', file);
+ form.append('name', name);
+ form.append('dataset_id', String(dataset.value));
+ if (description) form.append('description', description);
+ SupersetClient.post({
+ endpoint: '/api/v1/report_template/',
+ postPayload: form,
+ stringify: false,
+ })
+ .then(() => {
+ addSuccessToast(t('Template uploaded'));
+ reset();
+ onUpload();
+ onHide();
+ })
+ .catch(err => {
+ addDangerToast(t('Failed to upload template: %s', err.message));
+ });
+ };
+
+ return (
+ <Modal
+ show={show}
+ title={t('Add report template')}
+ onHide={onHide}
+ footer={
+ <div>
+ <Button buttonStyle="secondary" onClick={onHide}>
+ {t('Cancel')}
+ </Button>
+ <Button buttonStyle="primary" onClick={handleUpload}>
+ {t('Upload')}
+ </Button>
+ </div>
+ }
+ >
+ <div className="field">
+ <label htmlFor="template-name">{t('Name')}</label>
+ <Input
+ id="template-name"
+ value={name}
+ onChange={e => setName(e.target.value)}
+ />
+ </div>
+ <div className="field">
+ <label htmlFor="template-dataset">{t('Dataset')}</label>
+ <DatasetSelect
+ onChange={(opt: { label: string; value: number }) => setDataset(opt)}
+ value={dataset || undefined}
+ />
+ </div>
+ <div className="field">
+ <label htmlFor="template-desc">{t('Description')}</label>
+ <Input.TextArea
+ id="template-desc"
+ value={description}
+ onChange={e => setDescription(e.target.value)}
+ />
+ </div>
+ <div className="field">
+ <input
+ type="file"
+ accept=".odt"
+ onChange={e => setFile(e.target.files?.[0] || null)}
+ />
Review Comment:
### Inconsistent Form Element Styling <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The file input lacks styling consistency with other form elements and
missing label for accessibility.
###### Why this matters
Inconsistent styling and missing labels reduce code maintainability and
accessibility compliance.
###### Suggested change ∙ *Feature Preview*
```typescript
<div className="field">
<label htmlFor="template-file">{t('Template File (ODT)')}</label>
<Input
id="template-file"
type="file"
accept=".odt"
onChange={e => setFile(e.target.files?.[0] || null)}
/>
</div>
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/11336825-dce4-45cc-9588-03e828c80350/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/11336825-dce4-45cc-9588-03e828c80350?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/11336825-dce4-45cc-9588-03e828c80350?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/11336825-dce4-45cc-9588-03e828c80350?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/11336825-dce4-45cc-9588-03e828c80350)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:5701b6b3-a446-4508-a7bc-20c743d99b03 -->
[](5701b6b3-a446-4508-a7bc-20c743d99b03)
##########
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);
Review Comment:
### Direct Console Error Usage <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Using console.error for error logging instead of a proper logging framework.
###### Why this matters
Console logging doesn't provide structured logging capabilities, timestamp
information, log levels, or integration with logging aggregation systems. This
makes it harder to monitor and debug issues in production.
###### Suggested change ∙ *Feature Preview*
```typescript
import { logging } from '@superset-ui/core';
// ...
logging.error('Failed to load templates', { error, source:
'ReportTemplateModal' });
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ee886fad-db59-4c69-8807-8bf6931c986e/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ee886fad-db59-4c69-8807-8bf6931c986e?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ee886fad-db59-4c69-8807-8bf6931c986e?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ee886fad-db59-4c69-8807-8bf6931c986e?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ee886fad-db59-4c69-8807-8bf6931c986e)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:6aa320c6-549d-42b2-96f1-68fd5b28ed73 -->
[](6aa320c6-549d-42b2-96f1-68fd5b28ed73)
##########
superset/migrations/versions/2025-06-07_00-00_b4757fa23b89_add_report_templates_table.py:
##########
@@ -0,0 +1,52 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""add report templates table
+
+Revision ID: b4757fa23b89
+Revises: 363a9b1e8992
+Create Date: 2025-06-07 00:00:00.000000
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+from superset.migrations.shared.utils import create_table
+
+# revision identifiers, used by Alembic.
+revision = "b4757fa23b89"
+down_revision = "363a9b1e8992"
+
+
+def upgrade() -> None:
+ create_table(
+ "report_templates",
+ sa.Column("id", sa.Integer(), nullable=False),
Review Comment:
### Non-optimized Primary Key Type <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Using Integer type for primary key without auto-increment may lead to
performance issues as the table grows.
###### Why this matters
Without auto-increment, the database needs to check for ID uniqueness on
each insert, causing potential contention and slower inserts at scale.
###### Suggested change ∙ *Feature Preview*
Use auto-incrementing Integer type for better insert performance:
```python
sa.Column("id", sa.Integer(), autoincrement=True, nullable=False)
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b954e48b-f9b2-4084-b662-52490a2a00cf/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b954e48b-f9b2-4084-b662-52490a2a00cf?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b954e48b-f9b2-4084-b662-52490a2a00cf?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b954e48b-f9b2-4084-b662-52490a2a00cf?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b954e48b-f9b2-4084-b662-52490a2a00cf)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:3b935247-5207-4fb5-ba15-b22fa0e29c9e -->
[](3b935247-5207-4fb5-ba15-b22fa0e29c9e)
##########
superset/migrations/versions/2025-06-07_00-00_b4757fa23b89_add_report_templates_table.py:
##########
@@ -0,0 +1,52 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""add report templates table
+
+Revision ID: b4757fa23b89
+Revises: 363a9b1e8992
+Create Date: 2025-06-07 00:00:00.000000
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+from superset.migrations.shared.utils import create_table
+
+# revision identifiers, used by Alembic.
+revision = "b4757fa23b89"
+down_revision = "363a9b1e8992"
+
+
+def upgrade() -> None:
+ create_table(
+ "report_templates",
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("created_on", sa.DateTime(), nullable=True),
+ sa.Column("changed_on", sa.DateTime(), nullable=True),
Review Comment:
### Nullable Audit Timestamps <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Audit timestamps (created_on and changed_on) are marked as nullable, but
should be non-nullable with default values.
###### Why this matters
Allowing null values for audit timestamps can lead to gaps in tracking when
records were created or modified, compromising data audit capabilities.
###### Suggested change ∙ *Feature Preview*
Make timestamps non-nullable with defaults:
```python
sa.Column("created_on", sa.DateTime(), nullable=False,
server_default=sa.func.now()),
sa.Column("changed_on", sa.DateTime(), nullable=False,
server_default=sa.func.now()),
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/602d4b6a-6bf0-458c-b0af-9aec1258914f/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/602d4b6a-6bf0-458c-b0af-9aec1258914f?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/602d4b6a-6bf0-458c-b0af-9aec1258914f?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/602d4b6a-6bf0-458c-b0af-9aec1258914f?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/602d4b6a-6bf0-458c-b0af-9aec1258914f)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:eedfdc0c-4da3-43dd-b7fa-606e4a05c369 -->
[](eedfdc0c-4da3-43dd-b7fa-606e4a05c369)
##########
superset-frontend/src/features/reportTemplates/UploadReportTemplateModal.tsx:
##########
@@ -0,0 +1,107 @@
+import { useState } from 'react';
+import { t, SupersetClient } from '@superset-ui/core';
+import { Modal, Input, Button } from '@superset-ui/core/components';
+import DatasetSelect from
'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DatasetSelect';
+
+interface UploadReportTemplateModalProps {
+ show: boolean;
+ onHide: () => void;
+ onUpload: () => void;
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+}
+
+export default function UploadReportTemplateModal({
+ show,
+ onHide,
+ onUpload,
+ addDangerToast,
+ addSuccessToast,
+}: UploadReportTemplateModalProps) {
+ const [name, setName] = useState('');
+ const [dataset, setDataset] = useState<{ label: string; value: number } |
null>(null);
+ const [description, setDescription] = useState('');
+ const [file, setFile] = useState<File | null>(null);
+
+ const reset = () => {
+ setName('');
+ setDataset(null);
+ setDescription('');
+ setFile(null);
+ };
+
+ const handleUpload = () => {
+ if (!file || !name || !dataset) {
+ addDangerToast(t('All fields are required'));
+ return;
+ }
+ const form = new FormData();
+ form.append('template', file);
+ form.append('name', name);
+ form.append('dataset_id', String(dataset.value));
+ if (description) form.append('description', description);
+ SupersetClient.post({
+ endpoint: '/api/v1/report_template/',
+ postPayload: form,
+ stringify: false,
+ })
+ .then(() => {
+ addSuccessToast(t('Template uploaded'));
+ reset();
+ onUpload();
+ onHide();
+ })
+ .catch(err => {
+ addDangerToast(t('Failed to upload template: %s', err.message));
+ });
+ };
+
+ return (
+ <Modal
+ show={show}
+ title={t('Add report template')}
+ onHide={onHide}
+ footer={
+ <div>
+ <Button buttonStyle="secondary" onClick={onHide}>
+ {t('Cancel')}
+ </Button>
+ <Button buttonStyle="primary" onClick={handleUpload}>
+ {t('Upload')}
+ </Button>
+ </div>
+ }
+ >
+ <div className="field">
+ <label htmlFor="template-name">{t('Name')}</label>
+ <Input
+ id="template-name"
+ value={name}
+ onChange={e => setName(e.target.value)}
Review Comment:
### Missing Input Sanitization <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
User input for template name is not sanitized or validated for length or
special characters.
###### Why this matters
Unsanitized input could lead to XSS attacks if the template name is rendered
elsewhere in the application or cause issues with file system operations.
###### Suggested change ∙ *Feature Preview*
```typescript
const sanitizeName = (input: string) => {
// Remove special characters and limit length
return input.replace(/[^a-zA-Z0-9-_ ]/g, '').slice(0, 255);
};
onChange={e => setName(sanitizeName(e.target.value))}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0a063e34-6bf3-4073-ad0a-2150974bf18a/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0a063e34-6bf3-4073-ad0a-2150974bf18a?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0a063e34-6bf3-4073-ad0a-2150974bf18a?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0a063e34-6bf3-4073-ad0a-2150974bf18a?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0a063e34-6bf3-4073-ad0a-2150974bf18a)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:10453429-1753-419c-a2bb-913289735bf0 -->
[](10453429-1753-419c-a2bb-913289735bf0)
##########
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',
+ })
Review Comment:
### Unvalidated Template ID in API Endpoint <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The selected template ID is directly interpolated into the API endpoint URL
without validation, which could potentially lead to path traversal attacks.
###### Why this matters
If the selected value is manipulated, it could allow attackers to access
unintended API endpoints or cause unexpected behavior in the API request.
###### Suggested change ∙ *Feature Preview*
Validate that selected.value is a positive integer before using it in the
URL:
```typescript
SupersetClient.post({
endpoint: `/api/v1/report_template/${selected.value > 0 ? selected.value :
''}/generate`,
parseMethod: 'raw',
})
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c1db4d69-d4f3-40aa-b13f-ef90770efc0f/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c1db4d69-d4f3-40aa-b13f-ef90770efc0f?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c1db4d69-d4f3-40aa-b13f-ef90770efc0f?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c1db4d69-d4f3-40aa-b13f-ef90770efc0f?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c1db4d69-d4f3-40aa-b13f-ef90770efc0f)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:7ca96b2a-9f09-4a8f-8f22-45654f93a2bf -->
[](7ca96b2a-9f09-4a8f-8f22-45654f93a2bf)
##########
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 API endpoint <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The API endpoint uses the entire selected object instead of selected.value
for the template ID
###### Why this matters
This will result in a malformed API URL causing the report generation to
fail as the endpoint expects a numeric ID
###### Suggested change ∙ *Feature Preview*
Use the template ID from the selected object:
```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/c585c208-271a-4757-9a5b-12e62f32fda4/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c585c208-271a-4757-9a5b-12e62f32fda4?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c585c208-271a-4757-9a5b-12e62f32fda4?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c585c208-271a-4757-9a5b-12e62f32fda4?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c585c208-271a-4757-9a5b-12e62f32fda4)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:c9b4c45c-752b-4b94-af2b-f13e6fde692c -->
[](c9b4c45c-752b-4b94-af2b-f13e6fde692c)
##########
superset/report_templates/models.py:
##########
@@ -0,0 +1,25 @@
+from __future__ import annotations
+
+from flask_appbuilder import Model
+from sqlalchemy import Column, Integer, String, Text, ForeignKey
+from sqlalchemy.orm import relationship
+
+from superset.models.helpers import AuditMixinNullable
+from superset.connectors.sqla.models import SqlaTable
+
+
+class ReportTemplate(Model, AuditMixinNullable):
+ """Template for generating reports based on datasets."""
+
+ __tablename__ = "report_templates"
+
+ id = Column(Integer, primary_key=True)
+ name = Column(String(250), nullable=False)
+ description = Column(Text)
+ dataset_id = Column(Integer, ForeignKey("tables.id"), nullable=False)
+ template_path = Column(String(1024), nullable=False)
+
+ dataset = relationship(SqlaTable, foreign_keys=[dataset_id])
Review Comment:
### Undefined Dataset Relationship Cascade <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
No cascading delete behavior specified for the dataset relationship
###### Why this matters
If a dataset is deleted, related templates may become orphaned and cause
issues during report generation
###### Suggested change ∙ *Feature Preview*
Define appropriate cascade behavior for the dataset relationship:
```python
dataset = relationship(
SqlaTable,
foreign_keys=[dataset_id],
cascade='all, delete-orphan'
)
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ed3ebf5c-bad5-4915-818a-c01f4c95757c/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ed3ebf5c-bad5-4915-818a-c01f4c95757c?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ed3ebf5c-bad5-4915-818a-c01f4c95757c?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ed3ebf5c-bad5-4915-818a-c01f4c95757c?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ed3ebf5c-bad5-4915-818a-c01f4c95757c)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:9ef3e315-f2fd-4e7b-bf17-921866a553dd -->
[](9ef3e315-f2fd-4e7b-bf17-921866a553dd)
--
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]
