diff --git a/superset/reports/commands/execute.py b/superset/reports/commands/execute.py index 8712532738..9f9696259d 100644 --- a/superset/reports/commands/execute.py +++ b/superset/reports/commands/execute.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import json import logging from datetime import datetime, timedelta from typing import Any, List, Optional @@ -29,6 +30,8 @@ from superset.commands.exceptions import CommandException from superset.extensions import feature_flag_manager from superset.models.reports import ( ReportExecutionLog, + ReportRecipients, + ReportRecipientType, ReportSchedule, ReportScheduleType, ReportState, @@ -226,14 +229,17 @@ class BaseReportState: description=self._report_schedule.description, ) - def _send(self, notification_content: NotificationContent) -> None: + @staticmethod + def _send( + notification_content: NotificationContent, recipients: List[ReportRecipients] + ) -> None: """ Sends a notification to all recipients :raises: ReportScheduleNotificationError """ notification_errors = [] - for recipient in self._report_schedule.recipients: + for recipient in recipients: notification = create_notification(recipient, notification_content) try: notification.send() @@ -250,7 +256,7 @@ class BaseReportState: :raises: ReportScheduleNotificationError """ notification_content = self._get_notification_content() - self._send(notification_content) + self._send(notification_content, self._report_schedule.recipients) def send_error(self, name: str, message: str) -> None: """ @@ -259,7 +265,17 @@ class BaseReportState: :raises: ReportScheduleNotificationError """ notification_content = NotificationContent(name=name, text=message) - self._send(notification_content) + + # filter recipients to recipients who are also owners + owner_recipients = [ + ReportRecipients( + type=ReportRecipientType.EMAIL, + recipient_config_json=json.dumps({"target": owner.email}), + ) + for owner in self._report_schedule.owners + ] + + self._send(notification_content, owner_recipients) def is_in_grace_period(self) -> bool: """ diff --git a/tests/reports/commands_tests.py b/tests/reports/commands_tests.py index 7dfb70c48c..8d97269ef4 100644 --- a/tests/reports/commands_tests.py +++ b/tests/reports/commands_tests.py @@ -26,7 +26,7 @@ from flask_sqlalchemy import BaseQuery from freezegun import freeze_time from sqlalchemy.sql import func -from superset import db +from superset import db, security_manager from superset.models.core import Database from superset.models.dashboard import Dashboard from superset.models.reports import ( @@ -130,6 +130,12 @@ def create_report_notification( report_type = report_type or ReportScheduleType.REPORT target = email_target or slack_channel config_json = {"target": target} + owner = ( + db.session.query(security_manager.user_model) + .filter_by(email="admin@fab.org") + .one_or_none() + ) + if slack_channel: recipient = ReportRecipients( type=ReportRecipientType.SLACK, @@ -151,6 +157,7 @@ def create_report_notification( dashboard=dashboard, database=database, recipients=[recipient], + owners=[owner], validator_type=validator_type, validator_config_json=validator_config_json, grace_period=grace_period, @@ -890,7 +897,7 @@ def test_soft_timeout_alert(email_mock, create_alert_email_chart): notification_targets = get_target_from_report_schedule(create_alert_email_chart) # Assert the email smtp address, asserts a notification was sent with the error - assert email_mock.call_args[0][0] == notification_targets[0] + assert email_mock.call_args[0][0] == "admin@fab.org" assert_log( ReportState.ERROR, error_message="A timeout occurred while executing the query." @@ -919,9 +926,8 @@ def test_soft_timeout_screenshot(screenshot_mock, email_mock, create_alert_email test_id, create_alert_email_chart.id, datetime.utcnow() ).run() - notification_targets = get_target_from_report_schedule(create_alert_email_chart) # Assert the email smtp address, asserts a notification was sent with the error - assert email_mock.call_args[0][0] == notification_targets[0] + assert email_mock.call_args[0][0] == "admin@fab.org" assert_log( ReportState.ERROR, error_message="A timeout occurred while taking a screenshot." @@ -948,7 +954,7 @@ def test_fail_screenshot(screenshot_mock, email_mock, create_report_email_chart) notification_targets = get_target_from_report_schedule(create_report_email_chart) # Assert the email smtp address, asserts a notification was sent with the error - assert email_mock.call_args[0][0] == notification_targets[0] + assert email_mock.call_args[0][0] == "admin@fab.org" assert_log( ReportState.ERROR, error_message="Failed taking a screenshot Unexpected error" @@ -997,7 +1003,7 @@ def test_invalid_sql_alert(email_mock, create_invalid_sql_alert_email_chart): create_invalid_sql_alert_email_chart ) # Assert the email smtp address, asserts a notification was sent with the error - assert email_mock.call_args[0][0] == notification_targets[0] + assert email_mock.call_args[0][0] == "admin@fab.org" @pytest.mark.usefixtures("create_invalid_sql_alert_email_chart") @@ -1018,7 +1024,7 @@ def test_grace_period_error(email_mock, create_invalid_sql_alert_email_chart): create_invalid_sql_alert_email_chart ) # Assert the email smtp address, asserts a notification was sent with the error - assert email_mock.call_args[0][0] == notification_targets[0] + assert email_mock.call_args[0][0] == "admin@fab.org" assert ( get_notification_error_sent_count(create_invalid_sql_alert_email_chart) == 1 )