From 9baeafec9308ddafd3f962dbf609958e9b1963ab Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 22 Dec 2021 13:46:00 -0800 Subject: [PATCH] feat: configure force_screenshot (#17855) * Update existing tests * Add backend test * feat: add force option to report screenshots * Add tests * Rebase fixes * Do not force screenshot on dashboard alerts * feat: bypass cache on screenshots for alerts * Update existing tests * Add tests * feat: configure force_screenshot --- .../src/views/CRUD/alert/AlertReportModal.tsx | 53 ++++++++++++++----- .../src/views/CRUD/alert/types.ts | 1 + superset/reports/api.py | 2 + superset/reports/commands/execute.py | 2 - 4 files changed, 43 insertions(+), 15 deletions(-) diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx index 84021484fe..65020aeabf 100644 --- 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 = ({ const [reportFormat, setReportFormat] = useState( DEFAULT_NOTIFICATION_FORMAT, ); + const [forceScreenshot, setForceScreenshot] = useState(false); // Dropdown options const [conditionNotNull, setConditionNotNull] = useState(false); @@ -513,7 +519,7 @@ const AlertReportModal: FunctionComponent = ({ 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 = ({ 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 = ({ (!currentAlert || currentAlert.id || (isHidden && show)) ) { setCurrentAlert({ ...DEFAULT_ALERT }); + setForceScreenshot(contentType === 'chart' && !isReport); setNotificationSettings([]); setNotificationAddState('active'); } @@ -970,6 +981,7 @@ const AlertReportModal: FunctionComponent = ({ if (resource.chart) { setChartVizType((resource.chart as ChartObject).viz_type); } + setForceScreenshot(resource.force_screenshot); setCurrentAlert({ ...resource, @@ -1339,18 +1351,33 @@ const AlertReportModal: FunctionComponent = ({ onChange={onDashboardChange} /> {formatOptionEnabled && ( -
- - {t('Send as PNG')} - {t('Send as CSV')} - {TEXT_BASED_VISUALIZATION_TYPES.includes(chartVizType) && ( - {t('Send as text')} - )} - -
+ <> +
+ + {t('Send as PNG')} + {t('Send as CSV')} + {TEXT_BASED_VISUALIZATION_TYPES.includes(chartVizType) && ( + + {t('Send as text')} + + )} + +
+ {isReport && ( +
+ + Ignore cache when generating screenshot + +
+ )} + )}

{t('Notification method')}

diff --git a/superset-frontend/src/views/CRUD/alert/types.ts b/superset-frontend/src/views/CRUD/alert/types.ts index 1c40cdb1f9..ef320b5f7b 100644 --- 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/reports/api.py b/superset/reports/api.py index 1a5907fba7..7d8d548eb1 100644 --- a/superset/reports/api.py +++ b/superset/reports/api.py @@ -92,6 +92,7 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): "database.database_name", "database.id", "description", + "force_screenshot", "grace_period", "last_eval_dttm", "last_state", @@ -151,6 +152,7 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): "dashboard", "database", "description", + "force_screenshot", "grace_period", "log_retention", "name", diff --git a/superset/reports/commands/execute.py b/superset/reports/commands/execute.py index 687e05c61a..127eaf813c 100644 --- a/superset/reports/commands/execute.py +++ b/superset/reports/commands/execute.py @@ -146,8 +146,6 @@ class BaseReportState: """ Get the url for this report schedule: chart or dashboard """ - # For alerts we always want to send a fresh screenshot, bypassing - # the cache. force = "true" if self._report_schedule.force_screenshot else "false" if self._report_schedule.chart: