From 30b497e7583fad45215850d1235edbc0382f343b Mon Sep 17 00:00:00 2001 From: Jack <41238731+fisjac@users.noreply.github.com> Date: Fri, 22 Mar 2024 11:54:30 -0500 Subject: [PATCH] feat(alerts-reports): adding pdf filetype to email and slack reports (#27497) --- .../src/features/alerts/AlertReportModal.tsx | 39 +++++++-------- superset/commands/report/exceptions.py | 4 ++ superset/commands/report/execute.py | 23 +++++++-- superset/reports/models.py | 7 +-- superset/reports/notifications/base.py | 1 + superset/reports/notifications/email.py | 12 ++++- superset/reports/notifications/slack.py | 15 +++--- superset/reports/schemas.py | 4 +- superset/utils/core.py | 10 ++++ superset/utils/pdf.py | 48 +++++++++++++++++++ .../reports/commands_tests.py | 6 +-- tests/integration_tests/reports/utils.py | 2 +- 12 files changed, 130 insertions(+), 41 deletions(-) create mode 100644 superset/utils/pdf.py diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx index c297da2d71..cdc296d3d4 100644 --- a/superset-frontend/src/features/alerts/AlertReportModal.tsx +++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx @@ -159,6 +159,10 @@ const CONTENT_TYPE_OPTIONS = [ }, ]; const FORMAT_OPTIONS = { + pdf: { + label: t('Send as PDF'), + value: 'PDF', + }, png: { label: t('Send as PNG'), value: 'PNG', @@ -427,11 +431,8 @@ const AlertReportModal: FunctionComponent = ({ const [isScreenshot, setIsScreenshot] = useState(false); useEffect(() => { - setIsScreenshot( - contentType === 'dashboard' || - (contentType === 'chart' && reportFormat === 'PNG'), - ); - }, [contentType, reportFormat]); + setIsScreenshot(reportFormat === 'PNG'); + }, [reportFormat]); // Dropdown options const [conditionNotNull, setConditionNotNull] = useState(false); @@ -487,8 +488,7 @@ const AlertReportModal: FunctionComponent = ({ const reportOrAlert = isReport ? 'report' : 'alert'; const isEditMode = alert !== null; const formatOptionEnabled = - contentType === 'chart' && - (isFeatureEnabled(FeatureFlag.AlertsAttachReports) || isReport); + isFeatureEnabled(FeatureFlag.AlertsAttachReports) || isReport; const [notificationAddState, setNotificationAddState] = useState('active'); @@ -616,10 +616,7 @@ const AlertReportModal: FunctionComponent = ({ owner => (owner as MetaObject).value || owner.id, ), recipients, - report_format: - contentType === 'dashboard' - ? DEFAULT_NOTIFICATION_FORMAT - : reportFormat || DEFAULT_NOTIFICATION_FORMAT, + report_format: reportFormat || DEFAULT_NOTIFICATION_FORMAT, }; if (data.recipients && !data.recipients.length) { @@ -1128,11 +1125,7 @@ const AlertReportModal: FunctionComponent = ({ : 'active', ); setContentType(resource.chart ? 'chart' : 'dashboard'); - setReportFormat( - resource.chart - ? resource.report_format || DEFAULT_NOTIFICATION_FORMAT - : DEFAULT_NOTIFICATION_FORMAT, - ); + setReportFormat(resource.report_format || DEFAULT_NOTIFICATION_FORMAT); const validatorConfig = typeof resource.validator_config_json === 'string' ? JSON.parse(resource.validator_config_json) @@ -1516,7 +1509,9 @@ const AlertReportModal: FunctionComponent = ({ )} {formatOptionEnabled && ( <> @@ -1529,11 +1524,13 @@ const AlertReportModal: FunctionComponent = ({ onChange={onFormatChange} value={reportFormat} options={ - /* If chart is of text based viz type: show text + contentType === 'dashboard' + ? ['pdf', 'png'].map(key => FORMAT_OPTIONS[key]) + : /* If chart is of text based viz type: show text format option */ - TEXT_BASED_VISUALIZATION_TYPES.includes(chartVizType) - ? Object.values(FORMAT_OPTIONS) - : ['png', 'csv'].map(key => FORMAT_OPTIONS[key]) + TEXT_BASED_VISUALIZATION_TYPES.includes(chartVizType) + ? Object.values(FORMAT_OPTIONS) + : ['pdf', 'png', 'csv'].map(key => FORMAT_OPTIONS[key]) } placeholder={t('Select format')} /> diff --git a/superset/commands/report/exceptions.py b/superset/commands/report/exceptions.py index 2d82d5c312..db929c63a2 100644 --- a/superset/commands/report/exceptions.py +++ b/superset/commands/report/exceptions.py @@ -149,6 +149,10 @@ class ReportScheduleScreenshotFailedError(CommandException): message = _("Report Schedule execution failed when generating a screenshot.") +class ReportSchedulePdfFailedError(CommandException): + message = _("Report Schedule execution failed when generating a pdf.") + + class ReportScheduleCsvFailedError(CommandException): message = _("Report Schedule execution failed when generating a csv.") diff --git a/superset/commands/report/execute.py b/superset/commands/report/execute.py index 5cacb66134..42563c72d7 100644 --- a/superset/commands/report/execute.py +++ b/superset/commands/report/execute.py @@ -70,6 +70,7 @@ from superset.tasks.utils import get_executor from superset.utils.core import HeaderDataType, override_user from superset.utils.csv import get_chart_csv_data, get_chart_dataframe from superset.utils.decorators import logs_context +from superset.utils.pdf import build_pdf_from_screenshots from superset.utils.screenshots import ChartScreenshot, DashboardScreenshot from superset.utils.urls import get_url_path @@ -238,6 +239,16 @@ class BaseReportState: raise ReportScheduleScreenshotFailedError() return [image] + def _get_pdf(self) -> bytes: + """ + Get chart or dashboard pdf + :raises: ReportSchedulePdfFailedError + """ + screenshots = self._get_screenshots() + pdf = build_pdf_from_screenshots(screenshots) + + return pdf + def _get_csv_data(self) -> bytes: url = self._get_url(result_format=ChartDataResultFormat.CSV) _, username = get_executor( @@ -342,22 +353,27 @@ class BaseReportState: :raises: ReportScheduleScreenshotFailedError """ csv_data = None + screenshot_data = [] + pdf_data = None embedded_data = None error_text = None - screenshot_data = [] header_data = self._get_log_data() url = self._get_url(user_friendly=True) if ( feature_flag_manager.is_feature_enabled("ALERTS_ATTACH_REPORTS") or self._report_schedule.type == ReportScheduleType.REPORT ): - if self._report_schedule.report_format == ReportDataFormat.VISUALIZATION: + if self._report_schedule.report_format == ReportDataFormat.PNG: screenshot_data = self._get_screenshots() if not screenshot_data: error_text = "Unexpected missing screenshot" + elif self._report_schedule.report_format == ReportDataFormat.PDF: + pdf_data = self._get_pdf() + if not pdf_data: + error_text = "Unexpected missing pdf" elif ( self._report_schedule.chart - and self._report_schedule.report_format == ReportDataFormat.DATA + and self._report_schedule.report_format == ReportDataFormat.CSV ): csv_data = self._get_csv_data() if not csv_data: @@ -390,6 +406,7 @@ class BaseReportState: name=name, url=url, screenshots=screenshot_data, + pdf=pdf_data, description=self._report_schedule.description, csv=csv_data, embedded_data=embedded_data, diff --git a/superset/reports/models.py b/superset/reports/models.py index 022db5dc6f..63090e4c45 100644 --- a/superset/reports/models.py +++ b/superset/reports/models.py @@ -72,8 +72,9 @@ class ReportState(StrEnum): class ReportDataFormat(StrEnum): - VISUALIZATION = "PNG" - DATA = "CSV" + PDF = "PDF" + PNG = "PNG" + CSV = "CSV" TEXT = "TEXT" @@ -127,7 +128,7 @@ class ReportSchedule(AuditMixinNullable, ExtraJSONMixin, Model): String(255), server_default=ReportCreationMethod.ALERTS_REPORTS ) timezone = Column(String(100), default="UTC", nullable=False) - report_format = Column(String(50), default=ReportDataFormat.VISUALIZATION) + report_format = Column(String(50), default=ReportDataFormat.PNG) sql = Column(MediumText()) # (Alerts/Reports) M-O to chart chart_id = Column(Integer, ForeignKey("slices.id"), nullable=True) diff --git a/superset/reports/notifications/base.py b/superset/reports/notifications/base.py index 640b326fc5..9d72d81959 100644 --- a/superset/reports/notifications/base.py +++ b/superset/reports/notifications/base.py @@ -28,6 +28,7 @@ class NotificationContent: name: str header_data: HeaderDataType # this is optional to account for error states csv: Optional[bytes] = None # bytes for csv file + pdf: Optional[bytes] = None # bytes for PDF file screenshots: Optional[list[bytes]] = None # bytes for a list of screenshots text: Optional[str] = None description: Optional[str] = "" diff --git a/superset/reports/notifications/email.py b/superset/reports/notifications/email.py index 1b9e4ade72..b8da60b909 100644 --- a/superset/reports/notifications/email.py +++ b/superset/reports/notifications/email.py @@ -69,6 +69,7 @@ class EmailContent: body: str header_data: Optional[HeaderDataType] = None data: Optional[dict[str, Any]] = None + pdf: Optional[dict[str, bytes]] = None images: Optional[dict[str, bytes]] = None @@ -97,7 +98,7 @@ class EmailNotification(BaseNotification): # pylint: disable=too-few-public-met return EmailContent(body=self._error_template(self._content.text)) # Get the domain from the 'From' address .. # and make a message id without the < > in the end - csv_data = None + domain = self._get_smtp_domain() images = {} @@ -165,12 +166,18 @@ class EmailNotification(BaseNotification): # pylint: disable=too-few-public-met """ ) - + csv_data = None if self._content.csv: csv_data = {__("%(name)s.csv", name=self._content.name): self._content.csv} + + pdf_data = None + if self._content.pdf: + pdf_data = {__("%(name)s.pdf", name=self._content.name): self._content.pdf} + return EmailContent( body=body, images=images, + pdf=pdf_data, data=csv_data, header_data=self._content.header_data, ) @@ -198,6 +205,7 @@ class EmailNotification(BaseNotification): # pylint: disable=too-few-public-met app.config, files=[], data=content.data, + pdf=content.pdf, images=content.images, bcc="", mime_subtype="related", diff --git a/superset/reports/notifications/slack.py b/superset/reports/notifications/slack.py index 41ed2ceeb3..345e9d5b26 100644 --- a/superset/reports/notifications/slack.py +++ b/superset/reports/notifications/slack.py @@ -161,21 +161,24 @@ Error: %(text)s return self._message_template(table) - def _get_inline_files(self) -> Sequence[Union[str, IOBase, bytes]]: + def _get_inline_files( + self, + ) -> tuple[Union[str, None], Sequence[Union[str, IOBase, bytes]]]: if self._content.csv: - return [self._content.csv] + return ("csv", [self._content.csv]) if self._content.screenshots: - return self._content.screenshots - return [] + return ("png", self._content.screenshots) + if self._content.pdf: + return ("pdf", [self._content.pdf]) + return (None, []) @backoff.on_exception(backoff.expo, SlackApiError, factor=10, base=2, max_tries=5) @statsd_gauge("reports.slack.send") def send(self) -> None: - files = self._get_inline_files() + file_type, files = self._get_inline_files() title = self._content.name channel = self._get_channel() body = self._get_body() - file_type = "csv" if self._content.csv else "png" global_logs_context = getattr(g, "logs_context", {}) or {} try: token = app.config["SLACK_API_TOKEN"] diff --git a/superset/reports/schemas.py b/superset/reports/schemas.py index 84c075ffbd..286322a2da 100644 --- a/superset/reports/schemas.py +++ b/superset/reports/schemas.py @@ -204,7 +204,7 @@ class ReportSchedulePostSchema(Schema): recipients = fields.List(fields.Nested(ReportRecipientSchema)) report_format = fields.String( - dump_default=ReportDataFormat.VISUALIZATION, + dump_default=ReportDataFormat.PNG, validate=validate.OneOf(choices=tuple(key.value for key in ReportDataFormat)), ) extra = fields.Dict( @@ -335,7 +335,7 @@ class ReportSchedulePutSchema(Schema): ) recipients = fields.List(fields.Nested(ReportRecipientSchema), required=False) report_format = fields.String( - dump_default=ReportDataFormat.VISUALIZATION, + dump_default=ReportDataFormat.PNG, validate=validate.OneOf(choices=tuple(key.value for key in ReportDataFormat)), ) extra = fields.Dict(dump_default=None) diff --git a/superset/utils/core.py b/superset/utils/core.py index 0732aa2557..c093cbc118 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -822,6 +822,7 @@ def send_email_smtp( # pylint: disable=invalid-name,too-many-arguments,too-many config: dict[str, Any], files: list[str] | None = None, data: dict[str, str] | None = None, + pdf: dict[str, bytes] | None = None, images: dict[str, bytes] | None = None, dryrun: bool = False, cc: str | None = None, @@ -879,6 +880,15 @@ def send_email_smtp( # pylint: disable=invalid-name,too-many-arguments,too-many ) ) + for name, body_pdf in (pdf or {}).items(): + msg.attach( + MIMEApplication( + body_pdf, + Content_Disposition=f"attachment; filename='{name}'", + Name=name, + ) + ) + # Attach any inline images, which may be required for display in # HTML content (inline) for msgid, imgdata in (images or {}).items(): diff --git a/superset/utils/pdf.py b/superset/utils/pdf.py new file mode 100644 index 0000000000..7d75d2692e --- /dev/null +++ b/superset/utils/pdf.py @@ -0,0 +1,48 @@ +# 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. + +import logging +from io import BytesIO + +from superset.commands.report.exceptions import ReportSchedulePdfFailedError + +logger = logging.getLogger(__name__) +try: + from PIL import Image +except ModuleNotFoundError: + logger.info("No PIL installation found") + + +def build_pdf_from_screenshots(snapshots: list[bytes]) -> bytes: + images = [] + + for snap in snapshots: + img = Image.open(BytesIO(snap)) + if img.mode == "RGBA": + img = img.convert("RGB") + images.append(img) + logger.info("building pdf") + try: + new_pdf = BytesIO() + images[0].save(new_pdf, "PDF", save_all=True, append_images=images[1:]) + new_pdf.seek(0) + except Exception as ex: + raise ReportSchedulePdfFailedError( + f"Failed converting screenshots to pdf {str(ex)}" + ) from ex + + return new_pdf.read() diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py index 939c9c0cfa..0c353d1fab 100644 --- a/tests/integration_tests/reports/commands_tests.py +++ b/tests/integration_tests/reports/commands_tests.py @@ -205,7 +205,7 @@ def create_report_email_chart_with_csv(): report_schedule = create_report_notification( email_target="target@email.com", chart=chart, - report_format=ReportDataFormat.DATA, + report_format=ReportDataFormat.CSV, ) yield report_schedule cleanup_report_schedule(report_schedule) @@ -233,7 +233,7 @@ def create_report_email_chart_with_csv_no_query_context(): report_schedule = create_report_notification( email_target="target@email.com", chart=chart, - report_format=ReportDataFormat.DATA, + report_format=ReportDataFormat.CSV, name="report_csv_no_query_context", ) yield report_schedule @@ -284,7 +284,7 @@ def create_report_slack_chart_with_csv(): report_schedule = create_report_notification( slack_channel="slack_channel", chart=chart, - report_format=ReportDataFormat.DATA, + report_format=ReportDataFormat.CSV, ) yield report_schedule diff --git a/tests/integration_tests/reports/utils.py b/tests/integration_tests/reports/utils.py index 7672c5c940..8b860892ed 100644 --- a/tests/integration_tests/reports/utils.py +++ b/tests/integration_tests/reports/utils.py @@ -158,7 +158,7 @@ def create_report_notification( validator_type=validator_type, validator_config_json=validator_config_json, grace_period=grace_period, - report_format=report_format or ReportDataFormat.VISUALIZATION, + report_format=report_format or ReportDataFormat.PNG, extra=extra, force_screenshot=force_screenshot, )