iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: delete chart, dashboards, dbs with assoc reports (#11801) 

* fix: delete chart or dashboards with assoc reports

* database constraint to reports and tests

* add tests for dashboards and database

* fix exceptions default text
This commit is contained in:
Daniel Vaz Gaspar 2020-11-26 08:45:49 +00:00 committed by GitHub
parent 13c51d5211
commit bac84a3aac
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 304 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
superset/charts/commands/bulk_delete.py
View File

@ -18,9 +18,11 @@ import logging
from typing import List, Optional
from flask_appbuilder.security.sqla.models import User
from flask_babel import lazy_gettext as _
from superset.charts.commands.exceptions import (
ChartBulkDeleteFailedError,
ChartBulkDeleteFailedReportsExistError,
ChartForbiddenError,
ChartNotFoundError,
)
@ -29,6 +31,7 @@ from superset.commands.base import BaseCommand
from superset.commands.exceptions import DeleteFailedError
from superset.exceptions import SupersetSecurityException
from superset.models.slice import Slice
from superset.reports.dao import ReportScheduleDAO
from superset.views.base import check_ownership
logger = logging.getLogger(__name__)
@ -53,6 +56,13 @@ class BulkDeleteChartCommand(BaseCommand):
self._models = ChartDAO.find_by_ids(self._model_ids)
if not self._models or len(self._models) != len(self._model_ids):
raise ChartNotFoundError()
# Check there are no associated ReportSchedules
reports = ReportScheduleDAO.find_by_chart_ids(self._model_ids)
if reports:
report_names = [report.name for report in reports]
raise ChartBulkDeleteFailedReportsExistError(
_("There are associated alerts or reports: %s" % ",".join(report_names))
)
# Check ownership
for model in self._models:
try:

10
superset/charts/commands/delete.py
View File

@ -19,9 +19,11 @@ from typing import Optional
from flask_appbuilder.models.sqla import Model
from flask_appbuilder.security.sqla.models import User
from flask_babel import lazy_gettext as _
from superset.charts.commands.exceptions import (
ChartDeleteFailedError,
ChartDeleteFailedReportsExistError,
ChartForbiddenError,
ChartNotFoundError,
)
@ -31,6 +33,7 @@ from superset.dao.exceptions import DAODeleteFailedError
from superset.exceptions import SupersetSecurityException
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.reports.dao import ReportScheduleDAO
from superset.views.base import check_ownership
logger = logging.getLogger(__name__)
@ -57,6 +60,13 @@ class DeleteChartCommand(BaseCommand):
self._model = ChartDAO.find_by_id(self._model_id)
if not self._model:
raise ChartNotFoundError()
# Check there are no associated ReportSchedules
reports = ReportScheduleDAO.find_by_chart_id(self._model_id)
if reports:
report_names = [report.name for report in reports]
raise ChartDeleteFailedReportsExistError(
_("There are associated alerts or reports: %s" % ",".join(report_names))
)
# Check ownership
try:
check_ownership(self._model)

10
superset/charts/commands/exceptions.py
View File

@ -78,13 +78,21 @@ class ChartDeleteFailedError(DeleteFailedError):
message = _("Chart could not be deleted.")
class ChartDeleteFailedReportsExistError(ChartDeleteFailedError):
message = _("There are associated alerts or reports")
class ChartForbiddenError(ForbiddenError):
message = _("Changing this chart is forbidden")
class ChartBulkDeleteFailedError(CreateFailedError):
class ChartBulkDeleteFailedError(DeleteFailedError):
message = _("Charts could not be deleted.")
class ChartBulkDeleteFailedReportsExistError(ChartBulkDeleteFailedError):
message = _("There are associated alerts or reports")
class ChartImportError(ImportFailedError):
message = _("Import chart failed for an unknown reason")

10
superset/dashboards/commands/bulk_delete.py
View File

@ -18,17 +18,20 @@ import logging
from typing import List, Optional
from flask_appbuilder.security.sqla.models import User
from flask_babel import lazy_gettext as _
from superset.commands.base import BaseCommand
from superset.commands.exceptions import DeleteFailedError
from superset.dashboards.commands.exceptions import (
DashboardBulkDeleteFailedError,
DashboardBulkDeleteFailedReportsExistError,
DashboardForbiddenError,
DashboardNotFoundError,
)
from superset.dashboards.dao import DashboardDAO
from superset.exceptions import SupersetSecurityException
from superset.models.dashboard import Dashboard
from superset.reports.dao import ReportScheduleDAO
from superset.views.base import check_ownership
logger = logging.getLogger(__name__)
@ -54,6 +57,13 @@ class BulkDeleteDashboardCommand(BaseCommand):
self._models = DashboardDAO.find_by_ids(self._model_ids)
if not self._models or len(self._models) != len(self._model_ids):
raise DashboardNotFoundError()
# Check there are no associated ReportSchedules
reports = ReportScheduleDAO.find_by_dashboard_ids(self._model_ids)
if reports:
report_names = [report.name for report in reports]
raise DashboardBulkDeleteFailedReportsExistError(
_("There are associated alerts or reports: %s" % ",".join(report_names))
)
# Check ownership
for model in self._models:
try:

10
superset/dashboards/commands/delete.py
View File

@ -19,17 +19,20 @@ from typing import Optional
from flask_appbuilder.models.sqla import Model
from flask_appbuilder.security.sqla.models import User
from flask_babel import lazy_gettext as _
from superset.commands.base import BaseCommand
from superset.dao.exceptions import DAODeleteFailedError
from superset.dashboards.commands.exceptions import (
DashboardDeleteFailedError,
DashboardDeleteFailedReportsExistError,
DashboardForbiddenError,
DashboardNotFoundError,
)
from superset.dashboards.dao import DashboardDAO
from superset.exceptions import SupersetSecurityException
from superset.models.dashboard import Dashboard
from superset.reports.dao import ReportScheduleDAO
from superset.views.base import check_ownership
logger = logging.getLogger(__name__)
@ -55,6 +58,13 @@ class DeleteDashboardCommand(BaseCommand):
self._model = DashboardDAO.find_by_id(self._model_id)
if not self._model:
raise DashboardNotFoundError()
# Check there are no associated ReportSchedules
reports = ReportScheduleDAO.find_by_dashboard_id(self._model_id)
if reports:
report_names = [report.name for report in reports]
raise DashboardDeleteFailedReportsExistError(
_("There are associated alerts or reports: %s" % ",".join(report_names))
)
# Check ownership
try:
check_ownership(self._model)

8
superset/dashboards/commands/exceptions.py
View File

@ -53,6 +53,10 @@ class DashboardBulkDeleteFailedError(CreateFailedError):
message = _("Dashboards could not be deleted.")
class DashboardBulkDeleteFailedReportsExistError(DashboardBulkDeleteFailedError):
message = _("There are associated alerts or reports")
class DashboardUpdateFailedError(UpdateFailedError):
message = _("Dashboard could not be updated.")
@ -61,6 +65,10 @@ class DashboardDeleteFailedError(DeleteFailedError):
message = _("Dashboard could not be deleted.")
class DashboardDeleteFailedReportsExistError(DashboardDeleteFailedError):
message = _("There are associated alerts or reports")
class DashboardForbiddenError(ForbiddenError):
message = _("Changing this Dashboard is forbidden")

11
superset/databases/commands/delete.py
View File

@ -19,16 +19,19 @@ from typing import Optional
from flask_appbuilder.models.sqla import Model
from flask_appbuilder.security.sqla.models import User
from flask_babel import lazy_gettext as _
from superset.commands.base import BaseCommand
from superset.dao.exceptions import DAODeleteFailedError
from superset.databases.commands.exceptions import (
DatabaseDeleteDatasetsExistFailedError,
DatabaseDeleteFailedError,
DatabaseDeleteFailedReportsExistError,
DatabaseNotFoundError,
)
from superset.databases.dao import DatabaseDAO
from superset.models.core import Database
from superset.reports.dao import ReportScheduleDAO
logger = logging.getLogger(__name__)
@ -53,6 +56,14 @@ class DeleteDatabaseCommand(BaseCommand):
self._model = DatabaseDAO.find_by_id(self._model_id)
if not self._model:
raise DatabaseNotFoundError()
# Check there are no associated ReportSchedules
reports = ReportScheduleDAO.find_by_database_id(self._model_id)
if reports:
report_names = [report.name for report in reports]
raise DatabaseDeleteFailedReportsExistError(
_("There are associated alerts or reports: %s" % ",".join(report_names))
)
# Check if there are datasets for this database
if self._model.tables:
raise DatabaseDeleteDatasetsExistFailedError()

4
superset/databases/commands/exceptions.py
View File

@ -113,6 +113,10 @@ class DatabaseDeleteFailedError(DeleteFailedError):
message = _("Database could not be deleted.")
class DatabaseDeleteFailedReportsExistError(DatabaseDeleteFailedError):
message = _("There are associated alerts or reports")
class DatabaseSecurityUnsafeError(DBSecurityException):
message = _("Stopped an unsafe database connection")

48
superset/reports/dao.py
View File

@ -38,6 +38,54 @@ logger = logging.getLogger(__name__)
class ReportScheduleDAO(BaseDAO):
model_cls = ReportSchedule
@staticmethod
def find_by_chart_id(chart_id: int) -> List[ReportSchedule]:
return (
db.session.query(ReportSchedule)
.filter(ReportSchedule.chart_id == chart_id)
.all()
)
@staticmethod
def find_by_chart_ids(chart_ids: List[int]) -> List[ReportSchedule]:
return (
db.session.query(ReportSchedule)
.filter(ReportSchedule.chart_id.in_(chart_ids))
.all()
)
@staticmethod
def find_by_dashboard_id(dashboard_id: int) -> List[ReportSchedule]:
return (
db.session.query(ReportSchedule)
.filter(ReportSchedule.dashboard_id == dashboard_id)
.all()
)
@staticmethod
def find_by_dashboard_ids(dashboard_ids: List[int]) -> List[ReportSchedule]:
return (
db.session.query(ReportSchedule)
.filter(ReportSchedule.dashboard_id.in_(dashboard_ids))
.all()
)
@staticmethod
def find_by_database_id(database_id: int) -> List[ReportSchedule]:
return (
db.session.query(ReportSchedule)
.filter(ReportSchedule.database_id == database_id)
.all()
)
@staticmethod
def find_by_database_ids(database_ids: List[int]) -> List[ReportSchedule]:
return (
db.session.query(ReportSchedule)
.filter(ReportSchedule.database_id.in_(database_ids))
.all()
)
@staticmethod
def bulk_delete(
models: Optional[List[ReportSchedule]], commit: bool = True

2
superset/reports/schemas.py
View File

@ -136,7 +136,7 @@ class ReportSchedulePostSchema(Schema):
crontab = fields.String(
description=crontab_description,
validate=[validate_crontab, Length(1, 50)],
example="*/5 * * * * *",
example="*/5 * * * *",
allow_none=False,
required=True,
)

69
tests/charts/api_tests.py
View File

@ -38,6 +38,7 @@ from superset.connectors.connector_registry import ConnectorRegistry
from superset.extensions import db, security_manager
from superset.models.core import Database, FavStar, FavStarClassName
from superset.models.dashboard import Dashboard
from superset.models.reports import ReportSchedule, ReportScheduleType
from superset.models.slice import Slice
from superset.utils import core as utils
from tests.base_api_tests import ApiOwnersTestCaseMixin
@ -117,6 +118,26 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin):
db.session.delete(fav_chart)
db.session.commit()
@pytest.fixture()
def create_chart_with_report(self):
with self.create_app().app_context():
admin = self.get_user("admin")
chart = self.insert_chart(f"chart_report", [admin.id], 1)
report_schedule = ReportSchedule(
type=ReportScheduleType.REPORT,
name="report_with_chart",
crontab="* * * * *",
chart=chart,
)
db.session.commit()
yield chart
# rollback changes
db.session.delete(report_schedule)
db.session.delete(chart)
db.session.commit()
def test_delete_chart(self):
"""
Chart API: Test delete
@ -174,6 +195,26 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin):
rv = self.delete_assert_metric(uri, "delete")
self.assertEqual(rv.status_code, 404)
@pytest.mark.usefixtures("create_chart_with_report")
def test_delete_chart_with_report(self):
"""
Chart API: Test delete with associated report
"""
self.login(username="admin")
chart = (
db.session.query(Slice)
.filter(Slice.slice_name == "chart_report")
.one_or_none()
)
uri = f"api/v1/chart/{chart.id}"
rv = self.client.delete(uri)
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(rv.status_code, 422)
expected_response = {
"message": "There are associated alerts or reports: report_with_chart"
}
self.assertEqual(response, expected_response)
def test_delete_bulk_charts_not_found(self):
"""
Chart API: Test delete bulk not found
@ -181,11 +222,35 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin):
max_id = db.session.query(func.max(Slice.id)).scalar()
chart_ids = [max_id + 1, max_id + 2]
self.login(username="admin")
argument = chart_ids
uri = f"api/v1/chart/?q={prison.dumps(argument)}"
uri = f"api/v1/chart/?q={prison.dumps(chart_ids)}"
rv = self.delete_assert_metric(uri, "bulk_delete")
self.assertEqual(rv.status_code, 404)
@pytest.mark.usefixtures("create_chart_with_report", "create_charts")
def test_bulk_delete_chart_with_report(self):
"""
Chart API: Test bulk delete with associated report
"""
self.login(username="admin")
chart_with_report = (
db.session.query(Slice.id)
.filter(Slice.slice_name == "chart_report")
.one_or_none()
)
charts = db.session.query(Slice.id).filter(Slice.slice_name.like("name%")).all()
chart_ids = [chart.id for chart in charts]
chart_ids.append(chart_with_report.id)
uri = f"api/v1/chart/?q={prison.dumps(chart_ids)}"
rv = self.client.delete(uri)
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(rv.status_code, 422)
expected_response = {
"message": "There are associated alerts or reports: report_with_chart"
}
self.assertEqual(response, expected_response)
def test_delete_chart_admin_not_owned(self):
"""
Chart API: Test admin delete not owned

71
tests/dashboards/api_tests.py
View File

@ -33,6 +33,7 @@ from sqlalchemy import and_
from superset import db, security_manager
from superset.models.dashboard import Dashboard
from superset.models.core import FavStar, FavStarClassName
from superset.models.reports import ReportSchedule, ReportScheduleType
from superset.models.slice import Slice
from superset.views.base import generate_download_headers
@ -121,6 +122,28 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin):
db.session.delete(fav_dashboard)
db.session.commit()
@pytest.fixture()
def create_dashboard_with_report(self):
with self.create_app().app_context():
admin = self.get_user("admin")
dashboard = self.insert_dashboard(
f"dashboard_report", "dashboard_report", [admin.id]
)
report_schedule = ReportSchedule(
type=ReportScheduleType.REPORT,
name="report_with_dashboard",
crontab="* * * * *",
dashboard=dashboard,
)
db.session.commit()
yield dashboard
# rollback changes
db.session.delete(report_schedule)
db.session.delete(dashboard)
db.session.commit()
def test_get_dashboard(self):
"""
Dashboard API: Test get dashboard
@ -493,6 +516,26 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin):
rv = self.client.delete(uri)
self.assertEqual(rv.status_code, 404)
@pytest.mark.usefixtures("create_dashboard_with_report")
def test_delete_dashboard_with_report(self):
"""
Dashboard API: Test delete with associated report
"""
self.login(username="admin")
dashboard = (
db.session.query(Dashboard.id)
.filter(Dashboard.dashboard_title == "dashboard_report")
.one_or_none()
)
uri = f"api/v1/dashboard/{dashboard.id}"
rv = self.client.delete(uri)
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(rv.status_code, 422)
expected_response = {
"message": "There are associated alerts or reports: report_with_dashboard"
}
self.assertEqual(response, expected_response)
def test_delete_bulk_dashboards_not_found(self):
"""
Dashboard API: Test delete bulk not found
@ -504,6 +547,34 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin):
rv = self.client.delete(uri)
self.assertEqual(rv.status_code, 404)
@pytest.mark.usefixtures("create_dashboard_with_report", "create_dashboards")
def test_bulk_delete_dashboard_with_report(self):
"""
Dashboard API: Test bulk delete with associated report
"""
self.login(username="admin")
dashboard_with_report = (
db.session.query(Dashboard.id)
.filter(Dashboard.dashboard_title == "dashboard_report")
.one_or_none()
)
dashboards = (
db.session.query(Dashboard)
.filter(Dashboard.dashboard_title.like("title%"))
.all()
)
dashboard_ids = [dashboard.id for dashboard in dashboards]
dashboard_ids.append(dashboard_with_report.id)
uri = f"api/v1/dashboard/?q={prison.dumps(dashboard_ids)}"
rv = self.client.delete(uri)
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(rv.status_code, 422)
expected_response = {
"message": "There are associated alerts or reports: report_with_dashboard"
}
self.assertEqual(response, expected_response)
def test_delete_dashboard_admin_not_owned(self):
"""
Dashboard API: Test admin delete not owned

