diff --git a/elementary/monitor/data_monitoring/report/data_monitoring_report.py b/elementary/monitor/data_monitoring/report/data_monitoring_report.py index 7493b96e7..32b920ace 100644 --- a/elementary/monitor/data_monitoring/report/data_monitoring_report.py +++ b/elementary/monitor/data_monitoring/report/data_monitoring_report.py @@ -8,7 +8,7 @@ from elementary.clients.azure.client import AzureClient from elementary.clients.gcs.client import GCSClient from elementary.clients.s3.client import S3Client -from elementary.clients.slack.client import SlackClient +from elementary.clients.slack.client import SlackClient, SlackWebhookClient from elementary.config.config import Config from elementary.monitor.api.invocations.invocations import InvocationsAPI from elementary.monitor.api.report.report import ReportAPI @@ -227,7 +227,14 @@ def send_report( ) if should_send_report_over_slack: self.validate_report_selector() - self.send_report_attachment(local_html_path=local_html_path) + if isinstance(self.slack_client, SlackWebhookClient): + # Slack webhooks can only post messages, not upload files. + logger.warning( + "Skipping the report attachment: a Slack webhook cannot upload files. " + "Use a Slack token or a bucket website to share the report." + ) + else: + self.send_report_attachment(local_html_path=local_html_path) return self.success diff --git a/tests/unit/monitor/data_monitoring/report/test_data_monitoring_report.py b/tests/unit/monitor/data_monitoring/report/test_data_monitoring_report.py new file mode 100644 index 000000000..fa42ee849 --- /dev/null +++ b/tests/unit/monitor/data_monitoring/report/test_data_monitoring_report.py @@ -0,0 +1,77 @@ +from typing import Optional +from unittest.mock import MagicMock + +import pytest + +from elementary.clients.slack.client import SlackWebClient, SlackWebhookClient +from elementary.config.config import Config +from elementary.monitor.data_monitoring.report.data_monitoring_report import ( + DataMonitoringReport, +) +from tests.mocks.dbt_runner_mock import MockDbtRunner + +LOCAL_REPORT_PATH = "/tmp/elementary_report.html" + + +class DataMonitoringReportMock(DataMonitoringReport): + def __init__(self, config: Config, bucket_website_url: Optional[str] = None): + super().__init__(config=config, tracking=None) + # Stand-ins for the parts of send_report that need a warehouse or a bucket. + self.generate_report = MagicMock(return_value=(True, LOCAL_REPORT_PATH)) + self.send_test_results_summary = MagicMock(return_value=True) + self.s3_client = MagicMock() + self.s3_client.send_report.return_value = (True, bucket_website_url) + + def _init_internal_dbt_runner(self): + return MockDbtRunner() + + def get_latest_invocation(self): + return dict() + + def _get_warehouse_info(self, *args, **kwargs): + return None + + def get_elementary_database_and_schema(self): + return "." + + +@pytest.fixture +def webhook_and_s3_config(tmp_path) -> Config: + return Config( + config_dir=str(tmp_path / "config"), + target_path=str(tmp_path / "target"), + slack_webhook="https://hooks.slack.com/services/T000/B000/XXXX", + s3_bucket_name="elementary-reports", + ) + + +def test_send_report_with_slack_webhook_skips_html_attachment( + webhook_and_s3_config: Config, +): + report = DataMonitoringReportMock(webhook_and_s3_config) + assert isinstance(report.slack_client, SlackWebhookClient) + + assert report.send_report() is True + + report.s3_client.send_report.assert_called_once() + report.send_test_results_summary.assert_called_once() + assert report.success is True + + +def test_send_report_with_slack_token_still_sends_html_attachment(tmp_path): + config = Config( + config_dir=str(tmp_path / "config"), + target_path=str(tmp_path / "target"), + slack_token="xoxb-mock", + slack_channel_name="data-reports", + s3_bucket_name="elementary-reports", + ) + report = DataMonitoringReportMock(config) + assert isinstance(report.slack_client, SlackWebClient) + report.slack_client.send_report = MagicMock(return_value=True) + + assert report.send_report() is True + + report.slack_client.send_report.assert_called_once_with( + "data-reports", LOCAL_REPORT_PATH + )