You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2023/06/29 01:08:41 UTC

[superset] 03/05: Add affordances

This is an automated email from the ASF dual-hosted git repository.

beto pushed a commit to branch customize_screenshot_width
in repository https://gitbox.apache.org/repos/asf/superset.git

commit dbe32a207b4f6dc2e15277561cea190fb9774af2
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Wed Jun 28 16:03:38 2023 -0700

    Add affordances
---
 .../src/components/ReportModal/index.tsx           | 23 ++++++++
 .../src/features/alerts/AlertReportModal.tsx       | 64 ++++++++++++++++------
 superset-frontend/src/features/alerts/types.ts     |  3 +-
 superset-frontend/src/reports/types.ts             |  1 +
 superset/config.py                                 |  3 +
 superset/reports/api.py                            |  2 +
 superset/reports/schemas.py                        | 52 +++++++++++++++++-
 7 files changed, 128 insertions(+), 20 deletions(-)

diff --git a/superset-frontend/src/components/ReportModal/index.tsx b/superset-frontend/src/components/ReportModal/index.tsx
index 5bf4c2c4d2..cb229f1f3f 100644
--- a/superset-frontend/src/components/ReportModal/index.tsx
+++ b/superset-frontend/src/components/ReportModal/index.tsx
@@ -41,6 +41,7 @@ import {
   NOTIFICATION_FORMATS,
 } from 'src/reports/types';
 import { reportSelector } from 'src/views/CRUD/hooks';
