Address PR comments

apache · May 5, 2022 · 00f574f · 00f574f
1 parent d88aa85
commit 00f574f
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 42 deletions.
diff --git a/superset-frontend/src/components/ReportModal/index.tsx b/superset-frontend/src/components/ReportModal/index.tsx
@@ -38,7 +38,7 @@ import { CronError } from 'src/components/CronPicker';
 import { RadioChangeEvent } from 'src/components';
 import { ChartState } from 'src/explore/types';
 import {
-  ReportCreationMethodType,
+  ReportCreationMethod,
   ReportRecipientType,
   ReportScheduleType,
 } from 'src/reports/types';
@@ -94,7 +94,7 @@ interface ReportProps {
   chartName?: string;
   dashboardId?: number;
   dashboardName?: string;
-  creationMethod: ReportCreationMethodType;
+  creationMethod: ReportCreationMethod;
 }
 
 const TEXT_BASED_VISUALIZATION_TYPES = [

diff --git a/superset-frontend/src/reports/types.ts b/superset-frontend/src/reports/types.ts
@@ -21,9 +21,6 @@
  * Types mirroring enums in `superset/reports/models.py`:
  */
 export type ReportScheduleType = 'Alert' | 'Report';
-export type ReportCreationMethodType =
-  | 'charts'
-  | 'dashboards'
-  | 'alerts_reports';
+export type ReportCreationMethod = 'charts' | 'dashboards' | 'alerts_reports';
 
 export type ReportRecipientType = 'Email' | 'Slack';
diff --git a/superset-frontend/src/utils/getClientErrorObject.ts b/superset-frontend/src/utils/getClientErrorObject.ts
@@ -50,7 +50,7 @@ export function parseErrorJson(responseObject: JsonObject): ClientErrorObject {
   }
   // Marshmallow field validation returns the error mssage in the format
   // of { message: { field1: [msg1, msg2], field2: [msg], } }
-  if (typeof error.message === 'object' && !error.error) {
+  if (error.message && typeof error.message === 'object' && !error.error) {
     error.error =
       Object.values(error.message as Record<string, string[]>)[0]?.[0] ||
       t('Invalid input');

diff --git a/superset/models/reports.py b/superset/models/reports.py
@@ -77,7 +77,7 @@ class ReportDataFormat(str, enum.Enum):
     TEXT = "TEXT"
 
 
-class ReportCreationMethodType(str, enum.Enum):
+class ReportCreationMethod(str, enum.Enum):
     CHARTS = "charts"
     DASHBOARDS = "dashboards"
     ALERTS_REPORTS = "alerts_reports"
@@ -112,7 +112,7 @@ class ReportSchedule(Model, AuditMixinNullable):
     active = Column(Boolean, default=True, index=True)
     crontab = Column(String(1000), nullable=False)
     creation_method = Column(
-        String(255), server_default=ReportCreationMethodType.ALERTS_REPORTS
+        String(255), server_default=ReportCreationMethod.ALERTS_REPORTS
     )
     timezone = Column(String(100), default="UTC", nullable=False)
     report_format = Column(String(50), default=ReportDataFormat.VISUALIZATION)

diff --git a/superset/reports/commands/base.py b/superset/reports/commands/base.py
@@ -22,7 +22,7 @@
 from superset.charts.dao import ChartDAO
 from superset.commands.base import BaseCommand
 from superset.dashboards.dao import DashboardDAO
-from superset.models.reports import ReportCreationMethodType
+from superset.models.reports import ReportCreationMethod
 from superset.reports.commands.exceptions import (
     ChartNotFoundValidationError,
     ChartNotSavedValidationError,
@@ -52,12 +52,12 @@ def validate_chart_dashboard(
         dashboard_id = self._properties.get("dashboard")
         creation_method = self._properties.get("creation_method")
 
-        if creation_method == ReportCreationMethodType.CHARTS and not chart_id:
+        if creation_method == ReportCreationMethod.CHARTS and not chart_id:
             # User has not saved chart yet in Explore view
             exceptions.append(ChartNotSavedValidationError())
             return
 
-        if creation_method == ReportCreationMethodType.DASHBOARDS and not dashboard_id:
+        if creation_method == ReportCreationMethod.DASHBOARDS and not dashboard_id:
             exceptions.append(DashboardNotSavedValidationError())
             return
 

diff --git a/superset/reports/commands/create.py b/superset/reports/commands/create.py
@@ -25,7 +25,7 @@
 from superset.commands.base import CreateMixin
 from superset.dao.exceptions import DAOCreateFailedError
 from superset.databases.dao import DatabaseDAO
-from superset.models.reports import ReportCreationMethodType, ReportScheduleType
+from superset.models.reports import ReportCreationMethod, ReportScheduleType
 from superset.reports.commands.base import BaseReportScheduleCommand
 from superset.reports.commands.exceptions import (
     DatabaseNotFoundValidationError,
@@ -97,7 +97,7 @@ def validate(self) -> None:
         # Validate that each chart or dashboard only has one report with
         # the respective creation method.
         if (
-            creation_method != ReportCreationMethodType.ALERTS_REPORTS
+            creation_method != ReportCreationMethod.ALERTS_REPORTS
             and not ReportScheduleDAO.validate_unique_creation_method(
                 user_id, dashboard_id, chart_id
             )

diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py
@@ -24,7 +24,7 @@
 from pytz import all_timezones
 
 from superset.models.reports import (
-    ReportCreationMethodType,
+    ReportCreationMethod,
     ReportDataFormat,
     ReportRecipientType,
     ReportScheduleType,
@@ -164,7 +164,7 @@ class ReportSchedulePostSchema(Schema):
     )
     chart = fields.Integer(required=False, allow_none=True)
     creation_method = EnumField(
-        ReportCreationMethodType,
+        ReportCreationMethod,
         by_value=True,
         required=False,
         description=creation_method_description,
@@ -256,7 +256,7 @@ class ReportSchedulePutSchema(Schema):
     )
     chart = fields.Integer(required=False, allow_none=True)
     creation_method = EnumField(
-        ReportCreationMethodType,
+        ReportCreationMethod,
         by_value=True,
         allow_none=True,
         description=creation_method_description,

diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py
@@ -31,7 +31,7 @@
 from superset.models.dashboard import Dashboard
 from superset.models.reports import (
     ReportSchedule,
-    ReportCreationMethodType,
+    ReportCreationMethod,
     ReportRecipients,
     ReportExecutionLog,
     ReportScheduleType,
@@ -452,7 +452,7 @@ def test_create_report_schedule(self):
             "name": "new3",
             "description": "description",
             "crontab": "0 9 * * *",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "recipients": [
                 {
                     "type": ReportRecipientType.EMAIL,
@@ -499,7 +499,7 @@ def test_create_report_schedule_uniqueness(self):
             "type": ReportScheduleType.ALERT,
             "name": "name3",
             "description": "description",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "crontab": "0 9 * * *",
             "chart": chart.id,
             "database": example_db.id,
@@ -516,7 +516,7 @@ def test_create_report_schedule_uniqueness(self):
             "name": "name3",
             "description": "description",
             "crontab": "0 9 * * *",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "chart": chart.id,
         }
         uri = "api/v1/report/"
@@ -546,7 +546,7 @@ def test_create_report_schedule_schema(self):
             "type": ReportScheduleType.REPORT,
             "name": "name3",
             "description": "description",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "crontab": "0 9 * * *",
             "chart": chart.id,
             "database": example_db.id,
@@ -560,7 +560,7 @@ def test_create_report_schedule_schema(self):
             "type": ReportScheduleType.ALERT,
             "name": "new3",
             "description": "description",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "crontab": "0 9 * * *",
             "recipients": [
                 {
@@ -585,7 +585,7 @@ def test_create_report_schedule_schema(self):
             "type": ReportScheduleType.ALERT,
             "name": "new3",
             "description": "description",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "crontab": "0 9 * * *",
             "recipients": [
                 {
@@ -609,7 +609,7 @@ def test_create_report_schedule_schema(self):
             "type": ReportScheduleType.ALERT,
             "name": "new3",
             "description": "description",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "crontab": "0 9 * * *",
             "recipients": [
                 {
@@ -635,7 +635,7 @@ def test_create_report_schedule_schema(self):
             "type": ReportScheduleType.ALERT,
             "name": "new4",
             "description": "description",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "crontab": "0 9 * * *",
             "recipients": [
                 {
@@ -661,7 +661,7 @@ def test_create_report_schedule_schema(self):
             "type": ReportScheduleType.ALERT,
             "name": "new5",
             "description": "description",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "crontab": "0 9 * * *",
             "recipients": [
                 {
@@ -687,7 +687,7 @@ def test_create_report_schedule_schema(self):
             "type": ReportScheduleType.ALERT,
             "name": "new5",
             "description": "description",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "crontab": "0 9 * * *",
             "recipients": [
                 {
@@ -714,7 +714,7 @@ def test_create_report_schedule_schema(self):
             "type": ReportScheduleType.ALERT,
             "name": "new5",
             "description": "description",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "crontab": "0 9 * * *",
             "recipients": [
                 {
@@ -745,7 +745,7 @@ def test_create_report_schedule_schema(self):
             "type": ReportScheduleType.ALERT,
             "name": "new6",
             "description": "description",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "crontab": "0 9 * * *",
             "recipients": [
                 {
@@ -784,7 +784,7 @@ def test_unsaved_report_schedule_schema(self):
             "type": ReportScheduleType.REPORT,
             "name": "name3",
             "description": "description",
-            "creation_method": ReportCreationMethodType.CHARTS,
+            "creation_method": ReportCreationMethod.CHARTS,
             "crontab": "0 9 * * *",
             "chart": 0,
         }
@@ -812,7 +812,7 @@ def test_no_dashboard_report_schedule_schema(self):
             "type": ReportScheduleType.REPORT,
             "name": "name3",
             "description": "description",
-            "creation_method": ReportCreationMethodType.DASHBOARDS,
+            "creation_method": ReportCreationMethod.DASHBOARDS,
             "crontab": "0 9 * * *",
         }
         uri = "api/v1/report/"
@@ -839,7 +839,7 @@ def test_create_multiple_creation_method_report_schedule_charts(self):
             "type": ReportScheduleType.REPORT,
             "name": "name4",
             "description": "description",
-            "creation_method": ReportCreationMethodType.CHARTS,
+            "creation_method": ReportCreationMethod.CHARTS,
             "crontab": "0 9 * * *",
             "working_timeout": 3600,
             "chart": chart.id,
@@ -855,7 +855,7 @@ def test_create_multiple_creation_method_report_schedule_charts(self):
             "type": ReportScheduleType.REPORT,
             "name": "name5",
             "description": "description",
-            "creation_method": ReportCreationMethodType.CHARTS,
+            "creation_method": ReportCreationMethod.CHARTS,
             "crontab": "0 9 * * *",
             "working_timeout": 3600,
             "chart": chart.id,
@@ -897,7 +897,7 @@ def test_create_multiple_creation_method_report_schedule_dashboards(self):
             "type": ReportScheduleType.REPORT,
             "name": "name4",
             "description": "description",
-            "creation_method": ReportCreationMethodType.DASHBOARDS,
+            "creation_method": ReportCreationMethod.DASHBOARDS,
             "crontab": "0 9 * * *",
             "working_timeout": 3600,
             "dashboard": dashboard.id,
@@ -913,7 +913,7 @@ def test_create_multiple_creation_method_report_schedule_dashboards(self):
             "type": ReportScheduleType.REPORT,
             "name": "name5",
             "description": "description",
-            "creation_method": ReportCreationMethodType.DASHBOARDS,
+            "creation_method": ReportCreationMethod.DASHBOARDS,
             "crontab": "0 9 * * *",
             "working_timeout": 3600,
             "dashboard": dashboard.id,
@@ -956,7 +956,7 @@ def test_create_report_schedule_chart_dash_validation(self):
             "name": "new3",
             "description": "description",
             "crontab": "0 9 * * *",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "chart": chart.id,
             "dashboard": dashboard.id,
             "database": example_db.id,
@@ -980,7 +980,7 @@ def test_create_report_schedule_chart_db_validation(self):
             "type": ReportScheduleType.ALERT,
             "name": "new3",
             "description": "description",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "crontab": "0 9 * * *",
             "chart": chart.id,
         }
@@ -1006,7 +1006,7 @@ def test_create_report_schedule_relations_exist(self):
             "type": ReportScheduleType.ALERT,
             "name": "new3",
             "description": "description",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "crontab": "0 9 * * *",
             "chart": chart_max_id + 1,
             "database": database_max_id + 1,
@@ -1029,7 +1029,7 @@ def test_create_report_schedule_relations_exist(self):
             "name": "new3",
             "description": "description",
             "crontab": "0 9 * * *",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "dashboard": dashboard_max_id + 1,
             "database": examples_db.id,
         }
@@ -1546,7 +1546,7 @@ def test_when_invalid_tab_ids_are_given_it_raises_bad_request(self):
             "name": "new3",
             "description": "description",
             "crontab": "0 9 * * *",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "recipients": [
                 {
                     "type": ReportRecipientType.EMAIL,
@@ -1581,7 +1581,7 @@ def test_when_tab_ids_are_given_it_gets_added_to_extra(self):
             "name": "new3",
             "description": "description",
             "crontab": "0 9 * * *",
-            "creation_method": ReportCreationMethodType.ALERTS_REPORTS,
+            "creation_method": ReportCreationMethod.ALERTS_REPORTS,
             "recipients": [
                 {
                     "type": ReportRecipientType.EMAIL,