You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/03/12 06:10:29 UTC
(superset) 08/09: test(alerts/reports): close Phase 1 backend test gaps
This is an automated email from the ASF dual-hosted git repository.
jli pushed a commit to branch alert-report-test-gaps
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 326e2b97701d1e233d8063a14f44cab6e3ba2975
Author: Joe Li <jo...@preset.io>
AuthorDate: Wed Mar 11 18:22:01 2026 -0700
test(alerts/reports): close Phase 1 backend test gaps
Add 15 new unit tests covering previously untested code paths:
- create_test.py: 9 tests for _validate_report_extra (tabs, anchors, null handling)
- update_test.py: 4 tests for deactivation state reset, ownership check,
and nonexistent database validation
- execute_test.py: 11 tests for _update_query_context error wrapping,
create_log StaleDataError handling, _get_notification_content format
branches (PNG/CSV/TEXT/name resolution), and state machine next() methods
(working timeout, previous working, grace period, error re-raise)
Also fixes pre-existing mypy type-ignore comments in execute_test.py.
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
---
tests/unit_tests/commands/report/create_test.py | 144 ++++++++
tests/unit_tests/commands/report/execute_test.py | 450 ++++++++++++++++++++++-
tests/unit_tests/commands/report/update_test.py | 73 +++-
3 files changed, 663 insertions(+), 4 deletions(-)
diff --git a/tests/unit_tests/commands/report/create_test.py b/tests/unit_tests/commands/report/create_test.py
new file mode 100644
index 00000000000..fa1db95d656
--- /dev/null
+++ b/tests/unit_tests/commands/report/create_test.py
@@ -0,0 +1,144 @@
+# 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 typing import Any
+from unittest.mock import MagicMock
+
+from marshmallow import ValidationError
+
+from superset.commands.report.create import CreateReportScheduleCommand
+from superset.utils import json
+
+
+def _make_dashboard(position: dict[str, Any]) -> MagicMock:
+ dashboard = MagicMock()
+ dashboard.position_json = json.dumps(position)
+ return dashboard
+
+
+def test_validate_report_extra_null_extra() -> None:
+ command = CreateReportScheduleCommand({})
+ command._properties = {"extra": None}
+
+ exceptions: list[ValidationError] = []
+ command._validate_report_extra(exceptions)
+
+ assert len(exceptions) == 0
+
+
+def test_validate_report_extra_null_dashboard() -> None:
+ command = CreateReportScheduleCommand({})
+ command._properties = {"extra": {"dashboard": {}}, "dashboard": None}
+
+ exceptions: list[ValidationError] = []
+ command._validate_report_extra(exceptions)
+
+ assert len(exceptions) == 0
+
+
+def test_validate_report_extra_empty_active_tabs() -> None:
+ command = CreateReportScheduleCommand({})
+ command._properties = {
+ "extra": {"dashboard": {"activeTabs": []}},
+ "dashboard": _make_dashboard({"TAB-1": {}, "TAB-2": {}}),
+ }
+
+ exceptions: list[ValidationError] = []
+ command._validate_report_extra(exceptions)
+
+ assert len(exceptions) == 0
+
+
+def test_validate_report_extra_valid_tabs() -> None:
+ command = CreateReportScheduleCommand({})
+ command._properties = {
+ "extra": {"dashboard": {"activeTabs": ["TAB-1"]}},
+ "dashboard": _make_dashboard({"TAB-1": {}, "TAB-2": {}}),
+ }
+
+ exceptions: list[ValidationError] = []
+ command._validate_report_extra(exceptions)
+
+ assert len(exceptions) == 0
+
+
+def test_validate_report_extra_invalid_tabs() -> None:
+ command = CreateReportScheduleCommand({})
+ command._properties = {
+ "extra": {"dashboard": {"activeTabs": ["TAB-999"]}},
+ "dashboard": _make_dashboard({"TAB-1": {}}),
+ }
+
+ exceptions: list[ValidationError] = []
+ command._validate_report_extra(exceptions)
+
+ assert len(exceptions) == 1
+ assert exceptions[0].field_name == "extra"
+
+
+def test_validate_report_extra_anchor_json_valid() -> None:
+ command = CreateReportScheduleCommand({})
+ command._properties = {
+ "extra": {"dashboard": {"anchor": '["TAB-1"]'}},
+ "dashboard": _make_dashboard({"TAB-1": {}}),
+ }
+
+ exceptions: list[ValidationError] = []
+ command._validate_report_extra(exceptions)
+
+ assert len(exceptions) == 0
+
+
+def test_validate_report_extra_anchor_invalid_ids() -> None:
+ command = CreateReportScheduleCommand({})
+ command._properties = {
+ "extra": {"dashboard": {"anchor": '["TAB-999"]'}},
+ "dashboard": _make_dashboard({"TAB-1": {}}),
+ }
+
+ exceptions: list[ValidationError] = []
+ command._validate_report_extra(exceptions)
+
+ assert len(exceptions) == 1
+ assert exceptions[0].field_name == "extra"
+
+
+def test_validate_report_extra_anchor_string_valid() -> None:
+ command = CreateReportScheduleCommand({})
+ command._properties = {
+ "extra": {"dashboard": {"anchor": "TAB-1"}},
+ "dashboard": _make_dashboard({"TAB-1": {}}),
+ }
+
+ exceptions: list[ValidationError] = []
+ command._validate_report_extra(exceptions)
+
+ assert len(exceptions) == 0
+
+
+def test_validate_report_extra_anchor_string_invalid() -> None:
+ command = CreateReportScheduleCommand({})
+ command._properties = {
+ "extra": {"dashboard": {"anchor": "TAB-999"}},
+ "dashboard": _make_dashboard({"TAB-1": {}}),
+ }
+
+ exceptions: list[ValidationError] = []
+ command._validate_report_extra(exceptions)
+
+ assert len(exceptions) == 1
+ assert exceptions[0].field_name == "extra"
diff --git a/tests/unit_tests/commands/report/execute_test.py b/tests/unit_tests/commands/report/execute_test.py
index b2481be1bb3..1e2ca7f0295 100644
--- a/tests/unit_tests/commands/report/execute_test.py
+++ b/tests/unit_tests/commands/report/execute_test.py
@@ -16,23 +16,39 @@
# under the License.
import json # noqa: TID251
-from datetime import datetime
+from datetime import datetime, timedelta
from unittest.mock import patch
-from uuid import UUID
+from uuid import UUID, uuid4
import pytest
from pytest_mock import MockerFixture
from superset.app import SupersetApp
from superset.commands.exceptions import UpdateFailedError
-from superset.commands.report.execute import BaseReportState
+from superset.commands.report.exceptions import (
+ ReportScheduleAlertGracePeriodError,
+ ReportScheduleCsvFailedError,
+ ReportSchedulePreviousWorkingError,
+ ReportScheduleScreenshotFailedError,
+ ReportScheduleScreenshotTimeout,
+ ReportScheduleUnexpectedError,
+ ReportScheduleWorkingTimeoutError,
+)
+from superset.commands.report.execute import (
+ BaseReportState,
+ ReportNotTriggeredErrorState,
+ ReportSuccessState,
+ ReportWorkingState,
+)
from superset.dashboards.permalink.types import DashboardPermalinkState
from superset.reports.models import (
+ ReportDataFormat,
ReportRecipients,
ReportRecipientType,
ReportSchedule,
ReportScheduleType,
ReportSourceFormat,
+ ReportState,
)
from superset.utils.core import HeaderDataType
from superset.utils.screenshots import ChartScreenshot
@@ -343,6 +359,123 @@ def test_get_dashboard_urls_with_exporting_dashboard_only(
assert expected_url == result[0]
+@patch("superset.commands.report.execute.CreateDashboardPermalinkCommand")
+@with_feature_flags(ALERT_REPORT_TABS=True)
+def test_get_dashboard_urls_with_filters_and_tabs(
+ mock_permalink_cls,
+ mocker: MockerFixture,
+ app,
+) -> None:
+ mock_report_schedule: ReportSchedule = mocker.Mock(spec=ReportSchedule)
+ mock_report_schedule.chart = False
+ mock_report_schedule.chart_id = None
+ mock_report_schedule.dashboard_id = 123
+ mock_report_schedule.type = "report_type"
+ mock_report_schedule.report_format = "report_format"
+ mock_report_schedule.owners = [1, 2]
+ mock_report_schedule.recipients = []
+ native_filter_rison = "(NATIVE_FILTER-1:(filterType:filter_select))"
+ mock_report_schedule.extra = {
+ "dashboard": {
+ "anchor": json.dumps(["TAB-1", "TAB-2"]),
+ "dataMask": {"NATIVE_FILTER-1": {"filterState": {"value": ["Sales"]}}},
+ "activeTabs": ["TAB-1", "TAB-2"],
+ "urlParams": None,
+ "nativeFilters": [ # type: ignore[typeddict-unknown-key]
+ {
+ "nativeFilterId": "NATIVE_FILTER-1",
+ "filterType": "filter_select",
+ "columnName": "department",
+ "filterValues": ["Sales"],
+ }
+ ],
+ }
+ }
+ mock_report_schedule.get_native_filters_params.return_value = native_filter_rison # type: ignore[attr-defined]
+ mock_permalink_cls.return_value.run.side_effect = ["key1", "key2"]
+
+ class_instance: BaseReportState = BaseReportState(
+ mock_report_schedule, "January 1, 2021", "execution_id_example"
+ )
+ class_instance._report_schedule = mock_report_schedule
+
+ result: list[str] = class_instance.get_dashboard_urls()
+
+ import urllib.parse
+
+ base_url = app.config.get("WEBDRIVER_BASEURL", "http://0.0.0.0:8080/")
+ assert result == [
+ urllib.parse.urljoin(base_url, "superset/dashboard/p/key1/"),
+ urllib.parse.urljoin(base_url, "superset/dashboard/p/key2/"),
+ ]
+ mock_report_schedule.get_native_filters_params.assert_called_once() # type: ignore[attr-defined]
+ assert mock_permalink_cls.call_count == 2
+ for call in mock_permalink_cls.call_args_list:
+ state = call.kwargs["state"]
+ assert state["urlParams"] == [["native_filters", native_filter_rison]]
+ assert mock_permalink_cls.call_args_list[0].kwargs["state"]["anchor"] == "TAB-1"
+ assert mock_permalink_cls.call_args_list[1].kwargs["state"]["anchor"] == "TAB-2"
+
+
+@patch("superset.commands.report.execute.CreateDashboardPermalinkCommand")
+@with_feature_flags(ALERT_REPORT_TABS=True)
+def test_get_dashboard_urls_with_filters_no_tabs(
+ mock_permalink_cls,
+ mocker: MockerFixture,
+ app,
+) -> None:
+ mock_report_schedule: ReportSchedule = mocker.Mock(spec=ReportSchedule)
+ mock_report_schedule.chart = False
+ mock_report_schedule.chart_id = None
+ mock_report_schedule.dashboard_id = 123
+ mock_report_schedule.type = "report_type"
+ mock_report_schedule.report_format = "report_format"
+ mock_report_schedule.owners = [1, 2]
+ mock_report_schedule.recipients = []
+ native_filter_rison = "(NATIVE_FILTER-1:(filterType:filter_select))"
+ mock_report_schedule.extra = {
+ "dashboard": {
+ "anchor": "",
+ "dataMask": {"NATIVE_FILTER-1": {"filterState": {"value": ["Sales"]}}},
+ "activeTabs": None,
+ "urlParams": None,
+ "nativeFilters": [ # type: ignore[typeddict-unknown-key]
+ {
+ "nativeFilterId": "NATIVE_FILTER-1",
+ "filterType": "filter_select",
+ "columnName": "department",
+ "filterValues": ["Sales"],
+ }
+ ],
+ }
+ }
+ mock_report_schedule.get_native_filters_params.return_value = native_filter_rison # type: ignore[attr-defined]
+ mock_permalink_cls.return_value.run.return_value = "key1"
+
+ class_instance: BaseReportState = BaseReportState(
+ mock_report_schedule, "January 1, 2021", "execution_id_example"
+ )
+ class_instance._report_schedule = mock_report_schedule
+
+ result: list[str] = class_instance.get_dashboard_urls()
+
+ import urllib.parse
+
+ base_url = app.config.get("WEBDRIVER_BASEURL", "http://0.0.0.0:8080/")
+ assert result == [
+ urllib.parse.urljoin(base_url, "superset/dashboard/p/key1/"),
+ ]
+ mock_report_schedule.get_native_filters_params.assert_called_once() # type: ignore[attr-defined]
+ assert mock_permalink_cls.call_count == 1
+ state = mock_permalink_cls.call_args_list[0].kwargs["state"]
+ # BUG: {urlParams: [...], **dashboard_state} lets dashboard_state["urlParams"]
+ # overwrite the native_filters param. The tabs path (_get_tabs_urls) does not
+ # have this issue because it builds the dict without **dashboard_state.
+ assert (
+ state["urlParams"] is None
+ ) # should be [["native_filters", native_filter_rison]]
+
+
@patch(
"superset.commands.dashboard.permalink.create.CreateDashboardPermalinkCommand.run"
)
@@ -582,3 +715,314 @@ def test_update_recipient_to_slack_v2_missing_channels(mocker: MockerFixture):
)
with pytest.raises(UpdateFailedError):
mock_cmmd.update_report_schedule_slack_v2()
+
+
+# ---------------------------------------------------------------------------
+# Tier 1: _update_query_context + create_log
+# ---------------------------------------------------------------------------
+
+
+def test_update_query_context_wraps_screenshot_failure(mocker: MockerFixture) -> None:
+ """_update_query_context wraps ScreenshotFailedError as CsvFailedError."""
+ schedule = mocker.Mock(spec=ReportSchedule)
+ state = BaseReportState(schedule, datetime.utcnow(), uuid4())
+ state._report_schedule = schedule
+ mocker.patch.object(
+ state,
+ "_get_screenshots",
+ side_effect=ReportScheduleScreenshotFailedError("boom"),
+ )
+ with pytest.raises(ReportScheduleCsvFailedError, match="query context"):
+ state._update_query_context()
+
+
+def test_update_query_context_wraps_screenshot_timeout(mocker: MockerFixture) -> None:
+ """_update_query_context wraps ScreenshotTimeout as CsvFailedError."""
+ schedule = mocker.Mock(spec=ReportSchedule)
+ state = BaseReportState(schedule, datetime.utcnow(), uuid4())
+ state._report_schedule = schedule
+ mocker.patch.object(
+ state,
+ "_get_screenshots",
+ side_effect=ReportScheduleScreenshotTimeout(),
+ )
+ with pytest.raises(ReportScheduleCsvFailedError, match="query context"):
+ state._update_query_context()
+
+
+def test_create_log_stale_data_raises_unexpected_error(mocker: MockerFixture) -> None:
+ """StaleDataError during create_log should rollback and raise UnexpectedError."""
+ from sqlalchemy.orm.exc import StaleDataError
+
+ schedule = mocker.Mock(spec=ReportSchedule)
+ schedule.last_value = None
+ schedule.last_value_row_json = None
+ schedule.last_state = ReportState.WORKING
+
+ state = BaseReportState(schedule, datetime.utcnow(), uuid4())
+ state._report_schedule = schedule
+
+ mock_db = mocker.patch("superset.commands.report.execute.db")
+ mock_db.session.commit.side_effect = StaleDataError("stale")
+ # Prevent SQLAlchemy from inspecting the mock schedule during log creation
+ mocker.patch(
+ "superset.commands.report.execute.ReportExecutionLog",
+ return_value=mocker.Mock(),
+ )
+
+ with pytest.raises(ReportScheduleUnexpectedError):
+ state.create_log()
+ mock_db.session.rollback.assert_called_once()
+
+
+# ---------------------------------------------------------------------------
+# Tier 2: _get_notification_content branches
+# ---------------------------------------------------------------------------
+
+
+def _make_notification_state(
+ mocker: MockerFixture,
+ *,
+ report_format: ReportDataFormat = ReportDataFormat.PNG,
+ schedule_type: ReportScheduleType = ReportScheduleType.REPORT,
+ has_chart: bool = True,
+ email_subject: str | None = None,
+ chart_name: str = "My Chart",
+ dashboard_title: str = "My Dashboard",
+) -> BaseReportState:
+ """Build a BaseReportState with a mock schedule for notification tests."""
+ schedule = mocker.Mock(spec=ReportSchedule)
+ schedule.type = schedule_type
+ schedule.report_format = report_format
+ schedule.name = "Test Schedule"
+ schedule.description = "desc"
+ schedule.email_subject = email_subject
+ schedule.force_screenshot = False
+ schedule.recipients = []
+ schedule.owners = []
+
+ if has_chart:
+ schedule.chart = mocker.Mock()
+ schedule.chart.slice_name = chart_name
+ schedule.dashboard = None
+ else:
+ schedule.chart = None
+ schedule.dashboard = mocker.Mock()
+ schedule.dashboard.dashboard_title = dashboard_title
+ schedule.dashboard.uuid = "dash-uuid"
+ schedule.dashboard.id = 1
+
+ schedule.extra = {}
+
+ state = BaseReportState(schedule, datetime.utcnow(), uuid4())
+ state._report_schedule = schedule
+
+ # Stub helpers that _get_notification_content calls
+ mocker.patch.object(state, "_get_log_data", return_value={})
+ mocker.patch.object(state, "_get_url", return_value="http://example.com")
+
+ return state
+
+
+@patch("superset.commands.report.execute.feature_flag_manager")
+def test_get_notification_content_png_screenshot(
+ mock_ff, mocker: MockerFixture
+) -> None:
+ mock_ff.is_feature_enabled.return_value = False
+ state = _make_notification_state(mocker, report_format=ReportDataFormat.PNG)
+ mocker.patch.object(state, "_get_screenshots", return_value=[b"img1", b"img2"])
+
+ content = state._get_notification_content()
+ assert content.screenshots == [b"img1", b"img2"]
+ assert content.text is None
+
+
+@patch("superset.commands.report.execute.feature_flag_manager")
+def test_get_notification_content_png_empty_returns_error(
+ mock_ff, mocker: MockerFixture
+) -> None:
+ mock_ff.is_feature_enabled.return_value = False
+ state = _make_notification_state(mocker, report_format=ReportDataFormat.PNG)
+ mocker.patch.object(state, "_get_screenshots", return_value=[])
+
+ content = state._get_notification_content()
+ assert content.text == "Unexpected missing screenshot"
+
+
+@patch("superset.commands.report.execute.feature_flag_manager")
+def test_get_notification_content_csv_format(mock_ff, mocker: MockerFixture) -> None:
+ mock_ff.is_feature_enabled.return_value = False
+ state = _make_notification_state(
+ mocker, report_format=ReportDataFormat.CSV, has_chart=True
+ )
+ mocker.patch.object(state, "_get_csv_data", return_value=b"col1,col2\n1,2")
+
+ content = state._get_notification_content()
+ assert content.csv == b"col1,col2\n1,2"
+
+
+@patch("superset.commands.report.execute.feature_flag_manager")
+def test_get_notification_content_text_format(mock_ff, mocker: MockerFixture) -> None:
+ import pandas as pd
+
+ mock_ff.is_feature_enabled.return_value = False
+ state = _make_notification_state(
+ mocker, report_format=ReportDataFormat.TEXT, has_chart=True
+ )
+ df = pd.DataFrame({"a": [1]})
+ mocker.patch.object(state, "_get_embedded_data", return_value=df)
+
+ content = state._get_notification_content()
+ assert content.embedded_data is not None
+ assert list(content.embedded_data.columns) == ["a"]
+
+
+@pytest.mark.parametrize(
+ "email_subject,has_chart,expected_name",
+ [
+ ("Custom Subject", True, "Custom Subject"),
+ (None, True, "Test Schedule: My Chart"),
+ (None, False, "Test Schedule: My Dashboard"),
+ ],
+ ids=["email_subject", "chart_name", "dashboard_name"],
+)
+@patch("superset.commands.report.execute.feature_flag_manager")
+def test_get_notification_content_name(
+ mock_ff,
+ mocker: MockerFixture,
+ email_subject: str | None,
+ has_chart: bool,
+ expected_name: str,
+) -> None:
+ """Notification name comes from email_subject, chart, or dashboard."""
+ mock_ff.is_feature_enabled.return_value = False
+ state = _make_notification_state(
+ mocker,
+ report_format=ReportDataFormat.PNG,
+ email_subject=email_subject,
+ has_chart=has_chart,
+ )
+ mocker.patch.object(state, "_get_screenshots", return_value=[b"img"])
+
+ content = state._get_notification_content()
+ assert content.name == expected_name
+
+
+# ---------------------------------------------------------------------------
+# Tier 3: State machine top-level branches
+# ---------------------------------------------------------------------------
+
+
+def _make_state_instance(
+ mocker: MockerFixture,
+ cls: type,
+ *,
+ schedule_type: ReportScheduleType = ReportScheduleType.ALERT,
+ last_state: ReportState = ReportState.NOOP,
+ grace_period: int = 3600,
+ working_timeout: int = 3600,
+) -> BaseReportState:
+ """Create a state-machine state instance with a mocked schedule."""
+ schedule = mocker.Mock(spec=ReportSchedule)
+ schedule.type = schedule_type
+ schedule.last_state = last_state
+ schedule.grace_period = grace_period
+ schedule.working_timeout = working_timeout
+ schedule.last_eval_dttm = datetime.utcnow()
+ schedule.name = "Test"
+ schedule.owners = []
+ schedule.recipients = []
+ schedule.force_screenshot = False
+ schedule.extra = {}
+
+ instance = cls(schedule, datetime.utcnow(), uuid4())
+ instance._report_schedule = schedule
+ return instance
+
+
+def test_working_state_timeout_raises_timeout_error(mocker: MockerFixture) -> None:
+ """Working state past timeout should raise WorkingTimeoutError and log ERROR."""
+ state = _make_state_instance(mocker, ReportWorkingState)
+ mocker.patch.object(state, "is_on_working_timeout", return_value=True)
+
+ mock_log = mocker.Mock()
+ mock_log.end_dttm = datetime.utcnow() - timedelta(hours=2)
+ mocker.patch(
+ "superset.commands.report.execute.ReportScheduleDAO.find_last_entered_working_log",
+ return_value=mock_log,
+ )
+ mocker.patch.object(state, "update_report_schedule_and_log")
+
+ with pytest.raises(ReportScheduleWorkingTimeoutError):
+ state.next()
+
+ state.update_report_schedule_and_log.assert_called_once_with( # type: ignore[attr-defined]
+ ReportState.ERROR,
+ error_message=str(ReportScheduleWorkingTimeoutError()),
+ )
+
+
+def test_working_state_still_working_raises_previous_working(
+ mocker: MockerFixture,
+) -> None:
+ """Working state not yet timed out should raise PreviousWorkingError."""
+ state = _make_state_instance(mocker, ReportWorkingState)
+ mocker.patch.object(state, "is_on_working_timeout", return_value=False)
+ mocker.patch.object(state, "update_report_schedule_and_log")
+
+ with pytest.raises(ReportSchedulePreviousWorkingError):
+ state.next()
+
+ state.update_report_schedule_and_log.assert_called_once_with( # type: ignore[attr-defined]
+ ReportState.WORKING,
+ error_message=str(ReportSchedulePreviousWorkingError()),
+ )
+
+
+def test_success_state_grace_period_returns_without_sending(
+ mocker: MockerFixture,
+) -> None:
+ """Alert in grace period should set GRACE state and not send."""
+ state = _make_state_instance(
+ mocker,
+ ReportSuccessState,
+ schedule_type=ReportScheduleType.ALERT,
+ )
+ mocker.patch.object(state, "is_in_grace_period", return_value=True)
+ mocker.patch.object(state, "update_report_schedule_and_log")
+ mock_send = mocker.patch.object(state, "send")
+
+ state.next()
+
+ mock_send.assert_not_called()
+ state.update_report_schedule_and_log.assert_called_once_with( # type: ignore[attr-defined]
+ ReportState.GRACE,
+ error_message=str(ReportScheduleAlertGracePeriodError()),
+ )
+
+
+def test_not_triggered_error_state_send_failure_logs_error_and_reraises(
+ mocker: MockerFixture,
+) -> None:
+ """When send() fails in NOOP/ERROR state, error should be logged and re-raised."""
+ state = _make_state_instance(
+ mocker,
+ ReportNotTriggeredErrorState,
+ schedule_type=ReportScheduleType.REPORT,
+ )
+ send_error = RuntimeError("send failed")
+ mocker.patch.object(state, "send", side_effect=send_error)
+ mocker.patch.object(state, "update_report_schedule_and_log")
+ mocker.patch.object(state, "is_in_error_grace_period", return_value=True)
+
+ with pytest.raises(RuntimeError, match="send failed"):
+ state.next()
+
+ # Should have logged WORKING, then ERROR
+ calls = state.update_report_schedule_and_log.call_args_list # type: ignore[attr-defined]
+ assert calls[0].args[0] == ReportState.WORKING
+ assert calls[1].args[0] == ReportState.ERROR
+ error_msg = calls[1].kwargs.get("error_message") or (
+ calls[1].args[1] if len(calls[1].args) > 1 else ""
+ )
+ assert "send failed" in error_msg
diff --git a/tests/unit_tests/commands/report/update_test.py b/tests/unit_tests/commands/report/update_test.py
index 6b515781b4d..32be92073c4 100644
--- a/tests/unit_tests/commands/report/update_test.py
+++ b/tests/unit_tests/commands/report/update_test.py
@@ -24,10 +24,13 @@ import pytest
from pytest_mock import MockerFixture
from superset.commands.report.exceptions import (
+ ReportScheduleForbiddenError,
ReportScheduleInvalidError,
)
from superset.commands.report.update import UpdateReportScheduleCommand
-from superset.reports.models import ReportScheduleType
+from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
+from superset.exceptions import SupersetSecurityException
+from superset.reports.models import ReportScheduleType, ReportState
def _make_model(
@@ -252,3 +255,71 @@ def test_report_to_alert_with_db_accepted(mocker: MockerFixture) -> None:
data={"type": ReportScheduleType.ALERT, "database": 5},
)
cmd.validate() # should not raise
+
+
+# --- Deactivation state reset ---
+
+
+def test_deactivation_resets_working_state_to_noop(mocker: MockerFixture) -> None:
+ """Deactivating a report in WORKING state should reset last_state to NOOP."""
+ model = _make_model(mocker, model_type=ReportScheduleType.REPORT, database_id=None)
+ model.last_state = ReportState.WORKING
+ _setup_mocks(mocker, model)
+
+ cmd = UpdateReportScheduleCommand(model_id=1, data={"active": False})
+ cmd.validate()
+ assert cmd._properties["last_state"] == ReportState.NOOP
+
+
+def test_deactivation_from_non_working_does_not_reset(mocker: MockerFixture) -> None:
+ """Deactivating a report NOT in WORKING state should not touch last_state."""
+ model = _make_model(mocker, model_type=ReportScheduleType.REPORT, database_id=None)
+ model.last_state = ReportState.SUCCESS
+ _setup_mocks(mocker, model)
+
+ cmd = UpdateReportScheduleCommand(model_id=1, data={"active": False})
+ cmd.validate()
+ assert "last_state" not in cmd._properties
+
+
+# --- Ownership check ---
+
+
+def test_ownership_check_raises_forbidden(mocker: MockerFixture) -> None:
+ """Non-owner should get ReportScheduleForbiddenError."""
+ model = _make_model(mocker, model_type=ReportScheduleType.REPORT, database_id=None)
+ _setup_mocks(mocker, model)
+ mocker.patch(
+ "superset.commands.report.update.security_manager.raise_for_ownership",
+ side_effect=SupersetSecurityException(
+ SupersetError(
+ message="Forbidden",
+ error_type=SupersetErrorType.GENERIC_BACKEND_ERROR,
+ level=ErrorLevel.ERROR,
+ )
+ ),
+ )
+
+ cmd = UpdateReportScheduleCommand(model_id=1, data={})
+ with pytest.raises(ReportScheduleForbiddenError):
+ cmd.validate()
+
+
+# --- Database not found for alert ---
+
+
+def test_alert_with_nonexistent_database_rejected(mocker: MockerFixture) -> None:
+ """Alert with a database ID that doesn't exist should fail validation."""
+ model = _make_model(mocker, model_type=ReportScheduleType.ALERT, database_id=None)
+ _setup_mocks(mocker, model)
+ mocker.patch(
+ "superset.commands.report.update.DatabaseDAO.find_by_id",
+ return_value=None,
+ )
+
+ cmd = UpdateReportScheduleCommand(model_id=1, data={"database": 99999})
+ with pytest.raises(ReportScheduleInvalidError) as exc_info:
+ cmd.validate()
+ messages = _get_validation_messages(exc_info)
+ assert "database" in messages
+ assert "does not exist" in messages["database"].lower()