+import { TRANSLATIONS } from 'src/features/alerts/AlertReportModal';
 import { CreationMethod } from './HeaderReportDropdown';
 import {
   antDErrorAlertStyles,
@@ -170,6 +171,7 @@ function ReportModal({
       type: 'Report',
       active: true,
       force_screenshot: false,
+      custom_width: currentReport.custom_width,
       creation_method: creationMethod,
       dashboard: dashboardId,
       chart: chart?.id,
@@ -257,6 +259,26 @@ function ReportModal({
       </div>
     </>
   );
+  const renderCustomWidthSection = (
+    <div>
+      <div className="control-label">
+        {TRANSLATIONS.CUSTOM_SCREENSHOT_WIDTH_TEXT}
+      </div>
+      <div className="input-container">
+        <input
+          type="number"
+          name="custom_width"
+          value={currentReport?.custom_width || ''}
+          placeholder={TRANSLATIONS.CUSTOM_SCREENSHOT_WIDTH_PLACEHOLDER_TEXT}
+          onChange={(event: React.ChangeEvent<HTMLInputElement>) => {
+            setCurrentReport({
+              custom_width: parseInt(event.target.value, 10) || null,
+            });
+          }}
+        />
+      </div>
+    </div>
+  );
 
   return (
     <StyledModal
@@ -331,6 +353,7 @@ function ReportModal({
           }}
         />
         {isChart && renderMessageContentSection}
+        {(!isChart || !isTextBasedChart) && renderCustomWidthSection}
       </StyledBottomSection>
       {currentReport.error && (
         <Alert
diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx
index 8ff344c97e..0aeae9103e 100644
--- a/superset-frontend/src/features/alerts/AlertReportModal.tsx
+++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx
@@ -370,7 +370,7 @@ interface NotificationMethodAddProps {
   onClick: () => void;
 }
 
-const TRANSLATIONS = {
+export const TRANSLATIONS = {
   ADD_NOTIFICATION_METHOD_TEXT: t('Add notification method'),
   ADD_DELIVERY_METHOD_TEXT: t('Add delivery method'),
   SAVE_TEXT: t('Save'),
@@ -406,7 +406,9 @@ const TRANSLATIONS = {
   SEND_AS_PNG_TEXT: t('Send as PNG'),
   SEND_AS_CSV_TEXT: t('Send as CSV'),
   SEND_AS_TEXT: t('Send as text'),
-  IGNORE_CACHE_TEXT: t('Ignore cache when generating screenshot'),
+  IGNORE_CACHE_TEXT: t('Ignore cache when generating report'),
+  CUSTOM_SCREENSHOT_WIDTH_TEXT: t('Screenshot width'),
+  CUSTOM_SCREENSHOT_WIDTH_PLACEHOLDER_TEXT: t('Input custom width in pixels'),
   NOTIFICATION_METHOD_TEXT: t('Notification method'),
 };
 
@@ -466,6 +468,14 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
   );
   const [forceScreenshot, setForceScreenshot] = useState<boolean>(false);
 
+  const [isScreenshot, setIsScreenshot] = useState<boolean>(false);
+  useEffect(() => {
+    setIsScreenshot(
+      contentType === 'dashboard' ||
+        (contentType === 'chart' && reportFormat === 'PNG'),
+    );
+  }, [contentType, reportFormat]);
+
   // Dropdown options
   const [conditionNotNull, setConditionNotNull] = useState<boolean>(false);
   const [sourceOptions, setSourceOptions] = useState<MetaObject[]>([]);
@@ -853,12 +863,16 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
     }).then(response => setChartVizType(response.json.result.viz_type));
 
   // Handle input/textarea updates
-  const onTextChange = (
+  const onInputChange = (
     event: React.ChangeEvent<HTMLTextAreaElement | HTMLInputElement>,
   ) => {
     const { target } = event;
+    const value =
+      target.type === 'number'
+        ? parseInt(target.value, 10) || null
+        : target.value;
 
-    updateAlertState(target.name, target.value);
+    updateAlertState(target.name, value);
   };
 
   const onTimeoutVerifyChange = (
@@ -1180,7 +1194,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
                     ? TRANSLATIONS.REPORT_NAME_TEXT
                     : TRANSLATIONS.ALERT_NAME_TEXT
                 }
-                onChange={onTextChange}
+                onChange={onInputChange}
                 css={inputSpacer}
               />
             </div>
@@ -1216,7 +1230,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
                 name="description"
                 value={currentAlert ? currentAlert.description || '' : ''}
                 placeholder={TRANSLATIONS.DESCRIPTION_TEXT}
-                onChange={onTextChange}
+                onChange={onInputChange}
                 css={inputSpacer}
               />
             </div>
@@ -1471,18 +1485,34 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
                 </div>
               </>
             )}
-            {(isReport || contentType === 'dashboard') && (
-              <div className="inline-container">
-                <StyledCheckbox
-                  data-test="bypass-cache"
-                  className="checkbox"
-                  checked={forceScreenshot}
-                  onChange={onForceScreenshotChange}
-                >
-                  {TRANSLATIONS.IGNORE_CACHE_TEXT}
-                </StyledCheckbox>
-              </div>
+            {isScreenshot && (
+              <StyledInputContainer>
+                <div className="control-label">
+                  {TRANSLATIONS.CUSTOM_SCREENSHOT_WIDTH_TEXT}
+                </div>
+                <div className="input-container">
+                  <input
+                    type="number"
+                    name="custom_width"
+                    value={currentAlert?.custom_width || ''}
+                    placeholder={
+                      TRANSLATIONS.CUSTOM_SCREENSHOT_WIDTH_PLACEHOLDER_TEXT
+                    }
+                    onChange={onInputChange}
+                  />
+                </div>
+              </StyledInputContainer>
             )}
+            <div className="inline-container">
+              <StyledCheckbox
+                data-test="bypass-cache"
+                className="checkbox"
+                checked={forceScreenshot}
+                onChange={onForceScreenshotChange}
+              >
+                {TRANSLATIONS.IGNORE_CACHE_TEXT}
+              </StyledCheckbox>
+            </div>
             <StyledSectionTitle>
               <h4>{TRANSLATIONS.NOTIFICATION_METHOD_TEXT}</h4>
               <span className="required">*</span>
diff --git a/superset-frontend/src/features/alerts/types.ts b/superset-frontend/src/features/alerts/types.ts
index 36d2b1d35a..34eb7fd261 100644
--- a/superset-frontend/src/features/alerts/types.ts
+++ b/superset-frontend/src/features/alerts/types.ts
@@ -68,10 +68,12 @@ export type AlertObject = {
   created_by?: user;
   created_on?: string;
   crontab?: string;
+  custom_width?: number | null;
   dashboard?: MetaObject;
   dashboard_id?: number;
   database?: MetaObject;
   description?: string;
+  error?: string;
   force_screenshot: boolean;
   grace_period?: number;
   id: number;
@@ -91,7 +93,6 @@ export type AlertObject = {
   };
   validator_type?: string;
   working_timeout?: number;
-  error?: string;
 };
 
 export type LogObject = {
diff --git a/superset-frontend/src/reports/types.ts b/superset-frontend/src/reports/types.ts
index 38cb3865cf..b67a7bac7b 100644
--- a/superset-frontend/src/reports/types.ts
+++ b/superset-frontend/src/reports/types.ts
@@ -56,5 +56,6 @@ export interface ReportObject {
   working_timeout: number;
   creation_method: string;
   force_screenshot: boolean;
+  custom_width?: number | null;
   error?: string;
 }
diff --git a/superset/config.py b/superset/config.py
index abb73e9f56..7c05e925d7 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -1273,6 +1273,9 @@ ALERT_REPORTS_NOTIFICATION_DRY_RUN = False
 # Max tries to run queries to prevent false errors caused by transient errors
 # being returned to users. Set to a value >1 to enable retries.
 ALERT_REPORTS_QUERY_EXECUTION_MAX_TRIES = 1
+# Custom width for screenshots
+ALERT_REPORTS_MIN_CUSTOM_SCREENSHOT_WIDTH = 600
+ALERT_REPORTS_MAX_CUSTOM_SCREENSHOT_WIDTH = 2400
 
 # A custom prefix to use on all Alerts & Reports emails
 EMAIL_REPORTS_SUBJECT_PREFIX = "[Report] "
diff --git a/superset/reports/api.py b/superset/reports/api.py
index 125a3e6763..3686ab74bd 100644
--- a/superset/reports/api.py
+++ b/superset/reports/api.py
@@ -93,6 +93,7 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi):
         "context_markdown",
         "creation_method",
         "crontab",
+        "custom_width",
         "dashboard.dashboard_title",
         "dashboard.id",
         "database.database_name",
@@ -159,6 +160,7 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi):
         "context_markdown",
         "creation_method",
         "crontab",
+        "custom_width",
         "dashboard",
         "database",
         "description",
diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py
index fbe681be36..7bdbf34f12 100644
--- a/superset/reports/schemas.py
+++ b/superset/reports/schemas.py
@@ -17,8 +17,9 @@
 from typing import Any, Union
 
 from croniter import croniter
+from flask import current_app
 from flask_babel import gettext as _
-from marshmallow import fields, Schema, validate, validates_schema
+from marshmallow import fields, Schema, validate, validates, validates_schema
 from marshmallow.validate import Length, Range, ValidationError
 from pytz import all_timezones
 
@@ -208,10 +209,34 @@ class ReportSchedulePostSchema(Schema):
         dump_default=None,
     )
     force_screenshot = fields.Boolean(dump_default=False)
+    custom_width = fields.Integer(
+        metadata={
+            "description": _("Custom width of the screenshot in pixels"),
+            "example": 1000,
+        },
+        allow_none=True,
+        required=False,
+        default=None,
+    )
+
+    @validates("custom_width")
+    def validate_custom_width(self, value: int) -> None:  # pylint: disable=no-self-use
+        min_width = current_app.config["ALERT_REPORTS_MIN_CUSTOM_SCREENSHOT_WIDTH"]
+        max_width = current_app.config["ALERT_REPORTS_MAX_CUSTOM_SCREENSHOT_WIDTH"]
+        if not min_width <= value <= max_width:
+            raise ValidationError(
+                _(
+                    "Screenshot width must be between %(min)spx and %(max)spx",
+                    min=min_width,
+                    max=max_width,
+                )
+            )
 
     @validates_schema
     def validate_report_references(  # pylint: disable=unused-argument,no-self-use
-        self, data: dict[str, Any], **kwargs: Any
+        self,
+        data: dict[str, Any],
+        **kwargs: Any,
     ) -> None:
         if data["type"] == ReportScheduleType.REPORT:
             if "database" in data:
@@ -307,3 +332,26 @@ class ReportSchedulePutSchema(Schema):
     )
     extra = fields.Dict(dump_default=None)
     force_screenshot = fields.Boolean(dump_default=False)
+
+    custom_width = fields.Integer(
+        metadata={
+            "description": _("Custom width of the screenshot in pixels"),
+            "example": 1000,
+        },
+        allow_none=True,
+        required=False,
+        default=None,
+    )
+
+    @validates("custom_width")
+    def validate_custom_width(self, value: int) -> None:  # pylint: disable=no-self-use
+        min_width = current_app.config["ALERT_REPORTS_MIN_CUSTOM_SCREENSHOT_WIDTH"]
+        max_width = current_app.config["ALERT_REPORTS_MAX_CUSTOM_SCREENSHOT_WIDTH"]
+        if not min_width <= value <= max_width:
+            raise ValidationError(
+                _(
+                    "Screenshot width must be between %(min)spx and %(max)spx",
+                    min=min_width,
+                    max=max_width,
+                )
+            )