Skip to content

Commit

Permalink
feat: customize screenshot width for alerts/reports (#24547)
Browse files Browse the repository at this point in the history
  • Loading branch information
betodealmeida authored Jun 29, 2023
1 parent 8ba0b81 commit be9eb0f
Show file tree
Hide file tree
Showing 12 changed files with 193 additions and 18 deletions.
28 changes: 28 additions & 0 deletions superset-frontend/src/components/ReportModal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import LabeledErrorBoundInput from 'src/components/Form/LabeledErrorBoundInput';
import Icons from 'src/components/Icons';
import { CronError } from 'src/components/CronPicker';
import { RadioChangeEvent } from 'src/components';
import { Input } from 'src/components/Input';
import withToasts from 'src/components/MessageToasts/withToasts';
import { ChartState } from 'src/explore/types';
import {
Expand All @@ -41,9 +42,14 @@ import {
NOTIFICATION_FORMATS,
} from 'src/reports/types';
import { reportSelector } from 'src/views/CRUD/hooks';
import {
TRANSLATIONS,
StyledInputContainer,
} from 'src/features/alerts/AlertReportModal';
import { CreationMethod } from './HeaderReportDropdown';
import {
antDErrorAlertStyles,
CustomWidthHeaderStyle,
StyledModal,
StyledTopSection,
StyledBottomSection,
Expand Down Expand Up @@ -170,6 +176,7 @@ function ReportModal({
type: 'Report',
active: true,
force_screenshot: false,
custom_width: currentReport.custom_width,
creation_method: creationMethod,
dashboard: dashboardId,
chart: chart?.id,
Expand Down Expand Up @@ -257,6 +264,26 @@ function ReportModal({
</div>
</>
);
const renderCustomWidthSection = (
<StyledInputContainer>
<div className="control-label" css={CustomWidthHeaderStyle}>
{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>
</StyledInputContainer>
);

return (
<StyledModal
Expand Down Expand Up @@ -331,6 +358,7 @@ function ReportModal({
}}
/>
{isChart && renderMessageContentSection}
{(!isChart || !isTextBasedChart) && renderCustomWidthSection}
</StyledBottomSection>
{currentReport.error && (
<Alert
Expand Down
4 changes: 4 additions & 0 deletions superset-frontend/src/components/ReportModal/styles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ export const TimezoneHeaderStyle = (theme: SupersetTheme) => css`
margin: ${theme.gridUnit * 3}px 0 ${theme.gridUnit * 2}px;
`;

export const CustomWidthHeaderStyle = (theme: SupersetTheme) => css`
margin: ${theme.gridUnit * 3}px 0 ${theme.gridUnit * 2}px;
`;

export const SectionHeaderStyle = (theme: SupersetTheme) => css`
margin: ${theme.gridUnit * 3}px 0;
`;
Expand Down
47 changes: 40 additions & 7 deletions superset-frontend/src/features/alerts/AlertReportModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import rison from 'rison';
import { useSingleViewResource } from 'src/views/CRUD/hooks';

import Icons from 'src/components/Icons';
import { Input } from 'src/components/Input';
import { Switch } from 'src/components/Switch';
import Modal from 'src/components/Modal';
import TimezoneSelector from 'src/components/TimezoneSelector';
Expand All @@ -46,6 +47,7 @@ import Owner from 'src/types/Owner';
import { AntdCheckbox, AsyncSelect, Select } from 'src/components';
import TextAreaControl from 'src/explore/components/controls/TextAreaControl';
import { useCommonConf } from 'src/features/databases/state';
import { CustomWidthHeaderStyle } from 'src/components/ReportModal/styles';
import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
import {
NotificationMethodOption,
Expand Down Expand Up @@ -370,7 +372,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'),
Expand Down Expand Up @@ -406,7 +408,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'),
};

Expand Down Expand Up @@ -466,6 +470,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[]>([]);
Expand Down Expand Up @@ -853,12 +865,15 @@ 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 {
target: { type, value, name },
} = event;
const parsedValue = type === 'number' ? parseInt(value, 10) || null : value;

updateAlertState(target.name, target.value);
updateAlertState(name, parsedValue);
};

const onTimeoutVerifyChange = (
Expand Down Expand Up @@ -1180,7 +1195,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
? TRANSLATIONS.REPORT_NAME_TEXT
: TRANSLATIONS.ALERT_NAME_TEXT
}
onChange={onTextChange}
onChange={onInputChange}
css={inputSpacer}
/>
</div>
Expand Down Expand Up @@ -1216,7 +1231,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
name="description"
value={currentAlert ? currentAlert.description || '' : ''}
placeholder={TRANSLATIONS.DESCRIPTION_TEXT}
onChange={onTextChange}
onChange={onInputChange}
css={inputSpacer}
/>
</div>
Expand Down Expand Up @@ -1471,6 +1486,24 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
</div>
</>
)}
{isScreenshot && (
<StyledInputContainer>
<div className="control-label" css={CustomWidthHeaderStyle}>
{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>
)}
{(isReport || contentType === 'dashboard') && (
<div className="inline-container">
<StyledCheckbox
Expand Down
3 changes: 2 additions & 1 deletion superset-frontend/src/features/alerts/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -91,7 +93,6 @@ export type AlertObject = {
};
validator_type?: string;
working_timeout?: number;
error?: string;
};

export type LogObject = {
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/reports/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,6 @@ export interface ReportObject {
working_timeout: number;
creation_method: string;
force_screenshot: boolean;
custom_width?: number | null;
error?: string;
}
4 changes: 2 additions & 2 deletions superset/charts/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
from superset.models.slice import Slice
from superset.tasks.thumbnails import cache_chart_thumbnail
from superset.tasks.utils import get_current_user
from superset.utils.screenshots import ChartScreenshot
from superset.utils.screenshots import ChartScreenshot, DEFAULT_CHART_WINDOW_SIZE
from superset.utils.urls import get_url_path
from superset.views.base_api import (
BaseSupersetModelRestApi,
Expand Down Expand Up @@ -573,7 +573,7 @@ def cache_screenshot(self, pk: int, **kwargs: Any) -> WerkzeugResponse:
$ref: '#/components/responses/500'
"""
rison_dict = kwargs["rison"]
window_size = rison_dict.get("window_size") or (800, 600)
window_size = rison_dict.get("window_size") or DEFAULT_CHART_WINDOW_SIZE

# Don't shrink the image if thumb_size is not specified
thumb_size = rison_dict.get("thumb_size") or window_size
Expand Down
3 changes: 3 additions & 0 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1273,6 +1273,9 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
# 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] "
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
"""Add custom size columns to report schedule
Revision ID: 8e5b0fb85b9a
Revises: 6fbe660cac39
Create Date: 2023-06-27 16:54:57.161475
"""

import sqlalchemy as sa
from alembic import op

# revision identifiers, used by Alembic.
revision = "8e5b0fb85b9a"
down_revision = "6fbe660cac39"


def upgrade():
op.add_column(
"report_schedule",
sa.Column("custom_width", sa.Integer(), nullable=True),
)
op.add_column(
"report_schedule",
sa.Column("custom_height", sa.Integer(), nullable=True),
)


def downgrade():
op.drop_column("report_schedule", "custom_width")
op.drop_column("report_schedule", "custom_height")
2 changes: 2 additions & 0 deletions superset/reports/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ def ensure_alert_reports_enabled(self) -> Optional[Response]:
"context_markdown",
"creation_method",
"crontab",
"custom_width",
"dashboard.dashboard_title",
"dashboard.id",
"database.database_name",
Expand Down Expand Up @@ -159,6 +160,7 @@ def ensure_alert_reports_enabled(self) -> Optional[Response]:
"context_markdown",
"creation_method",
"crontab",
"custom_width",
"dashboard",
"database",
"description",
Expand Down
3 changes: 3 additions & 0 deletions superset/reports/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ class ReportSchedule(Model, AuditMixinNullable, ExtraJSONMixin):
# (Reports) When generating a screenshot, bypass the cache?
force_screenshot = Column(Boolean, default=False)

custom_width = Column(Integer, nullable=True)
custom_height = Column(Integer, nullable=True)

extra: ReportScheduleExtra # type: ignore

def __repr__(self) -> str:
Expand Down
52 changes: 50 additions & 2 deletions superset/reports/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
)
)
Loading

0 comments on commit be9eb0f

Please sign in to comment.