45
tests/databases/api_tests.py
View File

@ -30,6 +30,7 @@ from sqlalchemy.sql import func
from superset import db, security_manager
from superset.connectors.sqla.models import SqlaTable
from superset.models.core import Database
from superset.models.reports import ReportSchedule, ReportScheduleType
from superset.utils.core import get_example_database, get_main_database
from tests.base_tests import SupersetTestCase
from tests.fixtures.certificates import ssl_certificate
@ -66,6 +67,30 @@ class TestDatabaseApi(SupersetTestCase):
db.session.commit()
return database
@pytest.fixture()
def create_database_with_report(self):
with self.create_app().app_context():
example_db = get_example_database()
database = self.insert_database(
"database_with_report",
example_db.sqlalchemy_uri_decrypted,
expose_in_sqllab=True,
)
report_schedule = ReportSchedule(
type=ReportScheduleType.ALERT,
name="report_with_database",
crontab="* * * * *",
database=database,
)
db.session.add(report_schedule)
db.session.commit()
yield database
# rollback changes
db.session.delete(report_schedule)
db.session.delete(database)
db.session.commit()
def test_get_items(self):
"""
Database API: Test get items
@ -486,6 +511,26 @@ class TestDatabaseApi(SupersetTestCase):
rv = self.delete_assert_metric(uri, "delete")
self.assertEqual(rv.status_code, 422)
@pytest.mark.usefixtures("create_database_with_report")
def test_delete_database_with_report(self):
"""
Database API: Test delete with associated report
"""
self.login(username="admin")
database = (
db.session.query(Database)
.filter(Database.database_name == "database_with_report")
.one_or_none()
)
uri = f"api/v1/database/{database.id}"
rv = self.client.delete(uri)
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(rv.status_code, 422)
expected_response = {
"message": "There are associated alerts or reports: report_with_database"
}
self.assertEqual(response, expected_response)
def test_get_table_metadata(self):
"""
Database API: Test get table metadata info