feat: configure force_screenshot

apache · Dec 22, 2021 · 61e0148 · 61e0148
1 parent 63096e7
commit 61e0148
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 79 deletions.
diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx
@@ -42,6 +42,7 @@ import Select, { propertyComparator } from 'src/components/Select/Select';
 import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
 import withToasts from 'src/components/MessageToasts/withToasts';
 import Owner from 'src/types/Owner';
+import { Checkbox } from 'src/common/components';
 import TextAreaControl from 'src/explore/components/controls/TextAreaControl';
 import { useCommonConf } from 'src/views/CRUD/data/database/state';
 import {
@@ -341,6 +342,10 @@ const StyledRadioGroup = styled(Radio.Group)`
   margin-left: ${({ theme }) => theme.gridUnit * 5.5}px;
 `;
 
+const StyledCheckbox = styled(Checkbox)`
+  margin-left: ${({ theme }) => theme.gridUnit * 5.5}px;
+`;
+
 // Notification Method components
 const StyledNotificationAddButton = styled.div`
   color: ${({ theme }) => theme.colors.primary.dark1};
@@ -417,6 +422,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
   const [reportFormat, setReportFormat] = useState<string>(
     DEFAULT_NOTIFICATION_FORMAT,
   );
+  const [forceScreenshot, setForceScreenshot] = useState<boolean>(false);
 
   // Dropdown options
   const [conditionNotNull, setConditionNotNull] = useState<boolean>(false);
@@ -513,7 +519,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
     const data: any = {
       ...currentAlert,
       type: isReport ? 'Report' : 'Alert',
-      force_screenshot: contentType === 'chart' && !isReport ? 'true' : 'false',
+      force_screenshot: forceScreenshot ? 'true' : 'false',
       validator_type: conditionNotNull ? 'not null' : 'operator',
       validator_config_json: conditionNotNull
         ? {}
@@ -866,6 +872,10 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
     setReportFormat(target.value);
   };
 
+  const onForceScreenshotChange = (event: any) => {
+    setForceScreenshot(event.target.checked);
+  };
+
   // Make sure notification settings has the required info
   const checkNotificationSettings = () => {
     if (!notificationSettings.length) {
@@ -927,6 +937,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
       (!currentAlert || currentAlert.id || (isHidden && show))
     ) {
       setCurrentAlert({ ...DEFAULT_ALERT });
+      setForceScreenshot(contentType === 'chart' && !isReport);
       setNotificationSettings([]);
       setNotificationAddState('active');
     }
@@ -970,6 +981,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
       if (resource.chart) {
         setChartVizType((resource.chart as ChartObject).viz_type);
       }
+      setForceScreenshot(resource.force_screenshot);
 
       setCurrentAlert({
         ...resource,
@@ -1339,18 +1351,33 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
               onChange={onDashboardChange}
             />
             {formatOptionEnabled && (
-              <div className="inline-container">
-                <StyledRadioGroup
-                  onChange={onFormatChange}
-                  value={reportFormat}
-                >
-                  <StyledRadio value="PNG">{t('Send as PNG')}</StyledRadio>
-                  <StyledRadio value="CSV">{t('Send as CSV')}</StyledRadio>
-                  {TEXT_BASED_VISUALIZATION_TYPES.includes(chartVizType) && (
-                    <StyledRadio value="TEXT">{t('Send as text')}</StyledRadio>
-                  )}
-                </StyledRadioGroup>
-              </div>
+              <>
+                <div className="inline-container">
+                  <StyledRadioGroup
+                    onChange={onFormatChange}
+                    value={reportFormat}
+                  >
+                    <StyledRadio value="PNG">{t('Send as PNG')}</StyledRadio>
+                    <StyledRadio value="CSV">{t('Send as CSV')}</StyledRadio>
+                    {TEXT_BASED_VISUALIZATION_TYPES.includes(chartVizType) && (
+                      <StyledRadio value="TEXT">
+                        {t('Send as text')}
+                      </StyledRadio>
+                    )}
+                  </StyledRadioGroup>
+                </div>
+                {isReport && (
+                  <div className="inline-container">
+                    <StyledCheckbox
+                      className="checkbox"
+                      checked={forceScreenshot}
+                      onChange={onForceScreenshotChange}
+                    >
+                      Ignore cache when generating screenshot
+                    </StyledCheckbox>
+                  </div>
+                )}
+              </>
             )}
             <StyledSectionTitle>
               <h4>{t('Notification method')}</h4>

diff --git a/superset-frontend/src/views/CRUD/alert/types.ts b/superset-frontend/src/views/CRUD/alert/types.ts
@@ -68,6 +68,7 @@ export type AlertObject = {
   dashboard?: MetaObject;
   database?: MetaObject;
   description?: string;
+  force_screenshot: boolean;
   grace_period?: number;
   id: number;
   last_eval_dttm?: number;

diff --git a/superset/db_engine_specs/postgres.py b/superset/db_engine_specs/postgres.py
@@ -195,16 +195,8 @@ class PostgresEngineSpec(PostgresBaseEngineSpec, BasicParametersMixin):
             lambda match: ARRAY(int(match[2])) if match[2] else String(),
             utils.GenericDataType.STRING,
         ),
-        (
-            re.compile(r"^json.*", re.IGNORECASE),
-            JSON(),
-            utils.GenericDataType.STRING,
-        ),
-        (
-            re.compile(r"^enum.*", re.IGNORECASE),
-            ENUM(),
-            utils.GenericDataType.STRING,
-        ),
+        (re.compile(r"^json.*", re.IGNORECASE), JSON(), utils.GenericDataType.STRING,),
+        (re.compile(r"^enum.*", re.IGNORECASE), ENUM(), utils.GenericDataType.STRING,),
     )
 
     @classmethod

diff --git a/superset/reports/api.py b/superset/reports/api.py
@@ -92,6 +92,7 @@ def ensure_alert_reports_enabled(self) -> Optional[Response]:
         "database.database_name",
         "database.id",
         "description",
+        "force_screenshot",
         "grace_period",
         "last_eval_dttm",
         "last_state",
@@ -151,6 +152,7 @@ def ensure_alert_reports_enabled(self) -> Optional[Response]:
         "dashboard",
         "database",
         "description",
+        "force_screenshot",
         "grace_period",
         "log_retention",
         "name",

diff --git a/superset/reports/commands/execute.py b/superset/reports/commands/execute.py
@@ -95,9 +95,7 @@ def __init__(
         self._execution_id = execution_id
 
     def set_state_and_log(
-        self,
-        state: ReportState,
-        error_message: Optional[str] = None,
+        self, state: ReportState, error_message: Optional[str] = None,
     ) -> None:
         """
         Updates current ReportSchedule state and TS. If on final state writes the log
@@ -106,8 +104,7 @@ def set_state_and_log(
         now_dttm = datetime.utcnow()
         self.set_state(state, now_dttm)
         self.create_log(
-            state,
-            error_message=error_message,
+            state, error_message=error_message,
         )
 
     def set_state(self, state: ReportState, dttm: datetime) -> None:
@@ -521,14 +518,12 @@ def next(self) -> None:
         if self.is_on_working_timeout():
             exception_timeout = ReportScheduleWorkingTimeoutError()
             self.set_state_and_log(
-                ReportState.ERROR,
-                error_message=str(exception_timeout),
+                ReportState.ERROR, error_message=str(exception_timeout),
             )
             raise exception_timeout
         exception_working = ReportSchedulePreviousWorkingError()
         self.set_state_and_log(
-            ReportState.WORKING,
-            error_message=str(exception_working),
+            ReportState.WORKING, error_message=str(exception_working),
         )
         raise exception_working
 

diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py
@@ -382,9 +382,7 @@ def create_alert_slack_chart_success():
 
 
 @pytest.fixture(
-    params=[
-        "alert1",
-    ]
+    params=["alert1",]
 )
 def create_alert_slack_chart_grace(request):
     param_config = {
@@ -662,9 +660,7 @@ def create_invalid_sql_alert_email_chart(request):
 @patch("superset.reports.notifications.email.send_email_smtp")
 @patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
 def test_email_chart_report_schedule(
-    screenshot_mock,
-    email_mock,
-    create_report_email_chart,
+    screenshot_mock, email_mock, create_report_email_chart,
 ):
     """
     ExecuteReport Command: Test chart email report schedule with screenshot
@@ -704,9 +700,7 @@ def test_email_chart_report_schedule(
 @patch("superset.reports.notifications.email.send_email_smtp")
 @patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
 def test_email_chart_report_schedule_force_screenshot(
-    screenshot_mock,
-    email_mock,
-    create_report_email_chart_force_screenshot,
+    screenshot_mock, email_mock, create_report_email_chart_force_screenshot,
 ):
     """
     ExecuteReport Command: Test chart email report schedule with screenshot
@@ -748,9 +742,7 @@ def test_email_chart_report_schedule_force_screenshot(
 @patch("superset.reports.notifications.email.send_email_smtp")
 @patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
 def test_email_chart_alert_schedule(
-    screenshot_mock,
-    email_mock,
-    create_alert_email_chart,
+    screenshot_mock, email_mock, create_alert_email_chart,
 ):
     """
     ExecuteReport Command: Test chart email alert schedule with screenshot
@@ -787,9 +779,7 @@ def test_email_chart_alert_schedule(
 @patch("superset.reports.notifications.email.send_email_smtp")
 @patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
 def test_email_chart_report_dry_run(
-    screenshot_mock,
-    email_mock,
-    create_report_email_chart,
+    screenshot_mock, email_mock, create_report_email_chart,
 ):
     """
     ExecuteReport Command: Test chart email report schedule dry run
@@ -814,11 +804,7 @@ def test_email_chart_report_dry_run(
 @patch("superset.reports.notifications.email.send_email_smtp")
 @patch("superset.utils.csv.get_chart_csv_data")
 def test_email_chart_report_schedule_with_csv(
-    csv_mock,
-    email_mock,
-    mock_open,
-    mock_urlopen,
-    create_report_email_chart_with_csv,
+    csv_mock, email_mock, mock_open, mock_urlopen, create_report_email_chart_with_csv,
 ):
     """
     ExecuteReport Command: Test chart email report schedule with CSV
@@ -1007,9 +993,7 @@ def test_email_dashboard_report_schedule(
 @patch("superset.reports.notifications.slack.WebClient.files_upload")
 @patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
 def test_slack_chart_report_schedule(
-    screenshot_mock,
-    file_upload_mock,
-    create_report_slack_chart,
+    screenshot_mock, file_upload_mock, create_report_slack_chart,
 ):
     """
     ExecuteReport Command: Test chart slack report schedule
@@ -1228,9 +1212,7 @@ def test_report_schedule_success_grace_end(
 @patch("superset.reports.notifications.email.send_email_smtp")
 @patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
 def test_alert_limit_is_applied(
-    screenshot_mock,
-    email_mock,
-    create_alert_email_chart,
+    screenshot_mock, email_mock, create_alert_email_chart,
 ):
     """
     ExecuteReport Command: Test that all alerts apply a SQL limit to stmts
@@ -1286,9 +1268,7 @@ def test_email_dashboard_report_fails(
     ALERTS_ATTACH_REPORTS=True,
 )
 def test_slack_chart_alert(
-    screenshot_mock,
-    email_mock,
-    create_alert_email_chart,
+    screenshot_mock, email_mock, create_alert_email_chart,
 ):
     """
     ExecuteReport Command: Test chart slack alert
@@ -1345,9 +1325,7 @@ def test_slack_chart_alert_no_attachment(email_mock, create_alert_email_chart):
 @patch("superset.reports.notifications.slack.WebClient")
 @patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
 def test_slack_token_callable_chart_report(
-    screenshot_mock,
-    slack_client_mock_class,
-    create_report_slack_chart,
+    screenshot_mock, slack_client_mock_class, create_report_slack_chart,
 ):
     """
     ExecuteReport Command: Test chart slack alert (slack token callable)
@@ -1459,11 +1437,7 @@ def test_soft_timeout_screenshot(screenshot_mock, email_mock, create_alert_email
 @patch("superset.reports.notifications.email.send_email_smtp")
 @patch("superset.utils.csv.get_chart_csv_data")
 def test_soft_timeout_csv(
-    csv_mock,
-    email_mock,
-    mock_open,
-    mock_urlopen,
-    create_report_email_chart_with_csv,
+    csv_mock, email_mock, mock_open, mock_urlopen, create_report_email_chart_with_csv,
 ):
     """
     ExecuteReport Command: Test fail on generating csv
@@ -1487,8 +1461,7 @@ def test_soft_timeout_csv(
     assert email_mock.call_args[0][0] == OWNER_EMAIL
 
     assert_log(
-        ReportState.ERROR,
-        error_message="A timeout occurred while generating a csv.",
+        ReportState.ERROR, error_message="A timeout occurred while generating a csv.",
     )
 
 
@@ -1500,11 +1473,7 @@ def test_soft_timeout_csv(
 @patch("superset.reports.notifications.email.send_email_smtp")
 @patch("superset.utils.csv.get_chart_csv_data")
 def test_generate_no_csv(
-    csv_mock,
-    email_mock,
-    mock_open,
-    mock_urlopen,
-    create_report_email_chart_with_csv,
+    csv_mock, email_mock, mock_open, mock_urlopen, create_report_email_chart_with_csv,
 ):
     """
     ExecuteReport Command: Test fail on generating csv
@@ -1689,9 +1658,7 @@ def test_grace_period_error(email_mock, create_invalid_sql_alert_email_chart):
 @patch("superset.reports.notifications.email.send_email_smtp")
 @patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
 def test_grace_period_error_flap(
-    screenshot_mock,
-    email_mock,
-    create_invalid_sql_alert_email_chart,
+    screenshot_mock, email_mock, create_invalid_sql_alert_email_chart,
 ):
     """
     ExecuteReport Command: Test alert grace period on error