mirror of
https://github.com/apache/superset.git
synced 2026-04-19 08:04:53 +00:00
fix(report): fix last_eval_dttm sort and more tests (#12121)
* fix(report): fix last_eval_dttm sort and more tests * remove unnecessary permissions and split code path * remove SIP_34_ALERTS_UI * disabling an alert that is working will turn it to not triggered
This commit is contained in:
committed by
GitHub
parent
17769ca96e
commit
8d5dcc5784
@@ -157,7 +157,8 @@ class SupersetAppInitializer:
|
||||
AlertLogModelView,
|
||||
AlertModelView,
|
||||
AlertObservationModelView,
|
||||
AlertReportModelView,
|
||||
AlertView,
|
||||
ReportView,
|
||||
)
|
||||
from superset.views.annotations import (
|
||||
AnnotationLayerModelView,
|
||||
@@ -439,8 +440,17 @@ class SupersetAppInitializer:
|
||||
)
|
||||
appbuilder.add_view_no_menu(AlertLogModelView)
|
||||
appbuilder.add_view_no_menu(AlertObservationModelView)
|
||||
if feature_flag_manager.is_feature_enabled("SIP_34_ALERTS_UI"):
|
||||
appbuilder.add_view_no_menu(AlertReportModelView)
|
||||
|
||||
if feature_flag_manager.is_feature_enabled("ALERT_REPORTS"):
|
||||
appbuilder.add_view(
|
||||
AlertView,
|
||||
"Alerts & Report",
|
||||
label=__("Alerts & Reports"),
|
||||
category="Manage",
|
||||
category_label=__("Manage"),
|
||||
icon="fa-exclamation-triangle",
|
||||
)
|
||||
appbuilder.add_view_no_menu(ReportView)
|
||||
|
||||
#
|
||||
# Conditionally add Access Request Model View
|
||||
|
||||
@@ -155,6 +155,7 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi):
|
||||
"changed_on_delta_humanized",
|
||||
"created_on",
|
||||
"crontab",
|
||||
"last_eval_dttm",
|
||||
"name",
|
||||
"type",
|
||||
"crontab_humanized",
|
||||
|
||||
@@ -26,7 +26,7 @@ from superset.commands.utils import populate_owners
|
||||
from superset.dao.exceptions import DAOUpdateFailedError
|
||||
from superset.databases.dao import DatabaseDAO
|
||||
from superset.exceptions import SupersetSecurityException
|
||||
from superset.models.reports import ReportSchedule, ReportScheduleType
|
||||
from superset.models.reports import ReportSchedule, ReportScheduleType, ReportState
|
||||
from superset.reports.commands.base import BaseReportScheduleCommand
|
||||
from superset.reports.commands.exceptions import (
|
||||
DatabaseNotFoundValidationError,
|
||||
@@ -70,6 +70,16 @@ class UpdateReportScheduleCommand(BaseReportScheduleCommand):
|
||||
if not self._model:
|
||||
raise ReportScheduleNotFoundError()
|
||||
|
||||
# Change the state to not triggered when the user deactivates
|
||||
# A report that is currently in a working state. This prevents
|
||||
# an alert/report from being kept in a working state if activated back
|
||||
if (
|
||||
self._model.last_state == ReportState.WORKING
|
||||
and "active" in self._properties
|
||||
and not self._properties["active"]
|
||||
):
|
||||
self._properties["last_state"] = ReportState.NOOP
|
||||
|
||||
# Validate name uniqueness
|
||||
if not ReportScheduleDAO.validate_update_uniqueness(
|
||||
name, report_schedule_id=self._model_id
|
||||
|
||||
@@ -15,7 +15,8 @@
|
||||
# specific language governing permissions and limitations
|
||||
# under the License.
|
||||
from croniter import croniter
|
||||
from flask_appbuilder import CompactCRUDMixin
|
||||
from flask import abort
|
||||
from flask_appbuilder import CompactCRUDMixin, permission_name
|
||||
from flask_appbuilder.api import expose
|
||||
from flask_appbuilder.models.sqla.interface import SQLAInterface
|
||||
from flask_appbuilder.security.decorators import has_access
|
||||
@@ -24,14 +25,13 @@ from flask_babel import lazy_gettext as _
|
||||
from superset import is_feature_enabled
|
||||
from superset.constants import RouteMethod
|
||||
from superset.models.alerts import Alert, AlertLog, SQLObservation
|
||||
from superset.models.reports import ReportSchedule
|
||||
from superset.tasks.alerts.validator import check_validator
|
||||
from superset.typing import FlaskResponse
|
||||
from superset.utils import core as utils
|
||||
from superset.utils.core import get_email_address_str, markdown
|
||||
|
||||
from ..exceptions import SupersetException
|
||||
from .base import SupersetModelView
|
||||
from .base import BaseSupersetView, SupersetModelView
|
||||
|
||||
# TODO: access control rules for this module
|
||||
|
||||
@@ -68,37 +68,48 @@ class AlertObservationModelView(
|
||||
}
|
||||
|
||||
|
||||
class AlertReportModelView(SupersetModelView):
|
||||
datamodel = SQLAInterface(ReportSchedule)
|
||||
class BaseAlertReportView(BaseSupersetView):
|
||||
route_base = "/report"
|
||||
include_route_methods = RouteMethod.CRUD_SET | {"log"}
|
||||
class_permission_name = "ReportSchedule"
|
||||
|
||||
@expose("/list/")
|
||||
@has_access
|
||||
@permission_name("read")
|
||||
def list(self) -> FlaskResponse:
|
||||
if not (
|
||||
is_feature_enabled("ENABLE_REACT_CRUD_VIEWS")
|
||||
and is_feature_enabled("ALERT_REPORTS")
|
||||
):
|
||||
return super().list()
|
||||
return abort(404)
|
||||
|
||||
return super().render_app_template()
|
||||
|
||||
@expose("/<pk>/log/", methods=["GET"])
|
||||
@has_access
|
||||
@permission_name("read")
|
||||
def log(self, pk: int) -> FlaskResponse: # pylint: disable=unused-argument
|
||||
if not (
|
||||
is_feature_enabled("ENABLE_REACT_CRUD_VIEWS")
|
||||
and is_feature_enabled("ALERT_REPORTS")
|
||||
):
|
||||
return super().list()
|
||||
return abort(404)
|
||||
|
||||
return super().render_app_template()
|
||||
|
||||
|
||||
class AlertView(BaseAlertReportView):
|
||||
route_base = "/alert"
|
||||
class_permission_name = "ReportSchedule"
|
||||
|
||||
|
||||
class ReportView(BaseAlertReportView):
|
||||
route_base = "/report"
|
||||
class_permission_name = "ReportSchedule"
|
||||
|
||||
|
||||
class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors
|
||||
datamodel = SQLAInterface(Alert)
|
||||
route_base = "/alert"
|
||||
route_base = "/alerts"
|
||||
include_route_methods = RouteMethod.CRUD_SET | {"log"}
|
||||
|
||||
list_columns = (
|
||||
@@ -197,28 +208,6 @@ class AlertModelView(SupersetModelView): # pylint: disable=too-many-ancestors
|
||||
AlertLogModelView,
|
||||
]
|
||||
|
||||
@expose("/list/")
|
||||
@has_access
|
||||
def list(self) -> FlaskResponse:
|
||||
if not (
|
||||
is_feature_enabled("ENABLE_REACT_CRUD_VIEWS")
|
||||
and is_feature_enabled("ALERT_REPORTS")
|
||||
):
|
||||
return super().list()
|
||||
|
||||
return super().render_app_template()
|
||||
|
||||
@expose("/<pk>/log/", methods=["GET"])
|
||||
@has_access
|
||||
def log(self, pk: int) -> FlaskResponse: # pylint: disable=unused-argument
|
||||
if not (
|
||||
is_feature_enabled("ENABLE_REACT_CRUD_VIEWS")
|
||||
and is_feature_enabled("ALERT_REPORTS")
|
||||
):
|
||||
return super().list()
|
||||
|
||||
return super().render_app_template()
|
||||
|
||||
def pre_add(self, item: "AlertModelView") -> None:
|
||||
item.recipients = get_email_address_str(item.recipients)
|
||||
|
||||
|
||||
@@ -341,7 +341,7 @@ def test_deliver_alert_screenshot(
|
||||
|
||||
# TODO: fix AlertModelView.show url call from test
|
||||
url_mock.side_effect = [
|
||||
f"http://0.0.0.0:8080/alert/show/{alert.id}",
|
||||
f"http://0.0.0.0:8080/alerts/show/{alert.id}",
|
||||
f"http://0.0.0.0:8080/superset/slice/{alert.slice_id}/",
|
||||
]
|
||||
|
||||
@@ -354,7 +354,7 @@ def test_deliver_alert_screenshot(
|
||||
f"*Query*:```{alert.sql}```\n"
|
||||
f"*Result*: {alert.observations[-1].value}\n"
|
||||
f"*Reason*: {alert.observations[-1].value} {alert.pretty_config}\n"
|
||||
f"<http://0.0.0.0:8080/alert/show/{alert.id}"
|
||||
f"<http://0.0.0.0:8080/alerts/show/{alert.id}"
|
||||
f"|View Alert Details>\n<http://0.0.0.0:8080/superset/slice/{alert.slice_id}/"
|
||||
"|*Explore in Superset*>",
|
||||
"title": f"[Alert] {alert.label}",
|
||||
|
||||
@@ -17,10 +17,8 @@
|
||||
# isort:skip_file
|
||||
"""Unit tests for Superset"""
|
||||
from datetime import datetime
|
||||
from typing import List, Optional
|
||||
import json
|
||||
|
||||
from flask_appbuilder.security.sqla.models import User
|
||||
import pytest
|
||||
import prison
|
||||
from sqlalchemy.sql import func
|
||||
@@ -48,6 +46,32 @@ REPORTS_COUNT = 10
|
||||
|
||||
|
||||
class TestReportSchedulesApi(SupersetTestCase):
|
||||
@pytest.fixture()
|
||||
def create_working_report_schedule(self):
|
||||
with self.create_app().app_context():
|
||||
|
||||
admin_user = self.get_user("admin")
|
||||
alpha_user = self.get_user("alpha")
|
||||
chart = db.session.query(Slice).first()
|
||||
example_db = get_example_database()
|
||||
|
||||
report_schedule = insert_report_schedule(
|
||||
type=ReportScheduleType.ALERT,
|
||||
name=f"name_working",
|
||||
crontab=f"* * * * *",
|
||||
sql=f"SELECT value from table",
|
||||
description=f"Report working",
|
||||
chart=chart,
|
||||
database=example_db,
|
||||
owners=[admin_user, alpha_user],
|
||||
last_state=ReportState.WORKING,
|
||||
)
|
||||
|
||||
yield
|
||||
|
||||
db.session.delete(report_schedule)
|
||||
db.session.commit()
|
||||
|
||||
@pytest.fixture()
|
||||
def create_report_schedules(self):
|
||||
with self.create_app().app_context():
|
||||
@@ -260,8 +284,11 @@ class TestReportSchedulesApi(SupersetTestCase):
|
||||
"changed_on",
|
||||
"changed_on_delta_humanized",
|
||||
"created_on",
|
||||
"crontab",
|
||||
"last_eval_dttm",
|
||||
"name",
|
||||
"type",
|
||||
"crontab_humanized",
|
||||
]
|
||||
|
||||
for order_column in order_columns:
|
||||
@@ -578,6 +605,29 @@ class TestReportSchedulesApi(SupersetTestCase):
|
||||
assert updated_model.chart_id == report_schedule_data["chart"]
|
||||
assert updated_model.database_id == report_schedule_data["database"]
|
||||
|
||||
@pytest.mark.usefixtures("create_working_report_schedule")
|
||||
def test_update_report_schedule_state_working(self):
|
||||
"""
|
||||
ReportSchedule Api: Test update state in a working report
|
||||
"""
|
||||
report_schedule = (
|
||||
db.session.query(ReportSchedule)
|
||||
.filter(ReportSchedule.name == "name_working")
|
||||
.one_or_none()
|
||||
)
|
||||
|
||||
self.login(username="admin")
|
||||
report_schedule_data = {"active": False}
|
||||
uri = f"api/v1/report/{report_schedule.id}"
|
||||
rv = self.client.put(uri, json=report_schedule_data)
|
||||
assert rv.status_code == 200
|
||||
report_schedule = (
|
||||
db.session.query(ReportSchedule)
|
||||
.filter(ReportSchedule.name == "name_working")
|
||||
.one_or_none()
|
||||
)
|
||||
assert report_schedule.last_state == ReportState.NOOP
|
||||
|
||||
@pytest.mark.usefixtures("create_report_schedules")
|
||||
def test_update_report_schedule_uniqueness(self):
|
||||
"""
|
||||
@@ -864,12 +914,15 @@ class TestReportSchedulesApi(SupersetTestCase):
|
||||
"error_message",
|
||||
"end_dttm",
|
||||
"start_dttm",
|
||||
"scheduled_dttm",
|
||||
]
|
||||
|
||||
for order_column in order_columns:
|
||||
arguments = {"order_column": order_column, "order_direction": "asc"}
|
||||
uri = f"api/v1/report/{report_schedule.id}/log/?q={prison.dumps(arguments)}"
|
||||
rv = self.get_assert_metric(uri, "get_list")
|
||||
if rv.status_code == 400:
|
||||
raise Exception(json.loads(rv.data.decode("utf-8")))
|
||||
assert rv.status_code == 200
|
||||
|
||||
@pytest.mark.usefixtures("create_report_schedules")
|
||||
|
||||
@@ -38,6 +38,8 @@ from superset.models.reports import (
|
||||
)
|
||||
from superset.models.slice import Slice
|
||||
from superset.reports.commands.exceptions import (
|
||||
AlertQueryError,
|
||||
AlertQueryInvalidTypeError,
|
||||
AlertQueryMultipleColumnsError,
|
||||
AlertQueryMultipleRowsError,
|
||||
ReportScheduleNotFoundError,
|
||||
@@ -397,6 +399,41 @@ def create_mul_alert_email_chart(request):
|
||||
cleanup_report_schedule(report_schedule)
|
||||
|
||||
|
||||
@pytest.yield_fixture(params=["alert1", "alert2"])
|
||||
def create_invalid_sql_alert_email_chart(request):
|
||||
param_config = {
|
||||
"alert1": {
|
||||
"sql": "SELECT 'string' ",
|
||||
"validator_type": ReportScheduleValidatorType.OPERATOR,
|
||||
"validator_config_json": '{"op": "<", "threshold": 10}',
|
||||
},
|
||||
"alert2": {
|
||||
"sql": "SELECT first from foo_table",
|
||||
"validator_type": ReportScheduleValidatorType.OPERATOR,
|
||||
"validator_config_json": '{"op": "<", "threshold": 10}',
|
||||
},
|
||||
}
|
||||
with app.app_context():
|
||||
chart = db.session.query(Slice).first()
|
||||
example_database = get_example_database()
|
||||
with create_test_table_context(example_database):
|
||||
|
||||
report_schedule = create_report_notification(
|
||||
email_target="target@email.com",
|
||||
chart=chart,
|
||||
report_type=ReportScheduleType.ALERT,
|
||||
database=example_database,
|
||||
sql=param_config[request.param]["sql"],
|
||||
validator_type=param_config[request.param]["validator_type"],
|
||||
validator_config_json=param_config[request.param][
|
||||
"validator_config_json"
|
||||
],
|
||||
)
|
||||
yield report_schedule
|
||||
|
||||
cleanup_report_schedule(report_schedule)
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("create_report_email_chart")
|
||||
@patch("superset.reports.notifications.email.send_email_smtp")
|
||||
@patch("superset.utils.screenshots.ChartScreenshot.compute_and_cache")
|
||||
@@ -652,3 +689,15 @@ def test_email_mul_alert(create_mul_alert_email_chart):
|
||||
AsyncExecuteReportScheduleCommand(
|
||||
create_mul_alert_email_chart.id, datetime.utcnow()
|
||||
).run()
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("create_invalid_sql_alert_email_chart")
|
||||
def test_invalid_sql_alert(create_invalid_sql_alert_email_chart):
|
||||
"""
|
||||
ExecuteReport Command: Test alert with invalid SQL statements
|
||||
"""
|
||||
with freeze_time("2020-01-01T00:00:00Z"):
|
||||
with pytest.raises((AlertQueryError, AlertQueryInvalidTypeError)):
|
||||
AsyncExecuteReportScheduleCommand(
|
||||
create_invalid_sql_alert_email_chart.id, datetime.utcnow()
|
||||
).run()
|
||||
|
||||
@@ -22,7 +22,12 @@ from flask_appbuilder.security.sqla.models import User
|
||||
from superset import db
|
||||
from superset.models.core import Database
|
||||
from superset.models.dashboard import Dashboard
|
||||
from superset.models.reports import ReportExecutionLog, ReportRecipients, ReportSchedule
|
||||
from superset.models.reports import (
|
||||
ReportExecutionLog,
|
||||
ReportRecipients,
|
||||
ReportSchedule,
|
||||
ReportState,
|
||||
)
|
||||
from superset.models.slice import Slice
|
||||
|
||||
|
||||
@@ -39,6 +44,7 @@ def insert_report_schedule(
|
||||
validator_type: Optional[str] = None,
|
||||
validator_config_json: Optional[str] = None,
|
||||
log_retention: Optional[int] = None,
|
||||
last_state: Optional[ReportState] = None,
|
||||
grace_period: Optional[int] = None,
|
||||
recipients: Optional[List[ReportRecipients]] = None,
|
||||
logs: Optional[List[ReportExecutionLog]] = None,
|
||||
@@ -46,6 +52,7 @@ def insert_report_schedule(
|
||||
owners = owners or []
|
||||
recipients = recipients or []
|
||||
logs = logs or []
|
||||
last_state = last_state or ReportState.NOOP
|
||||
report_schedule = ReportSchedule(
|
||||
type=type,
|
||||
name=name,
|
||||
@@ -62,6 +69,7 @@ def insert_report_schedule(
|
||||
grace_period=grace_period,
|
||||
recipients=recipients,
|
||||
logs=logs,
|
||||
last_state=last_state,
|
||||
)
|
||||
db.session.add(report_schedule)
|
||||
db.session.commit()
|
||||
|
||||
Reference in New Issue
Block a user