Skip to content

Commit

Permalink
feat(alert/reports): adding logic to handle downstream reports when t…
Browse files Browse the repository at this point in the history 
…ab is deleted from dashboard (#29333)
  • Loading branch information
@fisjac
fisjac authored Aug 23, 2024
1 parent 9249fac commit 2fda221
Show file tree
Hide file tree
Showing 5 changed files with 318 additions and 12 deletions.
2 changes: 1 addition & 1 deletion superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ export class Tabs extends PureComponent {
content: (
<span>
{t(
'Deleting a tab will remove all content within it. You may still ' +
'Deleting a tab will remove all content within it and will deactivate any related alerts or reports. You may still ' +
'reverse this action with the',
)}{' '}
<b>{t('undo')}</b>{' '}
Expand Down
79 changes: 77 additions & 2 deletions superset/commands/dashboard/update.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@
# specific language governing permissions and limitations
# under the License.
import logging
import textwrap
from functools import partial
from typing import Any, Optional

from flask_appbuilder.models.sqla import Model
from marshmallow import ValidationError

from superset import security_manager
from superset import app, security_manager
from superset.commands.base import BaseCommand, UpdateMixin
from superset.commands.dashboard.exceptions import (
DashboardForbiddenError,
Expand All @@ -32,10 +33,13 @@
)
from superset.commands.utils import populate_roles, update_tags, validate_tags
from superset.daos.dashboard import DashboardDAO
from superset.daos.report import ReportScheduleDAO
from superset.exceptions import SupersetSecurityException
from superset.models.dashboard import Dashboard
from superset.reports.models import ReportSchedule
from superset.tags.models import ObjectType
from superset.utils import json
from superset.utils.core import send_email_smtp
from superset.utils.decorators import on_error, transaction

logger = logging.getLogger(__name__)
Expand All @@ -50,7 +54,8 @@ def __init__(self, model_id: int, data: dict[str, Any]):
@transaction(on_error=partial(on_error, reraise=DashboardUpdateFailedError))
def run(self) -> Model:
self.validate()
assert self._model
assert self._model is not None
self.process_tab_diff()

# Update tags
if (tags := self._properties.pop("tags", None)) is not None:
Expand Down Expand Up @@ -112,3 +117,73 @@ def validate(self) -> None:
exceptions.append(ex)
if exceptions:
raise DashboardInvalidError(exceptions=exceptions)

def process_tab_diff(self) -> None:
def find_deleted_tabs() -> list[str]:
position_json = self._properties.get("position_json", "")
current_tabs = self._model.tabs # type: ignore
if position_json and current_tabs:
position = json.loads(position_json)
deleted_tabs = [
tab for tab in current_tabs["all_tabs"] if tab not in position
]
return deleted_tabs
return []

def find_reports_containing_tabs(tabs: list[str]) -> list[ReportSchedule]:
alert_reports_list = []
for tab in tabs:
for report in ReportScheduleDAO.find_by_extra_metadata(tab):
alert_reports_list.append(report)
return alert_reports_list

def send_deactivated_email_warning(report: ReportSchedule) -> None:
description = textwrap.dedent(
"""
The dashboard tab used in this report has been deleted and your report has been deactivated.
Please update your report settings to remove or change the tab used.
"""
)

html_content = textwrap.dedent(
f"""
<html>
<head>
<style type="text/css">
table, th, td {{
border-collapse: collapse;
border-color: rgb(200, 212, 227);
color: rgb(42, 63, 95);
padding: 4px 8px;
}}
.image{{
margin-bottom: 18px;
}}
</style>
</head>
<body>
<div>{description}</div>
<br>
</body>
</html>
"""
)
for report_owner in report.owners:
if email := report_owner.email:
send_email_smtp(
to=email,
subject=f"[Report: {report.name}] Deactivated",
html_content=html_content,
config=app.config,
)

def deactivate_reports(reports_list: list[ReportSchedule]) -> None:
for report in reports_list:
# deactivate
ReportScheduleDAO.update(report, {"active": False})
# send email to report owner
send_deactivated_email_warning(report)

deleted_tabs = find_deleted_tabs()
reports = find_reports_containing_tabs(deleted_tabs)
deactivate_reports(reports)
8 changes: 8 additions & 0 deletions superset/daos/report.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,14 @@ def find_by_database_ids(database_ids: list[int]) -> list[ReportSchedule]:
.all()
)

@staticmethod
def find_by_extra_metadata(slug: str) -> list[ReportSchedule]:
return (
db.session.query(ReportSchedule)
.filter(ReportSchedule.extra_json.like(f"%{slug}%"))
.all()
)

@staticmethod
def validate_unique_creation_method(
dashboard_id: int | None = None, chart_id: int | None = None
Expand Down
219 changes: 219 additions & 0 deletions tests/integration_tests/dashboards/update_tabs_test.py
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
# 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.
from unittest.mock import ANY, call, MagicMock, patch

import pytest

from superset import db, security_manager
from superset.commands.dashboard.update import UpdateDashboardCommand
from superset.models.dashboard import Dashboard
from superset.utils.json import dumps
from tests.integration_tests.fixtures.tabbed_dashboard import (
tabbed_dashboard, # noqa: F401
)
from tests.integration_tests.reports.utils import (
cleanup_report_schedule,
create_report_notification,
)
from tests.integration_tests.test_app import app

tab1 = "TAB-L1AA"
tab2 = "TAB-L1AB"
tab3 = "TAB-L1BB"


@pytest.fixture(autouse=True, scope="module")
def initial_report_cleanup():
with app.app_context():
cleanup_report_schedule()


def remove_tabs_from_dashboard(dashboard: Dashboard, tabs: list[str]):
data = dashboard.data.copy()
position = data["position_json"]
for tab in tabs:
del position[tab]
data["position_json"] = dumps(position)
return data


@patch("superset.commands.dashboard.update.send_email_smtp")
@patch.dict(
"superset.extensions.feature_flag_manager._feature_flags", ALERT_REPORT_TABS=True
)
def test_tab_deletion_single_report(
send_email_smtp_mock: MagicMock,
tabbed_dashboard: Dashboard, # noqa: F811
login_as_admin,
):
tab_to_delete = tab1
users = db.session.query(security_manager.user_model).all()
report = create_report_notification(
dashboard=tabbed_dashboard,
extra={"dashboard": {"anchor": tab_to_delete}},
owners=[users[0]],
)
assert report.active is True
UpdateDashboardCommand(
tabbed_dashboard.id,
remove_tabs_from_dashboard(tabbed_dashboard, [tab_to_delete]),
).run()

assert report.active is False
send_email_smtp_mock.assert_called_with(
to=users[0].email,
subject="[Report: report] Deactivated",
html_content=ANY,
config=ANY,
)
cleanup_report_schedule(report)


@patch("superset.commands.dashboard.update.send_email_smtp")
@patch.dict(
"superset.extensions.feature_flag_manager._feature_flags", ALERT_REPORT_TABS=True
)
def test_tab_deletion_multiple_reports(
send_email_smtp_mock: MagicMock,
tabbed_dashboard: Dashboard, # noqa: F811
login_as_admin,
):
tab_to_delete = tab1
retained_tab = tab2
users = db.session.query(security_manager.user_model).all()
report1 = create_report_notification(
name="report 1",
dashboard=tabbed_dashboard,
extra={"dashboard": {"anchor": tab_to_delete}},
owners=[users[0], users[1]],
)
report2 = create_report_notification(
name="report 2",
dashboard=tabbed_dashboard,
extra={"dashboard": {"anchor": tab_to_delete}},
owners=[users[1]],
)
report3 = create_report_notification(
name="report 3",
dashboard=tabbed_dashboard,
extra={"dashboard": {"anchor": retained_tab}},
owners=[users[2]],
)

assert report1.active is True
assert report2.active is True
assert report3.active is True

UpdateDashboardCommand(
tabbed_dashboard.id,
remove_tabs_from_dashboard(tabbed_dashboard, [tab_to_delete]),
).run()

assert report1.active is False
assert report2.active is False
assert report3.active is True

expected_calls = [
call(
to=users[0].email,
subject="[Report: report 1] Deactivated",
html_content=ANY,
config=ANY,
),
call(
to=users[1].email,
subject="[Report: report 1] Deactivated",
html_content=ANY,
config=ANY,
),
call(
to=users[1].email,
subject="[Report: report 2] Deactivated",
html_content=ANY,
config=ANY,
),
]

assert send_email_smtp_mock.call_count == 3
assert send_email_smtp_mock.call_args_list == expected_calls

cleanup_report_schedule(report1)
cleanup_report_schedule(report2)
cleanup_report_schedule(report3)


@patch("superset.commands.dashboard.update.send_email_smtp")
@patch.dict(
"superset.extensions.feature_flag_manager._feature_flags", ALERT_REPORT_TABS=True
)
def test_multitple_tabs_removed(
send_email_smtp_mock: MagicMock,
tabbed_dashboard: Dashboard, # noqa: F811
login_as_admin,
):
tabs_to_delete = [tab1, tab2]
users = db.session.query(security_manager.user_model).all()
report1 = create_report_notification(
name="report 1",
dashboard=tabbed_dashboard,
extra={"dashboard": {"anchor": tabs_to_delete[0]}},
owners=[users[0], users[1]],
)
report2 = create_report_notification(
name="report 2",
dashboard=tabbed_dashboard,
extra={"dashboard": {"anchor": tabs_to_delete[1]}},
owners=[users[2]],
)

assert report1.active is True
assert report2.active is True

UpdateDashboardCommand(
tabbed_dashboard.id,
remove_tabs_from_dashboard(tabbed_dashboard, tabs_to_delete),
).run()

assert report1.active is False
assert report2.active is False

expected_calls = [
call(
to=users[0].email,
subject="[Report: report 1] Deactivated",
html_content=ANY,
config=ANY,
),
call(
to=users[1].email,
subject="[Report: report 1] Deactivated",
html_content=ANY,
config=ANY,
),
call(
to=users[2].email,
subject="[Report: report 2] Deactivated",
html_content=ANY,
config=ANY,
),
]

assert send_email_smtp_mock.call_count == 3
assert send_email_smtp_mock.call_args_list == expected_calls

cleanup_report_schedule(report1)
cleanup_report_schedule(report2)
22 changes: 13 additions & 9 deletions tests/integration_tests/reports/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,15 +170,19 @@ def create_report_notification(
return report_schedule


def cleanup_report_schedule(report_schedule: ReportSchedule) -> None:
db.session.query(ReportExecutionLog).filter(
ReportExecutionLog.report_schedule == report_schedule
).delete()
db.session.query(ReportRecipients).filter(
ReportRecipients.report_schedule == report_schedule
).delete()

db.session.delete(report_schedule)
def cleanup_report_schedule(report_schedule: Optional[ReportSchedule] = None) -> None:
if report_schedule:
db.session.query(ReportExecutionLog).filter(
ReportExecutionLog.report_schedule == report_schedule
).delete()
db.session.query(ReportRecipients).filter(
ReportRecipients.report_schedule == report_schedule
).delete()
db.session.delete(report_schedule)
else:
db.session.query(ReportExecutionLog).delete()
db.session.query(ReportRecipients).delete()
db.session.query(ReportSchedule).delete()
db.session.commit()


Expand Down

0 comments on commit 2fda221

Please sign in to comment.