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 41831c40729a9a98bb3825474a294e4eed805a70 Author: Beto Dealmeida <[email protected]> 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, + ) + )
