Skip to content

Commit

Permalink
fix send to owners and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
samtfm committed Apr 5, 2021
1 parent 9378a20 commit 2775344
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 12 deletions.
16 changes: 11 additions & 5 deletions superset/reports/commands/execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -29,6 +30,7 @@
from superset.extensions import feature_flag_manager
from superset.models.reports import (
ReportExecutionLog,
ReportRecipientType,
ReportSchedule,
ReportScheduleType,
ReportState,
Expand Down Expand Up @@ -261,11 +263,15 @@ def send_error(self, name: str, message: str) -> None:
notification_content = NotificationContent(name=name, text=message)

# filter recipients to recipients who are also owners
owner_ids = [owner.id for owner in self._report_schedule.owners]
owner_recipients = filter(
lambda recipient: recipient.id in owner_ids,
self._report_schedule.recipients
)

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)

Expand Down
20 changes: 13 additions & 7 deletions tests/reports/commands_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
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 (
Expand Down Expand Up @@ -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="[email protected]")
.one_or_none()
)

if slack_channel:
recipient = ReportRecipients(
type=ReportRecipientType.SLACK,
Expand All @@ -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,
Expand Down Expand Up @@ -879,7 +886,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] == "[email protected]"

assert_log(
ReportState.ERROR, error_message="A timeout occurred while executing the query."
Expand Down Expand Up @@ -908,9 +915,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] == "[email protected]"

assert_log(
ReportState.ERROR, error_message="A timeout occurred while taking a screenshot."
Expand All @@ -937,7 +943,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] == "[email protected]"

assert_log(
ReportState.ERROR, error_message="Failed taking a screenshot Unexpected error"
Expand Down Expand Up @@ -986,7 +992,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] == "[email protected]"


@pytest.mark.usefixtures("create_invalid_sql_alert_email_chart")
Expand All @@ -1007,7 +1013,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] == "[email protected]"
assert (
get_notification_error_sent_count(create_invalid_sql_alert_email_chart) == 1
)
Expand Down

0 comments on commit 2775344

Please sign in to comment.